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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Simon

On Thu, 9 Nov 2023 at 17:46, Simon Ser <contact@xxxxxxxxxxx> wrote:
>
> On Thursday, November 9th, 2023 at 16:42, Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> wrote:
>
> > > > - 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.
> >
> > It does need to scale.
> >
> > Pi5 adds 4 additional DRM devices (2xDSI, 1xDPI, and 1xComposite
> > Video), and just this last week I've been running Wayfire with TinyDRM
> > drivers for SPI displays and UDL (DisplayLink) outputs as well.
> > Presumably all of those drivers need to have appropriate hooks added
> > so they each expose a dma-heap to enable scanout buffers to be
> > allocated.
>
> I'm not sure this makes sense necessarily for all of these devices. For vc4 and
> the 4 additional RPi 5 DRM devices, probably. For the rest, e.g. UDL, I'm not
> sure it makes sense to expose scanout memory allocation via DMA heaps: AFAIK
> UDL needs CPU access to the buffers, it can't "scanout", and thus directly
> rendering via v3d to a "scanout-capable" buffer doesn't make sense. There will
> be a copy (CPU download) either way, and allocating via v3d wouldn't make a
> difference.

You as a developer may know that UDL is going to need CPU access, but
how does a generic userspace app know? Is it a case of falling back to
allocating via the renderer if there is no suitably named heap?

> > Can we add another function pointer to the struct drm_driver (or
> > similar) to do the allocation, and move the actual dmabuf handling
> > code into the core?
>
> Do you mean that the new logic introduced in this patch should be in DRM core
> to allow other drivers to more easily re-use it? Or do you mean something else?

Yes, make it easy to reuse between drivers.

> Indeed, there is nothing vc4-specific in this patch, the only requirement is
> that the driver uses drm_gem_dma_helper. So this code could be moved into (or
> alongside) that helper in DRM core. However, maybe it would be best to wait to
> have a second user for this infrastructure before we move into core.

Upstreaming of the DSI / DPI / composite drivers for Pi5 should only
be a few months away, and they can all directly scanout.

I expect the Rockchip platforms to also fall into the same category as
the Pi, with Mali as the 3D IP, and the VOP block as the scanout
engine. Have I missed some detail that means that they aren't a second
user for this?

> > > > - 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?
> >
> > That is true, but potentially vc4 could be rendering for scanout via UDL or SPI.
> > Is it easier to always have the dma-heap allocator for every DRM card
> > rather than making userspace mix and match depending on whether it is
> > all in one vs split?
>
> I don't think it's realistic to try to always make DMA heaps available for each
> and every driver which might need it from day one. It's too big of a task. And
> it's an even bigger task to try to design a fully generic heap compatibility
> uAPI from day one. I'd much rather add the heaps one by one, if and when we
> figure that it makes sense, and incrementally work our way through.

Is it really that massive a task? We have the dma heap UAPI for
handling the allocations, so what new UAPI is required?

If it's a new function pointer in struct drm_driver, then the heap is
only created by the core if that function is defined using the driver
name. The function returns a struct dma_buf *.
Any driver using drm_gem_dma_helper can use the new helper function
that is basically your vc4_dma_heap_allocate. The "if
(WARN_ON_ONCE(!vc4->is_vc5))" could be handled by not setting the
function pointer on those platforms.

Sorry, I feel I must be missing some critical piece of information here.

  Dave



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux