[PATCH i-g-t v6 4/5] tests/kms_plane_scaling: Find display mode fitting in BW for rotations

[Kamil Konieczny]

Hi Naladala,
On 2024-07-18 at 16:41:45 +0530, Naladala Ramanaidu wrote:
> Anticipating bandwidth issues, we expect many tests to fail. To
> address these failures, we will switch to the next lower display
> mode. Some higher display modes will be identified as insufficient
> for downscaling operations on plane scaling. As a solution, we
> will implement a fix: When bandwidth is inadequate for current
> modes, the system will automatically attempt the next lower

s/the system/test/

> display mode.
> 

Now subject and description looks better.

> Example for failure:
> [drm:intel_compute_min_cdclk [xe]] required cdclk (1066500 kHz)
> exceeds max (652800 kHz)
> 
> v2: Fix some styling issues in the patch (Ankit)
> v3: Split single plane and multi plane scaling
>     functions arguments in separate patch (Ankit)
> v4: Split single plane and multi plane scaling
>     functions in separate patch (Ankit)
> v5: Update check_scaling_pipe_plane_rot to handle
>     scaling mode failure by trying next lower mode
> v6: Update commit subject/message and add driver return value
>     in igt_debug and igt_skip print   (kamil)
> 
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala at intel.com>
> ---
>  tests/kms_plane_scaling.c | 101 +++++++++++++++++++++-----------------
>  1 file changed, 55 insertions(+), 46 deletions(-)
> 
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index 2c4490fd3..6cd7ed35d 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -582,55 +582,64 @@ static void check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane,
>  	int w, h;
>  	int width, height;
>  
> -	mode = igt_output_get_mode(output);
> -	if (is_clip_clamp == true) {
> -		width = mode->hdisplay + 100;
> -		height = mode->vdisplay + 100;
> -	} else {
> -		width = get_width(mode, sf_plane);
> -		height = get_height(mode, sf_plane);
> -	}
> +	for_each_connector_mode(output) {
> +		mode = &output->config.connector->modes[j__];
> +		igt_debug("Trying mode %dx%d\n",
> +				mode->hdisplay, mode->vdisplay);
> +		if (is_clip_clamp == true) {
> +			width = mode->hdisplay + 100;
> +			height = mode->vdisplay + 100;
> +		} else {
> +			width = get_width(mode, sf_plane);
> +			height = get_height(mode, sf_plane);
> +		}
>  
> -	if (is_upscale) {
> -		w = width;
> -		h = height;
> -	} else {
> -		w = mode->hdisplay;
> -		h = mode->vdisplay;
> +		if (is_upscale) {
> +			w = width;
> +			h = height;
> +		} else {
> +			w = mode->hdisplay;
> +			h = mode->vdisplay;
> +		}
> +		/*
> +		 * guarantee even value width/height to avoid fractional
> +		 * uv component in chroma subsampling for yuv 4:2:0 formats
> +		 */
> +		w = ALIGN(w, 2);
> +		h = ALIGN(h, 2);
> +
> +		igt_create_fb(display->drm_fd, w, h, pixel_format,
> +			      modifier, &d->fb[0]);
> +
> +		igt_plane_set_fb(plane, &d->fb[0]);
> +		igt_fb_set_position(&d->fb[0], plane, 0, 0);
> +		igt_fb_set_size(&d->fb[0], plane, w, h);
> +		igt_plane_set_position(plane, 0, 0);
> +		commit_ret = igt_display_try_commit2(display, COMMIT_ATOMIC);
> +		igt_skip_on_f(commit_ret == -ERANGE || commit_ret == -EINVAL,
> +			      "Mode fail with  %dx%d\n", w, h);
> +
> +		if (is_upscale)
> +			igt_plane_set_size(plane, mode->hdisplay, mode->vdisplay);
> +		else
> +			igt_plane_set_size(plane, width, height);
> +
> +		if (rot != IGT_ROTATION_0)
> +			igt_plane_set_rotation(plane, rot);
> +		commit_ret = igt_display_try_commit2(display, COMMIT_ATOMIC);
> +		if (commit_ret == -ERANGE || commit_ret == -EINVAL)
> +			igt_debug("Unsupported scaling factor with fb size %dx%d "
> +				  "with return value %s\n",
> +				   w, h, (commit_ret == -EINVAL) ? "-EINVAL" : "-ERANGE");

imho you could also print rot here, with or without it
Acked-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

Please wait for a review from kms developer(s) before merge.

Regards,
Kamil

> +		igt_plane_set_fb(plane, NULL);
> +		igt_plane_set_position(plane, 0, 0);
> +		cleanup_fbs(d);
> +		if (commit_ret == 0)
> +			break;
>  	}
> -
> -	/*
> -	 * guarantee even value width/height to avoid fractional
> -	 * uv component in chroma subsampling for yuv 4:2:0 formats
> -	 * */
> -	w = ALIGN(w, 2);
> -	h = ALIGN(h, 2);
> -
> -	igt_create_fb(display->drm_fd, w, h, pixel_format, modifier, &d->fb[0]);
> -
> -	igt_plane_set_fb(plane, &d->fb[0]);
> -	igt_fb_set_position(&d->fb[0], plane, 0, 0);
> -	igt_fb_set_size(&d->fb[0], plane, w, h);
> -	igt_plane_set_position(plane, 0, 0);
> -	commit_ret = igt_display_try_commit2(display, COMMIT_ATOMIC);
> -	igt_skip_on_f(commit_ret == -ERANGE || commit_ret == -EINVAL,
> -		      "Mode fail with  %dx%d\n", w, h);
> -	if (is_upscale)
> -		igt_plane_set_size(plane, mode->hdisplay, mode->vdisplay);
> -	else
> -		igt_plane_set_size(plane, width, height);
> -
> -	if (rot != IGT_ROTATION_0)
> -		igt_plane_set_rotation(plane, rot);
> -	commit_ret = igt_display_try_commit2(display, COMMIT_ATOMIC);
> -
> -	igt_plane_set_fb(plane, NULL);
> -	igt_plane_set_position(plane, 0, 0);
> -	cleanup_fbs(d);
> -
>  	igt_skip_on_f(commit_ret == -ERANGE || commit_ret == -EINVAL,
> -		      "Unsupported scaling factor with fb size %dx%d\n",
> -		      w, h);
> +		      "Unsupported scaling operation in driver with return value %s\n",
> +		      (commit_ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
>  	igt_assert_eq(commit_ret, 0);
>  }
>  
> -- 
> 2.43.0
>