[Intel-gfx] [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 at 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 at 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