Re: [PATCH 4/9] drm/i915/casf: Extract scaler_has_casf()

["Michał Grzelak" <michal.grzelak@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3943 bytes --]

On Thu, 26 Mar 2026, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Extract a small helper to determine if the scaler supports
> the sharpness filter or not.
>
> Cc: Nemesa Garg <nemesa.garg@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/skl_scaler.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
> index e9fe5c0bf6ff..525afd736195 100644
> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
> @@ -323,19 +323,24 @@ int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
> 				 need_scaler);
> }
>
> +static bool scaler_has_casf(struct intel_display *display, int scaler_id)
> +{
> +	return HAS_CASF(display) && scaler_id == 1;
> +}
> +
> static int intel_allocate_scaler(struct intel_crtc_scaler_state *scaler_state,
> 				 struct intel_crtc *crtc,
> 				 struct intel_plane_state *plane_state,
> 				 bool casf_scaler)
> {
> +	struct intel_display *display = to_intel_display(crtc);
> 	int i;
>
> 	for (i = 0; i < crtc->num_scalers; i++) {
> 		if (scaler_state->scalers[i].in_use)
> 			continue;
>
> -		/* CASF needs second scaler */
> -		if (!plane_state && casf_scaler && i != 1)
> +		if (casf_scaler && !scaler_has_casf(display, i))
> 			continue;
>
> 		scaler_state->scalers[i].in_use = true;
> @@ -982,8 +987,7 @@ void skl_scaler_get_config(struct intel_crtc_state *crtc_state)
>
> 		id = i;
>
> -		/* Read CASF regs for second scaler */
> -		if (HAS_CASF(display) && id == 1)
> +		if (scaler_has_casf(display, i))

With that being changed I am wondering if we need int id at all. The
only user of it is outside the loop. Since id is set to i on every
loop's pass, I guess we can replace it with last value of i. And if
crtc->num_scalers == -1, we wouldn't enter the loop anyway, so the id
is still set to -1. The only scenario I see where it can break is e.g.
when crtc->num_scalers == -2, but I have no clue if it is even possible.

To be precise, I am wondering about such change (diff without your change
being applied):

diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index 4c4deac7f9c8..78852267e60b 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -969,7 +969,6 @@ void skl_scaler_get_config(struct intel_crtc_state *crtc_state)
         struct intel_display *display = to_intel_display(crtc_state);
         struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
         struct intel_crtc_scaler_state *scaler_state = &crtc_state->scaler_state;
-       int id = -1;
         int i;

         /* find scaler attached to this pipe */
@@ -980,8 +979,6 @@ void skl_scaler_get_config(struct intel_crtc_state *crtc_state)
                 if ((ctl & (PS_SCALER_EN | PS_BINDING_MASK)) != (PS_SCALER_EN | PS_BINDING_PIPE))
                         continue;

-               id = i;
-
                 /* Read CASF regs for second scaler */
                 if (HAS_CASF(display) && id == 1)
                         intel_casf_sharpness_get_config(crtc_state);
@@ -1003,8 +1000,8 @@ void skl_scaler_get_config(struct intel_crtc_state *crtc_state)
                 break;
         }

-       scaler_state->scaler_id = id;
-       if (id >= 0)
+       scaler_state->scaler_id = crtc->num_scalers;
+       if (scaler_state->scaler_id >= 0)
                 scaler_state->scaler_users |= (1 << SKL_CRTC_INDEX);
         else
                 scaler_state->scaler_users &= ~(1 << SKL_CRTC_INDEX);

But I don't know if the reasoning above makes any sense.

Reviewed-by: Michał Grzelak <michal.grzelak@intel.com>

BR,
Michał

> 			intel_casf_sharpness_get_config(crtc_state);
>
> 		if (!crtc_state->pch_pfit.casf.enable)
> -- 
> 2.52.0
>
>