[Intel-gfx] [PATCH i-g-t 05/17] lib: Spin fast, retire early
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Thu Jul 5 15:29:32 UTC 2018
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 at chris-wilson.co.uk>
>>>>> Reviewed-by: Antonio Argenziano <antonio.argenziano at intel.com> #v1
>>>>> Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com> #v1
>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>>> ---
>>>>> 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 at intel.com>
Regards,
Tvrtko
More information about the Intel-gfx
mailing list