[PATCH v5 08/16] drm/sched: Consolidate entity run queue management

[Maíra Canal]

Hi Tvrtko,

On 23/06/25 09:27, Tvrtko Ursulin wrote:
> Move the code dealing with entities entering and exiting run queues to
> helpers to logically separate it from jobs entering and exiting entities.
> 
> 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_entity.c   | 60 ++-------------
>   drivers/gpu/drm/scheduler/sched_internal.h |  8 +-
>   drivers/gpu/drm/scheduler/sched_main.c     | 87 +++++++++++++++++++---
>   3 files changed, 83 insertions(+), 72 deletions(-)
> 

[...]

> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
> index 2c85be201605..bfec2e35a2f6 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -145,15 +145,18 @@ static __always_inline bool drm_sched_entity_compare_before(struct rb_node *a,
>   static void drm_sched_rq_remove_fifo_locked(struct drm_sched_entity *entity,
>   					    struct drm_sched_rq *rq)
>   {
> +	lockdep_assert_held(&entity->lock);
> +	lockdep_assert_held(&rq->lock);
> +

Nit: I'm not sure if this change belongs to this patch. I felt a bit out
of context to me. Also, the same assert exists in
`drm_sched_rq_update_fifo_locked()` (maybe remove there?).

>   	if (!RB_EMPTY_NODE(&entity->rb_tree_node)) {
>   		rb_erase_cached(&entity->rb_tree_node, &rq->rb_tree_root);
>   		RB_CLEAR_NODE(&entity->rb_tree_node);
>   	}
>   }
>   
> -void drm_sched_rq_update_fifo_locked(struct drm_sched_entity *entity,
> -				     struct drm_sched_rq *rq,
> -				     ktime_t ts)
> +static void drm_sched_rq_update_fifo_locked(struct drm_sched_entity *entity,
> +					    struct drm_sched_rq *rq,
> +					    ktime_t ts)
>   {
>   	/*
>   	 * Both locks need to be grabbed, one to protect from entity->rq change
> @@ -188,25 +191,58 @@ static void drm_sched_rq_init(struct drm_gpu_scheduler *sched,
>   	rq->sched = sched;
>   }
>   

[...]

>   

Missing kerneldoc.

> +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.
> +	 */
> +	next_job = drm_sched_entity_queue_peek(entity);
> +	if (!next_job)
> +		return;
> +
> +	if (drm_sched_policy == DRM_SCHED_POLICY_FIFO)
> +		ts = next_job->submit_ts;
> +	else
> +		ts = drm_sched_rq_get_rr_deadline(rq);

Same as previous patch :)

Best Regards,
- Maíra

> +
> +	spin_lock(&entity->lock);
> +	rq = entity->rq;
> +	spin_lock(&rq->lock);
> +	drm_sched_rq_update_fifo_locked(entity, rq, ts);
> +	spin_unlock(&rq->lock);
> +	spin_unlock(&entity->lock);
> +}
> +
>   /**
>    * drm_sched_rq_select_entity - Select an entity which provides a job to run
>    *