Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/i915/gt: Adjust usage of SFC_DONE registers on Gen 12
@ 2026-11-05  7:14 Krzysztof Karas
  2026-11-05  9:51 ` ✗ i915.CI.BAT: failure for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Krzysztof Karas @ 2026-11-05  7:14 UTC (permalink / raw)
  To: intel-gfx
  Cc: dri-devel, Andi Shyti, Janusz Krzysztofik, Sebastian Brzezinka,
	Krzysztof Niemiec, Krzysztof Karas

Initial, empty XE_LPD register list for Video-Enhance class was
introduced in [1] until [2] filled it out with debug SFC_DONE
entries, but it was never explained in any of those commits why
using those registers for VECS class specifically was chosen.
Furthermore, currently, it is seen as a regression [3].

Currently, gem_exec_capture@capture@vecs0-smem runs report
partial captures on DG1, DG2 and MTL [4].
SFC_DONE resides in the VD forcewake domain and is available
only when the corresponding VCS engine is present (explanation
from [5]), but XE_LP capture table currently associates these
registers with VIDEOENHANCE.

Furthermore, GuC firmware supports SFC_DONE registers in error
capture only for MTL platforms, DG1/DG2 are not included, which
was confirmed with experimentation and offline discussion with
GuC side.

Make a new list of register lists for MTL and use SFC_DONE
entries only when that platform is detected. For DG1/DG2 pass
an empty list for VECS class.

[1] commit 24492514ccbd
    ("drm/i915/guc: Update GuC ADS size for error capture lists")
[2] commit 8b72c21618de
    ("drm/i915/guc: Add XE_LP static registers for GuC error capture.")
[3] https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16904
[4] https://intel-gfx-ci.01.org/tree/drm-tip/index.html?testfilter=igt@gem_exec_capture@capture@vecs0-smem
[5] commit 24d032e2359e
    ("drm/i915: Only access SFC_DONE when media domain is not fused off")

Fixes: 8b72c21618de ("drm/i915/guc: Add XE_LP static registers for GuC error capture.")
Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
---
v2:
 * Changed commit title (was "drm/i915/gt: Use VCS instead of
   VECS for GuC error capture on Gen 12") and commit
   description;
 * Enabled SFC_DONE on MTL's VCS engines and disabled these
   registers on DG1/DG2;
 * Added a new list for MTL;
 * Renamed relevant objects vec -> vc.

I could not verify when exactly the issue started happening,
because:
A) all recent reports are red on DG1, DG2 and MTL,
B) checking for "partial-capture" in IGT [6] was introduced way
   later than the driver code asking for SFC_DONE registers.
I assume it has been present for at least a few months, if not
from the very beginning.

[6] commit 93d3b0e28d7d
    ("tests/intel/gem_exec_capture: Check for partial register captures")
---
 .../gpu/drm/i915/gt/uc/intel_guc_capture.c    | 33 +++++++++++++++----
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
index 865b61057566..88e6d706210c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
@@ -88,7 +88,7 @@
 	{ GEN12_SC_INSTDONE_EXTRA,  0,      0, "GEN12_SC_INSTDONE_EXTRA" }, \
 	{ GEN12_SC_INSTDONE_EXTRA2, 0,      0, "GEN12_SC_INSTDONE_EXTRA2" }
 
-#define COMMON_GEN12BASE_VEC \
+#define COMMON_GEN12BASE_VC \
 	{ GEN12_SFC_DONE(0),        0,      0, "SFC_DONE[0]" }, \
 	{ GEN12_SFC_DONE(1),        0,      0, "SFC_DONE[1]" }, \
 	{ GEN12_SFC_DONE(2),        0,      0, "SFC_DONE[2]" }, \
@@ -117,9 +117,9 @@ static const struct __guc_mmio_reg_descr gen8_vd_inst_regs[] = {
 	COMMON_BASE_ENGINE_INSTANCE,
 };
 
-/* XE_LP Video Enhancement Per-Class */
-static const struct __guc_mmio_reg_descr xe_lp_vec_class_regs[] = {
-	COMMON_GEN12BASE_VEC,
+/* XE_LP Video Per-Class */
+static const struct __guc_mmio_reg_descr xe_lp_vc_class_regs[] = {
+	COMMON_GEN12BASE_VC,
 };
 
 /* GEN8+ Video Enhancement Per-Engine-Instance */
@@ -189,7 +189,22 @@ static const struct __guc_mmio_reg_descr_group xe_lp_lists[] = {
 	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_RENDER_COMPUTE),
 	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEO),
 	MAKE_REGLIST(gen8_vd_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_VIDEO),
-	MAKE_REGLIST(xe_lp_vec_class_regs, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
+	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
+	MAKE_REGLIST(gen8_vec_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
+	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_BLITTER),
+	MAKE_REGLIST(gen8_blt_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_BLITTER),
+	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_GSC_OTHER),
+	MAKE_REGLIST(xe_lp_gsc_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_GSC_OTHER),
+	{}
+};
+
+static const struct __guc_mmio_reg_descr_group mtl_lists[] = {
+	MAKE_REGLIST(xe_lp_global_regs, PF, GLOBAL, 0),
+	MAKE_REGLIST(xe_lp_rc_class_regs, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_RENDER_COMPUTE),
+	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_RENDER_COMPUTE),
+	MAKE_REGLIST(xe_lp_vc_class_regs, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEO),
+	MAKE_REGLIST(gen8_vd_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_VIDEO),
+	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
 	MAKE_REGLIST(gen8_vec_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
 	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_BLITTER),
 	MAKE_REGLIST(gen8_blt_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_BLITTER),
@@ -354,7 +369,13 @@ guc_capture_get_device_reglist(struct intel_guc *guc)
 	struct drm_i915_private *i915 = guc_to_i915(guc);
 	const struct __guc_mmio_reg_descr_group *lists;
 
-	if (GRAPHICS_VER(i915) >= 12)
+	if (IS_METEORLAKE(i915))
+		/**
+		 * MTL is the only member of Gen 12 with SFC_DONE registers supported by
+		 * error capture in GuC firmware, so use a separate list of registers there.
+		 */
+		lists = mtl_lists;
+	else if (GRAPHICS_VER(i915) >= 12)
 		lists = xe_lp_lists;
 	else
 		lists = gen8_lists;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* ✗ i915.CI.BAT: failure for drm/i915/gt: Adjust usage of SFC_DONE registers on Gen 12
  2026-11-05  7:14 [PATCH v2] drm/i915/gt: Adjust usage of SFC_DONE registers on Gen 12 Krzysztof Karas
@ 2026-11-05  9:51 ` Patchwork
  2026-11-10 10:32 ` [PATCH v2] " Sebastian Brzezinka
  2026-11-12 11:23 ` Janusz Krzysztofik
  2 siblings, 0 replies; 4+ messages in thread
From: Patchwork @ 2026-11-05  9:51 UTC (permalink / raw)
  To: Krzysztof Karas; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 3170 bytes --]

== Series Details ==

Series: drm/i915/gt: Adjust usage of SFC_DONE registers on Gen 12
URL   : https://patchwork.freedesktop.org/series/177819/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_19452 -> Patchwork_177819v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_177819v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_177819v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177819v1/index.html

Participating hosts (40 -> 39)
------------------------------

  Missing    (1): bat-dg2-13 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_177819v1:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_hdmi_inject@inject-audio:
    - fi-tgl-1115g4:      [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19452/fi-tgl-1115g4/igt@kms_hdmi_inject@inject-audio.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177819v1/fi-tgl-1115g4/igt@kms_hdmi_inject@inject-audio.html

  
Known issues
------------

  Here are the changes found in Patchwork_177819v1 that come from known issues:

### IGT changes ###

#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - fi-tgl-1115g4:      [FAIL][3] ([i915#17751]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19452/fi-tgl-1115g4/igt@i915_pm_rpm@module-reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177819v1/fi-tgl-1115g4/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live:
    - bat-arlh-3:         [INCOMPLETE][5] ([i915#16547]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19452/bat-arlh-3/igt@i915_selftest@live.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177819v1/bat-arlh-3/igt@i915_selftest@live.html

  * igt@i915_selftest@live@dmabuf:
    - bat-arlh-3:         [INCOMPLETE][7] ([i915#17776]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19452/bat-arlh-3/igt@i915_selftest@live@dmabuf.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177819v1/bat-arlh-3/igt@i915_selftest@live@dmabuf.html

  
  [i915#16547]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/16547
  [i915#17751]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/17751
  [i915#17776]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/17776


Build changes
-------------

  * Linux: CI_DRM_19452 -> Patchwork_177819v1

  CI-20190529: 20190529
  CI_DRM_19452: 77675f3f5804ae46707c2d08afd75c64f424ef57 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_9177: 9177
  Patchwork_177819v1: 77675f3f5804ae46707c2d08afd75c64f424ef57 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_177819v1/index.html

[-- Attachment #2: Type: text/html, Size: 3861 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] drm/i915/gt: Adjust usage of SFC_DONE registers on Gen 12
  2026-11-05  7:14 [PATCH v2] drm/i915/gt: Adjust usage of SFC_DONE registers on Gen 12 Krzysztof Karas
  2026-11-05  9:51 ` ✗ i915.CI.BAT: failure for " Patchwork
@ 2026-11-10 10:32 ` Sebastian Brzezinka
  2026-11-12 11:23 ` Janusz Krzysztofik
  2 siblings, 0 replies; 4+ messages in thread
From: Sebastian Brzezinka @ 2026-11-10 10:32 UTC (permalink / raw)
  To: Krzysztof Karas, intel-gfx
  Cc: dri-devel, Andi Shyti, Janusz Krzysztofik, Sebastian Brzezinka,
	Krzysztof Niemiec

Hi,

On Thu Nov 5, 2026 at 8:14 AM CET, Krzysztof Karas wrote:
> Initial, empty XE_LPD register list for Video-Enhance class was
> introduced in [1] until [2] filled it out with debug SFC_DONE
> entries, but it was never explained in any of those commits why
> using those registers for VECS class specifically was chosen.
> Furthermore, currently, it is seen as a regression [3].
>
> Currently, gem_exec_capture@capture@vecs0-smem runs report
> partial captures on DG1, DG2 and MTL [4].
> SFC_DONE resides in the VD forcewake domain and is available
> only when the corresponding VCS engine is present (explanation
> from [5]), but XE_LP capture table currently associates these
> registers with VIDEOENHANCE.
>
> Furthermore, GuC firmware supports SFC_DONE registers in error
> capture only for MTL platforms, DG1/DG2 are not included, which
> was confirmed with experimentation and offline discussion with
> GuC side.
>
> Make a new list of register lists for MTL and use SFC_DONE
> entries only when that platform is detected. For DG1/DG2 pass
> an empty list for VECS class.
>
> [1] commit 24492514ccbd
>     ("drm/i915/guc: Update GuC ADS size for error capture lists")
> [2] commit 8b72c21618de
>     ("drm/i915/guc: Add XE_LP static registers for GuC error capture.")
> [3] https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16904
> [4] https://intel-gfx-ci.01.org/tree/drm-tip/index.html?testfilter=igt@gem_exec_capture@capture@vecs0-smem
> [5] commit 24d032e2359e
>     ("drm/i915: Only access SFC_DONE when media domain is not fused off")
>
> Fixes: 8b72c21618de ("drm/i915/guc: Add XE_LP static registers for GuC error capture.")
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
> v2:
>  * Changed commit title (was "drm/i915/gt: Use VCS instead of
>    VECS for GuC error capture on Gen 12") and commit
>    description;
>  * Enabled SFC_DONE on MTL's VCS engines and disabled these
>    registers on DG1/DG2;
>  * Added a new list for MTL;
>  * Renamed relevant objects vec -> vc.
>
> I could not verify when exactly the issue started happening,
> because:
> A) all recent reports are red on DG1, DG2 and MTL,
> B) checking for "partial-capture" in IGT [6] was introduced way
>    later than the driver code asking for SFC_DONE registers.
> I assume it has been present for at least a few months, if not
> from the very beginning.
>
> [6] commit 93d3b0e28d7d
>     ("tests/intel/gem_exec_capture: Check for partial register captures")
> ---
>  .../gpu/drm/i915/gt/uc/intel_guc_capture.c    | 33 +++++++++++++++----
>  1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> index 865b61057566..88e6d706210c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> @@ -88,7 +88,7 @@
>  	{ GEN12_SC_INSTDONE_EXTRA,  0,      0, "GEN12_SC_INSTDONE_EXTRA" }, \
>  	{ GEN12_SC_INSTDONE_EXTRA2, 0,      0, "GEN12_SC_INSTDONE_EXTRA2" }
>  
> -#define COMMON_GEN12BASE_VEC \
> +#define COMMON_GEN12BASE_VC \
s/VC/VD/ ?

and if
#define GEN12_SFC_DONE(n) _MMIO(0x1cc000 + (n) * 0x1000)
- SFC_DONE(0) = 0x1cc000
- SFC_DONE(1) = 0x1cd000
- SFC_DONE(2) = 0x1ce000
- SFC_DONE(3) = 0x1cf000

>  	{ GEN12_SFC_DONE(0),        0,      0, "SFC_DONE[0]" }, \
>  	{ GEN12_SFC_DONE(1),        0,      0, "SFC_DONE[1]" }, \
>  	{ GEN12_SFC_DONE(2),        0,      0, "SFC_DONE[2]" }, \
mtl_lists captures SFC_DONE(0..3) always. On the MTL media GT,
__xelpmp_fw_ranges (intel_uncore.c) has this entry:
------------------------------------------------------------
GEN_FW_RANGE(0x1cc000, 0x1cffff, FORCEWAKE_MEDIA_VDBOX0)
0x1cc000 - 0x1cdfff: VD0
0x1ce000 - 0x1cffff: reserved
------------------------------------------------------------

So only SFC_DONE(0) and (1) are in the documented range.  SFC_DONE(2)
and (3) (0x1ce000, 0x1cf000) are marked reserved.

I don't know how GuC handles reserved registers. Could you please either
trim the list to (0) and (1), or explain why reading (2) and (3) is fine.


> @@ -117,9 +117,9 @@ static const struct __guc_mmio_reg_descr gen8_vd_inst_regs[] = {
>  	COMMON_BASE_ENGINE_INSTANCE,
>  };
>  
> -/* XE_LP Video Enhancement Per-Class */
> -static const struct __guc_mmio_reg_descr xe_lp_vec_class_regs[] = {
> -	COMMON_GEN12BASE_VEC,
> +/* XE_LP Video Per-Class */
> +static const struct __guc_mmio_reg_descr xe_lp_vc_class_regs[] = {
> +	COMMON_GEN12BASE_VC,
>  };
>  
>  /* GEN8+ Video Enhancement Per-Engine-Instance */
> @@ -189,7 +189,22 @@ static const struct __guc_mmio_reg_descr_group xe_lp_lists[] = {
>  	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_RENDER_COMPUTE),
>  	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEO),
>  	MAKE_REGLIST(gen8_vd_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_VIDEO),
> -	MAKE_REGLIST(xe_lp_vec_class_regs, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
> +	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
> +	MAKE_REGLIST(gen8_vec_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
> +	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_BLITTER),
> +	MAKE_REGLIST(gen8_blt_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_BLITTER),
> +	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_GSC_OTHER),
> +	MAKE_REGLIST(xe_lp_gsc_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_GSC_OTHER),
> +	{}
> +};
> +
> +static const struct __guc_mmio_reg_descr_group mtl_lists[] = {
> +	MAKE_REGLIST(xe_lp_global_regs, PF, GLOBAL, 0),
> +	MAKE_REGLIST(xe_lp_rc_class_regs, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_RENDER_COMPUTE),
> +	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_RENDER_COMPUTE),
> +	MAKE_REGLIST(xe_lp_vc_class_regs, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEO),
a
Convention for CLASS_VIDEO is vd(e.g. gen8_vd_inst_regs).

> +	MAKE_REGLIST(gen8_vd_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_VIDEO),
> +	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
>  	MAKE_REGLIST(gen8_vec_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
>  	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_BLITTER),
>  	MAKE_REGLIST(gen8_blt_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_BLITTER),
> @@ -354,7 +369,13 @@ guc_capture_get_device_reglist(struct intel_guc *guc)
>  	struct drm_i915_private *i915 = guc_to_i915(guc);
>  	const struct __guc_mmio_reg_descr_group *lists;
>  
> -	if (GRAPHICS_VER(i915) >= 12)
> +	if (IS_METEORLAKE(i915))
> +		/**
Those are kernrl doc comments style.

> +		 * MTL is the only member of Gen 12 with SFC_DONE registers supported by
> +		 * error capture in GuC firmware, so use a separate list of registers there.
> +		 */
> +		lists = mtl_lists;
> +	else if (GRAPHICS_VER(i915) >= 12)
>  		lists = xe_lp_lists;
You never mention other gen12 platforms (TGL, RKL, ADL, PVC), and this
change is gonna affect them as well.

>  	else
>  		lists = gen8_lists;





-- 
Best regards,
Sebastian


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] drm/i915/gt: Adjust usage of SFC_DONE registers on Gen 12
  2026-11-05  7:14 [PATCH v2] drm/i915/gt: Adjust usage of SFC_DONE registers on Gen 12 Krzysztof Karas
  2026-11-05  9:51 ` ✗ i915.CI.BAT: failure for " Patchwork
  2026-11-10 10:32 ` [PATCH v2] " Sebastian Brzezinka
@ 2026-11-12 11:23 ` Janusz Krzysztofik
  2 siblings, 0 replies; 4+ messages in thread
From: Janusz Krzysztofik @ 2026-11-12 11:23 UTC (permalink / raw)
  To: Krzysztof Karas, intel-gfx
  Cc: dri-devel, Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec

Hi Krzysztof,

On Thu, 2026-11-05 at 07:14 +0000, Krzysztof Karas wrote:
> Initial, empty XE_LPD register list for Video-Enhance class was
> introduced in [1] until [2] filled it out with debug SFC_DONE
> entries, but it was never explained in any of those commits why
> using those registers for VECS class specifically was chosen.
> Furthermore, currently, it is seen as a regression [3].
> 
> Currently, gem_exec_capture@capture@vecs0-smem runs report
> partial captures on DG1, DG2 and MTL [4].
> SFC_DONE resides in the VD forcewake domain and is available
> only when the corresponding VCS engine is present (explanation
> from [5]), but XE_LP capture table currently associates these
> registers with VIDEOENHANCE.
> 
> Furthermore, GuC firmware supports SFC_DONE registers in error
> capture only for MTL platforms, DG1/DG2 are not included, which
> was confirmed with experimentation and offline discussion with
> GuC side.

Since that's highly GuC related, I think you should also Cc: / ask for 
review someone from GuC integration developers team directly, e.g., 
Daniele Ceraolo Spurio, Alan Previn or Umesh Nerlige Ramappa, maybe
others, not only i915 core.

Thanks,
Janusz

> 
> Make a new list of register lists for MTL and use SFC_DONE
> entries only when that platform is detected. For DG1/DG2 pass
> an empty list for VECS class.
> 
> [1] commit 24492514ccbd
>     ("drm/i915/guc: Update GuC ADS size for error capture lists")
> [2] commit 8b72c21618de
>     ("drm/i915/guc: Add XE_LP static registers for GuC error capture.")
> [3] https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16904
> [4] https://intel-gfx-ci.01.org/tree/drm-tip/index.html?testfilter=igt@gem_exec_capture@capture@vecs0-smem
> [5] commit 24d032e2359e
>     ("drm/i915: Only access SFC_DONE when media domain is not fused off")
> 
> Fixes: 8b72c21618de ("drm/i915/guc: Add XE_LP static registers for GuC error capture.")
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
> v2:
>  * Changed commit title (was "drm/i915/gt: Use VCS instead of
>    VECS for GuC error capture on Gen 12") and commit
>    description;
>  * Enabled SFC_DONE on MTL's VCS engines and disabled these
>    registers on DG1/DG2;
>  * Added a new list for MTL;
>  * Renamed relevant objects vec -> vc.
> 
> I could not verify when exactly the issue started happening,
> because:
> A) all recent reports are red on DG1, DG2 and MTL,
> B) checking for "partial-capture" in IGT [6] was introduced way
>    later than the driver code asking for SFC_DONE registers.
> I assume it has been present for at least a few months, if not
> from the very beginning.
> 
> [6] commit 93d3b0e28d7d
>     ("tests/intel/gem_exec_capture: Check for partial register captures")
> ---
>  .../gpu/drm/i915/gt/uc/intel_guc_capture.c    | 33 +++++++++++++++----
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> index 865b61057566..88e6d706210c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> @@ -88,7 +88,7 @@
>  	{ GEN12_SC_INSTDONE_EXTRA,  0,      0, "GEN12_SC_INSTDONE_EXTRA" }, \
>  	{ GEN12_SC_INSTDONE_EXTRA2, 0,      0, "GEN12_SC_INSTDONE_EXTRA2" }
>  
> -#define COMMON_GEN12BASE_VEC \
> +#define COMMON_GEN12BASE_VC \
>  	{ GEN12_SFC_DONE(0),        0,      0, "SFC_DONE[0]" }, \
>  	{ GEN12_SFC_DONE(1),        0,      0, "SFC_DONE[1]" }, \
>  	{ GEN12_SFC_DONE(2),        0,      0, "SFC_DONE[2]" }, \
> @@ -117,9 +117,9 @@ static const struct __guc_mmio_reg_descr gen8_vd_inst_regs[] = {
>  	COMMON_BASE_ENGINE_INSTANCE,
>  };
>  
> -/* XE_LP Video Enhancement Per-Class */
> -static const struct __guc_mmio_reg_descr xe_lp_vec_class_regs[] = {
> -	COMMON_GEN12BASE_VEC,
> +/* XE_LP Video Per-Class */
> +static const struct __guc_mmio_reg_descr xe_lp_vc_class_regs[] = {
> +	COMMON_GEN12BASE_VC,
>  };
>  
>  /* GEN8+ Video Enhancement Per-Engine-Instance */
> @@ -189,7 +189,22 @@ static const struct __guc_mmio_reg_descr_group xe_lp_lists[] = {
>  	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_RENDER_COMPUTE),
>  	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEO),
>  	MAKE_REGLIST(gen8_vd_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_VIDEO),
> -	MAKE_REGLIST(xe_lp_vec_class_regs, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
> +	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
> +	MAKE_REGLIST(gen8_vec_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
> +	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_BLITTER),
> +	MAKE_REGLIST(gen8_blt_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_BLITTER),
> +	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_GSC_OTHER),
> +	MAKE_REGLIST(xe_lp_gsc_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_GSC_OTHER),
> +	{}
> +};
> +
> +static const struct __guc_mmio_reg_descr_group mtl_lists[] = {
> +	MAKE_REGLIST(xe_lp_global_regs, PF, GLOBAL, 0),
> +	MAKE_REGLIST(xe_lp_rc_class_regs, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_RENDER_COMPUTE),
> +	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_RENDER_COMPUTE),
> +	MAKE_REGLIST(xe_lp_vc_class_regs, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEO),
> +	MAKE_REGLIST(gen8_vd_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_VIDEO),
> +	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
>  	MAKE_REGLIST(gen8_vec_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_VIDEOENHANCE),
>  	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_CAPTURE_LIST_CLASS_BLITTER),
>  	MAKE_REGLIST(gen8_blt_inst_regs, PF, ENGINE_INSTANCE, GUC_CAPTURE_LIST_CLASS_BLITTER),
> @@ -354,7 +369,13 @@ guc_capture_get_device_reglist(struct intel_guc *guc)
>  	struct drm_i915_private *i915 = guc_to_i915(guc);
>  	const struct __guc_mmio_reg_descr_group *lists;
>  
> -	if (GRAPHICS_VER(i915) >= 12)
> +	if (IS_METEORLAKE(i915))
> +		/**
> +		 * MTL is the only member of Gen 12 with SFC_DONE registers supported by
> +		 * error capture in GuC firmware, so use a separate list of registers there.
> +		 */
> +		lists = mtl_lists;
> +	else if (GRAPHICS_VER(i915) >= 12)
>  		lists = xe_lp_lists;
>  	else
>  		lists = gen8_lists;

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-12 11:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-05  7:14 [PATCH v2] drm/i915/gt: Adjust usage of SFC_DONE registers on Gen 12 Krzysztof Karas
2026-11-05  9:51 ` ✗ i915.CI.BAT: failure for " Patchwork
2026-11-10 10:32 ` [PATCH v2] " Sebastian Brzezinka
2026-11-12 11:23 ` Janusz Krzysztofik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox