Re: [PATCH 7/8] drm/panthor: Add support for entering and exiting protected mode

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

On Tue,  5 May 2026 16:05:13 +0200
Ketil Johnsen <ketil.johnsen@arm.com> wrote:

Here comes the second part of the review :-).

> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index 2ab444ee8c710..e93042eaf3fc8 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -100,8 +100,11 @@ static void panthor_gpu_irq_handler(struct panthor_device *ptdev, u32 status)
>  			 fault_status, panthor_exception_name(ptdev, fault_status & 0xFF),
>  			 address);
>  	}
> -	if (status & GPU_IRQ_PROTM_FAULT)
> +	if (status & GPU_IRQ_PROTM_FAULT) {
>  		drm_warn(&ptdev->base, "GPU Fault in protected mode\n");
> +		panthor_gpu_disable_protm_fault_interrupt(ptdev);

It's only used in a single place, so I'd just inline the content of
panthor_gpu_disable_protm_fault_interrupt() here. Also, I think
panthor_gpu_disable_protm_fault_interrupt() is not taking the right
lock (see below).

> +		panthor_device_schedule_reset(ptdev);
> +	}
>  
>  	spin_lock(&ptdev->gpu->reqs_lock);
>  	if (status & ptdev->gpu->pending_reqs) {
> @@ -367,6 +370,10 @@ int panthor_gpu_soft_reset(struct panthor_device *ptdev)
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&ptdev->gpu->reqs_lock, flags);
> +
> +	/** Re-enable the protm_irq_fault when reset is complete */
> +	ptdev->gpu->irq.mask |= GPU_IRQ_PROTM_FAULT;

panthor_irq::mask should only be modified with the
panthor_irq::mask_lock held. Besides, we have a helper for
that:

	panthor_gpu_irq_enable_events(&ptdev->gpu->irq,	GPU_IRQ_PROTM_FAULT);

> +
>  	if (!drm_WARN_ON(&ptdev->base,
>  			 ptdev->gpu->pending_reqs & GPU_IRQ_RESET_COMPLETED)) {
>  		ptdev->gpu->pending_reqs |= GPU_IRQ_RESET_COMPLETED;
> @@ -427,3 +434,8 @@ void panthor_gpu_resume(struct panthor_device *ptdev)
>  	panthor_hw_l2_power_on(ptdev);
>  }
>  
> +void panthor_gpu_disable_protm_fault_interrupt(struct panthor_device *ptdev)
> +{
> +	scoped_guard(spinlock_irqsave, &ptdev->gpu->reqs_lock)
> +		ptdev->gpu->irq.mask &= ~GPU_IRQ_PROTM_FAULT;

Same problem with wrong lock being taken to modify the mask, and
panthor_gpu_irq_disable_events() probably being a better option?

> +}
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.h b/drivers/gpu/drm/panthor/panthor_gpu.h
> index 12c263a399281..ca66c73f543e6 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.h
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.h
> @@ -54,4 +54,10 @@ int panthor_gpu_soft_reset(struct panthor_device *ptdev);
>  void panthor_gpu_power_changed_off(struct panthor_device *ptdev);
>  int panthor_gpu_power_changed_on(struct panthor_device *ptdev);
>  
> +/**
> + * panthor_gpu_disable_protm_fault_interrupt() - Disable GPU_PROTECTED_FAULT interrupt
> + * @ptdev: Device.
> + */
> +void panthor_gpu_disable_protm_fault_interrupt(struct panthor_device *ptdev);
> +
>  #endif
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 07f54176ec1bf..702f537905b56 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -31,6 +31,7 @@
>  #include <linux/sizes.h>
>  
>  #include "panthor_device.h"
> +#include "panthor_fw.h"
>  #include "panthor_gem.h"
>  #include "panthor_gpu.h"
>  #include "panthor_heap.h"
> @@ -1704,8 +1705,12 @@ static int panthor_vm_lock_region(struct panthor_vm *vm, u64 start, u64 size)
>  	if (drm_WARN_ON(&ptdev->base, vm->locked_region.size))
>  		return -EINVAL;
>  
> +	down_read(&ptdev->protm.lock);
> +
>  	mutex_lock(&ptdev->mmu->as.slots_lock);
>  	if (vm->as.id >= 0 && size) {
> +		panthor_fw_protm_exit_sync(ptdev);
> +
>  		/* Lock the region that needs to be updated */
>  		gpu_write64(ptdev, AS_LOCKADDR(vm->as.id),
>  			    pack_region_range(ptdev, &start, &size));
> @@ -1720,6 +1725,9 @@ static int panthor_vm_lock_region(struct panthor_vm *vm, u64 start, u64 size)
>  	}
>  	mutex_unlock(&ptdev->mmu->as.slots_lock);
>  
> +	if (ret)
> +		up_read(&ptdev->protm.lock);

Let's try to keep the locked section local to a function: the protm.lock
should, IMHO, be taken/released in panthor_vm_exec_op(). If we go for
that, this also makes the panthor_vm_lock_region() vs
panthor_vm_expand_locked_region() distinction useless, though it's
probably fine to keep it for clarity.

> +
>  	return ret;
>  }
>  
> @@ -1805,6 +1813,8 @@ static void panthor_vm_unlock_region(struct panthor_vm *vm)
>  	vm->locked_region.start = 0;
>  	vm->locked_region.size = 0;
>  	mutex_unlock(&ptdev->mmu->as.slots_lock);
> +
> +	up_read(&ptdev->protm.lock);
>  }
>  
>  static void panthor_mmu_irq_handler(struct panthor_device *ptdev, u32 status)
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 987072bd867c4..acb04250c7def 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -308,6 +308,15 @@ struct panthor_scheduler {
>  		 */
>  		struct list_head stopped_groups;
>  	} reset;
> +
> +	/** @protm: Protected mode related fields. */
> +	struct {
> +		/** @protected_mode: True if GPU is in protected mode. */
> +		bool protected_mode;

nit: s/protected_mode/enabled/s. But do we even need that boolean?
Isn't active_group != NULL providing the same info?

> +
> +		/** @active_group: The active protected group. */
> +		struct panthor_group *active_group;
> +	} protm;
>  };
>  
>  /**
> @@ -570,6 +579,16 @@ struct panthor_group {
>  	/** @fatal_queues: Bitmask reflecting the queues that hit a fatal exception. */
>  	u32 fatal_queues;
>  
> +	/**
> +	 * @protm_pending_queues: Bitmask reflecting the queues that are waiting
> +	 *                        on a CS_PROTM_PENDING.
> +	 *
> +	 * The GPU will set the bit associated to the queue pending protected mode
> +	 * when a PROT_REGION command is executing or when trying to resume previously
> +	 * suspended protected mode jobs.
> +	 */
> +	u32 protm_pending_queues;
> +
>  	/** @tiler_oom: Mask of queues that have a tiler OOM event to process. */
>  	atomic_t tiler_oom;
>  
> @@ -1176,6 +1195,7 @@ queue_resume_timeout(struct panthor_queue *queue)
>   * @ptdev: Device.
>   * @csg_id: Group slot ID.
>   * @cs_id: Queue slot ID.
> + * @protm_ack: Acknowledge pending protected mode queues
>   *
>   * Program a queue slot with the queue information so things can start being
>   * executed on this queue.
> @@ -1472,6 +1492,34 @@ csg_slot_prog_locked(struct panthor_device *ptdev, u32 csg_id, u32 priority)
>  	return 0;
>  }
>  
> +static void
> +cs_slot_process_protm_pending_event_locked(struct panthor_device *ptdev,
> +					   u32 csg_id, u32 cs_id)
> +{
> +	struct panthor_scheduler *sched = ptdev->scheduler;
> +	struct panthor_csg_slot *csg_slot = &sched->csg_slots[csg_id];
> +	struct panthor_group *group = csg_slot->group;
> +
> +	lockdep_assert_held(&sched->lock);
> +
> +	if (!group)
> +		return;
> +
> +	/* No protected memory heap, a user space program tried to
> +	 * submit a protected mode jobs resulting in the GPU raising
> +	 * a CS_PROTM_PENDING request.
> +	 *
> +	 * This scenario is invalid and the protected mode jobs must
> +	 * not be allowed to progress.
> +	 */
> +	if (!ptdev->protm.heap)
> +		return;

Should we flag the group unusable when that happens and schedule it out
as soon as possible?

	if (!ptdev->protm.heap)
		group->fatal_queues |= BIT(cs_id);
	else
		group->protm_pending_queues |= BIT(cs_id);

	sched_queue_delayed_work(sched, tick, 0);

> +
> +	group->protm_pending_queues |= BIT(cs_id);
> +
> +	sched_queue_delayed_work(sched, tick, 0);
> +}
> +
>  static void
>  cs_slot_process_fatal_event_locked(struct panthor_device *ptdev,
>  				   u32 csg_id, u32 cs_id)
> @@ -1718,6 +1766,9 @@ static bool cs_slot_process_irq_locked(struct panthor_device *ptdev,
>  	if (events & CS_TILER_OOM)
>  		cs_slot_process_tiler_oom_event_locked(ptdev, csg_id, cs_id);
>  
> +	if (events & CS_PROTM_PENDING)
> +		cs_slot_process_protm_pending_event_locked(ptdev, csg_id, cs_id);
> +
>  	/* We don't acknowledge the TILER_OOM event since its handling is
>  	 * deferred to a separate work.
>  	 */
> @@ -1848,6 +1899,17 @@ static void sched_process_idle_event_locked(struct panthor_device *ptdev)
>  	sched_queue_delayed_work(ptdev->scheduler, tick, 0);
>  }
>  
> +static void sched_process_protm_exit_event_locked(struct panthor_device *ptdev)
> +{
> +	lockdep_assert_held(&ptdev->scheduler->lock);
> +
> +	/* Acknowledge the protm exit and schedule a tick. */
> +	panthor_fw_protm_exit(ptdev);

Let's just inline the content of panthor_fw_protm_exit() here.*
It doesn't make sense to have all these indirections, especially
since PROTM and scheduling are intertwined anyway, so I consider
it part of the scheduler responsibility (just like the scheduler
deals with other GLB events).

The same goes for the other panthor_fw_protm_ helpers defined in
panthor_fw.c, I think panthor_sched.c would be a better fit for
those, or even inlining their content if they are only used in
a single place.

> +	sched_queue_delayed_work(ptdev->scheduler, tick, 0);
> +	ptdev->scheduler->protm.protected_mode = false;
> +	ptdev->scheduler->protm.active_group = NULL;
> +}
> +
>  /**
>   * sched_process_global_irq_locked() - Process the scheduling part of a global IRQ
>   * @ptdev: Device.
> @@ -1863,6 +1925,9 @@ static void sched_process_global_irq_locked(struct panthor_device *ptdev)
>  	ack = READ_ONCE(glb_iface->output->ack);
>  	evts = (req ^ ack) & GLB_EVT_MASK;
>  
> +	if (evts & GLB_PROTM_EXIT)
> +		sched_process_protm_exit_event_locked(ptdev);
> +
>  	if (evts & GLB_IDLE)
>  		sched_process_idle_event_locked(ptdev);
>  }
> @@ -1872,23 +1937,71 @@ static void process_fw_events_work(struct work_struct *work)
>  	struct panthor_scheduler *sched = container_of(work, struct panthor_scheduler,
>  						      fw_events_work);
>  	u32 events = atomic_xchg(&sched->fw_events, 0);
> +	u32 csg_events = events & ~JOB_INT_GLOBAL_IF;
>  	struct panthor_device *ptdev = sched->ptdev;
>  
>  	mutex_lock(&sched->lock);
>  
> +	while (csg_events) {
> +		u32 csg_id = ffs(csg_events) - 1;
> +
> +		sched_process_csg_irq_locked(ptdev, csg_id);
> +		csg_events &= ~BIT(csg_id);
> +	}

I'm sure you have a good reason to re-order the processing
of CSG and GLB events, and it'd be good to have it documented
in a comment.

> +
>  	if (events & JOB_INT_GLOBAL_IF) {
>  		sched_process_global_irq_locked(ptdev);
>  		events &= ~JOB_INT_GLOBAL_IF;
>  	}
>  
> -	while (events) {
> -		u32 csg_id = ffs(events) - 1;
> +	mutex_unlock(&sched->lock);
> +}
>  
> -		sched_process_csg_irq_locked(ptdev, csg_id);
> -		events &= ~BIT(csg_id);
> +static void handle_protm_fault(struct panthor_device *ptdev)

This is a bit of a misnomer, I think. It doesn't seem to be triggered
by a FAULT event, it's more a timeout on a PROTM section that would
lead to a reset being scheduled, and this "blocked-in-protm" situation
being detected as part of the reset.

> +{
> +	struct panthor_scheduler *sched = ptdev->scheduler;
> +	u32 csg_id;
> +	struct panthor_group *protm_group;
> +
> +	guard(mutex)(&sched->lock);
> +
> +	if (!sched->protm.protected_mode)
> +		return;
> +
> +	protm_group = sched->protm.active_group;
> +
> +	if (drm_WARN_ON(&ptdev->base, !protm_group))
> +		return;

See, that's a perfect example of sched->protm.protected_mode being redundant.
Now you're left with a potential protected_mode=true,active_group=NULL
inconsistency you don't expect.

> +
> +	/* Group will be terminated by the device reset */
> +	protm_group->fatal_queues |= GENMASK(protm_group->queue_count - 1, 0);
> +
> +	if (!panthor_fw_protm_exit_wait_event_timeout(ptdev))
> +		goto cleanup_protm;
> +
> +	/**
> +	 * GPU failed to exit protected mode. Mark all non-protected mode CSGs

	/* GPU failed to exit protected mode. Mark all non-protected mode CSGs

> +	 * as suspended so that they are unaffected by the GPU reset.
> +	 */
> +
> +	for (csg_id = 0; csg_id < sched->csg_slot_count; csg_id++) {
> +		struct panthor_group *group = sched->csg_slots[csg_id].group;
> +
> +		if (!group || group == protm_group)
> +			continue;
> +
> +		group->state = PANTHOR_CS_GROUP_SUSPENDED;
> +
> +		group_unbind_locked(group);
> +
> +		list_move(&group->run_node, group_is_idle(group) ?
> +						&sched->groups.idle[group->priority] :
> +						&sched->groups.runnable[group->priority]);

nit: Let's use a local struct list_head * variable to select the right list.

>  	}
>  
> -	mutex_unlock(&sched->lock);
> +cleanup_protm:
> +	sched->protm.protected_mode = false;
> +	sched->protm.active_group = NULL;
>  }
>  
>  /**
> @@ -2029,6 +2142,7 @@ struct panthor_sched_tick_ctx {
>  	bool immediate_tick;
>  	bool stop_tick;
>  	u32 csg_upd_failed_mask;
> +	struct panthor_group *protm_group;
>  };
>  
>  static bool
> @@ -2299,6 +2413,7 @@ tick_ctx_evict_group(struct panthor_scheduler *sched,
>  
>  static void
>  tick_ctx_reschedule_group(struct panthor_scheduler *sched,
> +			  struct panthor_sched_tick_ctx *ctx,
>  			  struct panthor_csg_slots_upd_ctx *upd_ctx,
>  			  struct panthor_group *group,
>  			  int new_csg_prio)
> @@ -2321,6 +2436,30 @@ tick_ctx_reschedule_group(struct panthor_scheduler *sched,
>  					csg_iface->output->ack ^ CSG_ENDPOINT_CONFIG,
>  					CSG_ENDPOINT_CONFIG);
>  	}
> +
> +	if (ctx->protm_group == group) {
> +		for (u32 q = 0; q < group->queue_count; q++) {
> +			struct panthor_fw_cs_iface *cs_iface;
> +
> +			if (!(group->protm_pending_queues & BIT(q)))
> +				continue;
> +
> +			cs_iface = panthor_fw_get_cs_iface(ptdev, group->csg_id, q);
> +			panthor_fw_update_reqs(cs_iface, req, cs_iface->output->ack,
> +					       CS_PROTM_PENDING);
> +		}
> +
> +		panthor_fw_toggle_reqs(csg_iface, doorbell_req, doorbell_ack,
> +				       group->protm_pending_queues);
> +		csgs_upd_ctx_ring_doorbell(upd_ctx, group->csg_id);
> +		group->protm_pending_queues = 0;
> +
> +		/*
> +		 * We only allow one protected group to run at same time,
> +		 * as it makes it easier to handle faults in protected mode.

It's more something to document in the panthor_scheduler::protm::active_group
section.

> +		 */
> +		sched->protm.active_group = group;

Would it make sense to move this logic to a tick_ctx_handle_protm_group()
helper that's called before/after tick_ctx_reschedule_group()? This way
there's no extra if (ctx->protm_group == group) conditional branch in here.


static void
tick_ctx_handle_protm_group(struct panthor_scheduler *sched,
			    struct panthor_sched_tick_ctx *ctx,
 			    struct panthor_csg_slots_upd_ctx *upd_ctx)
{
	struct panthor_device *ptdev = sched->ptdev;
	struct panthor_group *group = ctx->protm_group;
	struct panthor_fw_csg_iface *csg_iface;

	if (!group || drm_WARN_ON(&ptdev->base, group->csg_id < 0))
		return;

	csg_iface = panthor_fw_get_csg_iface(ptdev, group->csg_id);
	for (u32 q = 0; q < group->queue_count; q++) {
		struct panthor_fw_cs_iface *cs_iface;

		if (!(group->protm_pending_queues & BIT(q)))
			continue;

		cs_iface = panthor_fw_get_cs_iface(ptdev, group->csg_id, q);
		panthor_fw_update_reqs(cs_iface, req, cs_iface->output->ack,
				       CS_PROTM_PENDING);
	}

	panthor_fw_toggle_reqs(csg_iface, doorbell_req, doorbell_ack,
			       group->protm_pending_queues);
	csgs_upd_ctx_ring_doorbell(upd_ctx, group->csg_id);
	group->protm_pending_queues = 0;
	sched->protm.active_group = group;
}

> +	}
>  }
>  
>  static void
> @@ -2336,6 +2475,17 @@ tick_ctx_schedule_group(struct panthor_scheduler *sched,
>  	group_bind_locked(group, csg_id);
>  	csg_slot_prog_locked(ptdev, csg_id, csg_prio);
>  
> +	/* If the group was waiting for protected mode before suspension,
> +	 * and the tick context enters this mode, it should be serviced
> +	 * immediately because the slot reset should have set the
> +	 * CS_PROTM_PENDING bit to zero, and cs_prog_slot_locked() sets it to
> +	 * zero too.
> +	 * It's not clear if we will get a new CS_PROTM_PENDING event in that
> +	 * case, but it should be safe either way.
> +	 */
> +	if (group->protm_pending_queues && ctx->protm_group)
> +		group->protm_pending_queues = 0;

I'd move this to the path where we do the SUSPEND, or group_unbind(), even.

> +
>  	csgs_upd_ctx_queue_reqs(ptdev, upd_ctx, csg_id,
>  				group->state == PANTHOR_CS_GROUP_SUSPENDED ?
>  				CSG_STATE_RESUME : CSG_STATE_START,
> @@ -2365,7 +2515,7 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
>  
>  		/* Update priorities on already running groups. */
>  		list_for_each_entry(group, &ctx->groups[prio], run_node) {
> -			tick_ctx_reschedule_group(sched, &upd_ctx, group, new_csg_prio--);
> +			tick_ctx_reschedule_group(sched, ctx, &upd_ctx, group, new_csg_prio--);
>  		}
>  	}
>  
> @@ -2457,6 +2607,15 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
>  
>  	sched->used_csg_slot_count = ctx->group_count;
>  	sched->might_have_idle_groups = ctx->idle_group_count > 0;
> +
> +	if (ctx->protm_group) {
> +		ret = panthor_fw_protm_enter(ptdev);
> +		if (ret) {
> +			panthor_device_schedule_reset(ptdev);
> +			ctx->csg_upd_failed_mask = U32_MAX;

It's weird to flag it as all CSGs update failed. Should we instead
have

			/* If we failed to enter PROTM, consider the group who
			 * requested it as failed.
			 */
			ctx->csg_upd_failed_mask |= BIT(ctx->protm_group->csg_id);

> +		}
> +		sched->protm.protected_mode = true;

I'd move that to a tick_ctx_service_protm_req() helper that has the
panthor_fw_protm_enter() inlined, because again, it doesn't make
sense to have this defined in panthor_fw.c if the only user lives
in panthor_sched.c

> +	}
>  }
>  
>  static u64
> @@ -2490,7 +2649,7 @@ static void tick_work(struct work_struct *work)
>  	u64 resched_target = sched->resched_target;
>  	u64 remaining_jiffies = 0, resched_delay;
>  	u64 now = get_jiffies_64();
> -	int prio, ret, cookie;
> +	int prio, protm_prio, ret, cookie;
>  	bool full_tick;
>  
>  	if (!drm_dev_enter(&ptdev->base, &cookie))
> @@ -2564,14 +2723,49 @@ static void tick_work(struct work_struct *work)
>  		}
>  	}
>  
> +	/* Check if the highest priority group want to switch to protected mode */
> +	for (protm_prio = PANTHOR_CSG_PRIORITY_COUNT - 1; protm_prio >= 0; protm_prio--) {
> +		struct panthor_group *group;
> +
> +		group = list_first_entry_or_null(&ctx.groups[protm_prio],
> +						 struct panthor_group,
> +						 run_node);
> +		if (group) {
> +			ctx.protm_group = group;
> +			break;
> +		}

Should this be

		if (group) {
			if (group->protm_pending_queues)
				ctx.protm_group = group;

			break;
		}

?

> +	}
> +
>  	/* If we have free CSG slots left, pick idle groups */
> -	for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1;
> -	     prio >= 0 && !tick_ctx_is_full(sched, &ctx);
> -	     prio--) {

How about we keep it a single indentation level and skip higher prios if
PROTM is requested:

		/* Pick only idle groups with equal or lower priority than the
		 * group triggering protected mode. Do not bother picking
		 * unscheduled idle groups.
		 */
		if (ctx.protm_group && prio < protm_prio)
			continue;

This saves us an indentation level and limits the code duplication.

> -		/* Check the old_group queue first to avoid reprogramming the slots */
> -		tick_ctx_pick_groups_from_list(sched, &ctx, &ctx.old_groups[prio], false, true);
> -		tick_ctx_pick_groups_from_list(sched, &ctx, &sched->groups.idle[prio],
> -					       false, false);
> +	if (ctx.protm_group) {
> +		/* Pick only idle groups with equal or lower priority than the
> +		 * group triggering protected mode. Do not bother picking
> +		 * unscheduled idle groups.
> +		 */
> +		for (prio = protm_prio;
> +		     prio >= 0 && !tick_ctx_is_full(sched, &ctx);
> +		     prio--)
> +			tick_ctx_pick_groups_from_list(sched, &ctx,
> +						       &ctx.old_groups[prio],
> +						       false, true);
> +	} else {
> +		/* No switch to protected, just pick any idle group according
> +		 * to priority
> +		 */
> +		for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1;
> +		     prio >= 0 && !tick_ctx_is_full(sched, &ctx);
> +		     prio--) {
> +			/* Check the old_group queue first to avoid
> +			 * reprogramming the slots
> +			 */
> +			tick_ctx_pick_groups_from_list(sched, &ctx,
> +						       &ctx.old_groups[prio],
> +						       false, true);
> +			tick_ctx_pick_groups_from_list(sched, &ctx,
> +						       &sched->groups.idle[prio],
> +						       false, false);
> +		}
> +
>  	}
>  
>  	tick_ctx_apply(sched, &ctx);
> @@ -2993,6 +3187,8 @@ void panthor_sched_pre_reset(struct panthor_device *ptdev)
>  	cancel_work_sync(&sched->sync_upd_work);
>  	cancel_delayed_work_sync(&sched->tick_work);
>  
> +	handle_protm_fault(ptdev);

I actually wonder if this should be part of the panthor_sched_suspend()
logic. That is, we would automatically flag all non-protm groups as
suspended if the GPU was in PROTM mode at the time the hang happened.

> +
>  	panthor_sched_suspend(ptdev);
>  
>  	/* Stop all groups that might still accept jobs, so we don't get passed

Regards,

Boris