[PATCH v5 06/16] drm/sched: Consolidate drm_sched_rq_select_entity_rr

[Tvrtko Ursulin]

On 04/07/2025 14:32, Maíra Canal wrote:
> Hi Tvrtko,
> 
> On 23/06/25 09:27, Tvrtko Ursulin wrote:
>> Extract out two copies of the identical code to function epilogue to make
>> it smaller and more readable.
>>
>> 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_main.c | 48 +++++++++++---------------
>>   1 file changed, 20 insertions(+), 28 deletions(-)
>>
> 
> I'd consider dropping this patch. The function
> drm_sched_rq_select_entity_rr() will be deleted in the next patch
> anyway. Therefore, we could land 7/16 in drm-misc-next (even before
> landing fair scheduling) and get rid of drm_sched_rq_select_entity_rr().

If "drm/sched: Implement RR via FIFO" gets a green light I could indeed 
drop this one. It was there for the case RR-via-FIFO wouldn't be 
accepted. Wait a bit longer and then decide?

Regards,

Tvrtko

>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/ 
>> scheduler/sched_main.c
>> index 0627a82c02dc..436d58073823 100644
>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>> @@ -263,38 +263,14 @@ drm_sched_rq_select_entity_rr(struct 
>> drm_gpu_scheduler *sched,
>>       entity = rq->current_entity;
>>       if (entity) {
>>           list_for_each_entry_continue(entity, &rq->entities, list) {
>> -            if (drm_sched_entity_is_ready(entity)) {
>> -                /* If we can't queue yet, preserve the current
>> -                 * entity in terms of fairness.
>> -                 */
>> -                if (!drm_sched_can_queue(sched, entity)) {
>> -                    spin_unlock(&rq->lock);
>> -                    return ERR_PTR(-ENOSPC);
>> -                }
>> -
>> -                rq->current_entity = entity;
>> -                reinit_completion(&entity->entity_idle);
>> -                spin_unlock(&rq->lock);
>> -                return entity;
>> -            }
>> +            if (drm_sched_entity_is_ready(entity))
>> +                goto found;
>>           }
>>       }
>>       list_for_each_entry(entity, &rq->entities, list) {
>> -        if (drm_sched_entity_is_ready(entity)) {
>> -            /* If we can't queue yet, preserve the current entity in
>> -             * terms of fairness.
>> -             */
>> -            if (!drm_sched_can_queue(sched, entity)) {
>> -                spin_unlock(&rq->lock);
>> -                return ERR_PTR(-ENOSPC);
>> -            }
>> -
>> -            rq->current_entity = entity;
>> -            reinit_completion(&entity->entity_idle);
>> -            spin_unlock(&rq->lock);
>> -            return entity;
>> -        }
>> +        if (drm_sched_entity_is_ready(entity))
>> +            goto found;
>>           if (entity == rq->current_entity)
>>               break;
>> @@ -303,6 +279,22 @@ drm_sched_rq_select_entity_rr(struct 
>> drm_gpu_scheduler *sched,
>>       spin_unlock(&rq->lock);
>>       return NULL;
>> +
>> +found:
>> +    if (!drm_sched_can_queue(sched, entity)) {
>> +        /*
>> +         * If scheduler cannot take more jobs signal the caller to not
>> +         * consider lower priority queues.
>> +         */
>> +        entity = ERR_PTR(-ENOSPC);
>> +    } else {
>> +        rq->current_entity = entity;
>> +        reinit_completion(&entity->entity_idle);
>> +    }
>> +
>> +    spin_unlock(&rq->lock);
>> +
>> +    return entity;
>>   }
>>   /**
>