| Message ID | 20210128192413.1715802-4-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Preliminary Display13 support | expand |
On Thu, Jan 28, 2021 at 11:23:58AM -0800, Matt Roper wrote: >Display13 brings enhanced underrun recovery: the hardware can somewhat >mitigate underruns by using an interpolated replacement pixel (soft >underrun) or the previous pixel (hard underrun). Furthermore, underruns >can now be caused downstream by the port, even if the pipe itself is >operating properly. The interrupt register gives us extra bits to >determine hard/soft underruns and whether the underrun was caused by the >port, so let's pass the iir down to the underrun handler and print some >more descriptive errors on Display13 platforms. > >The context of the underrun is also available via PIPE_STATUS, but since >we have the same information in the IIR we don't have a need to read >from there. PIPE_STATUS might be useful in debugfs in the future >though. is this comment outdated? See below... > >Bspec: 50335 >Bspec: 50366 >Cc: Lucas De Marchi <lucas.demarchi@intel.com> >Signed-off-by: Matt Roper <matthew.d.roper@intel.com> >--- > .../drm/i915/display/intel_fifo_underrun.c | 55 ++++++++++++++++++- > drivers/gpu/drm/i915/i915_irq.c | 14 ++++- > drivers/gpu/drm/i915/i915_reg.h | 7 +++ > 3 files changed, 73 insertions(+), 3 deletions(-) > >diff --git a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c >index 813a4f7033e1..6c377f0fc1b3 100644 >--- a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c >+++ b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c >@@ -359,6 +359,39 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv, > return old; > } > >+static u32 >+underrun_pipestat_mask(struct drm_i915_private *dev_priv) >+{ >+ u32 mask = PIPE_FIFO_UNDERRUN_STATUS; >+ >+ if (HAS_DISPLAY13(dev_priv)) >+ mask |= PIPE_STAT_SOFT_UNDERRUN_D13 | >+ PIPE_STAT_HARD_UNDERRUN_D13 | >+ PIPE_STAT_PORT_UNDERRUN_D13; >+ >+ return mask; >+} >+ >+static const char * >+pipe_underrun_reason(u32 pipestat_underruns) >+{ >+ if (pipestat_underruns & PIPE_STAT_SOFT_UNDERRUN_D13) >+ /* >+ * Hardware used replacement/interpolated pixels at >+ * underrun locations. >+ */ >+ return "soft"; >+ else if (pipestat_underruns & PIPE_STAT_HARD_UNDERRUN_D13) >+ /* >+ * Hardware used previous pixel value at underrun >+ * locations. >+ */ >+ return "hard"; >+ else >+ /* Old platform or no extra soft/hard bit set */ >+ return "FIFO"; >+} >+ > /** > * intel_cpu_fifo_underrun_irq_handler - handle CPU fifo underrun interrupt > * @dev_priv: i915 device instance >@@ -372,6 +405,7 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv, > enum pipe pipe) > { > struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe); >+ u32 underruns = 0; > > /* We may be called too early in init, thanks BIOS! */ > if (crtc == NULL) >@@ -382,10 +416,27 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv, > crtc->cpu_fifo_underrun_disabled) > return; > >+ /* >+ * On Display13, we can find out whether an underrun is soft/hard from >+ * either the iir or PIPE_STAT, but we can only determine if underruns >+ * were due to downstream port logic from PIPE_STAT. >+ */ so... we are actually reading PIPE_STAT somce we want to report if it's from downstream port. >+ underruns = intel_uncore_read(&dev_priv->uncore, ICL_PIPESTAT(pipe)) & >+ underrun_pipestat_mask(dev_priv); >+ intel_uncore_write(&dev_priv->uncore, ICL_PIPESTAT(pipe), underruns); maybe I'm missing something, but this doesn't look right to me. We unconditionally read/write ICL_PIPESTAT(pipe), even if it's not display13. Also, the `underruns = 0` initialization is just being overwritten here. intel_cpu_fifo_underrun_irq_handler() is called by very old gens as well. Lucas De Marchi >+ > if (intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false)) { > trace_intel_cpu_fifo_underrun(dev_priv, pipe); >- drm_err(&dev_priv->drm, "CPU pipe %c FIFO underrun\n", >- pipe_name(pipe)); >+ >+ if (underruns & PIPE_STAT_PORT_UNDERRUN_D13) >+ /* Underrun was caused downstream from the pipes */ >+ drm_err(&dev_priv->drm, "Port triggered a %s underrun on pipe %c\n", >+ pipe_underrun_reason(underruns), >+ pipe_name(pipe)); >+ else >+ drm_err(&dev_priv->drm, "CPU pipe %c %s underrun\n", >+ pipe_name(pipe), >+ pipe_underrun_reason(underruns)); > } > > intel_fbc_handle_fifo_underrun_irq(dev_priv); >diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >index 1bced71470a5..407b42706a14 100644 >--- a/drivers/gpu/drm/i915/i915_irq.c >+++ b/drivers/gpu/drm/i915/i915_irq.c >@@ -2389,6 +2389,18 @@ static void gen11_dsi_te_interrupt_handler(struct drm_i915_private *dev_priv, > intel_uncore_write(&dev_priv->uncore, DSI_INTR_IDENT_REG(port), tmp); > } > >+static u32 >+underrun_iir_mask(struct drm_i915_private *dev_priv) >+{ >+ u32 mask = GEN8_PIPE_FIFO_UNDERRUN; >+ >+ if (HAS_DISPLAY13(dev_priv)) >+ mask |= D13_PIPE_SOFT_UNDERRUN | >+ D13_PIPE_HARD_UNDERRUN; >+ >+ return mask; >+} >+ > static irqreturn_t > gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl) > { >@@ -2497,7 +2509,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl) > if (iir & GEN8_PIPE_CDCLK_CRC_DONE) > hsw_pipe_crc_irq_handler(dev_priv, pipe); > >- if (iir & GEN8_PIPE_FIFO_UNDERRUN) >+ if (iir & underrun_iir_mask(dev_priv)) > intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe); > > fault_errors = iir & gen8_de_pipe_fault_mask(dev_priv); >diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >index 10fd0e3af2d4..a57593f7d7b1 100644 >--- a/drivers/gpu/drm/i915/i915_reg.h >+++ b/drivers/gpu/drm/i915/i915_reg.h >@@ -6039,14 +6039,18 @@ enum { > #define PIPECONF_DITHER_TYPE_ST2 (2 << 2) > #define PIPECONF_DITHER_TYPE_TEMP (3 << 2) > #define _PIPEASTAT 0x70024 >+#define _PIPEASTAT_ICL 0x70058 > #define PIPE_FIFO_UNDERRUN_STATUS (1UL << 31) > #define SPRITE1_FLIP_DONE_INT_EN_VLV (1UL << 30) > #define PIPE_CRC_ERROR_ENABLE (1UL << 29) > #define PIPE_CRC_DONE_ENABLE (1UL << 28) >+#define PIPE_STAT_SOFT_UNDERRUN_D13 (1UL << 28) > #define PERF_COUNTER2_INTERRUPT_EN (1UL << 27) > #define PIPE_GMBUS_EVENT_ENABLE (1UL << 27) >+#define PIPE_STAT_HARD_UNDERRUN_D13 (1UL << 27) > #define PLANE_FLIP_DONE_INT_EN_VLV (1UL << 26) > #define PIPE_HOTPLUG_INTERRUPT_ENABLE (1UL << 26) >+#define PIPE_STAT_PORT_UNDERRUN_D13 (1UL << 26) > #define PIPE_VSYNC_INTERRUPT_ENABLE (1UL << 25) > #define PIPE_DISPLAY_LINE_COMPARE_ENABLE (1UL << 24) > #define PIPE_DPST_EVENT_ENABLE (1UL << 23) >@@ -6111,6 +6115,7 @@ enum { > #define PIPEFRAME(pipe) _MMIO_PIPE2(pipe, _PIPEAFRAMEHIGH) > #define PIPEFRAMEPIXEL(pipe) _MMIO_PIPE2(pipe, _PIPEAFRAMEPIXEL) > #define PIPESTAT(pipe) _MMIO_PIPE2(pipe, _PIPEASTAT) >+#define ICL_PIPESTAT(pipe) _MMIO_PIPE2(pipe, _PIPEASTAT_ICL) > > #define _PIPEAGCMAX 0x70010 > #define _PIPEBGCMAX 0x71010 >@@ -7789,6 +7794,8 @@ enum { > #define GEN8_PIPE_FIFO_UNDERRUN (1 << 31) > #define GEN8_PIPE_CDCLK_CRC_ERROR (1 << 29) > #define GEN8_PIPE_CDCLK_CRC_DONE (1 << 28) >+#define D13_PIPE_SOFT_UNDERRUN (1 << 22) >+#define D13_PIPE_HARD_UNDERRUN (1 << 21) > #define GEN8_PIPE_CURSOR_FAULT (1 << 10) > #define GEN8_PIPE_SPRITE_FAULT (1 << 9) > #define GEN8_PIPE_PRIMARY_FAULT (1 << 8) >-- >2.25.4 > >_______________________________________________ >Intel-gfx mailing list >Intel-gfx@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Jan 28, 2021 at 11:23:58AM -0800, Matt Roper wrote: > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 10fd0e3af2d4..a57593f7d7b1 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6039,14 +6039,18 @@ enum { > #define PIPECONF_DITHER_TYPE_ST2 (2 << 2) > #define PIPECONF_DITHER_TYPE_TEMP (3 << 2) > #define _PIPEASTAT 0x70024 > +#define _PIPEASTAT_ICL 0x70058 PIPESTAT is a gmch thing. This is not that for sure. > #define PIPE_FIFO_UNDERRUN_STATUS (1UL << 31) > #define SPRITE1_FLIP_DONE_INT_EN_VLV (1UL << 30) > #define PIPE_CRC_ERROR_ENABLE (1UL << 29) > #define PIPE_CRC_DONE_ENABLE (1UL << 28) > +#define PIPE_STAT_SOFT_UNDERRUN_D13 (1UL << 28) > #define PERF_COUNTER2_INTERRUPT_EN (1UL << 27) > #define PIPE_GMBUS_EVENT_ENABLE (1UL << 27) > +#define PIPE_STAT_HARD_UNDERRUN_D13 (1UL << 27) > #define PLANE_FLIP_DONE_INT_EN_VLV (1UL << 26) > #define PIPE_HOTPLUG_INTERRUPT_ENABLE (1UL << 26) > +#define PIPE_STAT_PORT_UNDERRUN_D13 (1UL << 26) > #define PIPE_VSYNC_INTERRUPT_ENABLE (1UL << 25) > #define PIPE_DISPLAY_LINE_COMPARE_ENABLE (1UL << 24) > #define PIPE_DPST_EVENT_ENABLE (1UL << 23) > @@ -6111,6 +6115,7 @@ enum { > #define PIPEFRAME(pipe) _MMIO_PIPE2(pipe, _PIPEAFRAMEHIGH) > #define PIPEFRAMEPIXEL(pipe) _MMIO_PIPE2(pipe, _PIPEAFRAMEPIXEL) > #define PIPESTAT(pipe) _MMIO_PIPE2(pipe, _PIPEASTAT) > +#define ICL_PIPESTAT(pipe) _MMIO_PIPE2(pipe, _PIPEASTAT_ICL) > > #define _PIPEAGCMAX 0x70010 > #define _PIPEBGCMAX 0x71010 > @@ -7789,6 +7794,8 @@ enum { > #define GEN8_PIPE_FIFO_UNDERRUN (1 << 31) > #define GEN8_PIPE_CDCLK_CRC_ERROR (1 << 29) > #define GEN8_PIPE_CDCLK_CRC_DONE (1 << 28) > +#define D13_PIPE_SOFT_UNDERRUN (1 << 22) > +#define D13_PIPE_HARD_UNDERRUN (1 << 21) > #define GEN8_PIPE_CURSOR_FAULT (1 << 10) > #define GEN8_PIPE_SPRITE_FAULT (1 << 9) > #define GEN8_PIPE_PRIMARY_FAULT (1 << 8) > -- > 2.25.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c index 813a4f7033e1..6c377f0fc1b3 100644 --- a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c +++ b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c @@ -359,6 +359,39 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv, return old; } +static u32 +underrun_pipestat_mask(struct drm_i915_private *dev_priv) +{ + u32 mask = PIPE_FIFO_UNDERRUN_STATUS; + + if (HAS_DISPLAY13(dev_priv)) + mask |= PIPE_STAT_SOFT_UNDERRUN_D13 | + PIPE_STAT_HARD_UNDERRUN_D13 | + PIPE_STAT_PORT_UNDERRUN_D13; + + return mask; +} + +static const char * +pipe_underrun_reason(u32 pipestat_underruns) +{ + if (pipestat_underruns & PIPE_STAT_SOFT_UNDERRUN_D13) + /* + * Hardware used replacement/interpolated pixels at + * underrun locations. + */ + return "soft"; + else if (pipestat_underruns & PIPE_STAT_HARD_UNDERRUN_D13) + /* + * Hardware used previous pixel value at underrun + * locations. + */ + return "hard"; + else + /* Old platform or no extra soft/hard bit set */ + return "FIFO"; +} + /** * intel_cpu_fifo_underrun_irq_handler - handle CPU fifo underrun interrupt * @dev_priv: i915 device instance @@ -372,6 +405,7 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv, enum pipe pipe) { struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe); + u32 underruns = 0; /* We may be called too early in init, thanks BIOS! */ if (crtc == NULL) @@ -382,10 +416,27 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv, crtc->cpu_fifo_underrun_disabled) return; + /* + * On Display13, we can find out whether an underrun is soft/hard from + * either the iir or PIPE_STAT, but we can only determine if underruns + * were due to downstream port logic from PIPE_STAT. + */ + underruns = intel_uncore_read(&dev_priv->uncore, ICL_PIPESTAT(pipe)) & + underrun_pipestat_mask(dev_priv); + intel_uncore_write(&dev_priv->uncore, ICL_PIPESTAT(pipe), underruns); + if (intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false)) { trace_intel_cpu_fifo_underrun(dev_priv, pipe); - drm_err(&dev_priv->drm, "CPU pipe %c FIFO underrun\n", - pipe_name(pipe)); + + if (underruns & PIPE_STAT_PORT_UNDERRUN_D13) + /* Underrun was caused downstream from the pipes */ + drm_err(&dev_priv->drm, "Port triggered a %s underrun on pipe %c\n", + pipe_underrun_reason(underruns), + pipe_name(pipe)); + else + drm_err(&dev_priv->drm, "CPU pipe %c %s underrun\n", + pipe_name(pipe), + pipe_underrun_reason(underruns)); } intel_fbc_handle_fifo_underrun_irq(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 1bced71470a5..407b42706a14 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2389,6 +2389,18 @@ static void gen11_dsi_te_interrupt_handler(struct drm_i915_private *dev_priv, intel_uncore_write(&dev_priv->uncore, DSI_INTR_IDENT_REG(port), tmp); } +static u32 +underrun_iir_mask(struct drm_i915_private *dev_priv) +{ + u32 mask = GEN8_PIPE_FIFO_UNDERRUN; + + if (HAS_DISPLAY13(dev_priv)) + mask |= D13_PIPE_SOFT_UNDERRUN | + D13_PIPE_HARD_UNDERRUN; + + return mask; +} + static irqreturn_t gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl) { @@ -2497,7 +2509,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl) if (iir & GEN8_PIPE_CDCLK_CRC_DONE) hsw_pipe_crc_irq_handler(dev_priv, pipe); - if (iir & GEN8_PIPE_FIFO_UNDERRUN) + if (iir & underrun_iir_mask(dev_priv)) intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe); fault_errors = iir & gen8_de_pipe_fault_mask(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 10fd0e3af2d4..a57593f7d7b1 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6039,14 +6039,18 @@ enum { #define PIPECONF_DITHER_TYPE_ST2 (2 << 2) #define PIPECONF_DITHER_TYPE_TEMP (3 << 2) #define _PIPEASTAT 0x70024 +#define _PIPEASTAT_ICL 0x70058 #define PIPE_FIFO_UNDERRUN_STATUS (1UL << 31) #define SPRITE1_FLIP_DONE_INT_EN_VLV (1UL << 30) #define PIPE_CRC_ERROR_ENABLE (1UL << 29) #define PIPE_CRC_DONE_ENABLE (1UL << 28) +#define PIPE_STAT_SOFT_UNDERRUN_D13 (1UL << 28) #define PERF_COUNTER2_INTERRUPT_EN (1UL << 27) #define PIPE_GMBUS_EVENT_ENABLE (1UL << 27) +#define PIPE_STAT_HARD_UNDERRUN_D13 (1UL << 27) #define PLANE_FLIP_DONE_INT_EN_VLV (1UL << 26) #define PIPE_HOTPLUG_INTERRUPT_ENABLE (1UL << 26) +#define PIPE_STAT_PORT_UNDERRUN_D13 (1UL << 26) #define PIPE_VSYNC_INTERRUPT_ENABLE (1UL << 25) #define PIPE_DISPLAY_LINE_COMPARE_ENABLE (1UL << 24) #define PIPE_DPST_EVENT_ENABLE (1UL << 23) @@ -6111,6 +6115,7 @@ enum { #define PIPEFRAME(pipe) _MMIO_PIPE2(pipe, _PIPEAFRAMEHIGH) #define PIPEFRAMEPIXEL(pipe) _MMIO_PIPE2(pipe, _PIPEAFRAMEPIXEL) #define PIPESTAT(pipe) _MMIO_PIPE2(pipe, _PIPEASTAT) +#define ICL_PIPESTAT(pipe) _MMIO_PIPE2(pipe, _PIPEASTAT_ICL) #define _PIPEAGCMAX 0x70010 #define _PIPEBGCMAX 0x71010 @@ -7789,6 +7794,8 @@ enum { #define GEN8_PIPE_FIFO_UNDERRUN (1 << 31) #define GEN8_PIPE_CDCLK_CRC_ERROR (1 << 29) #define GEN8_PIPE_CDCLK_CRC_DONE (1 << 28) +#define D13_PIPE_SOFT_UNDERRUN (1 << 22) +#define D13_PIPE_HARD_UNDERRUN (1 << 21) #define GEN8_PIPE_CURSOR_FAULT (1 << 10) #define GEN8_PIPE_SPRITE_FAULT (1 << 9) #define GEN8_PIPE_PRIMARY_FAULT (1 << 8)
Display13 brings enhanced underrun recovery: the hardware can somewhat mitigate underruns by using an interpolated replacement pixel (soft underrun) or the previous pixel (hard underrun). Furthermore, underruns can now be caused downstream by the port, even if the pipe itself is operating properly. The interrupt register gives us extra bits to determine hard/soft underruns and whether the underrun was caused by the port, so let's pass the iir down to the underrun handler and print some more descriptive errors on Display13 platforms. The context of the underrun is also available via PIPE_STATUS, but since we have the same information in the IIR we don't have a need to read from there. PIPE_STATUS might be useful in debugfs in the future though. Bspec: 50335 Bspec: 50366 Cc: Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- .../drm/i915/display/intel_fifo_underrun.c | 55 ++++++++++++++++++- drivers/gpu/drm/i915/i915_irq.c | 14 ++++- drivers/gpu/drm/i915/i915_reg.h | 7 +++ 3 files changed, 73 insertions(+), 3 deletions(-)