On Tue, Oct 27, 2026 at 09:50:31PM -0700, Daniel Colascione wrote: Thanks for this - I have wondered why when I probe Xe it takes a noticable amount of time - I suspect this is why.
> We initialize the GGTT GPU page tables on startup. On my LNL, doing > this one PTE at a time through the BAR takes about 900 ms and involves > a million small writes. > > Use the GPU's copy engine to fill the PTEs in batches. We build > MI_UPDATE_GTT commands in normal memory, then send them to the GPU for > execution. This way, we avoid the per-PTE MMIO write and do the PTE fill > in 36.77 ms, ~24x faster. > > The new path is enabled by default. xe.ggtt_init_gpu=0 uses the old > CPU-driven path. > > LNL-only because I can test only LNL. Might it work elsewhere? > It should work anywhere. > Signed-off-by: Daniel Colascione <[email protected]> > --- > drivers/gpu/drm/xe/instructions/xe_mi_commands.h | 2 + > drivers/gpu/drm/xe/xe_ggtt.c | 128 > +++++++++++++++++++++-- > drivers/gpu/drm/xe/xe_ggtt.h | 1 + > drivers/gpu/drm/xe/xe_gt.c | 3 +- > drivers/gpu/drm/xe/xe_module.c | 5 + > drivers/gpu/drm/xe/xe_module.h | 1 + > 6 files changed, 130 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/xe/instructions/xe_mi_commands.h > b/drivers/gpu/drm/xe/instructions/xe_mi_commands.h > index 835ef75bbeb1..04bd4225adef 100644 > --- a/drivers/gpu/drm/xe/instructions/xe_mi_commands.h > +++ b/drivers/gpu/drm/xe/instructions/xe_mi_commands.h > @@ -62,6 +62,8 @@ > #define MI_LRI_FORCE_POSTED REG_BIT(12) > #define MI_LRI_LEN(x) (((x) & 0xff) + 1) > > +#define MI_UPDATE_GTT __MI_INSTR(0x23) > + > #define MI_STORE_REGISTER_MEM (__MI_INSTR(0x24) | > XE_INSTR_NUM_DW(4)) > #define MI_SRM_USE_GGTT REG_BIT(22) > #define MI_SRM_ADD_CS_OFFSET REG_BIT(19) > diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c > index 773777afc68a..3737d085ae68 100644 > --- a/drivers/gpu/drm/xe/xe_ggtt.c > +++ b/drivers/gpu/drm/xe/xe_ggtt.c > @@ -15,20 +15,25 @@ > #include <drm/intel/i915_drm.h> > #include <generated/xe_wa_oob.h> > > +#include "instructions/xe_mi_commands.h" > #include "regs/xe_gt_regs.h" > #include "regs/xe_gtt_defs.h" > #include "regs/xe_regs.h" > #include "xe_assert.h" > +#include "xe_bb.h" > #include "xe_bo.h" > +#include "xe_exec_queue.h" > #include "xe_gt.h" > #include "xe_gt_printk.h" > #include "xe_gt_types.h" > #include "xe_log.h" > #include "xe_map.h" > #include "xe_mmio.h" > +#include "xe_module.h" > #include "xe_pat.h" > #include "xe_pm.h" > #include "xe_res_cursor.h" > +#include "xe_sched_job.h" > #include "xe_sriov.h" > #include "xe_tile_printk.h" > #include "xe_tile_sriov_vf.h" > @@ -384,9 +389,9 @@ static void dev_fini_ggtt(void *arg) > * @ggtt: the &xe_ggtt to be initialized > * > * It allows to create new mappings usable by the GuC. > - * Mappings are not usable by the HW engines, as it doesn't have scratch nor > - * initial clear done to it yet. That will happen in the regular, non-early > - * GGTT initialization. > + * HW engines may only access explicitly mapped bootstrap buffers until the > + * remaining holes have been cleared. Normally xe_ggtt_init() does this; > + * the GPU clear finishes this work in xe_ggtt_init_late(). > * > * Return: 0 on success or a negative error code on failure. > */ > @@ -456,18 +461,119 @@ ALLOW_ERROR_INJECTION(xe_ggtt_init_early, ERRNO); /* > See xe_pci_probe() */ > static void xe_ggtt_invalidate(struct xe_ggtt *ggtt); > static void xe_ggtt_invalidate_engine(struct xe_ggtt *ggtt); > > -static void xe_ggtt_initial_clear(struct xe_ggtt *ggtt) > +static bool ggtt_use_gpu(struct xe_device *xe) > { > + return xe_modparam.ggtt_init_gpu && xe->info.platform == XE_LUNARLAKE; Per below (last reply) - let's just turn this everywhere in platform config. > +} > + > +/* > + * MI_UPDATE_GTT takes at most 511 inline PTEs (ACM PRM, Vol. 2a, > + * pp. 1096-1097). Limit batches to 512K to fit the kernel batch pool. > + */ > +#define GGTT_INIT_PTES_PER_PACKET 511 > +#define GGTT_INIT_BATCH_DWORDS (128 * (2 + 2 * > GGTT_INIT_PTES_PER_PACKET)) > + > +static int ggtt_clear_gpu(struct xe_exec_queue *q, struct xe_bb *bb, > + u64 pte, u64 start, u64 end) s/ggtt_clear_gpu/xe_ggtt_clear_gpu > +{ > + while (start < end) { > + struct xe_sched_job *job; > + int err; > + > + bb->len = 0; > + while (start < end && bb->len < GGTT_INIT_BATCH_DWORDS) { > + u32 count = min_t(u64, (end - start) >> XE_PTE_SHIFT, > + GGTT_INIT_PTES_PER_PACKET); > + > + bb->cs[bb->len++] = MI_UPDATE_GTT | (2 * count); > + bb->cs[bb->len++] = start; > + memset64((u64 *)&bb->cs[bb->len], pte, count); > + bb->len += 2 * count; > + start += (u64)count << XE_PTE_SHIFT; > + } > + > + job = xe_bb_create_job(q, bb); > + if (IS_ERR(job)) > + return PTR_ERR(job); > + > + xe_sched_job_arm(job); > + xe_sched_job_get(job); > + xe_sched_job_push(job); > + > + dma_fence_wait(&job->drm.s_fence->finished, false); You can pipeline this function. i.e., generate fences in a loop and only wait on the last one. This is fairly common pattern, xe_svm_copy is an example of function with loop which issues a bunch of jobs and only waits on the very last fence. > + err = dma_fence_get_status(&job->drm.s_fence->finished); > + xe_sched_job_put(job); > + if (err < 0) > + return err; > + } > + > + return 0; > +} > + > +static int xe_ggtt_initial_clear(struct xe_ggtt *ggtt, struct xe_hw_engine > *hwe) > +{ > + struct xe_device *xe = tile_to_xe(ggtt->tile); > + struct xe_exec_queue *q = NULL; > + struct xe_bb *bb = NULL; > struct drm_mm_node *hole; > - u64 start, end; > + u16 pat_index = xe_cache_pat_idx(xe, XE_CACHE_WB); > + u64 start, end, pte = 0; > + int err = 0; > + > + if (hwe) { > + q = xe_exec_queue_create(xe, NULL, BIT(hwe->logical_instance), > 1, > + hwe, EXEC_QUEUE_FLAG_KERNEL, 0); Can we use migration queue (xe_migrate.c)? We'd probably need a bit of layering rework to allow another layer like GGTT to safely use this queue but that seems better to keep a single isolated kernel queue. > + if (IS_ERR(q)) > + return PTR_ERR(q); > + > + bb = xe_bb_new(hwe->gt, GGTT_INIT_BATCH_DWORDS, false); > + if (IS_ERR(bb)) { > + err = PTR_ERR(bb); > + goto put_queue; > + } > + } > > - /* Display may have allocated inside ggtt, so be careful with clearing > here */ > + if (ggtt->scratch) > + pte = xe_bo_addr(ggtt->scratch, 0, XE_PAGE_SIZE) | > + ggtt->pt_ops->pte_encode_flags(ggtt->scratch, pat_index); > + > + /* Preserve boot mappings and exclude allocator changes until > completion. */ > mutex_lock(&ggtt->lock); CI and Sashiko correctly complain that 'ggtt->lock' is tainted with reclaim and ggtt_clear_gpu will be a malloc for the GPU job. I'm actually questioning if this taint is actually valid. afiak we never shrink or evict anything GGTT mapped (i.e., that memory is always pinned). So I think we can just drop the taint - I'd do this in a standalone patch. > - drm_mm_for_each_hole(hole, &ggtt->mm, start, end) > - xe_ggtt_clear(ggtt, ggtt->start + start, end - start); > + drm_mm_for_each_hole(hole, &ggtt->mm, start, end) { > + if (q) > + err = ggtt_clear_gpu(q, bb, pte, ggtt->start + start, > ggtt->start + end); > + else > + xe_ggtt_clear(ggtt, ggtt->start + start, end - start); > + if (err) > + goto unlock; > + } > > xe_ggtt_invalidate(ggtt); > +unlock: > mutex_unlock(&ggtt->lock); > + xe_bb_free(bb, NULL); > +put_queue: > + if (q) > + xe_exec_queue_put(q); > + return err; > +} > + > +/** > + * xe_ggtt_init_late - Finish deferred GGTT initialization after GuC startup > + * @ggtt: GGTT to initialize > + * > + * All default contexts must be recorded before waiting for a GPU clear with > + * the GGTT mutex held, so reset recovery need not allocate context mappings. > + * > + * Return: 0 on success or a negative error code on failure. > + */ > +int xe_ggtt_init_late(struct xe_ggtt *ggtt) > +{ > + if (!ggtt_use_gpu(tile_to_xe(ggtt->tile))) > + return 0; > + > + return xe_ggtt_initial_clear(ggtt, > xe_gt_hw_engine(ggtt->tile->primary_gt, > + XE_ENGINE_CLASS_COPY, > 0, false)); > } > > static void ggtt_node_fini(struct xe_ggtt_node *node) > @@ -536,6 +642,9 @@ void xe_ggtt_node_remove(struct xe_ggtt_node *node, bool > invalidate) > * xe_ggtt_init - Regular non-early GGTT initialization > * @ggtt: the &xe_ggtt to be initialized > * > + * Allocate the scratch page if needed and clear unused entries, or defer > + * the clear to xe_ggtt_init_late() when GPU initialization is enabled. > + * > * Return: 0 on success or a negative error code on failure. > */ > int xe_ggtt_init(struct xe_ggtt *ggtt) > @@ -572,7 +681,8 @@ int xe_ggtt_init(struct xe_ggtt *ggtt) > xe_bo_size(ggtt->scratch)); > } > > - xe_ggtt_initial_clear(ggtt); > + if (!ggtt_use_gpu(xe)) > + xe_ggtt_initial_clear(ggtt, NULL); > > return devm_add_action_or_reset(xe->drm.dev, ggtt_fini, ggtt); > err: > diff --git a/drivers/gpu/drm/xe/xe_ggtt.h b/drivers/gpu/drm/xe/xe_ggtt.h > index c864cc975a69..cae8600d7142 100644 > --- a/drivers/gpu/drm/xe/xe_ggtt.h > +++ b/drivers/gpu/drm/xe/xe_ggtt.h > @@ -17,6 +17,7 @@ struct xe_ggtt *xe_ggtt_alloc(struct xe_tile *tile); > int xe_ggtt_init_early(struct xe_ggtt *ggtt); > int xe_ggtt_init_kunit(struct xe_ggtt *ggtt, u32 reserved, u32 size); > int xe_ggtt_init(struct xe_ggtt *ggtt); > +int xe_ggtt_init_late(struct xe_ggtt *ggtt); > > void xe_ggtt_shift_nodes(struct xe_ggtt *ggtt, u64 new_base); > u64 xe_ggtt_start(struct xe_ggtt *ggtt); > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c > index 45fb074812a2..d1c14a02de66 100644 > --- a/drivers/gpu/drm/xe/xe_gt.c > +++ b/drivers/gpu/drm/xe/xe_gt.c > @@ -686,7 +686,8 @@ static int gt_init_with_all_forcewake(struct xe_gt *gt) > if (err) > return err; > > - return 0; > + return xe_gt_is_main_type(gt) ? > + xe_ggtt_init_late(gt_to_tile(gt)->mem.ggtt) : 0; > } > > static void xe_gt_fini(void *arg) > diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c > index 897724cb5cfb..44cc0107cf53 100644 > --- a/drivers/gpu/drm/xe/xe_module.c > +++ b/drivers/gpu/drm/xe/xe_module.c > @@ -22,6 +22,7 @@ > #include "xe_sched_job.h" > > struct xe_modparam xe_modparam = { > + .ggtt_init_gpu = true, I'd make this platform param in xe_pci.c and turn it on for every platform which supports the required GPU instructions (likely every Xe supported platform). We can let this soak in for a couple months in our CI before getting into a kernel release to ensure everything is functional and then perhaps rip out the non-GPU clear code. Matt > .probe_display = XE_DEFAULT_PROBE_DISPLAY, > .guc_log_level = XE_DEFAULT_GUC_LOG_LEVEL, > .force_probe = XE_DEFAULT_FORCE_PROBE, > @@ -34,6 +35,10 @@ struct xe_modparam xe_modparam = { > /* the rest are 0 by default */ > }; > > +module_param_named(ggtt_init_gpu, xe_modparam.ggtt_init_gpu, bool, 0444); > +MODULE_PARM_DESC(ggtt_init_gpu, > + "GPU GGTT initialization on Lunar Lake (default: true)"); > + > module_param_named(svm_notifier_size, xe_modparam.svm_notifier_size, uint, > 0600); > MODULE_PARM_DESC(svm_notifier_size, "Set the svm notifier size in MiB, must > be power of 2 " > "[default=" __stringify(XE_DEFAULT_SVM_NOTIFIER_SIZE) "]"); > diff --git a/drivers/gpu/drm/xe/xe_module.h b/drivers/gpu/drm/xe/xe_module.h > index 6272d9e41207..8aa91822c35c 100644 > --- a/drivers/gpu/drm/xe/xe_module.h > +++ b/drivers/gpu/drm/xe/xe_module.h > @@ -12,6 +12,7 @@ struct work_struct; > > /* Module modprobe variables */ > struct xe_modparam { > + bool ggtt_init_gpu; > bool probe_display; > int force_vram_bar_size; > int guc_log_level;
