On 12/12/2025 15:50, Christian König wrote:
On 12/11/25 16:13, Tvrtko Ursulin wrote:

On 11/12/2025 13:16, Christian König wrote:
Using the inline lock is now the recommended way for dma_fence implementations.

So use this approach for the scheduler fences as well just in case if
anybody uses this as blueprint for its own implementation.

Also saves about 4 bytes for the external spinlock.

Signed-off-by: Christian König <[email protected]>
---
   drivers/gpu/drm/scheduler/sched_fence.c | 7 +++----
   include/drm/gpu_scheduler.h             | 4 ----
   2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/sched_fence.c 
b/drivers/gpu/drm/scheduler/sched_fence.c
index 08ccbde8b2f5..47471b9e43f9 100644
--- a/drivers/gpu/drm/scheduler/sched_fence.c
+++ b/drivers/gpu/drm/scheduler/sched_fence.c
@@ -161,7 +161,7 @@ static void drm_sched_fence_set_deadline_finished(struct 
dma_fence *f,
       /* If we already have an earlier deadline, keep it: */
       if (test_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags) &&
           ktime_before(fence->deadline, deadline)) {
-        spin_unlock_irqrestore(&fence->lock, flags);
+        dma_fence_unlock_irqrestore(f, flags);

Rebase error I guess. Pull into the locking helpers patch.

No that is actually completely intentional here.

Previously we had a separate lock which protected both the DMA-fences as well 
as the deadline state.

Now we turn that upside down by dropping the separate lock and protecting the 
deadline state with the dma_fence lock instead.

I don't follow. The code is currently like this:

static void drm_sched_fence_set_deadline_finished(struct dma_fence *f,
                                                  ktime_t deadline)
{
        struct drm_sched_fence *fence = to_drm_sched_fence(f);
        struct dma_fence *parent;
        unsigned long flags;

        spin_lock_irqsave(&fence->lock, flags);

        /* If we already have an earlier deadline, keep it: */
        if (test_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags) &&
            ktime_before(fence->deadline, deadline)) {
                spin_unlock_irqrestore(&fence->lock, flags);
                return;
        }

        fence->deadline = deadline;
        set_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags);

        spin_unlock_irqrestore(&fence->lock, flags);...

The diff changes one out of the three lock/unlock operations. Other two are changed in 3/19. All three should surely be changed in the same patch.

Regards,

Tvrtko


Regards,
Christian.


Regards,

Tvrtko

           return;
       }
   @@ -217,7 +217,6 @@ struct drm_sched_fence *drm_sched_fence_alloc(struct 
drm_sched_entity *entity,
         fence->owner = owner;
       fence->drm_client_id = drm_client_id;
-    spin_lock_init(&fence->lock);
         return fence;
   }
@@ -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);
   }
     module_init(drm_sched_fence_slab_init);
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index fb88301b3c45..b77f24a783e3 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -297,10 +297,6 @@ struct drm_sched_fence {
            * belongs to.
            */
       struct drm_gpu_scheduler    *sched;
-        /**
-         * @lock: the lock used by the scheduled and the finished fences.
-         */
-    spinlock_t            lock;
           /**
            * @owner: job owner for debugging
            */



Reply via email to