On 04/07/2025 14:51, Maíra Canal wrote:
Hi Tvrtko,
On 23/06/25 09:27, Tvrtko Ursulin wrote:
Move the code dealing with entities entering and exiting run queues to
helpers to logically separate it from jobs entering and exiting entities.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@igalia.com>
Cc: Christian König <christian.koe...@amd.com>
Cc: Danilo Krummrich <d...@kernel.org>
Cc: Matthew Brost <matthew.br...@intel.com>
Cc: Philipp Stanner <pha...@kernel.org>
---
drivers/gpu/drm/scheduler/sched_entity.c | 60 ++-------------
drivers/gpu/drm/scheduler/sched_internal.h | 8 +-
drivers/gpu/drm/scheduler/sched_main.c | 87 +++++++++++++++++++---
3 files changed, 83 insertions(+), 72 deletions(-)
[...]
diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/
scheduler/sched_main.c
index 2c85be201605..bfec2e35a2f6 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -145,15 +145,18 @@ static __always_inline bool
drm_sched_entity_compare_before(struct rb_node *a,
static void drm_sched_rq_remove_fifo_locked(struct drm_sched_entity
*entity,
struct drm_sched_rq *rq)
{
+ lockdep_assert_held(&entity->lock);
+ lockdep_assert_held(&rq->lock);
+
Nit: I'm not sure if this change belongs to this patch. I felt a bit out
of context to me. Also, the same assert exists in
`drm_sched_rq_update_fifo_locked()` (maybe remove there?).
You are right - another bad rebase after moving patches around in the
series. Fixed locally.
if (!RB_EMPTY_NODE(&entity->rb_tree_node)) {
rb_erase_cached(&entity->rb_tree_node, &rq->rb_tree_root);
RB_CLEAR_NODE(&entity->rb_tree_node);
}
}
-void drm_sched_rq_update_fifo_locked(struct drm_sched_entity *entity,
- struct drm_sched_rq *rq,
- ktime_t ts)
+static void drm_sched_rq_update_fifo_locked(struct drm_sched_entity
*entity,
+ struct drm_sched_rq *rq,
+ ktime_t ts)
{
/*
* Both locks need to be grabbed, one to protect from entity->rq
change
@@ -188,25 +191,58 @@ static void drm_sched_rq_init(struct
drm_gpu_scheduler *sched,
rq->sched = sched;
}
[...]
Missing kerneldoc.
You got me there for a while, I always look up from the review comment.
:) Fixed locally, thanks!
Regards,
Tvrtko
+void drm_sched_rq_pop_entity(struct drm_sched_entity *entity)
+{
+ struct drm_sched_job *next_job;
+ struct drm_sched_rq *rq;
+ ktime_t ts;
+
+ /*
+ * Update the entity's location in the min heap according to
+ * the timestamp of the next job, if any.
+ */
+ next_job = drm_sched_entity_queue_peek(entity);
+ if (!next_job)
+ return;
+
+ if (drm_sched_policy == DRM_SCHED_POLICY_FIFO)
+ ts = next_job->submit_ts;
+ else
+ ts = drm_sched_rq_get_rr_deadline(rq);
Same as previous patch :)
Best Regards,
- Maíra
+
+ spin_lock(&entity->lock);
+ rq = entity->rq;
+ spin_lock(&rq->lock);
+ drm_sched_rq_update_fifo_locked(entity, rq, ts);
+ spin_unlock(&rq->lock);
+ spin_unlock(&entity->lock);
+}
+
/**
* drm_sched_rq_select_entity - Select an entity which provides a
job to run
*
