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

[Boris Brezillon <boris.brezillon@collabora.com>]

On Fri, 25 Apr 2025 14:34:53 +0100
Steven Price <steven.price@arm.com> wrote:

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

This should be fixed by [1], but I wonder why it's not been merged in
drm-misc-next yet.

[1]https://patches.linaro.org/project/linux-media/patch/20250416065820.26076-1-tzimmermann@suse.de/