Re: drm/i915/bdw: Provide the BDW specific HDMI buffer translation table

[Damien Lespiau <damien.lespiau@intel.com>]

On Wed, Aug 27, 2014 at 04:04:46PM +0300, Dan Carpenter wrote:
> Hello Damien Lespiau,
> 
> The patch a26aa8baee6c: "drm/i915/bdw: Provide the BDW specific HDMI
> buffer translation table" from Aug 1, 2014, leads to the following
> static checker warning:
> 
> 	drivers/gpu/drm/i915/intel_ddi.c:225 intel_prepare_ddi_buffers()
> 	error: buffer overflow 'ddi_translations_hdmi' 24 <= 31
> 
> drivers/gpu/drm/i915/intel_ddi.c
>    155  static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port)
>    156  {
>    157          struct drm_i915_private *dev_priv = dev->dev_private;
>    158          u32 reg;
>    159          int i, n_hdmi_entries, hdmi_800mV_0dB;
>    160          int hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
>                     ^^^^^^^^^^
> The static checker thinks this is a number between 0-15.  I didn't check
> if that was correct.
> 
>    161          const u32 *ddi_translations_fdi;
> 
> [ snip ]
> 
>    186                  ddi_translations_hdmi = bdw_ddi_translations_hdmi;
>    187                  n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
>                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> limit is the size of the array.
> 
>    188                  hdmi_800mV_0dB = 7;
>    189          }
>    190  
>    191          switch (port) {
>    192          case PORT_A:
>    193                  ddi_translations = ddi_translations_edp;
>    194                  break;
>    195          case PORT_B:
>    196          case PORT_C:
>    197                  ddi_translations = ddi_translations_dp;
>    198                  break;
>    199          case PORT_D:
>    200                  if (intel_dp_is_edp(dev, PORT_D))
>    201                          ddi_translations = ddi_translations_edp;
>    202                  else
>    203                          ddi_translations = ddi_translations_dp;
>    204                  break;
>    205          case PORT_E:
>    206                  ddi_translations = ddi_translations_fdi;
>    207                  break;
>    208          default:
>    209                  BUG();
>    210          }
>    211  
>    212          for (i = 0, reg = DDI_BUF_TRANS(port);
>    213               i < ARRAY_SIZE(hsw_ddi_translations_fdi); i++) {
>    214                  I915_WRITE(reg, ddi_translations[i]);
>    215                  reg += 4;
>    216          }
>    217  
>    218          /* Choose a good default if VBT is badly populated */
>    219          if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN ||
>    220              hdmi_level >= n_hdmi_entries)
>                                   ^^^^^^^^^^^^^^
> We cap it at the size of the array which is either 20 or 24.  The static
> checker thinks it's already capped at 15 so this is not a problem.
> 
> Should this be n_hdmi_entries / 2 or something?  Maybe a - 1 in there?

Indeed, n_hdmi_entries needed to be:

  n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi) / 2;

This was fixed by:

  commit d6699dd3a7f696a80a5f8e5bb6ecf6ff6dd7c998
  Author: Damien Lespiau <damien.lespiau@intel.com>
  Date:   Sat Aug 9 16:29:31 2014 +0100

      drm/i915: Fix wrong number of HDMI translation entries

-- 
Damien