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@xxxxxxxxx>
Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
---
.../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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
