On 03/11/2025 15:01, Boris Brezillon wrote:
> From: Ashley Smith <[email protected]>
> 
> Make sure the queue slot is reset even if we failed termination so
> we don't have garbage in the CS input interface after a reset. In
> practice that's not a problem because we zero out all RW sections when
> a hangs occurs, but it's safer to reset things manually, in case we
> decide to not conditionally reload RW sections based on the type of
> hang.
> 
> v4: Split the changes in two separate patches
> 
> v5:
> - No changes
> 
> v6:
> - Adjust the explanation in the commit message
> - Drop the Fixes tag
> - Put after the timeout changes and make the two patches independent
>   so one can be backported, and the other not
> 
> Signed-off-by: Ashley Smith <[email protected]>
> Signed-off-by: Boris Brezillon <[email protected]>

One nit below, but either way:

Reviewed-by: Steven Price <[email protected]>

> ---
>  drivers/gpu/drm/panthor/panthor_sched.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c 
> b/drivers/gpu/drm/panthor/panthor_sched.c
> index b440187798dd..f9a52252b268 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -2836,13 +2836,23 @@ void panthor_sched_suspend(struct panthor_device 
> *ptdev)
>               while (slot_mask) {
>                       u32 csg_id = ffs(slot_mask) - 1;
>                       struct panthor_csg_slot *csg_slot = 
> &sched->csg_slots[csg_id];
> +                     struct panthor_group *group = csg_slot->group;
>  
>                       /* Terminate command timedout, but the soft-reset will
>                        * automatically terminate all active groups, so let's
>                        * force the state to halted here.
>                        */
> -                     if (csg_slot->group->state != 
> PANTHOR_CS_GROUP_TERMINATED)
> -                             csg_slot->group->state = 
> PANTHOR_CS_GROUP_TERMINATED;
> +                     if (group->state != PANTHOR_CS_GROUP_TERMINATED) {
> +                             group->state = PANTHOR_CS_GROUP_TERMINATED;
> +
> +                             /* Reset the queue slots manually if the 
> termination
> +                              * request failed.
> +                              */
> +                             for (i = 0; i < csg_slot->group->queue_count; 
> i++) {
> +                                     if (csg_slot->group->queues[i])

NIT: You've introduced a "group" variable and used it above, but not here.

Thanks,
Steve

> +                                             cs_slot_reset_locked(ptdev, 
> csg_id, i);
> +                             }
> +                     }
>                       slot_mask &= ~BIT(csg_id);
>               }
>       }

Reply via email to