intel-xe.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Gustavo Sousa <gustavo.sousa@intel.com>
Cc: intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	"Ankit Nautiyal" <ankit.k.nautiyal@intel.com>,
	"Dnyaneshwar Bhadane" <dnyaneshwar.bhadane@intel.com>,
	"Jouni Högander" <jouni.hogander@intel.com>,
	"Juha-pekka Heikkila" <juha-pekka.heikkila@intel.com>,
	"Luca Coelho" <luciano.coelho@intel.com>,
	"Lucas De Marchi" <lucas.demarchi@intel.com>,
	"Matt Atwood" <matthew.s.atwood@intel.com>,
	"Matt Roper" <matthew.d.roper@intel.com>,
	"Ravi Kumar Vodapalli" <ravi.kumar.vodapalli@intel.com>,
	"Shekhar Chauhan" <shekhar.chauhan@intel.com>,
	"Vinod Govindapillai" <vinod.govindapillai@intel.com>,
	"Jani Nikula" <jani.nikula@intel.com>
Subject: Re: [PATCH v4 06/11] drm/i915/xe3p_lpd: Handle underrun debug bits
Date: Mon, 10 Nov 2025 19:03:44 +0200	[thread overview]
Message-ID: <aRIa8HC68K7zJOsJ@intel.com> (raw)
In-Reply-To: <20251107-xe3p_lpd-basic-enabling-v4-6-ab3367f65f15@intel.com>

On Fri, Nov 07, 2025 at 09:05:39PM -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/20251015-xe3p_lpd-basic-enabling-v1-12-d2d1e26520aa@intel.com/
> 
> Bspec: 69111, 69561, 74411, 74412
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
> 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_regs.h      |   2 +
>  drivers/gpu/drm/i915/display/intel_fifo_underrun.c | 128 +++++++++++++++++++++
>  4 files changed, 147 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_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..8a05b5c5fccd 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"
> @@ -34,6 +36,7 @@
>  #include "intel_display_trace.h"
>  #include "intel_display_types.h"
>  #include "intel_fbc.h"
> +#include "intel_fbc_regs.h"
>  #include "intel_fifo_underrun.h"
>  #include "intel_pch_display.h"
>  
> @@ -57,6 +60,118 @@
>   * The code also supports underrun detection on the PCH transcoder.
>   */
>  
> +#define UNDERRUN_DBG1_NUM_PLANES 6
> +
> +static void read_underrun_dbg1(struct intel_display *display, enum pipe pipe, bool log)
> +{
> +	u32 val = intel_de_read(display, UNDERRUN_DBG1(pipe));
> +	struct {
> +		u32 plane_mask;
> +		const char *info;
> +	} masks[] = {
> +		{ REG_FIELD_GET(UNDERRUN_DBUF_BLOCK_NOT_VALID_MASK, val), "DBUF block not valid" },
> +		{ REG_FIELD_GET(UNDERRUN_DDB_EMPTY_MASK, val), "DDB empty" },
> +		{ REG_FIELD_GET(UNDERRUN_DBUF_NOT_FILLED_MASK, val), "DBUF not completely filled" },
> +		{ REG_FIELD_GET(UNDERRUN_BELOW_WM0_MASK, val), "DBUF below WM0" },
> +	};
> +	DECLARE_SEQ_BUF(planes_desc, 32);
> +
> +	if (!val)
> +		return;
> +
> +	intel_de_write(display, UNDERRUN_DBG1(pipe), val);
> +
> +	if (!log)
> +		return;
> +
> +	for (int i = 0; i < ARRAY_SIZE(masks); i++) {
> +		if (!masks[i].plane_mask)
> +			continue;
> +
> +		seq_buf_clear(&planes_desc);
> +
> +		for (int j = 0; j < UNDERRUN_DBG1_NUM_PLANES; j++) {
> +			if (!(masks[i].plane_mask & REG_BIT(j)))
> +				continue;
> +
> +			if (j == 0)
> +				seq_buf_puts(&planes_desc, "[C]");
> +			else
> +				seq_buf_printf(&planes_desc, "[%d]", j);
> +		}
> +
> +		drm_err(display->drm,
> +			"Pipe %c FIFO underrun info: %s on planes: %s\n",
> +			pipe_name(pipe), masks[i].info, seq_buf_str(&planes_desc));
> +
> +		drm_WARN_ON(display->drm, seq_buf_has_overflowed(&planes_desc));
> +	}
> +}
> +
> +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_fbc(struct intel_display *display, enum pipe pipe, bool log)
> +{
> +	enum intel_fbc_id fbc_id = intel_fbc_id_for_pipe(pipe);
> +	u32 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));
> +}

FBC code belongs in intel_fbc.c

> +
> +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);
> +	read_underrun_dbg_fbc(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 +377,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 existing debug bit that is set 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 +505,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

-- 
Ville Syrjälä
Intel

  parent reply	other threads:[~2025-11-10 17:03 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-08  0:05 [PATCH v4 00/11] drm/i915/display: Add initial support for Xe3p_LPD Gustavo Sousa
2025-11-08  0:05 ` [PATCH v4 01/11] drm/i915/wm: Do not make latency values monotonic on Xe3 onward Gustavo Sousa
2025-11-12  3:46   ` Kandpal, Suraj
2025-11-12 12:45     ` Gustavo Sousa
2025-11-08  0:05 ` [PATCH v4 02/11] drm/i915/vbt: Add fields dedicated_external and dyn_port_over_tc Gustavo Sousa
2025-11-11 16:02   ` Imre Deak
2025-11-11 16:15     ` Imre Deak
2025-11-12 13:00       ` Gustavo Sousa
2025-11-12 13:20         ` Imre Deak
2025-11-13 22:01           ` Gustavo Sousa
2025-11-08  0:05 ` [PATCH v4 03/11] drm/i915/power: Use intel_encoder_is_tc() Gustavo Sousa
2025-11-12 16:19   ` Imre Deak
2025-11-13 22:01     ` Gustavo Sousa
2025-11-08  0:05 ` [PATCH v4 04/11] drm/i915/display: Handle dedicated external ports in intel_encoder_is_tc() Gustavo Sousa
2025-11-12 16:24   ` Imre Deak
2025-11-13 22:02     ` Gustavo Sousa
2025-11-08  0:05 ` [PATCH v4 05/11] drm/i915/fbc: Add intel_fbc_id_for_pipe() Gustavo Sousa
2025-11-10 16:35   ` Matt Roper
2025-11-10 17:03     ` Ville Syrjälä
2025-11-10 22:18       ` Gustavo Sousa
2025-11-08  0:05 ` [PATCH v4 06/11] drm/i915/xe3p_lpd: Handle underrun debug bits Gustavo Sousa
2025-11-10 11:45   ` Jani Nikula
2025-11-11  0:23     ` Gustavo Sousa
2025-11-11 10:22       ` Jani Nikula
2025-11-11 12:22         ` Gustavo Sousa
2025-11-10 17:03   ` Ville Syrjälä [this message]
2025-11-10 23:42     ` Gustavo Sousa
2025-11-08  0:05 ` [PATCH v4 07/11] drm/i915/xe3p_lpd: Extend Type-C flow for static DDI allocation Gustavo Sousa
2025-11-12 17:53   ` Imre Deak
2025-11-14 19:46     ` Gustavo Sousa
2025-11-15  0:40       ` Imre Deak
2025-11-15  1:22         ` Imre Deak
2025-11-17 15:02           ` Gustavo Sousa
2025-11-17 15:17             ` Imre Deak
2025-11-17 17:23               ` Gustavo Sousa
2025-11-17 17:58                 ` Imre Deak
2025-11-17 15:33         ` Gustavo Sousa
2025-11-17 16:01           ` Imre Deak
2025-11-08  0:05 ` [PATCH v4 08/11] drm/i915/nvls: Add NVL-S display support Gustavo Sousa
2025-11-08  0:05 ` [PATCH v4 09/11] drm/i915/display: Use platform check in HAS_LT_PHY() Gustavo Sousa
2025-11-08  0:05 ` [PATCH v4 10/11] drm/i915/display: Move HAS_LT_PHY() to intel_display_device.h Gustavo Sousa
2025-11-08  0:05 ` [PATCH v4 11/11] drm/i915/display: Use HAS_LT_PHY() for LT PHY AUX power Gustavo Sousa
2025-11-08  0:15 ` [PATCH v4 00/11] drm/i915/display: Add initial support for Xe3p_LPD Gustavo Sousa
2025-11-08  1:52 ` ✗ CI.checkpatch: warning for drm/i915/display: Add initial support for Xe3p_LPD (rev4) Patchwork
2025-11-08  1:54 ` ✓ CI.KUnit: success " Patchwork
2025-11-08  2:09 ` ✗ CI.checksparse: warning " Patchwork
2025-11-08  2:31 ` ✗ Xe.CI.BAT: failure " Patchwork
2025-11-09  8:10 ` ✗ Xe.CI.Full: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aRIa8HC68K7zJOsJ@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=ankit.k.nautiyal@intel.com \
    --cc=dnyaneshwar.bhadane@intel.com \
    --cc=gustavo.sousa@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=jouni.hogander@intel.com \
    --cc=juha-pekka.heikkila@intel.com \
    --cc=lucas.demarchi@intel.com \
    --cc=luciano.coelho@intel.com \
    --cc=matthew.d.roper@intel.com \
    --cc=matthew.s.atwood@intel.com \
    --cc=ravi.kumar.vodapalli@intel.com \
    --cc=shekhar.chauhan@intel.com \
    --cc=vinod.govindapillai@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).