[Intel-gfx] [PATCH] drm/i915/icl: Sanitize DDI port clock gating for DSI ports

[Jani Nikula]

On Tue, 27 Nov 2018, Imre Deak <imre.deak at intel.com> 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 at intel.com>
> Cc: Vandita Kulkarni <vandita.kulkarni at intel.com>
> Cc: Jani Nikula <jani.nikula at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Clint Taylor <clinton.a.taylor at intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>

Reviewed-by: Jani Nikula <jani.nikula at intel.com>

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