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

[Andi Shyti <andi.shyti@linux.intel.com>]

> > > 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

> 
> Thanks,
> Tejas
> > 
> > >
> > > Fixes: 0d97ecce16bd ("drm/xe: Enable Fixed CCS mode setting")
> > > Signed-off-by: Tejas Upadhyay <tejas.upadhyay@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));
> > >
> > >  	/*