Re: [PATCH v6 05/15] drm/sched: Consolidate drm_sched_rq_select_entity_rr

["Danilo Krummrich" <dakr@kernel.org>]

On Tue Jul 8, 2025 at 1:31 PM CEST, Philipp Stanner wrote:
> On Tue, 2025-07-08 at 10:51 +0100, 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@igalia.com>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Danilo Krummrich <dakr@kernel.org>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: Philipp Stanner <phasta@kernel.org>
>
> Can also be branched out.

There seems to be something going on with your mail client. I saw it a couple of
times now that it somehow messes up the original code in your replies, such as
below.

>> ---
>>  drivers/gpu/drm/scheduler/sched_main.c | 48 +++++++++++-------------
>> --
>>  1 file changed, 20 insertions(+), 28 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
>> b/drivers/gpu/drm/scheduler/sched_main.c
>> index 2335df4de2b8..9212ebae7476 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;
>>  }
>>  
>>  /**