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

[Boris Brezillon]

On Wed, 15 Apr 2026 23:09:17 +0100
Adrián Larumbe <adrian.larumbe@xxxxxxxxxxxxx> wrote:

> > > @@ -2251,14 +2383,17 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
> > >  	}
> > >
> > >  	if (op->remap.prev) {
> > > -		struct panthor_gem_object *bo = to_panthor_bo(op->remap.prev->gem.obj);
> > > -		u64 offset = op->remap.prev->gem.offset + unmap_start - op->remap.prev->va.addr;
> > > -		u64 size = op->remap.prev->va.addr + op->remap.prev->va.range - unmap_start;
> > > +		const struct drm_gpuva_op_map map_op = {
> > > +			.va.addr = unmap_start,
> > > +			.va.range =
> > > +			op->remap.prev->va.addr + op->remap.prev->va.range - unmap_start,
> > > +			.gem.obj = op->remap.prev->gem.obj,
> > > +			.gem.offset =
> > > +			op->remap.prev->gem.offset + unmap_start - op->remap.prev->va.addr,  
> >
> > I believe it should be forced to zero if this is a sparse
> > mapping, no? This makes me think we probably want this to be
> > NULL, in the case of a sparse mapping. It shouldn't prevent
> > reclaim from happening on the dummy BO, because the drm_gpuva
> > has a separate vm_bo field. Yes it forces us to add bunch of
> > is_sparse checks in a few other places, but I find it cleaner
> > than pretending this is a regular BO.  
> 
> The .gem.offset field is assigned here unconditionally, but discarded in cases it's a sparse mapping
> when calling panthor_vm_map_sparse() (which takes no offset argument). I assume what you mean is that
> in panthor_vm_exec_op(), I should abstain from assining .map.gem.obj and .map.gem.offset. However,
> if I do that, the 'va->vm_bo = drm_gpuvm_bo_get(vm_bo);' will never happen inside drm_gpuva_link().

Ah, crap! I thought drm_gpuva_link() was only considering vm_bo. Okay,
let's keep it the way you did it then, and just add a comment
explaining unmap_op.keep can't be trusted when the mapping is sparse.