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

> 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]>
> Cc: Satyanarayana K V P<[email protected]>
> ---
> v2: fix kernel-doc (Satya)
> ---
>   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..be0a413ee17c 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 doorbells.
>    * @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 doorbells. + * @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);

LGTM.

Reviewed-by: Satyanarayana K V P <[email protected]>