Re: [PATCH v1 4/9] drm/i915/scaler: move independent CASF check

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 10 Jun 2026, Michał Grzelak <michal.grzelak@intel.com> wrote:
> First clause of CASF check is independent of any loop iteration and
> causes intel_allocate_scaler() to fail when satisfied. Check it before
> entering the loop.
>
> Cc: Nemesa Garg <nemesa.garg@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Michał Grzelak <michal.grzelak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/skl_scaler.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
> index f2216a9ea3f8..59a5a3cd3e6a 100644
> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
> @@ -339,12 +339,14 @@ static int intel_allocate_scaler(struct intel_crtc_scaler_state *scaler_state,
>  	struct intel_display *display = to_intel_display(crtc);
>  	int scaler_id;
>  
> +	if (casf_scaler && !HAS_CASF(display))
> +		return -1;

Using -1 as magic error return value is one of my pet peeves.

-1 == -EPERM, and that's hardly the case here.

I know the function returns -1 elsewhere, both should be fixed. -EINVAL
is better.


BR,
Jani.

> +
>  	for (scaler_id = 0; scaler_id < crtc->num_scalers; scaler_id++) {
>  		if (scaler_state->scalers[scaler_id].in_use)
>  			continue;
>  
> -		if ((casf_scaler && !HAS_CASF(display)) ||
> -		    (casf_scaler && scaler_id != 1))
> +		if (casf_scaler && scaler_id != 1)
>  			continue;
>  
>  		scaler_state->scalers[scaler_id].in_use = true;

-- 
Jani Nikula, Intel