RE: [PATCH 04/10] drm/xe: Move HWSTAM programming to RTP

Lin, Shuicheng
2026-04-24T18:15:19Z

On Thu, Apr 23, 2026 3:49 PM Matt Roper wrote:
> The write to RING_HWSTAM to disable hardware status page writes on
> interrupt was unnecessarily open-coded in xe_hw_engine_enable_ring(); it's
> preferable to do such programming in the engine_entries[] RTP table since gets
> reflected/verified in debugfs, and will also automatically ensure that the
> register is properly saved/restored around engine resets.
>
> In this case the HWSTAM register wasn't explicitly added to the GuC ADS'
> save-restore list, so there was the potential for the value to be lost on engine
> resets. This doesn't seem to have happened in practice, so likely the GuC
> firmware is automatically saving/restoring this register on our behalf, but we
> shouldn't rely on this implicit behavior going forward.
>
> One other slight change with this patch is that HWSTAM will now be
> programmed on the vestigial execlist (non-GuC) initialization path.
> Since the register's default value is 0x0 and the documentation indicates that
> it's only legal to leave a single bit unmasked at a time, this likely would have
> been an illegal situation if the execlist code were actually usable.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

LGTM.
Reviewed-by: Shuicheng Lin <shuicheng.lin@intel.com>

> ---
> drivers/gpu/drm/xe/xe_hw_engine.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c
> b/drivers/gpu/drm/xe/xe_hw_engine.c
> index 62a47d7e3f02..06eb655a4ae5 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> @@ -327,7 +327,6 @@ void xe_hw_engine_enable_ring(struct xe_hw_engine
> *hwe) {
> u32 ring_mode =
> REG_MASKED_FIELD_ENABLE(GFX_DISABLE_LEGACY_MODE);
>
> - xe_hw_engine_mmio_write32(hwe, RING_HWSTAM(0), ~0x0);
> xe_hw_engine_mmio_write32(hwe, RING_HWS_PGA(0),
> xe_bo_ggtt_addr(hwe->hwsp));
>
> @@ -437,6 +436,11 @@ hw_engine_setup_default_state(struct
> xe_hw_engine *hwe)
> ring_cmd_cctl_val,
>
> XE_RTP_ACTION_FLAG(ENGINE_BASE)))
> },
> + { XE_RTP_NAME("Disable HW status page updates for
> interrupts"),
> + XE_RTP_RULES(FUNC(xe_rtp_match_always)),
> + XE_RTP_ACTIONS(SET(RING_HWSTAM(0), ~0x0,
> + XE_RTP_ACTION_FLAG(ENGINE_BASE)))
> + },
> /*
> * To allow the GSC engine to go idle on MTL we need to enable
> * idle messaging and set the hysteresis value (we use 0xA=5us
>
> --
> 2.53.0