On Tue, 27 Nov 2018, Imre Deak <[email protected]> wrote: > The requirement for the DDI port clock gating for a port in DSI mode is > the opposite wrt. the case when the port is in DDI mode: the clock > should be gated when the port is active and ungated when the port is > inactive. Note that we cannot simply keep the DDI clock gated when the > port will be only used in DSI mode: it must be gated/ungated at a > specific spot in the DSI enable/disable sequence. > > Ensure the above for all ports of a DSI encoder, also adding a sanity > check that we haven't registered another encoder using the same port > (VBT should never allow this to happen). > > Cc: Madhav Chauhan <[email protected]> > Cc: Vandita Kulkarni <[email protected]> > Cc: Jani Nikula <[email protected]> > Cc: Ville Syrjälä <[email protected]> > Cc: Clint Taylor <[email protected]> > Signed-off-by: Imre Deak <[email protected]>
Reviewed-by: Jani Nikula <[email protected]> > --- > drivers/gpu/drm/i915/intel_ddi.c | 65 > +++++++++++++++++++++++++++++----------- > drivers/gpu/drm/i915/intel_dsi.h | 5 ++++ > 2 files changed, 53 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index ad11540ac436..6d032a6be16c 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -28,6 +28,7 @@ > #include <drm/drm_scdc_helper.h> > #include "i915_drv.h" > #include "intel_drv.h" > +#include "intel_dsi.h" > > struct ddi_buf_trans { > u32 trans1; /* balance leg enable, de-emph level */ > @@ -2854,8 +2855,9 @@ void icl_sanitize_encoder_pll_mapping(struct > intel_encoder *encoder) > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > u32 val; > - enum port port = encoder->port; > - bool clk_enabled; > + enum port port; > + u32 port_mask; > + bool ddi_clk_needed; > > /* > * In case of DP MST, we sanitize the primary encoder only, not the > @@ -2864,9 +2866,6 @@ void icl_sanitize_encoder_pll_mapping(struct > intel_encoder *encoder) > 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; > @@ -2880,20 +2879,52 @@ void icl_sanitize_encoder_pll_mapping(struct > intel_encoder *encoder) > return; > } > > - if (clk_enabled == !!encoder->base.crtc) > - return; > + port_mask = BIT(encoder->port); > + ddi_clk_needed = encoder->base.crtc; > > - /* > - * Punt on the case now where clock is disabled, but the encoder is > - * enabled, something else is really broken then. > - */ > - if (WARN_ON(!clk_enabled)) > - return; > + if (encoder->type == INTEL_OUTPUT_DSI) { > + struct intel_encoder *other_encoder; > > - DRM_NOTE("Port %c is disabled but it has a mapped PLL, unmap it\n", > - port_name(port)); > - val |= icl_dpclka_cfgcr0_clk_off(dev_priv, port); > - I915_WRITE(DPCLKA_CFGCR0_ICL, val); > + port_mask = intel_dsi_encoder_ports(encoder); > + /* > + * Sanity check that we haven't incorrectly registered another > + * encoder using any of the ports of this DSI encoder. > + */ > + for_each_intel_encoder(&dev_priv->drm, other_encoder) { > + if (other_encoder == encoder) > + continue; > + > + if (WARN_ON(port_mask & BIT(other_encoder->port))) > + return; > + } > + /* > + * DSI ports should have their DDI clock ungated when disabled > + * and gated when enabled. > + */ > + ddi_clk_needed = !encoder->base.crtc; > + } > + > + val = I915_READ(DPCLKA_CFGCR0_ICL); > + for_each_port_masked(port, port_mask) { > + bool ddi_clk_ungated = !(val & > + icl_dpclka_cfgcr0_clk_off(dev_priv, > + port)); > + > + if (ddi_clk_needed == ddi_clk_ungated) > + continue; > + > + /* > + * Punt on the case now where clock is gated, but it would > + * be needed by the port. Something else is really broken then. > + */ > + if (WARN_ON(ddi_clk_needed)) > + continue; > + > + DRM_NOTE("Port %c is disabled/in DSI mode with an ungated DDI > clock, gate it\n", > + port_name(port)); > + val |= icl_dpclka_cfgcr0_clk_off(dev_priv, port); > + I915_WRITE(DPCLKA_CFGCR0_ICL, val); > + } > } > > static void intel_ddi_clk_select(struct intel_encoder *encoder, > diff --git a/drivers/gpu/drm/i915/intel_dsi.h > b/drivers/gpu/drm/i915/intel_dsi.h > index ee93137f4433..d968f1f13e09 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.h > +++ b/drivers/gpu/drm/i915/intel_dsi.h > @@ -146,6 +146,11 @@ static inline bool is_cmd_mode(struct intel_dsi > *intel_dsi) > return intel_dsi->operation_mode == INTEL_DSI_COMMAND_MODE; > } > > +static inline u16 intel_dsi_encoder_ports(struct intel_encoder *encoder) > +{ > + return enc_to_intel_dsi(&encoder->base)->ports; > +} > + > /* intel_dsi.c */ > int intel_dsi_bitrate(const struct intel_dsi *intel_dsi); > int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi); -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
