Re: [PATCH 1/2] drm/xe/pf: Limit fair VF LMEM provisioning

["Piotr Piórkowski" <piotr.piorkowski@intel.com>]

Michal Wajdeczko <michal.wajdeczko@intel.com> wrote on czw [2024-lip-11 22:21:07 +0200]:
> 
> 
> On 11.07.2024 22:02, Cavitt, Jonathan wrote:
> > -----Original Message-----
> > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Michal Wajdeczko
> > Sent: Thursday, July 11, 2024 12:23 PM
> > To: intel-xe@lists.freedesktop.org
> > Cc: Wajdeczko, Michal <Michal.Wajdeczko@intel.com>
> > Subject: [PATCH 1/2] drm/xe/pf: Limit fair VF LMEM provisioning
> >>
> >> Due to the current design of the BO and VRAM manager, any object
> >> with XE_BO_FLAG_PINNED flag, which the PF driver uses during VF
> >> LMEM provisionining, is created with the TTM_PL_FLAG_CONTIGUOUS
> >> flag, which may cause VRAM fragmentation that prevents subsequent
> >> allocations of larger objects, like fair VF LMEM provisioning.
> >>
> >> To avoid such failures, round down fair VF LMEM provisioning size
> >> to next power of two size, to compensate what xe_ttm_vram_mgr is
> >> doing to achieve contiguous allocations.
> >>
> >> Fixes: ac6598aed1b3 ("drm/xe/pf: Add support to configure SR-IOV VFs")
> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> >> ---
> >>  drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> 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 db6c213da847..4699b7836001 100644
> >> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> >> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> >> @@ -1543,6 +1543,7 @@ static u64 pf_estimate_fair_lmem(struct xe_gt *gt, unsigned int num_vfs)
> >>  	u64 fair;
> >>  
> >>  	fair = div_u64(available, num_vfs);
> >> +	fair = rounddown_pow_of_two(fair);	/* XXX: ttm_vram_mgr & drm_buddy limitation */
> >>  	fair = ALIGN_DOWN(fair, alignment);
> > 
> > If the alignment is also a power of two, then ALIGN_DOWN here probably
> > isn't necessary.  But I won't block on it because I'm probably just
> > misunderstanding something.
> 
> true, it's redundant, but I didn't want to remove it since this new
> rounddown_pow_of_two() is (I hope) temporary and I don't want to
> accidentally miss mandatory HW requirement
> 
> > 
> > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > -Jonathan Cavitt
> 
> thanks!
> 

LGTM: Reviewed-by: Piotr Piórkowski <piotr.piorkowski@intel.com>

> > 
> >>  #ifdef MAX_FAIR_LMEM
> >>  	fair = min_t(u64, MAX_FAIR_LMEM, fair);
> >> -- 
> >> 2.43.0
> >>
> >>

--