Re: [PATCH v2] drm/i915/bios: add support for MIPI sequence block v3

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 05, 2016 at 05:01:15PM +0200, Jani Nikula wrote:
> The changes since the sequence block v2 are:
> 
> * The whole MIPI bios data block has a separate 32-bit size field since
>   v3, stored after the version. This facilitates big sequences.
> 
> * The size of the panel specific sequence blocks has grown to 32
>   bits. This facilitates big sequences.
> 
> * The elements within sequences now have an 8-bit size field following
>   the operation byte. This facilitates skipping unknown new operation
>   bytes, i.e. forward compatibility.
> 
> v2 (of the patch): use DRM_ERROR for unknown operation byte
> 
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/intel_bios.c          | 84 ++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |  9 ++++
>  2 files changed, 84 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 69040a73c09a..6c09b5643942 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -754,21 +754,30 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
>  /* Find the sequence block and size for the given panel. */
>  static const u8 *
>  find_panel_sequence_block(const struct bdb_mipi_sequence *sequence,
> -			  u16 panel_id, u16 *seq_size)
> +			  u16 panel_id, u32 *seq_size)
>  {
>  	u32 total = get_blocksize(sequence);
>  	const u8 *data = &sequence->data[0];
>  	u8 current_id;
> -	u16 current_size;
> +	u32 current_size;
>  	int index = 0;
>  	int i;
>  
> +	/* skip new block size */
> +	if (sequence->version >= 3)
> +		data += 4;
> +
>  	for (i = 0; i < MAX_MIPI_CONFIGURATIONS && index + 3 < total; i++) {

The index vs. total check should be updated too.

>  		current_id = *(data + index);
>  		index++;
>  
> -		current_size = *((const u16 *)(data + index));
> -		index += 2;
> +		if (sequence->version >= 3) {
> +			current_size = *((const u32 *)(data + index));
> +			index += 4;
> +		} else {
> +			current_size = *((const u16 *)(data + index));
> +			index += 2;
> +		}
>  
>  		if (index + current_size > total) {
>  			DRM_ERROR("Invalid sequence block\n");
> @@ -826,13 +835,66 @@ static int goto_next_sequence(const u8 *data, int index, int total)
>  	return 0;
>  }
>  
> +static int goto_next_sequence_v3(const u8 *data, int index, int total)
> +{
> +	int seq_end;
> +	u16 len;
> +
> +	/*
> +	 * Could skip sequence based on Size of Sequence alone, but also do some
> +	 * checking on the structure.
> +	 */
> +	seq_end = index + *((const u32 *)(data + 1));

Should we check total to make sure we don't access beyond what's
available?

> +	if (seq_end > total) {
> +		DRM_ERROR("Invalid sequence size\n");
> +		return 0;
> +	}
> +
> +	/* Skip Sequence Byte and Size of Sequence. */
> +	for (index = index + 5; index < total; index += len) {
> +		u8 operation_byte = *(data + index);
> +		index++;
> +
> +		if (operation_byte == MIPI_SEQ_ELEM_END) {
> +			if (index != seq_end) {
> +				DRM_ERROR("Invalid element structure\n");
> +				return 0;
> +			}
> +			return index;
> +		}
> +
> +		len = *(data + index);
> +		index++;
> +
> +		/*
> +		 * FIXME: Would be nice to check elements like for v1/v2 in
> +		 * goto_next_sequence() above.
> +		 */
> +		switch (operation_byte) {
> +		case MIPI_SEQ_ELEM_SEND_PKT:
> +		case MIPI_SEQ_ELEM_DELAY:
> +		case MIPI_SEQ_ELEM_GPIO:
> +		case MIPI_SEQ_ELEM_I2C:
> +		case MIPI_SEQ_ELEM_SPI:
> +		case MIPI_SEQ_ELEM_PMIC:
> +			break;
> +		default:
> +			DRM_ERROR("Unknown operation byte %u\n",
> +				  operation_byte);
> +			break;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static void
>  parse_mipi_sequence(struct drm_i915_private *dev_priv,
>  		    const struct bdb_header *bdb)
>  {
>  	const struct bdb_mipi_sequence *sequence;
>  	const u8 *seq_data;
> -	u16 seq_size;
> +	u32 seq_size;
>  	u8 *data;
>  	int index = 0;
>  
> @@ -847,12 +909,13 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
>  	}
>  
>  	/* Fail gracefully for forward incompatible sequence block. */
> -	if (sequence->version >= 3) {
> -		DRM_ERROR("Unable to parse MIPI Sequence Block v3+\n");
> +	if (sequence->version >= 4) {
> +		DRM_ERROR("Unable to parse MIPI Sequence Block v%u\n",
> +			  sequence->version);
>  		return;
>  	}
>  
> -	DRM_DEBUG_DRIVER("Found MIPI sequence block\n");
> +	DRM_DEBUG_DRIVER("Found MIPI sequence block v%u\n", sequence->version);
>  
>  	seq_data = find_panel_sequence_block(sequence, panel_type, &seq_size);
>  	if (!seq_data)
> @@ -875,7 +938,10 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
>  
>  		dev_priv->vbt.dsi.sequence[seq_id] = data + index;
>  
> -		index = goto_next_sequence(data, index, seq_size);
> +		if (sequence->version >= 3)
> +			index = goto_next_sequence_v3(data, index, seq_size);
> +		else
> +			index = goto_next_sequence(data, index, seq_size);
>  		if (!index) {
>  			DRM_ERROR("Invalid sequence %u\n", seq_id);
>  			goto err;
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 5303d1c446d5..7f67749eb0ef 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -319,6 +319,7 @@ static const char *sequence_name(enum mipi_seq seq_id)
>  
>  static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8 *data)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
>  	fn_mipi_elem_exec mipi_elem_exec;
>  
>  	if (!data)
> @@ -330,6 +331,10 @@ static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8 *data)
>  	/* go to the first element of the sequence */
>  	data++;
>  
> +	/* Skip Size of Sequence. */
> +	if (dev_priv->vbt.dsi.seq_version >= 3)
> +		data += 4;
> +
>  	/* parse each byte till we reach end of sequence byte - 0x00 */
>  	while (1) {
>  		u8 operation_byte = *data++;
> @@ -341,6 +346,10 @@ static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8 *data)
>  		}
>  		mipi_elem_exec = exec_elem[operation_byte];
>  
> +		/* Skip Size of Operation. */
> +		if (dev_priv->vbt.dsi.seq_version >= 3)
> +			data++;
> +
>  		/* execute the element specific rotines */
>  		data = mipi_elem_exec(intel_dsi, data);
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux