| Message ID | 5dff444a-c041-ebad-6082-b3f19b2ec20a@fbihome.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | v3 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" | expand |
On Wed, 2018-08-08 at 10:53 +0200, Jan-Marek Glogowski wrote: > 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") > > Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de> Just as a heads up I haven't forgotten about this! Coincidentally; it turns out a real life friend of mine is seeing an issue that is very likely fixed by this patch :), so I am going to have them do some checks for me to make sure this fix is OK (it looks fine to me, but you can't be too careful) > --- > 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 8e0e14b..22b2452 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) > { > @@ -4923,7 +4911,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); > @@ -4982,6 +4971,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); > } > > /* > @@ -5043,7 +5048,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;
On Wed, Aug 08, 2018 at 10:53:35AM +0200, Jan-Marek Glogowski wrote: > 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. Would be nice to add in the commit message that this sink sends a long pulse to indicate link loss, hence check link status during long pulse. If there is a FDO bug associated with this you could also add the Bugzilla link before Sign off tag. But other than that this looks good to me. Manasi > > This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST > DP link retraining into the ->post_hotplug() hook") > > Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de> > --- > 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 8e0e14b..22b2452 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) > { > @@ -4923,7 +4911,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); > @@ -4982,6 +4971,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); > } > > /* > @@ -5043,7 +5048,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; > -- > 2.1.4 >
Am August 16, 2018 6:03:50 PM UTC schrieb Manasi Navare <manasi.d.navare@intel.com>: >On Wed, Aug 08, 2018 at 10:53:35AM +0200, Jan-Marek Glogowski wrote: >> 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. > >Would be nice to add in the commit message that this sink sends a >long pulse to indicate link loss, hence check link status during long >pulse. I have no idea, if this is happening. I just found the monitor wasn't working with the current kernel, when I was trying to debug an other bug I ran into while backporting DRM 4.15 to 4.4. I did a bisect while at it. Please read the FDO bug for more info. >If there is a FDO bug associated with this you could also add the >Bugzilla link before Sign off tag. >But other than that this looks good to me. FDO bug: https://bugs.freedesktop.org/show_bug.cgi?id=107446 > >Manasi > >> >> This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST >> DP link retraining into the ->post_hotplug() hook") >> >> Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de> >> --- >> 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 8e0e14b..22b2452 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) >> { >> @@ -4923,7 +4911,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); >> @@ -4982,6 +4971,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); >> } >> >> /* >> @@ -5043,7 +5048,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; >> -- >> 2.1.4 >>
Resend, as this was in my sent-mail folder, but it doesn't appear in the list archive… Am August 16, 2018 6:03:50 PM UTC schrieb Manasi Navare <manasi.d.navare@intel.com>: >On Wed, Aug 08, 2018 at 10:53:35AM +0200, Jan-Marek Glogowski wrote: >> 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. > >Would be nice to add in the commit message that this sink sends a >long pulse to indicate link loss, hence check link status during long >pulse. I have no idea, if this is happening. I just found the monitor wasn't working with the current kernel, when I was trying to debug an other bug I ran into while backporting DRM 4.15 to 4.4. I did a bisect while at it. Please read the FDO bug for more info. >If there is a FDO bug associated with this you could also add the >Bugzilla link before Sign off tag. >But other than that this looks good to me. FDO bug: https://bugs.freedesktop.org/show_bug.cgi?id=107446 Jan-Marek > >Manasi > >> >> This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST >> DP link retraining into the ->post_hotplug() hook") >> >> Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de> >> --- >> 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 8e0e14b..22b2452 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) >> { >> @@ -4923,7 +4911,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); >> @@ -4982,6 +4971,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); >> } >> >> /* >> @@ -5043,7 +5048,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; >> -- >> 2.1.4 >>
On Fri, 2018-08-17 at 13:12 +0200, Jan-Marek Glogowski wrote: > Resend, as this was in my sent-mail folder, but it doesn't appear in the list > archive… > > Am August 16, 2018 6:03:50 PM UTC schrieb Manasi Navare < > manasi.d.navare@intel.com>: > > On Wed, Aug 08, 2018 at 10:53:35AM +0200, Jan-Marek Glogowski wrote: > > > 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. > > > > Would be nice to add in the commit message that this sink sends a > > long pulse to indicate link loss, hence check link status during long > > pulse. > > I have no idea, if this is happening. I just found the monitor wasn't working > with the current > kernel, when I was trying to debug an other bug I ran into while backporting > DRM 4.15 to 4.4. I did > a bisect while at it. Please read the FDO bug for more info. I think you misunderstood-I believe mdnavare was asking if you could put the comment that was on 1a36147bb939 originally in your patch > > > If there is a FDO bug associated with this you could also add the > > Bugzilla link before Sign off tag. > > But other than that this looks good to me. > > FDO bug: https://bugs.freedesktop.org/show_bug.cgi?id=107446 > > Jan-Marek > > > > > Manasi > > > > > > > > This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST > > > DP link retraining into the ->post_hotplug() hook") > > > > > > Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de> > > > --- > > > 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 8e0e14b..22b2452 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) > > > { > > > @@ -4923,7 +4911,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); > > > @@ -4982,6 +4971,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); > > > } > > > > > > /* > > > @@ -5043,7 +5048,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; > > > -- > > > 2.1.4 > > > > >
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 8e0e14b..22b2452 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) { @@ -4923,7 +4911,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); @@ -4982,6 +4971,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); } /* @@ -5043,7 +5048,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;
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") Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de> --- drivers/gpu/drm/i915/intel_dp.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-)