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

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 08.03.2024 08:03, Tejas Upadhyay wrote:
> 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

> 
> 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));
>  
>  	/*