[Intel-gfx] [PATCH 03/18] drm/i915/display13: Enhanced pipe underrun reporting
Lucas De Marchi
lucas.demarchi at intel.com
Thu Feb 11 00:31:50 UTC 2021
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 at intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper at 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 at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list