Re: [PATCH 09/20] drm/sched: use inline locks for the drm-sched-fence

["Christian König" <christian.koenig@amd.com>]

On 11/7/25 09:33, Philipp Stanner wrote:
> On Tue, 2025-11-04 at 15:12 +0000, Tvrtko Ursulin wrote:
>>
>> On 31/10/2025 13:16, Christian König wrote:
>>> Just as proof of concept and minor cleanup.
> 
> That's by the way why I'm asking whether this series is intended as an
> RFC.
> 
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   drivers/gpu/drm/scheduler/sched_fence.c | 11 +++++------
>>>   include/drm/gpu_scheduler.h             |  4 ----
>>>   2 files changed, 5 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/scheduler/sched_fence.c b/drivers/gpu/drm/scheduler/sched_fence.c
>>> index 9391d6f0dc01..7a94e03341cb 100644
>>> --- a/drivers/gpu/drm/scheduler/sched_fence.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_fence.c
>>> @@ -156,19 +156,19 @@ static void drm_sched_fence_set_deadline_finished(struct dma_fence *f,
>>>   	struct dma_fence *parent;
>>>   	unsigned long flags;
>>>   
>>> -	spin_lock_irqsave(&fence->lock, flags);
>>> +	dma_fence_lock(f, flags);
>>
>> Moving to dma_fence_lock should either be a separate patch or squashed 
>> into the one which converts many other drivers. Even a separate patch
>> before that previous patch would be better.
> 
> Yes. +1

Now that I got what was meant here I agree as well.

> 
>>
>> Naming wise, I however still think dma_fence_lock_irqsave would probably 
>> be better to stick with the same pattern everyone is so used too.
> 
> I also think this would be better. Who knows, one day maybe someone
> really wants a lock that definitely must not be irqsave for some
> reason, and then the naming pattern would completely break.

Already done.

> 
>>>
>>>
> 
> […]
> 
>>> @@ -230,9 +229,9 @@ void drm_sched_fence_init(struct drm_sched_fence *fence,
>>>   	fence->sched = entity->rq->sched;
>>>   	seq = atomic_inc_return(&entity->fence_seq);
>>>   	dma_fence_init(&fence->scheduled, &drm_sched_fence_ops_scheduled,
>>> -		       &fence->lock, entity->fence_context, seq);
>>> +		       NULL, entity->fence_context, seq);
>>>   	dma_fence_init(&fence->finished, &drm_sched_fence_ops_finished,
>>> -		       &fence->lock, entity->fence_context + 1, seq);
>>> +		       NULL, entity->fence_context + 1, seq);
>>>   }
> 
> Do we agree that there is no benefit in porting the scheduler to the
> non-shared spinlock?

It saves something like 4 bytes :)

> 
> If so, I really prefer to not do it.

k, going to drop it. On the other hand if I'm not completely mistaken it slightly reduces the complexity.

Regards,
Christian.

> 
> 
> P.