[PATCH] drm/xe/gt: Remove redundant num_engine check

Upadhyay, Tejas tejas.upadhyay at intel.com
Fri Mar 8 14:00:31 UTC 2024



> -----Original Message-----
> From: Andi Shyti <andi.shyti at linux.intel.com>
> Sent: Friday, March 8, 2024 5:03 PM
> To: Upadhyay, Tejas <tejas.upadhyay at intel.com>
> Cc: Wajdeczko, Michal <Michal.Wajdeczko at intel.com>; intel-
> xe at lists.freedesktop.org; Andi Shyti <andi.shyti at linux.intel.com>;
> Vishwanathapura, Niranjana <niranjana.vishwanathapura at intel.com>
> Subject: Re: [PATCH] drm/xe/gt: Remove redundant num_engine check
> 
> > > > In __xe_gt_apply_ccs_mode num_engines will never be 0 as it is
> > > > already checked by parent function.
> > >
> > > and that's why we are using xe_assert() instead of explicit checks,
> > > so you can't say it's redundant
> > >
> > > note that the purpose of xe_assert() inside one function is to
> > > enforce the SLA for the caller who should be responsible for doing
> > > actual runtime checks
> >
> > The static analysis tool Coverity is also complaining about checking obvious
> condition on num_engines to be 0. So decided to remove this.
> 
> yesm because it was checked before, as well.
> 
> BTW, please do not consider static analysis failures as bugs to be fixed with
> the Fixes tag.
> 
> Unless they really are bugs, of course. Coverity is also able to detect memory
> leaks and buffer overflows that need to be fixed with massive Fixes tag :-)
> 
> Andi

Yes using all possible time to clean up Coverity issues along with other priorities. Things like this may not be bug but contribute in consuming some CPU cycles perhaps. Will take up all that comes in list.

Thanks,
Tejas
> 
> >
> > Thanks,
> > Tejas
> > >
> > > >
> > > > Fixes: 0d97ecce16bd ("drm/xe: Enable Fixed CCS mode setting")
> > > > Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/xe/xe_gt_ccs_mode.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
> > > > b/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
> > > > index 529fc286cd06..2bbd7b203cc2 100644
> > > > --- a/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
> > > > +++ b/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
> > > > @@ -22,7 +22,7 @@ static void __xe_gt_apply_ccs_mode(struct xe_gt
> > > > *gt,
> > > > u32 num_engines)
> > > >
> > > >  	xe_assert(xe, xe_gt_ccs_mode_enabled(gt));
> > > >
> > > > -	xe_assert(xe, num_engines && num_engines <= num_slices);
> > >
> > > btw, instead of dropping num_engines condition completely this
> > > should be rather split into two separate xe_asserts():
> > >
> > > 	xe_assert(xe, num_engines);
> > > 	xe_assert(xe, num_engines <= num_slices);
> > >
> > > > +	xe_assert(xe, num_engines <= num_slices);
> > > >  	xe_assert(xe, !(num_slices % num_engines));
> > > >
> > > >  	/*


More information about the Intel-xe mailing list