Re: [PATCH 4/5] drm/i915: Check live status before reading edid

From: David Weinehall <david.weinehall@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/5] drm/i915: Check live status before reading edid
Date: Tue, 12 Jul 2016 14:39:47 +0300	[thread overview]
Message-ID: <20160712113947.GB27260@boom> (raw)
In-Reply-To: <20150709172757.GE7410@phenom.ffwll.local>

On Thu, Jul 09, 2015 at 07:27:57PM +0200, Daniel Vetter wrote:
> On Thu, Jul 09, 2015 at 05:34:29PM +0530, Sonika Jindal wrote:
> > Adding this for SKL onwards.
> > 
> > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> 
> I think this is the critical piece really, and I'd like to roll it out for
> all platforms. git has the code for all the old ones, so no big deal to
> digg that out. Also we need your fix for the interrupt handling first ofc,
> otherwise this won't work.
> 
> Then we can apply this and give it some good testing before we start
> relying on it with caching hdmi probe status. I know this means that
> things will be slower, but I've been burned a bit too much the last few
> times we've tried this. And I really think we need the most amount of
> testing we can get (hence rolling this out for all platforms). If your
> hpd irq handling bugfix is indeed the bug that will be confirmed quickly,
> otherwise it means back to debugging.

"things will be slower" is a bit of an understatement, sadly.
On machines with no external display connected (the typical usecase for
any device with an eDP, such as a laptop, tablet, etc.), the approach to
repeat live status reads until buggy displays can be bothered to reply
makes us spend twice as long as needed on resume.

While it's nice that we can support buggy hardware, I think that we
also, at the very least, should allow for skipping said those
workarounds when not needed. Either by allow for disabling the
lvie status reads (proven to work on older platforms since that was
the default behaviour for a long, long time, and tested to work
on at least Broadwell & Skylake ThinkPads) or by making the number of
LSR reads configurable.

The former would be the simplest solution, the latter would meet the
letter of the spec, and allow for more precise tuning of behaviour;
people who have a display that needs -- say -- for LSR reads to work
reliably shouldn't have to wait for 9.

While allowing for unusual use-cases / buggy hardware is great,
we shouldn't miss out on the benefits that  working hardware can
give to the common use-cases.

Just my 2¢.

I'm feeling sorely tempted to treat this as a proper regression,
and simply submit a revert patch, seeing as it slows down resume by
something like 200ms, but seeing as there was mention of a case where
incorrect EDID-information might end up being read after resume on some
Chromebooks, I'll just try to open a discussion instead.

Kind regards, David Weinehall
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx