On Thu, 29 Nov 2018, Jani Nikula <jani.nikula@xxxxxxxxx> wrote: > On Tue, 27 Nov 2018, Imre Deak <imre.deak@xxxxxxxxx> 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 <madhav.chauhan@xxxxxxxxx> >> Cc: Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx> >> Cc: Jani Nikula <jani.nikula@xxxxxxxxx> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Cc: Clint Taylor <clinton.a.taylor@xxxxxxxxx> >> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > > Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx> Thanks for the patch, pushed to dinq as part of [1]. BR, Jani. [1] https://patchwork.freedesktop.org/series/51011/ > >> --- >> 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 Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
