Re: [PATCH v4 16/18] drm/panthor: Track user owned groups

[Adrian Larumbe <adrian.larumbe@collabora.com>]

Reviewed-by: Adrián Larumbe <adrian.larumbe@collabora.com>

On 26.08.2026 16:56, Boris Brezillon wrote:
> A group can outlive its user handle because of internal refs. In order
> to fix the unplug logic, we need to keep track of groups that have a
> valid user handle so we can release the references that were owned by
> the user processes in the unplug path.
> 
> This is the prep work to keep track of user owned groups. Note that
> the destroyed attribute is dropped because it's equivalent to checking
> whether the group is inserted in the user_owned list now.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_sched.c | 43 ++++++++++++++++++++++++---------
>  1 file changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 4ea16b40d6b9..bd5dcf4cb580 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -235,6 +235,15 @@ struct panthor_scheduler {
>  		 * This list is evaluated in the @sync_upd_work work.
>  		 */
>  		struct list_head waiting;
> +
> +		/**
> +		 * @user_owned: List of groups that have a valid user handle.
> +		 *
> +		 * All groups are inserted in this list at creation time through their
> +		 * panthor_group;:user_node, and evicted from this list when

                  Nit: :: instead of ;:

> +		 * panthor_group_destroy() is called.
> +		 */
> +		struct list_head user_owned;
>  	} groups;
>  
>  	/**
> @@ -586,15 +595,6 @@ struct panthor_group {
>  	 */
>  	int csg_id;
>  
> -	/**
> -	 * @destroyed: True when the group has been destroyed.
> -	 *
> -	 * If a group is destroyed it becomes useless: no further jobs can be submitted
> -	 * to its queues. We simply wait for all references to be dropped so we can
> -	 * release the group object.
> -	 */
> -	bool destroyed;
> -
>  	/**
>  	 * @timedout: True when a timeout occurred on any of the queues owned by
>  	 * this group.
> @@ -707,6 +707,17 @@ struct panthor_group {
>  	 * panthor_group::groups::waiting list.
>  	 */
>  	struct list_head wait_node;
> +
> +	/**
> +	 * @user_node: Used to insert the group in the panthor_scheduler::groups::user_owned list.
> +	 *
> +	 * When the group is created, it's inserted in panthor_scheduler::groups::user_owned,
> +	 * and when panthor_group_destroy, the group is remove from this list.
> +	 *
> +	 * When the device is unplugged, all groups that remain in this list must have an extra
> +	 * put_group() called on them to release the reference owned by the per-file group pool.
> +	 */
> +	struct list_head user_node;
>  };
>  
>  struct panthor_job_profiling_data {
> @@ -969,6 +980,7 @@ static void group_release(struct kref *kref)
>  	struct panthor_device *ptdev = group->ptdev;
>  
>  	drm_WARN_ON(&ptdev->base, group->csg_id >= 0);
> +	drm_WARN_ON(&ptdev->base, !list_empty(&group->user_node));
>  	drm_WARN_ON(&ptdev->base, !list_empty(&group->run_node));
>  	drm_WARN_ON(&ptdev->base, !list_empty(&group->wait_node));
>  
> @@ -1003,7 +1015,7 @@ group_can_run(struct panthor_group *group)
>  {
>  	return group->state != PANTHOR_CS_GROUP_TERMINATED &&
>  	       group->state != PANTHOR_CS_GROUP_UNKNOWN_STATE &&
> -	       !group->destroyed &&
> +	       !list_empty(&group->user_node) &&
>  	       !atomic_read(&group->fatal_queues) &&
>  	       !atomic_read(&group->timedout);
>  }
> @@ -2472,7 +2484,7 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
>  			 * re-evaluate as soon as possible and get rid of
>  			 * this dangling group.
>  			 */
> -			if (group->destroyed)
> +			if (list_empty(&group->user_node))
>  				ctx->immediate_tick = true;
>  			group_put(group);
>  		}
> @@ -3667,6 +3679,7 @@ int panthor_group_create(struct panthor_file *pfile,
>  	group->tiler_core_mask = group_args->tiler_core_mask;
>  	group->priority = group_args->priority;
>  
> +	INIT_LIST_HEAD(&group->user_node);
>  	INIT_LIST_HEAD(&group->wait_node);
>  	INIT_LIST_HEAD(&group->run_node);
>  	INIT_WORK(&group->term_work, group_term_work);
> @@ -3735,8 +3748,13 @@ int panthor_group_create(struct panthor_file *pfile,
>  	mutex_lock(&sched->reset.lock);
>  	if (atomic_read(&sched->reset.in_progress)) {
>  		panthor_group_stop(group);
> +
> +		mutex_lock(&sched->lock);
> +		list_add_tail(&group->user_node, &sched->groups.user_owned);
> +		mutex_unlock(&sched->lock);
>  	} else {
>  		mutex_lock(&sched->lock);
> +		list_add_tail(&group->user_node, &sched->groups.user_owned);
>  		list_add_tail(&group->run_node,
>  			      &sched->groups.idle[group->priority]);
>  		mutex_unlock(&sched->lock);
> @@ -3776,7 +3794,7 @@ int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
>  
>  	mutex_lock(&sched->reset.lock);
>  	mutex_lock(&sched->lock);
> -	group->destroyed = true;
> +	list_del_init(&group->user_node);
>  	if (group->csg_id >= 0) {
>  		sched_queue_delayed_work(sched, tick, 0);
>  	} else if (!atomic_read(&sched->reset.in_progress)) {
> @@ -4143,6 +4161,7 @@ int panthor_sched_init(struct panthor_device *ptdev)
>  		INIT_LIST_HEAD(&sched->groups.idle[prio]);
>  	}
>  	INIT_LIST_HEAD(&sched->groups.waiting);
> +	INIT_LIST_HEAD(&sched->groups.user_owned);
>  
>  	ret = drmm_mutex_init(&ptdev->base, &sched->reset.lock);
>  	if (ret)
> 
> -- 
> 2.55.0


Adrian Larumbe