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>
> ---
> 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
