Hi Krzysztof,
On Fri Oct 30, 2026 at 9:39 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, in current testing it leads to failures [3], as
> 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. Move them to the VIDEO class so GuC
> requests them for the class tied to VCS availability.
>
> [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 <[email protected]>
> ---
> 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.
>
> From empirical experiments, I was able to identify the classes
> that report "partial-capture" during the test: VIDEOENHANCE and
> BLITTER, while RENDER_COMPUTE, VIDEO and GSC_OTHER report
> "full-capture" for SFC_DONE registers. I simply changed the
> class of xe_lp_vec_class_regs inside
> "static const struct __guc_mmio_reg_descr_group xe_lp_lists[]"
> and ran the complete "capture" subtest mentioned above, so this
> change should not (fingers crossed) explode anything.
>
> [6] commit 93d3b0e28d7d
> ("tests/intel/gem_exec_capture: Check for partial register captures")
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 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 865b61057566d..bbf5188ae46cd 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> @@ -187,9 +187,9 @@ static const struct __guc_mmio_reg_descr_group
> xe_lp_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(empty_regs_list, PF, ENGINE_CLASS,
> GUC_CAPTURE_LIST_CLASS_VIDEO),
> + MAKE_REGLIST(xe_lp_vec_class_regs, PF, ENGINE_CLASS,
> GUC_CAPTURE_LIST_CLASS_VIDEO),
Now the naming is inconsistent: xe_lp_vec_class_regs is CLASS_VIDEO,
not CLASS_VIDEOENHANCE as its name suggests.
--
Best regards,
Sebastian
