[RFC PATCH 2/2] vc4: introduce DMA-BUF heap

[Simon Ser]

On Thursday, November 9th, 2023 at 10:11, Maxime Ripard <mripard at kernel.org> 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.