Re: [PATCH v1 06/10] drm/i915/casf: remove id in favor of scaler_id

Michał Grzelak
2026-04-11T20:04:14Z

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

On Fri, 10 Apr 2026, Ville Syrjälä wrote:
> On Sat, Mar 28, 2026 at 03:32:08PM +0100, Michał Grzelak wrote:
>> id is not really used anywhere in skl_scaler_get_config(). Replace it
>> with scaler_id.
>>
>> Cc: Nemesa Garg <nemesa.garg@intel.com>
>> Suggested-by: 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 | 9 +++------
>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
>> index 729ef51beeb4..5954f5beb19c 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 scaler_id;
>>
>> /* find scaler attached to this pipe */
>> @@ -980,10 +979,8 @@ 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 = scaler_id;
>> -
>> /* Read CASF regs for second scaler */
>> - if (HAS_CASF(display) && id == 1)
>> + if (HAS_CASF(display) && scaler_id == 1)
>> intel_casf_sharpness_get_config(crtc_state);
>>
>> if (!crtc_state->hw.casf_params.casf_enable)
>> @@ -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 = scaler_id;
>> + if (scaler_id >= 0)
>
> We'll have scaler_id == crtc->num_scalers if the loop
> didn't find a pipe scaler. So this is now a bit busted.

Definitively. Should be addressed in v2.

> Otherwise I think this stuff looks pretty reasonable, but it no longer
> applies since my casf reorganization landed. Please rebase and repost.

Thanks :)

BR,
Michał

>
>> scaler_state->scaler_users |= (1 << SKL_CRTC_INDEX);
>> else
>> scaler_state->scaler_users &= ~(1 << SKL_CRTC_INDEX);
>> --
>> 2.45.2
>
> --
> Ville Syrjälä
> Intel
>