Am 09.01.25 um 14:20 schrieb Tvrtko Ursulin:
On 09/01/2025 12:49, Christian König wrote:
Am 08.01.25 um 19:35 schrieb Tvrtko Ursulin:
It is not helping readability nor it is required to keep the line
length
in check.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx>
Cc: Christian König <christian.koenig@xxxxxxx>
Cc: Danilo Krummrich <dakr@xxxxxxxxxx>
Cc: Matthew Brost <matthew.brost@xxxxxxxxx>
Cc: Philipp Stanner <pstanner@xxxxxxxxxx>
---
drivers/gpu/drm/scheduler/sched_main.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index 1734c17aeea5..01e0d6e686d1 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -1175,7 +1175,6 @@ static void drm_sched_run_job_work(struct
work_struct *w)
container_of(w, struct drm_gpu_scheduler, work_run_job);
struct drm_sched_entity *entity;
struct dma_fence *fence;
- struct drm_sched_fence *s_fence;
struct drm_sched_job *sched_job;
int r;
@@ -1194,15 +1193,13 @@ static void drm_sched_run_job_work(struct
work_struct *w)
return;
}
- s_fence = sched_job->s_fence;
-
atomic_add(sched_job->credits, &sched->credit_count);
drm_sched_job_begin(sched_job);
trace_drm_run_job(sched_job, entity);
fence = sched->ops->run_job(sched_job);
complete_all(&entity->entity_idle);
- drm_sched_fence_scheduled(s_fence, fence);
+ drm_sched_fence_scheduled(sched_job->s_fence, fence);
Originally that was not for readability but for correctness.
As soon as complete_all(&entity->entity_idle); was called the
sched_job could have been released.
And without a comment ouch.
That changed long long time ago and IIRC we did had a comment for that.
But we changed that so that the sched_job is released from a separate
worker instead, so that is most likely not necessary any more.
Very subtle. Especially given some drivers use unordered queue.
Hui? unordered queue? How should that work?
Job submission ordering is a mandatory requirement of the dma_fence.
And for them sched_job is dereferenced a few more times in the block
below so not sure how it is safe.
Move complete_all() to the end of it all?
Most likely good idea, yes.
Regards,
Christian.
Regards,
Tvrtko
if (!IS_ERR_OR_NULL(fence)) {
/* Drop for original kref_init of the fence */
