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
