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

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 7/18/2024 4:41 PM, 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
> display mode.
>
> 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@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",

Please dont break the message string as mentioned in last patch.

With that fixed, this is:

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

> +				   w, h, (commit_ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
> +		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);
>   }
>