[PATCH] drm/i915: fix display ver 12-13 fault error handling

[PATCH] drm/i915: fix display ver 12-13 fault error handling

[Jani Nikula]

Unless I'm completely misreading the bspec, there are no defined bits
for plane gtt fault errors in DE PIPE IIR for a display versions
12-14. This would explain why DG2 in the linked bug is getting thousands
of fault errors.

Bspec: 50335
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9769
Fixes: 99e2d8bcb887 ("drm/i915/rkl: Limit number of universal planes to 5")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: <stable@vger.kernel.org> # v5.9+
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_irq.c | 4 +++-
 drivers/gpu/drm/i915/i915_reg.h                  | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
index f8ed53f30b2e..7bede5b56286 100644
--- a/drivers/gpu/drm/i915/display/intel_display_irq.c
+++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
@@ -834,7 +834,9 @@ static u32 gen8_de_port_aux_mask(struct drm_i915_private *dev_priv)
 
 static u32 gen8_de_pipe_fault_mask(struct drm_i915_private *dev_priv)
 {
-	if (DISPLAY_VER(dev_priv) >= 13 || HAS_D12_PLANE_MINIMIZATION(dev_priv))
+	if (DISPLAY_VER(dev_priv) >= 14)
+		return MTL_DE_PIPE_IRQ_FAULT_ERRORS;
+	else if (DISPLAY_VER(dev_priv) >= 12)
 		return RKL_DE_PIPE_IRQ_FAULT_ERRORS;
 	else if (DISPLAY_VER(dev_priv) >= 11)
 		return GEN11_DE_PIPE_IRQ_FAULT_ERRORS;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 27dc903f0553..fcf980694cb4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4354,7 +4354,8 @@
 	 GEN11_PIPE_PLANE7_FAULT | \
 	 GEN11_PIPE_PLANE6_FAULT | \
 	 GEN11_PIPE_PLANE5_FAULT)
-#define RKL_DE_PIPE_IRQ_FAULT_ERRORS \
+#define RKL_DE_PIPE_IRQ_FAULT_ERRORS	0
+#define MTL_DE_PIPE_IRQ_FAULT_ERRORS \
 	(GEN9_DE_PIPE_IRQ_FAULT_ERRORS | \
 	 GEN11_PIPE_PLANE5_FAULT)
 
-- 
2.39.2

Re: [PATCH] drm/i915: fix display ver 12-13 fault error handling

[Jani Nikula]

On Fri, 08 Dec 2023, Jani Nikula <jani.nikula@intel.com> wrote:
> Unless I'm completely misreading the bspec, there are no defined bits
> for plane gtt fault errors in DE PIPE IIR for a display versions
> 12-14. This would explain why DG2 in the linked bug is getting thousands
> of fault errors.
>
> Bspec: 50335
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9769

Okay, looking at the bug, this seems optimistic, but it might clear the
ratelimited fault errors.

> Fixes: 99e2d8bcb887 ("drm/i915/rkl: Limit number of universal planes to 5")
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: <stable@vger.kernel.org> # v5.9+
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_irq.c | 4 +++-
>  drivers/gpu/drm/i915/i915_reg.h                  | 3 ++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
> index f8ed53f30b2e..7bede5b56286 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> @@ -834,7 +834,9 @@ static u32 gen8_de_port_aux_mask(struct drm_i915_private *dev_priv)
>  
>  static u32 gen8_de_pipe_fault_mask(struct drm_i915_private *dev_priv)
>  {
> -	if (DISPLAY_VER(dev_priv) >= 13 || HAS_D12_PLANE_MINIMIZATION(dev_priv))
> +	if (DISPLAY_VER(dev_priv) >= 14)
> +		return MTL_DE_PIPE_IRQ_FAULT_ERRORS;
> +	else if (DISPLAY_VER(dev_priv) >= 12)
>  		return RKL_DE_PIPE_IRQ_FAULT_ERRORS;
>  	else if (DISPLAY_VER(dev_priv) >= 11)
>  		return GEN11_DE_PIPE_IRQ_FAULT_ERRORS;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 27dc903f0553..fcf980694cb4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4354,7 +4354,8 @@
>  	 GEN11_PIPE_PLANE7_FAULT | \
>  	 GEN11_PIPE_PLANE6_FAULT | \
>  	 GEN11_PIPE_PLANE5_FAULT)
> -#define RKL_DE_PIPE_IRQ_FAULT_ERRORS \
> +#define RKL_DE_PIPE_IRQ_FAULT_ERRORS	0
> +#define MTL_DE_PIPE_IRQ_FAULT_ERRORS \
>  	(GEN9_DE_PIPE_IRQ_FAULT_ERRORS | \
>  	 GEN11_PIPE_PLANE5_FAULT)

-- 
Jani Nikula, Intel

Re: [PATCH] drm/i915: fix display ver 12-13 fault error handling

[Matt Roper]

On Fri, Dec 08, 2023 at 01:20:08PM +0200, Jani Nikula wrote:
> Unless I'm completely misreading the bspec, there are no defined bits
> for plane gtt fault errors in DE PIPE IIR for a display versions
> 12-14. This would explain why DG2 in the linked bug is getting thousands
> of fault errors.

I think you might be misreading the spec?  On TGL, bits 7-11 are listed
as plane1-4+cursor fault status, and bits 20-22 are listed as plane 5-7
fault status.  Bits 7-11 are tagged with a REMOVEDBY tag that eventually
drops them for MTL onward, and bits 20-22 are tagged with a REMOVEDBY
tag that drops them for RKL onward (which makes sense because those
extra planes stopped existing at that point).

Maybe the bspec's way of displaying things is what's causing the
confusion?  When you see

        REMOVEDBY(xxxx)
        [ Foo, Bar, Baz ]

The "Foo, Bar, Baz" platform list is the *remaining* list of platforms
after the removal is taken into account, not the platforms actually
being removed.  You can hover over the REMOVEDBY link to see the actual
platforms being removed and a link to the change record for that.


Matt

> 
> Bspec: 50335
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9769
> Fixes: 99e2d8bcb887 ("drm/i915/rkl: Limit number of universal planes to 5")
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: <stable@vger.kernel.org> # v5.9+
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_irq.c | 4 +++-
>  drivers/gpu/drm/i915/i915_reg.h                  | 3 ++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
> index f8ed53f30b2e..7bede5b56286 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> @@ -834,7 +834,9 @@ static u32 gen8_de_port_aux_mask(struct drm_i915_private *dev_priv)
>  
>  static u32 gen8_de_pipe_fault_mask(struct drm_i915_private *dev_priv)
>  {
> -	if (DISPLAY_VER(dev_priv) >= 13 || HAS_D12_PLANE_MINIMIZATION(dev_priv))
> +	if (DISPLAY_VER(dev_priv) >= 14)
> +		return MTL_DE_PIPE_IRQ_FAULT_ERRORS;
> +	else if (DISPLAY_VER(dev_priv) >= 12)
>  		return RKL_DE_PIPE_IRQ_FAULT_ERRORS;
>  	else if (DISPLAY_VER(dev_priv) >= 11)
>  		return GEN11_DE_PIPE_IRQ_FAULT_ERRORS;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 27dc903f0553..fcf980694cb4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4354,7 +4354,8 @@
>  	 GEN11_PIPE_PLANE7_FAULT | \
>  	 GEN11_PIPE_PLANE6_FAULT | \
>  	 GEN11_PIPE_PLANE5_FAULT)
> -#define RKL_DE_PIPE_IRQ_FAULT_ERRORS \
> +#define RKL_DE_PIPE_IRQ_FAULT_ERRORS	0
> +#define MTL_DE_PIPE_IRQ_FAULT_ERRORS \
>  	(GEN9_DE_PIPE_IRQ_FAULT_ERRORS | \
>  	 GEN11_PIPE_PLANE5_FAULT)
>  
> -- 
> 2.39.2
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Re: [PATCH] drm/i915: fix display ver 12-13 fault error handling

[Jani Nikula]

On Fri, 08 Dec 2023, Matt Roper <matthew.d.roper@intel.com> wrote:
> On Fri, Dec 08, 2023 at 01:20:08PM +0200, Jani Nikula wrote:
>> Unless I'm completely misreading the bspec, there are no defined bits
>> for plane gtt fault errors in DE PIPE IIR for a display versions
>> 12-14. This would explain why DG2 in the linked bug is getting thousands
>> of fault errors.
>
> I think you might be misreading the spec?  On TGL, bits 7-11 are listed
> as plane1-4+cursor fault status, and bits 20-22 are listed as plane 5-7
> fault status.  Bits 7-11 are tagged with a REMOVEDBY tag that eventually
> drops them for MTL onward, and bits 20-22 are tagged with a REMOVEDBY
> tag that drops them for RKL onward (which makes sense because those
> extra planes stopped existing at that point).
>
> Maybe the bspec's way of displaying things is what's causing the
> confusion?  When you see
>
>         REMOVEDBY(xxxx)
>         [ Foo, Bar, Baz ]
>
> The "Foo, Bar, Baz" platform list is the *remaining* list of platforms
> after the removal is taken into account, not the platforms actually
> being removed.  You can hover over the REMOVEDBY link to see the actual
> platforms being removed and a link to the change record for that.

Yeah. It's not one of the more clear register specs out there. Sorry for
the noise, and thanks for taking the time to explain this.

BR,
Jani.


-- 
Jani Nikula, Intel