Re: [PATCH 3/3] drm/xe/pf: Add _locked variant of the doorbells bulk config function

On 05-Aug-26 9:17 PM, Michal Wajdeczko wrote:
> We will soon want to provision all VFs, including their GuC doorbell
> IDs config, in one step. Split existing doorbells bulk configuration
> function into two functions, one of which will expect the master lock
> to be already taken. Use this new function in the fair GuC doorbells
> provisioning.
>
> Signed-off-by: Michal Wajdeczko<[email protected]>
> ---
>   drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c | 37 ++++++++++++++++------
>   drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h |  2 ++
>   2 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> index 4976d8c018ce..b3145444b3d5 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> @@ -1397,7 +1397,7 @@ int xe_gt_sriov_pf_config_set_dbs(struct xe_gt *gt, unsigned int vfid, u32 num_d
>   }
>   
>   /**
> - * xe_gt_sriov_pf_config_bulk_set_dbs - Provision many VFs with GuC context IDs.
> + * xe_gt_sriov_pf_config_bulk_set_dbs_locked() - Provision many VFs with GuC context IDs.
nit: Should be "Provision many VFs with GuC door bells."
>    * @gt: the &xe_gt
>    * @vfid: starting VF identifier (can't be 0)
>    * @num_vfs: number of VFs to provision
> @@ -1407,30 +1407,48 @@ int xe_gt_sriov_pf_config_set_dbs(struct xe_gt *gt, unsigned int vfid, u32 num_d
>    *
>    * Return: 0 on success or a negative error code on failure.
>    */
> -int xe_gt_sriov_pf_config_bulk_set_dbs(struct xe_gt *gt, unsigned int vfid,
> -				       unsigned int num_vfs, u32 num_dbs)
> +int xe_gt_sriov_pf_config_bulk_set_dbs_locked(struct xe_gt *gt, unsigned int vfid,
> +					      unsigned int num_vfs, u32 num_dbs)
>   {
>   	unsigned int n;
>   	int err = 0;
>   
>   	xe_gt_assert(gt, vfid);
> +	lockdep_assert_held(xe_gt_sriov_pf_master_mutex(gt));
>   
>   	if (!num_vfs)
>   		return 0;
>   
> -	mutex_lock(xe_gt_sriov_pf_master_mutex(gt));
>   	for (n = vfid; n < vfid + num_vfs; n++) {
>   		err = pf_provision_vf_dbs(gt, n, num_dbs);
>   		if (err)
>   			break;
>   	}
> -	mutex_unlock(xe_gt_sriov_pf_master_mutex(gt));
>   
>   	return pf_config_bulk_set_u32_done(gt, vfid, num_vfs, num_dbs,
> -					   xe_gt_sriov_pf_config_get_dbs,
> +					   pf_get_vf_config_dbs,
>   					   "GuC doorbell IDs", no_unit, n, err);
>   }
>   
> +/**
> + * xe_gt_sriov_pf_config_bulk_set_dbs() - Provision many VFs with GuC context IDs.

nit: Should be Guc dbs instead of context IDs.

Otherwise looks good to me.

- Satya.

> + * @gt: the &xe_gt
> + * @vfid: starting VF identifier (can't be 0)
> + * @num_vfs: number of VFs to provision
> + * @num_dbs: requested number of GuC doorbell IDs (0 to release)
> + *
> + * This function can only be called on PF.
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int xe_gt_sriov_pf_config_bulk_set_dbs(struct xe_gt *gt, unsigned int vfid,
> +				       unsigned int num_vfs, u32 num_dbs)
> +{
> +	guard(mutex)(xe_gt_sriov_pf_master_mutex(gt));
> +
> +	return xe_gt_sriov_pf_config_bulk_set_dbs_locked(gt, vfid, num_vfs, num_dbs);
> +}
> +
>   static u32 pf_profile_fair_dbs(struct xe_gt *gt, unsigned int num_vfs)
>   {
>   	bool admin_only_pf = xe_sriov_pf_admin_only(gt_to_xe(gt));
> @@ -1480,10 +1498,9 @@ int xe_gt_sriov_pf_config_set_fair_dbs(struct xe_gt *gt, unsigned int vfid,
>   	xe_gt_assert(gt, vfid);
>   	xe_gt_assert(gt, num_vfs);
>   
> -	mutex_lock(xe_gt_sriov_pf_master_mutex(gt));
> +	guard(mutex)(xe_gt_sriov_pf_master_mutex(gt));
> +
>   	fair = pf_estimate_fair_dbs(gt, num_vfs);
> -	mutex_unlock(xe_gt_sriov_pf_master_mutex(gt));
> -
>   	if (!fair)
>   		return -ENOSPC;
>   
> @@ -1492,7 +1509,7 @@ int xe_gt_sriov_pf_config_set_fair_dbs(struct xe_gt *gt, unsigned int vfid,
>   		xe_gt_sriov_info(gt, "Using non-profile provisioning (%s %u vs %u)\n",
>   				 "GuC doorbell IDs", fair, profile);
>   
> -	return xe_gt_sriov_pf_config_bulk_set_dbs(gt, vfid, num_vfs, fair);
> +	return xe_gt_sriov_pf_config_bulk_set_dbs_locked(gt, vfid, num_vfs, fair);
>   }
>   
>   static u64 pf_get_lmem_alignment(struct xe_gt *gt)
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h
> index a14ca7bf7e4d..a56e63f3660a 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h
> @@ -34,6 +34,8 @@ int xe_gt_sriov_pf_config_set_dbs(struct xe_gt *gt, unsigned int vfid, u32 num_d
>   int xe_gt_sriov_pf_config_set_fair_dbs(struct xe_gt *gt, unsigned int vfid, unsigned int num_vfs);
>   int xe_gt_sriov_pf_config_bulk_set_dbs(struct xe_gt *gt, unsigned int vfid, unsigned int num_vfs,
>   				       u32 num_dbs);
> +int xe_gt_sriov_pf_config_bulk_set_dbs_locked(struct xe_gt *gt, unsigned int vfid,
> +					      unsigned int num_vfs, u32 num_dbs);
>   
>   u64 xe_gt_sriov_pf_config_get_lmem(struct xe_gt *gt, unsigned int vfid);
>   int xe_gt_sriov_pf_config_set_lmem(struct xe_gt *gt, unsigned int vfid, u64 size);