RE: [PATCH 07/10] drm/xe: Move GFX_MODE programming to RTP

Lin, Shuicheng
2026-04-24T17:24:41Z

On Thu, Apr 23, 2026 3:49 PM Matt Roper wrote:
> The write GFX_MODE to disable engine "legacy mode" and to enable MSI-X
> support 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. This also helps
> consolidate common logic that was duplicated between the main driver
> initialization path and the dead-code execlist initialization path.
>
> This also allows us to drop GFX_MODE from the list of extra registers to be
> added to the GuC ADS' save-restore list since all registers on the RTP table are
> added automatically.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/xe/xe_execlist.c | 5 -----
> drivers/gpu/drm/xe/xe_guc_ads.c | 1 -
> drivers/gpu/drm/xe/xe_hw_engine.c | 16 ++++++++++------
> drivers/gpu/drm/xe/xe_rtp.c | 8 ++++++++
> drivers/gpu/drm/xe/xe_rtp.h | 12 ++++++++++++
> 5 files changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_execlist.c
> b/drivers/gpu/drm/xe/xe_execlist.c
> index 337b9b4e8b4a..9fb99c038ea8 100644
> --- a/drivers/gpu/drm/xe/xe_execlist.c
> +++ b/drivers/gpu/drm/xe/xe_execlist.c
> @@ -47,7 +47,6 @@ static void __start_lrc(struct xe_hw_engine *hwe, struct
> xe_lrc *lrc,
> struct xe_mmio *mmio = &gt->mmio;
> struct xe_device *xe = gt_to_xe(gt);
> u64 lrc_desc;
> - u32 ring_mode =
> REG_MASKED_FIELD_ENABLE(GFX_DISABLE_LEGACY_MODE);
>
> lrc_desc = xe_lrc_descriptor(lrc);
>
> @@ -78,10 +77,6 @@ static void __start_lrc(struct xe_hw_engine *hwe,
> struct xe_lrc *lrc,
> xe_bo_ggtt_addr(hwe->hwsp));
> xe_mmio_read32(mmio, RING_HWS_PGA(hwe->mmio_base));
>
> - if (xe_device_has_msix(gt_to_xe(hwe->gt)))
> - ring_mode |=
> REG_MASKED_FIELD_ENABLE(GFX_MSIX_INTERRUPT_ENABLE);
> - xe_mmio_write32(mmio, GFX_MODE(hwe->mmio_base),
> ring_mode);
> -
> xe_mmio_write32(mmio, RING_EXECLIST_SQ_CONTENTS_LO(hwe-
> >mmio_base),
> lower_32_bits(lrc_desc));
> xe_mmio_write32(mmio, RING_EXECLIST_SQ_CONTENTS_HI(hwe-
> >mmio_base),
> diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c
> b/drivers/gpu/drm/xe/xe_guc_ads.c index b403ee0b5e74..ce651da6f318
> 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ads.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
> @@ -745,7 +745,6 @@ static unsigned int guc_mmio_regset_write(struct
> xe_guc_ads *ads,
> struct xe_reg reg;
> bool skip;
> } *e, extra_regs[] = {
> - { .reg = GFX_MODE(hwe->mmio_base), },
> { .reg = RING_HWS_PGA(hwe->mmio_base),
> },
> { .reg = RING_IMR(hwe->mmio_base), },
> { .reg = CCS_MODE,
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c
> b/drivers/gpu/drm/xe/xe_hw_engine.c
> index d28f50a64d88..8d54f943f507 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> @@ -325,14 +325,8 @@ u32 xe_hw_engine_mmio_read32(struct
> xe_hw_engine *hwe, struct xe_reg reg)
>
> 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_HWS_PGA(0),
> xe_bo_ggtt_addr(hwe->hwsp));
> -
> - if (xe_device_has_msix(gt_to_xe(hwe->gt)))
> - ring_mode |=
> REG_MASKED_FIELD_ENABLE(GFX_MSIX_INTERRUPT_ENABLE);
> - xe_hw_engine_mmio_write32(hwe, GFX_MODE(0), ring_mode);
> xe_hw_engine_mmio_write32(hwe, RING_MI_MODE(0),
> REG_MASKED_FIELD_DISABLE(STOP_RING));
> xe_hw_engine_mmio_read32(hwe, RING_MI_MODE(0)); @@ -441,6
> +435,11 @@ hw_engine_setup_default_state(struct xe_hw_engine *hwe)
> XE_RTP_ACTIONS(SET(RING_HWSTAM(0), ~0x0,
> XE_RTP_ACTION_FLAG(ENGINE_BASE)))
> },
> + { XE_RTP_NAME("Disable engine 'legacy' mode"),
> + XE_RTP_RULES(FUNC(xe_rtp_match_always)),
> + XE_RTP_ACTIONS(SET(GFX_MODE(0),
> GFX_DISABLE_LEGACY_MODE,
> + 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
> @@ -473,6 +472,11 @@ hw_engine_setup_default_state(struct
> xe_hw_engine *hwe)
> XE_RTP_ACTIONS(FIELD_SET(RCU_MODE,
> RCU_MODE_FIXED_SLICE_CCS_MODE,
>
> RCU_MODE_FIXED_SLICE_CCS_MODE))
> },
> + { XE_RTP_NAME("Enable MSI-X interrupt support"),
> + XE_RTP_RULES(FUNC(xe_rtp_match_always)),

The rule should be xe_rtp_match_has_msix?

Shuicheng

> + XE_RTP_ACTIONS(SET(GFX_MODE(0),
> GFX_MSIX_INTERRUPT_ENABLE,
> + XE_RTP_ACTION_FLAG(ENGINE_BASE)))
> + },
> };
>
> xe_rtp_process_to_sr(&ctx, engine_entries,
> ARRAY_SIZE(engine_entries), diff --git a/drivers/gpu/drm/xe/xe_rtp.c
> b/drivers/gpu/drm/xe/xe_rtp.c index 728933a1c34f..1a4dcbbbc176 100644
> --- a/drivers/gpu/drm/xe/xe_rtp.c
> +++ b/drivers/gpu/drm/xe/xe_rtp.c
> @@ -10,6 +10,7 @@
> #include <uapi/drm/xe_drm.h>
>
> #include "xe_configfs.h"
> +#include "xe_device.h"
> #include "xe_gt.h"
> #include "xe_gt_topology.h"
> #include "xe_reg_sr.h"
> @@ -404,3 +405,10 @@ bool xe_rtp_match_has_flat_ccs(const struct
> xe_device *xe, {
> return xe->info.has_flat_ccs;
> }
> +
> +bool xe_rtp_match_has_msix(const struct xe_device *xe,
> + const struct xe_gt *gt,
> + const struct xe_hw_engine *hwe)
> +{
> + return xe_device_has_msix(xe);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_rtp.h b/drivers/gpu/drm/xe/xe_rtp.h index
> d058a629cd3e..562082b18d7b 100644
> --- a/drivers/gpu/drm/xe/xe_rtp.h
> +++ b/drivers/gpu/drm/xe/xe_rtp.h
> @@ -536,4 +536,16 @@ bool xe_rtp_match_has_flat_ccs(const struct
> xe_device *xe,
> const struct xe_gt *gt,
> const struct xe_hw_engine *hwe);
>
> +/**
> + * xe_rtp_match_has_msix - Match when platform has MSI-X
> + * @xe: Device structure
> + * @gt: GT structure
> + * @hwe: Engine instance
> + *
> + * Returns: true if platform has MSI-X interrupt support */ bool
> +xe_rtp_match_has_msix(const struct xe_device *xe,
> + const struct xe_gt *gt,
> + const struct xe_hw_engine *hwe);
> +
> #endif
>
> --
> 2.53.0