Re: [PATCH 3/6] drm/xe/guc: Print register addresses in capture snapshot output

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Mismatched MMIO offset printed for 64-bit registers.
--

--- Patch [3]: [PATCH 3/6] drm/xe/guc: Print register addresses in capture snapshot output ---
commit 4cd73ae81306dc6a8f6e5245b976fc6aed0e2134
Author: Nareshkumar Gollakoti <[email protected]>

drm/xe/guc: Print register addresses in capture snapshot output
    
This commit updates the GuC capture snapshot printing logic to include the
register's MMIO offset alongside its name and value. This makes the output
easier to cross-reference with hardware debug documentation.

> diff --git a/drivers/gpu/drm/xe/xe_guc_capture.c b/drivers/gpu/drm/xe/xe_guc_capture.c
> index 82df19b304e17..368769857c9b8 100644
> --- a/drivers/gpu/drm/xe/xe_guc_capture.c
> +++ b/drivers/gpu/drm/xe/xe_guc_capture.c
> @@ -1774,7 +1774,9 @@ snapshot_print_by_list_order(struct xe_hw_engine_snapshot *snapshot, struct drm_
>  			XE_WARN_ON(!low32_ready);
>  			low32_ready = false;
>  
> -			drm_printf(p, "\t%s: 0x%016llx\n", reg_desc->regname, value_qw);
> +			drm_printf(p, "\t%s (0x%x): 0x%016llx\n", reg_desc->regname,
> +				   reg_desc->reg.addr,
> +				   value_qw);

[Severity: Medium]
Will this print the MMIO offset of the upper 32-bit register (HI_DW) instead
of the base address (LOW_DW)?

In snapshot_print_by_list_order() in drivers/gpu/drm/xe/xe_guc_capture.c,
the REG_64BIT_HI_DW case is processing the second entry of a 64-bit register
pair. Because it uses reg_desc->reg.addr from the HI_DW descriptor, the
printed output pairs the combined 64-bit value with the offset of the upper
32-bit register.

Could this regression cause confusion when cross-referencing the offset with
hardware documentation, which typically expects the base offset for a 64-bit
register?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3