On Tue, Mar 10, 2026 at 4:21 PM Thomas Zimmermann <[email protected]> wrote: > > Hi > > Am 10.03.26 um 04:25 schrieb Chen-Yu Tsai: > > In commit 9b54a32c7c6a ("drm/mediatek: mtk_gem: Partial refactor and > > use drm_gem_dma_object") the MediaTek DRM driver was refactored to use > > drm_gem_dma_object, but custom callbacks were still needed to deal with > > using the first device of the pipeline as the DMA device, instead of > > the MMSYS device that the DRM driver binds to. > > > > Turns out there is already partial support for dedicated DMA devices in > > the DRM subsystem for PRIME imports. The preceding patches add support > > for dedicated DMA devices to the GEM DMA helpers. > > > > This allows us to just set the dedicated DMA device for the DRM device, > > and drop all the custom GEM callbacks. Also drop the .dma_dev field > > from the driver private data as it is no longer needed. > > My impression is that this patch should be split up: first replace > dma_dev with the dma device; then replace the now-duplicate code with > shared helpers.
I'm not sure which of the following you are thinking about: a) one patch with just the drm_dev_set_dma_dev() call, one patch dropping the custom GEM callbacks In this case the first patch has no actual effect, since nothing is using drm_dev->dma_dev. I think this is not a good split. b) one patch adding drm_dev_set_dma_dev() and replacing priv->dma_dev with drm_dev_dma_dev(), one patch dropping the custom GEM callbacks In this case the first patch would modify a bunch of places that then get dropped in the second. The churn maybe isn't worth it. c) one patch adding drm_dev_set_dma_dev() and replacing drm_gem_prime_import_dev() with drm_gem_prime_import(), one patch dropping the custom GEM callbacks Functionally the second patch would depend on the first. I still think having one single patch is better. > > There are slight differences in the mmap helper: the VM_DONTDUMP and > > VM_IO flags are no longer set. Both were lifted from drm_gem_mmap_obj(). > > VM_IO probably doesn't make sense since the buffer is allocated using > > dma_alloc_attrs(). > > We can add VM_DONTDUMP to gem-dma's mmap helper. [1] It's most likely > the right thing to not dump graphics buffers and standard practice in > most other GEM memory managers. Makes sense. I can add a patch to the series to do this. > [1] > https://elixir.bootlin.com/linux/v6.19.6/source/drivers/gpu/drm/drm_gem_dma_helper.c#L537 > > > > > Signed-off-by: Chen-Yu Tsai <[email protected]> > > The changes look correct, so I'm here's the > > Reviewed-by: Thomas Zimmermann <[email protected]> > > But this patch requires a review from the driver maintainer as well. Sure, and I need to respin the series because I forgot to remove the deleted file from the Makefile. Thanks ChenYu
