On Wed, Nov 19, 2025 at 11:06:21AM -0800, Matt Roper wrote:
> On Fri, Nov 14, 2025 at 05:52:11PM -0300, Gustavo Sousa wrote:
> > Xe3p_LPD added several bits containing information that can be relevant
> > to debugging FIFO underruns. Add the logic necessary to handle them
> > when reporting underruns.
> >
> > This was adapted from the initial patch[1] from Sai Teja Pottumuttu.
> >
> > [1]
> > https://lore.kernel.org/all/[email protected]/
> >
> > v2:
> > - Handle FBC-related bits in intel_fbc.c. (Ville)
> > - Do not use intel_fbc_id_for_pipe (promoted from
> > skl_fbc_id_for_pipe), but use pipe's primary plane to get the FBC
> > instance. (Ville, Matt)
> > - Improve code readability by moving logic specific to logging fields
> > of UNDERRUN_DBG1 to a separate function. (Jani)
> >
> > Bspec: 69111, 69561, 74411, 74412
> > Cc: Jani Nikula <[email protected]>
> > Cc: Matt Roper <[email protected]>
> > Cc: Ville Syrjälä <[email protected]>
> > Signed-off-by: Gustavo Sousa <[email protected]>
> > ---
> > I tested this by adding a change on top of this series that updates
> > Xe3p_LPD's CDCLK table to use bad values and I got the following
> > messages:
> >
> > [ +0.000237] xe 0000:00:02.0: [drm:intel_modeset_verify_crtc [xe]]
> > [CRTC:88:pipe A]
> > [ +0.000674] xe 0000:00:02.0: [drm] *ERROR* CPU pipe A FIFO underrun
> > [ +0.000015] xe 0000:00:02.0: [drm] *ERROR* Pipe A FIFO underrun info:
> > DBUF block not valid on planes: [1]
> > [ +0.000001] xe 0000:00:02.0: [drm] *ERROR* Pipe A FIFO underrun info:
> > DDB empty on planes: [1]
> > [ +0.000001] xe 0000:00:02.0: [drm] *ERROR* Pipe A FIFO underrun info:
> > DBUF below WM0 on planes: [1]
> > [ +0.000004] xe 0000:00:02.0: [drm] *ERROR* Pipe A FIFO underrun info:
> > frame count: 1890, line count: 44
> > ---
> > .../gpu/drm/i915/display/intel_display_device.h | 1 +
> > drivers/gpu/drm/i915/display/intel_display_regs.h | 16 +++
> > drivers/gpu/drm/i915/display/intel_fbc.c | 50 ++++++++++
> > drivers/gpu/drm/i915/display/intel_fbc.h | 3 +
> > drivers/gpu/drm/i915/display/intel_fbc_regs.h | 2 +
> > drivers/gpu/drm/i915/display/intel_fifo_underrun.c | 109
> > +++++++++++++++++++++
> > 6 files changed, 181 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
> > b/drivers/gpu/drm/i915/display/intel_display_device.h
> > index b559ef43d547..91d8cfac5eff 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> > @@ -197,6 +197,7 @@ struct intel_display_platforms {
> > #define HAS_TRANSCODER(__display, trans)
> > ((DISPLAY_RUNTIME_INFO(__display)->cpu_transcoder_mask & \
> > BIT(trans)) != 0)
> > #define HAS_UNCOMPRESSED_JOINER(__display) (DISPLAY_VER(__display) >= 13)
> > +#define HAS_UNDERRUN_DBG_INFO(__display) (DISPLAY_VER(__display) >= 35)
> > #define HAS_ULTRAJOINER(__display) (((__display)->platform.dgfx && \
> > DISPLAY_VER(__display) == 14) &&
> > HAS_DSC(__display))
> > #define HAS_VRR(__display) (DISPLAY_VER(__display) >= 11)
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_regs.h
> > b/drivers/gpu/drm/i915/display/intel_display_regs.h
> > index 9d71e26a4fa2..89ea0156ee06 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_regs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_regs.h
> > @@ -882,6 +882,21 @@
> > #define PIPE_MISC2_FLIP_INFO_PLANE_SEL_MASK REG_GENMASK(2,
> > 0) /* tgl+ */
> > #define PIPE_MISC2_FLIP_INFO_PLANE_SEL(plane_id)
> > REG_FIELD_PREP(PIPE_MISC2_FLIP_INFO_PLANE_SEL_MASK, (plane_id))
> >
> > +#define _UNDERRUN_DBG1_A 0x70064
> > +#define _UNDERRUN_DBG1_B 0x71064
> > +#define UNDERRUN_DBG1(pipe) _MMIO_PIPE(pipe,
> > _UNDERRUN_DBG1_A, _UNDERRUN_DBG1_B)
> > +#define UNDERRUN_DBUF_BLOCK_NOT_VALID_MASK REG_GENMASK(29, 24)
> > +#define UNDERRUN_DDB_EMPTY_MASK REG_GENMASK(21, 16)
> > +#define UNDERRUN_DBUF_NOT_FILLED_MASK REG_GENMASK(13, 8)
> > +#define UNDERRUN_BELOW_WM0_MASK REG_GENMASK(5, 0)
> > +
> > +#define _UNDERRUN_DBG2_A 0x70068
> > +#define _UNDERRUN_DBG2_B 0x71068
> > +#define UNDERRUN_DBG2(pipe) _MMIO_PIPE(pipe,
> > _UNDERRUN_DBG2_A, _UNDERRUN_DBG2_B)
> > +#define UNDERRUN_FRAME_LINE_COUNTERS_FROZEN REG_BIT(31)
> > +#define UNDERRUN_PIPE_FRAME_COUNT_MASK REG_GENMASK(30, 20)
> > +#define UNDERRUN_LINE_COUNT_MASK REG_GENMASK(19, 0)
> > +
> > #define DPINVGTT _MMIO(VLV_DISPLAY_BASE +
> > 0x7002c) /* VLV/CHV only */
> > #define DPINVGTT_EN_MASK_CHV REG_GENMASK(27,
> > 16)
> > #define DPINVGTT_EN_MASK_VLV REG_GENMASK(23,
> > 16)
> > @@ -1416,6 +1431,7 @@
> >
> > #define GEN12_DCPR_STATUS_1 _MMIO(0x46440)
> > #define XELPDP_PMDEMAND_INFLIGHT_STATUS REG_BIT(26)
> > +#define XE3P_UNDERRUN_PKGC REG_BIT(21)
> >
> > #define FUSE_STRAP _MMIO(0x42014)
> > #define ILK_INTERNAL_GRAPHICS_DISABLE REG_BIT(31)
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> > b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index 437d2fda20a7..ec316f9843c6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -129,6 +129,25 @@ struct intel_fbc {
> > const char *no_fbc_reason;
> > };
> >
> > +static struct intel_fbc *intel_fbc_for_pipe(struct intel_display *display,
> > enum pipe pipe)
> > +{
> > + struct intel_crtc *crtc = intel_crtc_for_pipe(display, pipe);
> > + struct intel_plane *primary = NULL;
> > + struct intel_plane *plane;
> > +
> > + for_each_intel_plane_on_crtc(display->drm, crtc, plane) {
> > + if (plane->base.type == DRM_PLANE_TYPE_PRIMARY) {
>
> Can't we just use crtc->base.primary rather than looping to find it?
> While it's legal at the DRM layer for modern drivers to not register a
> primary plane (which in turn means no support for some pre-atomic KMS
> APIs), i915 has always always created and registered one in
> intel_crtc_init(). Other FBC code already uses the primary pointer to
> grab the FBC associated with a CRTC (intel_fbc_min_cdclk,
> intel_fbc_crtc_debugfs_add, etc.).
Yes, I think this is correct as well.
MattA
>
>
> Matt
>
> > + primary = plane;
> > + break;
> > + }
> > + }
> > +
> > + if (drm_WARN_ON(display->drm, !primary))
> > + return NULL;
> > +
> > + return primary->fbc;
> > +}
> > +
> > /* plane stride in pixels */
> > static unsigned int intel_fbc_plane_stride(const struct intel_plane_state
> > *plane_state)
> > {
> > @@ -2119,6 +2138,37 @@ void intel_fbc_handle_fifo_underrun_irq(struct
> > intel_display *display)
> > __intel_fbc_handle_fifo_underrun_irq(fbc);
> > }
> >
> > +/**
> > + * intel_fbc_read_underrun_dbg_info - Read and log FBC-related FIFO
> > underrun debug info
> > + * @display: display device instance
> > + * @pipe: the pipe possibly containing the FBC
> > + * @log: log the info?
> > + *
> > + * If @pipe does not contain an FBC instance, this function bails early.
> > + * Otherwise, FBC-related FIFO underrun is read and cleared, and then, if
> > @log
> > + * is true, printed with error level.
> > + */
> > +void intel_fbc_read_underrun_dbg_info(struct intel_display *display,
> > + enum pipe pipe, bool log)
> > +{
> > + struct intel_fbc *fbc = intel_fbc_for_pipe(display, pipe);
> > + u32 val;
> > +
> > + if (!fbc)
> > + return;
> > +
> > + val = intel_de_read(display, FBC_DEBUG_STATUS(fbc->id));
> > + if (!(val & FBC_UNDERRUN_DECMPR))
> > + return;
> > +
> > + intel_de_write(display, FBC_DEBUG_STATUS(fbc->id), FBC_UNDERRUN_DECMPR);
> > +
> > + if (log)
> > + drm_err(display->drm,
> > + "Pipe %c FIFO underrun info: FBC decompressing\n",
> > + pipe_name(pipe));
> > +}
> > +
> > /*
> > * The DDX driver changes its behavior depending on the value it reads from
> > * i915.enable_fbc, so sanitize it by translating the default value into
> > either
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h
> > b/drivers/gpu/drm/i915/display/intel_fbc.h
> > index 91424563206a..f0255ddae2b6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.h
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.h
> > @@ -9,6 +9,7 @@
> > #include <linux/types.h>
> >
> > enum fb_op_origin;
> > +enum pipe;
> > struct intel_atomic_state;
> > struct intel_crtc;
> > struct intel_crtc_state;
> > @@ -46,6 +47,8 @@ void intel_fbc_flush(struct intel_display *display,
> > unsigned int frontbuffer_bits, enum fb_op_origin origin);
> > void intel_fbc_add_plane(struct intel_fbc *fbc, struct intel_plane *plane);
> > void intel_fbc_handle_fifo_underrun_irq(struct intel_display *display);
> > +void intel_fbc_read_underrun_dbg_info(struct intel_display *display,
> > + enum pipe, bool log);
> > void intel_fbc_reset_underrun(struct intel_display *display);
> > void intel_fbc_crtc_debugfs_add(struct intel_crtc *crtc);
> > void intel_fbc_debugfs_register(struct intel_display *display);
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc_regs.h
> > b/drivers/gpu/drm/i915/display/intel_fbc_regs.h
> > index b1d0161a3196..77d8321c4fb3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc_regs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc_regs.h
> > @@ -88,6 +88,8 @@
> > #define DPFC_FENCE_YOFF _MMIO(0x3218)
> > #define ILK_DPFC_FENCE_YOFF(fbc_id) _MMIO_PIPE((fbc_id), 0x43218,
> > 0x43258)
> > #define DPFC_CHICKEN _MMIO(0x3224)
> > +#define FBC_DEBUG_STATUS(fbc_id) _MMIO_PIPE((fbc_id), 0x43220, 0x43260)
> > +#define FBC_UNDERRUN_DECMPR REG_BIT(27)
> > #define ILK_DPFC_CHICKEN(fbc_id) _MMIO_PIPE((fbc_id), 0x43224, 0x43264)
> > #define DPFC_HT_MODIFY REG_BIT(31) /* pre-ivb */
> > #define DPFC_NUKE_ON_ANY_MODIFICATION REG_BIT(23) /* bdw+ */
> > diff --git a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> > b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> > index c2ce8461ac9e..b413b3e871d8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> > @@ -25,6 +25,8 @@
> > *
> > */
> >
> > +#include <linux/seq_buf.h>
> > +
> > #include <drm/drm_print.h>
> >
> > #include "i915_reg.h"
> > @@ -57,6 +59,100 @@
> > * The code also supports underrun detection on the PCH transcoder.
> > */
> >
> > +#define UNDERRUN_DBG1_NUM_PLANES 6
> > +
> > +static void log_underrun_dbg1(struct intel_display *display, enum pipe
> > pipe,
> > + unsigned long plane_mask, const char *info)
> > +{
> > + DECLARE_SEQ_BUF(planes_desc, 32);
> > + unsigned int i;
> > +
> > + if (!plane_mask)
> > + return;
> > +
> > + for_each_set_bit(i, &plane_mask, UNDERRUN_DBG1_NUM_PLANES) {
> > + if (i == 0)
> > + seq_buf_puts(&planes_desc, "[C]");
> > + else
> > + seq_buf_printf(&planes_desc, "[%d]", i);
> > + }
> > +
> > + drm_err(display->drm, "Pipe %c FIFO underrun info: %s on planes: %s\n",
> > + pipe_name(pipe), info, seq_buf_str(&planes_desc));
> > +
> > + drm_WARN_ON(display->drm, seq_buf_has_overflowed(&planes_desc));
> > +}
> > +
> > +static void read_underrun_dbg1(struct intel_display *display, enum pipe
> > pipe, bool log)
> > +{
> > + u32 val = intel_de_read(display, UNDERRUN_DBG1(pipe));
> > +
> > + if (!val)
> > + return;
> > +
> > + intel_de_write(display, UNDERRUN_DBG1(pipe), val);
> > +
> > + if (!log)
> > + return;
> > +
> > + log_underrun_dbg1(display, pipe,
> > REG_FIELD_GET(UNDERRUN_DBUF_BLOCK_NOT_VALID_MASK, val),
> > + "DBUF block not valid");
> > + log_underrun_dbg1(display, pipe, REG_FIELD_GET(UNDERRUN_DDB_EMPTY_MASK,
> > val),
> > + "DDB empty");
> > + log_underrun_dbg1(display, pipe,
> > REG_FIELD_GET(UNDERRUN_DBUF_NOT_FILLED_MASK, val),
> > + "DBUF not completely filled");
> > + log_underrun_dbg1(display, pipe, REG_FIELD_GET(UNDERRUN_BELOW_WM0_MASK,
> > val),
> > + "DBUF below WM0");
> > +}
> > +
> > +static void read_underrun_dbg2(struct intel_display *display, enum pipe
> > pipe, bool log)
> > +{
> > + u32 val = intel_de_read(display, UNDERRUN_DBG2(pipe));
> > +
> > + if (!(val & UNDERRUN_FRAME_LINE_COUNTERS_FROZEN))
> > + return;
> > +
> > + intel_de_write(display, UNDERRUN_DBG2(pipe),
> > UNDERRUN_FRAME_LINE_COUNTERS_FROZEN);
> > +
> > + if (log)
> > + drm_err(display->drm,
> > + "Pipe %c FIFO underrun info: frame count: %u, line
> > count: %u\n",
> > + pipe_name(pipe),
> > + REG_FIELD_GET(UNDERRUN_PIPE_FRAME_COUNT_MASK, val),
> > + REG_FIELD_GET(UNDERRUN_LINE_COUNT_MASK, val));
> > +}
> > +
> > +static void read_underrun_dbg_pkgc(struct intel_display *display, bool log)
> > +{
> > + u32 val = intel_de_read(display, GEN12_DCPR_STATUS_1);
> > +
> > + if (!(val & XE3P_UNDERRUN_PKGC))
> > + return;
> > +
> > + /*
> > + * Note: If there are multiple pipes enabled, only one of them will see
> > + * XE3P_UNDERRUN_PKGC set.
> > + */
> > + intel_de_write(display, GEN12_DCPR_STATUS_1, XE3P_UNDERRUN_PKGC);
> > +
> > + if (log)
> > + drm_err(display->drm,
> > + "General FIFO underrun info: Package C-state blocking
> > memory\n");
> > +}
> > +
> > +static void read_underrun_dbg_info(struct intel_display *display,
> > + enum pipe pipe,
> > + bool log)
> > +{
> > + if (!HAS_UNDERRUN_DBG_INFO(display))
> > + return;
> > +
> > + read_underrun_dbg1(display, pipe, log);
> > + read_underrun_dbg2(display, pipe, log);
> > + intel_fbc_read_underrun_dbg_info(display, pipe, log);
> > + read_underrun_dbg_pkgc(display, log);
> > +}
> > +
> > static bool ivb_can_enable_err_int(struct intel_display *display)
> > {
> > struct intel_crtc *crtc;
> > @@ -262,6 +358,17 @@ static bool
> > __intel_set_cpu_fifo_underrun_reporting(struct intel_display *displa
> > old = !crtc->cpu_fifo_underrun_disabled;
> > crtc->cpu_fifo_underrun_disabled = !enable;
> >
> > + /*
> > + * The debug bits get latched at the time of the FIFO underrun ISR bit
> > + * getting set. That means that any non-zero debug bit that is read
> > when
> > + * handling a FIFO underrun interrupt has the potential to belong to
> > + * another underrun event (past or future). To alleviate this problem,
> > + * let's clear existing bits before enabling the interrupt, so that at
> > + * least we don't get information that is too out-of-date.
> > + */
> > + if (enable && !old)
> > + read_underrun_dbg_info(display, pipe, false);
> > +
> > if (HAS_GMCH(display))
> > i9xx_set_fifo_underrun_reporting(display, pipe, enable, old);
> > else if (display->platform.ironlake || display->platform.sandybridge)
> > @@ -379,6 +486,8 @@ void intel_cpu_fifo_underrun_irq_handler(struct
> > intel_display *display,
> > trace_intel_cpu_fifo_underrun(display, pipe);
> >
> > drm_err(display->drm, "CPU pipe %c FIFO underrun\n",
> > pipe_name(pipe));
> > +
> > + read_underrun_dbg_info(display, pipe, true);
> > }
> >
> > intel_fbc_handle_fifo_underrun_irq(display);
> >
> > --
> > 2.51.0
> >
>
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation
