| Message ID | 20181107200836.10191-2-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/ddi: Add more sanity check to the encoder HW readout | expand |
On 11/07/2018 12:08 PM, Imre Deak wrote: > We shouldn't consider an encoder inactive if it doesn't have a CRTC > linked, but has virtual MST encoders with a crtc linked. Fix this. > > Also we should not sanitize the mapping for MST encoders, as it's always > their primary encoder (which could be even in SST mode) whose active > state determines if we need the clock being enabled for the > corresponding physical port. Fix this too. > > This fixes at least an existing breakage where we incorrectly disabled > the clock for an active DP encoder when sanitizing its MST virtual > encoders. Not sure if there are BIOSes that enable an output in MST > mode, but our HW readout is mostly missing for it anyway, so just warn > for that case. > > Fixes: 70332ac539c5 ("drm/i915/icl+: Sanitize port to PLL mapping") > Cc: Antonio Argenziano <antonio.argenziano@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: José Roberto de Souza <jose.souza@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Reported-by: Antonio Argenziano <antonio.argenziano@intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 27 +++++++++++++++++++++++++-- > 1 file changed, 25 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index abc51693eec9..4913bbdac843 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -2853,9 +2853,32 @@ void icl_unmap_plls_to_ports(struct drm_crtc *crtc, > void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder) > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > - u32 val = I915_READ(DPCLKA_CFGCR0_ICL); > + u32 val; > enum port port = encoder->port; > - bool clk_enabled = !(val & icl_dpclka_cfgcr0_clk_off(dev_priv, port)); > + bool clk_enabled; > + > + /* > + * In case of DP MST, we sanitize the primary encoder only, not the > + * virtual ones. > + */ > + if (encoder->type == INTEL_OUTPUT_DP_MST) > + return; > + > + val = I915_READ(DPCLKA_CFGCR0_ICL); > + clk_enabled = !(val & icl_dpclka_cfgcr0_clk_off(dev_priv, port)); > + > + if (!encoder->base.crtc && intel_encoder_is_dp(encoder)) { > + u8 pipe_mask; > + bool is_mst; > + > + intel_ddi_get_encoder_pipes(encoder, &pipe_mask, &is_mst); > + /* > + * In the unlikely case that BIOS enables DP in MST mode, just > + * warn since our MST HW readout is incomplete. > + */ > + if (WARN_ON(is_mst)) > + return; > + } > > if (clk_enabled == !!encoder->base.crtc) > return; Fixes the mDP lock up issue. Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com> Tested-by: Clint Taylor <Clinton.A.Taylor@intel.com> -Clint
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index abc51693eec9..4913bbdac843 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -2853,9 +2853,32 @@ void icl_unmap_plls_to_ports(struct drm_crtc *crtc, void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); - u32 val = I915_READ(DPCLKA_CFGCR0_ICL); + u32 val; enum port port = encoder->port; - bool clk_enabled = !(val & icl_dpclka_cfgcr0_clk_off(dev_priv, port)); + bool clk_enabled; + + /* + * In case of DP MST, we sanitize the primary encoder only, not the + * virtual ones. + */ + if (encoder->type == INTEL_OUTPUT_DP_MST) + return; + + val = I915_READ(DPCLKA_CFGCR0_ICL); + clk_enabled = !(val & icl_dpclka_cfgcr0_clk_off(dev_priv, port)); + + if (!encoder->base.crtc && intel_encoder_is_dp(encoder)) { + u8 pipe_mask; + bool is_mst; + + intel_ddi_get_encoder_pipes(encoder, &pipe_mask, &is_mst); + /* + * In the unlikely case that BIOS enables DP in MST mode, just + * warn since our MST HW readout is incomplete. + */ + if (WARN_ON(is_mst)) + return; + } if (clk_enabled == !!encoder->base.crtc) return;
We shouldn't consider an encoder inactive if it doesn't have a CRTC linked, but has virtual MST encoders with a crtc linked. Fix this. Also we should not sanitize the mapping for MST encoders, as it's always their primary encoder (which could be even in SST mode) whose active state determines if we need the clock being enabled for the corresponding physical port. Fix this too. This fixes at least an existing breakage where we incorrectly disabled the clock for an active DP encoder when sanitizing its MST virtual encoders. Not sure if there are BIOSes that enable an output in MST mode, but our HW readout is mostly missing for it anyway, so just warn for that case. Fixes: 70332ac539c5 ("drm/i915/icl+: Sanitize port to PLL mapping") Cc: Antonio Argenziano <antonio.argenziano@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: José Roberto de Souza <jose.souza@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Reported-by: Antonio Argenziano <antonio.argenziano@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/intel_ddi.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)