Re: [PATCH v6 23/24] drm/xe: Add modparam to enable / disable ULLS on migrate queue

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Sep 09, 2026 at 10:03:32AM +0200, Thomas Hellström wrote:
> On Fri, 2026-09-04 at 14:16 -0700, Matthew Brost wrote:
> > Having modparam to enable / disable ULLS on migrate queue will help
> > with
> > quick experiments.
> > 
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > Link:
> > https://patch.msgid.link/20260228013501.106680-26-matthew.brost@intel.com
> > Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> > ---
> >  drivers/gpu/drm/xe/xe_debugfs.c      | 1 +
> >  drivers/gpu/drm/xe/xe_defaults.h     | 1 +
> >  drivers/gpu/drm/xe/xe_device.c       | 1 +
> >  drivers/gpu/drm/xe/xe_device_types.h | 5 +++++
> >  drivers/gpu/drm/xe/xe_migrate.c      | 2 +-
> >  drivers/gpu/drm/xe/xe_module.c       | 4 ++++
> >  drivers/gpu/drm/xe/xe_module.h       | 1 +
> >  7 files changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_debugfs.c
> > b/drivers/gpu/drm/xe/xe_debugfs.c
> > index 80f62634fae5..f3f37d116d62 100644
> > --- a/drivers/gpu/drm/xe/xe_debugfs.c
> > +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> > @@ -222,6 +222,7 @@ static int info(struct seq_file *m, void *data)
> >  	drm_printf(&p, "revid %d\n", xe->info.revid);
> >  	drm_printf(&p, "tile_count %d\n", xe->info.tile_count);
> >  	drm_printf(&p, "vm_max_level %d\n", xe->info.vm_max_level);
> > +	drm_printf(&p, "ulls_enable %s\n", str_yes_no(xe-
> > >info.ulls_enable));
> >  	drm_printf(&p, "has_flat_ccs %s\n", str_yes_no(xe-
> > >info.has_flat_ccs));
> >  	drm_printf(&p, "has_usm %s\n", str_yes_no(xe-
> > >info.has_usm));
> >  	drm_printf(&p, "skip_guc_pc %s\n", str_yes_no(xe-
> > >info.skip_guc_pc));
> > diff --git a/drivers/gpu/drm/xe/xe_defaults.h
> > b/drivers/gpu/drm/xe/xe_defaults.h
> > index 0884224ef7c7..9c7e89a169c5 100644
> > --- a/drivers/gpu/drm/xe/xe_defaults.h
> > +++ b/drivers/gpu/drm/xe/xe_defaults.h
> > @@ -14,6 +14,7 @@
> >  #endif
> >  
> >  #define
> > XE_DEFAULT_PROBE_DISPLAY		IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
> > +#define XE_DEFAULT_ULLS_ENABLE			true
> >  #define XE_DEFAULT_VRAM_BAR_SIZE		0
> >  #define
> > XE_DEFAULT_FORCE_PROBE			CONFIG_DRM_XE_FORCE_PROBE
> >  #define XE_DEFAULT_MAX_VFS			~0
> > diff --git a/drivers/gpu/drm/xe/xe_device.c
> > b/drivers/gpu/drm/xe/xe_device.c
> > index ade971de1652..9b81759af28a 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -520,6 +520,7 @@ static void xe_device_parse_modparam(struct
> > xe_device *xe)
> >  	xe->atomic_svm_timeslice_ms = 5;
> >  	xe->min_run_period_lr_ms = 5;
> >  	xe->info.num_pf_work = xe_modparam.num_pf_work;
> > +	xe->info.ulls_enable = xe_modparam.ulls_enable;
> 
> migrate_ulls_enable?
> 
> >  	if (xe->info.num_pf_work < 1)
> >  		xe->info.num_pf_work = 1;
> >  	else if (xe->info.num_pf_work > XE_PAGEFAULT_WORK_MAX)
> > diff --git a/drivers/gpu/drm/xe/xe_device_types.h
> > b/drivers/gpu/drm/xe/xe_device_types.h
> > index 997ac82fd571..fd4560378f5b 100644
> > --- a/drivers/gpu/drm/xe/xe_device_types.h
> > +++ b/drivers/gpu/drm/xe/xe_device_types.h
> > @@ -245,6 +245,11 @@ struct xe_device {
> >  		u8 skip_pcode:1;
> >  		/** @info.needs_shared_vf_gt_wq: needs shared GT WQ
> > on VF */
> >  		u8 needs_shared_vf_gt_wq:1;
> > +		/**
> > +		 * @info.ulls_enable: Enable ULLS on migration queue
> > in LR VM
> > +		 * open
> > +		 */
> > +		u8 ulls_enable:1;
> >  	} info;
> >  
> >  	/** @wa_active: keep track of active workarounds */
> > diff --git a/drivers/gpu/drm/xe/xe_migrate.c
> > b/drivers/gpu/drm/xe/xe_migrate.c
> > index 3e59aeeca614..3bc78f761f23 100644
> > --- a/drivers/gpu/drm/xe/xe_migrate.c
> > +++ b/drivers/gpu/drm/xe/xe_migrate.c
> > @@ -523,7 +523,7 @@ void xe_migrate_ulls_enter(struct xe_migrate *m)
> >  
> >  	xe_assert(xe, xe->info.has_usm);
> >  
> > -	if (!IS_DGFX(xe))
> > +	if (!IS_DGFX(xe) || !xe->info.ulls_enable)
> >  		return;
> >  
> >  job_alloc:
> > diff --git a/drivers/gpu/drm/xe/xe_module.c
> > b/drivers/gpu/drm/xe/xe_module.c
> > index 4bc28dfc1992..a8b546550ead 100644
> > --- a/drivers/gpu/drm/xe/xe_module.c
> > +++ b/drivers/gpu/drm/xe/xe_module.c
> > @@ -23,6 +23,7 @@
> >  
> >  struct xe_modparam xe_modparam = {
> >  	.probe_display =	XE_DEFAULT_PROBE_DISPLAY,
> > +	.ulls_enable =		XE_DEFAULT_ULLS_ENABLE,
> >  	.guc_log_level =	XE_DEFAULT_GUC_LOG_LEVEL,
> >  	.force_probe =		XE_DEFAULT_FORCE_PROBE,
> >  #ifdef CONFIG_PCI_IOV
> > @@ -44,6 +45,9 @@ MODULE_PARM_DESC(probe_display, "Probe display HW,
> > otherwise it's left untouched
> >  		 "[default=" __stringify(XE_DEFAULT_PROBE_DISPLAY)
> > "])");
> >  #endif
> >  
> > +module_param_named(ulls_enable, xe_modparam.ulls_enable, bool,
> > 0444);
> > +MODULE_PARM_DESC(ulls_enable, "Enable ULLS on migration queue if LR
> > VM open (default: true)");
> > +
> >  module_param_named(vram_bar_size, xe_modparam.force_vram_bar_size,
> > int, 0600);
> >  MODULE_PARM_DESC(vram_bar_size, "Set the vram bar size in MiB
> > (<0=disable-resize, 0=max-needed-size, >0=force-size "
> >  		 "[default=" __stringify(XE_DEFAULT_VRAM_BAR_SIZE)
> > "])");
> > diff --git a/drivers/gpu/drm/xe/xe_module.h
> > b/drivers/gpu/drm/xe/xe_module.h
> > index 6272d9e41207..5d88448c298a 100644
> > --- a/drivers/gpu/drm/xe/xe_module.h
> > +++ b/drivers/gpu/drm/xe/xe_module.h
> > @@ -13,6 +13,7 @@ struct work_struct;
> >  /* Module modprobe variables */
> >  struct xe_modparam {
> >  	bool probe_display;
> > +	bool ulls_enable;
> 
> Shouldn't this be per device rather than per driver? So sysfs knob,
> perhaps with a modparam default?
> 

I didn't really want to add a userspace ABI, such as a sysfs interface,
at this point, so I went with a module parameter instead. It also seems
a bit odd to enable this on some devices but not others.

The main reason for having this at all is to allow quick performance
comparisons and, of course, to give us an easy way to disable it and
retest if we encounter workloads where kernel jobs appear to hang.

I'm open to changing this as the series receives more review and we get
closer to merging.

Matt

> Thanks,
> Thomas
> 
> 
> 
> >  	int force_vram_bar_size;
> >  	int guc_log_level;
> >  	char *guc_firmware_path;