Re: [PATCH v3 03/12] drm/xe/sriov: Initialize scheduler groups

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 12/11/2025 10:52 AM, Michal Wajdeczko wrote:
>
> On 12/11/2025 2:57 AM, Daniele Ceraolo Spurio wrote:
>> Scheduler groups (a.k.a. Engine Groups Scheduling, or EGS) is a GuC
>> feature that allows the driver to define groups of engines that are
>> independently scheduled across VFs, which allows different VFs to be
>> active on the HW at the same time on different groups. The feature is
>> available for BMG and newer HW starting on GuC 70.53.0, but some
>> required fixes have been added to GuC 70.55.1.
>>
>> This is intended for specific scenarios where the admin knows that the
>> VFs are not going to fully utilize the HW and therefore assigning all of
>> it to a single VF would lead to part of it being permanently idle.
>> We do not allow the admin to decide how to divide the engines across
>> groups, but we instead support specific configurations that are designed
>> for specific use-cases. During PF initialization we detect which
>> configurations are possible on a given GT and create the relevant
>> groups. Since the GuC expect a mask for each class for each group, that
>> is what we save when we init the configs.
>>
>> Right now we only have one use-case on the media GT. If the VFs are
>> running a frame render + encoding at a not-too-high resolution (e.g.
>> 1080@30fps) the render can produce frames faster than the video engine
>> can encode them, which means that the maximum number of parallel VFs is
>> limited by the VCS bandwidth. Since our products can have multiple VCS
>> engines, allowing multiple VFs to be active on the different VCS engines
>> at the same time allows us to run more parallel VFs on the same HW.
>> Given that engines in the same media slice share some resources (e.g.
>> SFC), we assign each media slice to a different scheduling group. We
>> refer to this configuration as "media_slices", given that each slice
>> gets its own group. Since upcoming products have a different number of
>> video engines per-slice, for now we limit the media_slices mode to BMG,
>> but we expect to add support for newer HW soon.
>>
>> Note that while the GuC interface supports a maximum of 8 groups, the
>> actual number of groups that can be enabled can be lower than that and
>> can be different on different devices. For now, all devices support up
>> to 2 groups.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> ---
>> v2: Use asserts for coding errors, code cleanups, better docs (Michal),
>>      limit groups to 2, limit to BMG and newer, bump required GuC to
>>      70.55.1.
>> v3: Use a struct sched_groups array instead of an array of u32 masks,
>>      move the max_group check to the next patch, rename NONE to
>>      DISABLED (Michal), limit the media_slices mode to BMG.
>> ---
>>   drivers/gpu/drm/xe/abi/guc_scheduler_abi.h    |   9 ++
>>   drivers/gpu/drm/xe/xe_gt.h                    |   3 +
>>   drivers/gpu/drm/xe/xe_gt_sriov_pf.c           |   3 +
>>   drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.c    | 142 ++++++++++++++++++
>>   drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.h    |   2 +
>>   .../gpu/drm/xe/xe_gt_sriov_pf_policy_types.h  |  33 ++++
>>   6 files changed, 192 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/abi/guc_scheduler_abi.h b/drivers/gpu/drm/xe/abi/guc_scheduler_abi.h
>> index db9c171f8b64..513b22a87428 100644
>> --- a/drivers/gpu/drm/xe/abi/guc_scheduler_abi.h
>> +++ b/drivers/gpu/drm/xe/abi/guc_scheduler_abi.h
>> @@ -6,6 +6,8 @@
>>   #ifndef _ABI_GUC_SCHEDULER_ABI_H
>>   #define _ABI_GUC_SCHEDULER_ABI_H
>>   
>> +#include <linux/types.h>
>> +
>>   /**
>>    * Generic defines required for registration with and submissions to the GuC
>>    * scheduler. Includes engine class/instance defines and context attributes
>> @@ -45,4 +47,11 @@
>>   #define GUC_CONTEXT_DISABLE		0
>>   #define GUC_CONTEXT_ENABLE		1
>>   
>> +/* scheduler groups */
>> +#define GUC_MAX_SCHED_GROUPS		8
>> +
>> +struct guc_sched_group {
>> +	u32 engines[GUC_MAX_ENGINE_CLASSES];
>> +} __packed;
>> +
>>   #endif
>> diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
>> index a2ba80c954a6..de7e47763411 100644
>> --- a/drivers/gpu/drm/xe/xe_gt.h
>> +++ b/drivers/gpu/drm/xe/xe_gt.h
>> @@ -29,6 +29,9 @@
>>   #define CCS_INSTANCES(gt) XE_ENGINE_INSTANCES_FROM_MASK(gt, CCS)
>>   #define GSCCS_INSTANCES(gt) XE_ENGINE_INSTANCES_FROM_MASK(gt, GSCCS)
>>   
>> +/* Our devices have up to 4 media slices */
>> +#define MAX_MEDIA_SLICES 4
>> +
>>   #define GT_VER(gt) ({ \
>>   	typeof(gt) gt_ = (gt); \
>>   	struct xe_device *xe = gt_to_xe(gt_); \
>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
>> index 0714c758b9c1..0d97a823e702 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
>> @@ -14,6 +14,7 @@
>>   #include "xe_gt_sriov_pf_control.h"
>>   #include "xe_gt_sriov_pf_helpers.h"
>>   #include "xe_gt_sriov_pf_migration.h"
>> +#include "xe_gt_sriov_pf_policy.h"
>>   #include "xe_gt_sriov_pf_service.h"
>>   #include "xe_gt_sriov_printk.h"
>>   #include "xe_guc_submit.h"
>> @@ -123,6 +124,8 @@ int xe_gt_sriov_pf_init(struct xe_gt *gt)
>>   	if (err)
>>   		return err;
>>   
>> +	xe_gt_sriov_pf_policy_init(gt);
>> +
>>   	err = xe_gt_sriov_pf_migration_init(gt);
>>   	if (err)
>>   		return err;
>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.c
>> index 4445f660e6d1..003860661687 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.c
>> @@ -3,6 +3,8 @@
>>    * Copyright © 2023-2024 Intel Corporation
>>    */
>>   
>> +#include <drm/drm_managed.h>
>> +
>>   #include "abi/guc_actions_sriov_abi.h"
>>   
>>   #include "xe_bo.h"
>> @@ -10,6 +12,7 @@
>>   #include "xe_gt_sriov_pf_helpers.h"
>>   #include "xe_gt_sriov_pf_policy.h"
>>   #include "xe_gt_sriov_printk.h"
>> +#include "xe_guc.h"
>>   #include "xe_guc_buf.h"
>>   #include "xe_guc_ct.h"
>>   #include "xe_guc_klv_helpers.h"
>> @@ -351,6 +354,133 @@ u32 xe_gt_sriov_pf_policy_get_sample_period(struct xe_gt *gt)
>>   	return value;
>>   }
>>   
>> +static void pf_sched_group_media_slices(struct xe_gt *gt, struct guc_sched_group **groups,
>> +					u32 *num_groups)
>> +{
>> +	u8 slice_to_group[MAX_MEDIA_SLICES];
>> +	u32 vecs_mask = VECS_INSTANCES(gt);
>> +	u32 gsc_mask = GSCCS_INSTANCES(gt);
>> +	u32 vcs_mask = VCS_INSTANCES(gt);
>> +	struct guc_sched_group *values;
>> +	struct xe_hw_engine *hwe;
>> +	enum xe_hw_engine_id id;
>> +	int group = 0;
>> +	int slice;
>> +
>> +	xe_gt_assert(gt, xe_gt_is_media_type(gt));
>> +
>> +	/*
>> +	 * Post-BMG the matching of video engines to slices changes, so for now
>> +	 * we don't allow this mode on those platforms.
>> +	 */
>> +	if (gt_to_xe(gt)->info.platform > XE_BATTLEMAGE)
>> +		return;
>> +
>> +	/*
>> +	 * On BMG and older platforms a media slice has 2 VCS and a VECS. We
>> +	 * bundle the GSC with the first slice.
>> +	 */
>> +	for (slice = 0; slice < MAX_MEDIA_SLICES; slice++) {
>> +		if ((vcs_mask & 0x3) || (vecs_mask & 0x1) || (gsc_mask & 0x1))
>> +			slice_to_group[slice] = group++;
>> +
>> +		vcs_mask >>= 2;
>> +		vecs_mask >>= 1;
>> +		gsc_mask >>= 1;
>> +	}
>> +
>> +	xe_gt_assert(gt, !vcs_mask);
>> +	xe_gt_assert(gt, !vecs_mask);
>> +	xe_gt_assert(gt, !gsc_mask);
>> +
>> +	/* We need at least 2 slices to split them up */
>> +	if (group < 2)
>> +		return;
>> +
>> +	/* The GuC expects an array with a guc_sched_group entry for each group */
>> +	values = drmm_kcalloc(&gt_to_xe(gt)->drm, group, sizeof(struct guc_sched_group),
>> +			      GFP_KERNEL);
>> +	if (!values)
>> +		return;
>> +
>> +	for_each_hw_engine(hwe, gt, id) {
>> +		u8 guc_class = xe_engine_class_to_guc_class(hwe->class);
>> +
>> +		switch (hwe->class) {
>> +		case XE_ENGINE_CLASS_VIDEO_DECODE:
>> +			slice = hwe->instance / 2;
>> +			break;
>> +		case XE_ENGINE_CLASS_VIDEO_ENHANCE:
>> +			slice = hwe->instance;
>> +			break;
>> +		case XE_ENGINE_CLASS_OTHER:
>> +			slice = 0;
>> +			break;
>> +		default:
>> +			xe_gt_assert_msg(gt, false,
>> +					 "unknown media gt class %u (%s) during EGS setup\n",
>> +					 hwe->class, hwe->name);
>> +			slice = 0;
>> +		}
>> +
>> +		values[slice_to_group[slice]].engines[guc_class] |= BIT(hwe->logical_instance);
>> +	}
>> +
>> +	*groups = values;
>> +	*num_groups = group;
>> +}
>> +
>> +/**
>> + * xe_sriov_gt_pf_policy_has_sched_groups_support() - Checks whether scheduler
>> + * groups are supported.
>> + * @gt: the &xe_gt
>> + *
>> + * This function can only be called on PF.
>> + *
>> + * Return: true if scheduler groups are supported, false otherwise.
>> + */
>> +bool xe_sriov_gt_pf_policy_has_sched_groups_support(struct xe_gt *gt)
>> +{
>> +	xe_gt_assert(gt, IS_SRIOV_PF(gt_to_xe(gt)));
>> +
>> +	/*
>> +	 * The GuC supports scheduler groups from v70.53.0, but a fix for it has
>> +	 * been merged in v70.55.1, so we require the latter. The feature is
>> +	 * also only enabled on BMG and newer FW.
>> +	 */
>> +	return GUC_FIRMWARE_VER(&gt->uc.guc) >= MAKE_GUC_VER(70, 55, 1) &&
>> +	       gt_to_xe(gt)->info.platform >= XE_BATTLEMAGE;
>> +}
>> +
>> +static void pf_init_sched_groups(struct xe_gt *gt)
>> +{
>> +	int m;
>> +
>> +	if (!xe_sriov_gt_pf_policy_has_sched_groups_support(gt))
>> +		return;
>> +
>> +	for (m = 0; m < XE_SRIOV_SCHED_GROUPS_MODES_COUNT; m++) {
>> +		u32 *num_groups = &gt->sriov.pf.policy.guc.sched_groups.modes[m].num_groups;
>> +		struct guc_sched_group **groups =
>> +			&gt->sriov.pf.policy.guc.sched_groups.modes[m].groups;
>> +
>> +		switch (m) {
>> +		case XE_SRIOV_SCHED_GROUPS_DISABLED:
>> +			break;
> nit: since we know that for DISABLED we have nothing to do in this loop,
> so maybe we should not start with it?
>
> 	for (m = XE_SRIOV_SCHED_GROUPS_DISABLED + 1, ...
> 	for (m = XE_SRIOV_SCHED_GROUPS_DISABLED; ++m < ...
> 	
>
>> +		case XE_SRIOV_SCHED_GROUPS_MEDIA_SLICES:
>> +			/* this mode only has groups on the media GT */
>> +			if (xe_gt_is_media_type(gt))
>> +				pf_sched_group_media_slices(gt, groups, num_groups);
>> +			break;
>> +		default:
> nit: since XE_SRIOV_SCHED_GROUPS are defined as enum, then if we declare 'm' as enum,
> instead of this 'default' case we could just have dummy case for COUNT
> and compiler will help us catch any new missed GROUP mode
>
>> +			xe_gt_assert_msg(gt, false, "unknown sched group mode %u\n", m);
>> +			return;
> hmm, IIRC those modes are supposed to be per device, not per-GT,
> so there is a high chance that in this loop we will have to handle non-media modes,
> so maybe this assert and early return is too much?

This code is not media-specific. If we have a non media mode we'll just 
have to do:

case XE_SRIOV_SCHED_GROUPS_NEW_MODE
     if(!xe_gt_is_media_type(gt))
         fill_masks_for_new_mode(gt, groups, num_groups);
     break;

And that'll work. The return is only in the case where we have a 
programming error and the mode has not correctly been added to the 
switch. I can still drop the return and leave just the assert, or move 
as you said to use the enum for m so that the compiler will catch the 
missing case.

>
>> +		}
>> +
>> +		xe_gt_assert(gt, *num_groups < GUC_MAX_SCHED_GROUPS);
>> +	}
>> +}
>> +
>>   static void pf_sanitize_guc_policies(struct xe_gt *gt)
>>   {
>>   	pf_sanitize_sched_if_idle(gt);
>> @@ -401,6 +531,18 @@ int xe_gt_sriov_pf_policy_reprovision(struct xe_gt *gt, bool reset)
>>   	return err ? -ENXIO : 0;
>>   }
>>   
>> +/**
>> + * xe_gt_sriov_pf_policy_init() - Initializes the SW state of the PF policies.
>> + * @gt: the &xe_gt
>> + *
>> + * This function can only be called on PF. This function does not touch the HW,
>> + * but must be called after the engines have been initialized.
>> + */
>> +void xe_gt_sriov_pf_policy_init(struct xe_gt *gt)
>> +{
>> +	pf_init_sched_groups(gt);
>> +}
>> +
>>   static void print_guc_policies(struct drm_printer *p, struct xe_gt_sriov_guc_policies *policy)
>>   {
>>   	drm_printf(p, "%s:\t%s\n",
>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.h b/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.h
>> index 2a5dc33dc6d7..f5e3b2595063 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.h
>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.h
>> @@ -17,7 +17,9 @@ int xe_gt_sriov_pf_policy_set_reset_engine(struct xe_gt *gt, bool enable);
>>   bool xe_gt_sriov_pf_policy_get_reset_engine(struct xe_gt *gt);
>>   int xe_gt_sriov_pf_policy_set_sample_period(struct xe_gt *gt, u32 value);
>>   u32 xe_gt_sriov_pf_policy_get_sample_period(struct xe_gt *gt);
>> +bool xe_sriov_gt_pf_policy_has_sched_groups_support(struct xe_gt *gt);
>>   
>> +void xe_gt_sriov_pf_policy_init(struct xe_gt *gt);
>>   void xe_gt_sriov_pf_policy_sanitize(struct xe_gt *gt);
>>   int xe_gt_sriov_pf_policy_reprovision(struct xe_gt *gt, bool reset);
>>   int xe_gt_sriov_pf_policy_print(struct xe_gt *gt, struct drm_printer *p);
>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy_types.h b/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy_types.h
>> index 4de532af135e..d228cadcd8b0 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy_types.h
>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy_types.h
>> @@ -8,16 +8,49 @@
>>   
>>   #include <linux/types.h>
>>   
>> +#include "abi/guc_scheduler_abi.h"
>> +
>> +/**
>> + * enum xe_sriov_sched_group_modes - list of possible scheduler group modes
>> + * @XE_SRIOV_SCHED_GROUPS_DISABLED - no separate groups (i.e., all engines in group 0)
>> + * @XE_SRIOV_SCHED_GROUPS_MEDIA_SLICES - separate groups for each media slice
>> + * @XE_SRIOV_SCHED_GROUPS_MODES_COUNT - number of valid modes
>> + */
>> +enum xe_sriov_sched_group_modes {
>> +	XE_SRIOV_SCHED_GROUPS_DISABLED = 0,
>> +	XE_SRIOV_SCHED_GROUPS_MEDIA_SLICES,
>> +	XE_SRIOV_SCHED_GROUPS_MODES_COUNT
> nit: maybe to emphasize that COUNT enumerator is not a real group mode, prefix it with underscore ?
>
> 	__XE_SRIOV_SCHED_GROUPS_MODES_COUNT /* must be last */

I can add the comment, but I'd prefer not to add the underscore at the 
front, since it is pretty clear that this is not a real mode anyway.

Daniele

>
>> +};
>> +
>> +/**
>> + * struct xe_gt_sriov_scheduler_groups - Scheduler groups policy info
>> + * @modes: array of masks and their number for each mode
>> + * @modes.groups: array of engine instance groups in given mode, with each group
>> + *                consisting of GUC_MAX_ENGINE_CLASSES engine instances masks. A
>> + *                A NULL value indicates that all the engines are in the same
>> + *                group for this mode on this GT.
>> + * @modes.num_groups: number of groups in given mode, zero if all the engines
>> + *                    are in the same group.
>> + */
>> +struct xe_gt_sriov_scheduler_groups {
>> +	struct {
>> +		struct guc_sched_group *groups;
>> +		u32 num_groups;
>> +	} modes[XE_SRIOV_SCHED_GROUPS_MODES_COUNT];
>> +};
>> +
>>   /**
>>    * struct xe_gt_sriov_guc_policies - GuC SR-IOV policies.
>>    * @sched_if_idle: controls strict scheduling policy.
>>    * @reset_engine: controls engines reset on VF switch policy.
>>    * @sample_period: adverse events sampling period (in milliseconds).
>> + * @sched_groups: available scheduling group configurations.
>>    */
>>   struct xe_gt_sriov_guc_policies {
>>   	bool sched_if_idle;
>>   	bool reset_engine;
>>   	u32 sample_period;
>> +	struct xe_gt_sriov_scheduler_groups sched_groups;
>>   };
>>   
>>   /**
> mostly nits, so
>
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>
>