On 17/03/2025 13:06, Thomas Zimmermann wrote:
> Instead of testing import_attach for imported GEM buffers, invoke
> drm_gem_is_imported() to do the test. The helper tests the dma_buf
> itself while import_attach is just an artifact of the import. Prepares
> to make import_attach optional.
>
> Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
> Cc: Boris Brezillon <boris.brezil...@collabora.com>
> Cc: Steven Price <steven.pr...@arm.com>
> Cc: Liviu Dudau <liviu.du...@arm.com>
> ---
> drivers/gpu/drm/panthor/panthor_gem.c | 2 +-
> drivers/gpu/drm/panthor/panthor_mmu.c | 10 +++++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.c
> b/drivers/gpu/drm/panthor/panthor_gem.c
> index 8244a4e6c2a2..fd014ccc3bfc 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.c
> +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> @@ -155,7 +155,7 @@ static enum drm_gem_object_status
> panthor_gem_status(struct drm_gem_object *obj)
> struct panthor_gem_object *bo = to_panthor_bo(obj);
> enum drm_gem_object_status res = 0;
>
> - if (bo->base.base.import_attach || bo->base.pages)
> + if (drm_gem_is_imported(&bo->base.base) || bo->base.pages)
> res |= DRM_GEM_OBJECT_RESIDENT;
>
> return res;
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c
> b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 12a02e28f50f..3e123159ac10 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -1103,7 +1103,7 @@ static void panthor_vm_bo_put(struct drm_gpuvm_bo
> *vm_bo)
> /* If the vm_bo object was destroyed, release the pin reference that
> * was hold by this object.
> */
> - if (unpin && !bo->base.base.import_attach)
> + if (unpin && !drm_gem_is_imported(&bo->base.base))
> drm_gem_shmem_unpin(&bo->base);
I'm seeing issues on cleanup where drm_gem_is_imported() doesn't return
the same as !!import_attach in the above code. Specifically this appears
to be caused by drm_gem_object_exported_dma_buf_free() setting ->dma_buf
to NULL which makes the BO look like it isn't imported.
Stashing the imported state in the BO fixes the problem (see below
hack), but it would be nice to fix this more generally in case there are
other drivers that need to know the imported state during cleanup.
Any suggestions for how drm_gem_is_imported() can more accurately report
the state during cleanup?
Thanks,
Steve
diff --git a/drivers/gpu/drm/panthor/panthor_gem.h
b/drivers/gpu/drm/panthor/panthor_gem.h
index 4641994ddd7f..160facc67b23 100644
--- a/drivers/gpu/drm/panthor/panthor_gem.h
+++ b/drivers/gpu/drm/panthor/panthor_gem.h
@@ -97,6 +97,9 @@ struct panthor_gem_object {
/** @flags: Combination of drm_panthor_bo_flags flags. */
u32 flags;
+ /** @imported: Has this BO been imported? */
+ bool imported;
+
/**
* @label: BO tagging fields. The label can be assigned within the
* driver itself or through a specific IOCTL.
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c
b/drivers/gpu/drm/panthor/panthor_mmu.c
index 6ca9a2642a4e..f6c8cdc867be 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -1104,7 +1104,7 @@ static void panthor_vm_bo_put(struct drm_gpuvm_bo *vm_bo)
/* If the vm_bo object was destroyed, release the pin reference that
* was hold by this object.
*/
- if (unpin && !drm_gem_is_imported(&bo->base.base))
+ if (unpin && !bo->imported)
drm_gem_shmem_unpin(&bo->base);
drm_gpuvm_put(vm);
@@ -1235,7 +1235,8 @@ static int panthor_vm_prepare_map_op_ctx(struct
panthor_vm_op_ctx *op_ctx,
if (ret)
goto err_cleanup;
- if (!drm_gem_is_imported(&bo->base.base)) {
+ bo->imported = drm_gem_is_imported(&bo->base.base);
+ if (!bo->imported) {
/* Pre-reserve the BO pages, so the map operation doesn't have
to
* allocate.
*/
@@ -1246,7 +1247,7 @@ static int panthor_vm_prepare_map_op_ctx(struct
panthor_vm_op_ctx *op_ctx,
sgt = drm_gem_shmem_get_pages_sgt(&bo->base);
if (IS_ERR(sgt)) {
- if (!drm_gem_is_imported(&bo->base.base))
+ if (!bo->imported)
drm_gem_shmem_unpin(&bo->base);
ret = PTR_ERR(sgt);
@@ -1257,7 +1258,7 @@ static int panthor_vm_prepare_map_op_ctx(struct
panthor_vm_op_ctx *op_ctx,
preallocated_vm_bo = drm_gpuvm_bo_create(&vm->base, &bo->base.base);
if (!preallocated_vm_bo) {
- if (!drm_gem_is_imported(&bo->base.base))
+ if (!bo->imported)
drm_gem_shmem_unpin(&bo->base);
ret = -ENOMEM;
@@ -1282,8 +1283,7 @@ static int panthor_vm_prepare_map_op_ctx(struct
panthor_vm_op_ctx *op_ctx,
* If our pre-allocated vm_bo is picked, it now retains the pin ref,
* which will be released in panthor_vm_bo_put().
*/
- if (preallocated_vm_bo != op_ctx->map.vm_bo &&
- !drm_gem_is_imported(&bo->base.base))
+ if (preallocated_vm_bo != op_ctx->map.vm_bo && !bo->imported)
drm_gem_shmem_unpin(&bo->base);
op_ctx->map.bo_offset = offset;
