Re: [PATCH i-g-t 05/17] lib: Spin fast, retire early

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

 




On 05/07/2018 13:42, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-07-05 13:33:55)

On 05/07/2018 12:23, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-07-02 16:36:28)

On 02/07/2018 10:07, Chris Wilson wrote:
When using the pollable spinner, we often want to use it as a means of
ensuring the task is running on the GPU before switching to something
else. In which case we don't want to add extra delay inside the spinner,
but the current 1000 NOPs add on order of 5us, which is often larger
than the target latency.

v2: Don't change perf_pmu as that is sensitive to the extra CPU latency
from a tight GPU spinner.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Antonio Argenziano <antonio.argenziano@xxxxxxxxx> #v1
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> #v1
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
    lib/igt_dummyload.c       | 3 ++-
    lib/igt_dummyload.h       | 1 +
    tests/gem_ctx_isolation.c | 1 +
    tests/gem_eio.c           | 1 +
    tests/gem_exec_latency.c  | 4 ++--
    5 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 94efdf745..7beb66244 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -199,7 +199,8 @@ emit_recursive_batch(igt_spin_t *spin,
         * between function calls, that appears enough to keep SNB out of
         * trouble. See https://bugs.freedesktop.org/show_bug.cgi?id=102262
         */
-     batch += 1000;
+     if (!(opts->flags & IGT_SPIN_FAST))
+             batch += 1000;

igt_require(!snb) or something, given the comment whose last two lines
can be seen in the diff above?

It's not a machine killer since we have the required fix in the kernel,
it just has interesting system latency. That latency is not specific to
snb, and whether we want to trade system latency vs gpu latency is the
reason we punt the decision to the caller.

I am not sure if the comment and linked BZ is then still relevant?

It is, it's just too specific, and it particularly affects modesetting as
it doesn't defend itself against CPU starvation / timer latency (imo).
If it is, it is not nice to punt the responsibility to the caller. We
are exporting new API and it is hard to think callers will dig into the
implementation to find it (this comment).

Why not? Is not the sordid history of our hw supposed to be our
specialist topic? :) For what has once happened before is sure to happen
again. (Corollary to George Santayana.)
So a info/warn/skip when using the feature on a platform where it
doesn't work well is I think much friendlier approach.

But for those who want to use it, it is just noise.

For current users yes, for new users maybe not - maybe they need to have a way to notice they might be doing things unknowingly suboptimally. What would accomplish that goal? igt_info too much noise? igt_debug too low key? Blah..

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]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux