[PATCH i-g-t 1/2] tests/kms_plane_scaling: Improvise the scaling BW issues.
Nautiyal, Ankit K
ankit.k.nautiyal at intel.com
Thu Jul 11 06:25:51 UTC 2024
On 7/9/2024 6:33 PM, Naladala Ramanaidu wrote:
> Many tests are failing due to Bandwidth issues.
> To eliminate this failures we change the least
> working display modes.
>
> - Observed, for higher modes Bandwidth is not sufficient
> to downscale operation and multi-plane scaling operations.
> - Add a fix, when Bandwidth is not sufficent for higher
> modes it will try the next lower display mode.
> - Fixed some styling issues in the patch.
>
> v2: Fix some styling issues in the patch . (Ankit)
>
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala at intel.com>
> ---
> tests/kms_plane_scaling.c | 181 ++++++++++++++++++++++----------------
> 1 file changed, 106 insertions(+), 75 deletions(-)
>
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index 3f63d3cf4..3296f98e8 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -210,6 +210,9 @@ typedef struct {
> igt_display_t display;
> struct igt_fb fb[4];
> bool extended;
> + double sf_plane1;
> + double sf_plane2;
> + bool flag;
> } data_t;
>
> struct invalid_paramtests {
> @@ -579,48 +582,57 @@ static void check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane,
I think we can just change the functions to pass scaling factor instead
of width and height.
More details below.
> drmModeModeInfo *mode;
> int commit_ret;
> int w, h;
> + bool mode_support = false;
> +
> + for_each_connector_mode(output) {
> + mode = &output->config.connector->modes[j__];
> + if (is_upscale) {
> + w = width;
> + h = height;
> + } else {
> + if (d->flag == true) {
> + width = mode->hdisplay + 100;
> + height = mode->vdisplay + 100;
> + } else {
> + width = get_width(mode, d->sf_plane1);
> + height = get_height(mode, d->sf_plane1);
> + }
> + w = mode->hdisplay;
> + h = mode->vdisplay;
> + }
>
> - mode = igt_output_get_mode(output);
> -
> - 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,
> + "Unsupported resolution parameters %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);
> +
> + if (igt_display_try_commit2(display, COMMIT_ATOMIC) >= 0) {
> + igt_plane_set_fb(plane, NULL);
> + igt_plane_set_position(plane, 0, 0);
> + cleanup_fbs(d);
> + mode_support = true;
> + 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);
> -
> - 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);
> - igt_assert_eq(commit_ret, 0);
> + igt_skip_on_f(!mode_support, "Band Width not sufficent for scaling\n");
> }
>
> static const igt_rotation_t rotations[] = {
> @@ -847,41 +859,52 @@ __test_planes_scaling_combo(data_t *d, int w1, int h1, int w2, int h2,
I think we should do away with w1,h1,w2,h2 and just pass scaling factor1
and 2 in these functions.
We are anyway overwriting these. With that, we will not be required to
add extra members sf_plane1/2 in data.
Also we might want to split into smaller patches.
Otherwise the approach looks good to me.
Regards,
Ankit
> igt_display_t *display = &d->display;
> drmModeModeInfo *mode;
> int ret;
> -
> - mode = igt_output_get_mode(output);
> -
> - igt_plane_set_fb(p1, fb1);
> - igt_plane_set_fb(p2, fb2);
> -
> - switch (test_type) {
> - case TEST_PLANES_UPSCALE:
> - igt_plane_set_size(p1, mode->hdisplay, mode->vdisplay);
> - igt_plane_set_size(p2, mode->hdisplay - 20, mode->vdisplay - 20);
> - break;
> - case TEST_PLANES_DOWNSCALE:
> - igt_plane_set_size(p1, w1, h1);
> - igt_plane_set_size(p2, w2, h2);
> - break;
> - case TEST_PLANES_UPSCALE_DOWNSCALE:
> - igt_plane_set_size(p1, mode->hdisplay, mode->vdisplay);
> - igt_plane_set_size(p2, w2, h2);
> - break;
> - case TEST_PLANES_DOWNSCALE_UPSCALE:
> - igt_plane_set_size(p1, w1, h1);
> - igt_plane_set_size(p2, mode->hdisplay, mode->vdisplay);
> - break;
> - default:
> - igt_assert(0);
> + bool mode_support = false;
> +
> + for_each_connector_mode(output) {
> + mode = &output->config.connector->modes[j__];
> + w1 = get_width(mode, d->sf_plane1);
> + h1 = get_height(mode, d->sf_plane1);
> + w2 = get_width(mode, d->sf_plane2);
> + h2 = get_height(mode, d->sf_plane2);
> +
> + igt_output_override_mode(output, mode);
> + igt_plane_set_fb(p1, fb1);
> + igt_plane_set_fb(p2, fb2);
> + ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> + igt_skip_on_f(ret == -EINVAL || ret == -ERANGE,
> + "Resolution not supported by driver\n");
> +
> + switch (test_type) {
> + case TEST_PLANES_UPSCALE:
> + igt_plane_set_size(p1, mode->hdisplay, mode->vdisplay);
> + igt_plane_set_size(p2, mode->hdisplay - 20, mode->vdisplay - 20);
> + break;
> + case TEST_PLANES_DOWNSCALE:
> + igt_plane_set_size(p1, w1, h1);
> + igt_plane_set_size(p2, w2, h2);
> + break;
> + case TEST_PLANES_UPSCALE_DOWNSCALE:
> + igt_plane_set_size(p1, mode->hdisplay, mode->vdisplay);
> + igt_plane_set_size(p2, w2, h2);
> + break;
> + case TEST_PLANES_DOWNSCALE_UPSCALE:
> + igt_plane_set_size(p1, w1, h1);
> + igt_plane_set_size(p2, mode->hdisplay, mode->vdisplay);
> + break;
> + default:
> + igt_assert(0);
> + }
> + ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> + if (ret >= 0) {
> + mode_support = true;
> + igt_plane_set_fb(p1, NULL);
> + igt_plane_set_fb(p2, NULL);
> + igt_assert_eq(ret, 0);
> + break;
> + }
> }
> -
> - ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> -
> - igt_plane_set_fb(p1, NULL);
> - igt_plane_set_fb(p2, NULL);
> -
> - igt_skip_on_f(ret == -EINVAL || ret == -ERANGE,
> - "Scaling op not supported by driver\n");
> - igt_assert_eq(ret, 0);
> + igt_skip_on_f(!mode_support, "Band Width not sufficent for Multi Plane\n");
> }
>
> static void setup_fb(int fd, int width, int height, struct igt_fb *fb)
> @@ -902,9 +925,9 @@ test_planes_scaling_combo(data_t *d, int w1, int h1, int w2, int h2,
> int n_planes;
>
> cleanup_crtc(d);
> + mode = igt_output_get_mode(output);
>
> igt_output_set_pipe(output, pipe);
> - mode = igt_output_get_mode(output);
>
> n_planes = display->pipes[pipe].n_planes;
> igt_require(n_planes >= 2);
> @@ -1308,6 +1331,7 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>
> igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> drmModeModeInfo *mode = igt_output_get_mode(output);
> + data.sf_plane1 = scaler_with_pixel_format_tests[index].sf;
>
> test_scaler_with_pixel_format_pipe(&data,
> get_width(mode, scaler_with_pixel_format_tests[index].sf),
> @@ -1333,6 +1357,7 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>
> igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> drmModeModeInfo *mode = igt_output_get_mode(output);
> + data.sf_plane1 = scaler_with_rotation_tests[index].sf;
>
> test_scaler_with_rotation_pipe(&data,
> get_width(mode, scaler_with_rotation_tests[index].sf),
> @@ -1358,6 +1383,7 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>
> igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> drmModeModeInfo *mode = igt_output_get_mode(output);
> + data.sf_plane1 = scaler_with_modifiers_tests[index].sf;
>
> test_scaler_with_modifier_pipe(&data,
> get_width(mode, scaler_with_modifiers_tests[index].sf),
> @@ -1382,6 +1408,7 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>
> igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> drmModeModeInfo *mode = igt_output_get_mode(output);
> + data.flag = true;
>
> test_scaler_with_pixel_format_pipe(&data, mode->hdisplay + 100,
> mode->vdisplay + 100, false, pipe, output);
> @@ -1402,6 +1429,7 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>
> igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> drmModeModeInfo *mode = igt_output_get_mode(output);
> + data.flag = true;
>
> test_scaler_with_rotation_pipe(&data, mode->hdisplay + 100,
> mode->vdisplay + 100, false, pipe, output);
> @@ -1422,6 +1450,7 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>
> igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> drmModeModeInfo *mode = igt_output_get_mode(output);
> + data.flag = true;
> test_scaler_with_modifier_pipe(&data, mode->hdisplay + 100,
> mode->vdisplay + 100, false, pipe, output);
> }
> @@ -1442,6 +1471,8 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>
> igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> drmModeModeInfo *mode = igt_output_get_mode(output);
> + data.sf_plane1 = scaler_with_2_planes_tests[index].sf_plane1;
> + data.sf_plane2 = scaler_with_2_planes_tests[index].sf_plane2;
>
> test_planes_scaling_combo(&data,
> get_width(mode, scaler_with_2_planes_tests[index].sf_plane1),
More information about the igt-dev
mailing list