On Mon, Feb 22, 2021 at 05:25:46PM +0100, Thomas Zimmermann wrote: > Hi > > Am 22.02.21 um 17:10 schrieb Daniel Vetter: > > On Mon, Feb 22, 2021 at 2:24 PM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote: > > > > > > Hi > > > > > > Am 22.02.21 um 14:09 schrieb Christian König: > > > > > > > > > > > > Am 22.02.21 um 13:43 schrieb Thomas Zimmermann: > > > > > USB-based drivers cannot use DMA, so the importing of dma-buf attachments > > > > > currently fails for udl and gm12u320. This breaks joining/mirroring of > > > > > displays. > > > > > > > > > > The fix is now a little series. To solve the issue on the importer > > > > > side (i.e., the affected USB-based driver), patch 1 introduces a new > > > > > PRIME callback, struct drm_driver.gem_prime_create_object, which creates > > > > > an object and gives more control to the importing driver. Specifically, > > > > > udl and gm12u320 can now avoid the creation of a scatter/gather table > > > > > for the imported pages. Patch 1 is self-contained in the sense that it > > > > > can be backported into older kernels. > > > > > > > > Mhm, that sounds like a little overkill to me. > > > > > > > > Drivers can already import the DMA-bufs all by them selves without the > > > > help of the DRM functions. See amdgpu for an example. > > > > > > > > Daniel also already noted to me that he sees the DRM helper as a bit > > > > questionable middle layer. > > > > > > And this bug proves that it is. :) > > > > The trouble here is actually gem_bo->import_attach, which isn't really > > part of the questionable midlayer, but fairly mandatory (only > > exception is vmwgfx because not using gem) caching to make sure we > > don't end up with duped imports and fun stuff like that. > > > > And dma_buf_attach now implicitly creates the sg table already, so > > we're already in game over land. I think we'd need to make > > import_attach a union with import_buf or something like that, so that > > you can do attachment-less importing. > > Creating the sg table is not the problem; mapping it is. So dma_buf_attach > shouldn't be a problem. dma_buf_attach will create a cached sg-mapping for you if the exporter is dynamic. Currently that's only the case for amdgpu, I guess you didn't test with that. So yeah dma_buf_attach is a problem already. And if we can't attach, the entire obj->import_attach logic in drm_prime.c falls over, and we get all kinds of fun with double import and re-export. > > > > Have you thought about doing that instead? > > > > > > There appears to be some useful code in drm_gem_prime_import_dev(). But > > > if the general sentiment goes towards removing > > > gem_prime_import_sg_table, we can work towards that as well. > > > > I still think this part is a bit a silly midlayer for no good reason, > > but I think that's orthogonal to the issue at hand here. > > > > I'd suggest we first try to paper over the issue by using > > prime_import_dev with the host controller (which hopefully is > > dma-capable for most systems). And then, at leisure, try to untangle > > the obj->import_attach issue. > > I really don't want to do this. My time is also limited, and I''ll spend > time papering over the thing. And then more time for the real fix. I'd > rather pull drm_gem_prime_import_dev() in to USB drivers and avoid the > dma_buf_map(). Yeah I understand, it's just (as usual :-/) more complex than it seems ... -Daniel > > Best regard > Thomas > > > -Daniel > > > > > > > > Best regards > > > Thomas > > > > > > > > > > > Christian. > > > > > > > > > > > > > > Patches 2 and 3 update SHMEM and CMA helpers to use the new callback. > > > > > Effectively this moves the sg table setup from the PRIME helpers into > > > > > the memory managers. SHMEM now supports devices without DMA support, > > > > > so custom code can be removed from udl and g12u320. > > > > > > > > > > Tested by joining/mirroring displays of udl and radeon under Gnome/X11. > > > > > > > > > > v2: > > > > > * move fix to importer side (Christian, Daniel) > > > > > * update SHMEM and CMA helpers for new PRIME callbacks > > > > > > > > > > Thomas Zimmermann (3): > > > > > drm: Support importing dmabufs into drivers without DMA > > > > > drm/shmem-helper: Implement struct drm_driver.gem_prime_create_object > > > > > drm/cma-helper: Implement struct drm_driver.gem_prime_create_object > > > > > > > > > > drivers/gpu/drm/drm_gem_cma_helper.c | 62 ++++++++++++++----------- > > > > > drivers/gpu/drm/drm_gem_shmem_helper.c | 38 ++++++++++----- > > > > > drivers/gpu/drm/drm_prime.c | 43 +++++++++++------ > > > > > drivers/gpu/drm/lima/lima_drv.c | 2 +- > > > > > drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- > > > > > drivers/gpu/drm/panfrost/panfrost_gem.c | 6 +-- > > > > > drivers/gpu/drm/panfrost/panfrost_gem.h | 4 +- > > > > > drivers/gpu/drm/pl111/pl111_drv.c | 8 ++-- > > > > > drivers/gpu/drm/v3d/v3d_bo.c | 6 +-- > > > > > drivers/gpu/drm/v3d/v3d_drv.c | 2 +- > > > > > drivers/gpu/drm/v3d/v3d_drv.h | 5 +- > > > > > include/drm/drm_drv.h | 12 +++++ > > > > > include/drm/drm_gem_cma_helper.h | 12 ++--- > > > > > include/drm/drm_gem_shmem_helper.h | 6 +-- > > > > > 14 files changed, 120 insertions(+), 88 deletions(-) > > > > > > > > > > -- > > > > > 2.30.1 > > > > > > > > > > > > > > > -- > > > Thomas Zimmermann > > > Graphics Driver Developer > > > SUSE Software Solutions Germany GmbH > > > Maxfeldstr. 5, 90409 Nürnberg, Germany > > > (HRB 36809, AG Nürnberg) > > > Geschäftsführer: Felix Imendörffer > > > > > > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
