Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/selftests: Add compiler paranoia for checking HWSP values
Date: Fri, 17 Jul 2020 11:13:07 +0300	[thread overview]
Message-ID: <878sfi36gc.fsf@gaia.fi.intel.com> (raw)
In-Reply-To: <20200716203201.11977-1-chris@chris-wilson.co.uk>

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Since we want to read the values from the HWSP as written to by the GPU,
> warn the compiler that the values are volatile.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Yes, for consistency (and accuracy) this should be nudged also, even
tho it is on the safe side.

    GEM_BUG_ON(i915_seqno_passed(*tl->hwsp_seqno, *seqno));

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/gt/selftest_timeline.c | 22 ++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
> index fb5b7d3498a6..aabd46cea511 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
> @@ -491,7 +491,7 @@ checked_intel_timeline_create(struct intel_gt *gt)
>  	if (IS_ERR(tl))
>  		return tl;
>  
> -	if (*tl->hwsp_seqno != tl->seqno) {
> +	if (READ_ONCE(*tl->hwsp_seqno) != tl->seqno) {
>  		pr_err("Timeline created with incorrect breadcrumb, found %x, expected %x\n",
>  		       *tl->hwsp_seqno, tl->seqno);
>  		intel_timeline_put(tl);
> @@ -561,9 +561,9 @@ static int live_hwsp_engine(void *arg)
>  	for (n = 0; n < count; n++) {
>  		struct intel_timeline *tl = timelines[n];
>  
> -		if (!err && *tl->hwsp_seqno != n) {
> -			pr_err("Invalid seqno stored in timeline %lu @ %x, found 0x%x\n",
> -			       n, tl->hwsp_offset, *tl->hwsp_seqno);
> +		if (!err && READ_ONCE(*tl->hwsp_seqno) != n) {
> +			GEM_TRACE_ERR("Invalid seqno:%lu stored in timeline %llu @ %x, found 0x%x\n",
> +			       n, tl->fence_context, tl->hwsp_offset, *tl->hwsp_seqno);
>  			GEM_TRACE_DUMP();
>  			err = -EINVAL;
>  		}
> @@ -633,9 +633,9 @@ static int live_hwsp_alternate(void *arg)
>  	for (n = 0; n < count; n++) {
>  		struct intel_timeline *tl = timelines[n];
>  
> -		if (!err && *tl->hwsp_seqno != n) {
> -			pr_err("Invalid seqno stored in timeline %lu @ %x, found 0x%x\n",
> -			       n, tl->hwsp_offset, *tl->hwsp_seqno);
> +		if (!err && READ_ONCE(*tl->hwsp_seqno) != n) {
> +			GEM_TRACE_ERR("Invalid seqno:%lu stored in timeline %llu @ %x, found 0x%x\n",
> +			       n, tl->fence_context, tl->hwsp_offset, *tl->hwsp_seqno);
>  			GEM_TRACE_DUMP();
>  			err = -EINVAL;
>  		}
> @@ -733,7 +733,7 @@ static int live_hwsp_wrap(void *arg)
>  			goto out;
>  		}
>  
> -		if (*hwsp_seqno[0] != seqno[0] || *hwsp_seqno[1] != seqno[1]) {
> +		if (READ_ONCE(*hwsp_seqno[0]) != seqno[0] || READ_ONCE(*hwsp_seqno[1]) != seqno[1]) {
>  			pr_err("Bad timeline values: found (%x, %x), expected (%x, %x)\n",
>  			       *hwsp_seqno[0], *hwsp_seqno[1],
>  			       seqno[0], seqno[1]);
> @@ -966,9 +966,9 @@ static int live_hwsp_recycle(void *arg)
>  				break;
>  			}
>  
> -			if (*tl->hwsp_seqno != count) {
> -				pr_err("Invalid seqno stored in timeline %lu @ tl->hwsp_offset, found 0x%x\n",
> -				       count, *tl->hwsp_seqno);
> +			if (READ_ONCE(*tl->hwsp_seqno) != count) {
> +				GEM_TRACE_ERR("Invalid seqno:%lu stored in timeline %llu @ %x found 0x%x\n",
> +				       count, tl->fence_context, tl->hwsp_offset, *tl->hwsp_seqno);
>  				GEM_TRACE_DUMP();
>  				err = -EINVAL;
>  			}
> -- 
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

      parent reply	other threads:[~2020-07-17  8:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-16 20:32 [Intel-gfx] [PATCH 1/2] drm/i915/selftests: Add compiler paranoia for checking HWSP values Chris Wilson
2020-07-16 20:32 ` [Intel-gfx] [PATCH 2/2] drm/i915/gt: Wait for aux invalidation on Tigerlake Chris Wilson
2020-07-16 21:32   ` Chris Wilson
2020-07-17  8:30   ` Mika Kuoppala
2020-07-17 10:24     ` Chris Wilson
2020-07-16 20:40 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/selftests: Add compiler paranoia for checking HWSP values Patchwork
2020-07-16 20:40 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-07-16 21:02 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-07-17  8:13 ` Mika Kuoppala [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=878sfi36gc.fsf@gaia.fi.intel.com \
    --to=mika.kuoppala@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox