diff mbox series

[v2,2/6] drm/i915/dp: Restrict link retrain workaround to external monitors

Message ID 20180924224528.4744-2-dhinakaran.pandiyan@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() | expand

Commit Message

Dhinakaran Pandiyan Sept. 24, 2018, 10:45 p.m. UTC
Commit '3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check,
unconditionally during long pulse"")' applies a work around for sinks
that don't signal link loss. The work around does not need to have to be
that broad as the issue was seen with only one particular monitor; limit
this only for external displays as eDP features like PSR turn off the link
and the driver ends up retraining the link seeeing that link is not
synchronized.

Cc: Lyude Paul <lyude@redhat.com>
Cc: Jan-Marek Glogowski <glogow@fbihome.de>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Rodrigo Vivi Oct. 1, 2018, 7:49 p.m. UTC | #1
On Mon, Sep 24, 2018 at 03:45:24PM -0700, Dhinakaran Pandiyan wrote:
> Commit '3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check,
> unconditionally during long pulse"")' applies a work around for sinks

Fixes: ?

Should this patch be on drm-intel-fixes for 4.19?

If so, the cherry-pick failed anyway and a backported version is needed.

Please let me know how to proceed here.

Thanks in advance,
Rodrigo.

> that don't signal link loss. The work around does not need to have to be
> that broad as the issue was seen with only one particular monitor; limit
> this only for external displays as eDP features like PSR turn off the link
> and the driver ends up retraining the link seeeing that link is not
> synchronized.
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Jan-Marek Glogowski <glogow@fbihome.de>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 34c561011e7a..6130d05d8b88 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5072,12 +5072,13 @@ intel_dp_long_pulse(struct intel_connector *connector,
>  		 */
>  		status = connector_status_disconnected;
>  		goto out;
> -	} else {
> -		/*
> -		 * Some external monitors do not signal loss of link
> -		 * synchronization with an IRQ_HPD, so force a link status
> -		 * check.
> -		 */
> +	}
> +
> +	/*
> +	 * Some external monitors do not signal loss of link synchronization
> +	 * with an IRQ_HPD, so force a link status check.
> +	 */
> +	if (!intel_dp_is_edp(intel_dp)) {
>  		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>  
>  		intel_dp_retrain_link(encoder, ctx);
> -- 
> 2.17.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Dhinakaran Pandiyan Oct. 1, 2018, 10 p.m. UTC | #2
On Mon, 2018-10-01 at 12:49 -0700, Rodrigo Vivi wrote:
> On Mon, Sep 24, 2018 at 03:45:24PM -0700, Dhinakaran Pandiyan wrote:
> > Commit '3cf71bc9904d ("drm/i915: Re-apply "Perform link quality
> > check,
> > unconditionally during long pulse"")' applies a work around for
> > sinks
> 
> Fixes: ?

I had second thoughts about Fixes: when I wrote the patch, but now I
think it does make sense to cherry-pick to v4.19. 

> 
> Should this patch be on drm-intel-fixes for 4.19?
> 
> If so, the cherry-pick failed anyway and a backported version is
> needed.

I'll send a backported a version.

Thanks
DK
> 
> Please let me know how to proceed here.
> 
> Thanks in advance,
> Rodrigo.
> 
> > that don't signal link loss. The work around does not need to have
> > to be
> > that broad as the issue was seen with only one particular monitor;
> > limit
> > this only for external displays as eDP features like PSR turn off
> > the link
> > and the driver ends up retraining the link seeeing that link is not
> > synchronized.
> > 
> > Cc: Lyude Paul <lyude@redhat.com>
> > Cc: Jan-Marek Glogowski <glogow@fbihome.de>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality
> > check, unconditionally during long pulse"")
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 34c561011e7a..6130d05d8b88 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5072,12 +5072,13 @@ intel_dp_long_pulse(struct intel_connector
> > *connector,
> >  		 */
> >  		status = connector_status_disconnected;
> >  		goto out;
> > -	} else {
> > -		/*
> > -		 * Some external monitors do not signal loss of link
> > -		 * synchronization with an IRQ_HPD, so force a link
> > status
> > -		 * check.
> > -		 */
> > +	}
> > +
> > +	/*
> > +	 * Some external monitors do not signal loss of link
> > synchronization
> > +	 * with an IRQ_HPD, so force a link status check.
> > +	 */
> > +	if (!intel_dp_is_edp(intel_dp)) {
> >  		struct intel_encoder *encoder =
> > &dp_to_dig_port(intel_dp)->base;
> >  
> >  		intel_dp_retrain_link(encoder, ctx);
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lyude Paul Oct. 1, 2018, 10:30 p.m. UTC | #3
Reviewed-by: Lyude Paul <lyude@redhat.com>

On Mon, 2018-09-24 at 15:45 -0700, Dhinakaran Pandiyan wrote:
> Commit '3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check,
> unconditionally during long pulse"")' applies a work around for sinks
> that don't signal link loss. The work around does not need to have to be
> that broad as the issue was seen with only one particular monitor; limit
> this only for external displays as eDP features like PSR turn off the link
> and the driver ends up retraining the link seeeing that link is not
> synchronized.
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Jan-Marek Glogowski <glogow@fbihome.de>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check,
> unconditionally during long pulse"")
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 34c561011e7a..6130d05d8b88 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5072,12 +5072,13 @@ intel_dp_long_pulse(struct intel_connector
> *connector,
>  		 */
>  		status = connector_status_disconnected;
>  		goto out;
> -	} else {
> -		/*
> -		 * Some external monitors do not signal loss of link
> -		 * synchronization with an IRQ_HPD, so force a link status
> -		 * check.
> -		 */
> +	}
> +
> +	/*
> +	 * Some external monitors do not signal loss of link synchronization
> +	 * with an IRQ_HPD, so force a link status check.
> +	 */
> +	if (!intel_dp_is_edp(intel_dp)) {
>  		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
> >base;
>  
>  		intel_dp_retrain_link(encoder, ctx);
Dhinakaran Pandiyan Oct. 2, 2018, 12:30 a.m. UTC | #4
On Mon, 2018-10-01 at 12:49 -0700, Rodrigo Vivi wrote:
> On Mon, Sep 24, 2018 at 03:45:24PM -0700, Dhinakaran Pandiyan wrote:
> > Commit '3cf71bc9904d ("drm/i915: Re-apply "Perform link quality
> > check,
> > unconditionally during long pulse"")' applies a work around for
> > sinks
> 
> Fixes: ?
> 
> Should this patch be on drm-intel-fixes for 4.19?
> 
> If so, the cherry-pick failed anyway and a backported version is
> needed.
> 
> Please let me know how to proceed here.

Looks like the patch can be cherry-picked cleanly if 1/6 was also
picked. So, the relevant commits in dinq are - 

f24f6eb95807 drm/i915/dp: Restrict link retrain workaround to external
monitors
9ebd8202393d drm/i915/dp: Fix link retraining comment in
intel_dp_long_pulse()


-DK

> 
> Thanks in advance,
> Rodrigo.
> 
> > that don't signal link loss. The work around does not need to have
> > to be
> > that broad as the issue was seen with only one particular monitor;
> > limit
> > this only for external displays as eDP features like PSR turn off
> > the link
> > and the driver ends up retraining the link seeeing that link is not
> > synchronized.
> > 
> > Cc: Lyude Paul <lyude@redhat.com>
> > Cc: Jan-Marek Glogowski <glogow@fbihome.de>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality
> > check, unconditionally during long pulse"")
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 34c561011e7a..6130d05d8b88 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5072,12 +5072,13 @@ intel_dp_long_pulse(struct intel_connector
> > *connector,
> >  		 */
> >  		status = connector_status_disconnected;
> >  		goto out;
> > -	} else {
> > -		/*
> > -		 * Some external monitors do not signal loss of link
> > -		 * synchronization with an IRQ_HPD, so force a link
> > status
> > -		 * check.
> > -		 */
> > +	}
> > +
> > +	/*
> > +	 * Some external monitors do not signal loss of link
> > synchronization
> > +	 * with an IRQ_HPD, so force a link status check.
> > +	 */
> > +	if (!intel_dp_is_edp(intel_dp)) {
> >  		struct intel_encoder *encoder =
> > &dp_to_dig_port(intel_dp)->base;
> >  
> >  		intel_dp_retrain_link(encoder, ctx);
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula Oct. 23, 2018, 11:56 a.m. UTC | #5
On Mon, 01 Oct 2018, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> On Mon, 2018-10-01 at 12:49 -0700, Rodrigo Vivi wrote:
>> On Mon, Sep 24, 2018 at 03:45:24PM -0700, Dhinakaran Pandiyan wrote:
>> > Commit '3cf71bc9904d ("drm/i915: Re-apply "Perform link quality
>> > check,
>> > unconditionally during long pulse"")' applies a work around for
>> > sinks
>> 
>> Fixes: ?
>> 
>> Should this patch be on drm-intel-fixes for 4.19?
>> 
>> If so, the cherry-pick failed anyway and a backported version is
>> needed.
>> 
>> Please let me know how to proceed here.
>
> Looks like the patch can be cherry-picked cleanly if 1/6 was also
> picked. So, the relevant commits in dinq are - 
>
> f24f6eb95807 drm/i915/dp: Restrict link retrain workaround to external
> monitors
> 9ebd8202393d drm/i915/dp: Fix link retraining comment in
> intel_dp_long_pulse()

Looks like the ball got dropped here.

Joonas, please cherry-pick these to dinf (in reverse order to make them
apply), and add

Fixes: 399334708b4f ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")
Cc: stable@vger.kernel.org

To both to make them follow that commit wherever it goes. As is, I can't
even make a backport request because they're not upstream yet.


BR,
Jani.
Joonas Lahtinen Oct. 24, 2018, 10:49 a.m. UTC | #6
Quoting Jani Nikula (2018-10-23 14:56:13)
> On Mon, 01 Oct 2018, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> > On Mon, 2018-10-01 at 12:49 -0700, Rodrigo Vivi wrote:
> >> On Mon, Sep 24, 2018 at 03:45:24PM -0700, Dhinakaran Pandiyan wrote:
> >> > Commit '3cf71bc9904d ("drm/i915: Re-apply "Perform link quality
> >> > check,
> >> > unconditionally during long pulse"")' applies a work around for
> >> > sinks
> >> 
> >> Fixes: ?
> >> 
> >> Should this patch be on drm-intel-fixes for 4.19?
> >> 
> >> If so, the cherry-pick failed anyway and a backported version is
> >> needed.
> >> 
> >> Please let me know how to proceed here.
> >
> > Looks like the patch can be cherry-picked cleanly if 1/6 was also
> > picked. So, the relevant commits in dinq are - 
> >
> > f24f6eb95807 drm/i915/dp: Restrict link retrain workaround to external
> > monitors
> > 9ebd8202393d drm/i915/dp: Fix link retraining comment in
> > intel_dp_long_pulse()
> 
> Looks like the ball got dropped here.
> 
> Joonas, please cherry-pick these to dinf (in reverse order to make them
> apply), and add
> 
> Fixes: 399334708b4f ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")
> Cc: stable@vger.kernel.org
> 
> To both to make them follow that commit wherever it goes. As is, I can't
> even make a backport request because they're not upstream yet.

Cherry picked these now.

Regards, Joonas

> 
> 
> BR,
> Jani.
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 34c561011e7a..6130d05d8b88 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5072,12 +5072,13 @@  intel_dp_long_pulse(struct intel_connector *connector,
 		 */
 		status = connector_status_disconnected;
 		goto out;
-	} else {
-		/*
-		 * Some external monitors do not signal loss of link
-		 * synchronization with an IRQ_HPD, so force a link status
-		 * check.
-		 */
+	}
+
+	/*
+	 * Some external monitors do not signal loss of link synchronization
+	 * with an IRQ_HPD, so force a link status check.
+	 */
+	if (!intel_dp_is_edp(intel_dp)) {
 		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 
 		intel_dp_retrain_link(encoder, ctx);