| Message ID | 06517b37abd6f4a36aee984da25941341599d58d.1450702954.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Dec 21, 2015 at 03:11:05PM +0200, Jani Nikula wrote: > The sequence block has sizes of elements after the operation byte since > sequence block v3. Use it to skip elements we don't support yet. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 43 +++++++++++++++++------------- > 1 file changed, 24 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > index eabfd9eb9cc0..1f9c80d21904 100644 > --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > @@ -335,31 +335,36 @@ static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8 *data) > 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++; > - if (operation_byte >= ARRAY_SIZE(exec_elem) || > - !exec_elem[operation_byte]) { > + u8 operation_size = 0; > + > + if (operation_byte == MIPI_SEQ_ELEM_END) > + break; > + > + if (operation_byte < ARRAY_SIZE(exec_elem) && > + exec_elem[operation_byte]) && exec_elem[operation_byte] is redundant since you assing it to NULL anyway in the else clause. While I bikeshed: Might look prettier with ?: > + mipi_elem_exec = exec_elem[operation_byte]; > + else > + mipi_elem_exec = NULL; > + > + /* Size of Operation. */ > + if (dev_priv->vbt.dsi.seq_version >= 3) > + operation_size = *data++; > + > + if (mipi_elem_exec) { > + data = mipi_elem_exec(intel_dsi, data); > + } else if (operation_size) { > + /* We have size, skip. */ > + DRM_DEBUG_KMS("Unsupported MIPI operation byte %u\n", > + operation_byte); DRM_ERROR, like in the other cases we fail to parse the sequence fully? Either way on both bikesheds: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > + data += operation_size; > + } else { > + /* No size, can't skip without parsing. */ > DRM_ERROR("Unsupported MIPI operation byte %u\n", > operation_byte); > return; > } > - 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); > - > - /* > - * After processing the element, data should point to > - * next element or end of sequence > - * check if have we reached end of sequence > - */ > - if (*data == 0x00) > - break; > } > } > > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 05 Jan 2016, Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Dec 21, 2015 at 03:11:05PM +0200, Jani Nikula wrote: >> The sequence block has sizes of elements after the operation byte since >> sequence block v3. Use it to skip elements we don't support yet. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 43 +++++++++++++++++------------- >> 1 file changed, 24 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> index eabfd9eb9cc0..1f9c80d21904 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> @@ -335,31 +335,36 @@ static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8 *data) >> 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++; >> - if (operation_byte >= ARRAY_SIZE(exec_elem) || >> - !exec_elem[operation_byte]) { >> + u8 operation_size = 0; >> + >> + if (operation_byte == MIPI_SEQ_ELEM_END) >> + break; >> + >> + if (operation_byte < ARRAY_SIZE(exec_elem) && >> + exec_elem[operation_byte]) > > && exec_elem[operation_byte] is redundant since you assing it to NULL > anyway in the else clause. While I bikeshed: Might look prettier with ?: Silly me. I might prefer an if-else if the ?: doesn't fit on one line. > > >> + mipi_elem_exec = exec_elem[operation_byte]; >> + else >> + mipi_elem_exec = NULL; >> + >> + /* Size of Operation. */ >> + if (dev_priv->vbt.dsi.seq_version >= 3) >> + operation_size = *data++; >> + >> + if (mipi_elem_exec) { >> + data = mipi_elem_exec(intel_dsi, data); >> + } else if (operation_size) { >> + /* We have size, skip. */ >> + DRM_DEBUG_KMS("Unsupported MIPI operation byte %u\n", >> + operation_byte); > > DRM_ERROR, like in the other cases we fail to parse the sequence fully? I suppose we could add that in intel_bios.c to do it a limited number of times at driver load, but here it would keep spewing out the errors every modeset and possibly more. This is not necessarily a showstopper with sequence v3, as we have the size to move on to the next one. BR, Jani. > > Either way on both bikesheds: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > >> + data += operation_size; >> + } else { >> + /* No size, can't skip without parsing. */ >> DRM_ERROR("Unsupported MIPI operation byte %u\n", >> operation_byte); >> return; >> } >> - 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); >> - >> - /* >> - * After processing the element, data should point to >> - * next element or end of sequence >> - * check if have we reached end of sequence >> - */ >> - if (*data == 0x00) >> - break; >> } >> } >> >> -- >> 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_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c index eabfd9eb9cc0..1f9c80d21904 100644 --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c @@ -335,31 +335,36 @@ static void generic_exec_sequence(struct intel_dsi *intel_dsi, const u8 *data) 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++; - if (operation_byte >= ARRAY_SIZE(exec_elem) || - !exec_elem[operation_byte]) { + u8 operation_size = 0; + + if (operation_byte == MIPI_SEQ_ELEM_END) + break; + + if (operation_byte < ARRAY_SIZE(exec_elem) && + exec_elem[operation_byte]) + mipi_elem_exec = exec_elem[operation_byte]; + else + mipi_elem_exec = NULL; + + /* Size of Operation. */ + if (dev_priv->vbt.dsi.seq_version >= 3) + operation_size = *data++; + + if (mipi_elem_exec) { + data = mipi_elem_exec(intel_dsi, data); + } else if (operation_size) { + /* We have size, skip. */ + DRM_DEBUG_KMS("Unsupported MIPI operation byte %u\n", + operation_byte); + data += operation_size; + } else { + /* No size, can't skip without parsing. */ DRM_ERROR("Unsupported MIPI operation byte %u\n", operation_byte); return; } - 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); - - /* - * After processing the element, data should point to - * next element or end of sequence - * check if have we reached end of sequence - */ - if (*data == 0x00) - break; } }
The sequence block has sizes of elements after the operation byte since sequence block v3. Use it to skip elements we don't support yet. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 43 +++++++++++++++++------------- 1 file changed, 24 insertions(+), 19 deletions(-)