RE: [PATCH 09/10] drm/xe: Drop xe_hw_engine_mmio_write32()

Lin, Shuicheng
2026-04-24T17:06:16Z

On Thu, Apr 23, 2026 3:49 PM Matt Roper wrote:
> xe_hw_engine_mmio_write32() is only used in a single place and is easily
> replaced by a regular xe_mmio_write32() call. Register read/write interfaces
> are already complicated enough with MCR vs non-MCR handling, so we
> should avoid adding extra wrappers that just make it more confusing what to
> use.

The code itself LGTM.
BTW, the function declaration in header file could be removed also.

From git history, it is added for eudebug code. Add original author to confirm whether it is still needed or not.

Shuicheng

>
> xe_hw_engine_mmio_write32() did have a forcewake assertion that we're
> dropping here, but that assertion wasn't entirely correct anyway. It was
> checking hwe->domain which is currently set to XE_FW_RENDER for the BCS
> engine, even though BCS engines reside in the GT domain.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/xe/xe_hw_engine.c | 25 ++-----------------------
> 1 file changed, 2 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c
> b/drivers/gpu/drm/xe/xe_hw_engine.c
> index 154073027b51..a2a2b296af91 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> @@ -282,27 +282,6 @@ static void hw_engine_fini(void *arg)
> hwe->gt = NULL;
> }
>
> -/**
> - * xe_hw_engine_mmio_write32() - Write engine register
> - * @hwe: engine
> - * @reg: register to write into
> - * @val: desired 32-bit value to write
> - *
> - * This function will write val into an engine specific register.
> - * Forcewake must be held by the caller.
> - *
> - */
> -void xe_hw_engine_mmio_write32(struct xe_hw_engine *hwe,
> - struct xe_reg reg, u32 val)
> -{
> - xe_gt_assert(hwe->gt, !(reg.addr & hwe->mmio_base));
> - xe_force_wake_assert_held(gt_to_fw(hwe->gt), hwe->domain);
> -
> - reg.addr += hwe->mmio_base;
> -
> - xe_mmio_write32(&hwe->gt->mmio, reg, val);
> -}
> -
> /**
> * xe_hw_engine_mmio_read32() - Read engine register
> * @hwe: engine
> @@ -325,8 +304,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) {
> - xe_hw_engine_mmio_write32(hwe, RING_HWS_PGA(0),
> - xe_bo_ggtt_addr(hwe->hwsp));
> + xe_mmio_write32(&hwe->gt->mmio, RING_HWS_PGA(hwe-
> >mmio_base),
> + xe_bo_ggtt_addr(hwe->hwsp));
> }
>
> static bool xe_hw_engine_match_fixed_cslice_mode(const struct xe_device
> *xe,
>
> --
> 2.53.0