[PATCH v2 0/3] drm/panthor: Fix regressions introduced recently

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v2 0/3] drm/panthor: Fix regressions introduced recently
@ 2025-12-03 11:05 Boris Brezillon
  2025-12-03 11:05 ` [PATCH v2 1/3] drm/panthor: Drop a WARN_ON() in group_free_queue() Boris Brezillon
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Boris Brezillon @ 2025-12-03 11:05 UTC (permalink / raw)
  To: Boris Brezillon, Steven Price, Liviu Dudau, Adrián Larumbe
  Cc: dri-devel, Akash Goel, Chia-I Wu, kernel

Hello,

This is a set of fixes for regressions noticed while testing the
drm-misc-next branch against mesa-ci.

Regards,

Boris

Boris Brezillon (3):
  drm/panthor: Drop a WARN_ON() in group_free_queue()
  drm/panthor: Make sure caches are flushed/invalidated when an AS is
    recycled
  drm/panthor: Unlock the locked region before disabling an AS

 drivers/gpu/drm/panthor/panthor_mmu.c   | 39 +++++++++++++++++++++----
 drivers/gpu/drm/panthor/panthor_sched.c |  5 ++--
 2 files changed, 35 insertions(+), 9 deletions(-)

-- 
2.51.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/3] drm/panthor: Drop a WARN_ON() in group_free_queue()
  2025-12-03 11:05 [PATCH v2 0/3] drm/panthor: Fix regressions introduced recently Boris Brezillon
@ 2025-12-03 11:05 ` Boris Brezillon
  2025-12-03 11:05 ` [PATCH v2 2/3] drm/panthor: Make sure caches are flushed/invalidated when an AS is recycled Boris Brezillon
  2025-12-03 11:05 ` [PATCH v2 3/3] drm/panthor: Unlock the locked region before disabling an AS Boris Brezillon
  2 siblings, 0 replies; 5+ messages in thread
From: Boris Brezillon @ 2025-12-03 11:05 UTC (permalink / raw)
  To: Boris Brezillon, Steven Price, Liviu Dudau, Adrián Larumbe
  Cc: dri-devel, Akash Goel, Chia-I Wu, kernel

It appears the timeout can still be enabled when we reach that point,
because of the asynchronous progress check done on queues that resets
the timer when jobs are still in-flight, but progress was made.
We could add more checks to make sure the timer is not re-enabled when
a group can't run anymore, but we don't have a group to pass to
queue_check_job_completion() in some context.

It's just as safe (we just want to be sure the timer is stopped before
we destroy the queue) and simpler to drop the WARN_ON() in
group_free_queue().

v2:
- Collect R-bs

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_sched.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index 389d508b3848..203f6a0a6b9a 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -893,9 +893,8 @@ static void group_free_queue(struct panthor_group *group, struct panthor_queue *
 	if (IS_ERR_OR_NULL(queue))
 		return;
 
-	/* This should have been disabled before that point. */
-	drm_WARN_ON(&group->ptdev->base,
-		    disable_delayed_work_sync(&queue->timeout.work));
+	/* Disable the timeout before tearing down drm_sched components. */
+	disable_delayed_work_sync(&queue->timeout.work);
 
 	if (queue->entity.fence_context)
 		drm_sched_entity_destroy(&queue->entity);
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/3] drm/panthor: Make sure caches are flushed/invalidated when an AS is recycled
  2025-12-03 11:05 [PATCH v2 0/3] drm/panthor: Fix regressions introduced recently Boris Brezillon
  2025-12-03 11:05 ` [PATCH v2 1/3] drm/panthor: Drop a WARN_ON() in group_free_queue() Boris Brezillon
@ 2025-12-03 11:05 ` Boris Brezillon
  2025-12-03 11:05 ` [PATCH v2 3/3] drm/panthor: Unlock the locked region before disabling an AS Boris Brezillon
  2 siblings, 0 replies; 5+ messages in thread
From: Boris Brezillon @ 2025-12-03 11:05 UTC (permalink / raw)
  To: Boris Brezillon, Steven Price, Liviu Dudau, Adrián Larumbe
  Cc: dri-devel, Akash Goel, Chia-I Wu, kernel

When we re-assign a slot to a different VM, we need to make sure the
old VM caches are flushed before doing the switch. Specialize
panthor_mmu_as_disable() so we can skip the slot programmation while
still getting the cache flushing, and call this helper from
panthor_vm_active() when an idle slot is recycled.

v2:
- 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 | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 8ba5259e3d28..3644af1a8e56 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -585,7 +585,8 @@ static int panthor_mmu_as_enable(struct panthor_device *ptdev, u32 as_nr,
 	return as_send_cmd_and_wait(ptdev, as_nr, AS_COMMAND_UPDATE);
 }
 
-static int panthor_mmu_as_disable(struct panthor_device *ptdev, u32 as_nr)
+static int panthor_mmu_as_disable(struct panthor_device *ptdev, u32 as_nr,
+				  bool recycle_slot)
 {
 	int ret;
 
@@ -595,6 +596,12 @@ static int panthor_mmu_as_disable(struct panthor_device *ptdev, u32 as_nr)
 	if (ret)
 		return ret;
 
+	/* If the slot is going to be used immediately, don't bother changing
+	 * the config.
+	 */
+	if (recycle_slot)
+		return 0;
+
 	gpu_write64(ptdev, AS_TRANSTAB(as_nr), 0);
 	gpu_write64(ptdev, AS_MEMATTR(as_nr), 0);
 	gpu_write64(ptdev, AS_TRANSCFG(as_nr), AS_TRANSCFG_ADRMODE_UNMAPPED);
@@ -714,6 +721,11 @@ int panthor_vm_active(struct panthor_vm *vm)
 
 		drm_WARN_ON(&ptdev->base, refcount_read(&lru_vm->as.active_cnt));
 		as = lru_vm->as.id;
+
+		ret = panthor_mmu_as_disable(ptdev, as, true);
+		if (ret)
+			goto out_unlock;
+
 		panthor_vm_release_as_locked(lru_vm);
 	}
 
@@ -853,7 +865,7 @@ static void panthor_vm_declare_unusable(struct panthor_vm *vm)
 	vm->unusable = true;
 	mutex_lock(&ptdev->mmu->as.slots_lock);
 	if (vm->as.id >= 0 && drm_dev_enter(&ptdev->base, &cookie)) {
-		panthor_mmu_as_disable(ptdev, vm->as.id);
+		panthor_mmu_as_disable(ptdev, vm->as.id, false);
 		drm_dev_exit(cookie);
 	}
 	mutex_unlock(&ptdev->mmu->as.slots_lock);
@@ -1780,7 +1792,7 @@ static void panthor_mmu_irq_handler(struct panthor_device *ptdev, u32 status)
 			ptdev->mmu->as.slots[as].vm->unhandled_fault = true;
 
 		/* Disable the MMU to kill jobs on this AS. */
-		panthor_mmu_as_disable(ptdev, as);
+		panthor_mmu_as_disable(ptdev, as, false);
 		mutex_unlock(&ptdev->mmu->as.slots_lock);
 
 		status &= ~mask;
@@ -1809,7 +1821,8 @@ void panthor_mmu_suspend(struct panthor_device *ptdev)
 		struct panthor_vm *vm = ptdev->mmu->as.slots[i].vm;
 
 		if (vm) {
-			drm_WARN_ON(&ptdev->base, panthor_mmu_as_disable(ptdev, i));
+			drm_WARN_ON(&ptdev->base,
+				    panthor_mmu_as_disable(ptdev, i, false));
 			panthor_vm_release_as_locked(vm);
 		}
 	}
@@ -1930,7 +1943,7 @@ static void panthor_vm_free(struct drm_gpuvm *gpuvm)
 		int cookie;
 
 		if (drm_dev_enter(&ptdev->base, &cookie)) {
-			panthor_mmu_as_disable(ptdev, vm->as.id);
+			panthor_mmu_as_disable(ptdev, vm->as.id, false);
 			drm_dev_exit(cookie);
 		}
 
@@ -2790,7 +2803,8 @@ void panthor_mmu_unplug(struct panthor_device *ptdev)
 		struct panthor_vm *vm = ptdev->mmu->as.slots[i].vm;
 
 		if (vm) {
-			drm_WARN_ON(&ptdev->base, panthor_mmu_as_disable(ptdev, i));
+			drm_WARN_ON(&ptdev->base,
+				    panthor_mmu_as_disable(ptdev, i, false));
 			panthor_vm_release_as_locked(vm);
 		}
 	}
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 3/3] drm/panthor: Unlock the locked region before disabling an AS
  2025-12-03 11:05 [PATCH v2 0/3] drm/panthor: Fix regressions introduced recently Boris Brezillon
  2025-12-03 11:05 ` [PATCH v2 1/3] drm/panthor: Drop a WARN_ON() in group_free_queue() Boris Brezillon
  2025-12-03 11:05 ` [PATCH v2 2/3] drm/panthor: Make sure caches are flushed/invalidated when an AS is recycled Boris Brezillon
@ 2025-12-03 11:05 ` Boris Brezillon
  2025-12-03 11:47   ` Boris Brezillon
  2 siblings, 1 reply; 5+ messages in thread
From: Boris Brezillon @ 2025-12-03 11:05 UTC (permalink / raw)
  To: Boris Brezillon, Steven Price, Liviu Dudau, Adrián Larumbe
  Cc: dri-devel, Akash Goel, Chia-I Wu, kernel

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;
+	}
+
 	/* If the slot is going to be used immediately, don't bother changing
 	 * the config.
 	 */
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 3/3] drm/panthor: Unlock the locked region before disabling an AS
  2025-12-03 11:05 ` [PATCH v2 3/3] drm/panthor: Unlock the locked region before disabling an AS Boris Brezillon
@ 2025-12-03 11:47   ` Boris Brezillon
  0 siblings, 0 replies; 5+ messages in thread
From: Boris Brezillon @ 2025-12-03 11:47 UTC (permalink / raw)
  To: Boris Brezillon, Steven Price, Liviu Dudau, Adrián Larumbe
  Cc: dri-devel, Akash Goel, Chia-I Wu, kernel

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.
>  	 */


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-12-03 11:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-03 11:05 [PATCH v2 0/3] drm/panthor: Fix regressions introduced recently Boris Brezillon
2025-12-03 11:05 ` [PATCH v2 1/3] drm/panthor: Drop a WARN_ON() in group_free_queue() Boris Brezillon
2025-12-03 11:05 ` [PATCH v2 2/3] drm/panthor: Make sure caches are flushed/invalidated when an AS is recycled Boris Brezillon
2025-12-03 11:05 ` [PATCH v2 3/3] drm/panthor: Unlock the locked region before disabling an AS Boris Brezillon
2025-12-03 11:47   ` Boris Brezillon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.