[PATCH] drm/i915/scaler: Fix WA_14011503117

[Kandpal, Suraj]

> -----Original Message-----
> From: Jani Nikula <jani.nikula at linux.intel.com>
> Sent: Wednesday, August 6, 2025 1:54 PM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>; intel-xe at lists.freedesktop.org;
> intel-gfx at lists.freedesktop.org
> Cc: Garg, Nemesa <nemesa.garg at intel.com>; Kandpal, Suraj
> <suraj.kandpal at intel.com>; Manna, Animesh <animesh.manna at intel.com>
> Subject: Re: [PATCH] drm/i915/scaler: Fix WA_14011503117
> 
> On Wed, 06 Aug 2025, Jani Nikula <jani.nikula at linux.intel.com> wrote:
> > On Wed, 06 Aug 2025, Suraj Kandpal <suraj.kandpal at 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 at 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.
> 

Yes right I did forget to check the i915 patchwork while merging this will keep that in mind going ahead
As for the if statement we can send a fix patch immediately.
Sorry won't happen again

Regards,
Suraj Kandpal

> 
> 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