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. > 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? 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. > > > - 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.
