[Intel-gfx] [PATCH] drm/i915: Stop getting the fault address from RING_FAULT_REG

[Chris Wilson]

Quoting Oscar Mateo (2017-12-22 22:10:29)
> This register does not contain it. Instead, we have to look into FAULT_TLB_DATA0 & 1
> (where, by the way, we can also get the address space).
> 
> Fixes: b03ec3d67ab8 ("drm/i915: There is only one fault register from GEN8 onwards")
> Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
> Cc: Michel Thierry <michel.thierry at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 12 +++++++++++-
>  drivers/gpu/drm/i915/i915_reg.h     |  2 ++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index c5f3938..2680219 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2287,12 +2287,22 @@ static void gen8_check_and_clear_faults(struct drm_i915_private *dev_priv)
>         u32 fault = I915_READ(GEN8_RING_FAULT_REG);
>  
>         if (fault & RING_FAULT_VALID) {
> +               u32 fault_data0, fault_data1;
> +               u64 fault_addr;
> +
> +               fault_data0 = I915_READ(GEN8_FAULT_TLB_DATA0);
> +               fault_data1 = I915_READ(GEN8_FAULT_TLB_DATA1);
> +               fault_addr = ((u64)(fault_data1 & FAULT_VA_BITS_44_TO_47) << 44) |
> +                            ((u64)fault_data0 << PAGE_SHIFT);

That should be 12 not PAGE_SHIFT. Semantically different, the kernel's
choice of system page size and how the register stores its address.

> +
>                 DRM_DEBUG_DRIVER("Unexpected fault\n"
>                                  "    Addr: 0x%08lx\n"
%0x08lx_%08lx

upper_32_bits(fault_addr), lower_32_bits(fault_addr),

as is, it needs to be llx to prevent compiler warnings on 32b.

> +                                "    Address space: %s\n"
>                                  "    Engine ID: %d\n"
>                                  "    Source ID: %d\n"
>                                  "    Type: %d\n",
> -                                fault & PAGE_MASK,
> +                                fault_addr,
> +                                fault_data1 & FAULT_GTT_SEL ? "GGTT" : "PPGTT",
>                                  GEN8_RING_FAULT_ENGINE_ID(fault),
>                                  RING_FAULT_SRCID(fault),
>                                  RING_FAULT_FAULT_TYPE(fault));
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 41285be..51c16a7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2489,6 +2489,8 @@ enum i915_power_well_id {
>  
>  #define GEN8_FAULT_TLB_DATA0           _MMIO(0x4b10)
>  #define GEN8_FAULT_TLB_DATA1           _MMIO(0x4b14)
> +#define   FAULT_VA_BITS_44_TO_47       (0xf << 0)
HIGHBITS?
-Chris