On Mon, Dec 21, 2015 at 03:10:57PM +0200, Jani Nikula wrote:
> Make everything a bit more readable and clear.
> 
> Signed-off-by: Jani Nikula <jani.nik...@intel.com>

A real pain to check, but I think I convinced myself that this is
equivalent code.

Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>


> ---
>  drivers/gpu/drm/i915/i915_drv.h   |   2 +-
>  drivers/gpu/drm/i915/intel_bios.c | 158 
> +++++++++++++-------------------------
>  drivers/gpu/drm/i915/intel_bios.h |   4 +-
>  3 files changed, 57 insertions(+), 107 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9b82c4532893..07c4fc539680 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1487,7 +1487,7 @@ struct intel_vbt_data {
>               u8 seq_version;
>               u32 size;
>               u8 *data;
> -             u8 *sequence[MIPI_SEQ_MAX];
> +             const u8 *sequence[MIPI_SEQ_MAX];
>       } dsi;
>  
>       int crt_ddc_pin;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index 9511a5341562..d6eaf32f33e5 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -697,73 +697,6 @@ parse_psr(struct drm_i915_private *dev_priv, const 
> struct bdb_header *bdb)
>       dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time;
>  }
>  
> -static u8 *goto_next_sequence(u8 *data, u16 *size)
> -{
> -     u16 len;
> -     int tmp = *size;
> -
> -     if (--tmp < 0)
> -             return NULL;
> -
> -     /* goto first element */
> -     data++;
> -     while (1) {
> -             switch (*data) {
> -             case MIPI_SEQ_ELEM_SEND_PKT:
> -                     /*
> -                      * skip by this element payload size
> -                      * skip elem id, command flag and data type
> -                      */
> -                     tmp -= 5;
> -                     if (tmp < 0)
> -                             return NULL;
> -
> -                     data += 3;
> -                     len = *((u16 *)data);
> -
> -                     tmp -= len;
> -                     if (tmp < 0)
> -                             return NULL;
> -
> -                     /* skip by len */
> -                     data = data + 2 + len;
> -                     break;
> -             case MIPI_SEQ_ELEM_DELAY:
> -                     /* skip by elem id, and delay is 4 bytes */
> -                     tmp -= 5;
> -                     if (tmp < 0)
> -                             return NULL;
> -
> -                     data += 5;
> -                     break;
> -             case MIPI_SEQ_ELEM_GPIO:
> -                     tmp -= 3;
> -                     if (tmp < 0)
> -                             return NULL;
> -
> -                     data += 3;
> -                     break;
> -             default:
> -                     DRM_ERROR("Unknown element\n");
> -                     return NULL;
> -             }
> -
> -             /* end of sequence ? */
> -             if (*data == 0)
> -                     break;
> -     }
> -
> -     /* goto next sequence or end of block byte */
> -     if (--tmp < 0)
> -             return NULL;
> -
> -     data++;
> -
> -     /* update amount of data left for the sequence block to be parsed */
> -     *size = tmp;
> -     return data;
> -}
> -
>  static void
>  parse_mipi_config(struct drm_i915_private *dev_priv,
>                 const struct bdb_header *bdb)
> @@ -855,6 +788,39 @@ find_panel_sequence_block(const struct bdb_mipi_sequence 
> *sequence,
>       return NULL;
>  }
>  
> +static int goto_next_sequence(const u8 *data, int index, int total)
> +{
> +     u16 len;
> +
> +     /* Skip Sequence Byte. */
> +     for (index = index + 1; index < total; index += len) {
> +             u8 operation_byte = *(data + index);
> +             index++;
> +
> +             switch (operation_byte) {
> +             case MIPI_SEQ_ELEM_END:
> +                     return index;
> +             case MIPI_SEQ_ELEM_SEND_PKT:
> +                     if (index + 4 > total)
> +                             return 0;
> +
> +                     len = *((const u16 *)(data + index + 2)) + 4;
> +                     break;
> +             case MIPI_SEQ_ELEM_DELAY:
> +                     len = 4;
> +                     break;
> +             case MIPI_SEQ_ELEM_GPIO:
> +                     len = 2;
> +                     break;
> +             default:
> +                     DRM_ERROR("Unknown operation byte\n");
> +                     return 0;
> +             }
> +     }
> +
> +     return 0;
> +}
> +
>  static void
>  parse_mipi_sequence(struct drm_i915_private *dev_priv,
>                   const struct bdb_header *bdb)
> @@ -863,7 +829,7 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
>       const u8 *seq_data;
>       u16 seq_size;
>       u8 *data;
> -     u16 block_size;
> +     int index = 0;
>  
>       /* Only our generic panel driver uses the sequence block. */
>       if (dev_priv->vbt.dsi.panel_id != MIPI_DSI_GENERIC_PANEL_ID)
> @@ -883,59 +849,43 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
>  
>       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 = find_panel_sequence_block(sequence, panel_type, &seq_size);
>       if (!seq_data)
>               return;
>  
> -     dev_priv->vbt.dsi.data = kmemdup(seq_data, seq_size, GFP_KERNEL);
> -     if (!dev_priv->vbt.dsi.data)
> +     data = kmemdup(seq_data, seq_size, GFP_KERNEL);
> +     if (!data)
>               return;
>  
> -     /*
> -      * loop into the sequence data and split into multiple sequneces
> -      * There are only 5 types of sequences as of now
> -      */
> -     data = dev_priv->vbt.dsi.data;
> -     dev_priv->vbt.dsi.size = seq_size;
> +     /* Parse the sequences, store pointers to each sequence. */
> +     for (;;) {
> +             u8 seq_id = *(data + index);
> +             if (seq_id == MIPI_SEQ_END)
> +                     break;
>  
> -     /* two consecutive 0x00 indicate end of all sequences */
> -     while (1) {
> -             int seq_id = *data;
> -             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");
> +             if (seq_id >= MIPI_SEQ_MAX) {
> +                     DRM_ERROR("Unknown sequence %u\n", seq_id);
>                       goto err;
>               }
>  
> -             /* partial parsing to skip elements */
> -             data = goto_next_sequence(data, &seq_size);
> +             dev_priv->vbt.dsi.sequence[seq_id] = data + index;
>  
> -             if (data == NULL) {
> -                     DRM_ERROR("Sequence elements going beyond block itself. 
> Sequence block parsing failed\n");
> +             index = goto_next_sequence(data, index, seq_size);
> +             if (!index) {
> +                     DRM_ERROR("Invalid sequence %u\n", seq_id);
>                       goto err;
>               }
> -
> -             if (*data == 0)
> -                     break; /* end of sequence reached */
>       }
>  
> -     DRM_DEBUG_DRIVER("MIPI related vbt parsing complete\n");
> +     dev_priv->vbt.dsi.data = data;
> +     dev_priv->vbt.dsi.size = seq_size;
> +     dev_priv->vbt.dsi.seq_version = sequence->version;
> +
> +     DRM_DEBUG_DRIVER("MIPI related VBT parsing complete\n");
>       return;
> -err:
> -     kfree(dev_priv->vbt.dsi.data);
> -     dev_priv->vbt.dsi.data = NULL;
>  
> -     /* error during parsing so set all pointers to null
> -      * because of partial parsing */
> +err:
> +     kfree(data);
>       memset(dev_priv->vbt.dsi.sequence, 0, 
> sizeof(dev_priv->vbt.dsi.sequence));
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_bios.h 
> b/drivers/gpu/drm/i915/intel_bios.h
> index 21c162e01189..4e87df16e7b3 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -954,7 +954,7 @@ struct bdb_mipi_sequence {
>  
>  /* MIPI Sequnece Block definitions */
>  enum mipi_seq {
> -     MIPI_SEQ_UNDEFINED = 0,
> +     MIPI_SEQ_END = 0,
>       MIPI_SEQ_ASSERT_RESET,
>       MIPI_SEQ_INIT_OTP,
>       MIPI_SEQ_DISPLAY_ON,
> @@ -964,7 +964,7 @@ enum mipi_seq {
>  };
>  
>  enum mipi_seq_element {
> -     MIPI_SEQ_ELEM_UNDEFINED = 0,
> +     MIPI_SEQ_ELEM_END = 0,
>       MIPI_SEQ_ELEM_SEND_PKT,
>       MIPI_SEQ_ELEM_DELAY,
>       MIPI_SEQ_ELEM_GPIO,
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to