[RFC v4 12/16] drm/sched: Remove idle entity from tree

[Philipp Stanner]

On Fri, 2025-04-25 at 11:20 +0100, Tvrtko Ursulin wrote:
> There is no need to keep entities with no jobs in the tree so lets
> remove
> it once the last job is consumed. This keeps the tree smaller which
> is
> nicer and more efficient as entities are removed and re-added on
> every
> popped job.

That there is no need to do so doesn't imply that you can't keep them
around. The commit message doesn't make the motivation clear

> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Danilo Krummrich <dakr at kernel.org>
> Cc: Matthew Brost <matthew.brost at intel.com>
> Cc: Philipp Stanner <phasta at kernel.org>
> ---
>  drivers/gpu/drm/scheduler/sched_rq.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)

Since this doesn't simplify the code base, I think the only
justification would be a somewhat decent performance gain. Does this
patch result in that?

Otherwise it's probably better to keep git-blame intact here.

P.

> 
> diff --git a/drivers/gpu/drm/scheduler/sched_rq.c
> b/drivers/gpu/drm/scheduler/sched_rq.c
> index d477a027feb9..2cde89cf25fb 100644
> --- a/drivers/gpu/drm/scheduler/sched_rq.c
> +++ b/drivers/gpu/drm/scheduler/sched_rq.c
> @@ -149,25 +149,27 @@ void drm_sched_rq_pop_entity(struct
> drm_sched_entity *entity)
>  {
>  	struct drm_sched_job *next_job;
>  	struct drm_sched_rq *rq;
> -	ktime_t ts;
>  
>  	/*
>  	 * Update the entity's location in the min heap according to
>  	 * the timestamp of the next job, if any.
>  	 */
> +	spin_lock(&entity->lock);
> +	rq = entity->rq;
> +	spin_lock(&rq->lock);
>  	next_job = drm_sched_entity_queue_peek(entity);
> -	if (!next_job)
> -		return;
> +	if (next_job) {
> +		ktime_t ts;
>  
> -	if (drm_sched_policy == DRM_SCHED_POLICY_FIFO)
> -		ts = next_job->submit_ts;
> -	else
> -		ts = drm_sched_rq_get_rr_deadline(rq);
> +		if (drm_sched_policy == DRM_SCHED_POLICY_FIFO)
> +			ts = next_job->submit_ts;
> +		else
> +			ts = drm_sched_rq_get_rr_deadline(rq);
>  
> -	spin_lock(&entity->lock);
> -	rq = entity->rq;
> -	spin_lock(&rq->lock);
> -	drm_sched_rq_update_fifo_locked(entity, rq, ts);
> +		drm_sched_rq_update_fifo_locked(entity, rq, ts);
> +	} else {
> +		drm_sched_rq_remove_fifo_locked(entity, rq);
> +	}
>  	spin_unlock(&rq->lock);
>  	spin_unlock(&entity->lock);
>  }