Re: [PATCH 07/16] drm/i915/gt: Check for a completed last request once

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

 




On 24/11/2020 17:38, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2020-11-24 17:19:23)

On 24/11/2020 11:42, Chris Wilson wrote:
Pull the repeated check for the last active request being completed to a
single spot, when deciding whether or not execlist preemption is
required.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
   drivers/gpu/drm/i915/gt/intel_lrc.c | 15 ++++-----------
   1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index cf11cbac241b..43703efb36d1 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2141,12 +2141,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
        */
if ((last = *active)) {
-             if (need_preempt(engine, last, rb)) {
-                     if (i915_request_completed(last)) {
-                             tasklet_hi_schedule(&execlists->tasklet);
-                             return;
-                     }
-
+             if (i915_request_completed(last)) {
+                     goto check_secondary;
+             } else if (need_preempt(engine, last, rb)) {
                       ENGINE_TRACE(engine,
                                    "preempting last=%llx:%lld, prio=%d, hint=%d\n",
                                    last->fence.context,
@@ -2174,11 +2171,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
                       last = NULL;
               } else if (need_timeslice(engine, last, rb) &&
                          timeslice_expired(execlists, last)) {
-                     if (i915_request_completed(last)) {
-                             tasklet_hi_schedule(&execlists->tasklet);
-                             return;
-                     }
-
                       ENGINE_TRACE(engine,
                                    "expired last=%llx:%lld, prio=%d, hint=%d, yield?=%s\n",
                                    last->fence.context,
@@ -2214,6 +2206,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
                        * we hopefully coalesce several updates into a single
                        * submission.
                        */
+check_secondary:
                       if (!list_is_last(&last->sched.link,
                                         &engine->active.requests)) {

Is there a tasklet_hi_schedule in here? I see set_timeslice in my checkout.

That tasklet_hi_schedule() was a mistake. It just devolves into a
busy-spinner as we wait for HW to send the next event, which turns out
not to be as instantaneous as hoped.

I recall leaving the imprint of my palm on my face when I figured out
what was causing the spike in the profile.

Okay, if you mention that in the commit message you can add my rb.

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