Re: [PATCH 01/28] drm/i915: Adjust the sentinel assert to match implementation

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

 




On 09/06/2020 11:47, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2020-06-09 11:39:11)

On 09/06/2020 11:29, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2020-06-09 07:59:27)
666
On 08/06/2020 10:33, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2020-06-08 08:44:01)

On 07/06/2020 23:20, Chris Wilson wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Sentinels are supposed to be last reqeusts in the elsp queue, not the
only one, so adjust the assert accordingly.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
     drivers/gpu/drm/i915/gt/intel_lrc.c | 14 +++-----------
     1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index d55a5e0466e5..db8a170b0e5c 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1635,9 +1635,9 @@ assert_pending_valid(const struct intel_engine_execlists *execlists,
                 ccid = ce->lrc.ccid;
/*
-              * Sentinels are supposed to be lonely so they flush the
-              * current exection off the HW. Check that they are the
-              * only request in the pending submission.
+              * Sentinels are supposed to be the last request so they flush
+              * the current exection off the HW. Check that they are the only
+              * request in the pending submission.
                  */
                 if (sentinel) {
                         GEM_TRACE_ERR("%s: context:%llx after sentinel in pending[%zd]\n",
@@ -1646,15 +1646,7 @@ assert_pending_valid(const struct intel_engine_execlists *execlists,
                                       port - execlists->pending);
                         return false;
                 }
-
                 sentinel = i915_request_has_sentinel(rq);

FWIW I was changing it to "sentinel |= ..." so it keeps working if we
decide to use more than 2 elsp ports on Icelake one day.

But it will always fail on the next port...

I don't follow. Sentinel has to be last so if it fails on the next port
it is correct to do so, no?

Exactly. We only check the first port after setting sentinel, if that
port is occupied we fail. Hence why we don't need |=, since there is no
continuation.

But if more than two ports we also overwrite the bools so: sentinel,
non-sentinel, sentinel would not catch. I was just future proofing it. :)

[0] -> sentinel
[1] != NULL -> ERROR

[0] -> not sentinel
[1] -> sentinel
[2] != NULL -> ERROR

We fail if anything comes after a sentinel.

:) Joke is on me.

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