All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: David Weinehall <david.weinehall@linux.intel.com>,
	Chris Wilson <chris@chris-wilson.co.uk>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] Revert "drm/i915: Check live status before reading edid"
Date: Wed, 07 Sep 2016 14:56:51 +0300	[thread overview]
Message-ID: <87zinjx6rg.fsf@intel.com> (raw)
In-Reply-To: <20160819113136.iwni5xnp3amcur54@boom>

On Fri, 19 Aug 2016, David Weinehall <david.weinehall@linux.intel.com> wrote:
> On Thu, Aug 18, 2016 at 10:29:43AM +0300, David Weinehall wrote:
>> On Wed, Aug 17, 2016 at 04:43:36PM +0300, Jani Nikula wrote:
>> > On Wed, 17 Aug 2016, Chris Wilson <chris@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@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@chris-wilson.co.uk>
>> > > Cc: stable@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].
>
> [snip]
>
> Actually, I think it's safe to do the revert.
> A bit more testing yields that it's not a generic
> issue for Skylake, it seems to be specific to
> ThinkPads (or even a subset of them; I haven't
> got more than one model to test on).
>
> The gmbus never sends a NAK if there's nothing connected,
> so eventually gmbus_wait_for_status() gives up and we fall
> back to using the bitbanging method instead.
>
> Since things still work (albeit with reduced performance)
> on ThinkPads, and other platforms don't exhibit this
> behaviour, I suggest we move forward and revert
> the live status check.
>
> Chris has a partial fix for the problem; it fixes
> the first port (which is purely an HDMI-port), but
> the second port -- which I believe is routed through
> the dock connector and/or the type C USB-port,
> still remains problematic.

Pushed to drm-intel-next-queued after procrastination, thanks for the
patch.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2016-09-07 11:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-17 12:47 [PATCH] Revert "drm/i915: Check live status before reading edid" David Weinehall
2016-08-17 13:02 ` Chris Wilson
2016-08-17 13:43   ` Jani Nikula
2016-08-18  7:29     ` David Weinehall
2016-08-19 11:31       ` David Weinehall
2016-09-07 11:56         ` Jani Nikula [this message]
2016-08-17 14:22 ` ✗ Ro.CI.BAT: failure for " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87zinjx6rg.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=david.weinehall@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.