[v2 0/2] Support for new MIPI Blocks in VBT

[v2 0/2] Support for new MIPI Blocks in VBT

[Shobhit Kumar]

Hi,
To support a plethora of MIPI panels, the VBT has been enhanced to support
multiple panels in a generic manner. This involves adding one MIPI configuration
block (#52) which provide all panel specific configuration data along with the 
dphy configuration information as per panel/tcon specs. Also a sequence (#53) block 
is added to provide required sequences for panel enabling.

The driver will use this parsed data to enable the panel which is described in these
two block. The i-g-t tool intel_parse_bios changes to parse these new block is WIP.

The generic driver will be as one of sub-encoder driver in the existing design to support 
MIPI. Followup patches for this driver will come next.

v2: Address review comments from Jani Nikula
    - update datastructures and related code  to enable cleaner and simpler code
    - using kmemdup instead of kmalloc and memcpy
    - Proper error detection during parsing MIPI sequence block

Regards
Shobhit

Shobhit Kumar (2):
  drm/i915: Update VBT data structures to have MIPI block enhancements
  drm/i915: Add parsing support for new MIPI blocks in VBT

 drivers/gpu/drm/i915/i915_drv.h   |   6 ++
 drivers/gpu/drm/i915/intel_bios.c | 164 ++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_bios.h | 208 ++++++++++++++++++++++++++++++++------
 3 files changed, 343 insertions(+), 35 deletions(-)

-- 
1.8.3.2

[v2 1/2] drm/i915: Update VBT data structures to have MIPI block enhancements

[Shobhit Kumar]

MIPI Block #52 which provides configuration details for the MIPI panel
including dphy settings as per panel and tcon specs

Block #53 gives information on panel enable sequences

v2: Address review comemnts from Jani
    - Move panel ids from intel_dsi.h to intel_bios.h
    - bdb_mipi_config structure improvements for cleaner code
    - Adding units for the pps delays, all in ms
    - change data structure to be more cleaner and simple

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c |   4 +-
 drivers/gpu/drm/i915/intel_bios.h | 174 +++++++++++++++++++++++++++++++-------
 2 files changed, 147 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 86b95ca..4867f4c 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -599,14 +599,14 @@ parse_mipi(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
 {
 	struct bdb_mipi *mipi;
 
-	mipi = find_section(bdb, BDB_MIPI);
+	mipi = find_section(bdb, BDB_MIPI_CONFIG);
 	if (!mipi) {
 		DRM_DEBUG_KMS("No MIPI BDB found");
 		return;
 	}
 
 	/* XXX: add more info */
-	dev_priv->vbt.dsi.panel_id = mipi->panel_id;
+	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
 }
 
 static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index 282de5e..162d0d2 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -104,7 +104,8 @@ struct vbios_data {
 #define BDB_LVDS_LFP_DATA	 42
 #define BDB_LVDS_BACKLIGHT	 43
 #define BDB_LVDS_POWER		 44
-#define BDB_MIPI		 50
+#define BDB_MIPI_CONFIG		 52
+#define BDB_MIPI_SEQUENCE	 53
 #define BDB_SKIP		254 /* VBIOS private block, ignore */
 
 struct bdb_general_features {
@@ -711,44 +712,159 @@ int intel_parse_bios(struct drm_device *dev);
 #define DVO_PORT_DPD	9
 #define DVO_PORT_DPA	10
 
-/* MIPI DSI panel info */
-struct bdb_mipi {
-	u16 panel_id;
-	u16 bridge_revision;
-
-	/* General params */
-	u32 dithering:1;
-	u32 bpp_pixel_format:1;
-	u32 rsvd1:1;
-	u32 dphy_valid:1;
-	u32 resvd2:28;
+/* Block 52 contains MIPI Panel info
+ * 6 such enteries will there. Index into correct
+ * entery is based on the panel_index in #40 LFP
+ */
+#define MAX_MIPI_CONFIGURATIONS	6
 
-	u16 port_info;
-	u16 rsvd3:2;
-	u16 num_lanes:2;
-	u16 rsvd4:12;
+#define MIPI_DSI_UNDEFINED_PANEL_ID	0
+#define MIPI_DSI_GENERIC_PANEL_ID	1
 
-	/* DSI config */
-	u16 virt_ch_num:2;
-	u16 vtm:2;
-	u16 rsvd5:12;
+struct mipi_config {
+	u16 panel_id;
 
-	u32 dsi_clock;
+	/* General Params */
+	u32 enable_dithering:1;
+	u32 rsvd1:1;
+	u32 is_bridge:1;
+
+	u32 panel_arch_type:2;
+	u32 is_cmd_mode:1;
+
+#define NON_BURST_SYNC_PULSE	0x1
+#define NON_BURST_SYNC_EVENTS	0x2
+#define BURST_MODE		0x3
+	u32 video_transfer_mode:2;
+
+	u32 cabc_supported:1;
+	u32 pwm_blc:1;
+
+	/* Bit 13:10 */
+#define PIXEL_FORMAT_RGB565			0x1
+#define PIXEL_FORMAT_RGB666			0x2
+#define PIXEL_FORMAT_RGB666_LOOSELY_PACKED	0x3
+#define PIXEL_FORMAT_RGB888			0x4
+	u32 videomode_color_format:4;
+
+	/* Bit 15:14 */
+#define ENABLE_ROTATION_0	0x0
+#define ENABLE_ROTATION_90	0x1
+#define ENABLE_ROTATION_180	0x2
+#define ENABLE_ROTATION_270	0x3
+	u32 rotation:2;
+	u32 bta_enabled:1;
+	u32 rsvd2:15;
+
+	/* 2 byte Port Description */
+#define DUAL_LINK_NOT_SUPPORTED	0
+#define DUAL_LINK_FRONT_BACK	1
+#define DUAL_LINK_PIXEL_ALT	2
+	u16 dual_link:2;
+	u16 lane_cnt:2;
+	u16 rsvd3:12;
+
+	u16 rsvd4;
+
+	u8 rsvd5[5];
+	u32 dsi_ddr_clk;
 	u32 bridge_ref_clk;
-	u16 rsvd_pwr;
 
-	/* Dphy Params */
-	u32 prepare_cnt:5;
-	u32 rsvd6:3;
+#define  BYTE_CLK_SEL_20MHZ		0
+#define  BYTE_CLK_SEL_10MHZ		1
+#define  BYTE_CLK_SEL_5MHZ		2
+	u8 byte_clk_sel:2;
+
+	u8 rsvd6:6;
+
+	/* DPHY Flags */
+	u16 dphy_param_valid:1;
+	u16 eot_pkt_disabled:1;
+	u16 enable_clk_stop:1;
+	u16 rsvd7:13;
+
+	u32 hs_tx_timeout;
+	u32 lp_rx_timeout;
+	u32 turn_around_timeout;
+	u32 device_reset_timer;
+	u32 master_init_timer;
+	u32 dbi_bw_timer;
+	u32 lp_byte_clk_val;
+
+	/*  4 byte Dphy Params */
+	u32 prepare_cnt:6;
+	u32 rsvd8:2;
 	u32 clk_zero_cnt:8;
 	u32 trail_cnt:5;
-	u32 rsvd7:3;
+	u32 rsvd9:3;
 	u32 exit_zero_cnt:6;
-	u32 rsvd8:2;
+	u32 rsvd10:2;
 
-	u32 hl_switch_cnt;
-	u32 lp_byte_clk;
 	u32 clk_lane_switch_cnt;
+	u32 hl_switch_cnt;
+
+	u32 rsvd11[6];
+
+	/* timings based on dphy spec */
+	u8 tclk_miss;
+	u8 tclk_post;
+	u8 rsvd12;
+	u8 tclk_pre;
+	u8 tclk_prepare;
+	u8 tclk_settle;
+	u8 tclk_term_enable;
+	u8 tclk_trail;
+	u16 tclk_prepare_clkzero;
+	u8 rsvd13;
+	u8 td_term_enable;
+	u8 teot;
+	u8 ths_exit;
+	u8 ths_prepare;
+	u16 ths_prepare_hszero;
+	u8 rsvd14;
+	u8 ths_settle;
+	u8 ths_skip;
+	u8 ths_trail;
+	u8 tinit;
+	u8 tlpx;
+	u8 rsvd15[3];
+
+	/* GPIOs */
+	u8 panel_enable;
+	u8 bl_enable;
+	u8 pwm_enable;
+	u8 reset_r_n;
+	u8 pwr_down_r;
+	u8 stdby_r_n;
+
 } __packed;
 
+/* Block 52 contains MIPI configuration block
+ * 6 * bdb_mipi_config, followed by 6 pps data
+ * block below
+ *
+ * all delays in ms
+ */
+struct mipi_pps_data {
+	u16 panel_on_delay;
+	u16 bl_enable_delay;
+	u16 bl_disable_delay;
+	u16 panel_off_delay;
+	u16 panel_power_cycle_delay;
+};
+
+struct bdb_mipi_config {
+	struct mipi_config config[MAX_MIPI_CONFIGURATIONS];
+	struct mipi_pps_data pps[MAX_MIPI_CONFIGURATIONS];
+};
+
+/* Block 53 contains MIPI sequences as needed by the panel
+ * for enabling it. This block can be variable in size and
+ * can be maximum of 6 blocks
+ */
+struct bdb_mipi_sequence {
+	u8 version;
+	u8 data[0];
+};
+
 #endif /* _I830_BIOS_H_ */
-- 
1.8.3.2

Re: [v2 1/2] drm/i915: Update VBT data structures to have MIPI block enhancements

[Jani Nikula]

On Thu, 20 Feb 2014, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
> +/* Block 52 contains MIPI configuration block
> + * 6 * bdb_mipi_config, followed by 6 pps data
> + * block below
> + *
> + * all delays in ms

The spec you sent me has "... delay in 100us unit" for MIPI PPS entries
which is weird and which is the reason I asked you to document the
units. Please check.

With that checked,

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


-- 
Jani Nikula, Intel Open Source Technology Center

Re: [v2 1/2] drm/i915: Update VBT data structures to have MIPI block enhancements

[Shobhit Kumar]

On Thursday 27 February 2014 08:18 PM, Jani Nikula wrote:
> On Thu, 20 Feb 2014, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
>> +/* Block 52 contains MIPI configuration block
>> + * 6 * bdb_mipi_config, followed by 6 pps data
>> + * block below
>> + *
>> + * all delays in ms
>
> The spec you sent me has "... delay in 100us unit" for MIPI PPS entries
> which is weird and which is the reason I asked you to document the
> units. Please check.

Yeah, I made a mistake. It *is* in units of 100us. Will fix this.

Regards
Shobhit

[PATCH] drm/i915: Update VBT data structures to have MIPI block enhancements

[Shobhit Kumar]

MIPI Block #52 which provides configuration details for the MIPI panel
including dphy settings as per panel and tcon specs

Block #53 gives information on panel enable sequences

v2: Address review comemnts from Jani
    - Move panel ids from intel_dsi.h to intel_bios.h
    - bdb_mipi_config structure improvements for cleaner code
    - Adding units for the pps delays, all in ms
    - change data structure to be more cleaner and simple

v3: Corrected the unit for pps delays as 100us

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c |   4 +-
 drivers/gpu/drm/i915/intel_bios.h | 174 +++++++++++++++++++++++++++++++-------
 2 files changed, 147 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 86b95ca..4867f4c 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -599,14 +599,14 @@ parse_mipi(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
 {
 	struct bdb_mipi *mipi;
 
-	mipi = find_section(bdb, BDB_MIPI);
+	mipi = find_section(bdb, BDB_MIPI_CONFIG);
 	if (!mipi) {
 		DRM_DEBUG_KMS("No MIPI BDB found");
 		return;
 	}
 
 	/* XXX: add more info */
-	dev_priv->vbt.dsi.panel_id = mipi->panel_id;
+	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
 }
 
 static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index 282de5e..83b7629 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -104,7 +104,8 @@ struct vbios_data {
 #define BDB_LVDS_LFP_DATA	 42
 #define BDB_LVDS_BACKLIGHT	 43
 #define BDB_LVDS_POWER		 44
-#define BDB_MIPI		 50
+#define BDB_MIPI_CONFIG		 52
+#define BDB_MIPI_SEQUENCE	 53
 #define BDB_SKIP		254 /* VBIOS private block, ignore */
 
 struct bdb_general_features {
@@ -711,44 +712,159 @@ int intel_parse_bios(struct drm_device *dev);
 #define DVO_PORT_DPD	9
 #define DVO_PORT_DPA	10
 
-/* MIPI DSI panel info */
-struct bdb_mipi {
-	u16 panel_id;
-	u16 bridge_revision;
-
-	/* General params */
-	u32 dithering:1;
-	u32 bpp_pixel_format:1;
-	u32 rsvd1:1;
-	u32 dphy_valid:1;
-	u32 resvd2:28;
+/* Block 52 contains MIPI Panel info
+ * 6 such enteries will there. Index into correct
+ * entery is based on the panel_index in #40 LFP
+ */
+#define MAX_MIPI_CONFIGURATIONS	6
 
-	u16 port_info;
-	u16 rsvd3:2;
-	u16 num_lanes:2;
-	u16 rsvd4:12;
+#define MIPI_DSI_UNDEFINED_PANEL_ID	0
+#define MIPI_DSI_GENERIC_PANEL_ID	1
 
-	/* DSI config */
-	u16 virt_ch_num:2;
-	u16 vtm:2;
-	u16 rsvd5:12;
+struct mipi_config {
+	u16 panel_id;
 
-	u32 dsi_clock;
+	/* General Params */
+	u32 enable_dithering:1;
+	u32 rsvd1:1;
+	u32 is_bridge:1;
+
+	u32 panel_arch_type:2;
+	u32 is_cmd_mode:1;
+
+#define NON_BURST_SYNC_PULSE	0x1
+#define NON_BURST_SYNC_EVENTS	0x2
+#define BURST_MODE		0x3
+	u32 video_transfer_mode:2;
+
+	u32 cabc_supported:1;
+	u32 pwm_blc:1;
+
+	/* Bit 13:10 */
+#define PIXEL_FORMAT_RGB565			0x1
+#define PIXEL_FORMAT_RGB666			0x2
+#define PIXEL_FORMAT_RGB666_LOOSELY_PACKED	0x3
+#define PIXEL_FORMAT_RGB888			0x4
+	u32 videomode_color_format:4;
+
+	/* Bit 15:14 */
+#define ENABLE_ROTATION_0	0x0
+#define ENABLE_ROTATION_90	0x1
+#define ENABLE_ROTATION_180	0x2
+#define ENABLE_ROTATION_270	0x3
+	u32 rotation:2;
+	u32 bta_enabled:1;
+	u32 rsvd2:15;
+
+	/* 2 byte Port Description */
+#define DUAL_LINK_NOT_SUPPORTED	0
+#define DUAL_LINK_FRONT_BACK	1
+#define DUAL_LINK_PIXEL_ALT	2
+	u16 dual_link:2;
+	u16 lane_cnt:2;
+	u16 rsvd3:12;
+
+	u16 rsvd4;
+
+	u8 rsvd5[5];
+	u32 dsi_ddr_clk;
 	u32 bridge_ref_clk;
-	u16 rsvd_pwr;
 
-	/* Dphy Params */
-	u32 prepare_cnt:5;
-	u32 rsvd6:3;
+#define  BYTE_CLK_SEL_20MHZ		0
+#define  BYTE_CLK_SEL_10MHZ		1
+#define  BYTE_CLK_SEL_5MHZ		2
+	u8 byte_clk_sel:2;
+
+	u8 rsvd6:6;
+
+	/* DPHY Flags */
+	u16 dphy_param_valid:1;
+	u16 eot_pkt_disabled:1;
+	u16 enable_clk_stop:1;
+	u16 rsvd7:13;
+
+	u32 hs_tx_timeout;
+	u32 lp_rx_timeout;
+	u32 turn_around_timeout;
+	u32 device_reset_timer;
+	u32 master_init_timer;
+	u32 dbi_bw_timer;
+	u32 lp_byte_clk_val;
+
+	/*  4 byte Dphy Params */
+	u32 prepare_cnt:6;
+	u32 rsvd8:2;
 	u32 clk_zero_cnt:8;
 	u32 trail_cnt:5;
-	u32 rsvd7:3;
+	u32 rsvd9:3;
 	u32 exit_zero_cnt:6;
-	u32 rsvd8:2;
+	u32 rsvd10:2;
 
-	u32 hl_switch_cnt;
-	u32 lp_byte_clk;
 	u32 clk_lane_switch_cnt;
+	u32 hl_switch_cnt;
+
+	u32 rsvd11[6];
+
+	/* timings based on dphy spec */
+	u8 tclk_miss;
+	u8 tclk_post;
+	u8 rsvd12;
+	u8 tclk_pre;
+	u8 tclk_prepare;
+	u8 tclk_settle;
+	u8 tclk_term_enable;
+	u8 tclk_trail;
+	u16 tclk_prepare_clkzero;
+	u8 rsvd13;
+	u8 td_term_enable;
+	u8 teot;
+	u8 ths_exit;
+	u8 ths_prepare;
+	u16 ths_prepare_hszero;
+	u8 rsvd14;
+	u8 ths_settle;
+	u8 ths_skip;
+	u8 ths_trail;
+	u8 tinit;
+	u8 tlpx;
+	u8 rsvd15[3];
+
+	/* GPIOs */
+	u8 panel_enable;
+	u8 bl_enable;
+	u8 pwm_enable;
+	u8 reset_r_n;
+	u8 pwr_down_r;
+	u8 stdby_r_n;
+
 } __packed;
 
+/* Block 52 contains MIPI configuration block
+ * 6 * bdb_mipi_config, followed by 6 pps data
+ * block below
+ *
+ * all delays has a unit of 100us
+ */
+struct mipi_pps_data {
+	u16 panel_on_delay;
+	u16 bl_enable_delay;
+	u16 bl_disable_delay;
+	u16 panel_off_delay;
+	u16 panel_power_cycle_delay;
+};
+
+struct bdb_mipi_config {
+	struct mipi_config config[MAX_MIPI_CONFIGURATIONS];
+	struct mipi_pps_data pps[MAX_MIPI_CONFIGURATIONS];
+};
+
+/* Block 53 contains MIPI sequences as needed by the panel
+ * for enabling it. This block can be variable in size and
+ * can be maximum of 6 blocks
+ */
+struct bdb_mipi_sequence {
+	u8 version;
+	u8 data[0];
+};
+
 #endif /* _I830_BIOS_H_ */
-- 
1.8.3.2

Re: [PATCH] drm/i915: Update VBT data structures to have MIPI block enhancements

[Daniel Vetter]

On Fri, Feb 28, 2014 at 11:18:46AM +0530, Shobhit Kumar wrote:
> MIPI Block #52 which provides configuration details for the MIPI panel
> including dphy settings as per panel and tcon specs
> 
> Block #53 gives information on panel enable sequences
> 
> v2: Address review comemnts from Jani
>     - Move panel ids from intel_dsi.h to intel_bios.h
>     - bdb_mipi_config structure improvements for cleaner code
>     - Adding units for the pps delays, all in ms
>     - change data structure to be more cleaner and simple
> 
> v3: Corrected the unit for pps delays as 100us
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[v2 2/2] drm/i915: Add parsing support for new MIPI blocks in VBT

[Shobhit Kumar]

The parser extracts the config block(#52) and sequence(#53) data
and store in private data structures.

v2: Address review comments by Jani
    - adjust code for the structure changes for bdb_mipi_config
    - add boundry and buffer overflow checks as suggested
    - use kmemdup instead of kmalloc and memcpy

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |   6 ++
 drivers/gpu/drm/i915/intel_bios.c | 162 ++++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_bios.h |  34 ++++++++
 3 files changed, 197 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 728b9c3..5056ced 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1251,6 +1251,12 @@ struct intel_vbt_data {
 	/* MIPI DSI */
 	struct {
 		u16 panel_id;
+		struct mipi_config *config;
+		struct mipi_pps_data *pps;
+		u8 seq_version;
+		u32 size;
+		u8 *data;
+		u8 *sequence[MIPI_SEQ_MAX];
 	} dsi;
 
 	int crt_ddc_pin;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 4867f4c..ac37f6f 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -594,19 +594,171 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
 	}
 }
 
+static u8 *goto_next_sequence(u8 *data, int *size)
+{
+	u16 len;
+	int tmp = *size;
+	/* goto first element */
+	data++;
+	while (1) {
+		switch (*data++) {
+		case MIPI_SEQ_ELEM_SEND_PKT:
+			/* skip by this element payload size
+			 * skip command flag and data type */
+			data += 2;
+			len = *((u16 *)data);
+			/* skip by len */
+			data = data + 2 + len;
+			tmp = tmp - 2 - len;
+			break;
+		case MIPI_SEQ_ELEM_DELAY:
+			data += 4;
+			tmp = tmp - 4;
+			break;
+		case MIPI_SEQ_ELEM_GPIO:
+			data += 2;
+			tmp = tmp - 2;
+			break;
+		default:
+			DRM_ERROR("Unknown element\n");
+			break;
+		}
+
+		if (tmp < 0)
+			WARN(1, "Reading beyond remaining sequence size\n");
+
+		/* end of sequence ? */
+		if (*data == 0)
+			break;
+	}
+
+	/* goto next sequence or end of block byte */
+	data++;
+	tmp--;
+
+	/* update amount of data left for the sequence block to be parsed */
+	*size = tmp;
+	return data;
+}
+
 static void
 parse_mipi(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
 {
-	struct bdb_mipi *mipi;
+	struct bdb_mipi_config *start;
+	struct bdb_mipi_sequence *sequence;
+	struct mipi_config *config;
+	struct mipi_pps_data *pps;
+	u8 *data, *seq_data;
+	int i, panel_id, seq_size;
+	u16 block_size;
+
+	/* Initialize this to undefined indicating no generic MIPI support */
+	dev_priv->vbt.dsi.panel_id = MIPI_DSI_UNDEFINED_PANEL_ID;
+
+	/* Block #40 is already parsed and panel_fixed_mode is
+	 * stored in dev_priv->lfp_lvds_vbt_mode
+	 * resuse this when needed
+	 */
 
-	mipi = find_section(bdb, BDB_MIPI_CONFIG);
-	if (!mipi) {
-		DRM_DEBUG_KMS("No MIPI BDB found");
+	/* Parse #52 for panel index used from panel_type already
+	 * parsed
+	 */
+	start = find_section(bdb, BDB_MIPI_CONFIG);
+	if (!start) {
+		DRM_DEBUG_KMS("No MIPI config BDB found");
 		return;
 	}
 
-	/* XXX: add more info */
+	DRM_DEBUG_DRIVER("Found MIPI Config block, panel index = %d\n",
+								panel_type);
+
+	/*
+	 * get hold of the correct configuration block and pps data as per
+	 * the panel_type as index
+	 */
+	config = &start->config[panel_type];
+	pps = &start->pps[panel_type];
+
+	/* store as of now full data. Trim when we realise all is not needed */
+	dev_priv->vbt.dsi.config = kmemdup(config, sizeof(struct mipi_config), GFP_KERNEL);
+	if (!dev_priv->vbt.dsi.config)
+		return;
+
+	dev_priv->vbt.dsi.pps = kmemdup(pps, sizeof(struct mipi_pps_data), GFP_KERNEL);
+	if (!dev_priv->vbt.dsi.pps) {
+		kfree(dev_priv->vbt.dsi.config);
+		return;
+	}
+
+	/* We have mandatory mipi config blocks. Initialize as generic panel */
 	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
+
+	/* Check if we have sequence block as well */
+	sequence = find_section(bdb, BDB_MIPI_SEQUENCE);
+	if (!sequence) {
+		DRM_DEBUG_KMS("No MIPI Sequnece found, parsing complete\n");
+		return;
+	}
+
+	DRM_DEBUG_DRIVER("Found MIPI sequence block\n");
+
+	block_size = get_blocksize(sequence);
+
+	/*
+	 * parse the sequence block for individual sequences
+	 */
+	dev_priv->vbt.dsi.seq_version = sequence->version;
+
+	seq_data = &sequence->data[0];
+
+	/* sequence block is variable length and hence we need to parse and
+	 * get the sequnece data for specific panel id */
+	for (i = 0; i < MAX_MIPI_CONFIGURATIONS; i++) {
+		panel_id = *seq_data;
+		seq_size = *((u16 *) (seq_data + 1));
+		if (panel_id == panel_type)
+			break;
+
+		/* skip the sequence including seq header of 3 bytes */
+		seq_data = seq_data + 3 + seq_size;
+		if ((seq_data - &sequence->data[0]) > block_size)
+			WARN(1, "Sequence is beyond sequence block size\n");
+	}
+
+	if (i == MAX_MIPI_CONFIGURATIONS) {
+		DRM_ERROR("Sequence block detected but no valid configuration\n");
+		return;
+	}
+
+	/* skip the panel id(1 byte) and seq size(2 bytes) */
+	dev_priv->vbt.dsi.data = kmemdup(seq_data + 3, seq_size, GFP_KERNEL);
+	if (!dev_priv->vbt.dsi.data)
+		return;
+
+	/*
+	 * loop into the sequence data and split into multiple sequneces
+	 * There are only 5 types of sequences as of now
+	 */
+	data = dev_priv->vbt.dsi.data;
+	dev_priv->vbt.dsi.size = seq_size;
+
+	/* two consecutive 0x00 inidcate end of all sequences */
+	while (1) {
+		int seq_id = *data;
+		if (MIPI_SEQ_MAX > seq_id && seq_id > MIPI_SEQ_UNDEFINED) {
+			dev_priv->vbt.dsi.sequence[seq_id] = data;
+			DRM_DEBUG_DRIVER("Found mipi sequence - %d\n", seq_id);
+		} else
+			DRM_DEBUG_DRIVER("undefined sequence\n");
+
+		/* partial parsing to skip elements */
+		data = goto_next_sequence(data, &seq_size);
+
+		if (*data == 0)
+			break; /* end of sequence reached */
+	}
+
+	DRM_DEBUG_DRIVER("MIPI related vbt parsing complete\n");
 }
 
 static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index 162d0d2..78bee8e 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -867,4 +867,38 @@ struct bdb_mipi_sequence {
 	u8 data[0];
 };
 
+/* MIPI Sequnece Block definitions */
+enum MIPI_SEQ {
+	MIPI_SEQ_UNDEFINED = 0,
+	MIPI_SEQ_ASSERT_RESET,
+	MIPI_SEQ_INIT_OTP,
+	MIPI_SEQ_DISPLAY_ON,
+	MIPI_SEQ_DISPLAY_OFF,
+	MIPI_SEQ_DEASSERT_RESET,
+	MIPI_SEQ_MAX
+
+};
+
+enum MIPI_SEQ_ELEMENT {
+	MIPI_SEQ_ELEM_UNDEFINED = 0,
+	MIPI_SEQ_ELEM_SEND_PKT,
+	MIPI_SEQ_ELEM_DELAY,
+	MIPI_SEQ_ELEM_GPIO,
+	MIPI_SEQ_ELEM_STATUS,
+	MIPI_SEQ_ELEM_MAX
+
+};
+
+enum MIPI_GPIO_PIN_INDEX {
+	MIPI_GPIO_UNDEFINED = 0,
+	MIPI_GPIO_PANEL_ENABLE,
+	MIPI_GPIO_BL_ENABLE,
+	MIPI_GPIO_PWM_ENABLE,
+	MIPI_GPIO_RESET_N,
+	MIPI_GPIO_PWR_DOWN_R,
+	MIPI_GPIO_STDBY_RST_N,
+	MIPI_GPIO_MAX
+
+};
+
 #endif /* _I830_BIOS_H_ */
-- 
1.8.3.2

Re: [v2 2/2] drm/i915: Add parsing support for new MIPI blocks in VBT

[Jani Nikula]

On Thu, 20 Feb 2014, Shobhit Kumar <shobhit.kumar@intel.com> wrote:
> The parser extracts the config block(#52) and sequence(#53) data
> and store in private data structures.
>
> v2: Address review comments by Jani
>     - adjust code for the structure changes for bdb_mipi_config
>     - add boundry and buffer overflow checks as suggested
>     - use kmemdup instead of kmalloc and memcpy
>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |   6 ++
>  drivers/gpu/drm/i915/intel_bios.c | 162 ++++++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_bios.h |  34 ++++++++
>  3 files changed, 197 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 728b9c3..5056ced 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1251,6 +1251,12 @@ struct intel_vbt_data {
>  	/* MIPI DSI */
>  	struct {
>  		u16 panel_id;
> +		struct mipi_config *config;
> +		struct mipi_pps_data *pps;
> +		u8 seq_version;
> +		u32 size;
> +		u8 *data;
> +		u8 *sequence[MIPI_SEQ_MAX];
>  	} dsi;
>  
>  	int crt_ddc_pin;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 4867f4c..ac37f6f 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -594,19 +594,171 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>  	}
>  }
>  
> +static u8 *goto_next_sequence(u8 *data, int *size)
> +{
> +	u16 len;
> +	int tmp = *size;
> +	/* goto first element */
> +	data++;
> +	while (1) {
> +		switch (*data++) {
> +		case MIPI_SEQ_ELEM_SEND_PKT:
> +			/* skip by this element payload size
> +			 * skip command flag and data type */
> +			data += 2;
> +			len = *((u16 *)data);

You shouldn't touch the data before checking the pointer isn't past the
block.

> +			/* skip by len */
> +			data = data + 2 + len;
> +			tmp = tmp - 2 - len;
> +			break;
> +		case MIPI_SEQ_ELEM_DELAY:
> +			data += 4;
> +			tmp = tmp - 4;
> +			break;
> +		case MIPI_SEQ_ELEM_GPIO:
> +			data += 2;
> +			tmp = tmp - 2;
> +			break;
> +		default:
> +			DRM_ERROR("Unknown element\n");

Reading the spec, basically you have to bail out of the whole sequence
block at this error. If you don't know the element, you don't know how
to parse the payload of the element, which means you don't know when the
element ends, which means there is no reliable way to find the next
element.

Essentially this means the spec was written in a way that makes this
forward incompatible.

> +			break;
> +		}
> +
> +		if (tmp < 0)
> +			WARN(1, "Reading beyond remaining sequence size\n");

This is not a WARN. You need to bail out here, all the way out from the
sequence block parsing.

> +
> +		/* end of sequence ? */
> +		if (*data == 0)
> +			break;
> +	}
> +
> +	/* goto next sequence or end of block byte */
> +	data++;
> +	tmp--;
> +
> +	/* update amount of data left for the sequence block to be parsed */
> +	*size = tmp;
> +	return data;
> +}
> +
>  static void
>  parse_mipi(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>  {
> -	struct bdb_mipi *mipi;
> +	struct bdb_mipi_config *start;
> +	struct bdb_mipi_sequence *sequence;
> +	struct mipi_config *config;
> +	struct mipi_pps_data *pps;
> +	u8 *data, *seq_data;
> +	int i, panel_id, seq_size;
> +	u16 block_size;
> +
> +	/* Initialize this to undefined indicating no generic MIPI support */
> +	dev_priv->vbt.dsi.panel_id = MIPI_DSI_UNDEFINED_PANEL_ID;
> +
> +	/* Block #40 is already parsed and panel_fixed_mode is
> +	 * stored in dev_priv->lfp_lvds_vbt_mode
> +	 * resuse this when needed
> +	 */
>  
> -	mipi = find_section(bdb, BDB_MIPI_CONFIG);
> -	if (!mipi) {
> -		DRM_DEBUG_KMS("No MIPI BDB found");
> +	/* Parse #52 for panel index used from panel_type already
> +	 * parsed
> +	 */
> +	start = find_section(bdb, BDB_MIPI_CONFIG);
> +	if (!start) {
> +		DRM_DEBUG_KMS("No MIPI config BDB found");
>  		return;
>  	}
>  
> -	/* XXX: add more info */
> +	DRM_DEBUG_DRIVER("Found MIPI Config block, panel index = %d\n",
> +								panel_type);
> +
> +	/*
> +	 * get hold of the correct configuration block and pps data as per
> +	 * the panel_type as index
> +	 */
> +	config = &start->config[panel_type];
> +	pps = &start->pps[panel_type];
> +
> +	/* store as of now full data. Trim when we realise all is not needed */
> +	dev_priv->vbt.dsi.config = kmemdup(config, sizeof(struct mipi_config), GFP_KERNEL);
> +	if (!dev_priv->vbt.dsi.config)
> +		return;
> +
> +	dev_priv->vbt.dsi.pps = kmemdup(pps, sizeof(struct mipi_pps_data), GFP_KERNEL);
> +	if (!dev_priv->vbt.dsi.pps) {
> +		kfree(dev_priv->vbt.dsi.config);
> +		return;
> +	}
> +
> +	/* We have mandatory mipi config blocks. Initialize as generic panel */
>  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
> +
> +	/* Check if we have sequence block as well */
> +	sequence = find_section(bdb, BDB_MIPI_SEQUENCE);
> +	if (!sequence) {
> +		DRM_DEBUG_KMS("No MIPI Sequnece found, parsing complete\n");
> +		return;
> +	}
> +
> +	DRM_DEBUG_DRIVER("Found MIPI sequence block\n");
> +
> +	block_size = get_blocksize(sequence);
> +
> +	/*
> +	 * parse the sequence block for individual sequences
> +	 */
> +	dev_priv->vbt.dsi.seq_version = sequence->version;
> +
> +	seq_data = &sequence->data[0];
> +
> +	/* sequence block is variable length and hence we need to parse and
> +	 * get the sequnece data for specific panel id */
> +	for (i = 0; i < MAX_MIPI_CONFIGURATIONS; i++) {
> +		panel_id = *seq_data;
> +		seq_size = *((u16 *) (seq_data + 1));
> +		if (panel_id == panel_type)
> +			break;
> +
> +		/* skip the sequence including seq header of 3 bytes */
> +		seq_data = seq_data + 3 + seq_size;
> +		if ((seq_data - &sequence->data[0]) > block_size)
> +			WARN(1, "Sequence is beyond sequence block size\n");
> +	}
> +
> +	if (i == MAX_MIPI_CONFIGURATIONS) {
> +		DRM_ERROR("Sequence block detected but no valid configuration\n");
> +		return;
> +	}
> +
> +	/* skip the panel id(1 byte) and seq size(2 bytes) */
> +	dev_priv->vbt.dsi.data = kmemdup(seq_data + 3, seq_size, GFP_KERNEL);
> +	if (!dev_priv->vbt.dsi.data)
> +		return;
> +
> +	/*
> +	 * loop into the sequence data and split into multiple sequneces
> +	 * There are only 5 types of sequences as of now
> +	 */
> +	data = dev_priv->vbt.dsi.data;
> +	dev_priv->vbt.dsi.size = seq_size;
> +
> +	/* two consecutive 0x00 inidcate end of all sequences */
> +	while (1) {
> +		int seq_id = *data;
> +		if (MIPI_SEQ_MAX > seq_id && seq_id > MIPI_SEQ_UNDEFINED) {
> +			dev_priv->vbt.dsi.sequence[seq_id] = data;
> +			DRM_DEBUG_DRIVER("Found mipi sequence - %d\n", seq_id);
> +		} else
> +			DRM_DEBUG_DRIVER("undefined sequence\n");
> +
> +		/* partial parsing to skip elements */
> +		data = goto_next_sequence(data, &seq_size);

I am really quite unhappy the VBT does not have these in a TLV
structure. :(

> +
> +		if (*data == 0)
> +			break; /* end of sequence reached */
> +	}
> +
> +	DRM_DEBUG_DRIVER("MIPI related vbt parsing complete\n");
>  }
>  
>  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index 162d0d2..78bee8e 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -867,4 +867,38 @@ struct bdb_mipi_sequence {
>  	u8 data[0];
>  };
>  
> +/* MIPI Sequnece Block definitions */
> +enum MIPI_SEQ {
> +	MIPI_SEQ_UNDEFINED = 0,
> +	MIPI_SEQ_ASSERT_RESET,
> +	MIPI_SEQ_INIT_OTP,
> +	MIPI_SEQ_DISPLAY_ON,
> +	MIPI_SEQ_DISPLAY_OFF,
> +	MIPI_SEQ_DEASSERT_RESET,
> +	MIPI_SEQ_MAX
> +
> +};
> +
> +enum MIPI_SEQ_ELEMENT {
> +	MIPI_SEQ_ELEM_UNDEFINED = 0,
> +	MIPI_SEQ_ELEM_SEND_PKT,
> +	MIPI_SEQ_ELEM_DELAY,
> +	MIPI_SEQ_ELEM_GPIO,
> +	MIPI_SEQ_ELEM_STATUS,
> +	MIPI_SEQ_ELEM_MAX
> +
> +};
> +
> +enum MIPI_GPIO_PIN_INDEX {
> +	MIPI_GPIO_UNDEFINED = 0,
> +	MIPI_GPIO_PANEL_ENABLE,
> +	MIPI_GPIO_BL_ENABLE,
> +	MIPI_GPIO_PWM_ENABLE,
> +	MIPI_GPIO_RESET_N,
> +	MIPI_GPIO_PWR_DOWN_R,
> +	MIPI_GPIO_STDBY_RST_N,
> +	MIPI_GPIO_MAX
> +
> +};
> +
>  #endif /* _I830_BIOS_H_ */
> -- 
> 1.8.3.2
>

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH v3] drm/i915: Add parsing support for new MIPI blocks in VBT

[Shobhit Kumar]

The parser extracts the config block(#52) and sequence(#53) data
and store in private data structures.

v2: Address review comments by Jani
    - adjust code for the structure changes for bdb_mipi_config
    - add boundry and buffer overflow checks as suggested
    - use kmemdup instead of kmalloc and memcpy

v3: More strict check while parsing VBT
    - Ensure that at anytime we do not go beyond sequence block
      while parsing
    - On unknown element fail the whole parsing

v4: Style changes and spell check mostly as suggested by Jani

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |   6 ++
 drivers/gpu/drm/i915/intel_bios.c | 204 +++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_bios.h |  31 ++++++
 3 files changed, 236 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 85e362f..1b763aa 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1170,6 +1170,12 @@ struct intel_vbt_data {
 	/* MIPI DSI */
 	struct {
 		u16 panel_id;
+		struct mipi_config *config;
+		struct mipi_pps_data *pps;
+		u8 seq_version;
+		u32 size;
+		u8 *data;
+		u8 *sequence[MIPI_SEQ_MAX];
 	} dsi;
 
 	int crt_ddc_pin;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 862ca04..917f5bb 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -636,19 +636,213 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
 	}
 }
 
+static u8 *goto_next_sequence(u8 *data, int *size)
+{
+	u16 len;
+	int tmp = *size;
+
+	if (--tmp < 0)
+		return NULL;
+
+	/* goto first element */
+	data++;
+	while (1) {
+		switch (*data) {
+		case MIPI_SEQ_ELEM_SEND_PKT:
+			/*
+			 * skip by this element payload size
+			 * skip elem id, command flag and data type
+			 */
+			if ((tmp = tmp - 5) < 0)
+				return NULL;
+
+			data += 3;
+			len = *((u16 *)data);
+
+			if ((tmp = tmp - len) < 0)
+				return NULL;
+
+			/* skip by len */
+			data = data + 2 + len;
+			break;
+		case MIPI_SEQ_ELEM_DELAY:
+			/* skip by elem id, and delay is 4 bytes */
+			if ((tmp = tmp - 5) < 0)
+				return NULL;
+
+			data += 5;
+			break;
+		case MIPI_SEQ_ELEM_GPIO:
+			if ((tmp = tmp - 3) < 0)
+				return NULL;
+
+			data += 3;
+			break;
+		default:
+			DRM_ERROR("Unknown element\n");
+			return NULL;
+		}
+
+		/* end of sequence ? */
+		if (*data == 0)
+			break;
+	}
+
+	/* goto next sequence or end of block byte */
+	if (--tmp < 0)
+		return NULL;
+
+	data++;
+
+	/* update amount of data left for the sequence block to be parsed */
+	*size = tmp;
+	return data;
+}
+
 static void
 parse_mipi(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
 {
-	struct bdb_mipi *mipi;
+	struct bdb_mipi_config *start;
+	struct bdb_mipi_sequence *sequence;
+	struct mipi_config *config;
+	struct mipi_pps_data *pps;
+	u8 *data, *seq_data;
+	int i, panel_id, seq_size;
+	u16 block_size;
+
+	/* Initialize this to undefined indicating no generic MIPI support */
+	dev_priv->vbt.dsi.panel_id = MIPI_DSI_UNDEFINED_PANEL_ID;
+
+	/* Block #40 is already parsed and panel_fixed_mode is
+	 * stored in dev_priv->lfp_lvds_vbt_mode
+	 * resuse this when needed
+	 */
 
-	mipi = find_section(bdb, BDB_MIPI_CONFIG);
-	if (!mipi) {
-		DRM_DEBUG_KMS("No MIPI BDB found");
+	/* Parse #52 for panel index used from panel_type already
+	 * parsed
+	 */
+	start = find_section(bdb, BDB_MIPI_CONFIG);
+	if (!start) {
+		DRM_DEBUG_KMS("No MIPI config BDB found");
 		return;
 	}
 
-	/* XXX: add more info */
+	DRM_DEBUG_DRIVER("Found MIPI Config block, panel index = %d\n",
+								panel_type);
+
+	/*
+	 * get hold of the correct configuration block and pps data as per
+	 * the panel_type as index
+	 */
+	config = &start->config[panel_type];
+	pps = &start->pps[panel_type];
+
+	/* store as of now full data. Trim when we realise all is not needed */
+	dev_priv->vbt.dsi.config = kmemdup(config, sizeof(struct mipi_config), GFP_KERNEL);
+	if (!dev_priv->vbt.dsi.config)
+		return;
+
+	dev_priv->vbt.dsi.pps = kmemdup(pps, sizeof(struct mipi_pps_data), GFP_KERNEL);
+	if (!dev_priv->vbt.dsi.pps) {
+		kfree(dev_priv->vbt.dsi.config);
+		return;
+	}
+
+	/* We have mandatory mipi config blocks. Initialize as generic panel */
 	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
+
+	/* Check if we have sequence block as well */
+	sequence = find_section(bdb, BDB_MIPI_SEQUENCE);
+	if (!sequence) {
+		DRM_DEBUG_KMS("No MIPI Sequence found, parsing complete\n");
+		return;
+	}
+
+	DRM_DEBUG_DRIVER("Found MIPI sequence block\n");
+
+	block_size = get_blocksize(sequence);
+
+	/*
+	 * parse the sequence block for individual sequences
+	 */
+	dev_priv->vbt.dsi.seq_version = sequence->version;
+
+	seq_data = &sequence->data[0];
+
+	/*
+	 * sequence block is variable length and hence we need to parse and
+	 * get the sequence data for specific panel id
+	 */
+	for (i = 0; i < MAX_MIPI_CONFIGURATIONS; i++) {
+		panel_id = *seq_data;
+		seq_size = *((u16 *) (seq_data + 1));
+		if (panel_id == panel_type)
+			break;
+
+		/* skip the sequence including seq header of 3 bytes */
+		seq_data = seq_data + 3 + seq_size;
+		if ((seq_data - &sequence->data[0]) > block_size) {
+			DRM_ERROR("Sequence start is beyond sequence block size, corrupted sequence block\n");
+			return;
+		}
+	}
+
+	if (i == MAX_MIPI_CONFIGURATIONS) {
+		DRM_ERROR("Sequence block detected but no valid configuration\n");
+		return;
+	}
+
+	/* check if found sequence is completely within the sequence block
+	 * just being paranoid */
+	if (seq_size > block_size) {
+		DRM_ERROR("Corrupted sequence/size, bailing out\n");
+		return;
+	}
+
+	/* skip the panel id(1 byte) and seq size(2 bytes) */
+	dev_priv->vbt.dsi.data = kmemdup(seq_data + 3, seq_size, GFP_KERNEL);
+	if (!dev_priv->vbt.dsi.data)
+		return;
+
+	/*
+	 * loop into the sequence data and split into multiple sequneces
+	 * There are only 5 types of sequences as of now
+	 */
+	data = dev_priv->vbt.dsi.data;
+	dev_priv->vbt.dsi.size = seq_size;
+
+	/* two consecutive 0x00 indicate end of all sequences */
+	while (1) {
+		int seq_id = *data;
+		if (MIPI_SEQ_MAX > seq_id && seq_id > MIPI_SEQ_UNDEFINED) {
+			dev_priv->vbt.dsi.sequence[seq_id] = data;
+			DRM_DEBUG_DRIVER("Found mipi sequence - %d\n", seq_id);
+		} else {
+			DRM_ERROR("undefined sequence\n");
+			goto err;
+		}
+
+		/* partial parsing to skip elements */
+		data = goto_next_sequence(data, &seq_size);
+
+		if (data == NULL) {
+			DRM_ERROR("Sequence elements going beyond block itself. Sequence block parsing failed\n");
+			goto err;
+		}
+
+		if (*data == 0)
+			break; /* end of sequence reached */
+	}
+
+	DRM_DEBUG_DRIVER("MIPI related vbt parsing complete\n");
+	return;
+err:
+	kfree(dev_priv->vbt.dsi.data);
+	dev_priv->vbt.dsi.data = NULL;
+
+	/* error during parsing so set all pointers to null
+	 * because of partial parsing */
+	memset(dev_priv->vbt.dsi.sequence, 0, MIPI_SEQ_MAX);
 }
 
 static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index 036a799..6009deb 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -899,4 +899,35 @@ struct bdb_mipi_sequence {
 	u8 data[0];
 };
 
+/* MIPI Sequnece Block definitions */
+enum mipi_seq {
+	MIPI_SEQ_UNDEFINED = 0,
+	MIPI_SEQ_ASSERT_RESET,
+	MIPI_SEQ_INIT_OTP,
+	MIPI_SEQ_DISPLAY_ON,
+	MIPI_SEQ_DISPLAY_OFF,
+	MIPI_SEQ_DEASSERT_RESET,
+	MIPI_SEQ_MAX
+};
+
+enum mipi_seq_element {
+	MIPI_SEQ_ELEM_UNDEFINED = 0,
+	MIPI_SEQ_ELEM_SEND_PKT,
+	MIPI_SEQ_ELEM_DELAY,
+	MIPI_SEQ_ELEM_GPIO,
+	MIPI_SEQ_ELEM_STATUS,
+	MIPI_SEQ_ELEM_MAX
+};
+
+enum mipi_gpio_pin_index {
+	MIPI_GPIO_UNDEFINED = 0,
+	MIPI_GPIO_PANEL_ENABLE,
+	MIPI_GPIO_BL_ENABLE,
+	MIPI_GPIO_PWM_ENABLE,
+	MIPI_GPIO_RESET_N,
+	MIPI_GPIO_PWR_DOWN_R,
+	MIPI_GPIO_STDBY_RST_N,
+	MIPI_GPIO_MAX
+};
+
 #endif /* _I830_BIOS_H_ */
-- 
1.8.3.2

Re: [PATCH v3] drm/i915: Add parsing support for new MIPI blocks in VBT

[Daniel Vetter]

On Mon, Apr 14, 2014 at 11:00:34AM +0530, Shobhit Kumar wrote:
> The parser extracts the config block(#52) and sequence(#53) data
> and store in private data structures.
> 
> v2: Address review comments by Jani
>     - adjust code for the structure changes for bdb_mipi_config
>     - add boundry and buffer overflow checks as suggested
>     - use kmemdup instead of kmalloc and memcpy
> 
> v3: More strict check while parsing VBT
>     - Ensure that at anytime we do not go beyond sequence block
>       while parsing
>     - On unknown element fail the whole parsing
> 
> v4: Style changes and spell check mostly as suggested by Jani
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

I didn't spot Jani's r-b tag in earlier mails, was that done off-list?
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h   |   6 ++
>  drivers/gpu/drm/i915/intel_bios.c | 204 +++++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_bios.h |  31 ++++++
>  3 files changed, 236 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 85e362f..1b763aa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1170,6 +1170,12 @@ struct intel_vbt_data {
>  	/* MIPI DSI */
>  	struct {
>  		u16 panel_id;
> +		struct mipi_config *config;
> +		struct mipi_pps_data *pps;
> +		u8 seq_version;
> +		u32 size;
> +		u8 *data;
> +		u8 *sequence[MIPI_SEQ_MAX];
>  	} dsi;
>  
>  	int crt_ddc_pin;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 862ca04..917f5bb 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -636,19 +636,213 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>  	}
>  }
>  
> +static u8 *goto_next_sequence(u8 *data, int *size)
> +{
> +	u16 len;
> +	int tmp = *size;
> +
> +	if (--tmp < 0)
> +		return NULL;
> +
> +	/* goto first element */
> +	data++;
> +	while (1) {
> +		switch (*data) {
> +		case MIPI_SEQ_ELEM_SEND_PKT:
> +			/*
> +			 * skip by this element payload size
> +			 * skip elem id, command flag and data type
> +			 */
> +			if ((tmp = tmp - 5) < 0)
> +				return NULL;
> +
> +			data += 3;
> +			len = *((u16 *)data);
> +
> +			if ((tmp = tmp - len) < 0)
> +				return NULL;
> +
> +			/* skip by len */
> +			data = data + 2 + len;
> +			break;
> +		case MIPI_SEQ_ELEM_DELAY:
> +			/* skip by elem id, and delay is 4 bytes */
> +			if ((tmp = tmp - 5) < 0)
> +				return NULL;
> +
> +			data += 5;
> +			break;
> +		case MIPI_SEQ_ELEM_GPIO:
> +			if ((tmp = tmp - 3) < 0)
> +				return NULL;
> +
> +			data += 3;
> +			break;
> +		default:
> +			DRM_ERROR("Unknown element\n");
> +			return NULL;
> +		}
> +
> +		/* end of sequence ? */
> +		if (*data == 0)
> +			break;
> +	}
> +
> +	/* goto next sequence or end of block byte */
> +	if (--tmp < 0)
> +		return NULL;
> +
> +	data++;
> +
> +	/* update amount of data left for the sequence block to be parsed */
> +	*size = tmp;
> +	return data;
> +}
> +
>  static void
>  parse_mipi(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>  {
> -	struct bdb_mipi *mipi;
> +	struct bdb_mipi_config *start;
> +	struct bdb_mipi_sequence *sequence;
> +	struct mipi_config *config;
> +	struct mipi_pps_data *pps;
> +	u8 *data, *seq_data;
> +	int i, panel_id, seq_size;
> +	u16 block_size;
> +
> +	/* Initialize this to undefined indicating no generic MIPI support */
> +	dev_priv->vbt.dsi.panel_id = MIPI_DSI_UNDEFINED_PANEL_ID;
> +
> +	/* Block #40 is already parsed and panel_fixed_mode is
> +	 * stored in dev_priv->lfp_lvds_vbt_mode
> +	 * resuse this when needed
> +	 */
>  
> -	mipi = find_section(bdb, BDB_MIPI_CONFIG);
> -	if (!mipi) {
> -		DRM_DEBUG_KMS("No MIPI BDB found");
> +	/* Parse #52 for panel index used from panel_type already
> +	 * parsed
> +	 */
> +	start = find_section(bdb, BDB_MIPI_CONFIG);
> +	if (!start) {
> +		DRM_DEBUG_KMS("No MIPI config BDB found");
>  		return;
>  	}
>  
> -	/* XXX: add more info */
> +	DRM_DEBUG_DRIVER("Found MIPI Config block, panel index = %d\n",
> +								panel_type);
> +
> +	/*
> +	 * get hold of the correct configuration block and pps data as per
> +	 * the panel_type as index
> +	 */
> +	config = &start->config[panel_type];
> +	pps = &start->pps[panel_type];
> +
> +	/* store as of now full data. Trim when we realise all is not needed */
> +	dev_priv->vbt.dsi.config = kmemdup(config, sizeof(struct mipi_config), GFP_KERNEL);
> +	if (!dev_priv->vbt.dsi.config)
> +		return;
> +
> +	dev_priv->vbt.dsi.pps = kmemdup(pps, sizeof(struct mipi_pps_data), GFP_KERNEL);
> +	if (!dev_priv->vbt.dsi.pps) {
> +		kfree(dev_priv->vbt.dsi.config);
> +		return;
> +	}
> +
> +	/* We have mandatory mipi config blocks. Initialize as generic panel */
>  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
> +
> +	/* Check if we have sequence block as well */
> +	sequence = find_section(bdb, BDB_MIPI_SEQUENCE);
> +	if (!sequence) {
> +		DRM_DEBUG_KMS("No MIPI Sequence found, parsing complete\n");
> +		return;
> +	}
> +
> +	DRM_DEBUG_DRIVER("Found MIPI sequence block\n");
> +
> +	block_size = get_blocksize(sequence);
> +
> +	/*
> +	 * parse the sequence block for individual sequences
> +	 */
> +	dev_priv->vbt.dsi.seq_version = sequence->version;
> +
> +	seq_data = &sequence->data[0];
> +
> +	/*
> +	 * sequence block is variable length and hence we need to parse and
> +	 * get the sequence data for specific panel id
> +	 */
> +	for (i = 0; i < MAX_MIPI_CONFIGURATIONS; i++) {
> +		panel_id = *seq_data;
> +		seq_size = *((u16 *) (seq_data + 1));
> +		if (panel_id == panel_type)
> +			break;
> +
> +		/* skip the sequence including seq header of 3 bytes */
> +		seq_data = seq_data + 3 + seq_size;
> +		if ((seq_data - &sequence->data[0]) > block_size) {
> +			DRM_ERROR("Sequence start is beyond sequence block size, corrupted sequence block\n");
> +			return;
> +		}
> +	}
> +
> +	if (i == MAX_MIPI_CONFIGURATIONS) {
> +		DRM_ERROR("Sequence block detected but no valid configuration\n");
> +		return;
> +	}
> +
> +	/* check if found sequence is completely within the sequence block
> +	 * just being paranoid */
> +	if (seq_size > block_size) {
> +		DRM_ERROR("Corrupted sequence/size, bailing out\n");
> +		return;
> +	}
> +
> +	/* skip the panel id(1 byte) and seq size(2 bytes) */
> +	dev_priv->vbt.dsi.data = kmemdup(seq_data + 3, seq_size, GFP_KERNEL);
> +	if (!dev_priv->vbt.dsi.data)
> +		return;
> +
> +	/*
> +	 * loop into the sequence data and split into multiple sequneces
> +	 * There are only 5 types of sequences as of now
> +	 */
> +	data = dev_priv->vbt.dsi.data;
> +	dev_priv->vbt.dsi.size = seq_size;
> +
> +	/* two consecutive 0x00 indicate end of all sequences */
> +	while (1) {
> +		int seq_id = *data;
> +		if (MIPI_SEQ_MAX > seq_id && seq_id > MIPI_SEQ_UNDEFINED) {
> +			dev_priv->vbt.dsi.sequence[seq_id] = data;
> +			DRM_DEBUG_DRIVER("Found mipi sequence - %d\n", seq_id);
> +		} else {
> +			DRM_ERROR("undefined sequence\n");
> +			goto err;
> +		}
> +
> +		/* partial parsing to skip elements */
> +		data = goto_next_sequence(data, &seq_size);
> +
> +		if (data == NULL) {
> +			DRM_ERROR("Sequence elements going beyond block itself. Sequence block parsing failed\n");
> +			goto err;
> +		}
> +
> +		if (*data == 0)
> +			break; /* end of sequence reached */
> +	}
> +
> +	DRM_DEBUG_DRIVER("MIPI related vbt parsing complete\n");
> +	return;
> +err:
> +	kfree(dev_priv->vbt.dsi.data);
> +	dev_priv->vbt.dsi.data = NULL;
> +
> +	/* error during parsing so set all pointers to null
> +	 * because of partial parsing */
> +	memset(dev_priv->vbt.dsi.sequence, 0, MIPI_SEQ_MAX);
>  }
>  
>  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index 036a799..6009deb 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -899,4 +899,35 @@ struct bdb_mipi_sequence {
>  	u8 data[0];
>  };
>  
> +/* MIPI Sequnece Block definitions */
> +enum mipi_seq {
> +	MIPI_SEQ_UNDEFINED = 0,
> +	MIPI_SEQ_ASSERT_RESET,
> +	MIPI_SEQ_INIT_OTP,
> +	MIPI_SEQ_DISPLAY_ON,
> +	MIPI_SEQ_DISPLAY_OFF,
> +	MIPI_SEQ_DEASSERT_RESET,
> +	MIPI_SEQ_MAX
> +};
> +
> +enum mipi_seq_element {
> +	MIPI_SEQ_ELEM_UNDEFINED = 0,
> +	MIPI_SEQ_ELEM_SEND_PKT,
> +	MIPI_SEQ_ELEM_DELAY,
> +	MIPI_SEQ_ELEM_GPIO,
> +	MIPI_SEQ_ELEM_STATUS,
> +	MIPI_SEQ_ELEM_MAX
> +};
> +
> +enum mipi_gpio_pin_index {
> +	MIPI_GPIO_UNDEFINED = 0,
> +	MIPI_GPIO_PANEL_ENABLE,
> +	MIPI_GPIO_BL_ENABLE,
> +	MIPI_GPIO_PWM_ENABLE,
> +	MIPI_GPIO_RESET_N,
> +	MIPI_GPIO_PWR_DOWN_R,
> +	MIPI_GPIO_STDBY_RST_N,
> +	MIPI_GPIO_MAX
> +};
> +
>  #endif /* _I830_BIOS_H_ */
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH v3] drm/i915: Add parsing support for new MIPI blocks in VBT

[Kumar, Shobhit]

On 4/14/2014 12:32 PM, Daniel Vetter wrote:
> On Mon, Apr 14, 2014 at 11:00:34AM +0530, Shobhit Kumar wrote:
>> The parser extracts the config block(#52) and sequence(#53) data
>> and store in private data structures.
>>
>> v2: Address review comments by Jani
>>      - adjust code for the structure changes for bdb_mipi_config
>>      - add boundry and buffer overflow checks as suggested
>>      - use kmemdup instead of kmalloc and memcpy
>>
>> v3: More strict check while parsing VBT
>>      - Ensure that at anytime we do not go beyond sequence block
>>        while parsing
>>      - On unknown element fail the whole parsing
>>
>> v4: Style changes and spell check mostly as suggested by Jani
>>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
> I didn't spot Jani's r-b tag in earlier mails, was that done off-list?

Yeah, was reviewed along with the other DSI patchset you merged, sorry 
about that. But then some patches needed internal review while they were 
being approved for up-streaming to save time. And this one was related 
to the other panel driver patches which I published today so got stuck 
with them. Sorry about that.

Regards
Shobhit

Re: [PATCH v3] drm/i915: Add parsing support for new MIPI blocks in VBT

[Daniel Vetter]

On Mon, Apr 14, 2014 at 01:24:43PM +0530, Kumar, Shobhit wrote:
> On 4/14/2014 12:32 PM, Daniel Vetter wrote:
> >On Mon, Apr 14, 2014 at 11:00:34AM +0530, Shobhit Kumar wrote:
> >>The parser extracts the config block(#52) and sequence(#53) data
> >>and store in private data structures.
> >>
> >>v2: Address review comments by Jani
> >>     - adjust code for the structure changes for bdb_mipi_config
> >>     - add boundry and buffer overflow checks as suggested
> >>     - use kmemdup instead of kmalloc and memcpy
> >>
> >>v3: More strict check while parsing VBT
> >>     - Ensure that at anytime we do not go beyond sequence block
> >>       while parsing
> >>     - On unknown element fail the whole parsing
> >>
> >>v4: Style changes and spell check mostly as suggested by Jani
> >>
> >>Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> >>Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> >
> >I didn't spot Jani's r-b tag in earlier mails, was that done off-list?
> 
> Yeah, was reviewed along with the other DSI patchset you merged, sorry about
> that. But then some patches needed internal review while they were being
> approved for up-streaming to save time. And this one was related to the
> other panel driver patches which I published today so got stuck with them.
> Sorry about that.

Ok, pulled it in. checkpatch complained a few times about assignments in
if conditions, and I tend to agree. Can you please follow up with a
cleanup patch? Also it looks like assignment operators could be used.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH v3] drm/i915: Add parsing support for new MIPI blocks in VBT

[Kumar, Shobhit]

On 4/14/2014 2:35 PM, Daniel Vetter wrote:
> On Mon, Apr 14, 2014 at 01:24:43PM +0530, Kumar, Shobhit wrote:
>> On 4/14/2014 12:32 PM, Daniel Vetter wrote:
>>> On Mon, Apr 14, 2014 at 11:00:34AM +0530, Shobhit Kumar wrote:
>>>> The parser extracts the config block(#52) and sequence(#53) data
>>>> and store in private data structures.
>>>>
>>>> v2: Address review comments by Jani
>>>>      - adjust code for the structure changes for bdb_mipi_config
>>>>      - add boundry and buffer overflow checks as suggested
>>>>      - use kmemdup instead of kmalloc and memcpy
>>>>
>>>> v3: More strict check while parsing VBT
>>>>      - Ensure that at anytime we do not go beyond sequence block
>>>>        while parsing
>>>>      - On unknown element fail the whole parsing
>>>>
>>>> v4: Style changes and spell check mostly as suggested by Jani
>>>>
>>>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>>>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>>>
>>> I didn't spot Jani's r-b tag in earlier mails, was that done off-list?
>>
>> Yeah, was reviewed along with the other DSI patchset you merged, sorry about
>> that. But then some patches needed internal review while they were being
>> approved for up-streaming to save time. And this one was related to the
>> other panel driver patches which I published today so got stuck with them.
>> Sorry about that.
>
> Ok, pulled it in. checkpatch complained a few times about assignments in
> if conditions, and I tend to agree. Can you please follow up with a
> cleanup patch? Also it looks like assignment operators could be used.
> -Daniel

Ok. Thanks. I should have run checkpatch. Will fix and push a cleanup patch.

Regards
Shobhit

[PATCH] drm/i915: Code cleanup patch to fix checkpatch errors

[Shobhit Kumar]

This cleans up the checkpatch errors for the merged commit -

commit d3b542fcfc72d7724585e3fd2c5e75351bc3df47
Author: Shobhit Kumar <shobhit.kumar@intel.com>
Date:   Mon Apr 14 11:00:34 2014 +0530

    drm/i915: Add parsing support for new MIPI blocks in VBT

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 917f5bb..fba9efd 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -653,13 +653,15 @@ static u8 *goto_next_sequence(u8 *data, int *size)
 			 * skip by this element payload size
 			 * skip elem id, command flag and data type
 			 */
-			if ((tmp = tmp - 5) < 0)
+			tmp -= 5;
+			if (tmp < 0)
 				return NULL;
 
 			data += 3;
 			len = *((u16 *)data);
 
-			if ((tmp = tmp - len) < 0)
+			tmp -= len;
+			if (tmp < 0)
 				return NULL;
 
 			/* skip by len */
@@ -667,13 +669,15 @@ static u8 *goto_next_sequence(u8 *data, int *size)
 			break;
 		case MIPI_SEQ_ELEM_DELAY:
 			/* skip by elem id, and delay is 4 bytes */
-			if ((tmp = tmp - 5) < 0)
+			tmp -= 5;
+			if (tmp < 0)
 				return NULL;
 
 			data += 5;
 			break;
 		case MIPI_SEQ_ELEM_GPIO:
-			if ((tmp = tmp - 3) < 0)
+			tmp -= 3;
+			if (tmp < 0)
 				return NULL;
 
 			data += 3;
-- 
1.8.3.2

Re: [PATCH] drm/i915: Code cleanup patch to fix checkpatch errors

[Daniel Vetter]

On Tue, Apr 15, 2014 at 01:54:07PM +0530, Shobhit Kumar wrote:
> This cleans up the checkpatch errors for the merged commit -
> 
> commit d3b542fcfc72d7724585e3fd2c5e75351bc3df47
> Author: Shobhit Kumar <shobhit.kumar@intel.com>
> Date:   Mon Apr 14 11:00:34 2014 +0530
> 
>     drm/i915: Add parsing support for new MIPI blocks in VBT
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch