Re: [PATCH] drm/i915/scaler: Fix WA_14011503117

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

On Wed, 06 Aug 2025, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Wed, 06 Aug 2025, Suraj Kandpal <suraj.kandpal@intel.com> wrote:
>> This introduces and uses a variable id which is just uninitialized.
>> What really needs to be used is the scaler_id.
>>
>> Fixes: 73309ed9d598 ("drm/i915/display: WA_14011503117")
>> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/skl_scaler.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
>> index 1374fa94ff5c..cd7ebbeb9508 100644
>> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
>> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
>> @@ -959,11 +959,12 @@ void adl_scaler_ecc_unmask(const struct intel_crtc_state *crtc_state)
>>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>>  	const struct intel_crtc_scaler_state *scaler_state =
>>  		&crtc_state->scaler_state;
>> -	int id;
>>  
>>  	if (!scaler_state && scaler_state->scaler_id == -1)
>
> That check does also does not make any sense.
>
> scaler_state == NULL && scaler_state->scaler_id

So scaler_state can never be NULL. The whole if statement is a
nop. Previously uninitialized stuff got written to the register, and
with the fix -1 can be written to the register.

The fix passed xe CI, but got merged before i915 CI was run. But xe CI
is meaningless here, because the code doesn't get run with xe, only
i915.


BR,
Jani.


>
> BR,
> Jani.
>
>
>>  		return;
>>  
>> -	intel_de_write_fw(display, SKL_PS_ECC_STAT(crtc->pipe, id), 1);
>> +	intel_de_write_fw(display,
>> +			  SKL_PS_ECC_STAT(crtc->pipe, scaler_state->scaler_id),
>> +			  1);
>>  	intel_de_write(display, XELPD_DISPLAY_ERR_FATAL_MASK, 0);
>>  }

-- 
Jani Nikula, Intel