* [bug report] drm/i915: Make sure we have enough memory bandwidth on ICL
@ 2019-05-29 10:03 Dan Carpenter
2019-05-29 10:22 ` Ville Syrjälä
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2019-05-29 10:03 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
Hi Syrjälä,
I had a question about commit c457d9cf256e: ("drm/i915: Make sure we have
enough memory bandwidth on ICL").
drivers/gpu/drm/i915/intel_bw.c
64 static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
65 struct intel_qgv_point *sp,
66 int point)
67 {
68 u32 val = 0, val2;
^^^^
"val2" is uninitialized.
69 int ret;
70
71 ret = sandybridge_pcode_read(dev_priv,
72 ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
73 ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point),
74 &val, &val2);
75 if (ret)
76 return ret;
77
78 sp->dclk = val & 0xffff;
79 sp->t_rp = (val & 0xff0000) >> 16;
80 sp->t_rcd = (val & 0xff000000) >> 24;
81
82 sp->t_rdpre = val2 & 0xff;
83 sp->t_ras = (val2 & 0xff00) >> 8;
84
85 sp->t_rc = sp->t_rp + sp->t_ras;
86
87 return 0;
88 }
drivers/gpu/drm/i915/intel_sideband.c
376 static int __sandybridge_pcode_rw(struct drm_i915_private *i915,
377 u32 mbox, u32 *val, u32 *val1,
378 int fast_timeout_us,
379 int slow_timeout_ms,
380 bool is_read)
381 {
382 struct intel_uncore *uncore = &i915->uncore;
383
384 lockdep_assert_held(&i915->sb_lock);
385
386 /*
387 * GEN6_PCODE_* are outside of the forcewake domain, we can
388 * use te fw I915_READ variants to reduce the amount of work
389 * required when reading/writing.
390 */
391
392 if (intel_uncore_read_fw(uncore, GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY)
393 return -EAGAIN;
394
395 intel_uncore_write_fw(uncore, GEN6_PCODE_DATA, *val);
396 intel_uncore_write_fw(uncore, GEN6_PCODE_DATA1, val1 ? *val1 : 0);
^^^^^
We write uninitialized value out here. I'm sort of surprised that
UBSan doesn't complain. I don't know the code well enough to say if
this is a problem.
397 intel_uncore_write_fw(uncore,
398 GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
399
400 if (__intel_wait_for_register_fw(uncore,
401 GEN6_PCODE_MAILBOX,
402 GEN6_PCODE_READY, 0,
403 fast_timeout_us,
404 slow_timeout_ms,
405 &mbox))
406 return -ETIMEDOUT;
regards,
dan carpenter
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] drm/i915: Make sure we have enough memory bandwidth on ICL
2019-05-29 10:03 [bug report] drm/i915: Make sure we have enough memory bandwidth on ICL Dan Carpenter
@ 2019-05-29 10:22 ` Ville Syrjälä
0 siblings, 0 replies; 2+ messages in thread
From: Ville Syrjälä @ 2019-05-29 10:22 UTC (permalink / raw)
To: Dan Carpenter; +Cc: intel-gfx
On Wed, May 29, 2019 at 01:03:35PM +0300, Dan Carpenter wrote:
> Hi Syrjälä,
>
> I had a question about commit c457d9cf256e: ("drm/i915: Make sure we have
> enough memory bandwidth on ICL").
>
> drivers/gpu/drm/i915/intel_bw.c
> 64 static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
> 65 struct intel_qgv_point *sp,
> 66 int point)
> 67 {
> 68 u32 val = 0, val2;
> ^^^^
> "val2" is uninitialized.
>
> 69 int ret;
> 70
> 71 ret = sandybridge_pcode_read(dev_priv,
> 72 ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> 73 ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point),
> 74 &val, &val2);
> 75 if (ret)
> 76 return ret;
> 77
> 78 sp->dclk = val & 0xffff;
> 79 sp->t_rp = (val & 0xff0000) >> 16;
> 80 sp->t_rcd = (val & 0xff000000) >> 24;
> 81
> 82 sp->t_rdpre = val2 & 0xff;
> 83 sp->t_ras = (val2 & 0xff00) >> 8;
> 84
> 85 sp->t_rc = sp->t_rp + sp->t_ras;
> 86
> 87 return 0;
> 88 }
>
> drivers/gpu/drm/i915/intel_sideband.c
> 376 static int __sandybridge_pcode_rw(struct drm_i915_private *i915,
> 377 u32 mbox, u32 *val, u32 *val1,
> 378 int fast_timeout_us,
> 379 int slow_timeout_ms,
> 380 bool is_read)
> 381 {
> 382 struct intel_uncore *uncore = &i915->uncore;
> 383
> 384 lockdep_assert_held(&i915->sb_lock);
> 385
> 386 /*
> 387 * GEN6_PCODE_* are outside of the forcewake domain, we can
> 388 * use te fw I915_READ variants to reduce the amount of work
> 389 * required when reading/writing.
> 390 */
> 391
> 392 if (intel_uncore_read_fw(uncore, GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY)
> 393 return -EAGAIN;
> 394
> 395 intel_uncore_write_fw(uncore, GEN6_PCODE_DATA, *val);
> 396 intel_uncore_write_fw(uncore, GEN6_PCODE_DATA1, val1 ? *val1 : 0);
> ^^^^^
> We write uninitialized value out here. I'm sort of surprised that
> UBSan doesn't complain. I don't know the code well enough to say if
> this is a problem.
Thanks for catching this. I suspect this pcode command doesn't care
about the initial value in that register, but I can't be 100% sure.
So we should initialize it just in case.
>
> 397 intel_uncore_write_fw(uncore,
> 398 GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
> 399
> 400 if (__intel_wait_for_register_fw(uncore,
> 401 GEN6_PCODE_MAILBOX,
> 402 GEN6_PCODE_READY, 0,
> 403 fast_timeout_us,
> 404 slow_timeout_ms,
> 405 &mbox))
> 406 return -ETIMEDOUT;
>
> regards,
> dan carpenter
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-05-29 10:22 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29 10:03 [bug report] drm/i915: Make sure we have enough memory bandwidth on ICL Dan Carpenter
2019-05-29 10:22 ` Ville Syrjälä
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.