diff mbox series

[v4,3/7] drm/i915/scaler: Refactor max_scale computation

Message ID 20250108123541.2101643-4-mitulkumar.ajitkumar.golani@intel.com (mailing list archive)
State New, archived
Headers show
Series Check Scaler and DSC Prefill Latency Against Vblank | expand

Commit Message

Golani, Mitulkumar Ajitkumar Jan. 8, 2025, 12:35 p.m. UTC
Refactor max scaling factor computation into a reusable
function for scalers.

Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
---
 drivers/gpu/drm/i915/display/skl_scaler.c | 63 ++++++++++++-----------
 1 file changed, 34 insertions(+), 29 deletions(-)

Comments

Nautiyal, Ankit K Jan. 13, 2025, 8:29 a.m. UTC | #1
On 1/8/2025 6:05 PM, Mitul Golani wrote:
> Refactor max scaling factor computation into a reusable
> function for scalers.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>   drivers/gpu/drm/i915/display/skl_scaler.c | 63 ++++++++++++-----------
>   1 file changed, 34 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
> index 4f838220f880..19ab7b38c64e 100644
> --- a/drivers/gpu/drm/i915/display/skl_scaler.c
> +++ b/drivers/gpu/drm/i915/display/skl_scaler.c
> @@ -309,6 +309,37 @@ static int intel_allocate_scaler(struct intel_crtc_scaler_state *scaler_state,
>   	return -1;
>   }
>   
> +static void
> +calculate_max_scale(struct intel_crtc *crtc,
> +		    bool is_yuv_semiplanar,
> +		    int scaler_id,
> +		    int *max_hscale, int *max_vscale)
> +{
> +	struct intel_display *display = to_intel_display(crtc);
> +
> +	/*
> +	 * FIXME: When two scalers are needed, but only one of
> +	 * them needs to downscale, we should make sure that
> +	 * the one that needs downscaling support is assigned
> +	 * as the first scaler, so we don't reject downscaling
> +	 * unnecessarily.
> +	 */
> +	if (DISPLAY_VER(display) >= 14) {
> +		*max_hscale = 0x30000 - 1;
> +
> +		if (scaler_id == 0)
> +			*max_vscale = 0x30000 - 1;
> +		else
> +			*max_vscale = 0x10000;
> +	} else if (DISPLAY_VER(display) >= 10 || !is_yuv_semiplanar) {
> +		*max_hscale = 0x30000 - 1;
> +		*max_vscale = 0x30000 - 1;
> +	} else {
> +		*max_hscale = 0x20000 - 1;
> +		*max_vscale = 0x20000 - 1;
> +	}
> +}
> +
>   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,
> @@ -378,35 +409,9 @@ static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state,
>   		const struct drm_rect *dst = &plane_state->uapi.dst;
>   		int hscale, vscale, max_vscale, max_hscale;
>   
> -		/*
> -		 * FIXME: When two scalers are needed, but only one of
> -		 * them needs to downscale, we should make sure that
> -		 * the one that needs downscaling support is assigned
> -		 * as the first scaler, so we don't reject downscaling
> -		 * unnecessarily.
> -		 */
> -
> -		if (DISPLAY_VER(display) >= 14) {
> -			/*
> -			 * On versions 14 and up, only the first
> -			 * scaler supports a vertical scaling factor
> -			 * of more than 1.0, while a horizontal
> -			 * scaling factor of 3.0 is supported.
> -			 */
> -			max_hscale = 0x30000 - 1;
> -			if (*scaler_id == 0)
> -				max_vscale = 0x30000 - 1;
> -			else
> -				max_vscale = 0x10000;

This part seems to be removed. I don't see anything mentioned in the 
commit message about this too.

If you are moving code to a new function, then it should be a simple 
move, without any functional change.

Regards,

Ankit

> -
> -		} else if (DISPLAY_VER(display) >= 10 ||
> -			   !intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier)) {
> -			max_hscale = 0x30000 - 1;
> -			max_vscale = 0x30000 - 1;
> -		} else {
> -			max_hscale = 0x20000 - 1;
> -			max_vscale = 0x20000 - 1;
> -		}
> +		calculate_max_scale(crtc,
> +				    intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier),
> +				    *scaler_id, &max_hscale, &max_vscale);
>   
>   		/*
>   		 * FIXME: We should change the if-else block above to
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index 4f838220f880..19ab7b38c64e 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -309,6 +309,37 @@  static int intel_allocate_scaler(struct intel_crtc_scaler_state *scaler_state,
 	return -1;
 }
 
+static void
+calculate_max_scale(struct intel_crtc *crtc,
+		    bool is_yuv_semiplanar,
+		    int scaler_id,
+		    int *max_hscale, int *max_vscale)
+{
+	struct intel_display *display = to_intel_display(crtc);
+
+	/*
+	 * FIXME: When two scalers are needed, but only one of
+	 * them needs to downscale, we should make sure that
+	 * the one that needs downscaling support is assigned
+	 * as the first scaler, so we don't reject downscaling
+	 * unnecessarily.
+	 */
+	if (DISPLAY_VER(display) >= 14) {
+		*max_hscale = 0x30000 - 1;
+
+		if (scaler_id == 0)
+			*max_vscale = 0x30000 - 1;
+		else
+			*max_vscale = 0x10000;
+	} else if (DISPLAY_VER(display) >= 10 || !is_yuv_semiplanar) {
+		*max_hscale = 0x30000 - 1;
+		*max_vscale = 0x30000 - 1;
+	} else {
+		*max_hscale = 0x20000 - 1;
+		*max_vscale = 0x20000 - 1;
+	}
+}
+
 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,
@@ -378,35 +409,9 @@  static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state,
 		const struct drm_rect *dst = &plane_state->uapi.dst;
 		int hscale, vscale, max_vscale, max_hscale;
 
-		/*
-		 * FIXME: When two scalers are needed, but only one of
-		 * them needs to downscale, we should make sure that
-		 * the one that needs downscaling support is assigned
-		 * as the first scaler, so we don't reject downscaling
-		 * unnecessarily.
-		 */
-
-		if (DISPLAY_VER(display) >= 14) {
-			/*
-			 * On versions 14 and up, only the first
-			 * scaler supports a vertical scaling factor
-			 * of more than 1.0, while a horizontal
-			 * scaling factor of 3.0 is supported.
-			 */
-			max_hscale = 0x30000 - 1;
-			if (*scaler_id == 0)
-				max_vscale = 0x30000 - 1;
-			else
-				max_vscale = 0x10000;
-
-		} else if (DISPLAY_VER(display) >= 10 ||
-			   !intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier)) {
-			max_hscale = 0x30000 - 1;
-			max_vscale = 0x30000 - 1;
-		} else {
-			max_hscale = 0x20000 - 1;
-			max_vscale = 0x20000 - 1;
-		}
+		calculate_max_scale(crtc,
+				    intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier),
+				    *scaler_id, &max_hscale, &max_vscale);
 
 		/*
 		 * FIXME: We should change the if-else block above to