[PATCH 4/6] drm/sched: Move drm_sched_entity_is_ready to internal header

Tvrtko Ursulin tvrtko.ursulin at igalia.com
Wed Feb 12 09:49:57 UTC 2025


On 12/02/2025 09:02, Philipp Stanner wrote:
> btw. I still believe that it would be helpful (and congruent with the
> established norm) to have the version in all patch titles. I do use
> threaded view, but inboxes are huge, and everything that helps you
> orient yourself is welcome
> 
> On Fri, 2025-02-07 at 14:51 +0000, Tvrtko Ursulin wrote:
>> Helper is for scheduler internal use so lets hide it from DRM drivers
>> completely.
>>
>> At the same time we change the method of checking whethere there is
>> anything in the queue from peeking to looking at the node count.
>>
>> 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   | 12 ------------
>>   drivers/gpu/drm/scheduler/sched_internal.h | 13 +++++++++++++
>>   include/drm/gpu_scheduler.h                |  1 -
>>   3 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
>> b/drivers/gpu/drm/scheduler/sched_entity.c
>> index a171f05ad761..87f88259ddf6 100644
>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>> @@ -151,18 +151,6 @@ static bool drm_sched_entity_is_idle(struct
>> drm_sched_entity *entity)
>>   	return false;
>>   }
>>   
>> -/* Return true if entity could provide a job. */
>> -bool drm_sched_entity_is_ready(struct drm_sched_entity *entity)
>> -{
>> -	if (spsc_queue_peek(&entity->job_queue) == NULL)
>> -		return false;
>> -
>> -	if (READ_ONCE(entity->dependency))
>> -		return false;
>> -
>> -	return true;
>> -}
>> -
>>   /**
>>    * drm_sched_entity_error - return error of last scheduled job
>>    * @entity: scheduler entity to check
>> diff --git a/drivers/gpu/drm/scheduler/sched_internal.h
>> b/drivers/gpu/drm/scheduler/sched_internal.h
>> index 25ac62ac2bf3..9ff5cddb5708 100644
>> --- a/drivers/gpu/drm/scheduler/sched_internal.h
>> +++ b/drivers/gpu/drm/scheduler/sched_internal.h
>> @@ -43,4 +43,17 @@ drm_sched_entity_queue_peek(struct
>> drm_sched_entity *entity)
>>   	return container_of(node, struct drm_sched_job, queue_node);
>>   }
>>   
>> +/* Return true if entity could provide a job. */
>> +static inline bool
>> +drm_sched_entity_is_ready(struct drm_sched_entity *entity)
>> +{
>> +	if (!spsc_queue_count(&entity->job_queue))
>> +		return false;
>> +
>> +	if (READ_ONCE(entity->dependency))
>> +		return false;
>> +
>> +	return true;
>> +}
> 
> Is there any particular reason why you want the code in an header
> instead of a new .c file?

IMO it is too trivial to warrant emitting a function.

> We might want to consider this, since I'd expect that the number of
> internal helpers increases over the years.

I actually hope the scheduler code gets smaller and simpler in the 
future. In any case the things hidden in sched_internal.h will be easy 
to refactor since it will be impossible for drivers to grow a dependency 
on them.

Regards,

Tvrtko

>> +
>>   #endif
>> diff --git a/include/drm/gpu_scheduler.h
>> b/include/drm/gpu_scheduler.h
>> index 5188c7f3bd3b..962825613596 100644
>> --- a/include/drm/gpu_scheduler.h
>> +++ b/include/drm/gpu_scheduler.h
>> @@ -606,7 +606,6 @@ struct drm_sched_job
>> *drm_sched_entity_pop_job(struct drm_sched_entity *entity);
>>   void drm_sched_entity_push_job(struct drm_sched_job *sched_job);
>>   void drm_sched_entity_set_priority(struct drm_sched_entity *entity,
>>   				   enum drm_sched_priority
>> priority);
>> -bool drm_sched_entity_is_ready(struct drm_sched_entity *entity);
>>   int drm_sched_entity_error(struct drm_sched_entity *entity);
>>   
>>   struct drm_sched_fence *drm_sched_fence_alloc(
> 



More information about the dri-devel mailing list