[PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse()
@ 2018-09-24 22:45 Dhinakaran Pandiyan
  2018-09-24 22:45 ` [PATCH v2 2/6] drm/i915/dp: Restrict link retrain workaround to external monitors Dhinakaran Pandiyan
                   ` (12 more replies)
  0 siblings, 13 replies; 30+ messages in thread
From: Dhinakaran Pandiyan @ 2018-09-24 22:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jan-Marek Glogowski, Dhinakaran Pandiyan, Rodrigo Vivi

Comment claims link needs to be retrained because the connected sink raised
a long pulse to indicate link loss. If the sink did so,
intel_dp_hotplug() would have handled link retraining. Looking at the
logs in Bugzilla referenced in commit '3cf71bc9904d ("drm/i915: Re-apply
Perform link quality check, unconditionally during long pulse"")', the
issue is that the sink does not trigger an interrupt. What we want is
->detect() from user space to check link status and retrain. Ville's
review for the original patch also indicates the same root cause. So,
rewrite the comment.

v2: Patch split and rewrote comment.

Cc: Lyude Paul <lyude@redhat.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Jan-Marek Glogowski <glogow@fbihome.de>
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, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6b4c19123f2a..34c561011e7a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5074,16 +5074,9 @@ intel_dp_long_pulse(struct intel_connector *connector,
 		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.
+		 * Some external monitors do not signal loss of link
+		 * synchronization with an IRQ_HPD, so force a link status
+		 * check.
 		 */
 		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 2/6] drm/i915/dp: Restrict link retrain workaround to external monitors
  2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
@ 2018-09-24 22:45 ` Dhinakaran Pandiyan
  2018-10-01 19:49   ` Rodrigo Vivi
  2018-10-01 22:30   ` Lyude Paul
  2018-09-24 22:45 ` [PATCH v2 3/6] drm/i915/dp: Use a local variable for intel_encoder * Dhinakaran Pandiyan
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 30+ messages in thread
From: Dhinakaran Pandiyan @ 2018-09-24 22:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jan-Marek Glogowski, Dhinakaran Pandiyan, Rodrigo Vivi

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);
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/6] drm/i915/dp: Restrict link retrain workaround to external monitors
  2018-09-24 22:45 ` [PATCH v2 2/6] drm/i915/dp: Restrict link retrain workaround to external monitors Dhinakaran Pandiyan
@ 2018-10-01 19:49   ` Rodrigo Vivi
  2018-10-01 22:00     ` Dhinakaran Pandiyan
  2018-10-02  0:30     ` Dhinakaran Pandiyan
  2018-10-01 22:30   ` Lyude Paul
  1 sibling, 2 replies; 30+ messages in thread
From: Rodrigo Vivi @ 2018-10-01 19:49 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: Jan-Marek Glogowski, intel-gfx

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/6] drm/i915/dp: Restrict link retrain workaround to external monitors
  2018-10-01 19:49   ` Rodrigo Vivi
@ 2018-10-01 22:00     ` Dhinakaran Pandiyan
  2018-10-02  0:30     ` Dhinakaran Pandiyan
  1 sibling, 0 replies; 30+ messages in thread
From: Dhinakaran Pandiyan @ 2018-10-01 22:00 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Jan-Marek Glogowski, intel-gfx

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/6] drm/i915/dp: Restrict link retrain workaround to external monitors
  2018-10-01 19:49   ` Rodrigo Vivi
  2018-10-01 22:00     ` Dhinakaran Pandiyan
@ 2018-10-02  0:30     ` Dhinakaran Pandiyan
  2018-10-23 11:56       ` Jani Nikula
  1 sibling, 1 reply; 30+ messages in thread
From: Dhinakaran Pandiyan @ 2018-10-02  0:30 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Jan-Marek Glogowski, intel-gfx

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/6] drm/i915/dp: Restrict link retrain workaround to external monitors
  2018-10-02  0:30     ` Dhinakaran Pandiyan
@ 2018-10-23 11:56       ` Jani Nikula
  2018-10-24 10:49         ` Joonas Lahtinen
  0 siblings, 1 reply; 30+ messages in thread
From: Jani Nikula @ 2018-10-23 11:56 UTC (permalink / raw)
  To: dhinakaran.pandiyan, Rodrigo Vivi; +Cc: Jan-Marek Glogowski, intel-gfx

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.

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/6] drm/i915/dp: Restrict link retrain workaround to external monitors
  2018-10-23 11:56       ` Jani Nikula
@ 2018-10-24 10:49         ` Joonas Lahtinen
  0 siblings, 0 replies; 30+ messages in thread
From: Joonas Lahtinen @ 2018-10-24 10:49 UTC (permalink / raw)
  To: Jani Nikula, Rodrigo Vivi, dhinakaran.pandiyan
  Cc: Jan-Marek Glogowski, intel-gfx

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 2/6] drm/i915/dp: Restrict link retrain workaround to external monitors
  2018-09-24 22:45 ` [PATCH v2 2/6] drm/i915/dp: Restrict link retrain workaround to external monitors Dhinakaran Pandiyan
  2018-10-01 19:49   ` Rodrigo Vivi
@ 2018-10-01 22:30   ` Lyude Paul
  1 sibling, 0 replies; 30+ messages in thread
From: Lyude Paul @ 2018-10-01 22:30 UTC (permalink / raw)
  To: Dhinakaran Pandiyan, intel-gfx; +Cc: Jan-Marek Glogowski, Rodrigo Vivi

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);
-- 
Cheers,
	Lyude Paul

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 3/6] drm/i915/dp: Use a local variable for intel_encoder *
  2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
  2018-09-24 22:45 ` [PATCH v2 2/6] drm/i915/dp: Restrict link retrain workaround to external monitors Dhinakaran Pandiyan
@ 2018-09-24 22:45 ` Dhinakaran Pandiyan
  2018-09-25 20:51   ` Souza, Jose
  2018-09-24 22:45 ` [PATCH v2 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect() Dhinakaran Pandiyan
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 30+ messages in thread
From: Dhinakaran Pandiyan @ 2018-09-24 22:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan

We have two cases of intel_dp to intel_encoder conversions, use a
local variable to store the conversion.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6130d05d8b88..09229fc66dec 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5017,6 +5017,7 @@ intel_dp_long_pulse(struct intel_connector *connector,
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
+	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 	enum drm_connector_status status;
 	u8 sink_irq_vector = 0;
 
@@ -5027,7 +5028,7 @@ intel_dp_long_pulse(struct intel_connector *connector,
 	/* Can't disconnect eDP */
 	if (intel_dp_is_edp(intel_dp))
 		status = edp_detect(intel_dp);
-	else if (intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base))
+	else if (intel_digital_port_connected(encoder))
 		status = intel_dp_detect_dpcd(intel_dp);
 	else
 		status = connector_status_disconnected;
@@ -5078,11 +5079,8 @@ intel_dp_long_pulse(struct intel_connector *connector,
 	 * 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;
-
+	if (!intel_dp_is_edp(intel_dp))
 		intel_dp_retrain_link(encoder, ctx);
-	}
 
 	/*
 	 * Clearing NACK and defer counts to get their exact values
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 3/6] drm/i915/dp: Use a local variable for intel_encoder *
  2018-09-24 22:45 ` [PATCH v2 3/6] drm/i915/dp: Use a local variable for intel_encoder * Dhinakaran Pandiyan
@ 2018-09-25 20:51   ` Souza, Jose
  0 siblings, 0 replies; 30+ messages in thread
From: Souza, Jose @ 2018-09-25 20:51 UTC (permalink / raw)
  To: intel-gfx, Pandiyan, Dhinakaran

On Mon, 2018-09-24 at 15:45 -0700, Dhinakaran Pandiyan wrote:
> We have two cases of intel_dp to intel_encoder conversions, use a
> local variable to store the conversion.
> 

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 6130d05d8b88..09229fc66dec 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5017,6 +5017,7 @@ intel_dp_long_pulse(struct intel_connector
> *connector,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector-
> >base.dev);
>  	struct intel_dp *intel_dp = intel_attached_dp(&connector-
> >base);
> +	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
> >base;
>  	enum drm_connector_status status;
>  	u8 sink_irq_vector = 0;
>  
> @@ -5027,7 +5028,7 @@ intel_dp_long_pulse(struct intel_connector
> *connector,
>  	/* Can't disconnect eDP */
>  	if (intel_dp_is_edp(intel_dp))
>  		status = edp_detect(intel_dp);
> -	else if
> (intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base))
> +	else if (intel_digital_port_connected(encoder))
>  		status = intel_dp_detect_dpcd(intel_dp);
>  	else
>  		status = connector_status_disconnected;
> @@ -5078,11 +5079,8 @@ intel_dp_long_pulse(struct intel_connector
> *connector,
>  	 * 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;
> -
> +	if (!intel_dp_is_edp(intel_dp))
>  		intel_dp_retrain_link(encoder, ctx);
> -	}
>  
>  	/*
>  	 * Clearing NACK and defer counts to get their exact values
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()
  2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
  2018-09-24 22:45 ` [PATCH v2 2/6] drm/i915/dp: Restrict link retrain workaround to external monitors Dhinakaran Pandiyan
  2018-09-24 22:45 ` [PATCH v2 3/6] drm/i915/dp: Use a local variable for intel_encoder * Dhinakaran Pandiyan
@ 2018-09-24 22:45 ` Dhinakaran Pandiyan
  2018-09-25 20:58   ` Souza, Jose
  2018-09-26  2:54   ` [PATCH v3 " Dhinakaran Pandiyan
  2018-09-24 22:45 ` [PATCH v2 5/6] drm/i915/dp: Kill intel_dp->detect_done flag Dhinakaran Pandiyan
                   ` (9 subsequent siblings)
  12 siblings, 2 replies; 30+ messages in thread
From: Dhinakaran Pandiyan @ 2018-09-24 22:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan

A crtc modeset lock was added for link retraining but
intel_dp_retrain_link() knows to take the necessary locks since
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")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 09229fc66dec..87a631098a6d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5079,8 +5079,13 @@ intel_dp_long_pulse(struct intel_connector *connector,
 	 * 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))
-		intel_dp_retrain_link(encoder, ctx);
+	if (!intel_dp_is_edp(intel_dp)) {
+		int ret;
+
+		ret = intel_dp_retrain_link(encoder, ctx);
+		if (ret)
+			return ret;
+	}
 
 	/*
 	 * Clearing NACK and defer counts to get their exact values
@@ -5130,19 +5135,8 @@ intel_dp_detect(struct drm_connector *connector,
 		      connector->base.id, connector->name);
 
 	/* If full detect is not performed yet, do a full detect */
-	if (!intel_dp->detect_done) {
-		struct drm_crtc *crtc;
-		int ret;
-
-		crtc = connector->state->crtc;
-		if (crtc) {
-			ret = drm_modeset_lock(&crtc->mutex, ctx);
-			if (ret)
-				return ret;
-		}
-
+	if (!intel_dp->detect_done)
 		status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
-	}
 
 	intel_dp->detect_done = false;
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()
  2018-09-24 22:45 ` [PATCH v2 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect() Dhinakaran Pandiyan
@ 2018-09-25 20:58   ` Souza, Jose
  2018-09-26  2:54   ` [PATCH v3 " Dhinakaran Pandiyan
  1 sibling, 0 replies; 30+ messages in thread
From: Souza, Jose @ 2018-09-25 20:58 UTC (permalink / raw)
  To: intel-gfx, Pandiyan, Dhinakaran

On Mon, 2018-09-24 at 15:45 -0700, Dhinakaran Pandiyan wrote:
> A crtc modeset lock was added for link retraining but
> intel_dp_retrain_link() knows to take the necessary locks since
> 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")

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 09229fc66dec..87a631098a6d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5079,8 +5079,13 @@ intel_dp_long_pulse(struct intel_connector
> *connector,
>  	 * 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))
> -		intel_dp_retrain_link(encoder, ctx);
> +	if (!intel_dp_is_edp(intel_dp)) {
> +		int ret;
> +
> +		ret = intel_dp_retrain_link(encoder, ctx);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	/*
>  	 * Clearing NACK and defer counts to get their exact values
> @@ -5130,19 +5135,8 @@ intel_dp_detect(struct drm_connector
> *connector,
>  		      connector->base.id, connector->name);
>  
>  	/* If full detect is not performed yet, do a full detect */
> -	if (!intel_dp->detect_done) {
> -		struct drm_crtc *crtc;
> -		int ret;
> -
> -		crtc = connector->state->crtc;
> -		if (crtc) {
> -			ret = drm_modeset_lock(&crtc->mutex, ctx);
> -			if (ret)
> -				return ret;
> -		}
> -
> +	if (!intel_dp->detect_done)
>  		status = intel_dp_long_pulse(intel_dp-
> >attached_connector, ctx);
> -	}
>  
>  	intel_dp->detect_done = false;
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v3 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()
  2018-09-24 22:45 ` [PATCH v2 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect() Dhinakaran Pandiyan
  2018-09-25 20:58   ` Souza, Jose
@ 2018-09-26  2:54   ` Dhinakaran Pandiyan
  2018-10-01 19:48     ` Rodrigo Vivi
  1 sibling, 1 reply; 30+ messages in thread
From: Dhinakaran Pandiyan @ 2018-09-26  2:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan

A crtc modeset lock was added for link retraining but
intel_dp_retrain_link() knows to take the necessary locks since
commit c85d200e8321 ("drm/i915: Move SST DP link retraining into the
->post_hotplug() hook")
v2: Drop AUX power domain reference in the early return path

Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into the ->post_hotplug() hook")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 09229fc66dec..ef28cc0f122c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5079,8 +5079,15 @@ intel_dp_long_pulse(struct intel_connector *connector,
 	 * 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))
-		intel_dp_retrain_link(encoder, ctx);
+	if (!intel_dp_is_edp(intel_dp)) {
+		int ret;
+
+		ret = intel_dp_retrain_link(encoder, ctx);
+		if (ret) {
+			intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+			return ret;
+		}
+	}
 
 	/*
 	 * Clearing NACK and defer counts to get their exact values
@@ -5130,19 +5137,8 @@ intel_dp_detect(struct drm_connector *connector,
 		      connector->base.id, connector->name);
 
 	/* If full detect is not performed yet, do a full detect */
-	if (!intel_dp->detect_done) {
-		struct drm_crtc *crtc;
-		int ret;
-
-		crtc = connector->state->crtc;
-		if (crtc) {
-			ret = drm_modeset_lock(&crtc->mutex, ctx);
-			if (ret)
-				return ret;
-		}
-
+	if (!intel_dp->detect_done)
 		status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
-	}
 
 	intel_dp->detect_done = false;
 
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()
  2018-09-26  2:54   ` [PATCH v3 " Dhinakaran Pandiyan
@ 2018-10-01 19:48     ` Rodrigo Vivi
  2018-10-01 20:10       ` Ville Syrjälä
  0 siblings, 1 reply; 30+ messages in thread
From: Rodrigo Vivi @ 2018-10-01 19:48 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

On Tue, Sep 25, 2018 at 07:54:22PM -0700, Dhinakaran Pandiyan wrote:
> A crtc modeset lock was added for link retraining but
> intel_dp_retrain_link() knows to take the necessary locks since
> commit c85d200e8321 ("drm/i915: Move SST DP link retraining into the
> ->post_hotplug() hook")
> v2: Drop AUX power domain reference in the early return path
> 
> Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into the ->post_hotplug() hook")

This patch failed on cherry-pick for drm-intel-fixes targeting 4.19.

If it still makes sense to have it there please provide a backported version
to drm-intel-fixes or let me know if it has dependency on other patches?

Thanks,
Rodrigo.

> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 09229fc66dec..ef28cc0f122c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5079,8 +5079,15 @@ intel_dp_long_pulse(struct intel_connector *connector,
>  	 * 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))
> -		intel_dp_retrain_link(encoder, ctx);
> +	if (!intel_dp_is_edp(intel_dp)) {
> +		int ret;
> +
> +		ret = intel_dp_retrain_link(encoder, ctx);
> +		if (ret) {
> +			intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> +			return ret;
> +		}
> +	}
>  
>  	/*
>  	 * Clearing NACK and defer counts to get their exact values
> @@ -5130,19 +5137,8 @@ intel_dp_detect(struct drm_connector *connector,
>  		      connector->base.id, connector->name);
>  
>  	/* If full detect is not performed yet, do a full detect */
> -	if (!intel_dp->detect_done) {
> -		struct drm_crtc *crtc;
> -		int ret;
> -
> -		crtc = connector->state->crtc;
> -		if (crtc) {
> -			ret = drm_modeset_lock(&crtc->mutex, ctx);
> -			if (ret)
> -				return ret;
> -		}
> -
> +	if (!intel_dp->detect_done)
>  		status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
> -	}
>  
>  	intel_dp->detect_done = false;
>  
> -- 
> 2.14.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()
  2018-10-01 19:48     ` Rodrigo Vivi
@ 2018-10-01 20:10       ` Ville Syrjälä
  2018-10-01 22:01         ` Dhinakaran Pandiyan
  0 siblings, 1 reply; 30+ messages in thread
From: Ville Syrjälä @ 2018-10-01 20:10 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Dhinakaran Pandiyan

On Mon, Oct 01, 2018 at 12:48:28PM -0700, Rodrigo Vivi wrote:
> On Tue, Sep 25, 2018 at 07:54:22PM -0700, Dhinakaran Pandiyan wrote:
> > A crtc modeset lock was added for link retraining but
> > intel_dp_retrain_link() knows to take the necessary locks since
> > commit c85d200e8321 ("drm/i915: Move SST DP link retraining into the
> > ->post_hotplug() hook")
> > v2: Drop AUX power domain reference in the early return path
> > 
> > Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into the ->post_hotplug() hook")
> 
> This patch failed on cherry-pick for drm-intel-fixes targeting 4.19.
> 
> If it still makes sense to have it there please provide a backported version
> to drm-intel-fixes or let me know if it has dependency on other patches?

This is more of an optimization to not disturb page flips and whatnot
so much with detect. The double lock is otherwise harmless due to ww_mutex.

> 
> Thanks,
> Rodrigo.
> 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++--------------
> >  1 file changed, 10 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 09229fc66dec..ef28cc0f122c 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5079,8 +5079,15 @@ intel_dp_long_pulse(struct intel_connector *connector,
> >  	 * 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))
> > -		intel_dp_retrain_link(encoder, ctx);
> > +	if (!intel_dp_is_edp(intel_dp)) {
> > +		int ret;
> > +
> > +		ret = intel_dp_retrain_link(encoder, ctx);
> > +		if (ret) {
> > +			intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> > +			return ret;
> > +		}
> > +	}
> >  
> >  	/*
> >  	 * Clearing NACK and defer counts to get their exact values
> > @@ -5130,19 +5137,8 @@ intel_dp_detect(struct drm_connector *connector,
> >  		      connector->base.id, connector->name);
> >  
> >  	/* If full detect is not performed yet, do a full detect */
> > -	if (!intel_dp->detect_done) {
> > -		struct drm_crtc *crtc;
> > -		int ret;
> > -
> > -		crtc = connector->state->crtc;
> > -		if (crtc) {
> > -			ret = drm_modeset_lock(&crtc->mutex, ctx);
> > -			if (ret)
> > -				return ret;
> > -		}
> > -
> > +	if (!intel_dp->detect_done)
> >  		status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
> > -	}
> >  
> >  	intel_dp->detect_done = false;
> >  
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()
  2018-10-01 20:10       ` Ville Syrjälä
@ 2018-10-01 22:01         ` Dhinakaran Pandiyan
  2018-10-01 23:22           ` Rodrigo Vivi
  0 siblings, 1 reply; 30+ messages in thread
From: Dhinakaran Pandiyan @ 2018-10-01 22:01 UTC (permalink / raw)
  To: Ville Syrjälä, Rodrigo Vivi; +Cc: intel-gfx

On Mon, 2018-10-01 at 23:10 +0300, Ville Syrjälä wrote:
> On Mon, Oct 01, 2018 at 12:48:28PM -0700, Rodrigo Vivi wrote:
> > On Tue, Sep 25, 2018 at 07:54:22PM -0700, Dhinakaran Pandiyan
> > wrote:
> > > A crtc modeset lock was added for link retraining but
> > > intel_dp_retrain_link() knows to take the necessary locks since
> > > commit c85d200e8321 ("drm/i915: Move SST DP link retraining into
> > > the
> > > ->post_hotplug() hook")
> > > v2: Drop AUX power domain reference in the early return path
> > > 
> > > Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into
> > > the ->post_hotplug() hook")
> > 
> > This patch failed on cherry-pick for drm-intel-fixes targeting
> > 4.19.
> > 
> > If it still makes sense to have it there please provide a
> > backported version
> > to drm-intel-fixes or let me know if it has dependency on other
> > patches?
> 
> This is more of an optimization to not disturb page flips and whatnot
> so much with detect. The double lock is otherwise harmless due to
> ww_mutex.
> 
Yeah, let's not backport this.

-DK

> > 
> > Thanks,
> > Rodrigo.
> > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com
> > > >
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++--------------
> > >  1 file changed, 10 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 09229fc66dec..ef28cc0f122c 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -5079,8 +5079,15 @@ intel_dp_long_pulse(struct intel_connector
> > > *connector,
> > >  	 * 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))
> > > -		intel_dp_retrain_link(encoder, ctx);
> > > +	if (!intel_dp_is_edp(intel_dp)) {
> > > +		int ret;
> > > +
> > > +		ret = intel_dp_retrain_link(encoder, ctx);
> > > +		if (ret) {
> > > +			intel_display_power_put(dev_priv, intel_dp-
> > > >aux_power_domain);
> > > +			return ret;
> > > +		}
> > > +	}
> > >  
> > >  	/*
> > >  	 * Clearing NACK and defer counts to get their exact values
> > > @@ -5130,19 +5137,8 @@ intel_dp_detect(struct drm_connector
> > > *connector,
> > >  		      connector->base.id, connector->name);
> > >  
> > >  	/* If full detect is not performed yet, do a full detect */
> > > -	if (!intel_dp->detect_done) {
> > > -		struct drm_crtc *crtc;
> > > -		int ret;
> > > -
> > > -		crtc = connector->state->crtc;
> > > -		if (crtc) {
> > > -			ret = drm_modeset_lock(&crtc->mutex, ctx);
> > > -			if (ret)
> > > -				return ret;
> > > -		}
> > > -
> > > +	if (!intel_dp->detect_done)
> > >  		status = intel_dp_long_pulse(intel_dp-
> > > >attached_connector, ctx);
> > > -	}
> > >  
> > >  	intel_dp->detect_done = false;
> > >  
> > > -- 
> > > 2.14.1
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()
  2018-10-01 22:01         ` Dhinakaran Pandiyan
@ 2018-10-01 23:22           ` Rodrigo Vivi
  0 siblings, 0 replies; 30+ messages in thread
From: Rodrigo Vivi @ 2018-10-01 23:22 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

On Mon, Oct 01, 2018 at 03:01:18PM -0700, Dhinakaran Pandiyan wrote:
> On Mon, 2018-10-01 at 23:10 +0300, Ville Syrjälä wrote:
> > On Mon, Oct 01, 2018 at 12:48:28PM -0700, Rodrigo Vivi wrote:
> > > On Tue, Sep 25, 2018 at 07:54:22PM -0700, Dhinakaran Pandiyan
> > > wrote:
> > > > A crtc modeset lock was added for link retraining but
> > > > intel_dp_retrain_link() knows to take the necessary locks since
> > > > commit c85d200e8321 ("drm/i915: Move SST DP link retraining into
> > > > the
> > > > ->post_hotplug() hook")
> > > > v2: Drop AUX power domain reference in the early return path
> > > > 
> > > > Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into
> > > > the ->post_hotplug() hook")
> > > 
> > > This patch failed on cherry-pick for drm-intel-fixes targeting
> > > 4.19.
> > > 
> > > If it still makes sense to have it there please provide a
> > > backported version
> > > to drm-intel-fixes or let me know if it has dependency on other
> > > patches?
> > 
> > This is more of an optimization to not disturb page flips and whatnot
> > so much with detect. The double lock is otherwise harmless due to
> > ww_mutex.
> > 
> Yeah, let's not backport this.

cool, thanks.

> 
> -DK
> 
> > > 
> > > Thanks,
> > > Rodrigo.
> > > 
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com
> > > > >
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++--------------
> > > >  1 file changed, 10 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 09229fc66dec..ef28cc0f122c 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -5079,8 +5079,15 @@ intel_dp_long_pulse(struct intel_connector
> > > > *connector,
> > > >  	 * 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))
> > > > -		intel_dp_retrain_link(encoder, ctx);
> > > > +	if (!intel_dp_is_edp(intel_dp)) {
> > > > +		int ret;
> > > > +
> > > > +		ret = intel_dp_retrain_link(encoder, ctx);
> > > > +		if (ret) {
> > > > +			intel_display_power_put(dev_priv, intel_dp-
> > > > >aux_power_domain);
> > > > +			return ret;
> > > > +		}
> > > > +	}
> > > >  
> > > >  	/*
> > > >  	 * Clearing NACK and defer counts to get their exact values
> > > > @@ -5130,19 +5137,8 @@ intel_dp_detect(struct drm_connector
> > > > *connector,
> > > >  		      connector->base.id, connector->name);
> > > >  
> > > >  	/* If full detect is not performed yet, do a full detect */
> > > > -	if (!intel_dp->detect_done) {
> > > > -		struct drm_crtc *crtc;
> > > > -		int ret;
> > > > -
> > > > -		crtc = connector->state->crtc;
> > > > -		if (crtc) {
> > > > -			ret = drm_modeset_lock(&crtc->mutex, ctx);
> > > > -			if (ret)
> > > > -				return ret;
> > > > -		}
> > > > -
> > > > +	if (!intel_dp->detect_done)
> > > >  		status = intel_dp_long_pulse(intel_dp-
> > > > >attached_connector, ctx);
> > > > -	}
> > > >  
> > > >  	intel_dp->detect_done = false;
> > > >  
> > > > -- 
> > > > 2.14.1
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 5/6] drm/i915/dp: Kill intel_dp->detect_done flag
  2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
                   ` (2 preceding siblings ...)
  2018-09-24 22:45 ` [PATCH v2 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect() Dhinakaran Pandiyan
@ 2018-09-24 22:45 ` Dhinakaran Pandiyan
  2018-09-25 17:53   ` Ville Syrjälä
  2018-09-24 22:45 ` [PATCH v2 6/6] drm/i915/dp: Fix duplication of DEVICE_SERVICE_IRQ handling Dhinakaran Pandiyan
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 30+ messages in thread
From: Dhinakaran Pandiyan @ 2018-09-24 22:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan

The intel_dp->detect_done flag is no more useful. Pull
intel_dp_long_pulse() into the lone caller,

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 43 ++++++++------------------------
 drivers/gpu/drm/i915/intel_drv.h |  1 -
 2 files changed, 11 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 87a631098a6d..d6ea93e453a2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5012,15 +5012,18 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
 }
 
 static int
-intel_dp_long_pulse(struct intel_connector *connector,
-		    struct drm_modeset_acquire_ctx *ctx)
+intel_dp_detect(struct drm_connector *connector,
+		struct drm_modeset_acquire_ctx *ctx,
+		bool force)
 {
-	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
-	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
-	enum drm_connector_status status;
+	int status;
 	u8 sink_irq_vector = 0;
 
+	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
+		      connector->base.id, connector->name);
 	WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
 
 	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
@@ -5096,9 +5099,9 @@ intel_dp_long_pulse(struct intel_connector *connector,
 	intel_dp->aux.i2c_defer_count = 0;
 
 	intel_dp_set_edid(intel_dp);
-	if (intel_dp_is_edp(intel_dp) || connector->detect_edid)
+	if (intel_dp_is_edp(intel_dp) ||
+	    to_intel_connector(connector)->detect_edid)
 		status = connector_status_connected;
-	intel_dp->detect_done = true;
 
 	/* Try to read the source of the interrupt */
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
@@ -5123,26 +5126,6 @@ intel_dp_long_pulse(struct intel_connector *connector,
 	return status;
 }
 
-static int
-intel_dp_detect(struct drm_connector *connector,
-		struct drm_modeset_acquire_ctx *ctx,
-		bool force)
-{
-	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	int status = connector->status;
-
-	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
-		      connector->base.id, connector->name);
-
-	/* If full detect is not performed yet, do a full detect */
-	if (!intel_dp->detect_done)
-		status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
-
-	intel_dp->detect_done = false;
-
-	return status;
-}
-
 static void
 intel_dp_force(struct drm_connector *connector)
 {
@@ -5635,7 +5618,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 
 	if (long_hpd) {
 		intel_dp->reset_link_params = true;
-		intel_dp->detect_done = false;
 		return IRQ_NONE;
 	}
 
@@ -5652,7 +5634,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 			intel_dp->is_mst = false;
 			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
 							intel_dp->is_mst);
-			intel_dp->detect_done = false;
 			goto put_power;
 		}
 	}
@@ -5665,10 +5646,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 		/* Short pulse can signify loss of hdcp authentication */
 		intel_hdcp_check_link(intel_dp->attached_connector);
 
-		if (!handled) {
-			intel_dp->detect_done = false;
+		if (!handled)
 			goto put_power;
-		}
 	}
 
 	ret = IRQ_HANDLED;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8073a85d7178..a7c151c40e7d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1070,7 +1070,6 @@ struct intel_dp {
 	bool link_mst;
 	bool link_trained;
 	bool has_audio;
-	bool detect_done;
 	bool reset_link_params;
 	enum aux_ch aux_ch;
 	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 5/6] drm/i915/dp: Kill intel_dp->detect_done flag
  2018-09-24 22:45 ` [PATCH v2 5/6] drm/i915/dp: Kill intel_dp->detect_done flag Dhinakaran Pandiyan
@ 2018-09-25 17:53   ` Ville Syrjälä
  2018-09-26  2:31     ` Dhinakaran Pandiyan
  0 siblings, 1 reply; 30+ messages in thread
From: Ville Syrjälä @ 2018-09-25 17:53 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

On Mon, Sep 24, 2018 at 03:45:27PM -0700, Dhinakaran Pandiyan wrote:
> The intel_dp->detect_done flag is no more useful. Pull
> intel_dp_long_pulse() into the lone caller,
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 43 ++++++++------------------------
>  drivers/gpu/drm/i915/intel_drv.h |  1 -
>  2 files changed, 11 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 87a631098a6d..d6ea93e453a2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5012,15 +5012,18 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>  }
>  
>  static int
> -intel_dp_long_pulse(struct intel_connector *connector,
> -		    struct drm_modeset_acquire_ctx *ctx)
> +intel_dp_detect(struct drm_connector *connector,
> +		struct drm_modeset_acquire_ctx *ctx,
> +		bool force)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> -	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
> +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> -	enum drm_connector_status status;
> +	int status;

I guess we don't need this change anymore?

Series is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Not sure if ci actually found some issue with the series, or
whether it was just having a bad day.

>  	u8 sink_irq_vector = 0;
>  
> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> +		      connector->base.id, connector->name);
>  	WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
>  
>  	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> @@ -5096,9 +5099,9 @@ intel_dp_long_pulse(struct intel_connector *connector,
>  	intel_dp->aux.i2c_defer_count = 0;
>  
>  	intel_dp_set_edid(intel_dp);
> -	if (intel_dp_is_edp(intel_dp) || connector->detect_edid)
> +	if (intel_dp_is_edp(intel_dp) ||
> +	    to_intel_connector(connector)->detect_edid)
>  		status = connector_status_connected;
> -	intel_dp->detect_done = true;
>  
>  	/* Try to read the source of the interrupt */
>  	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> @@ -5123,26 +5126,6 @@ intel_dp_long_pulse(struct intel_connector *connector,
>  	return status;
>  }
>  
> -static int
> -intel_dp_detect(struct drm_connector *connector,
> -		struct drm_modeset_acquire_ctx *ctx,
> -		bool force)
> -{
> -	struct intel_dp *intel_dp = intel_attached_dp(connector);
> -	int status = connector->status;
> -
> -	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> -		      connector->base.id, connector->name);
> -
> -	/* If full detect is not performed yet, do a full detect */
> -	if (!intel_dp->detect_done)
> -		status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
> -
> -	intel_dp->detect_done = false;
> -
> -	return status;
> -}
> -
>  static void
>  intel_dp_force(struct drm_connector *connector)
>  {
> @@ -5635,7 +5618,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>  
>  	if (long_hpd) {
>  		intel_dp->reset_link_params = true;
> -		intel_dp->detect_done = false;
>  		return IRQ_NONE;
>  	}
>  
> @@ -5652,7 +5634,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>  			intel_dp->is_mst = false;
>  			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
>  							intel_dp->is_mst);
> -			intel_dp->detect_done = false;
>  			goto put_power;
>  		}
>  	}
> @@ -5665,10 +5646,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>  		/* Short pulse can signify loss of hdcp authentication */
>  		intel_hdcp_check_link(intel_dp->attached_connector);
>  
> -		if (!handled) {
> -			intel_dp->detect_done = false;
> +		if (!handled)
>  			goto put_power;
> -		}
>  	}
>  
>  	ret = IRQ_HANDLED;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8073a85d7178..a7c151c40e7d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1070,7 +1070,6 @@ struct intel_dp {
>  	bool link_mst;
>  	bool link_trained;
>  	bool has_audio;
> -	bool detect_done;
>  	bool reset_link_params;
>  	enum aux_ch aux_ch;
>  	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 5/6] drm/i915/dp: Kill intel_dp->detect_done flag
  2018-09-25 17:53   ` Ville Syrjälä
@ 2018-09-26  2:31     ` Dhinakaran Pandiyan
  0 siblings, 0 replies; 30+ messages in thread
From: Dhinakaran Pandiyan @ 2018-09-26  2:31 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Tue, 2018-09-25 at 20:53 +0300, Ville Syrjälä wrote:
> On Mon, Sep 24, 2018 at 03:45:27PM -0700, Dhinakaran Pandiyan wrote:
> > The intel_dp->detect_done flag is no more useful. Pull
> > intel_dp_long_pulse() into the lone caller,
> > 
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c  | 43 ++++++++------------------
> > ------
> >  drivers/gpu/drm/i915/intel_drv.h |  1 -
> >  2 files changed, 11 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 87a631098a6d..d6ea93e453a2 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5012,15 +5012,18 @@ intel_dp_unset_edid(struct intel_dp
> > *intel_dp)
> >  }
> >  
> >  static int
> > -intel_dp_long_pulse(struct intel_connector *connector,
> > -		    struct drm_modeset_acquire_ctx *ctx)
> > +intel_dp_detect(struct drm_connector *connector,
> > +		struct drm_modeset_acquire_ctx *ctx,
> > +		bool force)
> >  {
> > -	struct drm_i915_private *dev_priv = to_i915(connector-
> > >base.dev);
> > -	struct intel_dp *intel_dp = intel_attached_dp(&connector-
> > >base);
> > +	struct drm_i915_private *dev_priv = to_i915(connector-
> > >dev);
> > +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> >  	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
> > >base;
> > -	enum drm_connector_status status;
> > +	int status;
> 
> I guess we don't need this change anymore?

Yeah, I'll remove it.

> 
> Series is
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Not sure if ci actually found some issue with the series, or
> whether it was just having a bad day.

It did find an issue, patch 4/6 wasn't dropping the aux domain
reference in the error path. I've sent a new version with the fix.

-DK 

> 
> >  	u8 sink_irq_vector = 0;
> >  
> > +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> > +		      connector->base.id, connector->name);
> >  	WARN_ON(!drm_modeset_is_locked(&dev_priv-
> > >drm.mode_config.connection_mutex));
> >  
> >  	intel_display_power_get(dev_priv, intel_dp-
> > >aux_power_domain);
> > @@ -5096,9 +5099,9 @@ intel_dp_long_pulse(struct intel_connector
> > *connector,
> >  	intel_dp->aux.i2c_defer_count = 0;
> >  
> >  	intel_dp_set_edid(intel_dp);
> > -	if (intel_dp_is_edp(intel_dp) || connector->detect_edid)
> > +	if (intel_dp_is_edp(intel_dp) ||
> > +	    to_intel_connector(connector)->detect_edid)
> >  		status = connector_status_connected;
> > -	intel_dp->detect_done = true;
> >  
> >  	/* Try to read the source of the interrupt */
> >  	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> > @@ -5123,26 +5126,6 @@ intel_dp_long_pulse(struct intel_connector
> > *connector,
> >  	return status;
> >  }
> >  
> > -static int
> > -intel_dp_detect(struct drm_connector *connector,
> > -		struct drm_modeset_acquire_ctx *ctx,
> > -		bool force)
> > -{
> > -	struct intel_dp *intel_dp = intel_attached_dp(connector);
> > -	int status = connector->status;
> > -
> > -	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> > -		      connector->base.id, connector->name);
> > -
> > -	/* If full detect is not performed yet, do a full detect
> > */
> > -	if (!intel_dp->detect_done)
> > -		status = intel_dp_long_pulse(intel_dp-
> > >attached_connector, ctx);
> > -
> > -	intel_dp->detect_done = false;
> > -
> > -	return status;
> > -}
> > -
> >  static void
> >  intel_dp_force(struct drm_connector *connector)
> >  {
> > @@ -5635,7 +5618,6 @@ intel_dp_hpd_pulse(struct intel_digital_port
> > *intel_dig_port, bool long_hpd)
> >  
> >  	if (long_hpd) {
> >  		intel_dp->reset_link_params = true;
> > -		intel_dp->detect_done = false;
> >  		return IRQ_NONE;
> >  	}
> >  
> > @@ -5652,7 +5634,6 @@ intel_dp_hpd_pulse(struct intel_digital_port
> > *intel_dig_port, bool long_hpd)
> >  			intel_dp->is_mst = false;
> >  			drm_dp_mst_topology_mgr_set_mst(&intel_dp-
> > >mst_mgr,
> >  							intel_dp-
> > >is_mst);
> > -			intel_dp->detect_done = false;
> >  			goto put_power;
> >  		}
> >  	}
> > @@ -5665,10 +5646,8 @@ intel_dp_hpd_pulse(struct intel_digital_port
> > *intel_dig_port, bool long_hpd)
> >  		/* Short pulse can signify loss of hdcp
> > authentication */
> >  		intel_hdcp_check_link(intel_dp-
> > >attached_connector);
> >  
> > -		if (!handled) {
> > -			intel_dp->detect_done = false;
> > +		if (!handled)
> >  			goto put_power;
> > -		}
> >  	}
> >  
> >  	ret = IRQ_HANDLED;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 8073a85d7178..a7c151c40e7d 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1070,7 +1070,6 @@ struct intel_dp {
> >  	bool link_mst;
> >  	bool link_trained;
> >  	bool has_audio;
> > -	bool detect_done;
> >  	bool reset_link_params;
> >  	enum aux_ch aux_ch;
> >  	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
> > -- 
> > 2.17.1
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 6/6] drm/i915/dp: Fix duplication of DEVICE_SERVICE_IRQ handling
  2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
                   ` (3 preceding siblings ...)
  2018-09-24 22:45 ` [PATCH v2 5/6] drm/i915/dp: Kill intel_dp->detect_done flag Dhinakaran Pandiyan
@ 2018-09-24 22:45 ` Dhinakaran Pandiyan
  2018-09-25 21:01   ` Souza, Jose
  2018-09-24 23:47 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Patchwork
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 30+ messages in thread
From: Dhinakaran Pandiyan @ 2018-09-24 22:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan

There are two copies of the same code called from long and short
pulse handlers.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 59 ++++++++++++---------------------
 1 file changed, 22 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d6ea93e453a2..64c6158feb0b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4038,13 +4038,6 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
 					intel_dp->is_mst);
 }
 
-static bool
-intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
-{
-	return drm_dp_dpcd_readb(&intel_dp->aux, DP_DEVICE_SERVICE_IRQ_VECTOR,
-				 sink_irq_vector) == 1;
-}
-
 static bool
 intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector)
 {
@@ -4432,6 +4425,26 @@ static bool intel_dp_hotplug(struct intel_encoder *encoder,
 	return changed;
 }
 
+static void intel_dp_check_service_irq(struct intel_dp *intel_dp)
+{
+	u8 val;
+
+	if (intel_dp->dpcd[DP_DPCD_REV] < 0x11)
+		return;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux,
+			      DP_DEVICE_SERVICE_IRQ_VECTOR, &val) != 1 || !val)
+		return;
+
+	drm_dp_dpcd_writeb(&intel_dp->aux, DP_DEVICE_SERVICE_IRQ_VECTOR, val);
+
+	if (val & DP_AUTOMATED_TEST_REQUEST)
+		intel_dp_handle_test_request(intel_dp);
+
+	if (val & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
+		DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
+}
+
 /*
  * According to DP spec
  * 5.1.2:
@@ -4449,7 +4462,6 @@ static bool
 intel_dp_short_pulse(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	u8 sink_irq_vector = 0;
 	u8 old_sink_count = intel_dp->sink_count;
 	bool ret;
 
@@ -4472,20 +4484,7 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
 		return false;
 	}
 
-	/* Try to read the source of the interrupt */
-	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
-	    intel_dp_get_sink_irq(intel_dp, &sink_irq_vector) &&
-	    sink_irq_vector != 0) {
-		/* Clear interrupt source */
-		drm_dp_dpcd_writeb(&intel_dp->aux,
-				   DP_DEVICE_SERVICE_IRQ_VECTOR,
-				   sink_irq_vector);
-
-		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
-			intel_dp_handle_test_request(intel_dp);
-		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
-			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
-	}
+	intel_dp_check_service_irq(intel_dp);
 
 	/* Handle CEC interrupts, if any */
 	drm_dp_cec_irq(&intel_dp->aux);
@@ -5020,7 +5019,6 @@ intel_dp_detect(struct drm_connector *connector,
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 	int status;
-	u8 sink_irq_vector = 0;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);
@@ -5103,20 +5101,7 @@ intel_dp_detect(struct drm_connector *connector,
 	    to_intel_connector(connector)->detect_edid)
 		status = connector_status_connected;
 
-	/* Try to read the source of the interrupt */
-	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
-	    intel_dp_get_sink_irq(intel_dp, &sink_irq_vector) &&
-	    sink_irq_vector != 0) {
-		/* Clear interrupt source */
-		drm_dp_dpcd_writeb(&intel_dp->aux,
-				   DP_DEVICE_SERVICE_IRQ_VECTOR,
-				   sink_irq_vector);
-
-		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
-			intel_dp_handle_test_request(intel_dp);
-		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
-			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
-	}
+	intel_dp_check_service_irq(intel_dp);
 
 out:
 	if (status != connector_status_connected && !intel_dp->is_mst)
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 6/6] drm/i915/dp: Fix duplication of DEVICE_SERVICE_IRQ handling
  2018-09-24 22:45 ` [PATCH v2 6/6] drm/i915/dp: Fix duplication of DEVICE_SERVICE_IRQ handling Dhinakaran Pandiyan
@ 2018-09-25 21:01   ` Souza, Jose
  0 siblings, 0 replies; 30+ messages in thread
From: Souza, Jose @ 2018-09-25 21:01 UTC (permalink / raw)
  To: intel-gfx, Pandiyan, Dhinakaran

On Mon, 2018-09-24 at 15:45 -0700, Dhinakaran Pandiyan wrote:
> There are two copies of the same code called from long and short
> pulse handlers.


Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 59 ++++++++++++-------------------
> --
>  1 file changed, 22 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index d6ea93e453a2..64c6158feb0b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4038,13 +4038,6 @@ intel_dp_configure_mst(struct intel_dp
> *intel_dp)
>  					intel_dp->is_mst);
>  }
>  
> -static bool
> -intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8
> *sink_irq_vector)
> -{
> -	return drm_dp_dpcd_readb(&intel_dp->aux,
> DP_DEVICE_SERVICE_IRQ_VECTOR,
> -				 sink_irq_vector) == 1;
> -}
> -
>  static bool
>  intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8
> *sink_irq_vector)
>  {
> @@ -4432,6 +4425,26 @@ static bool intel_dp_hotplug(struct
> intel_encoder *encoder,
>  	return changed;
>  }
>  
> +static void intel_dp_check_service_irq(struct intel_dp *intel_dp)
> +{
> +	u8 val;
> +
> +	if (intel_dp->dpcd[DP_DPCD_REV] < 0x11)
> +		return;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux,
> +			      DP_DEVICE_SERVICE_IRQ_VECTOR, &val) != 1
> || !val)
> +		return;
> +
> +	drm_dp_dpcd_writeb(&intel_dp->aux,
> DP_DEVICE_SERVICE_IRQ_VECTOR, val);
> +
> +	if (val & DP_AUTOMATED_TEST_REQUEST)
> +		intel_dp_handle_test_request(intel_dp);
> +
> +	if (val & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
> +		DRM_DEBUG_DRIVER("CP or sink specific irq
> unhandled\n");
> +}
> +
>  /*
>   * According to DP spec
>   * 5.1.2:
> @@ -4449,7 +4462,6 @@ static bool
>  intel_dp_short_pulse(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> -	u8 sink_irq_vector = 0;
>  	u8 old_sink_count = intel_dp->sink_count;
>  	bool ret;
>  
> @@ -4472,20 +4484,7 @@ intel_dp_short_pulse(struct intel_dp
> *intel_dp)
>  		return false;
>  	}
>  
> -	/* Try to read the source of the interrupt */
> -	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> -	    intel_dp_get_sink_irq(intel_dp, &sink_irq_vector) &&
> -	    sink_irq_vector != 0) {
> -		/* Clear interrupt source */
> -		drm_dp_dpcd_writeb(&intel_dp->aux,
> -				   DP_DEVICE_SERVICE_IRQ_VECTOR,
> -				   sink_irq_vector);
> -
> -		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
> -			intel_dp_handle_test_request(intel_dp);
> -		if (sink_irq_vector & (DP_CP_IRQ |
> DP_SINK_SPECIFIC_IRQ))
> -			DRM_DEBUG_DRIVER("CP or sink specific irq
> unhandled\n");
> -	}
> +	intel_dp_check_service_irq(intel_dp);
>  
>  	/* Handle CEC interrupts, if any */
>  	drm_dp_cec_irq(&intel_dp->aux);
> @@ -5020,7 +5019,6 @@ intel_dp_detect(struct drm_connector
> *connector,
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
> >base;
>  	int status;
> -	u8 sink_irq_vector = 0;
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>  		      connector->base.id, connector->name);
> @@ -5103,20 +5101,7 @@ intel_dp_detect(struct drm_connector
> *connector,
>  	    to_intel_connector(connector)->detect_edid)
>  		status = connector_status_connected;
>  
> -	/* Try to read the source of the interrupt */
> -	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> -	    intel_dp_get_sink_irq(intel_dp, &sink_irq_vector) &&
> -	    sink_irq_vector != 0) {
> -		/* Clear interrupt source */
> -		drm_dp_dpcd_writeb(&intel_dp->aux,
> -				   DP_DEVICE_SERVICE_IRQ_VECTOR,
> -				   sink_irq_vector);
> -
> -		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
> -			intel_dp_handle_test_request(intel_dp);
> -		if (sink_irq_vector & (DP_CP_IRQ |
> DP_SINK_SPECIFIC_IRQ))
> -			DRM_DEBUG_DRIVER("CP or sink specific irq
> unhandled\n");
> -	}
> +	intel_dp_check_service_irq(intel_dp);
>  
>  out:
>  	if (status != connector_status_connected && !intel_dp->is_mst)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse()
  2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
                   ` (4 preceding siblings ...)
  2018-09-24 22:45 ` [PATCH v2 6/6] drm/i915/dp: Fix duplication of DEVICE_SERVICE_IRQ handling Dhinakaran Pandiyan
@ 2018-09-24 23:47 ` Patchwork
  2018-09-25  0:08 ` ✗ Fi.CI.BAT: failure " Patchwork
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2018-09-24 23:47 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse()
URL   : https://patchwork.freedesktop.org/series/50113/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
017be37fce85 drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse()
-:27: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#27: 
References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")

-:27: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")'
#27: 
References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")

total: 1 errors, 1 warnings, 0 checks, 19 lines checked
0973c8f3f60e drm/i915/dp: Restrict link retrain workaround to external monitors
-:22: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#22: 
References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")

-:22: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")'
#22: 
References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")

total: 1 errors, 1 warnings, 0 checks, 19 lines checked
3b988cab2936 drm/i915/dp: Use a local variable for intel_encoder *
9cdccd0a7cfb drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()
3a2b516cb993 drm/i915/dp: Kill intel_dp->detect_done flag
59778a072668 drm/i915/dp: Fix duplication of DEVICE_SERVICE_IRQ handling

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse()
  2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
                   ` (5 preceding siblings ...)
  2018-09-24 23:47 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Patchwork
@ 2018-09-25  0:08 ` Patchwork
  2018-09-25  6:33 ` Patchwork
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2018-09-25  0:08 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse()
URL   : https://patchwork.freedesktop.org/series/50113/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4869 -> Patchwork_10266 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_10266 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10266, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50113/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10266:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_module_reload@basic-reload:
      fi-skl-guc:         NOTRUN -> DMESG-WARN +1
      fi-skl-6770hq:      PASS -> DMESG-WARN
      fi-skl-6700k2:      PASS -> DMESG-WARN
      fi-bxt-j4205:       PASS -> DMESG-WARN

    igt@drv_module_reload@basic-reload-inject:
      fi-kbl-7567u:       PASS -> DMESG-WARN +1

    igt@pm_rpm@basic-rte:
      fi-skl-guc:         NOTRUN -> FAIL
      fi-kbl-7567u:       PASS -> FAIL
      fi-skl-6770hq:      PASS -> FAIL
      fi-skl-6700k2:      PASS -> FAIL
      fi-bxt-j4205:       PASS -> FAIL

    
    ==== Warnings ====

    igt@pm_rpm@basic-pci-d3-state:
      fi-skl-6770hq:      PASS -> SKIP +1
      fi-skl-6700k2:      PASS -> SKIP +1

    igt@pm_rpm@module-reload:
      fi-bxt-j4205:       PASS -> SKIP
      fi-kbl-7567u:       PASS -> SKIP +1

    
== Known issues ==

  Here are the changes found in Patchwork_10266 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-cfl-8109u:       PASS -> INCOMPLETE (fdo#107187)
      fi-bdw-samus:       PASS -> INCOMPLETE (fdo#107773)

    igt@gem_exec_suspend@basic-s4-devices:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362, fdo#103191)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       DMESG-WARN (fdo#102614) -> PASS

    igt@pm_rpm@module-reload:
      fi-skl-caroline:    INCOMPLETE (fdo#107807) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#107187 https://bugs.freedesktop.org/show_bug.cgi?id=107187
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807


== Participating hosts (46 -> 41) ==

  Additional (1): fi-skl-guc 
  Missing    (6): fi-hsw-4770r fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_4869 -> Patchwork_10266

  CI_DRM_4869: 9a74a6db272a007c3db063ae3375fbee60a7bd53 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4649: 19b0c74d20d9b53d4c82be14af0909a3b6846010 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10266: 59778a0726687dd01f1a9876199985052781f7b4 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

59778a072668 drm/i915/dp: Fix duplication of DEVICE_SERVICE_IRQ handling
3a2b516cb993 drm/i915/dp: Kill intel_dp->detect_done flag
9cdccd0a7cfb drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()
3b988cab2936 drm/i915/dp: Use a local variable for intel_encoder *
0973c8f3f60e drm/i915/dp: Restrict link retrain workaround to external monitors
017be37fce85 drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10266/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse()
  2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
                   ` (6 preceding siblings ...)
  2018-09-25  0:08 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2018-09-25  6:33 ` Patchwork
  2018-09-26  2:08 ` [PATCH v3 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect() Dhinakaran Pandiyan
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2018-09-25  6:33 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse()
URL   : https://patchwork.freedesktop.org/series/50113/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4869 -> Patchwork_10270 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_10270 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10270, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50113/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10270:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_module_reload@basic-reload:
      fi-skl-guc:         NOTRUN -> DMESG-WARN
      fi-hsw-4770r:       PASS -> DMESG-WARN +1
      fi-cfl-8109u:       PASS -> DMESG-WARN
      fi-skl-6770hq:      PASS -> DMESG-WARN
      fi-skl-6700k2:      PASS -> DMESG-WARN +1
      fi-bxt-j4205:       PASS -> DMESG-WARN

    igt@drv_module_reload@basic-reload-inject:
      fi-kbl-7567u:       PASS -> DMESG-WARN +1

    igt@pm_rpm@basic-rte:
      fi-skl-guc:         NOTRUN -> FAIL
      fi-kbl-7567u:       PASS -> FAIL
      fi-hsw-4770r:       PASS -> FAIL
      fi-cfl-8109u:       PASS -> FAIL
      fi-skl-6770hq:      PASS -> FAIL
      fi-skl-6700k2:      PASS -> FAIL
      fi-bxt-j4205:       PASS -> FAIL

    
    ==== Warnings ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-j4205:       PASS -> SKIP +4

    igt@pm_rpm@basic-pci-d3-state:
      fi-skl-6770hq:      PASS -> SKIP +1
      fi-skl-6700k2:      PASS -> SKIP +1
      fi-hsw-4770r:       PASS -> SKIP +1

    igt@pm_rpm@module-reload:
      fi-cfl-8109u:       PASS -> SKIP +1
      fi-kbl-7567u:       PASS -> SKIP +1

    
== Known issues ==

  Here are the changes found in Patchwork_10270 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_module_reload@basic-reload-inject:
      fi-hsw-4770r:       PASS -> DMESG-WARN (fdo#107924)

    igt@gem_exec_suspend@basic-s3:
      fi-bdw-samus:       PASS -> INCOMPLETE (fdo#107773)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-bxt-j4205:       PASS -> DMESG-FAIL (fdo#105602)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7560u:       INCOMPLETE (fdo#108044) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       DMESG-WARN (fdo#102614) -> PASS

    igt@pm_rpm@module-reload:
      fi-skl-caroline:    INCOMPLETE (fdo#107807) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107924 https://bugs.freedesktop.org/show_bug.cgi?id=107924
  fdo#108044 https://bugs.freedesktop.org/show_bug.cgi?id=108044


== Participating hosts (46 -> 40) ==

  Additional (1): fi-skl-guc 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-n3050 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_4869 -> Patchwork_10270

  CI_DRM_4869: 9a74a6db272a007c3db063ae3375fbee60a7bd53 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4649: 19b0c74d20d9b53d4c82be14af0909a3b6846010 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10270: d7e77e6ab2178dbcbe5d0d85a639e1fbc44292b7 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d7e77e6ab217 drm/i915/dp: Fix duplication of DEVICE_SERVICE_IRQ handling
ae605a974179 drm/i915/dp: Kill intel_dp->detect_done flag
760475370a38 drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()
5c0f60e4a08d drm/i915/dp: Use a local variable for intel_encoder *
b3430681eebd drm/i915/dp: Restrict link retrain workaround to external monitors
af60bb3d378c drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10270/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v3 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()
  2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
                   ` (7 preceding siblings ...)
  2018-09-25  6:33 ` Patchwork
@ 2018-09-26  2:08 ` Dhinakaran Pandiyan
  2018-09-26  3:18 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() (rev2) Patchwork
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 30+ messages in thread
From: Dhinakaran Pandiyan @ 2018-09-26  2:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan

A crtc modeset lock was added for link retraining but
intel_dp_retrain_link() knows to take the necessary locks since
commit c85d200e8321 ("drm/i915: Move SST DP link retraining into the
->post_hotplug() hook")
v2: Drop AUX power domain reference in the early return path

Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into the ->post_hotplug() hook")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 09229fc66dec..ef28cc0f122c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5079,8 +5079,15 @@ intel_dp_long_pulse(struct intel_connector *connector,
 	 * 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))
-		intel_dp_retrain_link(encoder, ctx);
+	if (!intel_dp_is_edp(intel_dp)) {
+		int ret;
+
+		ret = intel_dp_retrain_link(encoder, ctx);
+		if (ret) {
+			intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
+			return ret;
+		}
+	}
 
 	/*
 	 * Clearing NACK and defer counts to get their exact values
@@ -5130,19 +5137,8 @@ intel_dp_detect(struct drm_connector *connector,
 		      connector->base.id, connector->name);
 
 	/* If full detect is not performed yet, do a full detect */
-	if (!intel_dp->detect_done) {
-		struct drm_crtc *crtc;
-		int ret;
-
-		crtc = connector->state->crtc;
-		if (crtc) {
-			ret = drm_modeset_lock(&crtc->mutex, ctx);
-			if (ret)
-				return ret;
-		}
-
+	if (!intel_dp->detect_done)
 		status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
-	}
 
 	intel_dp->detect_done = false;
 
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() (rev2)
  2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
                   ` (8 preceding siblings ...)
  2018-09-26  2:08 ` [PATCH v3 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect() Dhinakaran Pandiyan
@ 2018-09-26  3:18 ` Patchwork
  2018-09-26  3:41 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2018-09-26  3:18 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() (rev2)
URL   : https://patchwork.freedesktop.org/series/50113/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
96702d287245 drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse()
-:27: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#27: 
References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")

-:27: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")'
#27: 
References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")

total: 1 errors, 1 warnings, 0 checks, 19 lines checked
d42e0b2d6fdc drm/i915/dp: Restrict link retrain workaround to external monitors
-:22: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#22: 
References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")

-:22: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")'
#22: 
References: 3cf71bc9904d ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")

total: 1 errors, 1 warnings, 0 checks, 19 lines checked
dd231edb1212 drm/i915/dp: Use a local variable for intel_encoder *
fd0fe50bcfd4 drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()
dd9370fb6c55 drm/i915/dp: Kill intel_dp->detect_done flag
9285188024d6 drm/i915/dp: Fix duplication of DEVICE_SERVICE_IRQ handling

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() (rev2)
  2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
                   ` (9 preceding siblings ...)
  2018-09-26  3:18 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() (rev2) Patchwork
@ 2018-09-26  3:41 ` Patchwork
  2018-09-26  4:32 ` ✓ Fi.CI.IGT: " Patchwork
  2018-10-01 22:30 ` [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Lyude Paul
  12 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2018-09-26  3:41 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() (rev2)
URL   : https://patchwork.freedesktop.org/series/50113/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4875 -> Patchwork_10280 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50113/revisions/2/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10280 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-bdw-samus:       PASS -> INCOMPLETE (fdo#107773)

    igt@kms_psr@primary_page_flip:
      fi-kbl-r:           PASS -> FAIL (fdo#107336)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s3:
      fi-kbl-guc:         INCOMPLETE (fdo#106693) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     FAIL (fdo#103167) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#106693 https://bugs.freedesktop.org/show_bug.cgi?id=106693
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773


== Participating hosts (47 -> 40) ==

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u2 fi-bwr-2160 fi-skl-caroline 


== Build changes ==

    * Linux: CI_DRM_4875 -> Patchwork_10280

  CI_DRM_4875: 5f099906779ca138ecc32e0c6b4b2860b71716f4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4650: a6e21812d100dce68450727e79fc09e0c0033683 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10280: 9285188024d6f25ee0b03210265a8e6235fdee88 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

9285188024d6 drm/i915/dp: Fix duplication of DEVICE_SERVICE_IRQ handling
dd9370fb6c55 drm/i915/dp: Kill intel_dp->detect_done flag
fd0fe50bcfd4 drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect()
dd231edb1212 drm/i915/dp: Use a local variable for intel_encoder *
d42e0b2d6fdc drm/i915/dp: Restrict link retrain workaround to external monitors
96702d287245 drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10280/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() (rev2)
  2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
                   ` (10 preceding siblings ...)
  2018-09-26  3:41 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-09-26  4:32 ` Patchwork
  2018-10-01 22:30 ` [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Lyude Paul
  12 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2018-09-26  4:32 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() (rev2)
URL   : https://patchwork.freedesktop.org/series/50113/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4875_full -> Patchwork_10280_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10280_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10280_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10280_full:

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_10280_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_await@wide-contexts:
      shard-glk:          PASS -> FAIL (fdo#106680)
      shard-apl:          PASS -> FAIL (fdo#106680)

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      shard-hsw:          PASS -> INCOMPLETE (fdo#103540) +1

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
      shard-snb:          NOTRUN -> DMESG-WARN (fdo#107956) +1

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#105363) +1

    igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
      shard-hsw:          PASS -> DMESG-WARN (fdo#102614)

    igt@kms_setmode@basic:
      shard-snb:          NOTRUN -> FAIL (fdo#99912)

    igt@perf@polling:
      shard-hsw:          PASS -> FAIL (fdo#102252)

    
    ==== Possible fixes ====

    igt@kms_busy@extended-pageflip-hang-newfb-render-a:
      shard-glk:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          FAIL (fdo#105363) -> PASS

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 5) ==

  Missing    (1): shard-skl 


== Build changes ==

    * Linux: CI_DRM_4875 -> Patchwork_10280

  CI_DRM_4875: 5f099906779ca138ecc32e0c6b4b2860b71716f4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4650: a6e21812d100dce68450727e79fc09e0c0033683 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10280: 9285188024d6f25ee0b03210265a8e6235fdee88 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10280/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse()
  2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
                   ` (11 preceding siblings ...)
  2018-09-26  4:32 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-10-01 22:30 ` Lyude Paul
  12 siblings, 0 replies; 30+ messages in thread
From: Lyude Paul @ 2018-10-01 22:30 UTC (permalink / raw)
  To: Dhinakaran Pandiyan, intel-gfx; +Cc: Jan-Marek Glogowski, Rodrigo Vivi

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Mon, 2018-09-24 at 15:45 -0700, Dhinakaran Pandiyan wrote:
> Comment claims link needs to be retrained because the connected sink raised
> a long pulse to indicate link loss. If the sink did so,
> intel_dp_hotplug() would have handled link retraining. Looking at the
> logs in Bugzilla referenced in commit '3cf71bc9904d ("drm/i915: Re-apply
> Perform link quality check, unconditionally during long pulse"")', the
> issue is that the sink does not trigger an interrupt. What we want is
> ->detect() from user space to check link status and retrain. Ville's
> review for the original patch also indicates the same root cause. So,
> rewrite the comment.
> 
> v2: Patch split and rewrote comment.
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Jan-Marek Glogowski <glogow@fbihome.de>
> 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, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 6b4c19123f2a..34c561011e7a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5074,16 +5074,9 @@ intel_dp_long_pulse(struct intel_connector
> *connector,
>  		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.
> +		 * Some external monitors do not signal loss of link
> +		 * synchronization with an IRQ_HPD, so force a link status
> +		 * check.
>  		 */
>  		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
> >base;
>  
-- 
Cheers,
	Lyude Paul

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2018-10-24 10:49 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-24 22:45 [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Dhinakaran Pandiyan
2018-09-24 22:45 ` [PATCH v2 2/6] drm/i915/dp: Restrict link retrain workaround to external monitors Dhinakaran Pandiyan
2018-10-01 19:49   ` Rodrigo Vivi
2018-10-01 22:00     ` Dhinakaran Pandiyan
2018-10-02  0:30     ` Dhinakaran Pandiyan
2018-10-23 11:56       ` Jani Nikula
2018-10-24 10:49         ` Joonas Lahtinen
2018-10-01 22:30   ` Lyude Paul
2018-09-24 22:45 ` [PATCH v2 3/6] drm/i915/dp: Use a local variable for intel_encoder * Dhinakaran Pandiyan
2018-09-25 20:51   ` Souza, Jose
2018-09-24 22:45 ` [PATCH v2 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect() Dhinakaran Pandiyan
2018-09-25 20:58   ` Souza, Jose
2018-09-26  2:54   ` [PATCH v3 " Dhinakaran Pandiyan
2018-10-01 19:48     ` Rodrigo Vivi
2018-10-01 20:10       ` Ville Syrjälä
2018-10-01 22:01         ` Dhinakaran Pandiyan
2018-10-01 23:22           ` Rodrigo Vivi
2018-09-24 22:45 ` [PATCH v2 5/6] drm/i915/dp: Kill intel_dp->detect_done flag Dhinakaran Pandiyan
2018-09-25 17:53   ` Ville Syrjälä
2018-09-26  2:31     ` Dhinakaran Pandiyan
2018-09-24 22:45 ` [PATCH v2 6/6] drm/i915/dp: Fix duplication of DEVICE_SERVICE_IRQ handling Dhinakaran Pandiyan
2018-09-25 21:01   ` Souza, Jose
2018-09-24 23:47 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Patchwork
2018-09-25  0:08 ` ✗ Fi.CI.BAT: failure " Patchwork
2018-09-25  6:33 ` Patchwork
2018-09-26  2:08 ` [PATCH v3 4/6] drm/i915/dp: Do not grab crtc modeset lock in intel_dp_detect() Dhinakaran Pandiyan
2018-09-26  3:18 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() (rev2) Patchwork
2018-09-26  3:41 ` ✓ Fi.CI.BAT: success " Patchwork
2018-09-26  4:32 ` ✓ Fi.CI.IGT: " Patchwork
2018-10-01 22:30 ` [PATCH v2 1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() Lyude Paul

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.