On 19.09.2022 15:32, Matt Roper wrote:
> On Xe_HP the fault registers are now in a multicast register range.
> However as part of the GAM these registers follow special rules and we
> need only read from the "primary" GAM's instance to get the information
> we need. So a single intel_gt_mcr_read_any() (which will automatically
> steer to the primary GAM) is sufficient; we don't need to loop over each
> instance of the MCR register.
>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 40 ++++++++++++++++++++++++-
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 3 ++
> 2 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 5ddae95d4886..1cb7dd40ec47 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -304,6 +304,42 @@ static void gen6_check_faults(struct intel_gt *gt)
> }
> }
>
> +static void xehp_check_faults(struct intel_gt *gt)
> +{
> + u32 fault;
> +
> + /*
> + * Although the fault register now lives in an MCR register range,
> + * the GAM registers are special and we only truly need to read
> + * the "primary" GAM instance rather than handling each instance
> + * individually. intel_gt_mcr_read_any() will automatically steer
> + * toward the primary instance.
> + */
> + fault = intel_gt_mcr_read_any(gt, XEHP_RING_FAULT_REG);
> + if (fault & RING_FAULT_VALID) {
> + u32 fault_data0, fault_data1;
> + u64 fault_addr;
> +
> + fault_data0 = intel_gt_mcr_read_any(gt, XEHP_FAULT_TLB_DATA0);
> + fault_data1 = intel_gt_mcr_read_any(gt, XEHP_FAULT_TLB_DATA1);
> +
> + fault_addr = ((u64)(fault_data1 & FAULT_VA_HIGH_BITS) << 44) |
> + ((u64)fault_data0 << 12);
> +
> + drm_dbg(>->i915->drm, "Unexpected fault\n"
> + "\tAddr: 0x%08x_%08x\n"
> + "\tAddress space: %s\n"
> + "\tEngine ID: %d\n"
> + "\tSource ID: %d\n"
> + "\tType: %d\n",
> + upper_32_bits(fault_addr), lower_32_bits(fault_addr),
> + fault_data1 & FAULT_GTT_SEL ? "GGTT" : "PPGTT",
> + GEN8_RING_FAULT_ENGINE_ID(fault),
> + RING_FAULT_SRCID(fault),
> + RING_FAULT_FAULT_TYPE(fault));
> + }
> +}
> +
> static void gen8_check_faults(struct intel_gt *gt)
> {
> struct intel_uncore *uncore = gt->uncore;
> @@ -350,7 +386,9 @@ void intel_gt_check_and_clear_faults(struct intel_gt *gt)
> struct drm_i915_private *i915 = gt->i915;
>
> /* From GEN8 onwards we only have one 'All Engine Fault Register' */
> - if (GRAPHICS_VER(i915) >= 8)
> + if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
> + xehp_check_faults(gt);
> + else if (GRAPHICS_VER(i915) >= 8)
> gen8_check_faults(gt);
> else if (GRAPHICS_VER(i915) >= 6)
> gen6_check_faults(gt);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index cf87a1b36a21..dff38b0c4430 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1024,11 +1024,14 @@
> #define GEN9_BLT_MOCS(i) _MMIO(__GEN9_BCS0_MOCS0 + (i) * 4)
>
> #define GEN12_FAULT_TLB_DATA0 _MMIO(0xceb8)
> +#define XEHP_FAULT_TLB_DATA0 _MMIO(0xceb8)
> #define GEN12_FAULT_TLB_DATA1 _MMIO(0xcebc)
> +#define XEHP_FAULT_TLB_DATA1 _MMIO(0xcebc)
> #define FAULT_VA_HIGH_BITS (0xf << 0)
> #define FAULT_GTT_SEL (1 << 4)
>
> #define GEN12_RING_FAULT_REG _MMIO(0xcec4)
> +#define XEHP_RING_FAULT_REG _MMIO(0xcec4)
The fault registers GEN12_FAULT_TLB_DATA0, GEN12_FAULT_TLB_DATA1,
GEN12_RING_FAULT_REG are used in few other places in the driver for
platforms including Xe_HP. Don't we need to take care of them?
Regards,
Bala
> #define GEN8_RING_FAULT_ENGINE_ID(x) (((x) >> 12) & 0x7)
> #define RING_FAULT_GTTSEL_MASK (1 << 11)
> #define RING_FAULT_SRCID(x) (((x) >> 3) & 0xff)
> --
> 2.37.3
>
