[PATCH] drm/xe/pf: Skip LMTT update if no LMEM was provisioned

[Cavitt, Jonathan]

-----Original Message-----
From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Michal Wajdeczko
Sent: Thursday, July 31, 2025 12:47 PM
To: intel-xe at lists.freedesktop.org
Cc: Wajdeczko, Michal <Michal.Wajdeczko at intel.com>
Subject: [PATCH] drm/xe/pf: Skip LMTT update if no LMEM was provisioned
> 
> During VF unprovisioning, if VF was not provisioned with LMEM,
> there is no need to trigger LMTT update, as VF LMTT was never
> set. This will spare us sending full TLB invalidation requests.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>

Some nits below, but I'm not going to make any of the suggestions a requirement,
so:
Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>

> ---
>  drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 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 f00cce81f1ff..692b174dd5b8 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> @@ -1434,8 +1434,10 @@ static int pf_update_vf_lmtt(struct xe_device *xe, unsigned int vfid)
>  	return err;
>  }
>  
> -static void pf_release_vf_config_lmem(struct xe_gt *gt, struct xe_gt_sriov_config *config)
> +static bool pf_release_vf_config_lmem(struct xe_gt *gt, struct xe_gt_sriov_config *config)
>  {
> +	bool released = false;
> +
>  	xe_gt_assert(gt, IS_DGFX(gt_to_xe(gt)));
>  	xe_gt_assert(gt, xe_gt_is_main_type(gt));
>  	lockdep_assert_held(xe_gt_sriov_pf_master_mutex(gt));
> @@ -1443,7 +1445,9 @@ static void pf_release_vf_config_lmem(struct xe_gt *gt, struct xe_gt_sriov_confi
>  	if (config->lmem_obj) {
>  		xe_bo_unpin_map_no_vm(config->lmem_obj);
>  		config->lmem_obj = NULL;
> +		released = true;
>  	}
> +	return released;

NIT:
Storing the return value of this function doesn't seem immediately necessary,
as we could probably just return the value directly as a part of this branch:

"""
	if (config->lmem_obj) {
		xe_bo_unpin_map_no_vm(config->lmem_obj);
		config->lmem_obj = NULL;
		return true;
	}
	return false;
"""

But I had a similar thought in a prior patch of yours, and I agree with
the sentiment that it may be useful in the future if we ever need to use
the return value for anything else.  It also doesn't hurt anything to keep,
so I won't make it a requirement.

>  }
>  
>  static int pf_provision_vf_lmem(struct xe_gt *gt, unsigned int vfid, u64 size)
> @@ -2021,12 +2025,13 @@ static void pf_release_vf_config(struct xe_gt *gt, unsigned int vfid)
>  {
>  	struct xe_gt_sriov_config *config = pf_pick_vf_config(gt, vfid);
>  	struct xe_device *xe = gt_to_xe(gt);
> +	bool released;
>  
>  	if (xe_gt_is_main_type(gt)) {
>  		pf_release_vf_config_ggtt(gt, config);
>  		if (IS_DGFX(xe)) {
> -			pf_release_vf_config_lmem(gt, config);
> -			if (xe_device_has_lmtt(xe))
> +			released = pf_release_vf_config_lmem(gt, config);
> +			if (released && xe_device_has_lmtt(xe))

NIT:
Same as above: storing the value of pf_release_vf_config_lmem isn't
immediately necessary, and we could just pass the result directly into
the if-statement:

"""
	if (pf_release_vf_config_lmem(gt, config) &&
	    xe_device_has_lmtt(xe))
"""

But, again, it doesn't hurt to have, and it'll be useful if we ever expand this
function in the future and need that value, so there's no required changes
here.

-Jonathan Cavitt


>  				pf_update_vf_lmtt(xe, vfid);
>  		}
>  	}
> -- 
> 2.47.1
> 
>