Re: [PATCH v11 5/6] drm/panthor: Support sparse mappings

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

On Tue, 19 May 2026 20:15:34 +0100
Adrián Larumbe <adrian.larumbe@collabora.com> wrote:

> Hi Steven,
> 
> On 13.05.2026 16:46, Steven Price wrote:
> > On 07/05/2026 22:49, Adrián Larumbe wrote:  
> > > Allow UM to bind sparsely populated memory regions by cyclically mapping
> > > virtual ranges over a kernel-allocated dummy BO. This alternative is
> > > preferable to the old method of handling sparseness in the UMD, because it
> > > relied on the creation of a buffer object to the same end, despite the fact
> > > Vulkan sparse resources don't need to be backed by a driver BO.
> > >
> > > The choice of backing sparsely-bound regions with a Panthor BO was made so
> > > as to profit from the existing shrinker reclaim code. That way no special
> > > treatment must be given to the dummy sparse BOs when reclaiming memory, as
> > > would be the case if we had chosen a raw kernel page implementation.  
> >
> > Do you need to fix up the remap_evicted_vma() path though? At the moment
> > that will go through panthor_vm_map_pages() without doing the
> > panthor_fix_sparse_map_offset() dance. Also I suspect it won't map the
> > whole region (just the first 2MB in the sgtable).  
> 
> You're right, we need to account for the case in which the VMA is of the sparse kind
> and backed by the dummy BO. I suggest something as follows:
> 
> ``` diff
>  		ret = panthor_vm_lock_region(vm, evicted_vma->base.va.addr,
>  					     evicted_vma->base.va.range);
>  		if (!ret) {
> -			ret = panthor_vm_map_pages(vm, evicted_vma->base.va.addr,
> -						   flags_to_prot(evicted_vma->flags),
> -						   bo->dmap.sgt,
> -						   evicted_vma->base.gem.offset,
> -						   evicted_vma->base.va.range);
> +			struct drm_gpuva_op_map map_op = {
> +				.va.addr = evicted_vma->base.va.addr,
> +				.va.range = evicted_vma->base.va.range,
> +				.gem.obj = &bo->base,
> +				.gem.offset = evicted_vma->base.gem.offset,
> +			};
> +			if (evicted_vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE)
> +				drm_WARN_ON(&vm->ptdev->base, map_op.gem.offset !=
> +					    (map_op.va.addr & (SZ_2M - 1)));
> +
> +			ret = panthor_vm_exec_map_op(vm, evicted_vma->flags, &map_op);
>  			if (!ret)
>  				evicted_vma->evicted = false;
> ```
> 
> However, it seems to me there's no need for panthor_fix_sparse_map_offset() in this code path,
> because a VMA should always have the right gem offset even after eviction. This holds true in
> the event of an sm_remap of a VMA that had already been evicted.

Yep, I think you're right.

> 
> This led me to think doing a WARN_ON test on the offset is the right way to go.

A WARN_ON[_ONCE]() would do, yes.