Hi Tvrtko,
On 23/06/25 09:27, Tvrtko Ursulin wrote:
Round-robin being the non-default policy and unclear how much it is used,
we can notice that it can be implemented using the FIFO data structures if
we only invent a fake submit timestamp which is monotonically increasing
inside drm_sched_rq instances.
So instead of remembering which was the last entity the scheduler worker
picked, we can bump the picked one to the bottom of the tree, achieving
the same round-robin behaviour.
Advantage is that we can consolidate to a single code path and remove a
bunch of code. Downside is round-robin mode now needs to lock on the job
pop path but that should not be visible.
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 | 45 ++++++++------
drivers/gpu/drm/scheduler/sched_main.c | 76 ++----------------------
include/drm/gpu_scheduler.h | 5 +-
3 files changed, 36 insertions(+), 90 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
b/drivers/gpu/drm/scheduler/sched_entity.c
index 0bdf52e27461..e69176765697 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -470,9 +470,19 @@ drm_sched_job_dependency(struct drm_sched_job *job,
return NULL;
}
+static ktime_t
+drm_sched_rq_get_rr_deadline(struct drm_sched_rq *rq)
+{
+ lockdep_assert_held(&rq->lock);
+
+ rq->rr_deadline = ktime_add_ns(rq->rr_deadline, 1);
+
+ return rq->rr_deadline;
+}
+
struct drm_sched_job *drm_sched_entity_pop_job(struct drm_sched_entity
*entity)
{
- struct drm_sched_job *sched_job;
+ struct drm_sched_job *sched_job, *next_job;
sched_job = drm_sched_entity_queue_peek(entity);
if (!sched_job)
@@ -507,21 +517,22 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct
drm_sched_entity *entity)
* Update the entity's location in the min heap according to
* the timestamp of the next job, if any.
*/
- if (drm_sched_policy == DRM_SCHED_POLICY_FIFO) {
- struct drm_sched_job *next;
+ next_job = drm_sched_entity_queue_peek(entity);
+ if (next_job) {
+ struct drm_sched_rq *rq;
+ ktime_t ts;
- next = drm_sched_entity_queue_peek(entity);
- if (next) {
- struct drm_sched_rq *rq;
+ if (drm_sched_policy == DRM_SCHED_POLICY_FIFO)
+ ts = next_job->submit_ts;
+ else
+ ts = drm_sched_rq_get_rr_deadline(rq);
I know that this chunk is going to be deleted in the next patches.
However, to keep things bisectable, I believe you need to set `rq =
entity->rq;` before calling `drm_sched_rq_get_rr_deadline(rq)`.
Otherwise, rq is undefined.
Moreover, IIUC you need to hold rq->lock to call
`drm_sched_rq_get_rr_deadline()`.
- spin_lock(&entity->lock);
- rq = entity->rq;
- spin_lock(&rq->lock);
- drm_sched_rq_update_fifo_locked(entity, rq,
- next->submit_ts);
- spin_unlock(&rq->lock);
- spin_unlock(&entity->lock);
- }
+ 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);
}
/* Jobs and entities might have different lifecycles. Since we're
[...]
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index e62a7214e052..9f8b3b78d24d 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -239,8 +239,7 @@ struct drm_sched_entity {
* struct drm_sched_rq - queue of entities to be scheduled.
*
* @sched: the scheduler to which this rq belongs to.
- * @lock: protects @entities, @rb_tree_root and @current_entity.
- * @current_entity: the entity which is to be scheduled.
+ * @lock: protects @entities, @rb_tree_root and @rr_deadline.
Could you add an entry for @rr_deadline?
Best Regards,
- Maíra
* @entities: list of the entities to be scheduled.
* @rb_tree_root: root of time based priority queue of entities for FIFO
scheduling
*
@@ -253,7 +252,7 @@ struct drm_sched_rq {
spinlock_t lock;
/* Following members are protected by the @lock: */
- struct drm_sched_entity *current_entity;
+ ktime_t rr_deadline;
struct list_head entities;
struct rb_root_cached rb_tree_root;
};
