Re: [PATCH v4 4/4] drm/xe/reg_sr: Allow register_save_restore_check debugfs to verify LRC values

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Wed, 18 Feb 2026 14:09:15 -0800, Matt Roper wrote:
>
> reg_sr programming that applies to an engines LRC cannot be verified by
> a simple CPU-based register readout because the reg_sr's values may not
> be in effect if no context is executing on the hardware at the time we
> check.  Instead, we should verify correct reg_sr application by
> searching for the register in the default_lrc.
>

This too LGTM:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>



> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt_debugfs.c |  4 ++--
>  drivers/gpu/drm/xe/xe_reg_sr.c     | 30 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_reg_sr.h     |  4 ++++
>  3 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> index aa43427a9f4b824b8ad41d02f634c07c76d989b2..f45306308cd66c87b261edaa8de532f6cc56499d 100644
> --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> @@ -173,8 +173,8 @@ static int register_save_restore_check(struct xe_gt *gt, struct drm_printer *p)
>	xe_reg_sr_readback_check(&gt->reg_sr, gt, p);
>	for_each_hw_engine(hwe, gt, id)
>		xe_reg_sr_readback_check(&hwe->reg_sr, gt, p);
> -
> -	/* TODO: Check hwe->reg_lrc against contents of default_lrc. */
> +	for_each_hw_engine(hwe, gt, id)
> +		xe_reg_sr_lrc_check(&hwe->reg_lrc, gt, hwe, p);
>
>	return 0;
>  }
> diff --git a/drivers/gpu/drm/xe/xe_reg_sr.c b/drivers/gpu/drm/xe/xe_reg_sr.c
> index 75aa4426b3ec6026bc13095df3c3f9cb0a794b97..83a668f2a0d5f16ea3f0b93eafb6b8e66abe3fe8 100644
> --- a/drivers/gpu/drm/xe/xe_reg_sr.c
> +++ b/drivers/gpu/drm/xe/xe_reg_sr.c
> @@ -21,6 +21,7 @@
>  #include "xe_gt_printk.h"
>  #include "xe_gt_types.h"
>  #include "xe_hw_engine_types.h"
> +#include "xe_lrc.h"
>  #include "xe_mmio.h"
>  #include "xe_rtp_types.h"
>
> @@ -242,3 +243,32 @@ void xe_reg_sr_readback_check(struct xe_reg_sr *sr,
>				   offset, mask, entry->set_bits, val & mask);
>	}
>  }
> +
> +/**
> + * xe_reg_sr_lrc_check() - Check LRC for registers referenced in save/restore
> + *     entries and check whether the programming is in place.
> + * @sr: Save/restore entries
> + * @gt: GT to read register from
> + * @hwe: Hardware engine type to check LRC for
> + * @p: DRM printer to report discrepancies on
> + */
> +void xe_reg_sr_lrc_check(struct xe_reg_sr *sr,
> +			 struct xe_gt *gt,
> +			 struct xe_hw_engine *hwe,
> +			 struct drm_printer *p)
> +{
> +	struct xe_reg_sr_entry *entry;
> +	unsigned long offset;
> +
> +	xa_for_each(&sr->xa, offset, entry) {
> +		u32 val;
> +		int ret = xe_lrc_lookup_default_reg_value(gt, hwe->class, offset, &val);
> +		u32 mask = entry->clr_bits | entry->set_bits;
> +
> +		if (ret == -ENOENT)
> +			drm_printf(p, "%#8lx :: not found in LRC for %s\n", offset, hwe->name);
> +		else if ((val & mask) != entry->set_bits)
> +			drm_printf(p, "%#8lx & %#10x :: expected %#10x got %#10x\n",
> +				   offset, mask, entry->set_bits, val & mask);
> +	}
> +}
> diff --git a/drivers/gpu/drm/xe/xe_reg_sr.h b/drivers/gpu/drm/xe/xe_reg_sr.h
> index cd133a09aa9b3f046dbd913aa9711250e771ee1a..1ec6e8ecf2784a3a832f30b7005f116ab7a368db 100644
> --- a/drivers/gpu/drm/xe/xe_reg_sr.h
> +++ b/drivers/gpu/drm/xe/xe_reg_sr.h
> @@ -22,6 +22,10 @@ void xe_reg_sr_dump(struct xe_reg_sr *sr, struct drm_printer *p);
>  void xe_reg_sr_readback_check(struct xe_reg_sr *sr,
>			      struct xe_gt *gt,
>			      struct drm_printer *p);
> +void xe_reg_sr_lrc_check(struct xe_reg_sr *sr,
> +			 struct xe_gt *gt,
> +			 struct xe_hw_engine *hwe,
> +			 struct drm_printer *p);
>
>  int xe_reg_sr_add(struct xe_reg_sr *sr, const struct xe_reg_sr_entry *e,
>		  struct xe_gt *gt);
>
> --
> 2.53.0
>