| Message ID | 1441841070-11532-6-git-send-email-m.deepak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 10 Sep 2015, Deepak M <m.deepak@intel.com> wrote: > From: vkorjani <vikas.korjani@intel.com> > > The Block 53 of the VBT, which is the MIPI sequence block > has undergone a design change because of which the parsing > logic has to be changed. > > The current code will handle the parsing of v3 and other > lower versions of the MIPI sequence block. > > v2: rebase > > Signed-off-by: vkorjani <vikas.korjani@intel.com> > Signed-off-by: Deepak M <m.deepak@intel.com> > --- > drivers/gpu/drm/i915/intel_bios.c | 119 +++++++++++++++++++++++----- > drivers/gpu/drm/i915/intel_bios.h | 8 ++ > drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 7 ++ > 3 files changed, 114 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index 34a1042..cea641f 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -45,6 +45,7 @@ find_section(struct drm_i915_private *dev_priv, > int index = 0; > u32 total, current_size; > u8 current_id; > + u8 version; > > /* skip to first section */ > index += bdb->header_size; > @@ -56,7 +57,17 @@ find_section(struct drm_i915_private *dev_priv, > current_id = *(base + index); > index++; > > - current_size = *((const u16 *)(base + index)); > + if (current_id == BDB_MIPI_SEQUENCE) { > + version = *(base + index + 2); > + if (version >= 3) > + current_size = *((const u32 *)(base + > + index + 3)); > + else > + current_size = *((const u16 *)(base + index)); > + } else { > + current_size = *((const u16 *)(base + index)); > + } > + While reviewing I've realized the old kernels will hit this hard. I've submitted a patch [1] to be applied to v4.3-rc and older stable kernels so that they fail gracefully instead of starting to parse garbage. The real parsing is too big to backport to upstream stable. Please review. [1] http://mid.gmane.org/1442497327-27033-1-git-send-email-jani.nikula@intel.com > index += 2; > > if (index + current_size > total) > @@ -745,6 +756,55 @@ static u8 *goto_next_sequence(u8 *data, int *size) > return data; > } > > +static u8 *goto_next_sequence_v3(u8 *data, int *size) > +{ > + int tmp = *size; > + int op_size; > + > + if (--tmp < 0) > + return NULL; > + > + /* Skip the panel id and the sequence size */ It's not panel id, it's the sequence byte, right? You could also store data + 1 + size of sequence, and check whether data ends up pointing at the same place in the end. They should. Shouldn't you also take 4 bytes of sequence size field into account in tmp? > + data = data + 5; > + while (*data != 0) { > + u8 element_type = *data++; > + > + switch (element_type) { Would be helpful to refer to operation_byte like in the spec. > + default: > + DRM_ERROR("Unknown element type %d\n", element_type); > + 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: > + /* > + * skip by this element payload size > + * skip elem id, command flag and data type > + */ > + op_size = *data++; > + tmp = tmp - (op_size + 1); > + if (tmp < 0) > + return NULL; Isn't each operation operation byte | size of operation | payload size, i.e. your tmp change is one byte short? The fact that the goto_next_sequence* functions increase data and decrease size is getting increasingly confusing to follow. One simple alternative would be to calculate some endp = start + size up front, and then pass the endp around, and check if we're about to go past the end marker. This is not a problem with your series, it was there already. And fixing it doesn't have to be part of your series. It just really takes ages to review this approach of range checking. Unless I close my eyes and trust there are no off-by-ones anywhere. But that kind of defeats the purpose of review... > + > + /* skip by len */ > + data += op_size; > + break; > + } > + } > + > + /* goto next sequence or end of block byte */ > + if (--tmp < 0) > + return NULL; > + > + /* Skip the end element marker */ > + data++; > + > + /* 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, const struct bdb_header *bdb) > { > @@ -754,7 +814,7 @@ parse_mipi(struct drm_i915_private *dev_priv, const struct bdb_header *bdb) > const struct mipi_pps_data *pps; > u8 *data; > const u8 *seq_data; > - int i, panel_id, seq_size; > + int i, panel_id, panel_seq_size; > u16 block_size; > > /* parse MIPI blocks only if LFP type is MIPI */ > @@ -811,29 +871,40 @@ parse_mipi(struct drm_i915_private *dev_priv, const struct bdb_header *bdb) > > 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]; > + if (dev_priv->vbt.dsi.seq_version >= 3) { > + block_size = *((unsigned int *)seq_data); (const u32 *) > + seq_data = seq_data + 4; > + } else > + block_size = get_blocksize(sequence); block_size should be changed to u32. > > /* > * sequence block is variable length and hence we need to parse and > * get the sequence data for specific panel id > */ > for (i = 0; i < MAX_MIPI_CONFIGURATIONS; i++) { > - panel_id = *seq_data; > - seq_size = *((u16 *) (seq_data + 1)); > + panel_id = *seq_data++; > + if (dev_priv->vbt.dsi.seq_version >= 3) { > + panel_seq_size = *((u32 *)seq_data); > + seq_data += sizeof(u32); > + } else { > + panel_seq_size = *((u16 *)seq_data); > + seq_data += sizeof(u16); > + } > + > if (panel_id == panel_type) > break; > > - /* skip the sequence including seq header of 3 bytes */ > - seq_data = seq_data + 3 + seq_size; > + seq_data += panel_seq_size; > + > if ((seq_data - &sequence->data[0]) > block_size) { > - DRM_ERROR("Sequence start is beyond sequence block size, corrupted sequence block\n"); > + DRM_ERROR("Sequence start is beyond seq block size\n"); > + DRM_ERROR("Corrupted sequence block\n"); Please don't add two consecutive DRM_ERRORs for the same error. > return; > } > } > @@ -845,13 +916,14 @@ parse_mipi(struct drm_i915_private *dev_priv, const struct bdb_header *bdb) > > /* check if found sequence is completely within the sequence block > * just being paranoid */ > - if (seq_size > block_size) { > + if (panel_seq_size > block_size) { > DRM_ERROR("Corrupted sequence/size, bailing out\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); > + > + dev_priv->vbt.dsi.data = kmemdup(seq_data, panel_seq_size, GFP_KERNEL); > + > if (!dev_priv->vbt.dsi.data) > return; > > @@ -860,29 +932,36 @@ parse_mipi(struct drm_i915_private *dev_priv, const struct bdb_header *bdb) > * There are only 5 types of sequences as of now > */ > data = dev_priv->vbt.dsi.data; > - dev_priv->vbt.dsi.size = seq_size; > + dev_priv->vbt.dsi.size = panel_seq_size; > > /* two consecutive 0x00 indicate end of all sequences */ > - while (1) { > + while (*data != 0) { > int seq_id = *data; > + int seq_size; u32 > + > 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"); > - goto err; > + DRM_ERROR("undefined sequence - %d\n", seq_id); > + seq_size = *(data + 1); Needs to be *((const u32 *)(data + 1)) or you'll ignore 3 highest order bytes. > + if (dev_priv->vbt.dsi.seq_version >= 3) { > + data = data + seq_size + 1; > + continue; > + } else > + goto err; > } > > /* partial parsing to skip elements */ > - data = goto_next_sequence(data, &seq_size); > + if (dev_priv->vbt.dsi.seq_version >= 3) > + data = goto_next_sequence_v3(data, &panel_seq_size); > + else > + data = goto_next_sequence(data, &panel_seq_size); > > if (data == NULL) { > DRM_ERROR("Sequence elements going beyond block itself. Sequence block parsing failed\n"); > goto err; > } > - > - if (*data == 0) > - break; /* end of sequence reached */ > } > > DRM_DEBUG_DRIVER("MIPI related vbt parsing complete\n"); > diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h > index 21a7f3f..7a4ba41 100644 > --- a/drivers/gpu/drm/i915/intel_bios.h > +++ b/drivers/gpu/drm/i915/intel_bios.h > @@ -948,6 +948,12 @@ enum mipi_seq { > MIPI_SEQ_DISPLAY_ON, > MIPI_SEQ_DISPLAY_OFF, > MIPI_SEQ_DEASSERT_RESET, > + MIPI_SEQ_BACKLIGHT_ON, > + MIPI_SEQ_BACKLIGHT_OFF, > + MIPI_SEQ_TEAR_ON, > + MIPI_SEQ_TEAR_OFF, > + MIPI_SEQ_POWER_ON, > + MIPI_SEQ_POWER_OFF, > MIPI_SEQ_MAX > }; > > @@ -957,6 +963,8 @@ enum mipi_seq_element { > MIPI_SEQ_ELEM_DELAY, > MIPI_SEQ_ELEM_GPIO, > MIPI_SEQ_ELEM_I2C, > + MIPI_SEQ_ELEM_SPI, > + MIPI_SEQ_ELEM_PMIC, > MIPI_SEQ_ELEM_STATUS, Again, MIPI_SEQ_ELEM_STATUS is not spec. > MIPI_SEQ_ELEM_MAX > }; > diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > index 9989f61..c6a6fa1 100644 > --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > @@ -317,6 +317,8 @@ static const char * const seq_name[] = { > > static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8 *data) > { > + struct drm_device *dev = intel_dsi->base.base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > fn_mipi_elem_exec mipi_elem_exec; > int index; > > @@ -327,6 +329,8 @@ static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8 *data) > > /* go to the first element of the sequence */ > data++; > + if (dev_priv->vbt.dsi.seq_version >= 3) > + data = data + 4; > > /* parse each byte till we reach end of sequence byte - 0x00 */ > while (1) { > @@ -340,6 +344,9 @@ static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8 *data) > /* goto element payload */ > data++; > > + if (dev_priv->vbt.dsi.seq_version >= 3) > + data++; > + > /* execute the element specific rotines */ > data = mipi_elem_exec(intel_dsi, data); > > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>-----Original Message----- >From: Jani Nikula [mailto:jani.nikula@linux.intel.com] >Sent: Thursday, September 17, 2015 8:09 PM >To: Deepak, M; intel-gfx@lists.freedesktop.org >Cc: Deepak, M >Subject: Re: [Intel-gfx] [MIPI SEQ PARSING v2 PATCH 05/11] drm/i915: Added >support the v3 mipi sequence block > >On Thu, 10 Sep 2015, Deepak M <m.deepak@intel.com> wrote: >> From: vkorjani <vikas.korjani@intel.com> >> >> The Block 53 of the VBT, which is the MIPI sequence block has >> undergone a design change because of which the parsing logic has to be >> changed. >> >> The current code will handle the parsing of v3 and other lower >> versions of the MIPI sequence block. >> >> v2: rebase >> >> Signed-off-by: vkorjani <vikas.korjani@intel.com> >> Signed-off-by: Deepak M <m.deepak@intel.com> >> --- >> drivers/gpu/drm/i915/intel_bios.c | 119 >+++++++++++++++++++++++----- >> drivers/gpu/drm/i915/intel_bios.h | 8 ++ >> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 7 ++ >> 3 files changed, 114 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_bios.c >> b/drivers/gpu/drm/i915/intel_bios.c >> index 34a1042..cea641f 100644 >> --- a/drivers/gpu/drm/i915/intel_bios.c >> +++ b/drivers/gpu/drm/i915/intel_bios.c >> @@ -45,6 +45,7 @@ find_section(struct drm_i915_private *dev_priv, >> int index = 0; >> u32 total, current_size; >> u8 current_id; >> + u8 version; >> >> /* skip to first section */ >> index += bdb->header_size; >> @@ -56,7 +57,17 @@ find_section(struct drm_i915_private *dev_priv, >> current_id = *(base + index); >> index++; >> >> - current_size = *((const u16 *)(base + index)); >> + if (current_id == BDB_MIPI_SEQUENCE) { >> + version = *(base + index + 2); >> + if (version >= 3) >> + current_size = *((const u32 *)(base + >> + index + 3)); >> + else >> + current_size = *((const u16 *)(base + index)); >> + } else { >> + current_size = *((const u16 *)(base + index)); >> + } >> + > >While reviewing I've realized the old kernels will hit this hard. I've submitted a >patch [1] to be applied to v4.3-rc and older stable kernels so that they fail >gracefully instead of starting to parse garbage. The real parsing is too big to >backport to upstream stable. Please review. > >[1] http://mid.gmane.org/1442497327-27033-1-git-send-email- >jani.nikula@intel.com > >> index += 2; >> >> if (index + current_size > total) >> @@ -745,6 +756,55 @@ static u8 *goto_next_sequence(u8 *data, int *size) >> return data; >> } >> >> +static u8 *goto_next_sequence_v3(u8 *data, int *size) { >> + int tmp = *size; >> + int op_size; >> + >> + if (--tmp < 0) >> + return NULL; >> + >> + /* Skip the panel id and the sequence size */ > >It's not panel id, it's the sequence byte, right? > >You could also store data + 1 + size of sequence, and check whether data ends >up pointing at the same place in the end. They should. > >Shouldn't you also take 4 bytes of sequence size field into account in tmp? > >> + data = data + 5; >> + while (*data != 0) { >> + u8 element_type = *data++; >> + >> + switch (element_type) { > >Would be helpful to refer to operation_byte like in the spec. > >> + default: >> + DRM_ERROR("Unknown element type %d\n", >element_type); >> + 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: >> + /* >> + * skip by this element payload size >> + * skip elem id, command flag and data type >> + */ >> + op_size = *data++; >> + tmp = tmp - (op_size + 1); >> + if (tmp < 0) >> + return NULL; > >Isn't each operation operation byte | size of operation | payload size, i.e. your >tmp change is one byte short? > >The fact that the goto_next_sequence* functions increase data and decrease >size is getting increasingly confusing to follow. One simple alternative would >be to calculate some endp = start + size up front, and then pass the endp >around, and check if we're about to go past the end marker. > >This is not a problem with your series, it was there already. And fixing it >doesn't have to be part of your series. It just really takes ages to review this >approach of range checking. Unless I close my eyes and trust there are no off- >by-ones anywhere. But that kind of defeats the purpose of review... > [Deepak M] Okay will try to simplify the logic. >> + >> + /* skip by len */ >> + data += op_size; >> + break; >> + } >> + } >> + >> + /* goto next sequence or end of block byte */ >> + if (--tmp < 0) >> + return NULL; >> + >> + /* Skip the end element marker */ >> + data++; >> + >> + /* 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, const struct bdb_header >> *bdb) { @@ -754,7 +814,7 @@ parse_mipi(struct drm_i915_private >> *dev_priv, const struct bdb_header *bdb) >> const struct mipi_pps_data *pps; >> u8 *data; >> const u8 *seq_data; >> - int i, panel_id, seq_size; >> + int i, panel_id, panel_seq_size; >> u16 block_size; >> >> /* parse MIPI blocks only if LFP type is MIPI */ @@ -811,29 +871,40 >> @@ parse_mipi(struct drm_i915_private *dev_priv, const struct >> bdb_header *bdb) >> >> 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]; >> + if (dev_priv->vbt.dsi.seq_version >= 3) { >> + block_size = *((unsigned int *)seq_data); > >(const u32 *) > >> + seq_data = seq_data + 4; >> + } else >> + block_size = get_blocksize(sequence); > >block_size should be changed to u32. > >> >> /* >> * sequence block is variable length and hence we need to parse and >> * get the sequence data for specific panel id >> */ >> for (i = 0; i < MAX_MIPI_CONFIGURATIONS; i++) { >> - panel_id = *seq_data; >> - seq_size = *((u16 *) (seq_data + 1)); >> + panel_id = *seq_data++; >> + if (dev_priv->vbt.dsi.seq_version >= 3) { >> + panel_seq_size = *((u32 *)seq_data); >> + seq_data += sizeof(u32); >> + } else { >> + panel_seq_size = *((u16 *)seq_data); >> + seq_data += sizeof(u16); >> + } >> + >> if (panel_id == panel_type) >> break; >> >> - /* skip the sequence including seq header of 3 bytes */ >> - seq_data = seq_data + 3 + seq_size; >> + seq_data += panel_seq_size; >> + >> if ((seq_data - &sequence->data[0]) > block_size) { >> - DRM_ERROR("Sequence start is beyond sequence >block size, corrupted sequence block\n"); >> + DRM_ERROR("Sequence start is beyond seq block >size\n"); >> + DRM_ERROR("Corrupted sequence block\n"); > >Please don't add two consecutive DRM_ERRORs for the same error. > >> return; >> } >> } >> @@ -845,13 +916,14 @@ parse_mipi(struct drm_i915_private *dev_priv, >> const struct bdb_header *bdb) >> >> /* check if found sequence is completely within the sequence block >> * just being paranoid */ >> - if (seq_size > block_size) { >> + if (panel_seq_size > block_size) { >> DRM_ERROR("Corrupted sequence/size, bailing out\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); >> + >> + dev_priv->vbt.dsi.data = kmemdup(seq_data, panel_seq_size, >> +GFP_KERNEL); >> + >> if (!dev_priv->vbt.dsi.data) >> return; >> >> @@ -860,29 +932,36 @@ parse_mipi(struct drm_i915_private *dev_priv, >const struct bdb_header *bdb) >> * There are only 5 types of sequences as of now >> */ >> data = dev_priv->vbt.dsi.data; >> - dev_priv->vbt.dsi.size = seq_size; >> + dev_priv->vbt.dsi.size = panel_seq_size; >> >> /* two consecutive 0x00 indicate end of all sequences */ >> - while (1) { >> + while (*data != 0) { >> int seq_id = *data; >> + int seq_size; > >u32 > >> + >> 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"); >> - goto err; >> + DRM_ERROR("undefined sequence - %d\n", seq_id); >> + seq_size = *(data + 1); > >Needs to be *((const u32 *)(data + 1)) or you'll ignore 3 highest order bytes. > >> + if (dev_priv->vbt.dsi.seq_version >= 3) { >> + data = data + seq_size + 1; >> + continue; >> + } else >> + goto err; >> } >> >> /* partial parsing to skip elements */ >> - data = goto_next_sequence(data, &seq_size); >> + if (dev_priv->vbt.dsi.seq_version >= 3) >> + data = goto_next_sequence_v3(data, >&panel_seq_size); >> + else >> + data = goto_next_sequence(data, &panel_seq_size); >> >> if (data == NULL) { >> DRM_ERROR("Sequence elements going beyond >block itself. Sequence block parsing failed\n"); >> goto err; >> } >> - >> - if (*data == 0) >> - break; /* end of sequence reached */ >> } >> >> DRM_DEBUG_DRIVER("MIPI related vbt parsing complete\n"); diff -- >git >> a/drivers/gpu/drm/i915/intel_bios.h >> b/drivers/gpu/drm/i915/intel_bios.h >> index 21a7f3f..7a4ba41 100644 >> --- a/drivers/gpu/drm/i915/intel_bios.h >> +++ b/drivers/gpu/drm/i915/intel_bios.h >> @@ -948,6 +948,12 @@ enum mipi_seq { >> MIPI_SEQ_DISPLAY_ON, >> MIPI_SEQ_DISPLAY_OFF, >> MIPI_SEQ_DEASSERT_RESET, >> + MIPI_SEQ_BACKLIGHT_ON, >> + MIPI_SEQ_BACKLIGHT_OFF, >> + MIPI_SEQ_TEAR_ON, >> + MIPI_SEQ_TEAR_OFF, >> + MIPI_SEQ_POWER_ON, >> + MIPI_SEQ_POWER_OFF, >> MIPI_SEQ_MAX >> }; >> >> @@ -957,6 +963,8 @@ enum mipi_seq_element { >> MIPI_SEQ_ELEM_DELAY, >> MIPI_SEQ_ELEM_GPIO, >> MIPI_SEQ_ELEM_I2C, >> + MIPI_SEQ_ELEM_SPI, >> + MIPI_SEQ_ELEM_PMIC, >> MIPI_SEQ_ELEM_STATUS, > >Again, MIPI_SEQ_ELEM_STATUS is not spec. > >> MIPI_SEQ_ELEM_MAX >> }; >> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> index 9989f61..c6a6fa1 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> @@ -317,6 +317,8 @@ static const char * const seq_name[] = { >> >> static void generic_exec_sequence(struct intel_dsi *intel_dsi, const >> u8 *data) { >> + struct drm_device *dev = intel_dsi->base.base.dev; >> + struct drm_i915_private *dev_priv = dev->dev_private; >> fn_mipi_elem_exec mipi_elem_exec; >> int index; >> >> @@ -327,6 +329,8 @@ static void generic_exec_sequence(struct intel_dsi >> *intel_dsi, const u8 *data) >> >> /* go to the first element of the sequence */ >> data++; >> + if (dev_priv->vbt.dsi.seq_version >= 3) >> + data = data + 4; >> >> /* parse each byte till we reach end of sequence byte - 0x00 */ >> while (1) { >> @@ -340,6 +344,9 @@ static void generic_exec_sequence(struct intel_dsi >*intel_dsi, const u8 *data) >> /* goto element payload */ >> data++; >> >> + if (dev_priv->vbt.dsi.seq_version >= 3) >> + data++; >> + >> /* execute the element specific rotines */ >> data = mipi_elem_exec(intel_dsi, data); >> >> -- >> 1.7.9.5 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Jani Nikula, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 34a1042..cea641f 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -45,6 +45,7 @@ find_section(struct drm_i915_private *dev_priv, int index = 0; u32 total, current_size; u8 current_id; + u8 version; /* skip to first section */ index += bdb->header_size; @@ -56,7 +57,17 @@ find_section(struct drm_i915_private *dev_priv, current_id = *(base + index); index++; - current_size = *((const u16 *)(base + index)); + if (current_id == BDB_MIPI_SEQUENCE) { + version = *(base + index + 2); + if (version >= 3) + current_size = *((const u32 *)(base + + index + 3)); + else + current_size = *((const u16 *)(base + index)); + } else { + current_size = *((const u16 *)(base + index)); + } + index += 2; if (index + current_size > total) @@ -745,6 +756,55 @@ static u8 *goto_next_sequence(u8 *data, int *size) return data; } +static u8 *goto_next_sequence_v3(u8 *data, int *size) +{ + int tmp = *size; + int op_size; + + if (--tmp < 0) + return NULL; + + /* Skip the panel id and the sequence size */ + data = data + 5; + while (*data != 0) { + u8 element_type = *data++; + + switch (element_type) { + default: + DRM_ERROR("Unknown element type %d\n", element_type); + 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: + /* + * skip by this element payload size + * skip elem id, command flag and data type + */ + op_size = *data++; + tmp = tmp - (op_size + 1); + if (tmp < 0) + return NULL; + + /* skip by len */ + data += op_size; + break; + } + } + + /* goto next sequence or end of block byte */ + if (--tmp < 0) + return NULL; + + /* Skip the end element marker */ + data++; + + /* 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, const struct bdb_header *bdb) { @@ -754,7 +814,7 @@ parse_mipi(struct drm_i915_private *dev_priv, const struct bdb_header *bdb) const struct mipi_pps_data *pps; u8 *data; const u8 *seq_data; - int i, panel_id, seq_size; + int i, panel_id, panel_seq_size; u16 block_size; /* parse MIPI blocks only if LFP type is MIPI */ @@ -811,29 +871,40 @@ parse_mipi(struct drm_i915_private *dev_priv, const struct bdb_header *bdb) 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]; + if (dev_priv->vbt.dsi.seq_version >= 3) { + block_size = *((unsigned int *)seq_data); + seq_data = seq_data + 4; + } else + block_size = get_blocksize(sequence); /* * sequence block is variable length and hence we need to parse and * get the sequence data for specific panel id */ for (i = 0; i < MAX_MIPI_CONFIGURATIONS; i++) { - panel_id = *seq_data; - seq_size = *((u16 *) (seq_data + 1)); + panel_id = *seq_data++; + if (dev_priv->vbt.dsi.seq_version >= 3) { + panel_seq_size = *((u32 *)seq_data); + seq_data += sizeof(u32); + } else { + panel_seq_size = *((u16 *)seq_data); + seq_data += sizeof(u16); + } + if (panel_id == panel_type) break; - /* skip the sequence including seq header of 3 bytes */ - seq_data = seq_data + 3 + seq_size; + seq_data += panel_seq_size; + if ((seq_data - &sequence->data[0]) > block_size) { - DRM_ERROR("Sequence start is beyond sequence block size, corrupted sequence block\n"); + DRM_ERROR("Sequence start is beyond seq block size\n"); + DRM_ERROR("Corrupted sequence block\n"); return; } } @@ -845,13 +916,14 @@ parse_mipi(struct drm_i915_private *dev_priv, const struct bdb_header *bdb) /* check if found sequence is completely within the sequence block * just being paranoid */ - if (seq_size > block_size) { + if (panel_seq_size > block_size) { DRM_ERROR("Corrupted sequence/size, bailing out\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); + + dev_priv->vbt.dsi.data = kmemdup(seq_data, panel_seq_size, GFP_KERNEL); + if (!dev_priv->vbt.dsi.data) return; @@ -860,29 +932,36 @@ parse_mipi(struct drm_i915_private *dev_priv, const struct bdb_header *bdb) * There are only 5 types of sequences as of now */ data = dev_priv->vbt.dsi.data; - dev_priv->vbt.dsi.size = seq_size; + dev_priv->vbt.dsi.size = panel_seq_size; /* two consecutive 0x00 indicate end of all sequences */ - while (1) { + while (*data != 0) { int seq_id = *data; + int seq_size; + 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"); - goto err; + DRM_ERROR("undefined sequence - %d\n", seq_id); + seq_size = *(data + 1); + if (dev_priv->vbt.dsi.seq_version >= 3) { + data = data + seq_size + 1; + continue; + } else + goto err; } /* partial parsing to skip elements */ - data = goto_next_sequence(data, &seq_size); + if (dev_priv->vbt.dsi.seq_version >= 3) + data = goto_next_sequence_v3(data, &panel_seq_size); + else + data = goto_next_sequence(data, &panel_seq_size); if (data == NULL) { DRM_ERROR("Sequence elements going beyond block itself. Sequence block parsing failed\n"); goto err; } - - if (*data == 0) - break; /* end of sequence reached */ } DRM_DEBUG_DRIVER("MIPI related vbt parsing complete\n"); diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h index 21a7f3f..7a4ba41 100644 --- a/drivers/gpu/drm/i915/intel_bios.h +++ b/drivers/gpu/drm/i915/intel_bios.h @@ -948,6 +948,12 @@ enum mipi_seq { MIPI_SEQ_DISPLAY_ON, MIPI_SEQ_DISPLAY_OFF, MIPI_SEQ_DEASSERT_RESET, + MIPI_SEQ_BACKLIGHT_ON, + MIPI_SEQ_BACKLIGHT_OFF, + MIPI_SEQ_TEAR_ON, + MIPI_SEQ_TEAR_OFF, + MIPI_SEQ_POWER_ON, + MIPI_SEQ_POWER_OFF, MIPI_SEQ_MAX }; @@ -957,6 +963,8 @@ enum mipi_seq_element { MIPI_SEQ_ELEM_DELAY, MIPI_SEQ_ELEM_GPIO, MIPI_SEQ_ELEM_I2C, + MIPI_SEQ_ELEM_SPI, + MIPI_SEQ_ELEM_PMIC, MIPI_SEQ_ELEM_STATUS, MIPI_SEQ_ELEM_MAX }; diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c index 9989f61..c6a6fa1 100644 --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c @@ -317,6 +317,8 @@ static const char * const seq_name[] = { static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8 *data) { + struct drm_device *dev = intel_dsi->base.base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; fn_mipi_elem_exec mipi_elem_exec; int index; @@ -327,6 +329,8 @@ static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8 *data) /* go to the first element of the sequence */ data++; + if (dev_priv->vbt.dsi.seq_version >= 3) + data = data + 4; /* parse each byte till we reach end of sequence byte - 0x00 */ while (1) { @@ -340,6 +344,9 @@ static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8 *data) /* goto element payload */ data++; + if (dev_priv->vbt.dsi.seq_version >= 3) + data++; + /* execute the element specific rotines */ data = mipi_elem_exec(intel_dsi, data);