Re: [PATCH 04/17] drm/i915: Tweak scheduler's kick_submission()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/03/2020 11:00, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2020-03-10 10:07:33)

On 06/03/2020 13:38, Chris Wilson wrote:
Skip useless priority bumping on adding a new dependency, but otherwise
prevent tasklet scheduling until we have completed all the potential
rescheduling.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
   drivers/gpu/drm/i915/i915_scheduler.c | 7 ++++++-
   1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 52f71e83e088..603cba36d6a4 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -209,6 +209,8 @@ static void kick_submission(struct intel_engine_cs *engine,
       if (!inflight)
               goto unlock;
+ engine->execlists.queue_priority_hint = prio;
+

What is the significance of moving this up? I couldn't correlate it to
the commit message.

It's correcting the priority bumping. If we have the same context as
active, we should not be skipping the hint update and so avoid the useless
bump on a later dependency.

       /*
        * If we are already the currently executing context, don't
        * bother evaluating if we should preempt ourselves.
@@ -216,7 +218,6 @@ static void kick_submission(struct intel_engine_cs *engine,
       if (inflight->context == rq->context)
               goto unlock;
- engine->execlists.queue_priority_hint = prio;
       if (need_preempt(prio, rq_prio(inflight)))
               tasklet_hi_schedule(&engine->execlists.tasklet);
@@ -463,11 +464,15 @@ int i915_sched_node_add_dependency(struct i915_sched_node *node,
       if (!dep)
               return -ENOMEM;
+ local_bh_disable();
+
       if (!__i915_sched_node_add_dependency(node, signal, dep,
                                             I915_DEPENDENCY_EXTERNAL |
                                             I915_DEPENDENCY_ALLOC))
               i915_dependency_free(dep);
+ local_bh_enable(); /* kick submission tasklet */
+

And this presumably postpones the tasklet until __bump_priority ->
__i915_schedule is finished. But then why the request submission path
which calls __i915_sched_node_add_dependency directly does not need this
treatment?

Because we haven't completed the updates by that point, and upon actual
request submission the tasklet is flushed. Plus on not all request
submission paths would it be legal.

Okay,

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Regards,

Tvrtko


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux