[PATCH] drm/i915: Catch non-existent registers in find_fw_domain

[PATCH] drm/i915: Catch non-existent registers in find_fw_domain

[Joonas Lahtinen]

Add WARN_ON to find_fw_domain to registers related to uninitialized
hardware.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Wang Elaine <elaine.wang@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 07779d0..b20b58e 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -625,7 +625,12 @@ find_fw_domain(struct drm_i915_private *dev_priv, u32 offset)
 			dev_priv->uncore.fw_domains_table_entries,
 			fw_range_cmp);
 
-	return entry ? entry->domains : 0;
+	if (!entry)
+		return 0;
+
+	WARN_ON(entry->domains & ~dev_priv->uncore.fw_domains);
+
+	return entry->domains;
 }
 
 static void
-- 
2.7.4

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

Re: [PATCH] drm/i915: Catch non-existent registers in find_fw_domain

[Chris Wilson]

On Wed, Dec 07, 2016 at 03:49:39PM +0200, Joonas Lahtinen wrote:
> Add WARN_ON to find_fw_domain to registers related to uninitialized
> hardware.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Wang Elaine <elaine.wang@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 07779d0..b20b58e 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -625,7 +625,12 @@ find_fw_domain(struct drm_i915_private *dev_priv, u32 offset)
>  			dev_priv->uncore.fw_domains_table_entries,
>  			fw_range_cmp);
>  
> -	return entry ? entry->domains : 0;
> +	if (!entry)
> +		return 0;
> +
> +	WARN_ON(entry->domains & ~dev_priv->uncore.fw_domains);

Ok, this is going to be hard to diagnose in the wild, how about

WARN(entry->domains & ~fw_domains,
"Attempting to use register 0x%04x from uninitialised fw domain %x\n",
offset, entry->domains & ~fw_domains); ?
-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: Catch non-existent registers in find_fw_domain

[Joonas Lahtinen]

On ke, 2016-12-07 at 14:01 +0000, Chris Wilson wrote:
> On Wed, Dec 07, 2016 at 03:49:39PM +0200, Joonas Lahtinen wrote:
> > 
> > +	WARN_ON(entry->domains & ~dev_priv->uncore.fw_domains);
> 
> Ok, this is going to be hard to diagnose in the wild, how about
> 
> WARN(entry->domains & ~fw_domains,
> "Attempting to use register 0x%04x from uninitialised fw domain %x\n",
> offset, entry->domains & ~fw_domains); ?

That's an excellent idea. Although I'll call it "forcewake domain" for
clarity.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915: Catch non-existent registers in find_fw_domain

[Joonas Lahtinen]

Add WARN_ON to find_fw_domain to registers related to uninitialized
hardware.

v2:
- Print the uninitialized domains and register (Chris)

Cc: Imre Deak <imre.deak@intel.com>
Cc: Wang Elaine <elaine.wang@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 07779d0..88f3611 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -625,7 +625,14 @@ find_fw_domain(struct drm_i915_private *dev_priv, u32 offset)
 			dev_priv->uncore.fw_domains_table_entries,
 			fw_range_cmp);
 
-	return entry ? entry->domains : 0;
+	if (!entry)
+		return 0;
+
+	WARN(entry->domains & ~dev_priv->uncore.fw_domains,
+	     "Uninitialized forcewake domain(s) 0x%x accessed at 0x%x\n",
+	     entry->domains & ~dev_priv->uncore.fw_domains, offset);
+
+	return entry->domains;
 }
 
 static void
-- 
2.7.4

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

Re: [PATCH v2] drm/i915: Catch non-existent registers in find_fw_domain

[Chris Wilson]

On Wed, Dec 07, 2016 at 04:22:39PM +0200, Joonas Lahtinen wrote:
> Add WARN_ON to find_fw_domain to registers related to uninitialized
> hardware.
> 
> v2:
> - Print the uninitialized domains and register (Chris)
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Wang Elaine <elaine.wang@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-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

✓ Fi.CI.BAT: success for drm/i915: Catch non-existent registers in find_fw_domain (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Catch non-existent registers in find_fw_domain (rev2)
URL   : https://patchwork.freedesktop.org/series/16487/
State : success

== Summary ==

Series 16487v2 drm/i915: Catch non-existent registers in find_fw_domain
https://patchwork.freedesktop.org/api/1.0/series/16487/revisions/2/mbox/


fi-bdw-5557u     total:247  pass:219  dwarn:0   dfail:0   fail:0   skip:28 
fi-bsw-n3050     total:247  pass:195  dwarn:0   dfail:0   fail:0   skip:52 
fi-byt-j1900     total:247  pass:206  dwarn:0   dfail:0   fail:0   skip:41 
fi-byt-n2820     total:247  pass:202  dwarn:0   dfail:0   fail:0   skip:45 
fi-hsw-4770      total:247  pass:214  dwarn:0   dfail:0   fail:0   skip:33 
fi-hsw-4770r     total:247  pass:214  dwarn:0   dfail:0   fail:0   skip:33 
fi-ilk-650       total:247  pass:181  dwarn:0   dfail:0   fail:0   skip:66 
fi-ivb-3520m     total:247  pass:213  dwarn:0   dfail:0   fail:0   skip:34 
fi-ivb-3770      total:247  pass:212  dwarn:0   dfail:0   fail:0   skip:35 
fi-kbl-7500u     total:247  pass:212  dwarn:0   dfail:0   fail:0   skip:35 
fi-skl-6260u     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-skl-6700hq    total:247  pass:214  dwarn:0   dfail:0   fail:0   skip:33 
fi-skl-6700k     total:247  pass:210  dwarn:3   dfail:0   fail:0   skip:34 
fi-skl-6770hq    total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-snb-2520m     total:247  pass:202  dwarn:0   dfail:0   fail:0   skip:45 
fi-snb-2600      total:247  pass:201  dwarn:0   dfail:0   fail:0   skip:46 

722bab6dc6ca5b0fd0c667e5dd65f7734c550f94 drm-tip: 2016y-12m-07d-16h-41m-46s UTC integration manifest
ee9b8cb drm/i915: Catch non-existent registers in find_fw_domain

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3219/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Catch non-existent registers in find_fw_domain

[Tvrtko Ursulin]

On 07/12/2016 14:22, Joonas Lahtinen wrote:
> Add WARN_ON to find_fw_domain to registers related to uninitialized
> hardware.
>
> v2:
> - Print the uninitialized domains and register (Chris)
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Wang Elaine <elaine.wang@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 07779d0..88f3611 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -625,7 +625,14 @@ find_fw_domain(struct drm_i915_private *dev_priv, u32 offset)
>  			dev_priv->uncore.fw_domains_table_entries,
>  			fw_range_cmp);
>
> -	return entry ? entry->domains : 0;
> +	if (!entry)
> +		return 0;
> +
> +	WARN(entry->domains & ~dev_priv->uncore.fw_domains,
> +	     "Uninitialized forcewake domain(s) 0x%x accessed at 0x%x\n",
> +	     entry->domains & ~dev_priv->uncore.fw_domains, offset);
> +
> +	return entry->domains;
>  }
>
>  static void
>

Only slight issues I can spot is that for some platforms, should it 
trigger, it would trigger twice since 
intel_uncore_forcewake_for_read/write functions have the same WARN.

If we wanted to avoid that then this WARN would work better in 
fwtable_read/write I think.

Regards,

Tvrtko

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

Re: [PATCH v2] drm/i915: Catch non-existent registers in find_fw_domain

[Chris Wilson]

On Wed, Dec 07, 2016 at 06:18:02PM +0000, Tvrtko Ursulin wrote:
> 
> On 07/12/2016 14:22, Joonas Lahtinen wrote:
> >Add WARN_ON to find_fw_domain to registers related to uninitialized
> >hardware.
> >
> >v2:
> >- Print the uninitialized domains and register (Chris)
> >
> >Cc: Imre Deak <imre.deak@intel.com>
> >Cc: Wang Elaine <elaine.wang@intel.com>
> >Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >---
> > drivers/gpu/drm/i915/intel_uncore.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> >index 07779d0..88f3611 100644
> >--- a/drivers/gpu/drm/i915/intel_uncore.c
> >+++ b/drivers/gpu/drm/i915/intel_uncore.c
> >@@ -625,7 +625,14 @@ find_fw_domain(struct drm_i915_private *dev_priv, u32 offset)
> > 			dev_priv->uncore.fw_domains_table_entries,
> > 			fw_range_cmp);
> >
> >-	return entry ? entry->domains : 0;
> >+	if (!entry)
> >+		return 0;
> >+
> >+	WARN(entry->domains & ~dev_priv->uncore.fw_domains,
> >+	     "Uninitialized forcewake domain(s) 0x%x accessed at 0x%x\n",
> >+	     entry->domains & ~dev_priv->uncore.fw_domains, offset);
> >+
> >+	return entry->domains;
> > }
> >
> > static void
> >
> 
> Only slight issues I can spot is that for some platforms, should it
> trigger, it would trigger twice since
> intel_uncore_forcewake_for_read/write functions have the same WARN.

I don't think that's too much of an issue whilst both are WARN() - we
are likely to get a flood of hits if we get any.
-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 v2] drm/i915: Catch non-existent registers in find_fw_domain

[Joonas Lahtinen]

On ke, 2016-12-07 at 18:28 +0000, Chris Wilson wrote:
> On Wed, Dec 07, 2016 at 06:18:02PM +0000, Tvrtko Ursulin wrote:
> > 
> > Only slight issues I can spot is that for some platforms, should it
> > trigger, it would trigger twice since
> > intel_uncore_forcewake_for_read/write functions have the same WARN.
> 
> I don't think that's too much of an issue whilst both are WARN() - we
> are likely to get a flood of hits if we get any.

Yep, and they have slightly differing calling paths. Merged the patch,
thanks for the review.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx