Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add hook for EmbeddedDevice XRT devices to allow moving the PSDDR memory region #1424

Closed
wants to merge 1 commit into from

Conversation

ld-cd
Copy link

@ld-cd ld-cd commented May 27, 2023

By default the EmbeddedDevice seems to have a placeholder PSDDR region that gets added onto any other memory regions in the address map if no .xclbin file is provided. zocl of course doesn't find a reserved 256 Meg region at address 0 and falls back to the kernel CMA region which seems to be limited to 256M on most boards and is by default only 128M of which ~8M is already used by the kernel. This behavior is a tad confusing to the end user and means that only 120M of memory can by default be allocated using the default allocate region.

In addition I'm not really sure what would happen if all or near all of the CMA region were allocated and the kernel needs some more (for e.g. a framebuffer on display hotplug), but I can't imagine it would be good things and this seems difficult to debug.

This patch allows one to point the default allocation region at any reserved region that XRT is willing to allocate from. On the RFSoC 4x2 I created such a region in the bottom half of the upper two gigs of PS DDR by adding:

        reserved-memory {
                #address-cells = <2>;
                #size-cells = <2>;

                ranges;

                bufferreserved: buffer@0 {
                        no-map;

                        reg = <0x08 0x00000000 0x00 0x40000000>;
                };
        };

        zocl@0 {
                compatible = "xlnx,zocl";
                memory-region = <&bufferreserved>;
        };

To the device tree, on Alveo platforms this appears to be done by allocating hugepages on boot and using xbutil but this doesn't seem to be supported on Zynq platforms.

To use it you call the set_psddr_region(addr, size) method on an EmbeddedDevice to give it the address of a region you want to use before downloading an overlay. It does check that everything is page aligned but does not check that the region is actually in the PSDDR or that it is unused by other drivers however those checks seem like they would be better placed in zocl if they aren't already there.

It also does not check that the overlay hasn't been downloaded, is this a robust way to do that?

… allocation region to a reserved memory region

Signed-off-by: Aled Cuda <aled@ucsb.edu>
@ld-cd
Copy link
Author

ld-cd commented May 27, 2023

An alternative approach I considered was adjusting this to check for the name "PSDDR" instead of the address zero and including .xclbin file along the bitstream, but that felt like a bit of a cludge.

@skalade
Copy link
Collaborator

skalade commented May 31, 2023

Hi @ld-cd, thanks for the contribution! Looks like this'll affect quite a few boards, so we'll need some time to evaluate the changes. Thanks! Shawn

@ld-cd
Copy link
Author

ld-cd commented May 31, 2023

Yeah that's what I assumed, I've tested the change on our 4x2, will give it a test on our ZCU111

@ld-cd
Copy link
Author

ld-cd commented Sep 5, 2023

I've tested this on our ZCU111 and RFSoC4x2 and it appears to work on both;

Is there anything we can do to help get this functionality or something equivalent to it into the next PYNQ release?

@skalade
Copy link
Collaborator

skalade commented May 1, 2024

Right now pynq only supports CMA-based allocation, see device tree snippet below.

You have an alternate way of using reserved memory that we probably can't support in parallel to CMA based allocation, as we support 30+ boards with only 1 device tree each.

@skalade skalade closed this May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants