Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: "Wang, X" <x.wang@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>,
	<intel-xe@lists.freedesktop.org>,
	 Shuicheng Lin <shuicheng.lin@intel.com>
Subject: Re: [PATCH v3] drm/xe: restrict multi-lrc to VCS/VECS engines
Date: Tue, 24 Feb 2026 15:03:36 -0800	[thread overview]
Message-ID: <aZ4uSB27jzyG9c+F@lstrano-desk.jf.intel.com> (raw)
In-Reply-To: <c814ab36-0861-43c4-924e-4a6cdaa11376@intel.com>

On Tue, Feb 24, 2026 at 02:57:48PM -0800, Wang, X wrote:
> 
> On 2/24/2026 14:40, Matthew Brost wrote:
> > On Tue, Feb 24, 2026 at 02:20:33PM -0800, Wang, X wrote:
> > > On 2/24/2026 14:05, Matt Roper wrote:
> > > > On Mon, Feb 23, 2026 at 06:33:54PM -0800, Xin Wang wrote:
> > > > > Tighten uapi validation to restrict multi-lrc support to VIDEO_DECODE and
> > > > > VIDEO_ENHANCE engines only. This check should have been in place from the
> > > > > start, as the driver typically avoids allowing uapi cases that we have
> > > > > no userspace consumer for.
> > > > > 
> > > > > Additionally, the GuC firmware on ModSched platforms no longer supports
> > > > > multi-lrc on non-media engines.
> > > > > 
> > > > > V3:
> > > > >    - store a multi-lrc enable class mask in xe->info and populate from
> > > > >      xe_device_desc in xe_pci.c (Matthew Brost)
> > > > > 
> > > > > V2:
> > > > >    - correct the typo (Shuicheng)
> > > > >    - move the check earlier to avoid VM lookup (Shuicheng, Matt Roper)
> > > > >    - remove the graphics version check (Matt Roper)
> > > > >    - input more details in the commit info (Matt Roper)
> > > > > 
> > > > > Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> > > > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > > > Cc: Matthew Brost <matthew.brost@intel.com>
> > > > > Signed-off-by: Xin Wang <x.wang@intel.com>
> > > > > ---
> > > > >    drivers/gpu/drm/xe/xe_device_types.h |  2 ++
> > > > >    drivers/gpu/drm/xe/xe_exec_queue.c   |  5 +++++
> > > > >    drivers/gpu/drm/xe/xe_pci.c          | 17 +++++++++++++++++
> > > > >    drivers/gpu/drm/xe/xe_pci_types.h    |  1 +
> > > > >    4 files changed, 25 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> > > > > index 8f3ef836541e..caa8f34a6744 100644
> > > > > --- a/drivers/gpu/drm/xe/xe_device_types.h
> > > > > +++ b/drivers/gpu/drm/xe/xe_device_types.h
> > > > > @@ -138,6 +138,8 @@ struct xe_device {
> > > > >    		u8 tile_count;
> > > > >    		/** @info.max_gt_per_tile: Number of GT IDs allocated to each tile */
> > > > >    		u8 max_gt_per_tile;
> > > > > +		/** @info.multi_lrc_mask: bitmask of engine classes which support multi-lrc */
> > > > > +		u8 multi_lrc_mask;
> > > > >    		/** @info.gt_count: Total number of GTs for entire device */
> > > > >    		u8 gt_count;
> > > > >    		/** @info.vm_max_level: Max VM level */
> > > > > diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> > > > > index 66d0e10ee2c4..5abb29454d1f 100644
> > > > > --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> > > > > +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> > > > > @@ -1184,6 +1184,11 @@ int xe_exec_queue_create_ioctl(struct drm_device *dev, void *data,
> > > > >    		if (XE_IOCTL_DBG(xe, !hwe))
> > > > >    			return -EINVAL;
> > > > > +		/* multi-lrc is only supported on select engine classes */
> > > > > +		if (XE_IOCTL_DBG(xe, args->width > 1 &&
> > > > > +				 !(xe->info.multi_lrc_mask & BIT(hwe->class))))
> > > > > +			return -EOPNOTSUPP;
> > > > > +
> > > > >    		vm = xe_vm_lookup(xef, args->vm_id);
> > > > >    		if (XE_IOCTL_DBG(xe, !vm))
> > > > >    			return -ENOENT;
> > > > > diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> > > > > index e1f569235d8a..fe63387d4077 100644
> > > > > --- a/drivers/gpu/drm/xe/xe_pci.c
> > > > > +++ b/drivers/gpu/drm/xe/xe_pci.c
> > > > > @@ -194,6 +194,7 @@ static const struct xe_device_desc tgl_desc = {
> > > > >    	.has_llc = true,
> > > > >    	.has_sriov = true,
> > > > >    	.max_gt_per_tile = 1,
> > > > > +	.multi_lrc_mask = BIT(XE_ENGINE_CLASS_VIDEO_DECODE),
> > > > Is there a reason why some platforms list both VCS + VECS but others
> > > > only list VCS?  The new .multi_lrc_mask is intended to restrict usage to
> > > > just the engine type(s) where we have a real userspace consumer, and for
> > > > now I'd expect that to be the same across all current platforms.  We
> > Matt R beat me to this suggestion too, +1.
> > 
> > > > should double check whether the media driver is actively using this on
> > > > both media engine types or just one of them (I don't know off the top of
> > > > my head), and then set the mask accordingly.
> > > > 
> > > > An ioctl request for multi-LRC might also get rejected on a platform if
> > > > the engine fusing indicates that there aren't 2+ engines of the given
> > > > type, but that's an orthogonal check that's independent of the
> > > > multi_lrc_mask we're defining here.  Finding out how many engines
> > > > actually exist on a device is something that we can only find out at
> > > > runtime after reading the fuse registers for a specific device.
> > > > 
> > > > 
> > > > Matt
> > > I checked the specs and found that some devices have fewer than two VCS or
> > > VCES
> > > instances. In these cases, I don't think it's necessary to allow multi-LRC
> > > to
> > > those engines. This would prevent the injection of some illegal ioctl
> > > parameters.
> > See Matt's response above, we check in the IOCTL how many engines the
> > platform has, if it only has 1 the IOCTL will prevent multi-LRC queue
> > from being created.
> > 
> > So again, I'd just enable XE_ENGINE_CLASS_VIDEO_DECODE |
> > XE_ENGINE_CLASS_VIDEO_ENHANCE on platforms.
> > 
> > Matt
> This means we can enable the VCS/VECS multi-lrc on all platforms without
> considering the number of engines. I can create a macro definition and add
> it to all current platforms.
> 
> #define MULTI_LRC_MASK \
>         .multi_lrc_mask = BIT(XE_ENGINE_CLASS_VIDEO_DECODE) | \
> BIT(XE_ENGINE_CLASS_VIDEO_ENHANCE)
> 
> My understanding is that the advantage of adding a mask for each platform
> is that if we need to support multi-LRC for more engines in the future,
> we can simply modify the platform's xe_device_desc without changing the
> code logic.

Correct. Most behavioral choices in our driver should be encoded in the
xe_pci.c descriptors, which propagate into xe->info, and then the core
driver logic operates on xe->info to decide the appropriate behavior as
needed.

Matt

> Xin
> > > Xin
> > > > >    	.require_force_probe = true,
> > > > >    	.va_bits = 48,
> > > > >    	.vm_max_level = 3,
> > > > > @@ -208,6 +209,7 @@ static const struct xe_device_desc rkl_desc = {
> > > > >    	.has_display = true,
> > > > >    	.has_llc = true,
> > > > >    	.max_gt_per_tile = 1,
> > > > > +	.multi_lrc_mask = BIT(XE_ENGINE_CLASS_VIDEO_DECODE),
> > > > >    	.require_force_probe = true,
> > > > >    	.va_bits = 48,
> > > > >    	.vm_max_level = 3,
> > > > > @@ -225,6 +227,7 @@ static const struct xe_device_desc adl_s_desc = {
> > > > >    	.has_llc = true,
> > > > >    	.has_sriov = true,
> > > > >    	.max_gt_per_tile = 1,
> > > > > +	.multi_lrc_mask = BIT(XE_ENGINE_CLASS_VIDEO_DECODE),
> > > > >    	.require_force_probe = true,
> > > > >    	.subplatforms = (const struct xe_subplatform_desc[]) {
> > > > >    		{ XE_SUBPLATFORM_ALDERLAKE_S_RPLS, "RPLS", adls_rpls_ids },
> > > > > @@ -246,6 +249,7 @@ static const struct xe_device_desc adl_p_desc = {
> > > > >    	.has_llc = true,
> > > > >    	.has_sriov = true,
> > > > >    	.max_gt_per_tile = 1,
> > > > > +	.multi_lrc_mask = BIT(XE_ENGINE_CLASS_VIDEO_DECODE),
> > > > >    	.require_force_probe = true,
> > > > >    	.subplatforms = (const struct xe_subplatform_desc[]) {
> > > > >    		{ XE_SUBPLATFORM_ALDERLAKE_P_RPLU, "RPLU", adlp_rplu_ids },
> > > > > @@ -283,6 +287,7 @@ static const struct xe_device_desc dg1_desc = {
> > > > >    	.has_gsc_nvm = 1,
> > > > >    	.has_heci_gscfi = 1,
> > > > >    	.max_gt_per_tile = 1,
> > > > > +	.multi_lrc_mask = BIT(XE_ENGINE_CLASS_VIDEO_DECODE),
> > > > >    	.require_force_probe = true,
> > > > >    	.va_bits = 48,
> > > > >    	.vm_max_level = 3,
> > > > > @@ -313,6 +318,8 @@ static const struct xe_device_desc ats_m_desc = {
> > > > >    	.pre_gmdid_media_ip = &media_ip_xehpm,
> > > > >    	.dma_mask_size = 46,
> > > > >    	.max_gt_per_tile = 1,
> > > > > +	.multi_lrc_mask = BIT(XE_ENGINE_CLASS_VIDEO_DECODE) |
> > > > > +			  BIT(XE_ENGINE_CLASS_VIDEO_ENHANCE),
> > > > >    	.require_force_probe = true,
> > > > >    	DG2_FEATURES,
> > > > > @@ -325,6 +332,8 @@ static const struct xe_device_desc dg2_desc = {
> > > > >    	.pre_gmdid_media_ip = &media_ip_xehpm,
> > > > >    	.dma_mask_size = 46,
> > > > >    	.max_gt_per_tile = 1,
> > > > > +	.multi_lrc_mask = BIT(XE_ENGINE_CLASS_VIDEO_DECODE) |
> > > > > +			  BIT(XE_ENGINE_CLASS_VIDEO_ENHANCE),
> > > > >    	.require_force_probe = true,
> > > > >    	DG2_FEATURES,
> > > > > @@ -358,6 +367,7 @@ static const struct xe_device_desc mtl_desc = {
> > > > >    	.has_display = true,
> > > > >    	.has_pxp = true,
> > > > >    	.max_gt_per_tile = 2,
> > > > > +	.multi_lrc_mask = BIT(XE_ENGINE_CLASS_VIDEO_DECODE),
> > > > >    	.va_bits = 48,
> > > > >    	.vm_max_level = 3,
> > > > >    };
> > > > > @@ -393,6 +403,8 @@ static const struct xe_device_desc bmg_desc = {
> > > > >    	.has_soc_remapper_telem = true,
> > > > >    	.has_sriov = true,
> > > > >    	.max_gt_per_tile = 2,
> > > > > +	.multi_lrc_mask = BIT(XE_ENGINE_CLASS_VIDEO_DECODE) |
> > > > > +			  BIT(XE_ENGINE_CLASS_VIDEO_ENHANCE),
> > > > >    	.needs_scratch = true,
> > > > >    	.subplatforms = (const struct xe_subplatform_desc[]) {
> > > > >    		{ XE_SUBPLATFORM_BATTLEMAGE_G21, "G21", bmg_g21_ids },
> > > > > @@ -445,6 +457,8 @@ static const struct xe_device_desc cri_desc = {
> > > > >    	.has_soc_remapper_telem = true,
> > > > >    	.has_sriov = true,
> > > > >    	.max_gt_per_tile = 2,
> > > > > +	.multi_lrc_mask = BIT(XE_ENGINE_CLASS_VIDEO_DECODE) |
> > > > > +			  BIT(XE_ENGINE_CLASS_VIDEO_ENHANCE),
> > > > >    	.require_force_probe = true,
> > > > >    	.va_bits = 57,
> > > > >    	.vm_max_level = 4,
> > > > > @@ -459,6 +473,8 @@ static const struct xe_device_desc nvlp_desc = {
> > > > >    	.has_page_reclaim_hw_assist = true,
> > > > >    	.has_pre_prod_wa = true,
> > > > >    	.max_gt_per_tile = 2,
> > > > > +	.multi_lrc_mask = BIT(XE_ENGINE_CLASS_VIDEO_DECODE) |
> > > > > +			  BIT(XE_ENGINE_CLASS_VIDEO_ENHANCE),
> > > > >    	.require_force_probe = true,
> > > > >    	.va_bits = 48,
> > > > >    	.vm_max_level = 4,
> > > > > @@ -746,6 +762,7 @@ static int xe_info_init_early(struct xe_device *xe,
> > > > >    	xe->info.skip_pcode = desc->skip_pcode;
> > > > >    	xe->info.needs_scratch = desc->needs_scratch;
> > > > >    	xe->info.needs_shared_vf_gt_wq = desc->needs_shared_vf_gt_wq;
> > > > > +	xe->info.multi_lrc_mask = desc->multi_lrc_mask;
> > > > >    	xe->info.probe_display = IS_ENABLED(CONFIG_DRM_XE_DISPLAY) &&
> > > > >    				 xe_modparam.probe_display &&
> > > > > diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
> > > > > index 470d31a1f0d6..47e8a1552c2b 100644
> > > > > --- a/drivers/gpu/drm/xe/xe_pci_types.h
> > > > > +++ b/drivers/gpu/drm/xe/xe_pci_types.h
> > > > > @@ -30,6 +30,7 @@ struct xe_device_desc {
> > > > >    	u8 dma_mask_size;
> > > > >    	u8 max_remote_tiles:2;
> > > > >    	u8 max_gt_per_tile:2;
> > > > > +	u8 multi_lrc_mask;
> > > > >    	u8 va_bits;
> > > > >    	u8 vm_max_level;
> > > > >    	u8 vram_flags;
> > > > > -- 
> > > > > 2.43.0
> > > > > 

      reply	other threads:[~2026-02-24 23:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-24  2:33 [PATCH v3] drm/xe: restrict multi-lrc to VCS/VECS engines Xin Wang
2026-02-24  2:40 ` ✓ CI.KUnit: success for drm/xe: restrict multi-lrc to VCS/VECS engines (rev2) Patchwork
2026-02-24 22:05 ` [PATCH v3] drm/xe: restrict multi-lrc to VCS/VECS engines Matt Roper
2026-02-24 22:20   ` Wang, X
2026-02-24 22:40     ` Matthew Brost
2026-02-24 22:57       ` Wang, X
2026-02-24 23:03         ` Matthew Brost [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aZ4uSB27jzyG9c+F@lstrano-desk.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.d.roper@intel.com \
    --cc=shuicheng.lin@intel.com \
    --cc=x.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox