From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [bug report] drm/i915: Make sure we have enough memory bandwidth on ICL
Date: Wed, 29 May 2019 13:22:55 +0300 [thread overview]
Message-ID: <20190529102255.GQ5942@intel.com> (raw)
In-Reply-To: <20190529100335.GA10365@mwanda>
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
prev parent reply other threads:[~2019-05-29 10:22 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190529102255.GQ5942@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=dan.carpenter@oracle.com \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.