Hi

Am 11.03.26 um 05:55 schrieb Chen-Yu Tsai:

On Tue, Mar 10, 2026 at 4:21 PM Thomas Zimmermann [off-list ref] wrote:

quoted

Hi

Am 10.03.26 um 04:25 schrieb Chen-Yu Tsai:

quoted

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.

Not my call to make, but the driver maintainers.

What I meant was option b). Switching dma_dev for drm_dev_dma_dev() and 
replacing the mediatek code with common helpers can both introduce 
regressions.  Having two patches might make later bisecting easier.

I still think having one single patch is better.

quoted

quoted

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.

Rather send it out separately. This affects many drivers and we 
shouldn't hide such a change somewhere in an unrelated series.

Best regards
Thomas

quoted

[1]
https://elixir.bootlin.com/linux/v6.19.6/source/drivers/gpu/drm/drm_gem_dma_helper.c#L537

quoted

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

The changes look correct, so I'm here's the

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

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

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)