drm/i915/icl: Sanitize DDI port clock gating for DSI ports

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

---
 drivers/gpu/drm/i915/intel_ddi.c | 65 +++++++++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_dsi.h |  5 ++++
 2 files changed, 53 insertions(+), 17 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +48 -17

index ad11540ac436..6d032a6be16c 100644
@@ -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,

drivers/gpu/drm/i915/intel_dsi.h +5 -0

index ee93137f4433..d968f1f13e09 100644
@@ -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);

Quoted 11 lines · Imre Deak

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

Quoted 6 lines

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

Quoted 121 lines

---
 drivers/gpu/drm/i915/intel_ddi.c | 65 +++++++++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_dsi.h |  5 ++++
 2 files changed, 53 insertions(+), 17 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +48 -17

index ad11540ac436..6d032a6be16c 100644
@@ -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,

drivers/gpu/drm/i915/intel_dsi.h +5 -0

index ee93137f4433..d968f1f13e09 100644
@@ -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);