[Intel-gfx] [PATCH] drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.
labbott at redhat.com
Wed Mar 14 19:56:52 UTC 2018
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 at linux.intel.com>
>>> Cc: Jani Nikula <jani.nikula at intel.com>
> Reported-by: Laura Abbott <labbott at redhat.com>
> Cc: stable at vger.kernel.org
> Fixes: 5ea2355a100a ("drm/i915/mst: Use MST sideband message
> transactions for dpms control")
Tested-by: Laura Abbott <labbott at redhat.com>
>>> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at 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);
>>> - 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 at 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.
>>> if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
>>> @@ -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);
More information about the Intel-gfx