[PATCH 1/2] drm/i915/dsi: lose the loose 666 format name in favor of packed

[PATCH 1/2] drm/i915/dsi: lose the loose 666 format name in favor of packed

[Jani Nikula]

The enum mipi_dsi_pixel_format defines MIPI_DSI_FMT_RGB666 for the
"loose" 24 bpp format and MIPI_DSI_FMT_RGB666_PACKED for the 18 bpp
format. We have this the other way round, defining a loose version for
24 bpp.

Follow suit with what's in enum mipi_dsi_pixel_format to avoid future
confusion. Rename

VID_MODE_FORMAT_RGB666 -> VID_MODE_FORMAT_RGB666_PACKED
VID_MODE_FORMAT_RGB666_LOOSE -> VID_MODE_FORMAT_RGB666

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 4 ++--
 drivers/gpu/drm/i915/intel_dsi_pll.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7dfc4007f3fa..85ceec611412 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7887,8 +7887,8 @@ enum skl_disp_power_wells {
 #define  VID_MODE_FORMAT_MASK				(0xf << 7)
 #define  VID_MODE_NOT_SUPPORTED				(0 << 7)
 #define  VID_MODE_FORMAT_RGB565				(1 << 7)
-#define  VID_MODE_FORMAT_RGB666				(2 << 7)
-#define  VID_MODE_FORMAT_RGB666_LOOSE			(3 << 7)
+#define  VID_MODE_FORMAT_RGB666_PACKED			(2 << 7)
+#define  VID_MODE_FORMAT_RGB666				(3 << 7)
 #define  VID_MODE_FORMAT_RGB888				(4 << 7)
 #define  CMD_MODE_CHANNEL_NUMBER_SHIFT			5
 #define  CMD_MODE_CHANNEL_NUMBER_MASK			(3 << 5)
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 2451c84949bd..9ef0f7806e4a 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -37,10 +37,10 @@ int dsi_pixel_format_bpp(int pixel_format)
 	switch (pixel_format) {
 	default:
 	case VID_MODE_FORMAT_RGB888:
-	case VID_MODE_FORMAT_RGB666_LOOSE:
+	case VID_MODE_FORMAT_RGB666:
 		bpp = 24;
 		break;
-	case VID_MODE_FORMAT_RGB666:
+	case VID_MODE_FORMAT_RGB666_PACKED:
 		bpp = 18;
 		break;
 	case VID_MODE_FORMAT_RGB565:
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915/dsi: start using enum mipi_dsi_pixel_format

[Jani Nikula]

A small step moving us closer to DRM MIPI DSI code. Use enum
mipi_dsi_pixel_format instead of our own. The first benefit is being
able to use common mipi_dsi_pixel_format_to_bpp().

There's a little back and forth conversion with the VBT -> enum ->
register, since we have just shoved the VBT value into the register
directly. Longer term, all the VBT parsing and deciphering should be
done in intel_bios.c, and abstracted there.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c           | 25 +++++++++++++++++-----
 drivers/gpu/drm/i915/intel_dsi.h           | 10 +++++----
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 33 ++++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_dsi_pll.c       | 30 +++++----------------------
 4 files changed, 56 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 01b8e9f4c272..ea78b0bf7e14 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -787,7 +787,7 @@ static void set_dsi_timings(struct drm_encoder *encoder,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 	enum port port;
-	unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format);
+	unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
 	unsigned int lane_count = intel_dsi->lane_count;
 
 	u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
@@ -849,6 +849,23 @@ static void set_dsi_timings(struct drm_encoder *encoder,
 	}
 }
 
+static u32 pixel_format_to_reg(enum mipi_dsi_pixel_format fmt)
+{
+	switch (fmt) {
+	case MIPI_DSI_FMT_RGB888:
+		return VID_MODE_FORMAT_RGB888;
+	case MIPI_DSI_FMT_RGB666:
+		return VID_MODE_FORMAT_RGB666;
+	case MIPI_DSI_FMT_RGB666_PACKED:
+		return VID_MODE_FORMAT_RGB666_PACKED;
+	case MIPI_DSI_FMT_RGB565:
+		return VID_MODE_FORMAT_RGB565;
+	default:
+		MISSING_CASE(fmt);
+		return VID_MODE_FORMAT_RGB666;
+	}
+}
+
 static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
 {
 	struct drm_encoder *encoder = &intel_encoder->base;
@@ -858,7 +875,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
 	enum port port;
-	unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format);
+	unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
 	u32 val, tmp;
 	u16 mode_hdisplay;
 
@@ -917,9 +934,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
 		val |= CMD_MODE_DATA_WIDTH_8_BIT; /* XXX */
 	} else {
 		val |= intel_dsi->channel << VID_MODE_CHANNEL_NUMBER_SHIFT;
-
-		/* XXX: cross-check bpp vs. pixel format? */
-		val |= intel_dsi->pixel_format;
+		val |= pixel_format_to_reg(intel_dsi->pixel_format);
 	}
 
 	tmp = 0;
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 92f39227b361..54f072cd78f1 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -34,8 +34,6 @@
 #define DSI_DUAL_LINK_FRONT_BACK	1
 #define DSI_DUAL_LINK_PIXEL_ALT		2
 
-int dsi_pixel_format_bpp(int pixel_format);
-
 struct intel_dsi_host;
 
 struct intel_dsi {
@@ -64,8 +62,12 @@ struct intel_dsi {
 	/* number of DSI lanes */
 	unsigned int lane_count;
 
-	/* video mode pixel format for MIPI_DSI_FUNC_PRG register */
-	u32 pixel_format;
+	/*
+	 * video mode pixel format
+	 *
+	 * XXX: consolidate on .format in struct mipi_dsi_device.
+	 */
+	enum mipi_dsi_pixel_format pixel_format;
 
 	/* video mode format for MIPI_VIDEO_MODE_FORMAT register */
 	u32 video_mode_format;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 7f145b4fec6a..8302a972d2d4 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -412,6 +412,25 @@ static const struct drm_panel_funcs vbt_panel_funcs = {
 	.get_modes = vbt_panel_get_modes,
 };
 
+/* XXX: This should be done when parsing the VBT in intel_bios.c */
+static enum mipi_dsi_pixel_format pixel_format_from_vbt(u32 fmt)
+{
+	/* It just so happens the VBT matches register contents. */
+	switch (fmt) {
+	case VID_MODE_FORMAT_RGB888:
+		return MIPI_DSI_FMT_RGB888;
+	case VID_MODE_FORMAT_RGB666:
+		return MIPI_DSI_FMT_RGB666;
+	case VID_MODE_FORMAT_RGB666_PACKED:
+		return MIPI_DSI_FMT_RGB666_PACKED;
+	case VID_MODE_FORMAT_RGB565:
+		return MIPI_DSI_FMT_RGB565;
+	default:
+		MISSING_CASE(fmt);
+		return MIPI_DSI_FMT_RGB666;
+	}
+}
+
 struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
 {
 	struct drm_device *dev = intel_dsi->base.base.dev;
@@ -420,7 +439,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
 	struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
 	struct vbt_panel *vbt_panel;
-	u32 bits_per_pixel = 24;
+	u32 bpp;
 	u32 tlpx_ns, extra_byte_count, bitrate, tlpx_ui;
 	u32 ui_num, ui_den;
 	u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt, trail_cnt;
@@ -436,12 +455,11 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	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 = mipi_config->videomode_color_format << 7;
+	intel_dsi->pixel_format = pixel_format_from_vbt(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;
-
-	bits_per_pixel = dsi_pixel_format_bpp(intel_dsi->pixel_format);
-
 	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;
@@ -475,8 +493,7 @@ struct drm_panel *vbt_panel_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 * bits_per_pixel) / intel_dsi->lane_count;
+			computed_ddr = (pclk * bpp) / intel_dsi->lane_count;
 
 			if (mipi_config->target_burst_mode_freq <
 								computed_ddr) {
@@ -499,7 +516,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	intel_dsi->burst_mode_ratio = burst_mode_ratio;
 	intel_dsi->pclk = pclk;
 
-	bitrate = (pclk * bits_per_pixel) / intel_dsi->lane_count;
+	bitrate = (pclk * bpp) / intel_dsi->lane_count;
 
 	switch (intel_dsi->escape_clk_div) {
 	case 0:
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 9ef0f7806e4a..e3e343c80221 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -30,27 +30,6 @@
 #include "i915_drv.h"
 #include "intel_dsi.h"
 
-int dsi_pixel_format_bpp(int pixel_format)
-{
-	int bpp;
-
-	switch (pixel_format) {
-	default:
-	case VID_MODE_FORMAT_RGB888:
-	case VID_MODE_FORMAT_RGB666:
-		bpp = 24;
-		break;
-	case VID_MODE_FORMAT_RGB666_PACKED:
-		bpp = 18;
-		break;
-	case VID_MODE_FORMAT_RGB565:
-		bpp = 16;
-		break;
-	}
-
-	return bpp;
-}
-
 struct dsi_mnp {
 	u32 dsi_pll_ctrl;
 	u32 dsi_pll_div;
@@ -64,10 +43,11 @@ static const u32 lfsr_converts[] = {
 };
 
 /* Get DSI clock from pixel clock */
-static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
+static u32 dsi_clk_from_pclk(u32 pclk, enum mipi_dsi_pixel_format fmt,
+			     int lane_count)
 {
 	u32 dsi_clk_khz;
-	u32 bpp = dsi_pixel_format_bpp(pixel_format);
+	u32 bpp = mipi_dsi_pixel_format_to_bpp(fmt);
 
 	/* DSI data rate = pixel clock * bits per pixel / lane count
 	   pixel clock is converted from KHz to Hz */
@@ -232,9 +212,9 @@ static void bxt_disable_dsi_pll(struct intel_encoder *encoder)
 		DRM_ERROR("Timeout waiting for PLL lock deassertion\n");
 }
 
-static void assert_bpp_mismatch(int pixel_format, int pipe_bpp)
+static void assert_bpp_mismatch(enum mipi_dsi_pixel_format fmt, int pipe_bpp)
 {
-	int bpp = dsi_pixel_format_bpp(pixel_format);
+	int bpp = mipi_dsi_pixel_format_to_bpp(fmt);
 
 	WARN(bpp != pipe_bpp,
 	     "bpp match assertion failure (expected %d, current %d)\n",
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/dsi: start using enum mipi_dsi_pixel_format

[Ville Syrjälä]

On Wed, Mar 16, 2016 at 12:21:40PM +0200, Jani Nikula wrote:
> A small step moving us closer to DRM MIPI DSI code. Use enum
> mipi_dsi_pixel_format instead of our own. The first benefit is being
> able to use common mipi_dsi_pixel_format_to_bpp().
> 
> There's a little back and forth conversion with the VBT -> enum ->
> register, since we have just shoved the VBT value into the register
> directly. Longer term, all the VBT parsing and deciphering should be
> done in intel_bios.c, and abstracted there.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dsi.c           | 25 +++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_dsi.h           | 10 +++++----
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 33 ++++++++++++++++++++++--------
>  drivers/gpu/drm/i915/intel_dsi_pll.c       | 30 +++++----------------------
>  4 files changed, 56 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 01b8e9f4c272..ea78b0bf7e14 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -787,7 +787,7 @@ static void set_dsi_timings(struct drm_encoder *encoder,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	enum port port;
> -	unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format);
> +	unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
>  	unsigned int lane_count = intel_dsi->lane_count;
>  
>  	u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
> @@ -849,6 +849,23 @@ static void set_dsi_timings(struct drm_encoder *encoder,
>  	}
>  }
>  
> +static u32 pixel_format_to_reg(enum mipi_dsi_pixel_format fmt)
> +{
> +	switch (fmt) {
> +	case MIPI_DSI_FMT_RGB888:
> +		return VID_MODE_FORMAT_RGB888;
> +	case MIPI_DSI_FMT_RGB666:
> +		return VID_MODE_FORMAT_RGB666;
> +	case MIPI_DSI_FMT_RGB666_PACKED:
> +		return VID_MODE_FORMAT_RGB666_PACKED;
> +	case MIPI_DSI_FMT_RGB565:
> +		return VID_MODE_FORMAT_RGB565;
> +	default:
> +		MISSING_CASE(fmt);
> +		return VID_MODE_FORMAT_RGB666;
> +	}
> +}
> +
>  static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
>  {
>  	struct drm_encoder *encoder = &intel_encoder->base;
> @@ -858,7 +875,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
>  	enum port port;
> -	unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format);
> +	unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
>  	u32 val, tmp;
>  	u16 mode_hdisplay;
>  
> @@ -917,9 +934,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
>  		val |= CMD_MODE_DATA_WIDTH_8_BIT; /* XXX */
>  	} else {
>  		val |= intel_dsi->channel << VID_MODE_CHANNEL_NUMBER_SHIFT;
> -
> -		/* XXX: cross-check bpp vs. pixel format? */
> -		val |= intel_dsi->pixel_format;
> +		val |= pixel_format_to_reg(intel_dsi->pixel_format);
>  	}
>  
>  	tmp = 0;
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 92f39227b361..54f072cd78f1 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -34,8 +34,6 @@
>  #define DSI_DUAL_LINK_FRONT_BACK	1
>  #define DSI_DUAL_LINK_PIXEL_ALT		2
>  
> -int dsi_pixel_format_bpp(int pixel_format);
> -
>  struct intel_dsi_host;
>  
>  struct intel_dsi {
> @@ -64,8 +62,12 @@ struct intel_dsi {
>  	/* number of DSI lanes */
>  	unsigned int lane_count;
>  
> -	/* video mode pixel format for MIPI_DSI_FUNC_PRG register */
> -	u32 pixel_format;
> +	/*
> +	 * video mode pixel format
> +	 *
> +	 * XXX: consolidate on .format in struct mipi_dsi_device.
> +	 */
> +	enum mipi_dsi_pixel_format pixel_format;
>  
>  	/* video mode format for MIPI_VIDEO_MODE_FORMAT register */
>  	u32 video_mode_format;
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 7f145b4fec6a..8302a972d2d4 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -412,6 +412,25 @@ static const struct drm_panel_funcs vbt_panel_funcs = {
>  	.get_modes = vbt_panel_get_modes,
>  };
>  
> +/* XXX: This should be done when parsing the VBT in intel_bios.c */
> +static enum mipi_dsi_pixel_format pixel_format_from_vbt(u32 fmt)
> +{
> +	/* It just so happens the VBT matches register contents. */
> +	switch (fmt) {
> +	case VID_MODE_FORMAT_RGB888:
> +		return MIPI_DSI_FMT_RGB888;
> +	case VID_MODE_FORMAT_RGB666:
> +		return MIPI_DSI_FMT_RGB666;
> +	case VID_MODE_FORMAT_RGB666_PACKED:
> +		return MIPI_DSI_FMT_RGB666_PACKED;
> +	case VID_MODE_FORMAT_RGB565:
> +		return MIPI_DSI_FMT_RGB565;
> +	default:
> +		MISSING_CASE(fmt);
> +		return MIPI_DSI_FMT_RGB666;
> +	}
> +}
> +
>  struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
>  {
>  	struct drm_device *dev = intel_dsi->base.base.dev;
> @@ -420,7 +439,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
>  	struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
>  	struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
>  	struct vbt_panel *vbt_panel;
> -	u32 bits_per_pixel = 24;
> +	u32 bpp;
>  	u32 tlpx_ns, extra_byte_count, bitrate, tlpx_ui;
>  	u32 ui_num, ui_den;
>  	u32 prepare_cnt, exit_zero_cnt, clk_zero_cnt, trail_cnt;
> @@ -436,12 +455,11 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
>  	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 = mipi_config->videomode_color_format << 7;
> +	intel_dsi->pixel_format = pixel_format_from_vbt(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;
> -
> -	bits_per_pixel = dsi_pixel_format_bpp(intel_dsi->pixel_format);
> -
>  	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;
> @@ -475,8 +493,7 @@ struct drm_panel *vbt_panel_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 * bits_per_pixel) / intel_dsi->lane_count;
> +			computed_ddr = (pclk * bpp) / intel_dsi->lane_count;
>  
>  			if (mipi_config->target_burst_mode_freq <
>  								computed_ddr) {
> @@ -499,7 +516,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
>  	intel_dsi->burst_mode_ratio = burst_mode_ratio;
>  	intel_dsi->pclk = pclk;
>  
> -	bitrate = (pclk * bits_per_pixel) / intel_dsi->lane_count;
> +	bitrate = (pclk * bpp) / intel_dsi->lane_count;
>  
>  	switch (intel_dsi->escape_clk_div) {
>  	case 0:
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index 9ef0f7806e4a..e3e343c80221 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -30,27 +30,6 @@
>  #include "i915_drv.h"
>  #include "intel_dsi.h"
>  
> -int dsi_pixel_format_bpp(int pixel_format)
> -{
> -	int bpp;
> -
> -	switch (pixel_format) {
> -	default:
> -	case VID_MODE_FORMAT_RGB888:
> -	case VID_MODE_FORMAT_RGB666:
> -		bpp = 24;
> -		break;
> -	case VID_MODE_FORMAT_RGB666_PACKED:
> -		bpp = 18;
> -		break;
> -	case VID_MODE_FORMAT_RGB565:
> -		bpp = 16;
> -		break;
> -	}
> -
> -	return bpp;
> -}
> -
>  struct dsi_mnp {
>  	u32 dsi_pll_ctrl;
>  	u32 dsi_pll_div;
> @@ -64,10 +43,11 @@ static const u32 lfsr_converts[] = {
>  };
>  
>  /* Get DSI clock from pixel clock */
> -static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
> +static u32 dsi_clk_from_pclk(u32 pclk, enum mipi_dsi_pixel_format fmt,
> +			     int lane_count)
>  {
>  	u32 dsi_clk_khz;
> -	u32 bpp = dsi_pixel_format_bpp(pixel_format);
> +	u32 bpp = mipi_dsi_pixel_format_to_bpp(fmt);
>  
>  	/* DSI data rate = pixel clock * bits per pixel / lane count
>  	   pixel clock is converted from KHz to Hz */
> @@ -232,9 +212,9 @@ static void bxt_disable_dsi_pll(struct intel_encoder *encoder)
>  		DRM_ERROR("Timeout waiting for PLL lock deassertion\n");
>  }
>  
> -static void assert_bpp_mismatch(int pixel_format, int pipe_bpp)
> +static void assert_bpp_mismatch(enum mipi_dsi_pixel_format fmt, int pipe_bpp)
>  {
> -	int bpp = dsi_pixel_format_bpp(pixel_format);
> +	int bpp = mipi_dsi_pixel_format_to_bpp(fmt);
>  
>  	WARN(bpp != pipe_bpp,
>  	     "bpp match assertion failure (expected %d, current %d)\n",
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/dsi: lose the loose 666 format name in favor of packed

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/dsi: lose the loose 666 format name in favor of packed
URL   : https://patchwork.freedesktop.org/series/4505/
State : failure

== Summary ==

Series 4505v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/4505/revisions/1/mbox/

Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> DMESG-WARN (skl-nuci5)
Test gem_mmap_gtt:
        Subgroup basic-small-copy:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test gem_ringfill:
        Subgroup basic-default-s3:
                dmesg-warn -> PASS       (skl-i5k-2)
                pass       -> DMESG-WARN (skl-nuci5)
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (bsw-nuc-2)
                dmesg-warn -> PASS       (hsw-brixbox)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-a:
                dmesg-warn -> PASS       (bdw-nuci7)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                fail       -> DMESG-FAIL (snb-x220t)

bdw-nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:194  pass:172  dwarn:1   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:194  pass:155  dwarn:1   dfail:0   fail:1   skip:37 
byt-nuc          total:194  pass:155  dwarn:4   dfail:0   fail:0   skip:35 
hsw-brixbox      total:194  pass:172  dwarn:0   dfail:0   fail:0   skip:22 
hsw-gt2          total:194  pass:177  dwarn:0   dfail:0   fail:0   skip:17 
ivb-t430s        total:194  pass:169  dwarn:0   dfail:0   fail:0   skip:25 
skl-i5k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23 
skl-i7k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:194  pass:181  dwarn:2   dfail:0   fail:0   skip:11 
snb-x220t        total:194  pass:159  dwarn:1   dfail:1   fail:0   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1613/

9f8709ffd099e85e5e116ed7d09f1b8009f40847 drm-intel-nightly: 2016y-03m-16d-10h-30m-26s UTC integration manifest
cb794e7f2181ebeb3ba9ba2acd0cf8a5d9d30a68 drm/i915/dsi: start using enum mipi_dsi_pixel_format
20d35c513b9aa849fdaf0bfabfdaffbd22a0bb92 drm/i915/dsi: lose the loose 666 format name in favor of packed

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/dsi: lose the loose 666 format name in favor of packed

[Jani Nikula]

On Wed, 16 Mar 2016, Patchwork <patchwork@emeril.freedesktop.org> wrote:
> [ text/plain ]
> == Series Details ==
>
> Series: series starting with [1/2] drm/i915/dsi: lose the loose 666 format name in favor of packed
> URL   : https://patchwork.freedesktop.org/series/4505/
> State : failure
>
> == Summary ==
>
> Series 4505v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/4505/revisions/1/mbox/
>
> Test drv_hangman:
>         Subgroup error-state-basic:
>                 pass       -> DMESG-WARN (skl-nuci5)
> Test gem_mmap_gtt:
>         Subgroup basic-small-copy:
>                 dmesg-warn -> PASS       (bsw-nuc-2)
> Test gem_ringfill:
>         Subgroup basic-default-s3:
>                 dmesg-warn -> PASS       (skl-i5k-2)
>                 pass       -> DMESG-WARN (skl-nuci5)
> Test kms_flip:
>         Subgroup basic-flip-vs-modeset:
>                 pass       -> DMESG-WARN (bdw-ultra)
>         Subgroup basic-flip-vs-wf_vblank:
>                 pass       -> FAIL       (bsw-nuc-2)
>                 dmesg-warn -> PASS       (hsw-brixbox)
> Test kms_pipe_crc_basic:
>         Subgroup read-crc-pipe-a:
>                 dmesg-warn -> PASS       (bdw-nuci7)
> Test pm_rpm:
>         Subgroup basic-pci-d3-state:
>                 fail       -> DMESG-FAIL (snb-x220t)

None of these platforms are affected by the patches.

BR,
Jani.

>
> bdw-nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:12 
> bdw-ultra        total:194  pass:172  dwarn:1   dfail:0   fail:0   skip:21 
> bsw-nuc-2        total:194  pass:155  dwarn:1   dfail:0   fail:1   skip:37 
> byt-nuc          total:194  pass:155  dwarn:4   dfail:0   fail:0   skip:35 
> hsw-brixbox      total:194  pass:172  dwarn:0   dfail:0   fail:0   skip:22 
> hsw-gt2          total:194  pass:177  dwarn:0   dfail:0   fail:0   skip:17 
> ivb-t430s        total:194  pass:169  dwarn:0   dfail:0   fail:0   skip:25 
> skl-i5k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23 
> skl-i7k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23 
> skl-nuci5        total:194  pass:181  dwarn:2   dfail:0   fail:0   skip:11 
> snb-x220t        total:194  pass:159  dwarn:1   dfail:1   fail:0   skip:33 
>
> Results at /archive/results/CI_IGT_test/Patchwork_1613/
>
> 9f8709ffd099e85e5e116ed7d09f1b8009f40847 drm-intel-nightly: 2016y-03m-16d-10h-30m-26s UTC integration manifest
> cb794e7f2181ebeb3ba9ba2acd0cf8a5d9d30a68 drm/i915/dsi: start using enum mipi_dsi_pixel_format
> 20d35c513b9aa849fdaf0bfabfdaffbd22a0bb92 drm/i915/dsi: lose the loose 666 format name in favor of packed
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dsi: lose the loose 666 format name in favor of packed

[Ville Syrjälä]

On Wed, Mar 16, 2016 at 12:21:39PM +0200, Jani Nikula wrote:
> The enum mipi_dsi_pixel_format defines MIPI_DSI_FMT_RGB666 for the
> "loose" 24 bpp format and MIPI_DSI_FMT_RGB666_PACKED for the 18 bpp
> format. We have this the other way round, defining a loose version for
> 24 bpp.
> 
> Follow suit with what's in enum mipi_dsi_pixel_format to avoid future
> confusion. Rename
> 
> VID_MODE_FORMAT_RGB666 -> VID_MODE_FORMAT_RGB666_PACKED
> VID_MODE_FORMAT_RGB666_LOOSE -> VID_MODE_FORMAT_RGB666

This goes against our spec a bit, and the DSI spec calls these
"packed" and "loosly packed". So I'm not entirely thrilled about the
names used in the mipi_dsi_pixel_format either. I think the best
option would be to call them something like 666_18 and 666_24 to
avoid all the confusion.

But patch looks sane anyway so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 4 ++--
>  drivers/gpu/drm/i915/intel_dsi_pll.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7dfc4007f3fa..85ceec611412 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7887,8 +7887,8 @@ enum skl_disp_power_wells {
>  #define  VID_MODE_FORMAT_MASK				(0xf << 7)
>  #define  VID_MODE_NOT_SUPPORTED				(0 << 7)
>  #define  VID_MODE_FORMAT_RGB565				(1 << 7)
> -#define  VID_MODE_FORMAT_RGB666				(2 << 7)
> -#define  VID_MODE_FORMAT_RGB666_LOOSE			(3 << 7)
> +#define  VID_MODE_FORMAT_RGB666_PACKED			(2 << 7)
> +#define  VID_MODE_FORMAT_RGB666				(3 << 7)
>  #define  VID_MODE_FORMAT_RGB888				(4 << 7)
>  #define  CMD_MODE_CHANNEL_NUMBER_SHIFT			5
>  #define  CMD_MODE_CHANNEL_NUMBER_MASK			(3 << 5)
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index 2451c84949bd..9ef0f7806e4a 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -37,10 +37,10 @@ int dsi_pixel_format_bpp(int pixel_format)
>  	switch (pixel_format) {
>  	default:
>  	case VID_MODE_FORMAT_RGB888:
> -	case VID_MODE_FORMAT_RGB666_LOOSE:
> +	case VID_MODE_FORMAT_RGB666:
>  		bpp = 24;
>  		break;
> -	case VID_MODE_FORMAT_RGB666:
> +	case VID_MODE_FORMAT_RGB666_PACKED:
>  		bpp = 18;
>  		break;
>  	case VID_MODE_FORMAT_RGB565:
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/dsi: lose the loose 666 format name in favor of packed

[Jani Nikula]

On Wed, 16 Mar 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> [ text/plain ]
> On Wed, Mar 16, 2016 at 12:21:39PM +0200, Jani Nikula wrote:
>> The enum mipi_dsi_pixel_format defines MIPI_DSI_FMT_RGB666 for the
>> "loose" 24 bpp format and MIPI_DSI_FMT_RGB666_PACKED for the 18 bpp
>> format. We have this the other way round, defining a loose version for
>> 24 bpp.
>> 
>> Follow suit with what's in enum mipi_dsi_pixel_format to avoid future
>> confusion. Rename
>> 
>> VID_MODE_FORMAT_RGB666 -> VID_MODE_FORMAT_RGB666_PACKED
>> VID_MODE_FORMAT_RGB666_LOOSE -> VID_MODE_FORMAT_RGB666
>
> This goes against our spec a bit, and the DSI spec calls these
> "packed" and "loosly packed". So I'm not entirely thrilled about the
> names used in the mipi_dsi_pixel_format either. I think the best
> option would be to call them something like 666_18 and 666_24 to
> avoid all the confusion.

Agreed, but for now I think conforming to mipi_dsi_pixel_format is the
least bad option.

> But patch looks sane anyway so
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks, both pushed to drm-intel-next-queued.

BR,
Jani.

>
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h      | 4 ++--
>>  drivers/gpu/drm/i915/intel_dsi_pll.c | 4 ++--
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 7dfc4007f3fa..85ceec611412 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7887,8 +7887,8 @@ enum skl_disp_power_wells {
>>  #define  VID_MODE_FORMAT_MASK				(0xf << 7)
>>  #define  VID_MODE_NOT_SUPPORTED				(0 << 7)
>>  #define  VID_MODE_FORMAT_RGB565				(1 << 7)
>> -#define  VID_MODE_FORMAT_RGB666				(2 << 7)
>> -#define  VID_MODE_FORMAT_RGB666_LOOSE			(3 << 7)
>> +#define  VID_MODE_FORMAT_RGB666_PACKED			(2 << 7)
>> +#define  VID_MODE_FORMAT_RGB666				(3 << 7)
>>  #define  VID_MODE_FORMAT_RGB888				(4 << 7)
>>  #define  CMD_MODE_CHANNEL_NUMBER_SHIFT			5
>>  #define  CMD_MODE_CHANNEL_NUMBER_MASK			(3 << 5)
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
>> index 2451c84949bd..9ef0f7806e4a 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
>> @@ -37,10 +37,10 @@ int dsi_pixel_format_bpp(int pixel_format)
>>  	switch (pixel_format) {
>>  	default:
>>  	case VID_MODE_FORMAT_RGB888:
>> -	case VID_MODE_FORMAT_RGB666_LOOSE:
>> +	case VID_MODE_FORMAT_RGB666:
>>  		bpp = 24;
>>  		break;
>> -	case VID_MODE_FORMAT_RGB666:
>> +	case VID_MODE_FORMAT_RGB666_PACKED:
>>  		bpp = 18;
>>  		break;
>>  	case VID_MODE_FORMAT_RGB565:
>> -- 
>> 2.1.4

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx