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

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

[Nemesa Garg]

Remove the redundant check statement.

Fixes: 73309ed9d598 ("drm/i915/display: WA_14011503117")
Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
---
 drivers/gpu/drm/i915/display/skl_scaler.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index cd7ebbeb9508..602198798174 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -960,9 +960,6 @@ void adl_scaler_ecc_unmask(const struct intel_crtc_state *crtc_state)
 	const struct intel_crtc_scaler_state *scaler_state =
 		&crtc_state->scaler_state;
 
-	if (!scaler_state && scaler_state->scaler_id == -1)
-		return;
-
 	intel_de_write_fw(display,
 			  SKL_PS_ECC_STAT(crtc->pipe, scaler_state->scaler_id),
 			  1);
-- 
2.25.1

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

[Jani Nikula]

On Wed, 06 Aug 2025, Nemesa Garg <nemesa.garg@intel.com> wrote:
> Remove the redundant check statement.

What if scaler_state->scaler_id == -1?

>
> Fixes: 73309ed9d598 ("drm/i915/display: WA_14011503117")
> Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
> ---
>  drivers/gpu/drm/i915/display/skl_scaler.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
> index cd7ebbeb9508..602198798174 100644
> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
> @@ -960,9 +960,6 @@ void adl_scaler_ecc_unmask(const struct intel_crtc_state *crtc_state)
>  	const struct intel_crtc_scaler_state *scaler_state =
>  		&crtc_state->scaler_state;
>  
> -	if (!scaler_state && scaler_state->scaler_id == -1)
> -		return;
> -
>  	intel_de_write_fw(display,
>  			  SKL_PS_ECC_STAT(crtc->pipe, scaler_state->scaler_id),
>  			  1);

-- 
Jani Nikula, Intel

RE: [PATCH] drm/i915/scaler: Fix for WA_14011503117

[Garg, Nemesa]

> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Wednesday, August 6, 2025 2:40 PM
> To: Garg, Nemesa <nemesa.garg@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Garg, Nemesa <nemesa.garg@intel.com>
> Subject: Re: [PATCH] drm/i915/scaler: Fix for WA_14011503117
> 
> On Wed, 06 Aug 2025, Nemesa Garg <nemesa.garg@intel.com> wrote:
> > Remove the redundant check statement.
> 
> What if scaler_state->scaler_id == -1?
> 
This is being called in case of pipe scaler getting used so scaler_id will be 0 or 1.
But yes adding this check can prevent the spurious write.

Thanks,
Nemesa 

> >
> > Fixes: 73309ed9d598 ("drm/i915/display: WA_14011503117")
> > Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/skl_scaler.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c
> b/drivers/gpu/drm/i915/display/skl_scaler.c
> > index cd7ebbeb9508..602198798174 100644
> > --- a/drivers/gpu/drm/i915/display/skl_scaler.c
> > +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
> > @@ -960,9 +960,6 @@ void adl_scaler_ecc_unmask(const struct
> intel_crtc_state *crtc_state)
> >  	const struct intel_crtc_scaler_state *scaler_state =
> >  		&crtc_state->scaler_state;
> >
> > -	if (!scaler_state && scaler_state->scaler_id == -1)
> > -		return;
> > -
> >  	intel_de_write_fw(display,
> >  			  SKL_PS_ECC_STAT(crtc->pipe, scaler_state-
> >scaler_id),
> >  			  1);
> 
> --
> Jani Nikula, Intel

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

[Nemesa Garg]

Remove the redundant check statement.

v2: Add scaler_id check [Jani]

Fixes: 73309ed9d598 ("drm/i915/display: WA_14011503117")
Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
---
 drivers/gpu/drm/i915/display/skl_scaler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index cd7ebbeb9508..6891a0b00f41 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -960,7 +960,7 @@ void adl_scaler_ecc_unmask(const struct intel_crtc_state *crtc_state)
 	const struct intel_crtc_scaler_state *scaler_state =
 		&crtc_state->scaler_state;
 
-	if (!scaler_state && scaler_state->scaler_id == -1)
+	if (scaler_state->scaler_id == -1)
 		return;
 
 	intel_de_write_fw(display,
-- 
2.25.1

RE: [PATCH] drm/i915/scaler: Fix for WA_14011503117

[Kandpal, Suraj]

> Subject: [PATCH] drm/i915/scaler: Fix for WA_14011503117

We already have a commit with that name don't really want many commits with the same subject

> 
> Remove the redundant check statement.

The is statement is not redundant just wrong will need to fix the commit description

> 
> v2: Add scaler_id check [Jani]
> 
> Fixes: 73309ed9d598 ("drm/i915/display: WA_14011503117")
> Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
> ---
>  drivers/gpu/drm/i915/display/skl_scaler.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c
> b/drivers/gpu/drm/i915/display/skl_scaler.c
> index cd7ebbeb9508..6891a0b00f41 100644
> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
> @@ -960,7 +960,7 @@ void adl_scaler_ecc_unmask(const struct
> intel_crtc_state *crtc_state)
>  	const struct intel_crtc_scaler_state *scaler_state =
>  		&crtc_state->scaler_state;
> 
> -	if (!scaler_state && scaler_state->scaler_id == -1)
> +	if (scaler_state->scaler_id == -1)

Hmm but what if scaler_state ends up being null now

This should have been scaler_state && scaler_state->scaler_id ==
Also please add people who had bought this issue up in cc to keep them in loop in this case that would be Jani.
It's very easy to miss fixes like this in all the mails that end up getting sent upstream more so if they are not cc'ed

Regards,
Suraj Kandpal
>  		return;
> 
>  	intel_de_write_fw(display,
> --
> 2.25.1

RE: [PATCH] drm/i915/scaler: Fix for WA_14011503117

[Jani Nikula]

On Thu, 07 Aug 2025, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote:
>> Subject: [PATCH] drm/i915/scaler: Fix for WA_14011503117
>
> We already have a commit with that name don't really want many commits with the same subject
>
>> 
>> Remove the redundant check statement.
>
> The is statement is not redundant just wrong will need to fix the commit description
>
>> 
>> v2: Add scaler_id check [Jani]
>> 
>> Fixes: 73309ed9d598 ("drm/i915/display: WA_14011503117")
>> Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/skl_scaler.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c
>> b/drivers/gpu/drm/i915/display/skl_scaler.c
>> index cd7ebbeb9508..6891a0b00f41 100644
>> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
>> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
>> @@ -960,7 +960,7 @@ void adl_scaler_ecc_unmask(const struct
>> intel_crtc_state *crtc_state)
>>  	const struct intel_crtc_scaler_state *scaler_state =
>>  		&crtc_state->scaler_state;
>> 
>> -	if (!scaler_state && scaler_state->scaler_id == -1)
>> +	if (scaler_state->scaler_id == -1)
>
> Hmm but what if scaler_state ends up being null now

It can't be NULL.

>
> This should have been scaler_state && scaler_state->scaler_id ==
> Also please add people who had bought this issue up in cc to keep them in loop in this case that would be Jani.
> It's very easy to miss fixes like this in all the mails that end up getting sent upstream more so if they are not cc'ed
>
> Regards,
> Suraj Kandpal
>>  		return;
>> 
>>  	intel_de_write_fw(display,
>> --
>> 2.25.1
>

-- 
Jani Nikula, Intel

RE: [PATCH] drm/i915/scaler: Fix for WA_14011503117

[Kandpal, Suraj]

> Subject: RE: [PATCH] drm/i915/scaler: Fix for WA_14011503117
> 
> 
> > Subject: [PATCH] drm/i915/scaler: Fix for WA_14011503117
> 
> We already have a commit with that name don't really want many commits
> with the same subject
> 
> >
> > Remove the redundant check statement.
> 
> The is statement is not redundant just wrong will need to fix the commit
> description
> 
> >
> > v2: Add scaler_id check [Jani]
> >
> > Fixes: 73309ed9d598 ("drm/i915/display: WA_14011503117")
> > Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/skl_scaler.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c
> > b/drivers/gpu/drm/i915/display/skl_scaler.c
> > index cd7ebbeb9508..6891a0b00f41 100644
> > --- a/drivers/gpu/drm/i915/display/skl_scaler.c
> > +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
> > @@ -960,7 +960,7 @@ void adl_scaler_ecc_unmask(const struct
> > intel_crtc_state *crtc_state)
> >  	const struct intel_crtc_scaler_state *scaler_state =
> >  		&crtc_state->scaler_state;
> >
> > -	if (!scaler_state && scaler_state->scaler_id == -1)
> > +	if (scaler_state->scaler_id == -1)
> 
> Hmm but what if scaler_state ends up being null now
> 
> This should have been scaler_state && scaler_state->scaler_id == Also please
> add people who had bought this issue up in cc to keep them in loop in this case
> that would be Jani.
> It's very easy to miss fixes like this in all the mails that end up getting sent
> upstream more so if they are not cc'ed

Also rather than all this I would like the condition to be something like
!scaler_state || scaler_state->scaler_id < 0
Looks cleaner and also takes care of scaler state is null case
There is a good chance that if scaler state really ends up being null it will blow elsewhere in code
Before reaching here but still good thing to have in place

Regards,
Suraj Kandpal
> 
> Regards,
> Suraj Kandpal
> >  		return;
> >
> >  	intel_de_write_fw(display,
> > --
> > 2.25.1