[PATCH 1/2] drm/i915: Clear per-engine fault register as early as possible

[PATCH 1/2] drm/i915: Clear per-engine fault register as early as possible

[Chris Wilson]

From gen6, the hardware tracks address lookup failures and so that we do
not trigger false positives from errors before we are initialised we
clear those upon startup (intel_uncore_early_sanitize()). However, this
is actually before we have the engines defined and this turns out to be
a nop. The earliest we can do so is inside intel_engine_setup().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index e28873cb0672..251e125a214f 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -97,6 +97,9 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 	engine->mmio_base = info->mmio_base;
 	engine->irq_shift = info->irq_shift;
 
+	if (INTEL_GEN(dev_priv) >= 6)
+		I915_WRITE(RING_FAULT_REG(engine), 0);
+
 	return engine;
 }
 
-- 
2.8.1

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

[PATCH 2/2] drm/i915: Fix use of engine->index for register offset

[Chris Wilson]

Since commit de1add360522 ("drm/i915: Decouple execbuf uAPI from internal
implementation") the index of the engine (its engine->id) in the
internal list no longer matches the hardware id. However, in a couple of
locations we missed fixing up the difference. In this case,
RING_FAULT_REG() refers to engine->id which is now not what the register
offset actually should be. Fortunately, in both case we should be more
or less looping over 0..I915_NUM_ENGINES.

Fixes: de1add360522 ("drm/i915: Decouple execbuf uAPI from internal...")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9397ddec26b9..3fe75966539d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1647,7 +1647,7 @@ enum skl_disp_power_wells {
 #define   ARB_MODE_BWGTLB_DISABLE (1<<9)
 #define   ARB_MODE_SWIZZLE_BDW	(1<<1)
 #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
-#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100*(engine)->id)
+#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100*(engine)->hw_id)
 #define   RING_FAULT_GTTSEL_MASK (1<<11)
 #define   RING_FAULT_SRCID(x)	(((x) >> 3) & 0xff)
 #define   RING_FAULT_FAULT_TYPE(x) (((x) >> 1) & 0x3)
-- 
2.8.1

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

Re: [PATCH 2/2] drm/i915: Fix use of engine->index for register offset

[Joonas Lahtinen]

On ke, 2016-07-27 at 19:11 +0100, Chris Wilson wrote:
> Since commit de1add360522 ("drm/i915: Decouple execbuf uAPI from internal
> implementation") the index of the engine (its engine->id) in the
> internal list no longer matches the hardware id. However, in a couple of
> locations we missed fixing up the difference. In this case,
> RING_FAULT_REG() refers to engine->id which is now not what the register
> offset actually should be. Fortunately, in both case we should be more
> or less looping over 0..I915_NUM_ENGINES.
> 
> Fixes: de1add360522 ("drm/i915: Decouple execbuf uAPI from internal...")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>

Seems to be the only instance,

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas

> ---
>  drivers/gpu/drm/i915/i915_reg.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9397ddec26b9..3fe75966539d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1647,7 +1647,7 @@ enum skl_disp_power_wells {
>  #define   ARB_MODE_BWGTLB_DISABLE (1<<9)
>  #define   ARB_MODE_SWIZZLE_BDW	(1<<1)
>  #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
> -#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100*(engine)->id)
> +#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100*(engine)->hw_id)
>  #define   RING_FAULT_GTTSEL_MASK (1<<11)
>  #define   RING_FAULT_SRCID(x)	(((x) >> 3) & 0xff)
>  #define   RING_FAULT_FAULT_TYPE(x) (((x) >> 1) & 0x3)
-- 
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

✗ Ro.CI.BAT: failure for series starting with [1/2] drm/i915: Clear per-engine fault register as early as possible

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Clear per-engine fault register as early as possible
URL   : https://patchwork.freedesktop.org/series/10326/
State : failure

== Summary ==

Series 10326v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/10326/revisions/1/mbox

Test drv_module_reload_basic:
                pass       -> SKIP       (fi-skl-i5-6260u)
Test gem_exec_gttfill:
        Subgroup basic:
                pass       -> SKIP       (fi-snb-i7-2600)
Test kms_cursor_legacy:
        Subgroup basic-flip-vs-cursor-legacy:
                pass       -> FAIL       (ro-hsw-i7-4770r)
                fail       -> PASS       (ro-bdw-i5-5250u)
                fail       -> PASS       (fi-skl-i7-6700k)
        Subgroup basic-flip-vs-cursor-varying-size:
                fail       -> PASS       (ro-snb-i7-2620M)
                pass       -> FAIL       (ro-bdw-i5-5250u)

fi-hsw-i7-4770k  total:239  pass:217  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-i5-6260u  total:239  pass:222  dwarn:0   dfail:0   fail:2   skip:15 
fi-skl-i7-6700k  total:239  pass:208  dwarn:0   dfail:0   fail:3   skip:28 
fi-snb-i7-2600   total:239  pass:196  dwarn:0   dfail:0   fail:0   skip:43 
ro-bdw-i5-5250u  total:239  pass:218  dwarn:4   dfail:0   fail:1   skip:16 
ro-bdw-i7-5600u  total:239  pass:206  dwarn:0   dfail:0   fail:1   skip:32 
ro-bsw-n3050     total:239  pass:194  dwarn:0   dfail:0   fail:3   skip:42 
ro-byt-n2820     total:239  pass:196  dwarn:0   dfail:0   fail:3   skip:40 
ro-hsw-i3-4010u  total:239  pass:213  dwarn:0   dfail:0   fail:0   skip:26 
ro-hsw-i7-4770r  total:239  pass:212  dwarn:0   dfail:0   fail:1   skip:26 
ro-ilk-i7-620lm  total:239  pass:172  dwarn:1   dfail:0   fail:1   skip:65 
ro-ilk1-i5-650   total:234  pass:172  dwarn:0   dfail:0   fail:2   skip:60 
ro-ivb-i7-3770   total:239  pass:204  dwarn:0   dfail:0   fail:0   skip:35 
ro-ivb2-i7-3770  total:239  pass:208  dwarn:0   dfail:0   fail:0   skip:31 
ro-skl3-i5-6260u total:239  pass:221  dwarn:0   dfail:0   fail:4   skip:14 
ro-snb-i7-2620M  total:239  pass:197  dwarn:0   dfail:0   fail:1   skip:41 
fi-kbl-qkkr failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1625/

e52c742 drm-intel-nightly: 2016y-07m-27d-15h-23m-38s UTC integration manifest
5e7bf1b drm/i915: Fix use of engine->index for register offset
2ea0a6f drm/i915: Clear per-engine fault register as early as possible

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

Re: [PATCH 1/2] drm/i915: Clear per-engine fault register as early as possible

[Joonas Lahtinen]

On ke, 2016-07-27 at 19:11 +0100, Chris Wilson wrote:
> From gen6, the hardware tracks address lookup failures and so that we do
> not trigger false positives from errors before we are initialised we
> clear those upon startup (intel_uncore_early_sanitize()). However, this
> is actually before we have the engines defined and this turns out to be
> a nop. The earliest we can do so is inside intel_engine_setup().
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>

Documentation I found on this was poor, so I'd prefer a Tested-by: tag
from somebody (wide platform coverage). But codewise it's consistent
with existing usage.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index e28873cb0672..251e125a214f 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -97,6 +97,9 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
>  	engine->mmio_base = info->mmio_base;
>  	engine->irq_shift = info->irq_shift;
>  
> +	if (INTEL_GEN(dev_priv) >= 6)
> +		I915_WRITE(RING_FAULT_REG(engine), 0);
> +
>  	return engine;
>  }
>  
-- 
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

Re: [PATCH 1/2] drm/i915: Clear per-engine fault register as early as possible

[Dave Gordon]

On 28/07/16 11:12, Joonas Lahtinen wrote:
> On ke, 2016-07-27 at 19:11 +0100, Chris Wilson wrote:
>> From gen6, the hardware tracks address lookup failures and so that we do
>> not trigger false positives from errors before we are initialised we
>> clear those upon startup (intel_uncore_early_sanitize()). However, this
>> is actually before we have the engines defined and this turns out to be
>> a nop. The earliest we can do so is inside intel_engine_setup().
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>
> Documentation I found on this was poor, so I'd prefer a Tested-by: tag
> from somebody (wide platform coverage). But codewise it's consistent
> with existing usage.
>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

AFAICT from the BSpec, only IVB-HSW have this scheme (there are at most 
four such registers 0x4[0-3]94, with 0x4494 being something different).

Thereafter (BDW+) there's just ONE fault register (0x4094) replacing all 
the separate ones, with a field showing which engine the rest of the 
content relates to.

So pretty much all code using this definition will be wrong on GEN8+; it 
will access undefined or nonexistent registers :-(

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

[PATCH 1/2] drm/i915: Clear per-engine fault register as early as possible

[Michel Thierry]

From gen6, the hardware tracks address lookup failures and we should
clear those registers upon startup to prevent false positives. However,
this was happening before we have the engines defined (intel_uncore_init())
and the for_each_engine loop was just a nop. The earliest we can call
this is inside intel_engines_init_mmio().

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 2 ++
 drivers/gpu/drm/i915/intel_uncore.c    | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index a0f9d0eb4bce..70bbe8ef8f54 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -289,6 +289,8 @@ int intel_engines_init_mmio(struct drm_i915_private *dev_priv)
 
 	device_info->num_rings = hweight32(mask);
 
+	i915_check_and_clear_faults(dev_priv);
+
 	return 0;
 
 cleanup:
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 211acee7c31d..a78ceafcc825 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1420,8 +1420,6 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 
 	iosf_mbi_register_pmic_bus_access_notifier(
 		&dev_priv->uncore.pmic_bus_access_nb);
-
-	i915_check_and_clear_faults(dev_priv);
 }
 
 void intel_uncore_fini(struct drm_i915_private *dev_priv)
-- 
2.15.0

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