Re: [PATCH 5/8] drm/panthor: Minor scheduler refactoring

Boris Brezillon
2026-05-05T16:19:49Z

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

> From: Florent Tomasin <florent.tomasin@arm.com>
>
> Refactor parts of the group scheduling logic into new helper functions.
> This will simplify addition of the protected mode feature.
>
> Remove redundant assignments of csg_slot.
>
> Signed-off-by: Florent Tomasin <florent.tomasin@arm.com>
> Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
> Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>

Glad to see this big tick_ctx_apply() function split into smaller
pieces.

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 135 +++++++++++++++---------
> 1 file changed, 86 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 5ee386338005c..987072bd867c4 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -1934,6 +1934,12 @@ static void csgs_upd_ctx_init(struct panthor_csg_slots_upd_ctx *ctx)
> memset(ctx, 0, sizeof(*ctx));
> }
>
> +static void csgs_upd_ctx_ring_doorbell(struct panthor_csg_slots_upd_ctx *ctx,
> + u32 csg_id)
> +{
> + ctx->update_mask |= BIT(csg_id);
> +}
> +
> static void csgs_upd_ctx_queue_reqs(struct panthor_device *ptdev,
> struct panthor_csg_slots_upd_ctx *ctx,
> u32 csg_id, u32 value, u32 mask)
> @@ -1944,7 +1950,8 @@ static void csgs_upd_ctx_queue_reqs(struct panthor_device *ptdev,
>
> ctx->requests[csg_id].value = (ctx->requests[csg_id].value & ~mask) | (value & mask);
> ctx->requests[csg_id].mask |= mask;
> - ctx->update_mask |= BIT(csg_id);
> +
> + csgs_upd_ctx_ring_doorbell(ctx, csg_id);
> }
>
> static int csgs_upd_ctx_apply_locked(struct panthor_device *ptdev,
> @@ -1961,8 +1968,12 @@ static int csgs_upd_ctx_apply_locked(struct panthor_device *ptdev,
> while (update_slots) {
> struct panthor_fw_csg_iface *csg_iface;
> u32 csg_id = ffs(update_slots) - 1;
> + u32 req_mask = ctx->requests[csg_id].mask;
>
> update_slots &= ~BIT(csg_id);
> + if (!req_mask)
> + continue;
> +
> csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
> panthor_fw_update_reqs(csg_iface, req,
> ctx->requests[csg_id].value,
> @@ -1979,6 +1990,9 @@ static int csgs_upd_ctx_apply_locked(struct panthor_device *ptdev,
> int ret;
>
> update_slots &= ~BIT(csg_id);
> + if (!req_mask)
> + continue;
> +
> csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
>
> ret = panthor_fw_csg_wait_acks(ptdev, csg_id, req_mask, &acked, 100);
> @@ -2266,12 +2280,76 @@ tick_ctx_cleanup(struct panthor_scheduler *sched,
> }
> }
>
> +static void
> +tick_ctx_evict_group(struct panthor_scheduler *sched,
> + struct panthor_csg_slots_upd_ctx *upd_ctx,
> + struct panthor_group *group)
> +{
> + struct panthor_device *ptdev = sched->ptdev;
> +
> + if (drm_WARN_ON(&ptdev->base, group->csg_id < 0))
> + return;
> +
> + csgs_upd_ctx_queue_reqs(ptdev, upd_ctx, group->csg_id,
> + group_can_run(group) ?
> + CSG_STATE_SUSPEND : CSG_STATE_TERMINATE,
> + CSG_STATE_MASK);
> +}
> +
> +
> +static void
> +tick_ctx_reschedule_group(struct panthor_scheduler *sched,
> + struct panthor_csg_slots_upd_ctx *upd_ctx,
> + struct panthor_group *group,
> + int new_csg_prio)
> +{
> + struct panthor_device *ptdev = sched->ptdev;
> + struct panthor_fw_csg_iface *csg_iface;
> + struct panthor_csg_slot *csg_slot;
> +
> + if (group->csg_id < 0)
> + return;
> +
> + csg_iface = panthor_fw_get_csg_iface(ptdev, group->csg_id);
> + csg_slot = &sched->csg_slots[group->csg_id];
> +
> + if (csg_slot->priority != new_csg_prio) {
> + panthor_fw_update_reqs(csg_iface, endpoint_req,
> + CSG_EP_REQ_PRIORITY(new_csg_prio),
> + CSG_EP_REQ_PRIORITY_MASK);
> + csgs_upd_ctx_queue_reqs(ptdev, upd_ctx, group->csg_id,
> + csg_iface->output->ack ^ CSG_ENDPOINT_CONFIG,
> + CSG_ENDPOINT_CONFIG);
> + }
> +}
> +
> +static void
> +tick_ctx_schedule_group(struct panthor_scheduler *sched,
> + struct panthor_sched_tick_ctx *ctx,
> + struct panthor_csg_slots_upd_ctx *upd_ctx,
> + struct panthor_group *group,
> + int csg_id, int csg_prio)
> +{
> + struct panthor_device *ptdev = sched->ptdev;
> + struct panthor_fw_csg_iface *csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
> +
> + group_bind_locked(group, csg_id);
> + csg_slot_prog_locked(ptdev, csg_id, csg_prio);
> +
> + csgs_upd_ctx_queue_reqs(ptdev, upd_ctx, csg_id,
> + group->state == PANTHOR_CS_GROUP_SUSPENDED ?
> + CSG_STATE_RESUME : CSG_STATE_START,
> + CSG_STATE_MASK);
> + csgs_upd_ctx_queue_reqs(ptdev, upd_ctx, csg_id,
> + csg_iface->output->ack ^ CSG_ENDPOINT_CONFIG,
> + CSG_ENDPOINT_CONFIG);
> +}
> +
> static void
> tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *ctx)
> {
> struct panthor_group *group, *tmp;
> struct panthor_device *ptdev = sched->ptdev;
> - struct panthor_csg_slot *csg_slot;
> int prio, new_csg_prio = MAX_CSG_PRIO, i;
> u32 free_csg_slots = 0;
> struct panthor_csg_slots_upd_ctx upd_ctx;
> @@ -2282,42 +2360,12 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
> for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1; prio >= 0; prio--) {
> /* Suspend or terminate evicted groups. */
> list_for_each_entry(group, &ctx->old_groups[prio], run_node) {
> - bool term = !group_can_run(group);
> - int csg_id = group->csg_id;
> -
> - if (drm_WARN_ON(&ptdev->base, csg_id < 0))
> - continue;
> -
> - csg_slot = &sched->csg_slots[csg_id];
> - csgs_upd_ctx_queue_reqs(ptdev, &upd_ctx, csg_id,
> - term ? CSG_STATE_TERMINATE : CSG_STATE_SUSPEND,
> - CSG_STATE_MASK);
> + tick_ctx_evict_group(sched, &upd_ctx, group);
> }
>
> /* Update priorities on already running groups. */
> list_for_each_entry(group, &ctx->groups[prio], run_node) {
> - struct panthor_fw_csg_iface *csg_iface;
> - int csg_id = group->csg_id;
> -
> - if (csg_id < 0) {
> - new_csg_prio--;
> - continue;
> - }
> -
> - csg_slot = &sched->csg_slots[csg_id];
> - csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
> - if (csg_slot->priority == new_csg_prio) {
> - new_csg_prio--;
> - continue;
> - }
> -
> - panthor_fw_csg_endpoint_req_update(ptdev, csg_iface,
> - CSG_EP_REQ_PRIORITY(new_csg_prio),
> - CSG_EP_REQ_PRIORITY_MASK);
> - csgs_upd_ctx_queue_reqs(ptdev, &upd_ctx, csg_id,
> - csg_iface->output->ack ^ CSG_ENDPOINT_CONFIG,
> - CSG_ENDPOINT_CONFIG);
> - new_csg_prio--;
> + tick_ctx_reschedule_group(sched, &upd_ctx, group, new_csg_prio--);
> }
> }
>
> @@ -2354,28 +2402,17 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
> for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1; prio >= 0; prio--) {
> list_for_each_entry(group, &ctx->groups[prio], run_node) {
> int csg_id = group->csg_id;
> - struct panthor_fw_csg_iface *csg_iface;
> + int csg_prio = new_csg_prio--;
>
> - if (csg_id >= 0) {
> - new_csg_prio--;
> + if (csg_id >= 0)
> continue;
> - }
>
> csg_id = ffs(free_csg_slots) - 1;
> if (drm_WARN_ON(&ptdev->base, csg_id < 0))
> break;
>
> - csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
> - csg_slot = &sched->csg_slots[csg_id];
> - group_bind_locked(group, csg_id);
> - csg_slot_prog_locked(ptdev, csg_id, new_csg_prio--);
> - csgs_upd_ctx_queue_reqs(ptdev, &upd_ctx, csg_id,
> - group->state == PANTHOR_CS_GROUP_SUSPENDED ?
> - CSG_STATE_RESUME : CSG_STATE_START,
> - CSG_STATE_MASK);
> - csgs_upd_ctx_queue_reqs(ptdev, &upd_ctx, csg_id,
> - csg_iface->output->ack ^ CSG_ENDPOINT_CONFIG,
> - CSG_ENDPOINT_CONFIG);
> + tick_ctx_schedule_group(sched, ctx, &upd_ctx, group, csg_id, csg_prio);
> +
> free_csg_slots &= ~BIT(csg_id);
> }
> }