diff mbox

[06/15] drm/i915/bios: rewrite sequence block parsing

Message ID e8f2a62d78d90981a6b49fdf9ab3594f60a46033.1450702954.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jani Nikula Dec. 21, 2015, 1:10 p.m. UTC
Make everything a bit more readable and clear.

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

Comments

Daniel Vetter Jan. 5, 2016, 2:12 p.m. UTC | #1
On Mon, Dec 21, 2015 at 03:10:57PM +0200, Jani Nikula wrote:
> Make everything a bit more readable and clear.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

A real pain to check, but I think I convinced myself that this is
equivalent code.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>


> ---
>  drivers/gpu/drm/i915/i915_drv.h   |   2 +-
>  drivers/gpu/drm/i915/intel_bios.c | 158 +++++++++++++-------------------------
>  drivers/gpu/drm/i915/intel_bios.h |   4 +-
>  3 files changed, 57 insertions(+), 107 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9b82c4532893..07c4fc539680 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1487,7 +1487,7 @@ struct intel_vbt_data {
>  		u8 seq_version;
>  		u32 size;
>  		u8 *data;
> -		u8 *sequence[MIPI_SEQ_MAX];
> +		const 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 9511a5341562..d6eaf32f33e5 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -697,73 +697,6 @@ parse_psr(struct drm_i915_private *dev_priv, const struct bdb_header *bdb)
>  	dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
>  }
>  
> -static u8 *goto_next_sequence(u8 *data, u16 *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
> -			 */
> -			tmp -= 5;
> -			if (tmp < 0)
> -				return NULL;
> -
> -			data += 3;
> -			len = *((u16 *)data);
> -
> -			tmp -= len;
> -			if (tmp < 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 */
> -			tmp -= 5;
> -			if (tmp < 0)
> -				return NULL;
> -
> -			data += 5;
> -			break;
> -		case MIPI_SEQ_ELEM_GPIO:
> -			tmp -= 3;
> -			if (tmp < 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_config(struct drm_i915_private *dev_priv,
>  		  const struct bdb_header *bdb)
> @@ -855,6 +788,39 @@ find_panel_sequence_block(const struct bdb_mipi_sequence *sequence,
>  	return NULL;
>  }
>  
> +static int goto_next_sequence(const u8 *data, int index, int total)
> +{
> +	u16 len;
> +
> +	/* Skip Sequence Byte. */
> +	for (index = index + 1; index < total; index += len) {
> +		u8 operation_byte = *(data + index);
> +		index++;
> +
> +		switch (operation_byte) {
> +		case MIPI_SEQ_ELEM_END:
> +			return index;
> +		case MIPI_SEQ_ELEM_SEND_PKT:
> +			if (index + 4 > total)
> +				return 0;
> +
> +			len = *((const u16 *)(data + index + 2)) + 4;
> +			break;
> +		case MIPI_SEQ_ELEM_DELAY:
> +			len = 4;
> +			break;
> +		case MIPI_SEQ_ELEM_GPIO:
> +			len = 2;
> +			break;
> +		default:
> +			DRM_ERROR("Unknown operation byte\n");
> +			return 0;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static void
>  parse_mipi_sequence(struct drm_i915_private *dev_priv,
>  		    const struct bdb_header *bdb)
> @@ -863,7 +829,7 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
>  	const u8 *seq_data;
>  	u16 seq_size;
>  	u8 *data;
> -	u16 block_size;
> +	int index = 0;
>  
>  	/* Only our generic panel driver uses the sequence block. */
>  	if (dev_priv->vbt.dsi.panel_id != MIPI_DSI_GENERIC_PANEL_ID)
> @@ -883,59 +849,43 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
>  
>  	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 = find_panel_sequence_block(sequence, panel_type, &seq_size);
>  	if (!seq_data)
>  		return;
>  
> -	dev_priv->vbt.dsi.data = kmemdup(seq_data, seq_size, GFP_KERNEL);
> -	if (!dev_priv->vbt.dsi.data)
> +	data = kmemdup(seq_data, seq_size, GFP_KERNEL);
> +	if (!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;
> +	/* Parse the sequences, store pointers to each sequence. */
> +	for (;;) {
> +		u8 seq_id = *(data + index);
> +		if (seq_id == MIPI_SEQ_END)
> +			break;
>  
> -	/* 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");
> +		if (seq_id >= MIPI_SEQ_MAX) {
> +			DRM_ERROR("Unknown sequence %u\n", seq_id);
>  			goto err;
>  		}
>  
> -		/* partial parsing to skip elements */
> -		data = goto_next_sequence(data, &seq_size);
> +		dev_priv->vbt.dsi.sequence[seq_id] = data + index;
>  
> -		if (data == NULL) {
> -			DRM_ERROR("Sequence elements going beyond block itself. Sequence block parsing failed\n");
> +		index = goto_next_sequence(data, index, seq_size);
> +		if (!index) {
> +			DRM_ERROR("Invalid sequence %u\n", seq_id);
>  			goto err;
>  		}
> -
> -		if (*data == 0)
> -			break; /* end of sequence reached */
>  	}
>  
> -	DRM_DEBUG_DRIVER("MIPI related vbt parsing complete\n");
> +	dev_priv->vbt.dsi.data = data;
> +	dev_priv->vbt.dsi.size = seq_size;
> +	dev_priv->vbt.dsi.seq_version = sequence->version;
> +
> +	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 */
> +err:
> +	kfree(data);
>  	memset(dev_priv->vbt.dsi.sequence, 0, sizeof(dev_priv->vbt.dsi.sequence));
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index 21c162e01189..4e87df16e7b3 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -954,7 +954,7 @@ struct bdb_mipi_sequence {
>  
>  /* MIPI Sequnece Block definitions */
>  enum mipi_seq {
> -	MIPI_SEQ_UNDEFINED = 0,
> +	MIPI_SEQ_END = 0,
>  	MIPI_SEQ_ASSERT_RESET,
>  	MIPI_SEQ_INIT_OTP,
>  	MIPI_SEQ_DISPLAY_ON,
> @@ -964,7 +964,7 @@ enum mipi_seq {
>  };
>  
>  enum mipi_seq_element {
> -	MIPI_SEQ_ELEM_UNDEFINED = 0,
> +	MIPI_SEQ_ELEM_END = 0,
>  	MIPI_SEQ_ELEM_SEND_PKT,
>  	MIPI_SEQ_ELEM_DELAY,
>  	MIPI_SEQ_ELEM_GPIO,
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9b82c4532893..07c4fc539680 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1487,7 +1487,7 @@  struct intel_vbt_data {
 		u8 seq_version;
 		u32 size;
 		u8 *data;
-		u8 *sequence[MIPI_SEQ_MAX];
+		const 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 9511a5341562..d6eaf32f33e5 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -697,73 +697,6 @@  parse_psr(struct drm_i915_private *dev_priv, const struct bdb_header *bdb)
 	dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
 }
 
-static u8 *goto_next_sequence(u8 *data, u16 *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
-			 */
-			tmp -= 5;
-			if (tmp < 0)
-				return NULL;
-
-			data += 3;
-			len = *((u16 *)data);
-
-			tmp -= len;
-			if (tmp < 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 */
-			tmp -= 5;
-			if (tmp < 0)
-				return NULL;
-
-			data += 5;
-			break;
-		case MIPI_SEQ_ELEM_GPIO:
-			tmp -= 3;
-			if (tmp < 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_config(struct drm_i915_private *dev_priv,
 		  const struct bdb_header *bdb)
@@ -855,6 +788,39 @@  find_panel_sequence_block(const struct bdb_mipi_sequence *sequence,
 	return NULL;
 }
 
+static int goto_next_sequence(const u8 *data, int index, int total)
+{
+	u16 len;
+
+	/* Skip Sequence Byte. */
+	for (index = index + 1; index < total; index += len) {
+		u8 operation_byte = *(data + index);
+		index++;
+
+		switch (operation_byte) {
+		case MIPI_SEQ_ELEM_END:
+			return index;
+		case MIPI_SEQ_ELEM_SEND_PKT:
+			if (index + 4 > total)
+				return 0;
+
+			len = *((const u16 *)(data + index + 2)) + 4;
+			break;
+		case MIPI_SEQ_ELEM_DELAY:
+			len = 4;
+			break;
+		case MIPI_SEQ_ELEM_GPIO:
+			len = 2;
+			break;
+		default:
+			DRM_ERROR("Unknown operation byte\n");
+			return 0;
+		}
+	}
+
+	return 0;
+}
+
 static void
 parse_mipi_sequence(struct drm_i915_private *dev_priv,
 		    const struct bdb_header *bdb)
@@ -863,7 +829,7 @@  parse_mipi_sequence(struct drm_i915_private *dev_priv,
 	const u8 *seq_data;
 	u16 seq_size;
 	u8 *data;
-	u16 block_size;
+	int index = 0;
 
 	/* Only our generic panel driver uses the sequence block. */
 	if (dev_priv->vbt.dsi.panel_id != MIPI_DSI_GENERIC_PANEL_ID)
@@ -883,59 +849,43 @@  parse_mipi_sequence(struct drm_i915_private *dev_priv,
 
 	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 = find_panel_sequence_block(sequence, panel_type, &seq_size);
 	if (!seq_data)
 		return;
 
-	dev_priv->vbt.dsi.data = kmemdup(seq_data, seq_size, GFP_KERNEL);
-	if (!dev_priv->vbt.dsi.data)
+	data = kmemdup(seq_data, seq_size, GFP_KERNEL);
+	if (!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;
+	/* Parse the sequences, store pointers to each sequence. */
+	for (;;) {
+		u8 seq_id = *(data + index);
+		if (seq_id == MIPI_SEQ_END)
+			break;
 
-	/* 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");
+		if (seq_id >= MIPI_SEQ_MAX) {
+			DRM_ERROR("Unknown sequence %u\n", seq_id);
 			goto err;
 		}
 
-		/* partial parsing to skip elements */
-		data = goto_next_sequence(data, &seq_size);
+		dev_priv->vbt.dsi.sequence[seq_id] = data + index;
 
-		if (data == NULL) {
-			DRM_ERROR("Sequence elements going beyond block itself. Sequence block parsing failed\n");
+		index = goto_next_sequence(data, index, seq_size);
+		if (!index) {
+			DRM_ERROR("Invalid sequence %u\n", seq_id);
 			goto err;
 		}
-
-		if (*data == 0)
-			break; /* end of sequence reached */
 	}
 
-	DRM_DEBUG_DRIVER("MIPI related vbt parsing complete\n");
+	dev_priv->vbt.dsi.data = data;
+	dev_priv->vbt.dsi.size = seq_size;
+	dev_priv->vbt.dsi.seq_version = sequence->version;
+
+	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 */
+err:
+	kfree(data);
 	memset(dev_priv->vbt.dsi.sequence, 0, sizeof(dev_priv->vbt.dsi.sequence));
 }
 
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index 21c162e01189..4e87df16e7b3 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -954,7 +954,7 @@  struct bdb_mipi_sequence {
 
 /* MIPI Sequnece Block definitions */
 enum mipi_seq {
-	MIPI_SEQ_UNDEFINED = 0,
+	MIPI_SEQ_END = 0,
 	MIPI_SEQ_ASSERT_RESET,
 	MIPI_SEQ_INIT_OTP,
 	MIPI_SEQ_DISPLAY_ON,
@@ -964,7 +964,7 @@  enum mipi_seq {
 };
 
 enum mipi_seq_element {
-	MIPI_SEQ_ELEM_UNDEFINED = 0,
+	MIPI_SEQ_ELEM_END = 0,
 	MIPI_SEQ_ELEM_SEND_PKT,
 	MIPI_SEQ_ELEM_DELAY,
 	MIPI_SEQ_ELEM_GPIO,