Re: [PATCH v5 3/4] drm/xe/guc: Start use explicitly usable GuC IDs for for submission

["K V P, Satyanarayana" <satyanarayana.k.v.p@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 10747 bytes --]


On 05-Oct-26 2:21 PM, Piórkowski, Piotr wrote:
> From: Piotr Piórkowski<piotr.piorkowski@intel.com>
>
> Switch GuC submission to explicitly allocate IDs from the usable
> ID pool.
> Also lets configure the GuC ID manager according to the device operating
> mode during submission initialization. Use dedicated initialization
> paths for native, PF and VF modes to expose the appropriate usable and
> shareable GuC ID ranges.
>
> v2: Use xe_root_mmio_gt() to get the primary GT so no NULL check is
>      needed (Sashiko).
>
> Assisted-by: Claude:claude-5-sonnet
> Signed-off-by: Piotr Piórkowski<piotr.piorkowski@intel.com>
> Cc: Michal Wajdeczko<michal.wajdeczko@intel.com>
> ---
>   drivers/gpu/drm/xe/tests/xe_guc_id_mgr_test.c | 33 +++++++++++++
>   drivers/gpu/drm/xe/xe_guc.c                   |  4 +-
>   drivers/gpu/drm/xe/xe_guc_id_mgr.c            | 49 ++++++++++++++++---
>   drivers/gpu/drm/xe/xe_guc_id_mgr.h            | 13 ++++-
>   drivers/gpu/drm/xe/xe_guc_submit.c            | 27 +++++-----
>   drivers/gpu/drm/xe/xe_guc_submit.h            |  2 +-
>   6 files changed, 105 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/tests/xe_guc_id_mgr_test.c b/drivers/gpu/drm/xe/tests/xe_guc_id_mgr_test.c
> index 5fe71eac2e380..2e62809b4d4c5 100644
> --- a/drivers/gpu/drm/xe/tests/xe_guc_id_mgr_test.c
> +++ b/drivers/gpu/drm/xe/tests/xe_guc_id_mgr_test.c
> @@ -65,6 +65,23 @@ static void init_fini(struct kunit *test)
>   	KUNIT_EXPECT_EQ(test, idm_total(idm), 0);
>   }
>   
> +static void check_init_small(struct kunit *test)
> +{
> +	struct xe_guc_id_mgr *idm = test->priv;
> +	unsigned int count = 8;
> +
> +	KUNIT_ASSERT_EQ(test, xe_guc_id_mgr_init_small(idm, count), 0);
> +	KUNIT_EXPECT_EQ(test, xe_guc_id_mgr_max_usable(idm), count);
> +	KUNIT_EXPECT_EQ(test, xe_guc_id_mgr_max_shareable(idm), 0);
> +	KUNIT_EXPECT_EQ(test, idm_total(idm), count);
> +
> +	KUNIT_EXPECT_EQ(test, xe_guc_id_mgr_reserve_usable(idm, count), 0);
> +	KUNIT_EXPECT_EQ(test, xe_guc_id_mgr_reserve_usable(idm, 1), -ENOSPC);
> +	KUNIT_EXPECT_EQ(test, xe_guc_id_mgr_release_usable(idm, 0, count), 0);
> +
> +	KUNIT_EXPECT_EQ(test, xe_guc_id_mgr_reserve_shareable(idm, 1, 0), -ENODATA);
> +}
> +
>   static bool require_iov_config_or_skip(struct kunit *test)
>   {
>   	if (!IS_ENABLED(CONFIG_PCI_IOV)) {
> @@ -75,6 +92,20 @@ static bool require_iov_config_or_skip(struct kunit *test)
>   	return true;
>   }
>   
> +static void check_init_shared(struct kunit *test)
> +{
> +	struct xe_guc_id_mgr *idm = test->priv;
> +
> +	if (!require_iov_config_or_skip(test))
> +		return;
> +
> +	KUNIT_ASSERT_TRUE(test, IS_SRIOV_PF(idm_to_xe(idm)));
> +	KUNIT_ASSERT_EQ(test, xe_guc_id_mgr_init_shared(idm), 0);
> +	KUNIT_EXPECT_EQ(test, xe_guc_id_mgr_max_usable(idm), GUC_ID_MAX);
> +	KUNIT_EXPECT_EQ(test, xe_guc_id_mgr_max_shareable(idm), GUC_ID_MAX);
> +	KUNIT_EXPECT_EQ(test, idm_total(idm), GUC_ID_MAX);
> +}
> +
>   static unsigned int test_idm_used_total(struct xe_guc_id_mgr *idm)
>   {
>   	lockdep_assert_held(idm_mutex(idm));
> @@ -671,6 +702,8 @@ static struct kunit_case guc_id_mgr_test_cases[] = {
>   	KUNIT_CASE(bad_init),
>   	KUNIT_CASE(no_init),
>   	KUNIT_CASE(init_fini),
> +	KUNIT_CASE(check_init_small),
> +	KUNIT_CASE(check_init_shared),
>   	KUNIT_CASE(check_used),
>   	KUNIT_CASE(check_quota),
>   	KUNIT_CASE_SLOW(check_all),
> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> index 3ab4cb9e4964c..6c35474ab843a 100644
> --- a/drivers/gpu/drm/xe/xe_guc.c
> +++ b/drivers/gpu/drm/xe/xe_guc.c
> @@ -853,7 +853,7 @@ static int vf_guc_init_post_hwconfig(struct xe_guc *guc)
>   {
>   	int err;
>   
> -	err = xe_guc_submit_init(guc, xe_gt_sriov_vf_guc_ids(guc_to_gt(guc)));
> +	err = xe_guc_submit_init(guc);
>   	if (err)
>   		return err;
>   
> @@ -897,7 +897,7 @@ int xe_guc_init_post_hwconfig(struct xe_guc *guc)
>   
>   	guc_init_params_post_hwconfig(guc);
>   
> -	ret = xe_guc_submit_init(guc, ~0);
> +	ret = xe_guc_submit_init(guc);
>   	if (ret)
>   		return ret;
>   
> diff --git a/drivers/gpu/drm/xe/xe_guc_id_mgr.c b/drivers/gpu/drm/xe/xe_guc_id_mgr.c
> index 8f90a5337fe4c..dc918221b78de 100644
> --- a/drivers/gpu/drm/xe/xe_guc_id_mgr.c
> +++ b/drivers/gpu/drm/xe/xe_guc_id_mgr.c
> @@ -120,23 +120,56 @@ static int idm_init(struct xe_guc_id_mgr *idm, unsigned int usable, unsigned int
>   }
>   
>   /**
> - * xe_guc_id_mgr_init() - Initialize GuC context ID Manager.
> + * xe_guc_id_mgr_init() - Initialize GuC context ID manager in native mode.
>    * @idm: the &xe_guc_id_mgr to initialize
> - * @limit: number of IDs to manage
>    *
> - * The bare-metal or PF driver can pass ~0 as &limit to indicate that all
> - * context IDs supported by the GuC firmware are available for use.
> + * Can only be called when the device is not in SR-IOV mode.
>    *
>    * Return: 0 on success or a negative error code on failure.
>    */
> -int xe_guc_id_mgr_init(struct xe_guc_id_mgr *idm, unsigned int limit)
> +int xe_guc_id_mgr_init(struct xe_guc_id_mgr *idm)
>   {
> -	if (limit == ~0)
> -		limit = GUC_ID_MAX;
> +	xe_gt_assert(idm_to_gt(idm), !IS_SRIOV(idm_to_xe(idm)));
>   
> -	return idm_init(idm, limit, 0);
> +	return idm_init(idm, GUC_ID_MAX, 0);
>   }
>   
> +/**
> + * xe_guc_id_mgr_init_small() - Initialize GuC ID manager for small count.
> + * @idm: the &xe_guc_id_mgr to initialize
> + * @count: the number of usable GuC context IDs
> + *
> + * This function initializes the GuC ID manager to manage a small number of
> + * GuC context IDs (0..count-1) for use by the GuC submission code.
> + *
> + * Return: 0 on success, or a negative error code on failure.
> + */
> +int xe_guc_id_mgr_init_small(struct xe_guc_id_mgr *idm, unsigned int count)
> +{
> +	return idm_init(idm, count, 0);
> +}
> +
> +#ifdef CONFIG_PCI_IOV
> +/**
> + * xe_guc_id_mgr_init_shared() - Initialize GuC ID manager for PF.
> + * @idm: the &xe_guc_id_mgr to initialize
> + *
> + * This function initializes the GuC ID manager to manage the full range of
> + * GuC context IDs (0..GUC_ID_MAX - 1) for use by the GuC submission code.
> + * All GuC context IDs will be shareable with VFs.
> + *
> + * Can only be called when the device is in SR-IOV PF mode.
> + *
> + * Return: 0 on success, or a negative error code on failure.
> + */
> +int xe_guc_id_mgr_init_shared(struct xe_guc_id_mgr *idm)
> +{
> +	xe_gt_assert(idm_to_gt(idm), IS_SRIOV_PF(idm_to_xe(idm)));
> +
> +	return idm_init(idm, GUC_ID_MAX, GUC_ID_MAX);
> +}
> +#endif /* CONFIG_PCI_IOV */
> +
>   /**
>    * xe_guc_id_mgr_max_usable() - Get maximum number of usable GuC context IDs.
>    * @idm: the &xe_guc_id_mgr to query
> diff --git a/drivers/gpu/drm/xe/xe_guc_id_mgr.h b/drivers/gpu/drm/xe/xe_guc_id_mgr.h
> index 209f99831f1b4..ed9970829a939 100644
> --- a/drivers/gpu/drm/xe/xe_guc_id_mgr.h
> +++ b/drivers/gpu/drm/xe/xe_guc_id_mgr.h
> @@ -6,10 +6,21 @@
>   #ifndef _XE_GUC_ID_MGR_H_
>   #define _XE_GUC_ID_MGR_H_
>   
> +#include <linux/errno.h>
> +
>   struct drm_printer;
>   struct xe_guc_id_mgr;
>   
> -int xe_guc_id_mgr_init(struct xe_guc_id_mgr *idm, unsigned int limit);
> +int xe_guc_id_mgr_init(struct xe_guc_id_mgr *idm);
> +int xe_guc_id_mgr_init_small(struct xe_guc_id_mgr *idm, unsigned int count);
> +#ifdef CONFIG_PCI_IOV
> +int xe_guc_id_mgr_init_shared(struct xe_guc_id_mgr *idm);
> +#else
> +static inline int xe_guc_id_mgr_init_shared(struct xe_guc_id_mgr *idm)
> +{
> +	return -ENODEV;
> +}
> +#endif
>   
>   unsigned int xe_guc_id_mgr_max_usable(struct xe_guc_id_mgr *idm);
>   unsigned int xe_guc_id_mgr_max_shareable(struct xe_guc_id_mgr *idm);
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 7ce73489c35fc..cef59d5d82aa5 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -286,20 +286,26 @@ static void primelockdep(struct xe_guc *guc)
>   	fs_reclaim_release(GFP_KERNEL);
>   }
>   
> +static int guc_id_mgr_init(struct xe_guc *guc)
> +{
> +	if (IS_SRIOV_PF(guc_to_xe(guc)))
> +		return xe_guc_id_mgr_init_shared(&guc->submission_state.idm);
> +	else if (IS_SRIOV_VF(guc_to_xe(guc)))
> +		return xe_guc_id_mgr_init_small(&guc->submission_state.idm,
> +						xe_gt_sriov_vf_guc_ids(guc_to_gt(guc)));
> +	else
> +		return xe_guc_id_mgr_init(&guc->submission_state.idm);
> +}
> +
>   /**
>    * xe_guc_submit_init() - Initialize GuC submission.
>    * @guc: the &xe_guc to initialize
> - * @num_ids: number of GuC context IDs to use
> - *
> - * The bare-metal or PF driver can pass ~0 as &num_ids to indicate that all
> - * GuC context IDs supported by the GuC firmware should be used for submission.
>    *
> - * Only VF drivers will have to provide explicit number of GuC context IDs
> - * that they can use for submission.
> + * This function initializes the GuC submission state.
>    *
>    * Return: 0 on success or a negative error code on failure.
>    */
> -int xe_guc_submit_init(struct xe_guc *guc, unsigned int num_ids)
> +int xe_guc_submit_init(struct xe_guc *guc)
>   {
>   	struct xe_device *xe = guc_to_xe(guc);
>   	struct xe_gt *gt = guc_to_gt(guc);
> @@ -309,7 +315,7 @@ int xe_guc_submit_init(struct xe_guc *guc, unsigned int num_ids)
>   	if (err)
>   		return err;
>   
> -	err = xe_guc_id_mgr_init(&guc->submission_state.idm, num_ids);
> +	err = guc_id_mgr_init(guc);
>   	if (err)
>   		return err;
>   
> @@ -415,8 +421,7 @@ static int alloc_guc_id(struct xe_guc *guc, struct xe_exec_queue *q)
>   	int ret, i;
>   
>   	mutex_lock(&guc->submission_state.lock);
> -	ret = xe_guc_id_mgr_reserve_locked(&guc->submission_state.idm,
> -					   q->width);
> +	ret = xe_guc_id_mgr_reserve_usable_locked(&guc->submission_state.idm, q->width);
>   	mutex_unlock(&guc->submission_state.lock);
>   	if (ret < 0)
>   		return ret;
> @@ -3298,7 +3303,7 @@ g2h_exec_queue_lookup(struct xe_guc *guc, u32 guc_id)
>   	struct xe_gt *gt = guc_to_gt(guc);
>   	struct xe_exec_queue *q;
>   
> -	if (unlikely(guc_id >= GUC_ID_MAX)) {
> +	if (unlikely(guc_id >= xe_guc_id_mgr_max_usable(&guc->submission_state.idm))) {
>   		xe_gt_err(gt, "Invalid guc_id %u\n", guc_id);
>   		return NULL;
>   	}
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.h b/drivers/gpu/drm/xe/xe_guc_submit.h
> index 29abf07d8f041..7a81d0aa19bb2 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.h
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.h
> @@ -13,7 +13,7 @@ struct xe_exec_queue;
>   struct xe_guc;
>   struct xe_hw_engine;
>   
> -int xe_guc_submit_init(struct xe_guc *guc, unsigned int num_ids);
> +int xe_guc_submit_init(struct xe_guc *guc);
>   int xe_guc_submit_enable(struct xe_guc *guc);
>   void xe_guc_submit_disable(struct xe_guc *guc);
>   

LGTM.

Reviewed-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>

[-- Attachment #2: Type: text/html, Size: 11193 bytes --]