[RFC 16/18] drm/sched: Connect with dma-fence deadlines

[Tvrtko Ursulin]

On 09/01/2025 13:07, Christian König wrote:
> Am 08.01.25 um 19:35 schrieb Tvrtko Ursulin:
>> Now that the scheduling policy is deadline based it feels completely
>> natural to allow propagating externaly set deadlines to the scheduler.
>>
>> Scheduler deadlines are not a guarantee but as the dma-fence facility is
>> already in use by userspace lets wire it up.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
>> Cc: Christian König <christian.koenig at amd.com>
>> Cc: Danilo Krummrich <dakr at redhat.com>
>> Cc: Matthew Brost <matthew.brost at intel.com>
>> Cc: Philipp Stanner <pstanner at redhat.com>
>> Cc: Rob Clark <robdclark at gmail.com>
>> ---
>>   drivers/gpu/drm/scheduler/sched_entity.c | 24 ++++++++++++++++++++++--
>>   1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c 
>> b/drivers/gpu/drm/scheduler/sched_entity.c
>> index 98c78d1373d8..db5d34310b18 100644
>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>> @@ -410,7 +410,16 @@ ktime_t
>>   drm_sched_entity_get_job_deadline(struct drm_sched_entity *entity,
>>                     struct drm_sched_job *job)
>>   {
>> -    return __drm_sched_entity_get_job_deadline(entity, job->submit_ts);
>> +    struct drm_sched_fence *s_fence = job->s_fence;
>> +    struct dma_fence *fence = &s_fence->finished;
>> +    ktime_t deadline;
>> +
>> +    deadline = __drm_sched_entity_get_job_deadline(entity, 
>> job->submit_ts);
>> +    if (test_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, 
>> &fence->flags) &&
>> +        ktime_before(s_fence->deadline, deadline))
>> +        deadline = s_fence->deadline;
>> +
>> +    return deadline;
>>   }
>>   /*
>> @@ -579,9 +588,12 @@ void drm_sched_entity_select_rq(struct 
>> drm_sched_entity *entity)
>>    */
>>   void drm_sched_entity_push_job(struct drm_sched_job *sched_job)
>>   {
>> +    struct drm_sched_fence *s_fence = sched_job->s_fence;
>>       struct drm_sched_entity *entity = sched_job->entity;
>> -    bool first;
>> +    struct dma_fence *fence = &s_fence->finished;
>> +    ktime_t fence_deadline;
>>       ktime_t submit_ts;
>> +    bool first;
>>       trace_drm_sched_job(sched_job, entity);
>>       atomic_inc(entity->rq->sched->score);
>> @@ -593,6 +605,11 @@ void drm_sched_entity_push_job(struct 
>> drm_sched_job *sched_job)
>>        * Make sure to set the submit_ts first, to avoid a race.
>>        */
>>       sched_job->submit_ts = submit_ts = ktime_get();
>> +    if (test_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &fence->flags))
>> +        fence_deadline = s_fence->deadline;
>> +    else
>> +        fence_deadline = KTIME_MAX;
>> +
> 
> That makes no sense. When the job is pushed the fence is not made public 
> yet.
> 
> So no deadline can be set on the fence.

You are correct, the push side of things was a mistake a laziness that I 
did not remove it from the RFC.

>>       first = spsc_queue_push(&entity->job_queue, 
>> &sched_job->queue_node);
>>       /* first job wakes up scheduler */
>> @@ -601,6 +618,9 @@ void drm_sched_entity_push_job(struct 
>> drm_sched_job *sched_job)
>>           submit_ts = __drm_sched_entity_get_job_deadline(entity,
>>                                   submit_ts);
>> +        if (ktime_before(fence_deadline, submit_ts))
>> +            submit_ts = fence_deadline;
>> +
> 
> Yeah, that won't work at all as far as I can see.

It works from the pop side though.

When job N is scheduled, deadline is taken from N+1 and tree 
re-balanced. At the point of N scheduling N+1 can definitely have a real 
deadline set.

What does not work is for queue depth of one. No way at the moment to 
"bump" the entity in the tree while N is waiting for submission because 
we cannot dereference the entity from the job. (I had that in v1 of the 
series and realized it was unsafe.)

I (very) briefly though about reference counting entities but quickly 
had a feeling it would be annoying. So for now this patch only offers a 
partial solution.

Regards,

Tvrtko

>>           sched = drm_sched_rq_add_entity(entity->rq, entity, submit_ts);
>>           if (sched)
>>               drm_sched_wakeup(sched);
>