[Intel-gfx] [PATCH] Revert "drm/i915: Check live status before reading edid"

[David Weinehall]

On Wed, Aug 17, 2016 at 04:43:36PM +0300, Jani Nikula wrote:
> On Wed, 17 Aug 2016, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > On Wed, Aug 17, 2016 at 03:47:48PM +0300, David Weinehall wrote:
> >> This reverts commit 237ed86c693d8a8e4db476976aeb30df4deac74b.
> >> 
> >> Our current implementation of live status check (repeat 9 times
> >> with 10ms delays between each attempt as a workaround for
> >> buggy displays) imposes a rather serious penalty, time wise,
> >> on intel_hdmi_detect().  Since we we already skip live status
> >> checks on platforms before gen 7, and since we seem to have
> >> coped quite well before the live status check was introduced
> >> for newer platforms too, the previous behaviour is probably
> >> preferable, at least unless someone can point to a use-case
> >> that the live status check improves (apart from "Bspec says so".
> >> 
> >> Signed-off-by: David Weinehall <david.weinehall at linux.intel.com>
> >
> > Fixes: 237ed86c693d ("drm/i915: Check live status before reading edid")
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97139
> > Acked-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: stable at vger.kernel.org
> 
> And we've come full circle on live status. Again.
> 
> References: https://upload.wikimedia.org/wikipedia/commons/3/3b/Paris_Tuileries_Garden_Facepalm_statue.jpg

Actually, we might have to take (at least) one more
lap around the circle.

I did some more benchmarks, this time on Cherryview,
and two different Skylake models (one ThinkPad 13,
one NUC i5).

The results are rather frustrating:

On Skylake gmbus_wait_hw_status() seems to be
really expensive[1].

So expensive that unless we can figure out how to
get it down, the live status check is still
the lesser of two evils. In a scenario with only
sensible displays and no displays, the solution
would be simple enough -- just eliminate the retries,
keep the live status check, since it seems to work
as it should on Skylake (haven't tried on Broxton
or Kabylake yet, so I'm not sure whether it's all
gen9 platforms, or only Skylake).

Reality, however, indicates that not all displays
are sensible, so where (almost) back on square one.

Clearly we can optimise away the live status checks
completely on < (INTEL_GEN(dev_priv) < 7 ||
IS_IVYBRIDGE(dev_priv))[2]. My testing on Broadwell
suggests that this might even be possible to
extend to < (INTEL_GEN(dev_priv) < 9).

That leaves gen9+ though, and seeing as those
platforms are the current and future, respectively,
priorities, we clearly need a solution for those
too.

So, anyone with any theory why gmbus_wait_hw_status()
is horribly slow on Skylake?


Kind regards, David

[1] More than the live_status loop, *if* msleep(10)
    only sleeps 10ms. If we keep that loop we might
    want to consider using usleep_range() instead, to
    get better precision -- I've experienced
    msleep(10) taking up to 20ms.

[2] As in the patch I already submitted earlier.