Test struct drm_gem_object.import_attach to detect imported objects during cleanup. At that point, the imported DMA buffer might have already been released and the dma_buf field is NULL. The object's free callback then does a cleanup as for native objects.
Happens for calls to drm_mode_destroy_dumb_ioctl() that eventually clear the dma_buf field in drm_gem_object_exported_dma_buf_free().
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan andyshrk@163.com Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andyshr... Tested-by: Andy Yan andyshrk@163.com Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Anusha Srivatsa asrivats@redhat.com Cc: Christian König christian.koenig@amd.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: David Airlie airlied@gmail.com Cc: Simona Vetter simona@ffwll.ch Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Christian König" christian.koenig@amd.com Cc: dri-devel@lists.freedesktop.org Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@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
Am 15.04.25 um 11:20 schrieb Thomas Zimmermann:
Test struct drm_gem_object.import_attach to detect imported objects during cleanup. At that point, the imported DMA buffer might have already been released and the dma_buf field is NULL. The object's free callback then does a cleanup as for native objects.
I don't think that this is a good idea.
The DMA-buf is separately reference counted through the import attachment.
Happens for calls to drm_mode_destroy_dumb_ioctl() that eventually clear the dma_buf field in drm_gem_object_exported_dma_buf_free().
That is for exported DMA-buf and should *NEVER* be used for imported ones.
Regards, Christian.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan andyshrk@163.com Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andyshr... Tested-by: Andy Yan andyshrk@163.com Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Anusha Srivatsa asrivats@redhat.com Cc: Christian König christian.koenig@amd.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: David Airlie airlied@gmail.com Cc: Simona Vetter simona@ffwll.ch Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Christian König" christian.koenig@amd.com Cc: dri-devel@lists.freedesktop.org Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@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
Hi
Am 15.04.25 um 11:39 schrieb Christian König:
Am 15.04.25 um 11:20 schrieb Thomas Zimmermann:
Test struct drm_gem_object.import_attach to detect imported objects during cleanup. At that point, the imported DMA buffer might have already been released and the dma_buf field is NULL. The object's free callback then does a cleanup as for native objects.
I don't think that this is a good idea.
The DMA-buf is separately reference counted through the import attachment.
I understand that it's not ideal, but testing for import_attach to be set is what we currently do throughout drivers. Putting this behind an interface is already a step forward.
Happens for calls to drm_mode_destroy_dumb_ioctl() that eventually clear the dma_buf field in drm_gem_object_exported_dma_buf_free().
That is for exported DMA-buf and should *NEVER* be used for imported ones.
Did you look at the discussion at the Closes tag? Where else could dma-buf be cleared?
Best regards Thomas
Regards, Christian.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan andyshrk@163.com Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andyshr... Tested-by: Andy Yan andyshrk@163.com Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Anusha Srivatsa asrivats@redhat.com Cc: Christian König christian.koenig@amd.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: David Airlie airlied@gmail.com Cc: Simona Vetter simona@ffwll.ch Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Christian König" christian.koenig@amd.com Cc: dri-devel@lists.freedesktop.org Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@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
Am 15.04.25 um 12:45 schrieb Thomas Zimmermann:
Hi
Am 15.04.25 um 11:39 schrieb Christian König:
Am 15.04.25 um 11:20 schrieb Thomas Zimmermann:
Test struct drm_gem_object.import_attach to detect imported objects during cleanup. At that point, the imported DMA buffer might have already been released and the dma_buf field is NULL. The object's free callback then does a cleanup as for native objects.
I don't think that this is a good idea.
The DMA-buf is separately reference counted through the import attachment.
I understand that it's not ideal, but testing for import_attach to be set is what we currently do throughout drivers. Putting this behind an interface is already a step forward.
Happens for calls to drm_mode_destroy_dumb_ioctl() that eventually clear the dma_buf field in drm_gem_object_exported_dma_buf_free().
That is for exported DMA-buf and should *NEVER* be used for imported ones.
Did you look at the discussion at the Closes tag? Where else could dma-buf be cleared?
Yeah, I've seen that. The solution is just completely wrong.
See for the export case obj->dma_buf points to the exported DMA-buf and causes a circle dependency when not set to NULL when the last handle is released.
But for the import case obj->dma_buf is actually not that relevant. Instead obj->import_attach->dma_buf should be used.
Regards, Christian.
Best regards Thomas
Regards, Christian.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan andyshrk@163.com Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andyshr... Tested-by: Andy Yan andyshrk@163.com Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Anusha Srivatsa asrivats@redhat.com Cc: Christian König christian.koenig@amd.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: David Airlie airlied@gmail.com Cc: Simona Vetter simona@ffwll.ch Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Christian König" christian.koenig@amd.com Cc: dri-devel@lists.freedesktop.org Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@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
Hi
Am 15.04.25 um 14:19 schrieb Christian König:
Am 15.04.25 um 12:45 schrieb Thomas Zimmermann:
Hi
Am 15.04.25 um 11:39 schrieb Christian König:
Am 15.04.25 um 11:20 schrieb Thomas Zimmermann:
Test struct drm_gem_object.import_attach to detect imported objects during cleanup. At that point, the imported DMA buffer might have already been released and the dma_buf field is NULL. The object's free callback then does a cleanup as for native objects.
I don't think that this is a good idea.
The DMA-buf is separately reference counted through the import attachment.
I understand that it's not ideal, but testing for import_attach to be set is what we currently do throughout drivers. Putting this behind an interface is already a step forward.
Happens for calls to drm_mode_destroy_dumb_ioctl() that eventually clear the dma_buf field in drm_gem_object_exported_dma_buf_free().
That is for exported DMA-buf and should *NEVER* be used for imported ones.
Did you look at the discussion at the Closes tag? Where else could dma-buf be cleared?
Yeah, I've seen that. The solution is just completely wrong.
See for the export case obj->dma_buf points to the exported DMA-buf and causes a circle dependency when not set to NULL when the last handle is released.
But for the import case obj->dma_buf is actually not that relevant. Instead obj->import_attach->dma_buf should be used.
So if I understand correctly, the tests in that helper should be done by looking at import_attach->dma_buf.
The long-term goal is to make import_attach optional because its setup requires a DMA-capable device.
Best regards Thomas
Regards, Christian.
Best regards Thomas
Regards, Christian.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan andyshrk@163.com Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andyshr... Tested-by: Andy Yan andyshrk@163.com Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Anusha Srivatsa asrivats@redhat.com Cc: Christian König christian.koenig@amd.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: David Airlie airlied@gmail.com Cc: Simona Vetter simona@ffwll.ch Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Christian König" christian.koenig@amd.com Cc: dri-devel@lists.freedesktop.org Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@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
Am 15.04.25 um 14:40 schrieb Thomas Zimmermann:
Hi
Am 15.04.25 um 14:19 schrieb Christian König:
Am 15.04.25 um 12:45 schrieb Thomas Zimmermann:
Hi
Am 15.04.25 um 11:39 schrieb Christian König:
Am 15.04.25 um 11:20 schrieb Thomas Zimmermann:
Test struct drm_gem_object.import_attach to detect imported objects during cleanup. At that point, the imported DMA buffer might have already been released and the dma_buf field is NULL. The object's free callback then does a cleanup as for native objects.
I don't think that this is a good idea.
The DMA-buf is separately reference counted through the import attachment.
I understand that it's not ideal, but testing for import_attach to be set is what we currently do throughout drivers. Putting this behind an interface is already a step forward.
Happens for calls to drm_mode_destroy_dumb_ioctl() that eventually clear the dma_buf field in drm_gem_object_exported_dma_buf_free().
That is for exported DMA-buf and should *NEVER* be used for imported ones.
Did you look at the discussion at the Closes tag? Where else could dma-buf be cleared?
Yeah, I've seen that. The solution is just completely wrong.
See for the export case obj->dma_buf points to the exported DMA-buf and causes a circle dependency when not set to NULL when the last handle is released.
But for the import case obj->dma_buf is actually not that relevant. Instead obj->import_attach->dma_buf should be used.
So if I understand correctly, the tests in that helper should be done by looking at import_attach->dma_buf.
At least in theory yes. IIRC we also set obj->dma_buf to the same value as import_attach->dma_buf for convenient so that code doesn't need to check both.
But it can be that obj->dma_buf is already NULL while the import attachment is still in the process of being cleaned up. So there are a couple of cases where we have to look at obj->import_attach->dma_buf.
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@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@amd.com Reviewed-by: Simona Vetter simona.vetter@ffwll.ch Reviewed-by: Dmitry Osipenko dmitry.osipenko@collabora.com Link: https://patchwork.freedesktop.org/patch/msgid/20250211163109.12200-5-christi...
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?
Regards, Christian.
Best regards Thomas
Regards, Christian.
Best regards Thomas
Regards, Christian.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan andyshrk@163.com Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andyshr... Tested-by: Andy Yan andyshrk@163.com Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Anusha Srivatsa asrivats@redhat.com Cc: Christian König christian.koenig@amd.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: David Airlie airlied@gmail.com Cc: Simona Vetter simona@ffwll.ch Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Christian König" christian.koenig@amd.com Cc: dri-devel@lists.freedesktop.org Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@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
Hi
Am 15.04.25 um 14:52 schrieb Christian König:
Am 15.04.25 um 14:40 schrieb Thomas Zimmermann:
Hi
Am 15.04.25 um 14:19 schrieb Christian König:
Am 15.04.25 um 12:45 schrieb Thomas Zimmermann:
Hi
Am 15.04.25 um 11:39 schrieb Christian König:
Am 15.04.25 um 11:20 schrieb Thomas Zimmermann:
Test struct drm_gem_object.import_attach to detect imported objects during cleanup. At that point, the imported DMA buffer might have already been released and the dma_buf field is NULL. The object's free callback then does a cleanup as for native objects.
I don't think that this is a good idea.
The DMA-buf is separately reference counted through the import attachment.
I understand that it's not ideal, but testing for import_attach to be set is what we currently do throughout drivers. Putting this behind an interface is already a step forward.
Happens for calls to drm_mode_destroy_dumb_ioctl() that eventually clear the dma_buf field in drm_gem_object_exported_dma_buf_free().
That is for exported DMA-buf and should *NEVER* be used for imported ones.
Did you look at the discussion at the Closes tag? Where else could dma-buf be cleared?
Yeah, I've seen that. The solution is just completely wrong.
See for the export case obj->dma_buf points to the exported DMA-buf and causes a circle dependency when not set to NULL when the last handle is released.
But for the import case obj->dma_buf is actually not that relevant. Instead obj->import_attach->dma_buf should be used.
So if I understand correctly, the tests in that helper should be done by looking at import_attach->dma_buf.
At least in theory yes. IIRC we also set obj->dma_buf to the same value as import_attach->dma_buf for convenient so that code doesn't need to check both.
But it can be that obj->dma_buf is already NULL while the import attachment is still in the process of being cleaned up. So there are a couple of cases where we have to look at obj->import_attach->dma_buf.
Ok, that should also solve the problem for now. The point here is to limit exposure of import_attach.
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@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@amd.com Reviewed-by: Simona Vetter simona.vetter@ffwll.ch Reviewed-by: Dmitry Osipenko dmitry.osipenko@collabora.com Link: https://patchwork.freedesktop.org/patch/msgid/20250211163109.12200-5-christi...
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.
Best regards Thomas
Regards, Christian.
Best regards Thomas
Regards, Christian.
Best regards Thomas
Regards, Christian.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan andyshrk@163.com Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andyshr... Tested-by: Andy Yan andyshrk@163.com Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Anusha Srivatsa asrivats@redhat.com Cc: Christian König christian.koenig@amd.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: David Airlie airlied@gmail.com Cc: Simona Vetter simona@ffwll.ch Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Christian König" christian.koenig@amd.com Cc: dri-devel@lists.freedesktop.org Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@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
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@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@amd.com Reviewed-by: Simona Vetter simona.vetter@ffwll.ch Reviewed-by: Dmitry Osipenko dmitry.osipenko@collabora.com Link: https://patchwork.freedesktop.org/patch/msgid/20250211163109.12200-5-christi...
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@suse.de > Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") > Reported-by: Andy Yan andyshrk@163.com > Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andyshr... > Tested-by: Andy Yan andyshrk@163.com > Cc: Thomas Zimmermann tzimmermann@suse.de > Cc: Anusha Srivatsa asrivats@redhat.com > Cc: Christian König christian.koenig@amd.com > Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com > Cc: Maxime Ripard mripard@kernel.org > Cc: David Airlie airlied@gmail.com > Cc: Simona Vetter simona@ffwll.ch > Cc: Sumit Semwal sumit.semwal@linaro.org > Cc: "Christian König" christian.koenig@amd.com > Cc: dri-devel@lists.freedesktop.org > Cc: linux-media@vger.kernel.org > Cc: linaro-mm-sig@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
linaro-mm-sig@lists.linaro.org
-
Christian König -
Thomas Zimmermann