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

[Ben Widawsky <benjamin.widawsky@intel.com>]

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 <benjamin.widawsky@intel.com>
>Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

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