[PATCH] drm/gem: Internally test import_attach for imported objects

[Christian König]

Am 15.04.25 um 15:14 schrieb Thomas Zimmermann:
>>
>>> The long-term goal is to make import_attach optional because its setup requires a DMA-capable device.
>> HUI WHAT?
>>
>> Dmitry and I put quite some effort into being able to create an import_attach without the requirement to have a DMA-capable device.
>>
>> The last puzzle piece of that landed a month ago in the form of this patch here:
>>
>> commit b72f66f22c0e39ae6684c43fead774c13db24e73
>> Author: Christian König <christian.koenig at amd.com>
>> Date:   Tue Feb 11 17:20:53 2025 +0100
>>
>>      dma-buf: drop caching of sg_tables
>>           That was purely for the transition from static to dynamic dma-buf
>>      handling and can be removed again now.
>>           Signed-off-by: Christian König <christian.koenig at amd.com>
>>      Reviewed-by: Simona Vetter <simona.vetter at ffwll.ch>
>>      Reviewed-by: Dmitry Osipenko <dmitry.osipenko at collabora.com>
>>      Link: https://patchwork.freedesktop.org/patch/msgid/20250211163109.12200-5-christian.koenig@amd.com
>>
>> When you don't create an import attachment the exporter wouldn't know that his buffer is actually used which is usually a quite bad idea.
>>
>> This is for devices who only want to do a vmap of the buffer, isn't it?
>
> Yeah, the vmap needs the sgtable, which needs import_attach IIRC. Somewhere in there a DMA device is required. But it's not of high priority as we have workarounds.

I've removed the need to create an sgtable just to create an import_attach.

The crux is that exporters sometimes need to distinct between the case when DMA-buf is just used for inter process passing of buffers and inter device passing of buffers. Usually we use the list of attachments for that.

Because of this I though of changing the dma_buf_vmap functions to take an attachment instead of an dma_buf as parameter. That would also resolve the problem is making sure to signal buffer movement through the move notifier.

BTW: Where do we currently pin the buffers to make sure that the pointers returned by dma_buf_vmap() stays valid after dropping the reservation lock?

Regards,
Christian.

>
> Best regards
> Thomas
>
>>
>> Regards,
>> Christian.
>>
>>> Best regards
>>> Thomas
>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>> Best regards
>>>>> Thomas
>>>>>
>>>>>> Regards,
>>>>>> Christian.
>>>>>>
>>>>>>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>>>>>>> Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper")
>>>>>>> Reported-by: Andy Yan <andyshrk at 163.com>
>>>>>>> Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andyshrk@163.com/
>>>>>>> Tested-by: Andy Yan <andyshrk at 163.com>
>>>>>>> Cc: Thomas Zimmermann <tzimmermann at suse.de>
>>>>>>> Cc: Anusha Srivatsa <asrivats at redhat.com>
>>>>>>> Cc: Christian König <christian.koenig at amd.com>
>>>>>>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>>>>>> Cc: Maxime Ripard <mripard at kernel.org>
>>>>>>> Cc: David Airlie <airlied at gmail.com>
>>>>>>> Cc: Simona Vetter <simona at ffwll.ch>
>>>>>>> Cc: Sumit Semwal <sumit.semwal at linaro.org>
>>>>>>> Cc: "Christian König" <christian.koenig at amd.com>
>>>>>>> Cc: dri-devel at lists.freedesktop.org
>>>>>>> Cc: linux-media at vger.kernel.org
>>>>>>> Cc: linaro-mm-sig at lists.linaro.org
>>>>>>> ---
>>>>>>>     include/drm/drm_gem.h | 8 +++++++-
>>>>>>>     1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
>>>>>>> index 9b71f7a9f3f8..f09b8afcf86d 100644
>>>>>>> --- a/include/drm/drm_gem.h
>>>>>>> +++ b/include/drm/drm_gem.h
>>>>>>> @@ -589,7 +589,13 @@ static inline bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_obje
>>>>>>>     static inline bool drm_gem_is_imported(const struct drm_gem_object *obj)
>>>>>>>     {
>>>>>>>         /* The dma-buf's priv field points to the original GEM object. */
>>>>>>> -    return obj->dma_buf && (obj->dma_buf->priv != obj);
>>>>>>> +    return (obj->dma_buf && (obj->dma_buf->priv != obj)) ||
>>>>>>> +           /*
>>>>>>> +        * TODO: During object release, the dma-buf might already
>>>>>>> +        *       be gone. For now keep testing import_attach, but
>>>>>>> +        *       this should be removed at some point.
>>>>>>> +        */
>>>>>>> +           obj->import_attach;
>>>>>>>     }
>>>>>>>       #ifdef CONFIG_LOCKDEP
>