All of lore.kernel.org
 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: 50+ 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:10 ` ✓ i915.CI.BAT: success for drm/i915/display: Add initial support for Xe3p_LPD (rev4) Patchwork
2025-11-08  1:52 ` ✗ CI.checkpatch: warning " 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  0:12 ` ✗ i915.CI.Full: " 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.