* Re: drm/i915/bdw: Provide the BDW specific HDMI buffer translation table
@ 2014-08-27 13:04 Dan Carpenter
2014-08-27 13:12 ` Damien Lespiau
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2014-08-27 13:04 UTC (permalink / raw)
To: damien.lespiau; +Cc: intel-gfx, dri-devel
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?
221 hdmi_level = hdmi_800mV_0dB;
222
223 /* Entry 9 is for HDMI: */
224 for (i = 0; i < 2; i++) {
225 I915_WRITE(reg, ddi_translations_hdmi[hdmi_level * 2 + i]);
^^^^^^^^^^^^^^^^^^
The static checker thinks "15 * 2 + 1 = 31" so we are beyond the size
of the largest array (either 20 or 24).
226 reg += 4;
227 }
228 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: drm/i915/bdw: Provide the BDW specific HDMI buffer translation table
2014-08-27 13:04 drm/i915/bdw: Provide the BDW specific HDMI buffer translation table Dan Carpenter
@ 2014-08-27 13:12 ` Damien Lespiau
0 siblings, 0 replies; 2+ messages in thread
From: Damien Lespiau @ 2014-08-27 13:12 UTC (permalink / raw)
To: Dan Carpenter; +Cc: intel-gfx, dri-devel
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-08-27 13:12 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-27 13:04 drm/i915/bdw: Provide the BDW specific HDMI buffer translation table Dan Carpenter
2014-08-27 13:12 ` Damien Lespiau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox