Re: [PATCH v1 3/3] drm/panthor: Unlock the locked region before disabling an AS

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

On Tue, 2 Dec 2025 15:09:42 +0000
Akash Goel <akash.goel@arm.com> wrote:

> Hi Boris,
> 
> 
> On 12/2/25 13:35, Boris Brezillon wrote:
> > An AS can be disabled in the middle of a VM operation (VM being
> > evicted from an AS slot, for instance). In that case, we need the
> > locked section to be unlocked before releasing the slot.
> > 
> > Fixes: 6e2d3b3e8589 ("drm/panthor: Add support for atomic page table updates")
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> >   drivers/gpu/drm/panthor/panthor_mmu.c | 11 +++++++++++
> >   1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> > index 3644af1a8e56..4ed28bd1b8ec 100644
> > --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> > +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> > @@ -588,6 +588,7 @@ static int panthor_mmu_as_enable(struct panthor_device *ptdev, u32 as_nr,
> >   static int panthor_mmu_as_disable(struct panthor_device *ptdev, u32 as_nr,
> >   				  bool recycle_slot)
> >   {
> > +	struct panthor_vm *vm = ptdev->mmu->as.slots[as_nr].vm;
> >   	int ret;
> >     
> 
> Thanks for the fix.
> 
> Please can you consider adding a lockdep_assert_held for 
> mmu->as.slots_lock. That would in a way make it explicit that it is safe 
> to de-reference vm pointer here.

Sure, I can do that.

> 
> Best regards
> Akash
> 
> 
> >   	/* Flush+invalidate RW caches, invalidate RO ones. */
> > @@ -596,6 +597,16 @@ static int panthor_mmu_as_disable(struct panthor_device *ptdev, u32 as_nr,
> >   	if (ret)
> >   		return ret;
> >   
> > +	if (vm && vm->locked_region.size) {
> > +		/* Unlock the region if there a lock pending. */
> > +		ret = as_send_cmd_and_wait(ptdev, vm->as.id, AS_COMMAND_UNLOCK);
> > +		if (ret)
> > +			return ret;
> > +
> > +		vm->locked_region.start = 0;
> > +		vm->locked_region.size = 0;
> > +	}
> > +
> >   	/* If the slot is going to be used immediately, don't bother changing
> >   	 * the config.
> >   	 */