Re: [Intel-gfx] [PATCH] drm/i915: Force uncached PPAT for debugging purposes.

[Vivi, Rodrigo]

On Tue, 2017-02-28 at 09:05 +, Chris Wilson wrote:
> On Mon, Feb 27, 2017 at 05:18:08PM -0800, Ben Widawsky wrote:
> > On 17-02-27 17:12:07, Rodrigo Vivi wrote:
> > >Many screen corruptions and hangs in the past were somehow
> > >related to the caches. In many situations forcing the uncached
> > >was useful at least to narrow down the issue by confirming it
> > >was cache related.
> > >
> > >Instead of having to hardcode it everytime that we suspect on
> > >this table let's provide a mechanism to disable these
> > >cache leves on this private table (PPAT).
> > >
> > >Cc: Ben Widawsky 
> > >Cc: Daniele Ceraolo Spurio 
> > >Signed-off-by: Rodrigo Vivi 
> > 
> > I think this is a cool idea, it could equally be achieved by modifying the 
> > PTE
> > encoding function. In my head, modifying the pte encode makes more sense 
> > since
> > it applies to all legacy context, advanced context, and ggtt entries.
> 
> Changing the mocs table is not the same as changing the PTE. Both are
> useful for different types of bugs.

Indeed.

Although I believe Ben mentioned the pat_sel inside PTE. But if this is
the case I checked here and it seems that it is spread in different
functions here so I believe the simplest and safiest place to do is here
inside the PPAT setup.

> -Chris
> 

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

Re: [Intel-gfx] [PATCH] drm/i915: Force uncached PPAT for debugging purposes.

[Chris Wilson]

On Mon, Feb 27, 2017 at 05:18:08PM -0800, Ben Widawsky wrote:
> On 17-02-27 17:12:07, Rodrigo Vivi wrote:
> >Many screen corruptions and hangs in the past were somehow
> >related to the caches. In many situations forcing the uncached
> >was useful at least to narrow down the issue by confirming it
> >was cache related.
> >
> >Instead of having to hardcode it everytime that we suspect on
> >this table let's provide a mechanism to disable these
> >cache leves on this private table (PPAT).
> >
> >Cc: Ben Widawsky 
> >Cc: Daniele Ceraolo Spurio 
> >Signed-off-by: Rodrigo Vivi 
> 
> I think this is a cool idea, it could equally be achieved by modifying the PTE
> encoding function. In my head, modifying the pte encode makes more sense since
> it applies to all legacy context, advanced context, and ggtt entries.

Changing the mocs table is not the same as changing the PTE. Both are
useful for different types of bugs.
-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: [Intel-gfx] [PATCH] drm/i915: Force uncached PPAT for debugging purposes.

[Ben Widawsky]

On 17-02-27 17:12:07, Rodrigo Vivi wrote:

Many screen corruptions and hangs in the past were somehow
related to the caches. In many situations forcing the uncached
was useful at least to narrow down the issue by confirming it
was cache related.

Instead of having to hardcode it everytime that we suspect on
this table let's provide a mechanism to disable these
cache leves on this private table (PPAT).

Cc: Ben Widawsky 
Cc: Daniele Ceraolo Spurio 
Signed-off-by: Rodrigo Vivi 


I think this is a cool idea, it could equally be achieved by modifying the PTE
encoding function. In my head, modifying the pte encode makes more sense since
it applies to all legacy context, advanced context, and ggtt entries.


---
drivers/gpu/drm/i915/i915_gem_gtt.c | 6 +-
drivers/gpu/drm/i915/i915_params.c  | 5 +
drivers/gpu/drm/i915/i915_params.h  | 1 +
3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e0c9542..df9f71e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2645,7 +2645,10 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 
size)
 * writing this data shouldn't be harmful even in those cases. */
static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv)
{
-   u64 pat;
+   u64 pat = 0;
+
+   if (i915.uncached_pat)
+   goto out;

pat = GEN8_PPAT(0, GEN8_PPAT_WB | GEN8_PPAT_LLC) | /* for normal 
objects, no eLLC */
  GEN8_PPAT(1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC) | /* for something 
pointing to ptes? */
@@ -2672,6 +2675,7 @@ static void bdw_setup_private_ppat(struct 
drm_i915_private *dev_priv)
 */
pat = GEN8_PPAT(0, GEN8_PPAT_UC);

+out:
/* XXX: spec defines this as 2 distinct registers. It's unclear if a 64b
 * write would work. */
I915_WRITE(GEN8_PRIVATE_PAT_LO, pat);
diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index 2e9645e..505afda 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -37,6 +37,7 @@ struct i915_params i915 __read_mostly = {
.enable_fbc = -1,
.enable_execlists = -1,
.enable_hangcheck = true,
+   .uncached_pat = false,
.enable_ppgtt = -1,
.enable_psr = -1,
.alpha_support = IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT),
@@ -130,6 +131,10 @@ struct i915_params i915 __read_mostly = {
"WARNING: Disabling this can cause system wide hangs. "
"(default: true)");

+module_param_named_unsafe(uncached_pat, i915.uncached_pat, bool, 0400);
+MODULE_PARM_DESC(uncached_pat,
+   "Force Uncached Private PPAT for debugging purposes. (default:false)");
+
module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, 0400);
MODULE_PARM_DESC(enable_ppgtt,
"Override PPGTT usage. "
diff --git a/drivers/gpu/drm/i915/i915_params.h 
b/drivers/gpu/drm/i915/i915_params.h
index 55d47ee..2453431 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -54,6 +54,7 @@
func(bool, alpha_support); \
func(bool, enable_cmd_parser); \
func(bool, enable_hangcheck); \
+   func(bool, uncached_pat); \
func(bool, fastboot); \
func(bool, prefault_disable); \
func(bool, load_detect_test); \
--
1.9.1



--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Force uncached PPAT for debugging purposes.

[Rodrigo Vivi]

Many screen corruptions and hangs in the past were somehow
related to the caches. In many situations forcing the uncached
was useful at least to narrow down the issue by confirming it
was cache related.

Instead of having to hardcode it everytime that we suspect on
this table let's provide a mechanism to disable these
cache leves on this private table (PPAT).

Cc: Ben Widawsky 
Cc: Daniele Ceraolo Spurio 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 6 +-
 drivers/gpu/drm/i915/i915_params.c  | 5 +
 drivers/gpu/drm/i915/i915_params.h  | 1 +
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e0c9542..df9f71e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2645,7 +2645,10 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 
size)
  * writing this data shouldn't be harmful even in those cases. */
 static void bdw_setup_private_ppat(struct drm_i915_private *dev_priv)
 {
-   u64 pat;
+   u64 pat = 0;
+
+   if (i915.uncached_pat)
+   goto out;
 
pat = GEN8_PPAT(0, GEN8_PPAT_WB | GEN8_PPAT_LLC) | /* for normal 
objects, no eLLC */
  GEN8_PPAT(1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC) | /* for something 
pointing to ptes? */
@@ -2672,6 +2675,7 @@ static void bdw_setup_private_ppat(struct 
drm_i915_private *dev_priv)
 */
pat = GEN8_PPAT(0, GEN8_PPAT_UC);
 
+out:
/* XXX: spec defines this as 2 distinct registers. It's unclear if a 64b
 * write would work. */
I915_WRITE(GEN8_PRIVATE_PAT_LO, pat);
diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index 2e9645e..505afda 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -37,6 +37,7 @@ struct i915_params i915 __read_mostly = {
.enable_fbc = -1,
.enable_execlists = -1,
.enable_hangcheck = true,
+   .uncached_pat = false,
.enable_ppgtt = -1,
.enable_psr = -1,
.alpha_support = IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT),
@@ -130,6 +131,10 @@ struct i915_params i915 __read_mostly = {
"WARNING: Disabling this can cause system wide hangs. "
"(default: true)");
 
+module_param_named_unsafe(uncached_pat, i915.uncached_pat, bool, 0400);
+MODULE_PARM_DESC(uncached_pat,
+   "Force Uncached Private PPAT for debugging purposes. (default:false)");
+
 module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, 0400);
 MODULE_PARM_DESC(enable_ppgtt,
"Override PPGTT usage. "
diff --git a/drivers/gpu/drm/i915/i915_params.h 
b/drivers/gpu/drm/i915/i915_params.h
index 55d47ee..2453431 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -54,6 +54,7 @@
func(bool, alpha_support); \
func(bool, enable_cmd_parser); \
func(bool, enable_hangcheck); \
+   func(bool, uncached_pat); \
func(bool, fastboot); \
func(bool, prefault_disable); \
func(bool, load_detect_test); \
-- 
1.9.1

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