[PATCH] drm/i915: Just clear the mmiodebug before a register access

[PATCH] drm/i915: Just clear the mmiodebug before a register access

[Chris Wilson]

When we enable the per-register access mmiodebug, it is to detect which
access is illegal. Reporting on earlier untraced access outside of the
mmiodebug does not help debugging (as the suspicion is immediately put
upon the current register which is not at fault)!

References: https://bugs.freedesktop.org/show_bug.cgi?id=97985
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index a9b6c936aadd..ee2306a79747 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -796,10 +796,9 @@ __unclaimed_reg_debug(struct drm_i915_private *dev_priv,
 		      const bool read,
 		      const bool before)
 {
-	if (WARN(check_for_unclaimed_mmio(dev_priv),
-		 "Unclaimed register detected %s %s register 0x%x\n",
-		 before ? "before" : "after",
-		 read ? "reading" : "writing to",
+	if (WARN(check_for_unclaimed_mmio(dev_priv) && !before,
+		 "Unclaimed %s register 0x%x\n",
+		 read ? "read from" : "write to",
 		 i915_mmio_reg_offset(reg)))
 		i915.mmio_debug--; /* Only report the first N failures */
 }
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for drm/i915: Just clear the mmiodebug before a register access

[Patchwork]

== Series Details ==

Series: drm/i915: Just clear the mmiodebug before a register access
URL   : https://patchwork.freedesktop.org/series/13161/
State : warning

== Summary ==

Series 13161v1 drm/i915: Just clear the mmiodebug before a register access
https://patchwork.freedesktop.org/api/1.0/series/13161/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-byt-j1900)
Test kms_psr_sink_crc:
        Subgroup psr_basic:
                dmesg-warn -> PASS       (fi-skl-6700hq)

fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:244  pass:214  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:244  pass:211  dwarn:1   dfail:0   fail:1   skip:31 
fi-byt-n2820     total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:244  pass:182  dwarn:0   dfail:0   fail:2   skip:60 
fi-ivb-3520m     total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6700k     total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2606/

d0534bd0217c83c083ba146b9c987e19b219e0e4 drm-intel-nightly: 2016y-09m-30d-10h-31m-00s UTC integration manifest
c7d2a2a drm/i915: Just clear the mmiodebug before a register access

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Just clear the mmiodebug before a register access

[Mika Kuoppala]

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

> When we enable the per-register access mmiodebug, it is to detect which
> access is illegal. Reporting on earlier untraced access outside of the
> mmiodebug does not help debugging (as the suspicion is immediately put
> upon the current register which is not at fault)!
>

I think the original intent here was that if the unclaimed access was
before driver register access, it would be sometimes possible to assess
the real culprit by understanding what happened before this moment.

For example if it is before first register access after resume, you can blame
BIOS.

On the other hand we should have now the checks from and after
suspending in place so perhaps it is not worthwhile anymore to keep
reporting the prior unclaimed access in generic case.

However as the check also clears the unclaimed bit, we might
really squels some real problems with this change.

-Mika

> References: https://bugs.freedesktop.org/show_bug.cgi?id=97985
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index a9b6c936aadd..ee2306a79747 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -796,10 +796,9 @@ __unclaimed_reg_debug(struct drm_i915_private *dev_priv,
>  		      const bool read,
>  		      const bool before)
>  {
> -	if (WARN(check_for_unclaimed_mmio(dev_priv),
> -		 "Unclaimed register detected %s %s register 0x%x\n",
> -		 before ? "before" : "after",
> -		 read ? "reading" : "writing to",
> +	if (WARN(check_for_unclaimed_mmio(dev_priv) && !before,
> +		 "Unclaimed %s register 0x%x\n",
> +		 read ? "read from" : "write to",
>  		 i915_mmio_reg_offset(reg)))
>  		i915.mmio_debug--; /* Only report the first N failures */
>  }
> -- 
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Just clear the mmiodebug before a register access

[Chris Wilson]

On Mon, Oct 03, 2016 at 01:09:31PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > When we enable the per-register access mmiodebug, it is to detect which
> > access is illegal. Reporting on earlier untraced access outside of the
> > mmiodebug does not help debugging (as the suspicion is immediately put
> > upon the current register which is not at fault)!
> >
> 
> I think the original intent here was that if the unclaimed access was
> before driver register access, it would be sometimes possible to assess
> the real culprit by understanding what happened before this moment.

We have warnings elsewhere to catch this, and since we provide no debug
output in these cases flagging an error to the user is just noise for
them *and* us.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Just clear the mmiodebug before a register access

[Mika Kuoppala]

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

> When we enable the per-register access mmiodebug, it is to detect which
> access is illegal. Reporting on earlier untraced access outside of the
> mmiodebug does not help debugging (as the suspicion is immediately put
> upon the current register which is not at fault)!
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=97985
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index a9b6c936aadd..ee2306a79747 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -796,10 +796,9 @@ __unclaimed_reg_debug(struct drm_i915_private *dev_priv,
>  		      const bool read,
>  		      const bool before)
>  {
> -	if (WARN(check_for_unclaimed_mmio(dev_priv),
> -		 "Unclaimed register detected %s %s register 0x%x\n",
> -		 before ? "before" : "after",
> -		 read ? "reading" : "writing to",
> +	if (WARN(check_for_unclaimed_mmio(dev_priv) && !before,
> +		 "Unclaimed %s register 0x%x\n",
> +		 read ? "read from" : "write to",
>  		 i915_mmio_reg_offset(reg)))
>  		i915.mmio_debug--; /* Only report the first N failures */
>  }
> -- 
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx