On Thursday, November 9th, 2023 at 10:11, Maxime Ripard <mripard@xxxxxxxxxx> wrote: > > - Does this approach make sense to y'all in general? > > Makes sense to me :) Great to hear! > > - What would be a good name for the heap? "vc4" is maybe a bit naive and > > not precise enough. Something with "cma"? Do we need to plan a naming > > scheme to accomodate for multiple vc4 devices? > > That's a general issue though that happens with pretty much all devices > with a separate node for modesetting and rendering, so I don't think > addressing it only for vc4 make sense, we should make it generic. > > So maybe something like "scanout"? > > One thing we need to consider too is that the Pi5 will have multiple > display nodes (4(?) iirc) with different capabilities, vc4 being only > one of them. This will impact that solution too. I'm not sure trying to find a unique generic name for all split render/display SoC is a good idea: - For the purposes of replacing DRM dumb buffers usage from v3d, we don't actually need a generic name, it's perfectly fine to hardcode a name here since vc4 is pretty much hardcoded there already. - As you said, "scanout" may be ill-defined depending on the system. What if a PCIe or USB device is plugged in? What if vkms is loaded? What if there are multiple platform display devices? What does "scanout" mean then? - A generic solution to advertise what DMA heaps a DRM display device is compatible with is probably better addressed by populating sysfs with new files. We've talked with Sima at XDC about adding a symlink pointing to the DMA heap and extra metadata files describing priorities and such. However we don't actually need that part for the purposes of v3d -- I think I'd rather defer that until more DMA heaps are plumbed across the DRM drivers. So I'd be in favor of using something descriptive, platform-specific for the heap name, something that user-space can't generically try to interpret or parse. > > - Right now root privileges are necessary to open the heap. Should we > > allow everybody to open the heap by default (after all, user-space can > > already allocate arbitrary amounts of GPU memory)? Should we leave it > > up to user-space to solve this issue (via logind/seatd or a Wayland > > protocol or something else)? > > I would have expected a udev rule to handle that? Right. That sounds fine to me. (It's not the end of the world if v3d can't allocate scanout-capable memory if the heap is not accessible -- just means we will go through an extra copy in the compositor. And we can always keep the DRM legacy primary node stuff as a fallback if there really are concerns.) > > TODO: > > > > - Need to add !vc5 support. > > If by !vc5 you mean RPi0-3, then it's probably not needed there at all > since it has a single node for both modesetting and rendering? Ah that's a good point. So we can just not expose any heap for !vc5.