[PATCH v1 0/9] drm/i915/scaler: allocation cleanup

[PATCH v1 0/9] drm/i915/scaler: allocation cleanup

[Michał Grzelak]

Apparently during review of commit 17d70a0760f2 ("drm/i915/casf: Extract
scaler_has_casf()") I missed that argument plane_state isn't used
anymore. While at it couple other fixes included.

This series will almost surely need a better splitting than currently
is, but I hope now it is slightly easier to review.

BR,
Michał


Michał Grzelak (9):
  drm/i915/scaler: remove unused plane_state argument
  drm/i915/scaler: s/i/scaler_id/
  drm/i915/scaler: de-Morgan-ize !scaler_has_casf()
  drm/i915/scaler: move independent CASF check
  drm/i915/scaler: check CASF before allocation's loop
  drm/i915/scaler: inline CASF scaler allocation
  drm/i915/scaler: linearize scaler allocation
  drm/i915/scaler: move CASF alloc check into
    intel_atomic_setup_scaler()
  drm/i915/scaler: move CASF alloc check into setup_crtc_scaler()

 drivers/gpu/drm/i915/display/skl_scaler.c | 43 ++++++++++++-----------
 1 file changed, 22 insertions(+), 21 deletions(-)

-- 
2.45.2

[PATCH v1 1/9] drm/i915/scaler: remove unused plane_state argument

[Michał Grzelak]

intel_allocate_scaler() no longer uses plane_state since condition
containing it has been removed while adding scaler_has_casf(). Remove
plane_state as well.

Cc: Nemesa Garg <nemesa.garg@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Fixes: 17d70a0760f2 ("drm/i915/casf: Extract scaler_has_casf()")
Signed-off-by: Michał Grzelak <michal.grzelak@intel.com>
---
 drivers/gpu/drm/i915/display/skl_scaler.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index 7994b983d509..4b456924fdef 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -334,7 +334,6 @@ static bool scaler_has_casf(struct intel_display *display, int scaler_id)
 
 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);
@@ -406,7 +405,7 @@ static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state,
 	int vscale = 0;
 
 	if (*scaler_id < 0)
-		*scaler_id = intel_allocate_scaler(scaler_state, crtc, plane_state, casf_scaler);
+		*scaler_id = intel_allocate_scaler(scaler_state, crtc, casf_scaler);
 
 	if (drm_WARN(display->drm, *scaler_id < 0,
 		     "Cannot find scaler for %s:%d\n", name, idx))
-- 
2.45.2

[PATCH v1 2/9] drm/i915/scaler: s/i/scaler_id/

[Michał Grzelak]

Rename generic iterator into more descriptive one.

Cc: Nemesa Garg <nemesa.garg@intel.com>
Cc: 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 | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index 4b456924fdef..0cfa5b7564ba 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -337,18 +337,18 @@ static int intel_allocate_scaler(struct intel_crtc_scaler_state *scaler_state,
 				 bool casf_scaler)
 {
 	struct intel_display *display = to_intel_display(crtc);
-	int i;
+	int scaler_id;
 
-	for (i = 0; i < crtc->num_scalers; i++) {
-		if (scaler_state->scalers[i].in_use)
+	for (scaler_id = 0; scaler_id < crtc->num_scalers; scaler_id++) {
+		if (scaler_state->scalers[scaler_id].in_use)
 			continue;
 
-		if (casf_scaler && !scaler_has_casf(display, i))
+		if (casf_scaler && !scaler_has_casf(display, scaler_id))
 			continue;
 
-		scaler_state->scalers[i].in_use = true;
+		scaler_state->scalers[scaler_id].in_use = true;
 
-		return i;
+		return scaler_id;
 	}
 
 	return -1;
-- 
2.45.2

[PATCH v1 3/9] drm/i915/scaler: de-Morgan-ize !scaler_has_casf()

[Michał Grzelak]

Distribute negation inside scaler_has_casf() using de Morgan law
resulting in (!HAS_CASF(display) || scaler_id != 1). Distribute the AND
operator with casf_scaler using de Morgan law once again in order to
have two OR'd clauses.

This will be used by next patches to separate and move both clauses.

Cc: Nemesa Garg <nemesa.garg@intel.com>
Cc: 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 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index 0cfa5b7564ba..f2216a9ea3f8 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -343,7 +343,8 @@ static int intel_allocate_scaler(struct intel_crtc_scaler_state *scaler_state,
 		if (scaler_state->scalers[scaler_id].in_use)
 			continue;
 
-		if (casf_scaler && !scaler_has_casf(display, scaler_id))
+		if ((casf_scaler && !HAS_CASF(display)) ||
+		    (casf_scaler && scaler_id != 1))
 			continue;
 
 		scaler_state->scalers[scaler_id].in_use = true;
-- 
2.45.2

[PATCH v1 4/9] drm/i915/scaler: move independent CASF check

[Michał Grzelak]

First clause of CASF check is independent of any loop iteration and
causes intel_allocate_scaler() to fail when satisfied. Check it before
entering the loop.

Cc: Nemesa Garg <nemesa.garg@intel.com>
Cc: 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 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index f2216a9ea3f8..59a5a3cd3e6a 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -339,12 +339,14 @@ static int intel_allocate_scaler(struct intel_crtc_scaler_state *scaler_state,
 	struct intel_display *display = to_intel_display(crtc);
 	int scaler_id;
 
+	if (casf_scaler && !HAS_CASF(display))
+		return -1;
+
 	for (scaler_id = 0; scaler_id < crtc->num_scalers; scaler_id++) {
 		if (scaler_state->scalers[scaler_id].in_use)
 			continue;
 
-		if ((casf_scaler && !HAS_CASF(display)) ||
-		    (casf_scaler && scaler_id != 1))
+		if (casf_scaler && scaler_id != 1)
 			continue;
 
 		scaler_state->scalers[scaler_id].in_use = true;
-- 
2.45.2

[PATCH v1 5/9] drm/i915/scaler: check CASF before allocation's loop

[Michał Grzelak]

intel_allocate_scaler() checks if casf_scaler is true and either
allocates second scaler or fails if it was already allocated. Mimic this
outside of the loop. Also fail the allocation if number of scalers is
less than 2.

Cc: Nemesa Garg <nemesa.garg@intel.com>
Cc: 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 | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index 59a5a3cd3e6a..29e4047c3f99 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -339,16 +339,24 @@ static int intel_allocate_scaler(struct intel_crtc_scaler_state *scaler_state,
 	struct intel_display *display = to_intel_display(crtc);
 	int scaler_id;
 
-	if (casf_scaler && !HAS_CASF(display))
-		return -1;
+	if (casf_scaler) {
+		if (!HAS_CASF(display))
+			return -1;
+
+		if (crtc->num_scalers < 2)
+			return -1;
+
+		if (scaler_state->scalers[1].in_use)
+			return -1;
+
+		scaler_state->scalers[1].in_use = true;
+		return 1;
+	}
 
 	for (scaler_id = 0; scaler_id < crtc->num_scalers; scaler_id++) {
 		if (scaler_state->scalers[scaler_id].in_use)
 			continue;
 
-		if (casf_scaler && scaler_id != 1)
-			continue;
-
 		scaler_state->scalers[scaler_id].in_use = true;
 
 		return scaler_id;
-- 
2.45.2

[PATCH v1 6/9] drm/i915/scaler: inline CASF scaler allocation

[Michał Grzelak]

Combine all CASF checks into one and invert it. By doing so check when
to allocate CASF instead of checking when to fail it.

Cc: Nemesa Garg <nemesa.garg@intel.com>
Cc: 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 | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index 29e4047c3f99..89920954c4b3 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -340,17 +340,13 @@ static int intel_allocate_scaler(struct intel_crtc_scaler_state *scaler_state,
 	int scaler_id;
 
 	if (casf_scaler) {
-		if (!HAS_CASF(display))
-			return -1;
-
-		if (crtc->num_scalers < 2)
-			return -1;
-
-		if (scaler_state->scalers[1].in_use)
-			return -1;
+		if (HAS_CASF(display) && crtc->num_scalers >= 2 &&
+		    !scaler_state->scalers[1].in_use) {
+			scaler_state->scalers[1].in_use = true;
+			return 1;
+		}
 
-		scaler_state->scalers[1].in_use = true;
-		return 1;
+		return -1;
 	}
 
 	for (scaler_id = 0; scaler_id < crtc->num_scalers; scaler_id++) {
-- 
2.45.2

[PATCH v1 7/9] drm/i915/scaler: linearize scaler allocation

[Michał Grzelak]

Remove last continue statement from intel_allocate_scaler() by inverting
the condition. Thus allocate the scaler if it has not been already
allocated.

Cc: Nemesa Garg <nemesa.garg@intel.com>
Cc: 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 | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index 89920954c4b3..b3c6376c8603 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -350,12 +350,10 @@ static int intel_allocate_scaler(struct intel_crtc_scaler_state *scaler_state,
 	}
 
 	for (scaler_id = 0; scaler_id < crtc->num_scalers; scaler_id++) {
-		if (scaler_state->scalers[scaler_id].in_use)
-			continue;
-
-		scaler_state->scalers[scaler_id].in_use = true;
-
-		return scaler_id;
+		if (!scaler_state->scalers[scaler_id].in_use) {
+			scaler_state->scalers[scaler_id].in_use = true;
+			return scaler_id;
+		}
 	}
 
 	return -1;
-- 
2.45.2

[PATCH v1 8/9] drm/i915/scaler: move CASF alloc check into intel_atomic_setup_scaler()

[Michał Grzelak]

Move CASF allocation check into only caller of intel_allocate_scaler().
Allocate scaler as usual if CASF was not requested. Remove casf_scaler
from intel_allocate_scaler()'s argument list. Also remove intel_display
as both are no longer used.

Benefits of doing so are not clearly visible as it is a mid-step for
moving the check to the caller again.

Cc: Nemesa Garg <nemesa.garg@intel.com>
Cc: 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 | 27 ++++++++++-------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index b3c6376c8603..b40c9e500bbc 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -333,22 +333,10 @@ static bool scaler_has_casf(struct intel_display *display, int scaler_id)
 }
 
 static int intel_allocate_scaler(struct intel_crtc_scaler_state *scaler_state,
-				 struct intel_crtc *crtc,
-				 bool casf_scaler)
+				 struct intel_crtc *crtc)
 {
-	struct intel_display *display = to_intel_display(crtc);
 	int scaler_id;
 
-	if (casf_scaler) {
-		if (HAS_CASF(display) && crtc->num_scalers >= 2 &&
-		    !scaler_state->scalers[1].in_use) {
-			scaler_state->scalers[1].in_use = true;
-			return 1;
-		}
-
-		return -1;
-	}
-
 	for (scaler_id = 0; scaler_id < crtc->num_scalers; scaler_id++) {
 		if (!scaler_state->scalers[scaler_id].in_use) {
 			scaler_state->scalers[scaler_id].in_use = true;
@@ -409,8 +397,17 @@ static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state,
 	int hscale = 0;
 	int vscale = 0;
 
-	if (*scaler_id < 0)
-		*scaler_id = intel_allocate_scaler(scaler_state, crtc, casf_scaler);
+	if (*scaler_id < 0) {
+		if (casf_scaler) {
+			if (HAS_CASF(display) && crtc->num_scalers >= 2 &&
+			    !scaler_state->scalers[1].in_use) {
+				scaler_state->scalers[1].in_use = true;
+				*scaler_id = 1;
+			}
+		} else {
+			*scaler_id = intel_allocate_scaler(scaler_state, crtc);
+		}
+	}
 
 	if (drm_WARN(display->drm, *scaler_id < 0,
 		     "Cannot find scaler for %s:%d\n", name, idx))
-- 
2.45.2

[PATCH v1 9/9] drm/i915/scaler: move CASF alloc check into setup_crtc_scaler()

[Michał Grzelak]

Move CASF allocation check to the caller of intel_atomic_setup_scaler().
Add intel_display to make the check work. Remove casf_scaler from
intel_atomic_setup_scaler()'s argument list since it is no longer used.

Do not add the check into setup_plane_scaler() as it calls
intel_atomic_setup_scaler() with casf_scaler set to false.

Cc: Nemesa Garg <nemesa.garg@intel.com>
Cc: 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 | 32 +++++++++++------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index b40c9e500bbc..da9c7ef8610d 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -389,7 +389,7 @@ static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state,
 				     int num_scalers_need, struct intel_crtc *crtc,
 				     const char *name, int idx,
 				     struct intel_plane_state *plane_state,
-				     int *scaler_id, bool casf_scaler)
+				     int *scaler_id)
 {
 	struct intel_display *display = to_intel_display(crtc);
 	struct intel_crtc_scaler_state *scaler_state = &crtc_state->scaler_state;
@@ -397,17 +397,8 @@ static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state,
 	int hscale = 0;
 	int vscale = 0;
 
-	if (*scaler_id < 0) {
-		if (casf_scaler) {
-			if (HAS_CASF(display) && crtc->num_scalers >= 2 &&
-			    !scaler_state->scalers[1].in_use) {
-				scaler_state->scalers[1].in_use = true;
-				*scaler_id = 1;
-			}
-		} else {
-			*scaler_id = intel_allocate_scaler(scaler_state, crtc);
-		}
-	}
+	if (*scaler_id < 0)
+		*scaler_id = intel_allocate_scaler(scaler_state, crtc);
 
 	if (drm_WARN(display->drm, *scaler_id < 0,
 		     "Cannot find scaler for %s:%d\n", name, idx))
@@ -534,16 +525,26 @@ static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state,
 static int setup_crtc_scaler(struct intel_atomic_state *state,
 			     struct intel_crtc *crtc)
 {
+	struct intel_display *display = to_intel_display(state);
 	struct intel_crtc_state *crtc_state =
 		intel_atomic_get_new_crtc_state(state, crtc);
 	struct intel_crtc_scaler_state *scaler_state =
 		&crtc_state->scaler_state;
 
+	if (scaler_state->scaler_id < 0) {
+		if (crtc_state->pch_pfit.casf.enable) {
+			if (HAS_CASF(display) && crtc->num_scalers >= 2 &&
+			    !scaler_state->scalers[1].in_use) {
+				scaler_state->scalers[1].in_use = true;
+				scaler_state->scaler_id = 1;
+			}
+		}
+	}
+
 	return intel_atomic_setup_scaler(crtc_state,
 					 hweight32(scaler_state->scaler_users),
 					 crtc, "CRTC", crtc->base.base.id,
-					 NULL, &scaler_state->scaler_id,
-					 crtc_state->pch_pfit.casf.enable);
+					 NULL, &scaler_state->scaler_id);
 }
 
 static int setup_plane_scaler(struct intel_atomic_state *state,
@@ -578,8 +579,7 @@ static int setup_plane_scaler(struct intel_atomic_state *state,
 	return intel_atomic_setup_scaler(crtc_state,
 					 hweight32(scaler_state->scaler_users),
 					 crtc, "PLANE", plane->base.base.id,
-					 plane_state, &plane_state->scaler_id,
-					 false);
+					 plane_state, &plane_state->scaler_id);
 }
 
 /**
-- 
2.45.2

RE: [PATCH v1 9/9] drm/i915/scaler: move CASF alloc check into setup_crtc_scaler()

[Garg, Nemesa]

> -----Original Message-----
> From: Grzelak, Michal <michal.grzelak@intel.com>
> Sent: Wednesday, June 10, 2026 4:42 AM
> To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
> Cc: Grzelak, Michal <michal.grzelak@intel.com>; Garg, Nemesa
> <nemesa.garg@intel.com>; Ville Syrjälä <ville.syrjala@linux.intel.com>
> Subject: [PATCH v1 9/9] drm/i915/scaler: move CASF alloc check into
> setup_crtc_scaler()
> 
> Move CASF allocation check to the caller of intel_atomic_setup_scaler().
> Add intel_display to make the check work. Remove casf_scaler from
> intel_atomic_setup_scaler()'s argument list since it is no longer used.
> 
> Do not add the check into setup_plane_scaler() as it calls
> intel_atomic_setup_scaler() with casf_scaler set to false.
> 
> Cc: Nemesa Garg <nemesa.garg@intel.com>
> Cc: 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 | 32 +++++++++++------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c
> b/drivers/gpu/drm/i915/display/skl_scaler.c
> index b40c9e500bbc..da9c7ef8610d 100644
> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
> @@ -389,7 +389,7 @@ static int intel_atomic_setup_scaler(struct
> intel_crtc_state *crtc_state,
>  				     int num_scalers_need, struct intel_crtc
> *crtc,
>  				     const char *name, int idx,
>  				     struct intel_plane_state *plane_state,
> -				     int *scaler_id, bool casf_scaler)
> +				     int *scaler_id)
>  {
>  	struct intel_display *display = to_intel_display(crtc);
>  	struct intel_crtc_scaler_state *scaler_state = &crtc_state-
> >scaler_state; @@ -397,17 +397,8 @@ static int
> intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state,
>  	int hscale = 0;
>  	int vscale = 0;
> 
> -	if (*scaler_id < 0) {
> -		if (casf_scaler) {
> -			if (HAS_CASF(display) && crtc->num_scalers >= 2 &&
> -			    !scaler_state->scalers[1].in_use) {
> -				scaler_state->scalers[1].in_use = true;
> -				*scaler_id = 1;
> -			}
> -		} else {
> -			*scaler_id = intel_allocate_scaler(scaler_state, crtc);
> -		}
> -	}
> +	if (*scaler_id < 0)
> +		*scaler_id = intel_allocate_scaler(scaler_state, crtc);
> 
>  	if (drm_WARN(display->drm, *scaler_id < 0,
>  		     "Cannot find scaler for %s:%d\n", name, idx)) @@ -534,16
> +525,26 @@ static int intel_atomic_setup_scaler(struct intel_crtc_state
> *crtc_state,  static int setup_crtc_scaler(struct intel_atomic_state *state,
>  			     struct intel_crtc *crtc)
>  {
> +	struct intel_display *display = to_intel_display(state);
>  	struct intel_crtc_state *crtc_state =
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	struct intel_crtc_scaler_state *scaler_state =
>  		&crtc_state->scaler_state;
> 
> +	if (scaler_state->scaler_id < 0) {
> +		if (crtc_state->pch_pfit.casf.enable) {
> +			if (HAS_CASF(display) && crtc->num_scalers >= 2 &&
> +			    !scaler_state->scalers[1].in_use) {
> +				scaler_state->scalers[1].in_use = true;
> +				scaler_state->scaler_id = 1;
> +			}
> +		}
> +	}
> +
>  	return intel_atomic_setup_scaler(crtc_state,
>  					 hweight32(scaler_state-
> >scaler_users),
>  					 crtc, "CRTC", crtc->base.base.id,
> -					 NULL, &scaler_state->scaler_id,
> -					 crtc_state->pch_pfit.casf.enable);
> +					 NULL, &scaler_state->scaler_id);
>  }
> 
>  static int setup_plane_scaler(struct intel_atomic_state *state, @@ -578,8
> +579,7 @@ static int setup_plane_scaler(struct intel_atomic_state *state,
>  	return intel_atomic_setup_scaler(crtc_state,
>  					 hweight32(scaler_state-
> >scaler_users),
>  					 crtc, "PLANE", plane->base.base.id,
> -					 plane_state, &plane_state->scaler_id,
> -					 false);
> +					 plane_state, &plane_state-
> >scaler_id);
>  }
> 
Hi Michal,

The rational behind adding CASF in allocate_scaler() was to have all scaler related stuff together.
As CASF also needs scaler and before allocating we need to check its availability, so not sure whether we should move it outside of allocation function or keep as it is.

Thanks and Regards,
Nemesa

>  /**
> --
> 2.45.2

✗ i915.CI.BAT: failure for drm/i915/scaler: allocation cleanup

[Patchwork]

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

== Series Details ==

Series: drm/i915/scaler: allocation cleanup
URL   : https://patchwork.freedesktop.org/series/168207/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_18653 -> Patchwork_168207v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_168207v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_168207v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_168207v1/index.html

Participating hosts (42 -> 39)
------------------------------

  Missing    (3): bat-dg2-13 fi-glk-j4005 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_168207v1:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live:
    - bat-twl-1:          [PASS][1] -> [DMESG-FAIL][2] +1 other test dmesg-fail
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18653/bat-twl-1/igt@i915_selftest@live.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_168207v1/bat-twl-1/igt@i915_selftest@live.html

  


Build changes
-------------

  * Linux: CI_DRM_18653 -> Patchwork_168207v1

  CI-20190529: 20190529
  CI_DRM_18653: 724e06af260b571e2d970d4a806ae3d620ff0b7a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8956: 8956
  Patchwork_168207v1: 724e06af260b571e2d970d4a806ae3d620ff0b7a @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_168207v1/index.html

[-- Attachment #2: Type: text/html, Size: 2414 bytes --]

Re: [PATCH v1 0/9] drm/i915/scaler: allocation cleanup

[Ville Syrjälä]

On Wed, Jun 10, 2026 at 01:12:08AM +0200, Michał Grzelak wrote:
> Apparently during review of commit 17d70a0760f2 ("drm/i915/casf: Extract
> scaler_has_casf()") I missed that argument plane_state isn't used
> anymore. While at it couple other fixes included.
> 
> This series will almost surely need a better splitting than currently
> is, but I hope now it is slightly easier to review.
> 
> BR,
> Michał
> 
> 
> Michał Grzelak (9):
>   drm/i915/scaler: remove unused plane_state argument
>   drm/i915/scaler: s/i/scaler_id/

Those seem fine.

>   drm/i915/scaler: de-Morgan-ize !scaler_has_casf()
>   drm/i915/scaler: move independent CASF check
>   drm/i915/scaler: check CASF before allocation's loop
>   drm/i915/scaler: inline CASF scaler allocation
>   drm/i915/scaler: linearize scaler allocation
>   drm/i915/scaler: move CASF alloc check into
>     intel_atomic_setup_scaler()
>   drm/i915/scaler: move CASF alloc check into setup_crtc_scaler()

This looks like it's replacing a simple "does this scale have CASF?"
check with some kind of more complicated special casing of the CASF
scaler elsewhere. I don't like it.

> 
>  drivers/gpu/drm/i915/display/skl_scaler.c | 43 ++++++++++++-----------
>  1 file changed, 22 insertions(+), 21 deletions(-)
> 
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH v1 4/9] drm/i915/scaler: move independent CASF check

[Jani Nikula]

On Wed, 10 Jun 2026, Michał Grzelak <michal.grzelak@intel.com> wrote:
> First clause of CASF check is independent of any loop iteration and
> causes intel_allocate_scaler() to fail when satisfied. Check it before
> entering the loop.
>
> Cc: Nemesa Garg <nemesa.garg@intel.com>
> Cc: 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 | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
> index f2216a9ea3f8..59a5a3cd3e6a 100644
> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
> @@ -339,12 +339,14 @@ static int intel_allocate_scaler(struct intel_crtc_scaler_state *scaler_state,
>  	struct intel_display *display = to_intel_display(crtc);
>  	int scaler_id;
>  
> +	if (casf_scaler && !HAS_CASF(display))
> +		return -1;

Using -1 as magic error return value is one of my pet peeves.

-1 == -EPERM, and that's hardly the case here.

I know the function returns -1 elsewhere, both should be fixed. -EINVAL
is better.


BR,
Jani.

> +
>  	for (scaler_id = 0; scaler_id < crtc->num_scalers; scaler_id++) {
>  		if (scaler_state->scalers[scaler_id].in_use)
>  			continue;
>  
> -		if ((casf_scaler && !HAS_CASF(display)) ||
> -		    (casf_scaler && scaler_id != 1))
> +		if (casf_scaler && scaler_id != 1)
>  			continue;
>  
>  		scaler_state->scalers[scaler_id].in_use = true;

-- 
Jani Nikula, Intel

Re: [PATCH v1 0/9] drm/i915/scaler: allocation cleanup

[Michał Grzelak]

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

On Wed, 10 Jun 2026, Ville Syrjälä wrote:
> On Wed, Jun 10, 2026 at 01:12:08AM +0200, Michał Grzelak wrote:
>> Apparently during review of commit 17d70a0760f2 ("drm/i915/casf: Extract
>> scaler_has_casf()") I missed that argument plane_state isn't used
>> anymore. While at it couple other fixes included.
>>
>> This series will almost surely need a better splitting than currently
>> is, but I hope now it is slightly easier to review.
>>
>> BR,
>> Michał
>>
>>
>> Michał Grzelak (9):
>>   drm/i915/scaler: remove unused plane_state argument
>>   drm/i915/scaler: s/i/scaler_id/
>
> Those seem fine.
>
>>   drm/i915/scaler: de-Morgan-ize !scaler_has_casf()
>>   drm/i915/scaler: move independent CASF check
>>   drm/i915/scaler: check CASF before allocation's loop
>>   drm/i915/scaler: inline CASF scaler allocation
>>   drm/i915/scaler: linearize scaler allocation
>>   drm/i915/scaler: move CASF alloc check into
>>     intel_atomic_setup_scaler()
>>   drm/i915/scaler: move CASF alloc check into setup_crtc_scaler()
>
> This looks like it's replacing a simple "does this scale have CASF?"
> check with some kind of more complicated special casing of the CASF
> scaler elsewhere. I don't like it.

Basically this part was my attempt at "remove `continue`s from
allocation loop" following [1], but no idea how to do it without moving
& obscuring the CASF check. Anyways can drop those.

BR,
Michał

[1] https://lore.kernel.org/intel-gfx/acZezWKFo1QwLAci@intel.com/

>
>>
>>  drivers/gpu/drm/i915/display/skl_scaler.c | 43 ++++++++++++-----------
>>  1 file changed, 22 insertions(+), 21 deletions(-)
>>
>> --
>> 2.45.2
>
> -- 
> Ville Syrjälä
> Intel
>

RE: [PATCH v1 2/9] drm/i915/scaler: s/i/scaler_id/

[Garg, Nemesa]

> -----Original Message-----
> From: Grzelak, Michal <michal.grzelak@intel.com>
> Sent: Wednesday, June 10, 2026 4:42 AM
> To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
> Cc: Grzelak, Michal <michal.grzelak@intel.com>; Garg, Nemesa
> <nemesa.garg@intel.com>; Ville Syrjälä <ville.syrjala@linux.intel.com>
> Subject: [PATCH v1 2/9] drm/i915/scaler: s/i/scaler_id/
> 
> Rename generic iterator into more descriptive one.
> 
> Cc: Nemesa Garg <nemesa.garg@intel.com>
> Cc: 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 | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c
> b/drivers/gpu/drm/i915/display/skl_scaler.c
> index 4b456924fdef..0cfa5b7564ba 100644
> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
> @@ -337,18 +337,18 @@ static int intel_allocate_scaler(struct
> intel_crtc_scaler_state *scaler_state,
>  				 bool casf_scaler)
>  {
>  	struct intel_display *display = to_intel_display(crtc);
> -	int i;
> +	int scaler_id;
> 
> -	for (i = 0; i < crtc->num_scalers; i++) {
> -		if (scaler_state->scalers[i].in_use)
> +	for (scaler_id = 0; scaler_id < crtc->num_scalers; scaler_id++) {
> +		if (scaler_state->scalers[scaler_id].in_use)
>  			continue;
> 
> -		if (casf_scaler && !scaler_has_casf(display, i))
> +		if (casf_scaler && !scaler_has_casf(display, scaler_id))
>  			continue;
> 
> -		scaler_state->scalers[i].in_use = true;
> +		scaler_state->scalers[scaler_id].in_use = true;
> 
> -		return i;
> +		return scaler_id;
>  	}
> 
>  	return -1;
> --
LGTM,
Reviewed-by: Nemesa Garg <nemesa.garg@intel.com>

> 2.45.2