| Message ID | 20250307080836.42848-1-tzimmermann@suse.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm: Provide a dedicated DMA device for PRIME import | expand |
On Fri, 07 Mar 2025, Thomas Zimmermann <tzimmermann@suse.de> wrote: > Importing dma-bufs via PRIME requires a DMA-capable hardware device. > This is not the case for USB, where DMA is performed entirely by the > USB controller instead of the USB devices. > > Drivers for USB-based hardware maintain their own workarounds for this > problem. The original idea to resolve this was to provide different > PRIME helpers for such devices, but the dma-buf code internally assumes > DMA functionality as well. So that ideas is not realistic. > > Let's instead turn the current workaround into a feature. Patch 1 adds a > dma_dev field to struct drm_device and makes the PRIME code use it. Patches > 2 to 5 replace related driver code. > > It will also be useful in other code. The exynos and mediatek drivers > already maintain a dedicated DMA device for non-PRIME code. They could > likely use dma_dev as well. GEM-DMA helpers currently allocate DMA > memory with the regular parent device. They should support the dma_dev > settings as well. > > Tested with udl. I mainly reviewed the first patch, but then glanced through all the rest too, and didn't spot anything obviously wrong. So FWIW, Reviewed-by: Jani Nikula <jani.nikula@intel.com> on the rest too. > > v2: > - maintain reference on dma_dev (Jani) > - improve docs (Maxime) > - update appletbdrm > > Thomas Zimmermann (5): > drm/prime: Support dedicated DMA device for dma-buf imports > drm/appletbdrm: Set struct drm_device.dma_dev > drm/gm12u320: Set struct drm_device.dma_dev > drm/gud: Set struct drm_device.dma_dev > drm/udl: Set struct drm_device.dma_dev > > drivers/gpu/drm/drm_drv.c | 21 ++++++++++++++ > drivers/gpu/drm/drm_prime.c | 2 +- > drivers/gpu/drm/gud/gud_drv.c | 33 ++++++--------------- > drivers/gpu/drm/gud/gud_internal.h | 1 - > drivers/gpu/drm/tiny/appletbdrm.c | 27 +++++++----------- > drivers/gpu/drm/tiny/gm12u320.c | 46 +++++++++--------------------- > drivers/gpu/drm/udl/udl_drv.c | 17 ----------- > drivers/gpu/drm/udl/udl_drv.h | 1 - > drivers/gpu/drm/udl/udl_main.c | 14 ++++----- > include/drm/drm_device.h | 41 ++++++++++++++++++++++++++ > 10 files changed, 102 insertions(+), 101 deletions(-)
On Fri, Mar 07, 2025 at 09:03:58AM +0100, Thomas Zimmermann wrote: > Importing dma-bufs via PRIME requires a DMA-capable hardware device. > This is not the case for USB, where DMA is performed entirely by the > USB controller instead of the USB devices. > > Drivers for USB-based hardware maintain their own workarounds for this > problem. The original idea to resolve this was to provide different > PRIME helpers for such devices, but the dma-buf code internally assumes > DMA functionality as well. So that ideas is not realistic. So dma-buf without dma is doable, but you have to avoid dma_buf_attach. And that is a lot of surgery in the current prime helpers, since those assume that an attachment always exists. But dma-buf itself is entirely fine with cpu-only access through either userspace mmap or kernel vmap. I think as an interim step this is still good, since it makes the current hacks easier to find because at least it's all common now. -Sima > Let's instead turn the current workaround into a feature. Patch 1 adds a > dma_dev field to struct drm_device and makes the PRIME code use it. Patches > 2 to 5 replace related driver code. > > It will also be useful in other code. The exynos and mediatek drivers > already maintain a dedicated DMA device for non-PRIME code. They could > likely use dma_dev as well. GEM-DMA helpers currently allocate DMA > memory with the regular parent device. They should support the dma_dev > settings as well. > > Tested with udl. > > v2: > - maintain reference on dma_dev (Jani) > - improve docs (Maxime) > - update appletbdrm > > Thomas Zimmermann (5): > drm/prime: Support dedicated DMA device for dma-buf imports > drm/appletbdrm: Set struct drm_device.dma_dev > drm/gm12u320: Set struct drm_device.dma_dev > drm/gud: Set struct drm_device.dma_dev > drm/udl: Set struct drm_device.dma_dev > > drivers/gpu/drm/drm_drv.c | 21 ++++++++++++++ > drivers/gpu/drm/drm_prime.c | 2 +- > drivers/gpu/drm/gud/gud_drv.c | 33 ++++++--------------- > drivers/gpu/drm/gud/gud_internal.h | 1 - > drivers/gpu/drm/tiny/appletbdrm.c | 27 +++++++----------- > drivers/gpu/drm/tiny/gm12u320.c | 46 +++++++++--------------------- > drivers/gpu/drm/udl/udl_drv.c | 17 ----------- > drivers/gpu/drm/udl/udl_drv.h | 1 - > drivers/gpu/drm/udl/udl_main.c | 14 ++++----- > include/drm/drm_device.h | 41 ++++++++++++++++++++++++++ > 10 files changed, 102 insertions(+), 101 deletions(-) > > -- > 2.48.1 >
Hi Am 07.03.25 um 14:32 schrieb Simona Vetter: > On Fri, Mar 07, 2025 at 09:03:58AM +0100, Thomas Zimmermann wrote: >> Importing dma-bufs via PRIME requires a DMA-capable hardware device. >> This is not the case for USB, where DMA is performed entirely by the >> USB controller instead of the USB devices. >> >> Drivers for USB-based hardware maintain their own workarounds for this >> problem. The original idea to resolve this was to provide different >> PRIME helpers for such devices, but the dma-buf code internally assumes >> DMA functionality as well. So that ideas is not realistic. > So dma-buf without dma is doable, but you have to avoid dma_buf_attach. > And that is a lot of surgery in the current prime helpers, since those > assume that an attachment always exists. But dma-buf itself is entirely > fine with cpu-only access through either userspace mmap or kernel vmap. Right. That's roughly how far I got in this direction. The field import_attach, set up by dma_buf_attach(), is currently used throughout the DRM code and drivers. Hence this series and the other one that replaced some of the uses of import_attach. Once this has all been resolved, there will be a few users of the field left, which might be uncritical. Best regards Thomas > > I think as an interim step this is still good, since it makes the current > hacks easier to find because at least it's all common now. > -Sima > >> Let's instead turn the current workaround into a feature. Patch 1 adds a >> dma_dev field to struct drm_device and makes the PRIME code use it. Patches >> 2 to 5 replace related driver code. >> >> It will also be useful in other code. The exynos and mediatek drivers >> already maintain a dedicated DMA device for non-PRIME code. They could >> likely use dma_dev as well. GEM-DMA helpers currently allocate DMA >> memory with the regular parent device. They should support the dma_dev >> settings as well. >> >> Tested with udl. >> >> v2: >> - maintain reference on dma_dev (Jani) >> - improve docs (Maxime) >> - update appletbdrm >> >> Thomas Zimmermann (5): >> drm/prime: Support dedicated DMA device for dma-buf imports >> drm/appletbdrm: Set struct drm_device.dma_dev >> drm/gm12u320: Set struct drm_device.dma_dev >> drm/gud: Set struct drm_device.dma_dev >> drm/udl: Set struct drm_device.dma_dev >> >> drivers/gpu/drm/drm_drv.c | 21 ++++++++++++++ >> drivers/gpu/drm/drm_prime.c | 2 +- >> drivers/gpu/drm/gud/gud_drv.c | 33 ++++++--------------- >> drivers/gpu/drm/gud/gud_internal.h | 1 - >> drivers/gpu/drm/tiny/appletbdrm.c | 27 +++++++----------- >> drivers/gpu/drm/tiny/gm12u320.c | 46 +++++++++--------------------- >> drivers/gpu/drm/udl/udl_drv.c | 17 ----------- >> drivers/gpu/drm/udl/udl_drv.h | 1 - >> drivers/gpu/drm/udl/udl_main.c | 14 ++++----- >> include/drm/drm_device.h | 41 ++++++++++++++++++++++++++ >> 10 files changed, 102 insertions(+), 101 deletions(-) >> >> -- >> 2.48.1 >>
Am 10.03.25 um 10:50 schrieb Thomas Zimmermann: > Hi > > Am 07.03.25 um 14:32 schrieb Simona Vetter: >> On Fri, Mar 07, 2025 at 09:03:58AM +0100, Thomas Zimmermann wrote: >>> Importing dma-bufs via PRIME requires a DMA-capable hardware device. >>> This is not the case for USB, where DMA is performed entirely by the >>> USB controller instead of the USB devices. >>> >>> Drivers for USB-based hardware maintain their own workarounds for this >>> problem. The original idea to resolve this was to provide different >>> PRIME helpers for such devices, but the dma-buf code internally assumes >>> DMA functionality as well. So that ideas is not realistic. >> So dma-buf without dma is doable, but you have to avoid dma_buf_attach. FYI. I was referring to [1], which use the attachment to get the S/G table for import. [1] https://elixir.bootlin.com/linux/v6.13.6/source/drivers/gpu/drm/drm_prime.c#L964 >> And that is a lot of surgery in the current prime helpers, since those >> assume that an attachment always exists. But dma-buf itself is entirely >> fine with cpu-only access through either userspace mmap or kernel vmap. > > Right. That's roughly how far I got in this direction. The field > import_attach, set up by dma_buf_attach(), is currently used > throughout the DRM code and drivers. Hence this series and the other > one that replaced some of the uses of import_attach. Once this has all > been resolved, there will be a few users of the field left, which > might be uncritical. > > Best regards > Thomas > >> >> I think as an interim step this is still good, since it makes the >> current >> hacks easier to find because at least it's all common now. >> -Sima >> >>> Let's instead turn the current workaround into a feature. Patch 1 >>> adds a >>> dma_dev field to struct drm_device and makes the PRIME code use it. >>> Patches >>> 2 to 5 replace related driver code. >>> >>> It will also be useful in other code. The exynos and mediatek drivers >>> already maintain a dedicated DMA device for non-PRIME code. They could >>> likely use dma_dev as well. GEM-DMA helpers currently allocate DMA >>> memory with the regular parent device. They should support the dma_dev >>> settings as well. >>> >>> Tested with udl. >>> >>> v2: >>> - maintain reference on dma_dev (Jani) >>> - improve docs (Maxime) >>> - update appletbdrm >>> >>> Thomas Zimmermann (5): >>> drm/prime: Support dedicated DMA device for dma-buf imports >>> drm/appletbdrm: Set struct drm_device.dma_dev >>> drm/gm12u320: Set struct drm_device.dma_dev >>> drm/gud: Set struct drm_device.dma_dev >>> drm/udl: Set struct drm_device.dma_dev >>> >>> drivers/gpu/drm/drm_drv.c | 21 ++++++++++++++ >>> drivers/gpu/drm/drm_prime.c | 2 +- >>> drivers/gpu/drm/gud/gud_drv.c | 33 ++++++--------------- >>> drivers/gpu/drm/gud/gud_internal.h | 1 - >>> drivers/gpu/drm/tiny/appletbdrm.c | 27 +++++++----------- >>> drivers/gpu/drm/tiny/gm12u320.c | 46 >>> +++++++++--------------------- >>> drivers/gpu/drm/udl/udl_drv.c | 17 ----------- >>> drivers/gpu/drm/udl/udl_drv.h | 1 - >>> drivers/gpu/drm/udl/udl_main.c | 14 ++++----- >>> include/drm/drm_device.h | 41 ++++++++++++++++++++++++++ >>> 10 files changed, 102 insertions(+), 101 deletions(-) >>> >>> -- >>> 2.48.1 >>> >
Am 10.03.25 um 10:50 schrieb Thomas Zimmermann: > Hi > > Am 07.03.25 um 14:32 schrieb Simona Vetter: >> On Fri, Mar 07, 2025 at 09:03:58AM +0100, Thomas Zimmermann wrote: >>> Importing dma-bufs via PRIME requires a DMA-capable hardware device. >>> This is not the case for USB, where DMA is performed entirely by the >>> USB controller instead of the USB devices. >>> >>> Drivers for USB-based hardware maintain their own workarounds for this >>> problem. The original idea to resolve this was to provide different >>> PRIME helpers for such devices, but the dma-buf code internally assumes >>> DMA functionality as well. So that ideas is not realistic. >> So dma-buf without dma is doable, but you have to avoid dma_buf_attach. >> And that is a lot of surgery in the current prime helpers, since those >> assume that an attachment always exists. But dma-buf itself is entirely >> fine with cpu-only access through either userspace mmap or kernel vmap. > > Right. That's roughly how far I got in this direction. The field import_attach, set up by dma_buf_attach(), is currently used throughout the DRM code and drivers. Hence this series and the other one that replaced some of the uses of import_attach. Once this has all been resolved, there will be a few users of the field left, which might be uncritical. Mhm, if I remember correctly the DMA subsystem used to use the DMA mask and other parameters of the parent device when a specific device couldn't do DMA by itself. I do remember a lot of discussion about that for the DMA-buf tee driver. Going to read up on that once more. Could be that this patch here is not really necessary. Regards, Christian. > > Best regards > Thomas > >> >> I think as an interim step this is still good, since it makes the current >> hacks easier to find because at least it's all common now. >> -Sima >> >>> Let's instead turn the current workaround into a feature. Patch 1 adds a >>> dma_dev field to struct drm_device and makes the PRIME code use it. Patches >>> 2 to 5 replace related driver code. >>> >>> It will also be useful in other code. The exynos and mediatek drivers >>> already maintain a dedicated DMA device for non-PRIME code. They could >>> likely use dma_dev as well. GEM-DMA helpers currently allocate DMA >>> memory with the regular parent device. They should support the dma_dev >>> settings as well. >>> >>> Tested with udl. >>> >>> v2: >>> - maintain reference on dma_dev (Jani) >>> - improve docs (Maxime) >>> - update appletbdrm >>> >>> Thomas Zimmermann (5): >>> drm/prime: Support dedicated DMA device for dma-buf imports >>> drm/appletbdrm: Set struct drm_device.dma_dev >>> drm/gm12u320: Set struct drm_device.dma_dev >>> drm/gud: Set struct drm_device.dma_dev >>> drm/udl: Set struct drm_device.dma_dev >>> >>> drivers/gpu/drm/drm_drv.c | 21 ++++++++++++++ >>> drivers/gpu/drm/drm_prime.c | 2 +- >>> drivers/gpu/drm/gud/gud_drv.c | 33 ++++++--------------- >>> drivers/gpu/drm/gud/gud_internal.h | 1 - >>> drivers/gpu/drm/tiny/appletbdrm.c | 27 +++++++----------- >>> drivers/gpu/drm/tiny/gm12u320.c | 46 +++++++++--------------------- >>> drivers/gpu/drm/udl/udl_drv.c | 17 ----------- >>> drivers/gpu/drm/udl/udl_drv.h | 1 - >>> drivers/gpu/drm/udl/udl_main.c | 14 ++++----- >>> include/drm/drm_device.h | 41 ++++++++++++++++++++++++++ >>> 10 files changed, 102 insertions(+), 101 deletions(-) >>> >>> -- >>> 2.48.1 >>> >
Hi Am 10.03.25 um 14:56 schrieb Christian König: > Am 10.03.25 um 10:50 schrieb Thomas Zimmermann: >> Hi >> >> Am 07.03.25 um 14:32 schrieb Simona Vetter: >>> On Fri, Mar 07, 2025 at 09:03:58AM +0100, Thomas Zimmermann wrote: >>>> Importing dma-bufs via PRIME requires a DMA-capable hardware device. >>>> This is not the case for USB, where DMA is performed entirely by the >>>> USB controller instead of the USB devices. >>>> >>>> Drivers for USB-based hardware maintain their own workarounds for this >>>> problem. The original idea to resolve this was to provide different >>>> PRIME helpers for such devices, but the dma-buf code internally assumes >>>> DMA functionality as well. So that ideas is not realistic. >>> So dma-buf without dma is doable, but you have to avoid dma_buf_attach. >>> And that is a lot of surgery in the current prime helpers, since those >>> assume that an attachment always exists. But dma-buf itself is entirely >>> fine with cpu-only access through either userspace mmap or kernel vmap. >> Right. That's roughly how far I got in this direction. The field import_attach, set up by dma_buf_attach(), is currently used throughout the DRM code and drivers. Hence this series and the other one that replaced some of the uses of import_attach. Once this has all been resolved, there will be a few users of the field left, which might be uncritical. > Mhm, if I remember correctly the DMA subsystem used to use the DMA mask and other parameters of the parent device when a specific device couldn't do DMA by itself. This went away (for USB at least) with commit 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices"). Buffer sharing with USB graphics devices failed and we worked around it in 659ab7a49cbe ("drm: Use USB controller's DMA mask when importing dmabufs"). Best regards Thomas > > I do remember a lot of discussion about that for the DMA-buf tee driver. > > Going to read up on that once more. Could be that this patch here is not really necessary. > > Regards, > Christian. > >> Best regards >> Thomas >> >>> I think as an interim step this is still good, since it makes the current >>> hacks easier to find because at least it's all common now. >>> -Sima >>> >>>> Let's instead turn the current workaround into a feature. Patch 1 adds a >>>> dma_dev field to struct drm_device and makes the PRIME code use it. Patches >>>> 2 to 5 replace related driver code. >>>> >>>> It will also be useful in other code. The exynos and mediatek drivers >>>> already maintain a dedicated DMA device for non-PRIME code. They could >>>> likely use dma_dev as well. GEM-DMA helpers currently allocate DMA >>>> memory with the regular parent device. They should support the dma_dev >>>> settings as well. >>>> >>>> Tested with udl. >>>> >>>> v2: >>>> - maintain reference on dma_dev (Jani) >>>> - improve docs (Maxime) >>>> - update appletbdrm >>>> >>>> Thomas Zimmermann (5): >>>> drm/prime: Support dedicated DMA device for dma-buf imports >>>> drm/appletbdrm: Set struct drm_device.dma_dev >>>> drm/gm12u320: Set struct drm_device.dma_dev >>>> drm/gud: Set struct drm_device.dma_dev >>>> drm/udl: Set struct drm_device.dma_dev >>>> >>>> drivers/gpu/drm/drm_drv.c | 21 ++++++++++++++ >>>> drivers/gpu/drm/drm_prime.c | 2 +- >>>> drivers/gpu/drm/gud/gud_drv.c | 33 ++++++--------------- >>>> drivers/gpu/drm/gud/gud_internal.h | 1 - >>>> drivers/gpu/drm/tiny/appletbdrm.c | 27 +++++++----------- >>>> drivers/gpu/drm/tiny/gm12u320.c | 46 +++++++++--------------------- >>>> drivers/gpu/drm/udl/udl_drv.c | 17 ----------- >>>> drivers/gpu/drm/udl/udl_drv.h | 1 - >>>> drivers/gpu/drm/udl/udl_main.c | 14 ++++----- >>>> include/drm/drm_device.h | 41 ++++++++++++++++++++++++++ >>>> 10 files changed, 102 insertions(+), 101 deletions(-) >>>> >>>> -- >>>> 2.48.1 >>>>