Re: [PATCH v4] drm/i915: Optimistically spin for the request completion

Daniel Vetter
2015-03-23T08:27:42Z

On Fri, Mar 20, 2015 at 04:31:29PM +0000, Tvrtko Ursulin wrote:
>
> On 03/20/2015 04:19 PM, Chris Wilson wrote:
> >On Fri, Mar 20, 2015 at 04:01:52PM +0000, Tvrtko Ursulin wrote:
> >>
> >>On 03/20/2015 02:36 PM, Chris Wilson wrote:
> >>>This provides a nice boost to mesa in swap bound scenarios (as mesa
> >>>throttles itself to the previous frame and given the scenario that will
> >>>complete shortly). It will also provide a good boost to systems running
> >>>with semaphores disabled and so frequently waiting on the GPU as it
> >>>switches rings. In the most favourable of microbenchmarks, this can
> >>>increase performance by around 15% - though in practice improvements
> >>>will be marginal and rarely noticeable.
> >>>
> >>>v2: Account for user timeouts
> >>>v3: Limit the spinning to a single jiffie (~1us) at most. On an
> >>>otherwise idle system, there is no scheduler contention and so without a
> >>>limit we would spin until the GPU is ready.
> >>>v4: Drop forcewake - the lazy coherent access doesn't require it, and we
> >>>have no reason to believe that the forcewake itself improves seqno
> >>>coherency - it only adds delay.
> >>>
> >>>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>>Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> >>>Cc: Eero Tamminen <eero.t.tamminen@intel.com>
> >>>Cc: "Rantala, Valtteri" <valtteri.rantala@intel.com>
> >>
> >>Still against a toggle switch like a simple module parameter?
> >
> >Yes. I'd much rather tackle the corner cases than ignore them.
>
> I'll say it once more then leave it be - my point of view is that module
> param does not mean ignoring any issues. It rather means that, if
> pathological use case if found in the field, you can provide a better user
> experience and then work in parallel in coming with improvements.
>
> Your view is probably that if there is a toggle, someone somewhere will put
> on some wiki "yeah if that happens just put this in modprobe.conf" and there
> won't even be a bug report.

This is generally what indeed happens. We've suffered for years from
google returning outdated wiki pages with "clever" suggestions ...

Nowadays all mod params are generally of the kernel tainting kind to
increase the odds that we get bug reports. I still don't really like them.

> It is a downside yes, but to me much better than finding a bad corner case
> and then saying "Yeah, please recompile your kernel", or downgrade your
> kernel and wait for X weeks/months until the fix propagates.

I agree but only for stuff that causes crashes and hangs. There a module
option makes sense for quicker debug. For general tuning I don't really
want them - the kernel should get things right, period.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation