Re: [PATCH v17 5/8] drm/xe: Export xe_emit_aux_table_inv

Matthew Brost
2026-03-03T18:34:51Z

On Wed, Jan 28, 2026 at 10:13:30AM +0000, Tvrtko Ursulin wrote:
> Export the existing AuxCCS invalidation ring buffer programming helper
> which we will need to use to setup the indirect context workaround in the
> next patch.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/xe/xe_ring_ops.c | 67 +++++++++++++++++++++-----------
> drivers/gpu/drm/xe/xe_ring_ops.h | 3 ++
> 2 files changed, 48 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_ring_ops.c b/drivers/gpu/drm/xe/xe_ring_ops.c
> index 9867a316c12b..e6ecd70618c3 100644
> --- a/drivers/gpu/drm/xe/xe_ring_ops.c
> +++ b/drivers/gpu/drm/xe/xe_ring_ops.c
> @@ -12,6 +12,7 @@
> #include "regs/xe_engine_regs.h"
> #include "regs/xe_gt_regs.h"
> #include "xe_exec_queue.h"
> +#include "xe_gt_printk.h"
> #include "xe_gt_types.h"
> #include "xe_lrc.h"
> #include "xe_sched_job.h"
> @@ -48,22 +49,49 @@ static u32 preparser_disable(bool state)
> return MI_ARB_CHECK | BIT(8) | state;
> }
>
> -static int emit_aux_table_inv(struct xe_gt *gt, struct xe_reg reg,
> - u32 *dw, int i)
> +u32 *xe_emit_aux_table_inv(struct xe_hw_engine *hwe, u32 *cmd)

I'd prefer not to export the ring operation helpers directly as
functions, or at least not from this layer, as that opens a huge can of
worms. For example, look at the i915 ring operation helpers and the mess
they turned into—this is exactly what I'd like to avoid.

I see three options:

1. We add a xe_ring_ops_helper layer and stick this function there
2. In the next patch [1] we open code the ring programing there.
3. We add an emit_aux_table_inv vfunc to 'struct xe_ring_ops' and call
in [1].

I'd strongly lean toward option #3, with three vfuncs: render_compute,
video_decode, and video_encode. We'd also need to split
ring_ops_gen12_video into two distinct structures. This results in more
code, but it's structurally correct, avoids open coding in xe_lrc.c, and
prevents the floodgate that would be opened by exporting ring ops helper
functions.

Matt

[1] https://patchwork.freedesktop.org/patch/701228/?series=160748&rev=1

> {
> - dw[i++] = MI_LOAD_REGISTER_IMM | MI_LRI_NUM_REGS(1) | MI_LRI_MMIO_REMAP_EN;
> - dw[i++] = reg.addr + gt->mmio.adj_offset;
> - dw[i++] = AUX_INV;
> - dw[i++] = MI_SEMAPHORE_WAIT_TOKEN |
> - MI_SEMAPHORE_REGISTER_POLL |
> - MI_SEMAPHORE_POLL |
> - MI_SEMAPHORE_SAD_EQ_SDD;
> - dw[i++] = 0;
> - dw[i++] = reg.addr + gt->mmio.adj_offset;
> - dw[i++] = 0;
> - dw[i++] = 0;
> + struct xe_gt *gt = hwe->gt;
> + struct xe_reg reg;
>
> - return i;
> + switch (hwe->class) {
> + case XE_ENGINE_CLASS_RENDER:
> + case XE_ENGINE_CLASS_COMPUTE:
> + reg = CCS_AUX_INV;
> + break;
> + case XE_ENGINE_CLASS_VIDEO_DECODE:
> + reg = VD0_AUX_INV;
> + break;
> + case XE_ENGINE_CLASS_VIDEO_ENHANCE:
> + reg = VE0_AUX_INV;
> + break;
> + default:
> + xe_gt_err_once(gt, "AuxCCS invalidation not implemented!\n");
> + return cmd;
> + };
> +
> + *cmd++ = MI_LOAD_REGISTER_IMM | MI_LRI_NUM_REGS(1) |
> + MI_LRI_MMIO_REMAP_EN;
> + *cmd++ = reg.addr + gt->mmio.adj_offset;
> + *cmd++ = AUX_INV;
> + *cmd++ = MI_SEMAPHORE_WAIT_TOKEN | MI_SEMAPHORE_REGISTER_POLL |
> + MI_SEMAPHORE_POLL | MI_SEMAPHORE_SAD_EQ_SDD;
> + *cmd++ = 0;
> + *cmd++ = reg.addr + gt->mmio.adj_offset;
> + *cmd++ = 0;
> + *cmd++ = 0;
> +
> + return cmd;
> +}
> +
> +static int emit_aux_table_inv(struct xe_hw_engine *hwe, u32 *dw, int i)
> +{
> + u32 *start, *end;
> +
> + start = dw + i;
> + end = xe_emit_aux_table_inv(hwe, start);
> +
> + return i + (end - start);
> }
>
> static int emit_user_interrupt(u32 *dw, int i)
> @@ -324,7 +352,6 @@ static void __emit_job_gen12_video(struct xe_sched_job *job, struct xe_lrc *lrc,
> u32 ppgtt_flag = get_ppgtt_flag(job);
> struct xe_gt *gt = job->q->gt;
> struct xe_device *xe = gt_to_xe(gt);
> - bool decode = job->q->class == XE_ENGINE_CLASS_VIDEO_DECODE;
>
> *head = lrc->ring.tail;
>
> @@ -333,12 +360,8 @@ static void __emit_job_gen12_video(struct xe_sched_job *job, struct xe_lrc *lrc,
> dw[i++] = preparser_disable(true);
>
> /* hsdes: 1809175790 */
> - if (has_aux_ccs(xe)) {
> - if (decode)
> - i = emit_aux_table_inv(gt, VD0_AUX_INV, dw, i);
> - else
> - i = emit_aux_table_inv(gt, VE0_AUX_INV, dw, i);
> - }
> + if (has_aux_ccs(xe))
> + i = emit_aux_table_inv(job->q->hwe, dw, i);
>
> if (job->ring_ops_flush_tlb)
> i = emit_flush_imm_ggtt(xe_lrc_start_seqno_ggtt_addr(lrc),
> @@ -403,7 +426,7 @@ static void __emit_job_gen12_render_compute(struct xe_sched_job *job,
>
> /* hsdes: 1809175790 */
> if (aux_ccs)
> - i = emit_aux_table_inv(gt, CCS_AUX_INV, dw, i);
> + i = emit_aux_table_inv(job->q->hwe, dw, i);
>
> dw[i++] = preparser_disable(false);
>
> diff --git a/drivers/gpu/drm/xe/xe_ring_ops.h b/drivers/gpu/drm/xe/xe_ring_ops.h
> index e942735d76a6..5a2d32f9bb25 100644
> --- a/drivers/gpu/drm/xe/xe_ring_ops.h
> +++ b/drivers/gpu/drm/xe/xe_ring_ops.h
> @@ -10,8 +10,11 @@
> #include "xe_ring_ops_types.h"
>
> struct xe_gt;
> +struct xe_hw_engine;
>
> const struct xe_ring_ops *
> xe_ring_ops_get(struct xe_gt *gt, enum xe_engine_class class);
>
> +u32 *xe_emit_aux_table_inv(struct xe_hw_engine *hwe, u32 *cmd);
> +
> #endif
> --
> 2.52.0
>