drm/i915/icl: dsi enabling

Encoders are not alike, make enable and disable hooks optional like other hooks. Utilize this in DSI code, and remove the silly nop hook.

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/intel_display.c |  6 ++++--
  drivers/gpu/drm/i915/vlv_dsi.c       | 16 ++++------------
  2 files changed, 8 insertions(+), 14 deletions(-)

drivers/gpu/drm/i915/intel_display.c +2 -1

index 41abd03ce6a6..32ea71bac663 100644
@@ -5461,7 +5461,8 @@ static void intel_encoders_enable(struct drm_crtc *crtc,
  		if (conn_state->crtc != crtc)
  			continue;
 
-		encoder->enable(encoder, crtc_state, conn_state);
+		if (encoder->enable)
+			encoder->enable(encoder, crtc_state, conn_state);
  		intel_opregion_notify_encoder(encoder, true);
  	}
  }

drivers/gpu/drm/i915/intel_display.c +0 -0

  }
 

drivers/gpu/drm/i915/vlv_dsi.c +4 -11

index bafeb2a19b90..dbca30460a6b 100644
@@ -794,6 +794,10 @@ static void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec)
   * - wait t4                                           - wait t4
   */
 
+/*
+ * DSI port enable has to be done before pipe and plane enable, so we do it in
+ * the pre_enable hook instead of the enable hook.
+ */
  static void intel_dsi_pre_enable(struct intel_encoder *encoder,
  				 const struct intel_crtc_state *pipe_config,
  				 const struct drm_connector_state *conn_state)

@@ -896,17 +900,6 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
  }
 
  /*
- * DSI port enable has to be done before pipe and plane enable, so we do it in
- * the pre_enable hook.
- */
-static void intel_dsi_enable_nop(struct intel_encoder *encoder,
-				 const struct intel_crtc_state *pipe_config,
-				 const struct drm_connector_state *conn_state)
-{
-	DRM_DEBUG_KMS("\n");
-}
-
-/*
   * DSI port disable has to be done after pipe and plane disable, so we do it in
   * the post_disable hook.
   */

Abstract bitrate calculation to a newly resurrected intel_dsi.c file that will contain common code for VLV and ICL DSI.

No functional changes.

Cc Madhav Chauhan <[email protected]>

Cc Ville Syrjala <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/Makefile        |  1 +
  drivers/gpu/drm/i915/intel_dsi.c     | 17 +++++++++++++++++
  drivers/gpu/drm/i915/intel_dsi.h     |  3 +++
  drivers/gpu/drm/i915/intel_dsi_vbt.c | 28 ++++++++++------------------
  4 files changed, 31 insertions(+), 18 deletions(-)

create mode 100644 drivers/gpu/drm/i915/intel_dsi.c

drivers/gpu/drm/i915/Makefile +1 -0

index 48cae0eae3f9..22cbf9c3bb0c 100644
@@ -143,6 +143,7 @@ i915-y += dvo_ch7017.o \
  	  intel_dp_link_training.o \
  	  intel_dp_mst.o \
  	  intel_dp.o \
+	  intel_dsi.o \
  	  intel_dsi_dcs_backlight.o \
  	  intel_dsi_vbt.o \
  	  intel_dvo.o \

drivers/gpu/drm/i915/intel_dsi.c new file +4 -0

new file mode 100644
index 000000000000..4daa1da94047
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2018 Intel Corporation
+ */

drivers/gpu/drm/i915/intel_dsi.c new file +3 -0

+
+	return intel_dsi->pclk * bpp / intel_dsi->lane_count;
+}

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

index ad7c1cb32983..68f14d8f1e18 100644
@@ -129,6 +129,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
  	return container_of(encoder, struct intel_dsi, base.base);
  }
 
+/* intel_dsi.c */
+int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
+
  /* vlv_dsi.c */
  void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
  enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);

drivers/gpu/drm/i915/intel_dsi_vbt.c +9 -17

index ac83d6b89ae0..6c4cc92f5947 100644
@@ -506,14 +506,12 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
  	struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
  	struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
-	u32 bpp;
-	u32 tlpx_ns, extra_byte_count, bitrate, tlpx_ui;
+	u32 tlpx_ns, extra_byte_count, tlpx_ui;
  	u32 ui_num, ui_den;
  	u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt, trail_cnt;
  	u32 ths_prepare_ns, tclk_trail_ns;
  	u32 tclk_prepare_clkzero, ths_prepare_hszero;
  	u32 lp_to_hs_switch, hs_to_lp_switch;
-	u32 pclk, computed_ddr;
  	u32 mul;
  	u16 burst_mode_ratio;
  	enum port port;

@@ -526,7 +524,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  	intel_dsi->pixel_format =
  			pixel_format_from_register_bits(
  				mipi_config->videomode_color_format << 7);
-	bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
 
  	intel_dsi->dual_link = mipi_config->dual_link;
  	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;

@@ -541,19 +538,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  	intel_dsi->video_frmt_cfg_bits =
  		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
 
-	pclk = mode->clock;
+	/* Starting point, adjusted depending on dual link and burst mode */
+	intel_dsi->pclk = mode->clock;
 
  	/* In dual link mode each port needs half of pixel clock */
  	if (intel_dsi->dual_link) {
-		pclk = pclk / 2;
+		intel_dsi->pclk /= 2;
 
  		/* we can enable pixel_overlap if needed by panel. In this
  		 * case we need to increase the pixelclock for extra pixels
  		 */
  		if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
-			pclk += DIV_ROUND_UP(mode->vtotal *
-						intel_dsi->pixel_overlap *
-						60, 1000);
+			intel_dsi->pclk += DIV_ROUND_UP(mode->vtotal * intel_dsi->pixel_overlap * 60, 1000);
  		}
  	}
 

@@ -563,19 +559,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  	 */
  	if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
  		if (mipi_config->target_burst_mode_freq) {
-			computed_ddr = (pclk * bpp) / intel_dsi->lane_count;
+			u32 bitrate = intel_dsi_bitrate(intel_dsi);
 
-			if (mipi_config->target_burst_mode_freq <
-								computed_ddr) {
+			if (mipi_config->target_burst_mode_freq < bitrate) {
  				DRM_ERROR("Burst mode freq is less than computed\n");
  				return false;
  			}
 
  			burst_mode_ratio = DIV_ROUND_UP(
  				mipi_config->target_burst_mode_freq * 100,
-				computed_ddr);
+				bitrate);
 
-			pclk = DIV_ROUND_UP(pclk * burst_mode_ratio, 100);
+			intel_dsi->pclk = DIV_ROUND_UP(intel_dsi->pclk * burst_mode_ratio, 100);
  		} else {
  			DRM_ERROR("Burst mode target is not set\n");
  			return false;

@@ -584,9 +579,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  		burst_mode_ratio = 100;
 
  	intel_dsi->burst_mode_ratio = burst_mode_ratio;
-	intel_dsi->pclk = pclk;
-
-	bitrate = (pclk * bpp) / intel_dsi->lane_count;
 
  	switch (intel_dsi->escape_clk_div) {
  	case 0:

Quoted 43 lines · Jani Nikula

Abstract bitrate calculation to a newly resurrected intel_dsi.c file that will contain common code for VLV and ICL DSI.

No functional changes.

Cc Madhav Chauhan <[email protected]>

Cc Ville Syrjala <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/Makefile        |  1 +
  drivers/gpu/drm/i915/intel_dsi.c     | 17 +++++++++++++++++
  drivers/gpu/drm/i915/intel_dsi.h     |  3 +++
  drivers/gpu/drm/i915/intel_dsi_vbt.c | 28 ++++++++++------------------
  4 files changed, 31 insertions(+), 18 deletions(-)

create mode 100644 drivers/gpu/drm/i915/intel_dsi.c

drivers/gpu/drm/i915/Makefile +1 -0

index 48cae0eae3f9..22cbf9c3bb0c 100644
@@ -143,6 +143,7 @@ i915-y += dvo_ch7017.o \
  	  intel_dp_link_training.o \
  	  intel_dp_mst.o \
  	  intel_dp.o \
+	  intel_dsi.o \
  	  intel_dsi_dcs_backlight.o \
  	  intel_dsi_vbt.o \
  	  intel_dvo.o \

drivers/gpu/drm/i915/intel_dsi.c new file +9 -0

new file mode 100644
index 000000000000..4daa1da94047
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2018 Intel Corporation
+ */
+
+#include <drm/drm_mipi_dsi.h>
+#include "intel_dsi.h"
+
+int intel_dsi_bitrate(const struct intel_dsi *intel_dsi)

drivers/gpu/drm/i915/intel_dsi.c line 14

Quoted 5 lines

drivers/gpu/drm/i915/intel_dsi.c new file +5 -0

+{
+	int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
+
+	if (WARN_ON(bpp < 0))
+		bpp = 16;

Shouldn't we keep the default bpp to 24 here as in most of the cases bpp is 24 for DSI or why 16??

Regards,
Madhav

Quoted 112 lines

drivers/gpu/drm/i915/intel_dsi.c new file +3 -0

+
+	return intel_dsi->pclk * bpp / intel_dsi->lane_count;
+}

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

index ad7c1cb32983..68f14d8f1e18 100644
@@ -129,6 +129,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
  	return container_of(encoder, struct intel_dsi, base.base);
  }
 
+/* intel_dsi.c */
+int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
+
  /* vlv_dsi.c */
  void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
  enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);

drivers/gpu/drm/i915/intel_dsi_vbt.c +10 -18

index ac83d6b89ae0..6c4cc92f5947 100644
@@ -506,14 +506,12 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
  	struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
  	struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
-	u32 bpp;
-	u32 tlpx_ns, extra_byte_count, bitrate, tlpx_ui;
+	u32 tlpx_ns, extra_byte_count, tlpx_ui;
  	u32 ui_num, ui_den;
  	u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt, trail_cnt;
  	u32 ths_prepare_ns, tclk_trail_ns;
  	u32 tclk_prepare_clkzero, ths_prepare_hszero;
  	u32 lp_to_hs_switch, hs_to_lp_switch;
-	u32 pclk, computed_ddr;
  	u32 mul;
  	u16 burst_mode_ratio;
  	enum port port;

@@ -526,7 +524,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  	intel_dsi->pixel_format =
  			pixel_format_from_register_bits(
  				mipi_config->videomode_color_format << 7);
-	bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
 
  	intel_dsi->dual_link = mipi_config->dual_link;
  	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;

@@ -541,19 +538,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  	intel_dsi->video_frmt_cfg_bits =
  		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
 
-	pclk = mode->clock;
+	/* Starting point, adjusted depending on dual link and burst mode */
+	intel_dsi->pclk = mode->clock;
 
  	/* In dual link mode each port needs half of pixel clock */
  	if (intel_dsi->dual_link) {
-		pclk = pclk / 2;
+		intel_dsi->pclk /= 2;
 
  		/* we can enable pixel_overlap if needed by panel. In this
  		 * case we need to increase the pixelclock for extra pixels
  		 */
  		if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
-			pclk += DIV_ROUND_UP(mode->vtotal *
-						intel_dsi->pixel_overlap *
-						60, 1000);
+			intel_dsi->pclk += DIV_ROUND_UP(mode->vtotal * intel_dsi->pixel_overlap * 60, 1000);
  		}
  	}
 

@@ -563,19 +559,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  	 */
  	if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
  		if (mipi_config->target_burst_mode_freq) {
-			computed_ddr = (pclk * bpp) / intel_dsi->lane_count;
+			u32 bitrate = intel_dsi_bitrate(intel_dsi);
 
-			if (mipi_config->target_burst_mode_freq <
-								computed_ddr) {
+			if (mipi_config->target_burst_mode_freq < bitrate) {
  				DRM_ERROR("Burst mode freq is less than computed\n");
  				return false;
  			}
 
  			burst_mode_ratio = DIV_ROUND_UP(
  				mipi_config->target_burst_mode_freq * 100,
-				computed_ddr);
+				bitrate);
 
-			pclk = DIV_ROUND_UP(pclk * burst_mode_ratio, 100);
+			intel_dsi->pclk = DIV_ROUND_UP(intel_dsi->pclk * burst_mode_ratio, 100);
  		} else {
  			DRM_ERROR("Burst mode target is not set\n");
  			return false;

@@ -584,9 +579,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  		burst_mode_ratio = 100;
 
  	intel_dsi->burst_mode_ratio = burst_mode_ratio;
-	intel_dsi->pclk = pclk;
-
-	bitrate = (pclk * bpp) / intel_dsi->lane_count;
 
  	switch (intel_dsi->escape_clk_div) {
  	case 0:

@@ -620,7 +612,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 
  	/* in Kbps */
  	ui_num = NS_KHZ_RATIO;
-	ui_den = bitrate;
+	ui_den = intel_dsi_bitrate(intel_dsi);
 
  	tclk_prepare_clkzero = mipi_config->tclk_prepare_clkzero;
  	ths_prepare_hszero = mipi_config->ths_prepare_hszero;

Quoted 43 lines · Madhav Chauhan

Quoted 42 lines · Jani Nikula

Abstract bitrate calculation to a newly resurrected intel_dsi.c file that will contain common code for VLV and ICL DSI.

No functional changes.

Cc Madhav Chauhan <[email protected]>

Cc Ville Syrjala <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
   drivers/gpu/drm/i915/Makefile        |  1 +
   drivers/gpu/drm/i915/intel_dsi.c     | 17 +++++++++++++++++
   drivers/gpu/drm/i915/intel_dsi.h     |  3 +++
   drivers/gpu/drm/i915/intel_dsi_vbt.c | 28 ++++++++++------------------
   4 files changed, 31 insertions(+), 18 deletions(-)

create mode 100644 drivers/gpu/drm/i915/intel_dsi.c

drivers/gpu/drm/i915/Makefile +1 -0

index 48cae0eae3f9..22cbf9c3bb0c 100644
@@ -143,6 +143,7 @@ i915-y += dvo_ch7017.o \
   	  intel_dp_link_training.o \
   	  intel_dp_mst.o \
   	  intel_dp.o \
+	  intel_dsi.o \
   	  intel_dsi_dcs_backlight.o \
   	  intel_dsi_vbt.o \
   	  intel_dvo.o \

drivers/gpu/drm/i915/intel_dsi.c new file +8 -0

new file mode 100644
index 000000000000..4daa1da94047
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2018 Intel Corporation
+ */
+
+#include <drm/drm_mipi_dsi.h>
+#include "intel_dsi.h"
+

Quoted 8 lines

drivers/gpu/drm/i915/intel_dsi.c line 14

Quoted 6 lines

drivers/gpu/drm/i915/intel_dsi.c new file +6 -0

+int intel_dsi_bitrate(const struct intel_dsi *intel_dsi)
+{
+	int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
+
+	if (WARN_ON(bpp < 0))
+		bpp = 16;

Shouldn't we keep the default bpp to 24 here as in most of the cases bpp is 24 for DSI or why 16??

*shrug*

Not sure it matters all that much really. We set the pixel format in intel_dsi_vbt.c, and if that gives us something bogus, not much of a chance any of it will work. More than anything I just wanted to avoid a negative return from this function.

BR,
Jani.

Quoted 108 lines

Regards,
Madhav

Quoted 105 lines

drivers/gpu/drm/i915/intel_dsi.c new file +3 -0

+
+	return intel_dsi->pclk * bpp / intel_dsi->lane_count;
+}

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

index ad7c1cb32983..68f14d8f1e18 100644
@@ -129,6 +129,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
   	return container_of(encoder, struct intel_dsi, base.base);
   }
 
+/* intel_dsi.c */
+int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
+
   /* vlv_dsi.c */
   void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
   enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);

drivers/gpu/drm/i915/intel_dsi_vbt.c +9 -17

index ac83d6b89ae0..6c4cc92f5947 100644
@@ -506,14 +506,12 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
   	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
   	struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
   	struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
-	u32 bpp;
-	u32 tlpx_ns, extra_byte_count, bitrate, tlpx_ui;
+	u32 tlpx_ns, extra_byte_count, tlpx_ui;
   	u32 ui_num, ui_den;
   	u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt, trail_cnt;
   	u32 ths_prepare_ns, tclk_trail_ns;
   	u32 tclk_prepare_clkzero, ths_prepare_hszero;
   	u32 lp_to_hs_switch, hs_to_lp_switch;
-	u32 pclk, computed_ddr;
   	u32 mul;
   	u16 burst_mode_ratio;
   	enum port port;

@@ -526,7 +524,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
   	intel_dsi->pixel_format =
   			pixel_format_from_register_bits(
   				mipi_config->videomode_color_format << 7);
-	bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
 
   	intel_dsi->dual_link = mipi_config->dual_link;
   	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;

@@ -541,19 +538,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
   	intel_dsi->video_frmt_cfg_bits =
   		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
 
-	pclk = mode->clock;
+	/* Starting point, adjusted depending on dual link and burst mode */
+	intel_dsi->pclk = mode->clock;
 
   	/* In dual link mode each port needs half of pixel clock */
   	if (intel_dsi->dual_link) {
-		pclk = pclk / 2;
+		intel_dsi->pclk /= 2;
 
   		/* we can enable pixel_overlap if needed by panel. In this
   		 * case we need to increase the pixelclock for extra pixels
   		 */
   		if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
-			pclk += DIV_ROUND_UP(mode->vtotal *
-						intel_dsi->pixel_overlap *
-						60, 1000);
+			intel_dsi->pclk += DIV_ROUND_UP(mode->vtotal * intel_dsi->pixel_overlap * 60, 1000);
   		}
   	}
 

@@ -563,19 +559,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
   	 */
   	if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
   		if (mipi_config->target_burst_mode_freq) {
-			computed_ddr = (pclk * bpp) / intel_dsi->lane_count;
+			u32 bitrate = intel_dsi_bitrate(intel_dsi);
 
-			if (mipi_config->target_burst_mode_freq <
-								computed_ddr) {
+			if (mipi_config->target_burst_mode_freq < bitrate) {
   				DRM_ERROR("Burst mode freq is less than computed\n");
   				return false;
   			}
 
   			burst_mode_ratio = DIV_ROUND_UP(
   				mipi_config->target_burst_mode_freq * 100,
-				computed_ddr);
+				bitrate);
 
-			pclk = DIV_ROUND_UP(pclk * burst_mode_ratio, 100);
+			intel_dsi->pclk = DIV_ROUND_UP(intel_dsi->pclk * burst_mode_ratio, 100);
   		} else {
   			DRM_ERROR("Burst mode target is not set\n");
   			return false;

@@ -584,9 +579,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
   		burst_mode_ratio = 100;
 
   	intel_dsi->burst_mode_ratio = burst_mode_ratio;
-	intel_dsi->pclk = pclk;
-
-	bitrate = (pclk * bpp) / intel_dsi->lane_count;
 
   	switch (intel_dsi->escape_clk_div) {
   	case 0:

@@ -620,7 +612,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 

Quoted 7 lines

drivers/gpu/drm/i915/intel_dsi_vbt.c bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id) · line 618

Quoted 7 lines

drivers/gpu/drm/i915/intel_dsi_vbt.c +1 -1

   	/* in Kbps */
   	ui_num = NS_KHZ_RATIO;
-	ui_den = bitrate;
+	ui_den = intel_dsi_bitrate(intel_dsi);
 
   	tclk_prepare_clkzero = mipi_config->tclk_prepare_clkzero;
   	ths_prepare_hszero = mipi_config->ths_prepare_hszero;

intel_dsi_vbt_init() has grown too unwieldy, and it's about to be modified due to ICL DSI. Abstract out the VLV specific dphy param init. No functional changes. Intentionally no stylistic changes during code movement.

Cc Madhav Chauhan <[email protected]>

Cc Ville Syrjala <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/intel_dsi_vbt.c | 147 +++++++++++++++++++----------------
  1 file changed, 78 insertions(+), 69 deletions(-)

drivers/gpu/drm/i915/intel_dsi_vbt.c +71 -69

index 6c4cc92f5947..fdeba8386d53 100644
@@ -499,13 +499,11 @@ int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi)
  	return 1;
  }
 
-bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
+static void vlv_dphy_param_init(struct intel_dsi *intel_dsi)
  {
  	struct drm_device *dev = intel_dsi->base.base.dev;
  	struct drm_i915_private *dev_priv = to_i915(dev);
  	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
-	struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
-	struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
  	u32 tlpx_ns, extra_byte_count, tlpx_ui;
  	u32 ui_num, ui_den;
  	u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt, trail_cnt;

@@ -513,72 +511,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  	u32 tclk_prepare_clkzero, ths_prepare_hszero;
  	u32 lp_to_hs_switch, hs_to_lp_switch;
  	u32 mul;
-	u16 burst_mode_ratio;
-	enum port port;
-
-	DRM_DEBUG_KMS("\n");
-
-	intel_dsi->eotp_pkt = mipi_config->eot_pkt_disabled ? 0 : 1;
-	intel_dsi->clock_stop = mipi_config->enable_clk_stop ? 1 : 0;
-	intel_dsi->lane_count = mipi_config->lane_cnt + 1;
-	intel_dsi->pixel_format =
-			pixel_format_from_register_bits(
-				mipi_config->videomode_color_format << 7);
-
-	intel_dsi->dual_link = mipi_config->dual_link;
-	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
-	intel_dsi->operation_mode = mipi_config->is_cmd_mode;
-	intel_dsi->video_mode_format = mipi_config->video_transfer_mode;
-	intel_dsi->escape_clk_div = mipi_config->byte_clk_sel;
-	intel_dsi->lp_rx_timeout = mipi_config->lp_rx_timeout;
-	intel_dsi->turn_arnd_val = mipi_config->turn_around_timeout;
-	intel_dsi->rst_timer_val = mipi_config->device_reset_timer;
-	intel_dsi->init_count = mipi_config->master_init_timer;
-	intel_dsi->bw_timer = mipi_config->dbi_bw_timer;
-	intel_dsi->video_frmt_cfg_bits =
-		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
-
-	/* Starting point, adjusted depending on dual link and burst mode */
-	intel_dsi->pclk = mode->clock;
-
-	/* In dual link mode each port needs half of pixel clock */
-	if (intel_dsi->dual_link) {
-		intel_dsi->pclk /= 2;
-
-		/* we can enable pixel_overlap if needed by panel. In this
-		 * case we need to increase the pixelclock for extra pixels
-		 */
-		if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
-			intel_dsi->pclk += DIV_ROUND_UP(mode->vtotal * intel_dsi->pixel_overlap * 60, 1000);
-		}
-	}
-
-	/* Burst Mode Ratio
-	 * Target ddr frequency from VBT / non burst ddr freq
-	 * multiply by 100 to preserve remainder
-	 */
-	if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
-		if (mipi_config->target_burst_mode_freq) {
-			u32 bitrate = intel_dsi_bitrate(intel_dsi);
-
-			if (mipi_config->target_burst_mode_freq < bitrate) {
-				DRM_ERROR("Burst mode freq is less than computed\n");
-				return false;
-			}
-
-			burst_mode_ratio = DIV_ROUND_UP(
-				mipi_config->target_burst_mode_freq * 100,
-				bitrate);
-
-			intel_dsi->pclk = DIV_ROUND_UP(intel_dsi->pclk * burst_mode_ratio, 100);
-		} else {
-			DRM_ERROR("Burst mode target is not set\n");
-			return false;
-		}
-	} else
-		burst_mode_ratio = 100;
-
-	intel_dsi->burst_mode_ratio = burst_mode_ratio;
 
  	switch (intel_dsi->escape_clk_div) {
  	case 0:

@@ -738,6 +670,83 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  		DIV_ROUND_UP(2 * tlpx_ui + trail_cnt * 2 + 8,
  			8);
  	intel_dsi->clk_hs_to_lp_count += extra_byte_count;
+}
+
+bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
+{
+	struct drm_device *dev = intel_dsi->base.base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
+	struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
+	struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
+	u16 burst_mode_ratio;
+	enum port port;
+
+	DRM_DEBUG_KMS("\n");
+
+	intel_dsi->eotp_pkt = mipi_config->eot_pkt_disabled ? 0 : 1;
+	intel_dsi->clock_stop = mipi_config->enable_clk_stop ? 1 : 0;
+	intel_dsi->lane_count = mipi_config->lane_cnt + 1;
+	intel_dsi->pixel_format =
+			pixel_format_from_register_bits(
+				mipi_config->videomode_color_format << 7);
+
+	intel_dsi->dual_link = mipi_config->dual_link;
+	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
+	intel_dsi->operation_mode = mipi_config->is_cmd_mode;
+	intel_dsi->video_mode_format = mipi_config->video_transfer_mode;
+	intel_dsi->escape_clk_div = mipi_config->byte_clk_sel;
+	intel_dsi->lp_rx_timeout = mipi_config->lp_rx_timeout;
+	intel_dsi->turn_arnd_val = mipi_config->turn_around_timeout;
+	intel_dsi->rst_timer_val = mipi_config->device_reset_timer;
+	intel_dsi->init_count = mipi_config->master_init_timer;
+	intel_dsi->bw_timer = mipi_config->dbi_bw_timer;
+	intel_dsi->video_frmt_cfg_bits =
+		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
+
+	/* Starting point, adjusted depending on dual link and burst mode */
+	intel_dsi->pclk = mode->clock;
+
+	/* In dual link mode each port needs half of pixel clock */
+	if (intel_dsi->dual_link) {
+		intel_dsi->pclk /= 2;
+
+		/* we can enable pixel_overlap if needed by panel. In this
+		 * case we need to increase the pixelclock for extra pixels
+		 */
+		if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
+			intel_dsi->pclk += DIV_ROUND_UP(mode->vtotal * intel_dsi->pixel_overlap * 60, 1000);
+		}
+	}
+
+	/* Burst Mode Ratio
+	 * Target ddr frequency from VBT / non burst ddr freq
+	 * multiply by 100 to preserve remainder
+	 */
+	if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
+		if (mipi_config->target_burst_mode_freq) {
+			u32 bitrate = intel_dsi_bitrate(intel_dsi);
+
+			if (mipi_config->target_burst_mode_freq < bitrate) {
+				DRM_ERROR("Burst mode freq is less than computed\n");
+				return false;
+			}
+
+			burst_mode_ratio = DIV_ROUND_UP(
+				mipi_config->target_burst_mode_freq * 100,
+				bitrate);
+
+			intel_dsi->pclk = DIV_ROUND_UP(intel_dsi->pclk * burst_mode_ratio, 100);
+		} else {
+			DRM_ERROR("Burst mode target is not set\n");
+			return false;

Quoted 5 lines · Jani Nikula

intel_dsi_vbt_init() has grown too unwieldy, and it's about to be modified due to ICL DSI. Abstract out the VLV specific dphy param init. No functional changes. Intentionally no stylistic changes during code movement.

Quoted 3 lines

Cc Madhav Chauhan <[email protected]>

Cc Ville Syrjala <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

Looks ok to me,

Reviewed-by Madhav Chauhan <[email protected]>

Regards,
Madhav

Quoted 180 lines

---
  drivers/gpu/drm/i915/intel_dsi_vbt.c | 147 +++++++++++++++++++----------------
  1 file changed, 78 insertions(+), 69 deletions(-)

drivers/gpu/drm/i915/intel_dsi_vbt.c +78 -69

index 6c4cc92f5947..fdeba8386d53 100644
@@ -499,13 +499,11 @@ int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi)
  	return 1;
  }
 
-bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
+static void vlv_dphy_param_init(struct intel_dsi *intel_dsi)
  {
  	struct drm_device *dev = intel_dsi->base.base.dev;
  	struct drm_i915_private *dev_priv = to_i915(dev);
  	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
-	struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
-	struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
  	u32 tlpx_ns, extra_byte_count, tlpx_ui;
  	u32 ui_num, ui_den;
  	u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt, trail_cnt;

@@ -513,72 +511,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  	u32 tclk_prepare_clkzero, ths_prepare_hszero;
  	u32 lp_to_hs_switch, hs_to_lp_switch;
  	u32 mul;
-	u16 burst_mode_ratio;
-	enum port port;
-
-	DRM_DEBUG_KMS("\n");
-
-	intel_dsi->eotp_pkt = mipi_config->eot_pkt_disabled ? 0 : 1;
-	intel_dsi->clock_stop = mipi_config->enable_clk_stop ? 1 : 0;
-	intel_dsi->lane_count = mipi_config->lane_cnt + 1;
-	intel_dsi->pixel_format =
-			pixel_format_from_register_bits(
-				mipi_config->videomode_color_format << 7);
-
-	intel_dsi->dual_link = mipi_config->dual_link;
-	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
-	intel_dsi->operation_mode = mipi_config->is_cmd_mode;
-	intel_dsi->video_mode_format = mipi_config->video_transfer_mode;
-	intel_dsi->escape_clk_div = mipi_config->byte_clk_sel;
-	intel_dsi->lp_rx_timeout = mipi_config->lp_rx_timeout;
-	intel_dsi->turn_arnd_val = mipi_config->turn_around_timeout;
-	intel_dsi->rst_timer_val = mipi_config->device_reset_timer;
-	intel_dsi->init_count = mipi_config->master_init_timer;
-	intel_dsi->bw_timer = mipi_config->dbi_bw_timer;
-	intel_dsi->video_frmt_cfg_bits =
-		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
-
-	/* Starting point, adjusted depending on dual link and burst mode */
-	intel_dsi->pclk = mode->clock;
-
-	/* In dual link mode each port needs half of pixel clock */
-	if (intel_dsi->dual_link) {
-		intel_dsi->pclk /= 2;
-
-		/* we can enable pixel_overlap if needed by panel. In this
-		 * case we need to increase the pixelclock for extra pixels
-		 */
-		if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
-			intel_dsi->pclk += DIV_ROUND_UP(mode->vtotal * intel_dsi->pixel_overlap * 60, 1000);
-		}
-	}
-
-	/* Burst Mode Ratio
-	 * Target ddr frequency from VBT / non burst ddr freq
-	 * multiply by 100 to preserve remainder
-	 */
-	if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
-		if (mipi_config->target_burst_mode_freq) {
-			u32 bitrate = intel_dsi_bitrate(intel_dsi);
-
-			if (mipi_config->target_burst_mode_freq < bitrate) {
-				DRM_ERROR("Burst mode freq is less than computed\n");
-				return false;
-			}
-
-			burst_mode_ratio = DIV_ROUND_UP(
-				mipi_config->target_burst_mode_freq * 100,
-				bitrate);
-
-			intel_dsi->pclk = DIV_ROUND_UP(intel_dsi->pclk * burst_mode_ratio, 100);
-		} else {
-			DRM_ERROR("Burst mode target is not set\n");
-			return false;
-		}
-	} else
-		burst_mode_ratio = 100;
-
-	intel_dsi->burst_mode_ratio = burst_mode_ratio;
 
  	switch (intel_dsi->escape_clk_div) {
  	case 0:

@@ -738,6 +670,83 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  		DIV_ROUND_UP(2 * tlpx_ui + trail_cnt * 2 + 8,
  			8);
  	intel_dsi->clk_hs_to_lp_count += extra_byte_count;
+}
+
+bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
+{
+	struct drm_device *dev = intel_dsi->base.base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
+	struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
+	struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
+	u16 burst_mode_ratio;
+	enum port port;
+
+	DRM_DEBUG_KMS("\n");
+
+	intel_dsi->eotp_pkt = mipi_config->eot_pkt_disabled ? 0 : 1;
+	intel_dsi->clock_stop = mipi_config->enable_clk_stop ? 1 : 0;
+	intel_dsi->lane_count = mipi_config->lane_cnt + 1;
+	intel_dsi->pixel_format =
+			pixel_format_from_register_bits(
+				mipi_config->videomode_color_format << 7);
+
+	intel_dsi->dual_link = mipi_config->dual_link;
+	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
+	intel_dsi->operation_mode = mipi_config->is_cmd_mode;
+	intel_dsi->video_mode_format = mipi_config->video_transfer_mode;
+	intel_dsi->escape_clk_div = mipi_config->byte_clk_sel;
+	intel_dsi->lp_rx_timeout = mipi_config->lp_rx_timeout;
+	intel_dsi->turn_arnd_val = mipi_config->turn_around_timeout;
+	intel_dsi->rst_timer_val = mipi_config->device_reset_timer;
+	intel_dsi->init_count = mipi_config->master_init_timer;
+	intel_dsi->bw_timer = mipi_config->dbi_bw_timer;
+	intel_dsi->video_frmt_cfg_bits =
+		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
+
+	/* Starting point, adjusted depending on dual link and burst mode */
+	intel_dsi->pclk = mode->clock;
+
+	/* In dual link mode each port needs half of pixel clock */
+	if (intel_dsi->dual_link) {
+		intel_dsi->pclk /= 2;
+
+		/* we can enable pixel_overlap if needed by panel. In this
+		 * case we need to increase the pixelclock for extra pixels
+		 */
+		if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
+			intel_dsi->pclk += DIV_ROUND_UP(mode->vtotal * intel_dsi->pixel_overlap * 60, 1000);
+		}
+	}
+
+	/* Burst Mode Ratio
+	 * Target ddr frequency from VBT / non burst ddr freq
+	 * multiply by 100 to preserve remainder
+	 */
+	if (intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
+		if (mipi_config->target_burst_mode_freq) {
+			u32 bitrate = intel_dsi_bitrate(intel_dsi);
+
+			if (mipi_config->target_burst_mode_freq < bitrate) {
+				DRM_ERROR("Burst mode freq is less than computed\n");
+				return false;
+			}
+
+			burst_mode_ratio = DIV_ROUND_UP(
+				mipi_config->target_burst_mode_freq * 100,
+				bitrate);
+
+			intel_dsi->pclk = DIV_ROUND_UP(intel_dsi->pclk * burst_mode_ratio, 100);
+		} else {
+			DRM_ERROR("Burst mode target is not set\n");
+			return false;
+		}
+	} else
+		burst_mode_ratio = 100;
+
+	intel_dsi->burst_mode_ratio = burst_mode_ratio;
+
+	vlv_dphy_param_init(intel_dsi);
 
  	DRM_DEBUG_KMS("Pclk %d\n", intel_dsi->pclk);
  	DRM_DEBUG_KMS("Pixel overlap %d\n", intel_dsi->pixel_overlap);

Will be needed in the future. No functional changes.

Cc Madhav Chauhan <[email protected]>

Cc Ville Syrjala <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/intel_dsi.c     | 13 +++++++++++++
  drivers/gpu/drm/i915/intel_dsi.h     |  1 +
  drivers/gpu/drm/i915/intel_dsi_vbt.c | 16 +---------------
  3 files changed, 15 insertions(+), 15 deletions(-)

drivers/gpu/drm/i915/intel_dsi.c +3 -0

index 4daa1da94047..a32cc1f4b384 100644
@@ -15,3 +15,16 @@ int intel_dsi_bitrate(const struct intel_dsi *intel_dsi)
 
  	return intel_dsi->pclk * bpp / intel_dsi->lane_count;
  }
+
+int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi)
+{

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

index 68f14d8f1e18..0d911a4adfaa 100644
@@ -131,6 +131,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 
  /* intel_dsi.c */
  int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
+int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi);
 
  /* vlv_dsi.c */
  void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);

drivers/gpu/drm/i915/intel_dsi_vbt.c +0 -9

index fdeba8386d53..b0d8548f0462 100644
@@ -512,21 +512,7 @@ static void vlv_dphy_param_init(struct intel_dsi *intel_dsi)
  	u32 lp_to_hs_switch, hs_to_lp_switch;
  	u32 mul;
 
-	switch (intel_dsi->escape_clk_div) {
-	case 0:
-		tlpx_ns = 50;
-		break;
-	case 1:
-		tlpx_ns = 100;
-		break;
-
-	case 2:

Quoted 1 line · Jani Nikula

Will be needed in the future. No functional changes.

Agree, will be needing this while setting up DSI protocol timeouts for ICL.

Quoted 26 lines

Cc Madhav Chauhan <[email protected]>

Cc Ville Syrjala <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/intel_dsi.c     | 13 +++++++++++++
  drivers/gpu/drm/i915/intel_dsi.h     |  1 +
  drivers/gpu/drm/i915/intel_dsi_vbt.c | 16 +---------------
  3 files changed, 15 insertions(+), 15 deletions(-)

drivers/gpu/drm/i915/intel_dsi.c +8 -0

index 4daa1da94047..a32cc1f4b384 100644
@@ -15,3 +15,16 @@ int intel_dsi_bitrate(const struct intel_dsi *intel_dsi)
 
  	return intel_dsi->pclk * bpp / intel_dsi->lane_count;
  }
+
+int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi)
+{
+	switch (intel_dsi->escape_clk_div) {
+	default:
+	case 0:
+		return 50;
+	case 1:

drivers/gpu/drm/i915/intel_dsi.c int intel_dsi_bitrate(const struct intel_dsi *intel_dsi) · line 30

Quoted 5 lines

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

+		return 100;
+	case 2:
+		return 200;
+	}
+}

Can we change the return of this function to unsigned int, there is no way that this function can return < 0 as per current implementation??

Regards,
Madhav

Quoted 39 lines

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

index 68f14d8f1e18..0d911a4adfaa 100644
@@ -131,6 +131,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 
  /* intel_dsi.c */
  int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
+int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi);
 
  /* vlv_dsi.c */
  void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);

drivers/gpu/drm/i915/intel_dsi_vbt.c +1 -15

index fdeba8386d53..b0d8548f0462 100644
@@ -512,21 +512,7 @@ static void vlv_dphy_param_init(struct intel_dsi *intel_dsi)
  	u32 lp_to_hs_switch, hs_to_lp_switch;
  	u32 mul;
 
-	switch (intel_dsi->escape_clk_div) {
-	case 0:
-		tlpx_ns = 50;
-		break;
-	case 1:
-		tlpx_ns = 100;
-		break;
-
-	case 2:
-		tlpx_ns = 200;
-		break;
-	default:
-		tlpx_ns = 50;
-		break;
-	}
+	tlpx_ns = intel_dsi_tlpx_ns(intel_dsi);
 
  	switch (intel_dsi->lane_count) {
  	case 1:

Quoted 28 lines · Madhav Chauhan

Quoted 1 line · Jani Nikula

Will be needed in the future. No functional changes.

Agree, will be needing this while setting up DSI protocol timeouts for ICL.

Quoted 24 lines

Cc Madhav Chauhan <[email protected]>

Cc Ville Syrjala <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
   drivers/gpu/drm/i915/intel_dsi.c     | 13 +++++++++++++
   drivers/gpu/drm/i915/intel_dsi.h     |  1 +
   drivers/gpu/drm/i915/intel_dsi_vbt.c | 16 +---------------
   3 files changed, 15 insertions(+), 15 deletions(-)

drivers/gpu/drm/i915/intel_dsi.c +7 -0

index 4daa1da94047..a32cc1f4b384 100644
@@ -15,3 +15,16 @@ int intel_dsi_bitrate(const struct intel_dsi *intel_dsi)
 
   	return intel_dsi->pclk * bpp / intel_dsi->lane_count;
   }
+
+int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi)
+{
+	switch (intel_dsi->escape_clk_div) {
+	default:
+	case 0:
+		return 50;

Quoted 8 lines

drivers/gpu/drm/i915/intel_dsi.c int intel_dsi_bitrate(const struct intel_dsi *intel_dsi) · line 30

Quoted 6 lines

drivers/gpu/drm/i915/intel_dsi.c +6 -0

+	case 1:
+		return 100;
+	case 2:
+		return 200;
+	}
+}

Can we change the return of this function to unsigned int, there is no way that this function can return < 0 as per current implementation??

I'd rather not. I'm sure I should be able to justify this exhaustively with examples, but I think gratuitous use of unsigned types leads to issues with integer promotions and signed/unsigned comparisons etc. So if it's "just a number", used for arithmetics, I'll pretty much choose int every time.

And because you can silently assign negative numbers to unsigned types, it's not a good safeguard to ensure positive numbers. Unless you're happy with large positive numbers...

Unsigned types are best for register contents, bits, masks, etc.

BR,
Jani.

Quoted 40 lines

Regards,
Madhav

Quoted 37 lines

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

index 68f14d8f1e18..0d911a4adfaa 100644
@@ -131,6 +131,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 
   /* intel_dsi.c */
   int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
+int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi);
 
   /* vlv_dsi.c */
   void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);

drivers/gpu/drm/i915/intel_dsi_vbt.c +1 -15

index fdeba8386d53..b0d8548f0462 100644
@@ -512,21 +512,7 @@ static void vlv_dphy_param_init(struct intel_dsi *intel_dsi)
   	u32 lp_to_hs_switch, hs_to_lp_switch;
   	u32 mul;
 
-	switch (intel_dsi->escape_clk_div) {
-	case 0:
-		tlpx_ns = 50;
-		break;
-	case 1:
-		tlpx_ns = 100;
-		break;
-
-	case 2:
-		tlpx_ns = 200;
-		break;
-	default:
-		tlpx_ns = 50;
-		break;
-	}
+	tlpx_ns = intel_dsi_tlpx_ns(intel_dsi);
 

Quoted 2 lines

drivers/gpu/drm/i915/intel_dsi_vbt.c static void vlv_dphy_param_init(struct intel_dsi *intel_dsi) · line 518

Quoted 2 lines

drivers/gpu/drm/i915/intel_dsi_vbt.c +0 -0

   	switch (intel_dsi->lane_count) {
   	case 1:

From: Madhav Chauhan <[email protected]>

This patch moves couple of legacy DSI functions to header and common DSI files so that they can be re-used by Gen11 DSI. No functional change.

v2 by Jani:
- Move intel_dsi_msleep() to intel_dsi_vbt.c

Signed-off-by Madhav Chauhan <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/intel_dsi.h     | 11 +++++++++++
  drivers/gpu/drm/i915/intel_dsi_vbt.c | 11 +++++++++++
  drivers/gpu/drm/i915/vlv_dsi.c       | 21 ---------------------
  3 files changed, 22 insertions(+), 21 deletions(-)

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

index 0d911a4adfaa..d7c0c599b52d 100644
@@ -129,6 +129,16 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
  	return container_of(encoder, struct intel_dsi, base.base);
  }
 
+static inline bool is_vid_mode(struct intel_dsi *intel_dsi)
+{
+	return intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE;
+}
+
+static inline bool is_cmd_mode(struct intel_dsi *intel_dsi)
+{
+	return intel_dsi->operation_mode == INTEL_DSI_COMMAND_MODE;
+}
+
  /* intel_dsi.c */
  int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
  int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi);

@@ -162,5 +172,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
  int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi);
  void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
  				 enum mipi_seq seq_id);
+void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec);
 
  #endif /* _INTEL_DSI_H */

drivers/gpu/drm/i915/intel_dsi_vbt.c +11 -0

index b0d8548f0462..5e16b4c5f531 100644
@@ -481,6 +481,17 @@ void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
  	}
  }
 
+void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec)
+{
+	struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
+
+	/* For v3 VBTs in vid-mode the delays are part of the VBT sequences */
+	if (is_vid_mode(intel_dsi) && dev_priv->vbt.dsi.seq_version >= 3)
+		return;
+
+	msleep(msec);
+}
+
  int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi)
  {
  	struct intel_connector *connector = intel_dsi->attached_connector;

drivers/gpu/drm/i915/vlv_dsi.c +0 -14

index dbca30460a6b..ee0cd5d0bf91 100644
@@ -290,16 +290,6 @@ static void band_gap_reset(struct drm_i915_private *dev_priv)
  	mutex_unlock(&dev_priv->sb_lock);
  }
 
-static inline bool is_vid_mode(struct intel_dsi *intel_dsi)
-{
-	return intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE;
-}
-
-static inline bool is_cmd_mode(struct intel_dsi *intel_dsi)
-{
-	return intel_dsi->operation_mode == INTEL_DSI_COMMAND_MODE;
-}
-
  static bool intel_dsi_compute_config(struct intel_encoder *encoder,
  				     struct intel_crtc_state *pipe_config,
  				     struct drm_connector_state *conn_state)

@@ -746,17 +736,6 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder,
  			      const struct intel_crtc_state *pipe_config);
  static void intel_dsi_unprepare(struct intel_encoder *encoder);
 
-static void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec)
-{
-	struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
-

Quoted 5 lines · Jani Nikula

From: Madhav Chauhan <[email protected]>

This patch moves couple of legacy DSI functions to header and common DSI files so that they can be re-used by Gen11 DSI. No functional change.

Quoted 2 lines

v2 by Jani:
- Move intel_dsi_msleep() to intel_dsi_vbt.c

This will be used by icl dsi as well and and delay is directly passed Shouldn't we have this inside intel_dsi.c??
Or
Because seq version from VBT is getting checked inside this function, so that is taking the precedence??

Regards,
Madhav

Quoted 97 lines

Signed-off-by Madhav Chauhan <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/intel_dsi.h     | 11 +++++++++++
  drivers/gpu/drm/i915/intel_dsi_vbt.c | 11 +++++++++++
  drivers/gpu/drm/i915/vlv_dsi.c       | 21 ---------------------
  3 files changed, 22 insertions(+), 21 deletions(-)

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

index 0d911a4adfaa..d7c0c599b52d 100644
@@ -129,6 +129,16 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
  	return container_of(encoder, struct intel_dsi, base.base);
  }
 
+static inline bool is_vid_mode(struct intel_dsi *intel_dsi)
+{
+	return intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE;
+}
+
+static inline bool is_cmd_mode(struct intel_dsi *intel_dsi)
+{
+	return intel_dsi->operation_mode == INTEL_DSI_COMMAND_MODE;
+}
+
  /* intel_dsi.c */
  int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
  int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi);

@@ -162,5 +172,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
  int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi);
  void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
  				 enum mipi_seq seq_id);
+void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec);
 
  #endif /* _INTEL_DSI_H */

drivers/gpu/drm/i915/intel_dsi_vbt.c +11 -0

index b0d8548f0462..5e16b4c5f531 100644
@@ -481,6 +481,17 @@ void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
  	}
  }
 
+void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec)
+{
+	struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
+
+	/* For v3 VBTs in vid-mode the delays are part of the VBT sequences */
+	if (is_vid_mode(intel_dsi) && dev_priv->vbt.dsi.seq_version >= 3)
+		return;
+
+	msleep(msec);
+}
+
  int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi)
  {
  	struct intel_connector *connector = intel_dsi->attached_connector;

drivers/gpu/drm/i915/vlv_dsi.c +0 -21

index dbca30460a6b..ee0cd5d0bf91 100644
@@ -290,16 +290,6 @@ static void band_gap_reset(struct drm_i915_private *dev_priv)
  	mutex_unlock(&dev_priv->sb_lock);
  }
 
-static inline bool is_vid_mode(struct intel_dsi *intel_dsi)
-{
-	return intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE;
-}
-
-static inline bool is_cmd_mode(struct intel_dsi *intel_dsi)
-{
-	return intel_dsi->operation_mode == INTEL_DSI_COMMAND_MODE;
-}
-
  static bool intel_dsi_compute_config(struct intel_encoder *encoder,
  				     struct intel_crtc_state *pipe_config,
  				     struct drm_connector_state *conn_state)

@@ -746,17 +736,6 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder,
  			      const struct intel_crtc_state *pipe_config);
  static void intel_dsi_unprepare(struct intel_encoder *encoder);
 
-static void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec)
-{
-	struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
-
-	/* For v3 VBTs in vid-mode the delays are part of the VBT sequences */
-	if (is_vid_mode(intel_dsi) && dev_priv->vbt.dsi.seq_version >= 3)
-		return;
-
-	msleep(msec);
-}
-
  /*
   * Panel enable/disable sequences from the VBT spec.
   *

Quoted 8 lines · Madhav Chauhan

Quoted 7 lines · Jani Nikula

From: Madhav Chauhan <[email protected]>

This patch moves couple of legacy DSI functions to header and common DSI files so that they can be re-used by Gen11 DSI. No functional change.

v2 by Jani:
- Move intel_dsi_msleep() to intel_dsi_vbt.c

Quoted 5 lines

This will be used by icl dsi as well and and delay is directly passed Shouldn't we have this inside intel_dsi.c??
Or
Because seq version from VBT is getting checked inside this function, so that is taking the precedence??

I had it in intel_dsi.c at first, but decided on intel_dsi_vbt.c instead because it does have a dependency on the VBT. And I presume on ICL this will be a NOP due to v3+ sequence.

BR,
Jani.

Quoted 86 lines

Regards,
Madhav

Quoted 83 lines

Signed-off-by Madhav Chauhan <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
   drivers/gpu/drm/i915/intel_dsi.h     | 11 +++++++++++
   drivers/gpu/drm/i915/intel_dsi_vbt.c | 11 +++++++++++
   drivers/gpu/drm/i915/vlv_dsi.c       | 21 ---------------------
   3 files changed, 22 insertions(+), 21 deletions(-)

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

index 0d911a4adfaa..d7c0c599b52d 100644
@@ -129,6 +129,16 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
   	return container_of(encoder, struct intel_dsi, base.base);
   }
 
+static inline bool is_vid_mode(struct intel_dsi *intel_dsi)
+{
+	return intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE;
+}
+
+static inline bool is_cmd_mode(struct intel_dsi *intel_dsi)
+{
+	return intel_dsi->operation_mode == INTEL_DSI_COMMAND_MODE;
+}
+
   /* intel_dsi.c */
   int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
   int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi);

@@ -162,5 +172,6 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
   int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi);
   void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
   				 enum mipi_seq seq_id);
+void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec);
 
   #endif /* _INTEL_DSI_H */

drivers/gpu/drm/i915/intel_dsi_vbt.c +11 -0

index b0d8548f0462..5e16b4c5f531 100644
@@ -481,6 +481,17 @@ void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
   	}
   }
 
+void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec)
+{
+	struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
+
+	/* For v3 VBTs in vid-mode the delays are part of the VBT sequences */
+	if (is_vid_mode(intel_dsi) && dev_priv->vbt.dsi.seq_version >= 3)
+		return;
+
+	msleep(msec);
+}
+
   int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi)
   {
   	struct intel_connector *connector = intel_dsi->attached_connector;

drivers/gpu/drm/i915/vlv_dsi.c +0 -10

index dbca30460a6b..ee0cd5d0bf91 100644
@@ -290,16 +290,6 @@ static void band_gap_reset(struct drm_i915_private *dev_priv)
   	mutex_unlock(&dev_priv->sb_lock);
   }
 
-static inline bool is_vid_mode(struct intel_dsi *intel_dsi)
-{
-	return intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE;
-}
-
-static inline bool is_cmd_mode(struct intel_dsi *intel_dsi)
-{
-	return intel_dsi->operation_mode == INTEL_DSI_COMMAND_MODE;
-}
-
   static bool intel_dsi_compute_config(struct intel_encoder *encoder,
   				     struct intel_crtc_state *pipe_config,
   				     struct drm_connector_state *conn_state)

@@ -746,17 +736,6 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder,
   			      const struct intel_crtc_state *pipe_config);
   static void intel_dsi_unprepare(struct intel_encoder *encoder);
 

Quoted 6 lines

Quoted 6 lines

drivers/gpu/drm/i915/vlv_dsi.c +0 -6

-static void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec)
-{
-	struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
-
-	/* For v3 VBTs in vid-mode the delays are part of the VBT sequences */
-	if (is_vid_mode(intel_dsi) && dev_priv->vbt.dsi.seq_version >= 3)

Quoted 8 lines

drivers/gpu/drm/i915/vlv_dsi.c static void intel_dsi_prepare(struct intel_encoder *intel_encoder, · line 741

Quoted 8 lines

drivers/gpu/drm/i915/vlv_dsi.c +0 -5

-		return;
-
-	msleep(msec);
-}
-
   /*
    * Panel enable/disable sequences from the VBT spec.
    *

From: Madhav Chauhan <[email protected]>

This patch programs D-PHY timing parameters for the clock and data lane (in escape clocks) of DSI controller (DSI port 0 and 1).
These programmed timings would be used by DSI Controller to calculate link transition latencies of the data and clock lanes.

v2: Use newly defined bitfields for data and clock lane

v3 by Jani:
- Rebase on dphy abstraction
- Reduce local variables
- Remove unrelated comment changes (Ville)
- Use the same style for range checks as VLV (Ville)
- Assign, don't OR dphy_reg contents

Signed-off-by Madhav Chauhan <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/icl_dsi.c       |  18 ++++++
  drivers/gpu/drm/i915/intel_dsi.h     |   3 +
  drivers/gpu/drm/i915/intel_dsi_vbt.c | 110 ++++++++++++++++++++++++++++++++++-
  3 files changed, 130 insertions(+), 1 deletion(-)

drivers/gpu/drm/i915/icl_dsi.c +18 -0

index ff5b285ca495..9602b6532028 100644
@@ -291,6 +291,24 @@ static void gen11_dsi_setup_dphy_timings(struct intel_encoder *encoder)
  		tmp |= intel_dsi->init_count;
  		I915_WRITE(ICL_DSI_T_INIT_MASTER(port), tmp);
  	}
+
+	/* Program DPHY clock lanes timings */
+	for_each_dsi_port(port, intel_dsi->ports) {
+		I915_WRITE(DPHY_CLK_TIMING_PARAM(port), intel_dsi->dphy_reg);
+
+		/* shadow register inside display core */
+		I915_WRITE(DSI_CLK_TIMING_PARAM(port), intel_dsi->dphy_reg);
+	}
+
+	/* Program DPHY data lanes timings */
+	for_each_dsi_port(port, intel_dsi->ports) {
+		I915_WRITE(DPHY_DATA_TIMING_PARAM(port),
+			   intel_dsi->dphy_data_lane_reg);
+
+		/* shadow register inside display core */
+		I915_WRITE(DSI_DATA_TIMING_PARAM(port),
+			   intel_dsi->dphy_data_lane_reg);
+	}
  }
 
  static void gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder)

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

index d7c0c599b52d..12b758ebefce 100644
@@ -85,6 +85,9 @@ struct intel_dsi {
  	u32 port_bits;
  	u32 bw_timer;
  	u32 dphy_reg;
+
+	/* data lanes dphy timing */
+	u32 dphy_data_lane_reg;
  	u32 video_frmt_cfg_bits;
  	u16 lp_byte_clk;
 

drivers/gpu/drm/i915/intel_dsi_vbt.c +105 -0

index 5e16b4c5f531..3035422aa0d6 100644
@@ -510,6 +510,111 @@ int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi)
  	return 1;
  }
 
+#define ICL_PREPARE_CNT_MAX	0x7
+#define ICL_CLK_ZERO_CNT_MAX	0xf
+#define ICL_TRAIL_CNT_MAX	0x7
+#define ICL_TCLK_PRE_CNT_MAX	0x3
+#define ICL_TCLK_POST_CNT_MAX	0x7
+#define ICL_HS_ZERO_CNT_MAX	0xf
+#define ICL_EXIT_ZERO_CNT_MAX	0x7
+
+static void icl_dphy_param_init(struct intel_dsi *intel_dsi)
+{
+	struct drm_device *dev = intel_dsi->base.base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
+	u32 tlpx_ns;
+	u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt, trail_cnt;
+	u32 ths_prepare_ns, tclk_trail_ns;
+	u32 hs_zero_cnt;
+	u32 tclk_pre_cnt, tclk_post_cnt;
+
+	tlpx_ns = intel_dsi_tlpx_ns(intel_dsi);
+
+	tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail);
+	ths_prepare_ns = max(mipi_config->ths_prepare,
+			     mipi_config->tclk_prepare);
+
+	/*
+	 * prepare cnt in escape clocks
+	 * this field represents a hexadecimal value with a precision
+	 * of 1.2 – i.e. the most significant bit is the integer
+	 * and the least significant 2 bits are fraction bits.
+	 * so, the field can represent a range of 0.25 to 1.75
+	 */
+	prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * 4, tlpx_ns);
+	if (prepare_cnt > ICL_PREPARE_CNT_MAX) {
+		DRM_DEBUG_KMS("prepare_cnt out of range (%d)\n", prepare_cnt);
+		prepare_cnt = ICL_PREPARE_CNT_MAX;
+	}
+
+	/* clk zero count in escape clocks */
+	clk_zero_cnt = DIV_ROUND_UP(mipi_config->tclk_prepare_clkzero -
+				    ths_prepare_ns, tlpx_ns);
+	if (clk_zero_cnt > ICL_CLK_ZERO_CNT_MAX) {
+		DRM_DEBUG_KMS("clk_zero_cnt out of range (%d)\n", clk_zero_cnt);
+		clk_zero_cnt = ICL_CLK_ZERO_CNT_MAX;
+	}
+
+	/* trail cnt in escape clocks*/
+	trail_cnt = DIV_ROUND_UP(tclk_trail_ns, tlpx_ns);
+	if (trail_cnt > ICL_TRAIL_CNT_MAX) {
+		DRM_DEBUG_KMS("trail_cnt out of range (%d)\n", trail_cnt);
+		trail_cnt = ICL_TRAIL_CNT_MAX;
+	}
+
+	/* tclk pre count in escape clocks */
+	tclk_pre_cnt = DIV_ROUND_UP(mipi_config->tclk_pre, tlpx_ns);
+	if (tclk_pre_cnt > ICL_TCLK_PRE_CNT_MAX) {
+		DRM_DEBUG_KMS("tclk_pre_cnt out of range (%d)\n", tclk_pre_cnt);
+		tclk_pre_cnt = ICL_TCLK_PRE_CNT_MAX;
+	}
+
+	/* tclk post count in escape clocks */
+	tclk_post_cnt = DIV_ROUND_UP(mipi_config->tclk_post, tlpx_ns);
+	if (tclk_post_cnt > ICL_TCLK_POST_CNT_MAX) {
+		DRM_DEBUG_KMS("tclk_post_cnt out of range (%d)\n", tclk_post_cnt);
+		tclk_post_cnt = ICL_TCLK_POST_CNT_MAX;
+	}
+
+	/* hs zero cnt in escape clocks */
+	hs_zero_cnt = DIV_ROUND_UP(mipi_config->ths_prepare_hszero -
+				   ths_prepare_ns, tlpx_ns);
+	if (hs_zero_cnt > ICL_HS_ZERO_CNT_MAX) {
+		DRM_DEBUG_KMS("hs_zero_cnt out of range (%d)\n", hs_zero_cnt);
+		hs_zero_cnt = ICL_HS_ZERO_CNT_MAX;
+	}
+
+	/* hs exit zero cnt in escape clocks */
+	exit_zero_cnt = DIV_ROUND_UP(mipi_config->ths_exit, tlpx_ns);
+	if (exit_zero_cnt > ICL_EXIT_ZERO_CNT_MAX) {
+		DRM_DEBUG_KMS("exit_zero_cnt out of range (%d)\n", exit_zero_cnt);
+		exit_zero_cnt = ICL_EXIT_ZERO_CNT_MAX;
+	}
+
+	/* clock lane dphy timings */
+	intel_dsi->dphy_reg = (CLK_PREPARE_OVERRIDE |
+			       CLK_PREPARE(prepare_cnt) |
+			       CLK_ZERO_OVERRIDE |
+			       CLK_ZERO(clk_zero_cnt) |
+			       CLK_PRE_OVERRIDE |
+			       CLK_PRE(tclk_pre_cnt) |
+			       CLK_POST_OVERRIDE |
+			       CLK_POST(tclk_post_cnt) |
+			       CLK_TRAIL_OVERRIDE |
+			       CLK_TRAIL(trail_cnt));
+
+	/* data lanes dphy timings */
+	intel_dsi->dphy_data_lane_reg = (HS_PREPARE_OVERRIDE |
+					 HS_PREPARE(prepare_cnt) |
+					 HS_ZERO_OVERRIDE |
+					 HS_ZERO(hs_zero_cnt) |
+					 HS_TRAIL_OVERRIDE |
+					 HS_TRAIL(trail_cnt) |
+					 HS_EXIT_OVERRIDE |
+					 HS_EXIT(exit_zero_cnt));
+}
+
  static void vlv_dphy_param_init(struct intel_dsi *intel_dsi)
  {
  	struct drm_device *dev = intel_dsi->base.base.dev;

@@ -743,7 +848,10 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 

From: Madhav Chauhan <[email protected]>

This patch programs D-PHY timing parameters for the bus turn around flow(in escape clocks) only if dsi link frequency <=800 MHz using DPHY_TA_TIMING_PARAM and its identical register DSI_TA_TIMING_PARAM (inside DSI Controller within the Display Core).

v2: Changes
- Don't use KHz() macro (Ville/Jani N)
- Use newly defined bitfields

v3 by Jani:
- Use intel_dsi_bitrate() in favor of a new field
- Remove redundant parens

Signed-off-by Madhav Chauhan <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/icl_dsi.c | 21 +++++++++++++++++++++
  1 file changed, 21 insertions(+)

drivers/gpu/drm/i915/icl_dsi.c +14 -0

index 9602b6532028..f9df3a7fa66b 100644
@@ -309,6 +309,27 @@ static void gen11_dsi_setup_dphy_timings(struct intel_encoder *encoder)
  		I915_WRITE(DSI_DATA_TIMING_PARAM(port),
  			   intel_dsi->dphy_data_lane_reg);
  	}
+
+	/*
+	 * If DSI link operating at or below an 800 MHz,
+	 * TA_SURE should be override and programmed to
+	 * a value '0' inside TA_PARAM_REGISTERS otherwise
+	 * leave all fields at HW default values.
+	 */
+	if (intel_dsi_bitrate(intel_dsi) <= 800000) {
+		for_each_dsi_port(port, intel_dsi->ports) {
+			tmp = I915_READ(DPHY_TA_TIMING_PARAM(port));
+			tmp &= ~TA_SURE_MASK;
+			tmp |= TA_SURE_OVERRIDE | TA_SURE(0);
+			I915_WRITE(DPHY_TA_TIMING_PARAM(port), tmp);
+

From: Madhav Chauhan <[email protected]>

This patch defines transcoder function configuration registers and its bitfields for both DSI ports. Used while programming/enabling DSI transcoder.

v2: Changes (Jani N)
- Define _SHIFT and _MASK for bitfields
- Define values for fields already shifted in place

v3 by Jani:
- Fix _SHIFT fields copy-pasted from _MASK
- Indentation fixes
- Reduce S3D orientation to single macro
- Wrap a macro parameter in parens

Signed-off-by Madhav Chauhan <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/i915_reg.h | 45 +++++++++++++++++++++++++++++++++++++++++
  1 file changed, 45 insertions(+)

drivers/gpu/drm/i915/i915_reg.h +38 -0

index 436ff68b6b18..b065e4ca0b45 100644
@@ -10379,6 +10379,51 @@ enum skl_power_gate {
  #define  TA_GET_MASK			(0xf << 0)
  #define  TA_GET_SHIFT			0
 
+/* DSI transcoder configuration */
+#define _DSI_TRANS_FUNC_CONF_0		0x6b030
+#define _DSI_TRANS_FUNC_CONF_1		0x6b830
+#define DSI_TRANS_FUNC_CONF(tc)		_MMIO_DSI(tc,	\
+						  _DSI_TRANS_FUNC_CONF_0,\
+						  _DSI_TRANS_FUNC_CONF_1)
+#define  OP_MODE_MASK			(0x3 << 28)
+#define  OP_MODE_SHIFT			28
+#define  CMD_MODE_NO_GATE		(0x0 << 28)
+#define  CMD_MODE_TE_GATE		(0x1 << 28)
+#define  VIDEO_MODE_SYNC_EVENT		(0x2 << 28)
+#define  VIDEO_MODE_SYNC_PULSE		(0x3 << 28)
+#define  LINK_READY			(1 << 20)
+#define  PIX_FMT_MASK			(0x3 << 16)
+#define  PIX_FMT_SHIFT			16
+#define  PIX_FMT_RGB565			(0x0 << 16)
+#define  PIX_FMT_RGB666_PACKED		(0x1 << 16)
+#define  PIX_FMT_RGB666_LOOSE		(0x2 << 16)
+#define  PIX_FMT_RGB888			(0x3 << 16)
+#define  PIX_FMT_RGB101010		(0x4 << 16)
+#define  PIX_FMT_RGB121212		(0x5 << 16)
+#define  PIX_FMT_COMPRESSED		(0x6 << 16)
+#define  BGR_TRANSMISSION		(1 << 15)
+#define  PIX_VIRT_CHAN(x)		((x) << 12)
+#define  PIX_VIRT_CHAN_MASK		(0x3 << 12)
+#define  PIX_VIRT_CHAN_SHIFT		12
+#define  PIX_BUF_THRESHOLD_MASK		(0x3 << 10)
+#define  PIX_BUF_THRESHOLD_SHIFT	10
+#define  PIX_BUF_THRESHOLD_1_4		(0x0 << 10)
+#define  PIX_BUF_THRESHOLD_1_2		(0x1 << 10)
+#define  PIX_BUF_THRESHOLD_3_4		(0x2 << 10)
+#define  PIX_BUF_THRESHOLD_FULL		(0x3 << 10)
+#define  CONTINUOUS_CLK_MASK		(0x3 << 8)
+#define  CONTINUOUS_CLK_SHIFT		8
+#define  CLK_ENTER_LP_AFTER_DATA	(0x0 << 8)
+#define  CLK_HS_OR_LP			(0x2 << 8)
+#define  CLK_HS_CONTINUOUS		(0x3 << 8)
+#define  LINK_CALIBRATION_MASK		(0x3 << 4)

From: Madhav Chauhan <[email protected]>

This patch programs DSI operation mode, pixel format, BGR info, link calibration etc for the DSI transcoder. This patch also extract BGR info of the DSI panel from VBT and save it inside struct intel_dsi which used for configuring DSI transcoder.

v2: Rebase
v3: Use newly defined bitfields.

v4 by Jani:
- Use intel_dsi_bitrate()
- Make bgr_enabled bool
- Use 0 instead of 0x0
- Replace DRM_ERROR() with MISSING_CASE() on pixel format and video mode
- Use is_vid_mode()

Signed-off-by Madhav Chauhan <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/icl_dsi.c       | 87 +++++++++++++++++++++++++++++++++++-
  drivers/gpu/drm/i915/intel_dsi.h     |  3 ++
  drivers/gpu/drm/i915/intel_dsi_vbt.c |  1 +
  3 files changed, 90 insertions(+), 1 deletion(-)

drivers/gpu/drm/i915/icl_dsi.c +86 -1

index 407c3065d08d..756c75d0c86c 100644
@@ -27,7 +27,7 @@
 
  #include "intel_dsi.h"
 
-static enum transcoder __attribute__((unused)) dsi_port_to_transcoder(enum port port)
+static enum transcoder dsi_port_to_transcoder(enum port port)
  {
  	if (port == PORT_A)
  		return TRANSCODER_DSI_0;

@@ -340,6 +340,88 @@ static void gen11_dsi_setup_dphy_timings(struct intel_encoder *encoder)
  	}
  }
 
+static void gen11_dsi_configure_transcoder(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	u32 tmp;
+	enum port port;
+	enum transcoder dsi_trans;
+
+	for_each_dsi_port(port, intel_dsi->ports) {
+		dsi_trans = dsi_port_to_transcoder(port);
+		tmp = I915_READ(DSI_TRANS_FUNC_CONF(dsi_trans));
+
+		if (intel_dsi->eotp_pkt)
+			tmp &= ~EOTP_DISABLED;
+		else
+			tmp |= EOTP_DISABLED;
+
+		/* enable link calibration if freq > 1.5Gbps */
+		if (intel_dsi_bitrate(intel_dsi) >= 1500 * 1000) {
+			tmp &= ~LINK_CALIBRATION_MASK;
+			tmp |= CALIBRATION_ENABLED_INITIAL_ONLY;
+		}
+
+		/* configure continuous clock */
+		tmp &= ~CONTINUOUS_CLK_MASK;
+		if (intel_dsi->clock_stop)
+			tmp |= CLK_ENTER_LP_AFTER_DATA;
+		else
+			tmp |= CLK_HS_CONTINUOUS;
+
+		/* configure buffer threshold limit to minimum */
+		tmp &= ~PIX_BUF_THRESHOLD_MASK;
+		tmp |= PIX_BUF_THRESHOLD_1_4;
+
+		/* set virtual channel to '0' */
+		tmp &= ~PIX_VIRT_CHAN_MASK;
+		tmp |= PIX_VIRT_CHAN(0);
+
+		/* program BGR transmission */
+		if (intel_dsi->bgr_enabled)
+			tmp |= BGR_TRANSMISSION;
+
+		/* select pixel format */
+		tmp &= ~PIX_FMT_MASK;
+		switch (intel_dsi->pixel_format) {
+		default:
+			MISSING_CASE(intel_dsi->pixel_format);
+			/* fallthrough */
+		case MIPI_DSI_FMT_RGB565:
+			tmp |= PIX_FMT_RGB565;
+			break;
+		case MIPI_DSI_FMT_RGB666_PACKED:
+			tmp |= PIX_FMT_RGB666_PACKED;
+			break;
+		case MIPI_DSI_FMT_RGB666:
+			tmp |= PIX_FMT_RGB666_LOOSE;
+			break;
+		case MIPI_DSI_FMT_RGB888:
+			tmp |= PIX_FMT_RGB888;
+			break;
+		}
+
+		/* program DSI operation mode */
+		if (is_vid_mode(intel_dsi)) {
+			tmp &= ~OP_MODE_MASK;
+			switch (intel_dsi->video_mode_format) {
+			default:
+				MISSING_CASE(intel_dsi->video_mode_format);
+				/* fallthrough */
+			case VIDEO_MODE_NON_BURST_WITH_SYNC_EVENTS:
+				tmp |= VIDEO_MODE_SYNC_EVENT;
+				break;
+			case VIDEO_MODE_NON_BURST_WITH_SYNC_PULSE:
+				tmp |= VIDEO_MODE_SYNC_PULSE;
+				break;
+			}
+		}
+
+		I915_WRITE(DSI_TRANS_FUNC_CONF(dsi_trans), tmp);
+	}
+}
+
  static void gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder)
  {
  	/* step 4a: power up all lanes of the DDI used by DSI */

@@ -356,6 +438,9 @@ static void gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder)
 
  	/* setup D-PHY timings */
  	gen11_dsi_setup_dphy_timings(encoder);
+
+	/* Step (4h, 4i, 4j, 4k): Configure transcoder */
+	gen11_dsi_configure_transcoder(encoder);
  }
 
  static void __attribute__((unused))

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

index 12b758ebefce..14567929de9a 100644
@@ -81,6 +81,9 @@ struct intel_dsi {
  	u16 dcs_backlight_ports;
  	u16 dcs_cabc_ports;
 
+	/* RGB or BGR */
+	bool bgr_enabled;
+
  	u8 pixel_overlap;
  	u32 port_bits;
  	u32 bw_timer;

drivers/gpu/drm/i915/intel_dsi_vbt.c +0 -0

index 3035422aa0d6..cca071406c25 100644

Quoted 11 lines · Jani Nikula

From: Madhav Chauhan <[email protected]>

This patch programs DSI operation mode, pixel format, BGR info, link calibration etc for the DSI transcoder. This patch also extract BGR info of the DSI panel from VBT and save it inside struct intel_dsi which used for configuring DSI transcoder.

v2: Rebase
v3: Use newly defined bitfields.

Quoted 6 lines

v4 by Jani:
- Use intel_dsi_bitrate()
- Make bgr_enabled bool
- Use 0 instead of 0x0
- Replace DRM_ERROR() with MISSING_CASE() on pixel format and video mode
- Use is_vid_mode()

v4 changes are fine.

Regards,
Madhav

Quoted 147 lines

Signed-off-by Madhav Chauhan <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/icl_dsi.c       | 87 +++++++++++++++++++++++++++++++++++-
  drivers/gpu/drm/i915/intel_dsi.h     |  3 ++
  drivers/gpu/drm/i915/intel_dsi_vbt.c |  1 +
  3 files changed, 90 insertions(+), 1 deletion(-)

drivers/gpu/drm/i915/icl_dsi.c +86 -1

index 407c3065d08d..756c75d0c86c 100644
@@ -27,7 +27,7 @@
 
  #include "intel_dsi.h"
 
-static enum transcoder __attribute__((unused)) dsi_port_to_transcoder(enum port port)
+static enum transcoder dsi_port_to_transcoder(enum port port)
  {
  	if (port == PORT_A)
  		return TRANSCODER_DSI_0;

@@ -340,6 +340,88 @@ static void gen11_dsi_setup_dphy_timings(struct intel_encoder *encoder)
  	}
  }
 
+static void gen11_dsi_configure_transcoder(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	u32 tmp;
+	enum port port;
+	enum transcoder dsi_trans;
+
+	for_each_dsi_port(port, intel_dsi->ports) {
+		dsi_trans = dsi_port_to_transcoder(port);
+		tmp = I915_READ(DSI_TRANS_FUNC_CONF(dsi_trans));
+
+		if (intel_dsi->eotp_pkt)
+			tmp &= ~EOTP_DISABLED;
+		else
+			tmp |= EOTP_DISABLED;
+
+		/* enable link calibration if freq > 1.5Gbps */
+		if (intel_dsi_bitrate(intel_dsi) >= 1500 * 1000) {
+			tmp &= ~LINK_CALIBRATION_MASK;
+			tmp |= CALIBRATION_ENABLED_INITIAL_ONLY;
+		}
+
+		/* configure continuous clock */
+		tmp &= ~CONTINUOUS_CLK_MASK;
+		if (intel_dsi->clock_stop)
+			tmp |= CLK_ENTER_LP_AFTER_DATA;
+		else
+			tmp |= CLK_HS_CONTINUOUS;
+
+		/* configure buffer threshold limit to minimum */
+		tmp &= ~PIX_BUF_THRESHOLD_MASK;
+		tmp |= PIX_BUF_THRESHOLD_1_4;
+
+		/* set virtual channel to '0' */
+		tmp &= ~PIX_VIRT_CHAN_MASK;
+		tmp |= PIX_VIRT_CHAN(0);
+
+		/* program BGR transmission */
+		if (intel_dsi->bgr_enabled)
+			tmp |= BGR_TRANSMISSION;
+
+		/* select pixel format */
+		tmp &= ~PIX_FMT_MASK;
+		switch (intel_dsi->pixel_format) {
+		default:
+			MISSING_CASE(intel_dsi->pixel_format);
+			/* fallthrough */
+		case MIPI_DSI_FMT_RGB565:
+			tmp |= PIX_FMT_RGB565;
+			break;
+		case MIPI_DSI_FMT_RGB666_PACKED:
+			tmp |= PIX_FMT_RGB666_PACKED;
+			break;
+		case MIPI_DSI_FMT_RGB666:
+			tmp |= PIX_FMT_RGB666_LOOSE;
+			break;
+		case MIPI_DSI_FMT_RGB888:
+			tmp |= PIX_FMT_RGB888;
+			break;
+		}
+
+		/* program DSI operation mode */
+		if (is_vid_mode(intel_dsi)) {
+			tmp &= ~OP_MODE_MASK;
+			switch (intel_dsi->video_mode_format) {
+			default:
+				MISSING_CASE(intel_dsi->video_mode_format);
+				/* fallthrough */
+			case VIDEO_MODE_NON_BURST_WITH_SYNC_EVENTS:
+				tmp |= VIDEO_MODE_SYNC_EVENT;
+				break;
+			case VIDEO_MODE_NON_BURST_WITH_SYNC_PULSE:
+				tmp |= VIDEO_MODE_SYNC_PULSE;
+				break;
+			}
+		}
+
+		I915_WRITE(DSI_TRANS_FUNC_CONF(dsi_trans), tmp);
+	}
+}
+
  static void gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder)
  {
  	/* step 4a: power up all lanes of the DDI used by DSI */

@@ -356,6 +438,9 @@ static void gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder)
 
  	/* setup D-PHY timings */
  	gen11_dsi_setup_dphy_timings(encoder);
+
+	/* Step (4h, 4i, 4j, 4k): Configure transcoder */
+	gen11_dsi_configure_transcoder(encoder);
  }
 
  static void __attribute__((unused))

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

index 12b758ebefce..14567929de9a 100644
@@ -81,6 +81,9 @@ struct intel_dsi {
  	u16 dcs_backlight_ports;
  	u16 dcs_cabc_ports;
 
+	/* RGB or BGR */
+	bool bgr_enabled;
+
  	u8 pixel_overlap;
  	u32 port_bits;
  	u32 bw_timer;

drivers/gpu/drm/i915/intel_dsi_vbt.c +1 -0

index 3035422aa0d6..cca071406c25 100644
@@ -805,6 +805,7 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
  	intel_dsi->bw_timer = mipi_config->dbi_bw_timer;
  	intel_dsi->video_frmt_cfg_bits =
  		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
+	intel_dsi->bgr_enabled = mipi_config->rgb_flip;
 
  	/* Starting point, adjusted depending on dual link and burst mode */
  	intel_dsi->pclk = mode->clock;

From: Madhav Chauhan <[email protected]>

This patch defines TRANS_DDI_FUNC_CTL and TRANS_DDI_FUNC_CTL2 registers and their bitfields for DSI. These registers are used for enabling port sync mode, input pipe select, data lane width configuration etc.

v2: Changes:
- Remove redundant extra line
- Correct some of bitfield definition

v3 by Jani:
- Move DSI transcoder offsets to GEN11_FEATURES

Signed-off-by Madhav Chauhan <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/i915/i915_pci.c |  3 +++
  drivers/gpu/drm/i915/i915_reg.h | 17 +++++++++++++++++
  2 files changed, 20 insertions(+)

drivers/gpu/drm/i915/i915_pci.c +3 -0

index 0a05cc7ace14..b86b735a8634 100644
@@ -595,6 +595,9 @@ static const struct intel_device_info intel_cannonlake_info = {
 
  #define GEN11_FEATURES \
  	GEN10_FEATURES, \
+	.trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
+			   TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET, \
+			   TRANSCODER_DSI0_OFFSET, TRANSCODER_DSI1_OFFSET}, \
  	GEN(11), \
  	.ddb_size = 2048, \
  	.has_logical_ring_elsq = 1

drivers/gpu/drm/i915/i915_reg.h +10 -0

index b065e4ca0b45..79e633c1e9ad 100644
@@ -4066,6 +4066,8 @@ enum {
  #define TRANSCODER_C_OFFSET 0x62000
  #define CHV_TRANSCODER_C_OFFSET 0x63000
  #define TRANSCODER_EDP_OFFSET 0x6f000
+#define TRANSCODER_DSI0_OFFSET	0x6b000
+#define TRANSCODER_DSI1_OFFSET	0x6b800
 
  #define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv->info.trans_offsets[(pipe)] - \
  	dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \

@@ -9021,6 +9023,8 @@ enum skl_power_gate {
  #define _TRANS_DDI_FUNC_CTL_B		0x61400
  #define _TRANS_DDI_FUNC_CTL_C		0x62400
  #define _TRANS_DDI_FUNC_CTL_EDP		0x6F400
+#define _TRANS_DDI_FUNC_CTL_DSI0	0x6b400
+#define _TRANS_DDI_FUNC_CTL_DSI1	0x6bc00
  #define TRANS_DDI_FUNC_CTL(tran) _MMIO_TRANS2(tran, _TRANS_DDI_FUNC_CTL_A)
 
  #define  TRANS_DDI_FUNC_ENABLE		(1 << 31)

@@ -9058,6 +9062,19 @@ enum skl_power_gate {
  					| TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
  					| TRANS_DDI_HDMI_SCRAMBLING)
 
+#define _TRANS_DDI_FUNC_CTL2_A		0x60404
+#define _TRANS_DDI_FUNC_CTL2_B		0x61404
+#define _TRANS_DDI_FUNC_CTL2_C		0x62404
+#define _TRANS_DDI_FUNC_CTL2_EDP	0x6f404
+#define _TRANS_DDI_FUNC_CTL2_DSI0	0x6b404
+#define _TRANS_DDI_FUNC_CTL2_DSI1	0x6bc04