Re: [PATCH] drm/i915: Replace some more busy waits with normal ones

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

Should have sent this as RFC..

On 23/03/16 14:32, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> When I added an assert to catch non-atomic users of
> wait_for_atomic_us in 0351b93992aa463cc3e7f358ddec2709f9390756
> ("drm/i915: Do not lie about atomic timeout granularity"),
> I have missed some callers which use it from obviously
> non-atomic context.
>
> Replace them with sleeping waits which support micro-second
> timeout granularity since 3f177625ee896f5d3c62fa6a49554a9c0243bceb
> ("drm/i915: Add wait_for_us").
>
> Note however than a fix for wait_for is needed to a clock with
> more granularity than jiffies. In the above referenced patch
> I have switched the arguments to micro-seconds, but failed to
> upgrade the clock as well, as Mika has later discovered.
>
> Open question here is whether we should allow sleeping waits
> of less than 10us which usleep_range recommends against. And
> this patch actually touches one call site which asks for 1us
> timeout.
>
> These might be better served with wait_for_atomic_us, in which
> case the inatomic warning there should be made dependant on
> the requested timeout.

For discussion - does the above sound like a better plan than this 
patch? To sum up my proposal:

1. Allow wait for_atomic_us for < 10us waits and keep using it for such 
waiters.

2. Upgrade the clock in wait_for to something more precise than jiffies 
so timeouts from 10us and up can be handled properly. Note that 
currently this is  only and issue in the failure/timeout mode. In the 
expected case the current implementation is fine.

Equally as under 1), put a BUILD_BUG_ON in wait_for for <10us waits.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx