| Message ID | 1452006075-19826-1-git-send-email-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > --- > 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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++) { 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)); + 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);
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@intel.com> --- 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(-)