All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.
@ 2018-03-14  5:48 Dhinakaran Pandiyan
  2018-03-14  7:23 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Dhinakaran Pandiyan @ 2018-03-14  5:48 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Dhinakaran Pandiyan

If bios sets up an MST output and hardware state readout code sees this is
an SST configuration, when disabling the encoder we end up calling
->post_disable_dp() hook instead of the MST version. Consequently, we write
to the DP_SET_POWER dpcd to set it D3 state. Further along when we try
enable the encoder in MST mode, POWER_UP_PHY transaction fails to power up
the MST hub. This results in continuous link training failures which keep
the system busy delaying boot. We could identify bios MST boot discrepancy
and handle it accordingly but a simple way to solve this is to write to the
DP_SET_POWER dpcd for MST too.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105470
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index dbcf1a0586f9..8c2d778560f0 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2205,8 +2205,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 		intel_prepare_dp_ddi_buffers(encoder, crtc_state);
 
 	intel_ddi_init_dp_buf_reg(encoder);
-	if (!is_mst)
-		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
+	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
 	intel_dp_start_link_train(intel_dp);
 	if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
 		intel_dp_stop_link_train(intel_dp);
@@ -2304,14 +2303,12 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 	struct intel_dp *intel_dp = &dig_port->dp;
-	bool is_mst = intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST);
 
 	/*
 	 * Power down sink before disabling the port, otherwise we end
 	 * up getting interrupts from the sink on detecting link loss.
 	 */
-	if (!is_mst)
-		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
+	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 
 	intel_disable_ddi_buf(encoder);
 
-- 
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] 6+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.
  2018-03-14  5:48 [PATCH] drm/i915/dp: Write to SET_POWER dpcd to enable MST hub Dhinakaran Pandiyan
@ 2018-03-14  7:23 ` Patchwork
  2018-03-14  8:45 ` ✗ Fi.CI.IGT: failure " Patchwork
  2018-03-14 13:35 ` [PATCH] " Ville Syrjälä
  2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-03-14  7:23 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.
URL   : https://patchwork.freedesktop.org/series/39927/
State : success

== Summary ==

Series 39927v1 drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.
https://patchwork.freedesktop.org/api/1.0/series/39927/revisions/1/mbox/

---- Known issues:

Test kms_chamelium:
        Subgroup hdmi-hpd-fast:
                skip       -> FAIL       (fi-kbl-7500u) fdo#102672

fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:430s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:432s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:388s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:531s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:297s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:509s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:506s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:508s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:494s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:410s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:584s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:584s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:429s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:314s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:404s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:420s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:479s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:432s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:1   skip:23  time:472s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:476s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:515s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:435s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:524s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:539s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:502s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:501s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:425s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:434s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:525s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:400s
Blacklisted hosts:
fi-cfl-u         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:510s
fi-cnl-drrs      total:288  pass:257  dwarn:3   dfail:0   fail:0   skip:28  time:533s

613eb885b69e808a46f11125870e47b67a326d76 drm-tip: 2018y-03m-14d-05h-56m-23s UTC integration manifest
dd76746289e8 drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.

== Logs ==

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

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

* ✗ Fi.CI.IGT: failure for drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.
  2018-03-14  5:48 [PATCH] drm/i915/dp: Write to SET_POWER dpcd to enable MST hub Dhinakaran Pandiyan
  2018-03-14  7:23 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-03-14  8:45 ` Patchwork
  2018-03-14 13:35 ` [PATCH] " Ville Syrjälä
  2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-03-14  8:45 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.
URL   : https://patchwork.freedesktop.org/series/39927/
State : failure

== Summary ==

---- Possible new issues:

Test gem_pwrite:
        Subgroup big-cpu-backwards:
                skip       -> PASS       (shard-apl)
Test perf_pmu:
        Subgroup busy-double-start-vcs0:
                pass       -> FAIL       (shard-snb)

---- Known issues:

Test gem_softpin:
        Subgroup noreloc-s3:
                skip       -> PASS       (shard-snb) fdo#103375
Test kms_cursor_crc:
        Subgroup cursor-64x64-suspend:
                pass       -> SKIP       (shard-hsw) fdo#103540
Test kms_cursor_legacy:
        Subgroup flip-vs-cursor-atomic:
                pass       -> FAIL       (shard-hsw) fdo#102670
Test kms_flip:
        Subgroup plain-flip-ts-check-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912
Test prime_vgem:
        Subgroup basic-fence-flip:
                fail       -> PASS       (shard-apl) fdo#104008

fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008

shard-apl        total:3444 pass:1817 dwarn:1   dfail:0   fail:7   skip:1619 time:12992s
shard-hsw        total:3444 pass:1767 dwarn:1   dfail:0   fail:3   skip:1672 time:11666s
shard-snb        total:3444 pass:1359 dwarn:1   dfail:0   fail:3   skip:2081 time:7188s
Blacklisted hosts:
shard-kbl        total:3234 pass:1828 dwarn:1   dfail:0   fail:9   skip:1393 time:9119s

== Logs ==

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

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

* Re: [PATCH] drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.
  2018-03-14  5:48 [PATCH] drm/i915/dp: Write to SET_POWER dpcd to enable MST hub Dhinakaran Pandiyan
  2018-03-14  7:23 ` ✓ Fi.CI.BAT: success for " Patchwork
  2018-03-14  8:45 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-03-14 13:35 ` Ville Syrjälä
  2018-03-14 18:26   ` Pandiyan, Dhinakaran
  2 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2018-03-14 13:35 UTC (permalink / raw)
  To: Dhinakaran Pandiyan; +Cc: Jani Nikula, intel-gfx

On Tue, Mar 13, 2018 at 10:48:25PM -0700, Dhinakaran Pandiyan wrote:
> If bios sets up an MST output and hardware state readout code sees this is
> an SST configuration, when disabling the encoder we end up calling
> ->post_disable_dp() hook instead of the MST version. Consequently, we write
> to the DP_SET_POWER dpcd to set it D3 state. Further along when we try
> enable the encoder in MST mode, POWER_UP_PHY transaction fails to power up
> the MST hub. This results in continuous link training failures which keep
> the system busy delaying boot. We could identify bios MST boot discrepancy
> and handle it accordingly but a simple way to solve this is to write to the
> DP_SET_POWER dpcd for MST too.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105470
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index dbcf1a0586f9..8c2d778560f0 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2205,8 +2205,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  		intel_prepare_dp_ddi_buffers(encoder, crtc_state);
>  
>  	intel_ddi_init_dp_buf_reg(encoder);
> -	if (!is_mst)
> -		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> +	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);

The spec is perhaps a bit unclear on this. 

"If the message is sent as a path request, all DP nodes from the source
 immediate downstream device and the targeted DP node will be placed in
 the D0 power state."

Doesn't quite tell me whether the immediate downstream device is
included in that list of nodes.

However the spec goes on to say
"Each nodes immediate upstream device will use Native AUX writes to the
 SET_POWER & SET_DP_PWR_VOLTAGE register (DPCD Address 00600h) to set
 the power state of the downstream node."

and since we are the immediate upstream for that device I guess it makes
sense that we should still do the DP_SET_POWER manually.

But I have to wonder what the original issue was before we started to use
POWER_UP/DOWN_PHY. I suppose somehow some further downstream device
wasn't in D0 when we needed it. But that is a bit weird as I believe all
devices should really start up in D0.

Anyways based on my reading of the spec I can justify this so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I presume we want cc:stable + fixes: on this?

>  	intel_dp_start_link_train(intel_dp);
>  	if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
>  		intel_dp_stop_link_train(intel_dp);
> @@ -2304,14 +2303,12 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>  	struct intel_dp *intel_dp = &dig_port->dp;
> -	bool is_mst = intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST);
>  
>  	/*
>  	 * Power down sink before disabling the port, otherwise we end
>  	 * up getting interrupts from the sink on detecting link loss.
>  	 */
> -	if (!is_mst)
> -		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> +	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>  
>  	intel_disable_ddi_buf(encoder);
>  
> -- 
> 2.14.1

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

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

* Re: [PATCH] drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.
  2018-03-14 13:35 ` [PATCH] " Ville Syrjälä
@ 2018-03-14 18:26   ` Pandiyan, Dhinakaran
  2018-03-14 19:56     ` Laura Abbott
  0 siblings, 1 reply; 6+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-14 18:26 UTC (permalink / raw)
  To: ville.syrjala; +Cc: Nikula, Jani, intel-gfx, labbott




On Wed, 2018-03-14 at 15:35 +0200, Ville Syrjälä wrote:
> On Tue, Mar 13, 2018 at 10:48:25PM -0700, Dhinakaran Pandiyan wrote:
> > If bios sets up an MST output and hardware state readout code sees this is
> > an SST configuration, when disabling the encoder we end up calling
> > ->post_disable_dp() hook instead of the MST version. Consequently, we write
> > to the DP_SET_POWER dpcd to set it D3 state. Further along when we try
> > enable the encoder in MST mode, POWER_UP_PHY transaction fails to power up
> > the MST hub. This results in continuous link training failures which keep
> > the system busy delaying boot. We could identify bios MST boot discrepancy
> > and handle it accordingly but a simple way to solve this is to write to the
> > DP_SET_POWER dpcd for MST too.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105470
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>

Reported-by: Laura Abbott <labbott@redhat.com>
Cc: stable@vger.kernel.org
Fixes: 5ea2355a100a ("drm/i915/mst: Use MST sideband message
transactions for dpms control")

> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index dbcf1a0586f9..8c2d778560f0 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2205,8 +2205,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> >  		intel_prepare_dp_ddi_buffers(encoder, crtc_state);
> >  
> >  	intel_ddi_init_dp_buf_reg(encoder);
> > -	if (!is_mst)
> > -		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> > +	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> 
> The spec is perhaps a bit unclear on this. 
> 
> "If the message is sent as a path request, all DP nodes from the source
>  immediate downstream device and the targeted DP node will be placed in
>  the D0 power state."
> 
> Doesn't quite tell me whether the immediate downstream device is
> included in that list of nodes.
> 
> However the spec goes on to say
> "Each nodes immediate upstream device will use Native AUX writes to the
>  SET_POWER & SET_DP_PWR_VOLTAGE register (DPCD Address 00600h) to set
>  the power state of the downstream node."
> 
> and since we are the immediate upstream for that device I guess it makes
> sense that we should still do the DP_SET_POWER manually.
> 
> But I have to wonder what the original issue was before we started to use
> POWER_UP/DOWN_PHY. I suppose somehow some further downstream device
> wasn't in D0 when we needed it.

Correct, sinks farther downstream weren't lighting up.

>  But that is a bit weird as I believe all
> devices should really start up in D0.
> 
> Anyways based on my reading of the spec I can justify this so
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 

Thanks for the review.

> I presume we want cc:stable + fixes: on this?

Yeah, I suppose we should wait for the reporter to confirm that this
indeed fixes the bug. It does fix the problem on the Thinkpad laptop +
dock I tested it on.


> 
> >  	intel_dp_start_link_train(intel_dp);
> >  	if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
> >  		intel_dp_stop_link_train(intel_dp);
> > @@ -2304,14 +2303,12 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> >  	struct intel_dp *intel_dp = &dig_port->dp;
> > -	bool is_mst = intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST);
> >  
> >  	/*
> >  	 * Power down sink before disabling the port, otherwise we end
> >  	 * up getting interrupts from the sink on detecting link loss.
> >  	 */
> > -	if (!is_mst)
> > -		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> > +	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> >  
> >  	intel_disable_ddi_buf(encoder);
> >  
> > -- 
> > 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.
  2018-03-14 18:26   ` Pandiyan, Dhinakaran
@ 2018-03-14 19:56     ` Laura Abbott
  0 siblings, 0 replies; 6+ messages in thread
From: Laura Abbott @ 2018-03-14 19:56 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran, ville.syrjala; +Cc: Nikula, Jani, intel-gfx

On 03/14/2018 11:26 AM, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Wed, 2018-03-14 at 15:35 +0200, Ville Syrjälä wrote:
>> On Tue, Mar 13, 2018 at 10:48:25PM -0700, Dhinakaran Pandiyan wrote:
>>> If bios sets up an MST output and hardware state readout code sees this is
>>> an SST configuration, when disabling the encoder we end up calling
>>> ->post_disable_dp() hook instead of the MST version. Consequently, we write
>>> to the DP_SET_POWER dpcd to set it D3 state. Further along when we try
>>> enable the encoder in MST mode, POWER_UP_PHY transaction fails to power up
>>> the MST hub. This results in continuous link training failures which keep
>>> the system busy delaying boot. We could identify bios MST boot discrepancy
>>> and handle it accordingly but a simple way to solve this is to write to the
>>> DP_SET_POWER dpcd for MST too.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105470
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
> 
> Reported-by: Laura Abbott <labbott@redhat.com>
> Cc: stable@vger.kernel.org
> Fixes: 5ea2355a100a ("drm/i915/mst: Use MST sideband message
> transactions for dpms control")
> 

Tested-by: Laura Abbott <labbott@redhat.com>

>>> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_ddi.c | 7 ++-----
>>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>>> index dbcf1a0586f9..8c2d778560f0 100644
>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>> @@ -2205,8 +2205,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>>>   		intel_prepare_dp_ddi_buffers(encoder, crtc_state);
>>>   
>>>   	intel_ddi_init_dp_buf_reg(encoder);
>>> -	if (!is_mst)
>>> -		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
>>> +	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
>>
>> The spec is perhaps a bit unclear on this.
>>
>> "If the message is sent as a path request, all DP nodes from the source
>>   immediate downstream device and the targeted DP node will be placed in
>>   the D0 power state."
>>
>> Doesn't quite tell me whether the immediate downstream device is
>> included in that list of nodes.
>>
>> However the spec goes on to say
>> "Each nodes immediate upstream device will use Native AUX writes to the
>>   SET_POWER & SET_DP_PWR_VOLTAGE register (DPCD Address 00600h) to set
>>   the power state of the downstream node."
>>
>> and since we are the immediate upstream for that device I guess it makes
>> sense that we should still do the DP_SET_POWER manually.
>>
>> But I have to wonder what the original issue was before we started to use
>> POWER_UP/DOWN_PHY. I suppose somehow some further downstream device
>> wasn't in D0 when we needed it.
> 
> Correct, sinks farther downstream weren't lighting up.
> 
>>   But that is a bit weird as I believe all
>> devices should really start up in D0.
>>
>> Anyways based on my reading of the spec I can justify this so
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
> 
> Thanks for the review.
> 
>> I presume we want cc:stable + fixes: on this?
> 
> Yeah, I suppose we should wait for the reporter to confirm that this
> indeed fixes the bug. It does fix the problem on the Thinkpad laptop +
> dock I tested it on.
> 
> 
>>
>>>   	intel_dp_start_link_train(intel_dp);
>>>   	if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
>>>   		intel_dp_stop_link_train(intel_dp);
>>> @@ -2304,14 +2303,12 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
>>>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>>   	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>>>   	struct intel_dp *intel_dp = &dig_port->dp;
>>> -	bool is_mst = intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST);
>>>   
>>>   	/*
>>>   	 * Power down sink before disabling the port, otherwise we end
>>>   	 * up getting interrupts from the sink on detecting link loss.
>>>   	 */
>>> -	if (!is_mst)
>>> -		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>>> +	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>>>   
>>>   	intel_disable_ddi_buf(encoder);
>>>   
>>> -- 
>>> 2.14.1
>>

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

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

end of thread, other threads:[~2018-03-14 19:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-14  5:48 [PATCH] drm/i915/dp: Write to SET_POWER dpcd to enable MST hub Dhinakaran Pandiyan
2018-03-14  7:23 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-03-14  8:45 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-03-14 13:35 ` [PATCH] " Ville Syrjälä
2018-03-14 18:26   ` Pandiyan, Dhinakaran
2018-03-14 19:56     ` Laura Abbott

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.