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

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

On Wed,  3 Dec 2025 12:05:27 +0100
Boris Brezillon <boris.brezillon@collabora.com> 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.
> 
> v2:
> - Add an lockdep_assert_held() in panthor_mmu_as_disable()
> - Collect R-bs
> 
> Fixes: 6e2d3b3e8589 ("drm/panthor: Add support for atomic page table updates")
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
> ---
>  drivers/gpu/drm/panthor/panthor_mmu.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 3644af1a8e56..493e6428d478 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -588,14 +588,27 @@ 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;
>  
> +	lockdep_assert_held(&ptdev->mmu->as.slots_lock);
> +
>  	/* Flush+invalidate RW caches, invalidate RO ones. */
>  	ret = panthor_gpu_flush_caches(ptdev, CACHE_CLEAN | CACHE_INV,
>  				       CACHE_CLEAN | CACHE_INV, CACHE_INV);
>  	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;

Actually, resetting the locked_region range triggers the WARN_ON()s in
the vm_[un]map_pages() path, so I'll get rid of that in v3.

> +	}
> +
>  	/* If the slot is going to be used immediately, don't bother changing
>  	 * the config.
>  	 */