Quoting Tvrtko Ursulin (2018-07-05 16:29:32) > > 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.. Rather, as I see it unless it _impacts_ their test, ignorance is bliss. Having opted into using SPIN_FAST and their test failing on the farm on some machines, I don't expect it to be hard for a novice to debug. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
