[PATCH 09/15] drm/panthor: Test for imported buffers with drm_gem_is_imported()

[Steven Price]

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 <tzimmermann at suse.de>
> Cc: Boris Brezillon <boris.brezillon at collabora.com>
> Cc: Steven Price <steven.price at arm.com>
> Cc: Liviu Dudau <liviu.dudau at 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;