[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c87f70879c87b0d29880e32f6632c4c7acd337bb.camel@redhat.com>
Date: Mon, 27 Aug 2018 13:39:02 -0400
From: Lyude Paul <lyude@...hat.com>
To: Jani Nikula <jani.nikula@...ux.intel.com>,
intel-gfx@...ts.freedesktop.org
Cc: Jan-Marek Glogowski <glogow@...home.de>, stable@...r.kernel.org,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
Rodrigo Vivi <rodrigo.vivi@...el.com>,
David Airlie <airlied@...ux.ie>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] drm/i915: Re-apply "Perform link quality check,
unconditionally during long pulse"
On Mon, 2018-08-27 at 11:43 +0300, Jani Nikula wrote:
> On Sat, 25 Aug 2018, Lyude Paul <lyude@...hat.com> wrote:
> > From: Jan-Marek Glogowski <glogow@...home.de>
> >
> > This re-applies the workaround for "some DP sinks, [which] are a
> > little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
> > quality check unconditionally during long pulse").
> > It makes the secondary AOC E2460P monitor connected via DP to an
> > acer Veriton N4640G usable again.
> >
> > This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
> > DP link retraining into the ->post_hotplug() hook")
> >
> > Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into the
> > ->post_hotplug() hook")
> > [Cleaned up commit message, added stable cc]
> > Signed-off-by: Lyude Paul <lyude@...hat.com>
> > Signed-off-by: Jan-Marek Glogowski <glogow@...home.de>
> > Cc: stable@...r.kernel.org
> > ---
> > Resending this to update patchwork; will push in a little bit
>
> Is there a bugzilla? Reference to a list discussion? Something with a
> dmesg where someone can actually verify this is the right fix?
This patch has actually been on the list for a while now-I have had mdnavare
take a look at it as well (they said it looked fine with the only change being
in regards to the comment), and it'd been on the list for a while already.
>
> IMO needs an ack from Ville too. He should be in Cc: in the first place
> as the author of the regressing commit.
>
> 'dim fixes c85d200e8321' gives you the output:
aaah-I had thought it was just for generating the Fixes line, I will be more
careful about that in the future
>
> Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into the
> ->post_hotplug() hook")
> Cc: Manasi Navare <manasi.d.navare@...el.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@...ux.intel.com>
> Cc: Lyude Paul <lyude@...hat.com>
> Cc: Jani Nikula <jani.nikula@...ux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@...el.com>
> Cc: intel-gfx@...ts.freedesktop.org
> Cc: <stable@...r.kernel.org> # v4.17+
>
> BR,
> Jani.
>
> >
> > drivers/gpu/drm/i915/intel_dp.c | 33 +++++++++++++++++++--------------
> > 1 file changed, 19 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index b3f6f04c3c7d..db8515171270 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4333,18 +4333,6 @@ intel_dp_needs_link_retrain(struct intel_dp
> > *intel_dp)
> > return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
> > }
> >
> > -/*
> > - * If display is now connected check links status,
> > - * there has been known issues of link loss triggering
> > - * long pulse.
> > - *
> > - * Some sinks (eg. ASUS PB287Q) seem to perform some
> > - * weird HPD ping pong during modesets. So we can apparently
> > - * end up with HPD going low during a modeset, and then
> > - * going back up soon after. And once that happens we must
> > - * retrain the link to get a picture. That's in case no
> > - * userspace component reacted to intermittent HPD dip.
> > - */
> > int intel_dp_retrain_link(struct intel_encoder *encoder,
> > struct drm_modeset_acquire_ctx *ctx)
> > {
> > @@ -5031,7 +5019,8 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> > }
> >
> > static int
> > -intel_dp_long_pulse(struct intel_connector *connector)
> > +intel_dp_long_pulse(struct intel_connector *connector,
> > + struct drm_modeset_acquire_ctx *ctx)
> > {
> > struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
> > @@ -5090,6 +5079,22 @@ intel_dp_long_pulse(struct intel_connector
> > *connector)
> > */
> > status = connector_status_disconnected;
> > goto out;
> > + } else {
> > + /*
> > + * If display is now connected check links status,
> > + * there has been known issues of link loss triggering
> > + * long pulse.
> > + *
> > + * Some sinks (eg. ASUS PB287Q) seem to perform some
> > + * weird HPD ping pong during modesets. So we can apparently
> > + * end up with HPD going low during a modeset, and then
> > + * going back up soon after. And once that happens we must
> > + * retrain the link to get a picture. That's in case no
> > + * userspace component reacted to intermittent HPD dip.
> > + */
> > + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
> > >base;
> > +
> > + intel_dp_retrain_link(encoder, ctx);
> > }
> >
> > /*
> > @@ -5151,7 +5156,7 @@ intel_dp_detect(struct drm_connector *connector,
> > return ret;
> > }
> >
> > - status = intel_dp_long_pulse(intel_dp->attached_connector);
> > + status = intel_dp_long_pulse(intel_dp->attached_connector,
> > ctx);
> > }
> >
> > intel_dp->detect_done = false;
>
>
--
Cheers,
Lyude Paul
Powered by blists - more mailing lists