Re: [PATCH v5 9/9] drm/panthor: Add a GEM shrinker

[Boris Brezillon]

On Wed, 25 Mar 2026 00:14:46 +0000
Adrián Larumbe <adrian.laru...@collabora.com> wrote:

> > +static bool is_gpu_mapped(struct panthor_gem_object *bo,
> > +                     enum panthor_gem_reclaim_state *state)
> > +{
> > +   struct drm_gpuvm *vm = NULL;
> > +   struct drm_gpuvm_bo *vm_bo;
> > +
> > +   drm_gem_for_each_gpuvm_bo(vm_bo, &bo->base) {
> > +           /* Skip evicted GPU mappings. */
> > +           if (vm_bo->evicted)
> > +                   continue;
> > +
> > +           if (!vm) {
> > +                   *state = PANTHOR_GEM_GPU_MAPPED_PRIVATE;
> > +                   vm = vm_bo->vm;
> > +           } else if (vm != vm_bo->vm) {  
> 
> I might be wrong to think that no two VM_BO's for a given BO can have the same
> VM, but if that were the case, then the above check could be omitted and left
> as an 'else'.

No you're correct. I'll go for:

static bool is_gpu_mapped(struct panthor_gem_object *bo,
                          enum panthor_gem_reclaim_state *state)
{
        struct drm_gpuvm_bo *vm_bo;
        u32 vm_count = 0;

        drm_gem_for_each_gpuvm_bo(vm_bo, &bo->base) {
                /* Skip evicted GPU mappings. */
                if (vm_bo->evicted)
                        continue;

                if (vm_count++) {
                        *state = PANTHOR_GEM_GPU_MAPPED_MULTI_VM;
                        break;
                }

                *state = PANTHOR_GEM_GPU_MAPPED_SINGLE_VM;
        }

        return vm_count > 0; 
}

in v6.

> 
> > +                   *state = PANTHOR_GEM_GPU_MAPPED_SHARED;
> > +                   break;
> > +           }
> > +   }
> > +
> > +   return !!vm;
> > +}
> > +

[...]

> > +int panthor_gem_swapin_locked(struct panthor_gem_object *bo)
> > +{
> > +   struct sg_table *sgt;
> > +   int ret;
> > +
> > +   dma_resv_assert_held(bo->base.resv);
> > +
> > +   if (drm_WARN_ON_ONCE(bo->base.dev, drm_gem_is_imported(&bo->base)))
> > +           return -EINVAL;
> > +  
> 
> Following call is redundant because it's already being done in 
> panthor_gem_dev_map_get_sgt_locked().

Indeed, I'll drop this panthor_gem_backing_get_pages_locked().

> 
> > +   ret = panthor_gem_backing_get_pages_locked(bo);
> > +   if (ret)
> > +           return ret;
> > +
> > +   sgt = panthor_gem_dev_map_get_sgt_locked(bo);
> > +   if (IS_ERR(sgt))
> > +           return PTR_ERR(sgt);
> > +
> > +   return 0;
> > +}