| Message ID | 20230125185234.21599-6-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Error/underrun interrupt fixes | expand |
Hi Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com> BR vinod PS: With this patch seems my comment for the prev patch in this series might not be relevant. On Wed, 2023-01-25 at 20:52 +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > FBC on gen2/3 seems to trigger page table errors. No visual > artifacts are visible, and essentially the same FBC > code works on gen4 so these seem entirely spurious. There > are also hints in gen3 bspec indicating that certain bits > in PGTBL_ER are just not wired up correctly in the > hardware. > > Ideally we'd want to mask out only the bogus bits, but > sadly there is no mask for PGTBL_ER, and instead we are > forced to mask out all page table errors via EMR :( > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 081b79d00521..496f76bf42f3 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -3473,8 +3473,23 @@ static void i8xx_irq_reset(struct drm_i915_private *dev_priv) > > static u32 i9xx_error_mask(struct drm_i915_private *i915) > { > - return ~(I915_ERROR_PAGE_TABLE | > - I915_ERROR_MEMORY_REFRESH); > + /* > + * On gen2/3 FBC generates (seemingly spurious) > + * display INVALID_GTT/INVALID_GTT_PTE table errors. > + * > + * Also gen3 bspec has this to say: > + * "DISPA_INVALID_GTT_PTE > + " [DevNapa] : Reserved. This bit does not reflect the page > + " table error for the display plane A." > + * > + * Unfortunately we can't mask off individual PGTBL_ER bits, > + * so we just have to mask off all page table errors via EMR. > + */ > + if (HAS_FBC(i915)) > + return ~I915_ERROR_MEMORY_REFRESH; > + else > + return ~(I915_ERROR_PAGE_TABLE | > + I915_ERROR_MEMORY_REFRESH); > } > > static void i8xx_irq_postinstall(struct drm_i915_private *dev_priv) > @@ -3762,6 +3777,9 @@ static u32 i965_error_mask(struct drm_i915_private *i915) > /* > * Enable some error detection, note the instruction error mask > * bit is reserved, so we leave it masked. > + * > + * i965 FBC no longer generates spurious GTT errors, > + * so we can always enable the page table errors. > */ > if (IS_G4X(i915)) > return ~(GM45_ERROR_PAGE_TABLE |
On Thu, Feb 23, 2023 at 12:46:21PM +0000, Govindapillai, Vinod wrote: > Hi > > Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com> > > BR > vinod > > PS: With this patch seems my comment for the prev patch in this series might not be relevant. Aye. On a related note, sometimes I do muse about unifying all the (non-vlv/chv) gmch platform irq code into just one copy, but in this case that wouldn't really change anything anyway since we need different values for different platforms. Thaks for the review. > > On Wed, 2023-01-25 at 20:52 +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > FBC on gen2/3 seems to trigger page table errors. No visual > > artifacts are visible, and essentially the same FBC > > code works on gen4 so these seem entirely spurious. There > > are also hints in gen3 bspec indicating that certain bits > > in PGTBL_ER are just not wired up correctly in the > > hardware. > > > > Ideally we'd want to mask out only the bogus bits, but > > sadly there is no mask for PGTBL_ER, and instead we are > > forced to mask out all page table errors via EMR :( > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_irq.c | 22 ++++++++++++++++++++-- > > 1 file changed, 20 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > index 081b79d00521..496f76bf42f3 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -3473,8 +3473,23 @@ static void i8xx_irq_reset(struct drm_i915_private *dev_priv) > > > > static u32 i9xx_error_mask(struct drm_i915_private *i915) > > { > > - return ~(I915_ERROR_PAGE_TABLE | > > - I915_ERROR_MEMORY_REFRESH); > > + /* > > + * On gen2/3 FBC generates (seemingly spurious) > > + * display INVALID_GTT/INVALID_GTT_PTE table errors. > > + * > > + * Also gen3 bspec has this to say: > > + * "DISPA_INVALID_GTT_PTE > > + " [DevNapa] : Reserved. This bit does not reflect the page > > + " table error for the display plane A." > > + * > > + * Unfortunately we can't mask off individual PGTBL_ER bits, > > + * so we just have to mask off all page table errors via EMR. > > + */ > > + if (HAS_FBC(i915)) > > + return ~I915_ERROR_MEMORY_REFRESH; > > + else > > + return ~(I915_ERROR_PAGE_TABLE | > > + I915_ERROR_MEMORY_REFRESH); > > } > > > > static void i8xx_irq_postinstall(struct drm_i915_private *dev_priv) > > @@ -3762,6 +3777,9 @@ static u32 i965_error_mask(struct drm_i915_private *i915) > > /* > > * Enable some error detection, note the instruction error mask > > * bit is reserved, so we leave it masked. > > + * > > + * i965 FBC no longer generates spurious GTT errors, > > + * so we can always enable the page table errors. > > */ > > if (IS_G4X(i915)) > > return ~(GM45_ERROR_PAGE_TABLE | >
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 081b79d00521..496f76bf42f3 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3473,8 +3473,23 @@ static void i8xx_irq_reset(struct drm_i915_private *dev_priv) static u32 i9xx_error_mask(struct drm_i915_private *i915) { - return ~(I915_ERROR_PAGE_TABLE | - I915_ERROR_MEMORY_REFRESH); + /* + * On gen2/3 FBC generates (seemingly spurious) + * display INVALID_GTT/INVALID_GTT_PTE table errors. + * + * Also gen3 bspec has this to say: + * "DISPA_INVALID_GTT_PTE + " [DevNapa] : Reserved. This bit does not reflect the page + " table error for the display plane A." + * + * Unfortunately we can't mask off individual PGTBL_ER bits, + * so we just have to mask off all page table errors via EMR. + */ + if (HAS_FBC(i915)) + return ~I915_ERROR_MEMORY_REFRESH; + else + return ~(I915_ERROR_PAGE_TABLE | + I915_ERROR_MEMORY_REFRESH); } static void i8xx_irq_postinstall(struct drm_i915_private *dev_priv) @@ -3762,6 +3777,9 @@ static u32 i965_error_mask(struct drm_i915_private *i915) /* * Enable some error detection, note the instruction error mask * bit is reserved, so we leave it masked. + * + * i965 FBC no longer generates spurious GTT errors, + * so we can always enable the page table errors. */ if (IS_G4X(i915)) return ~(GM45_ERROR_PAGE_TABLE |