From: "Sharma, Swati2" <swati2.sharma@intel.com>
To: Naladala Ramanaidu <ramanaidu.naladala@intel.com>,
igt-dev@lists.freedesktop.org
Cc: ankit.k.nautiyal@intel.com, kunal1.joshi@intel.com,
karthik.b.s@intel.com
Subject: Re: [PATCH i-g-t 1/3] tests/kms_plane_scaling: Update scaling functions to return error codes
Date: Fri, 30 Aug 2024 12:39:03 +0530 [thread overview]
Message-ID: <68c65065-7cdc-41ab-bf3c-cafc1774d195@intel.com> (raw)
In-Reply-To: <20240826045630.1340226-2-ramanaidu.naladala@intel.com>
On 26-Aug-24 10:26 AM, Naladala Ramanaidu wrote:
> This will ensures that the system can automatically find a suitable
> configuration, improving the robustness and flexibility of the
> testing process. Introduce a `ret` variable to capture the return
> value of scaling operations. Modify `check_scaling_pipe_plane_rot`
> to return an integer error code instead of void. Update
> `test_scaler_with_modifier_pipe`, `test_scaler_with_rotation_pipe`,
> and `test_scaler_with_pixel_format_pipe` to handle the returned
> error code. Add informative logs to indicate when a scaling operation
> is not supported on a specific output.
>
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala@intel.com>
> ---
> tests/kms_plane_scaling.c | 250 +++++++++++++++++++++++---------------
> 1 file changed, 150 insertions(+), 100 deletions(-)
>
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index 42eb67409..588a9c5f0 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -566,7 +566,7 @@ static void cleanup_crtc(data_t *data)
> cleanup_fbs(data);
> }
>
> -static void check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane,
> +static int check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane,
> uint32_t pixel_format,
> uint64_t modifier,
> double sf_plane,
> @@ -634,11 +634,7 @@ static void check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane,
> if (commit_ret == 0)
> break;
> }
> -
> - igt_skip_on_f(commit_ret == -ERANGE || commit_ret == -EINVAL,
> - "Unsupported scaling operation in driver with return value %s\n",
> - (commit_ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
> - igt_assert_eq(commit_ret, 0);
> + return commit_ret;
> }
>
> static const igt_rotation_t rotations[] = {
> @@ -710,16 +706,17 @@ static const uint64_t modifiers[] = {
> I915_FORMAT_MOD_4_TILED
> };
>
> -static void test_scaler_with_modifier_pipe(data_t *d,
> - double sf_plane,
> - bool is_clip_clamp,
> - bool is_upscale,
> - enum pipe pipe,
> - igt_output_t *output)
> +static int test_scaler_with_modifier_pipe(data_t *d,
> + double sf_plane,
> + bool is_clip_clamp,
> + bool is_upscale,
> + enum pipe pipe,
> + igt_output_t *output)
> {
> igt_display_t *display = &d->display;
> unsigned format = DRM_FORMAT_XRGB8888;
> igt_plane_t *plane;
> + uint32_t ret;
>
> cleanup_crtc(d);
>
> @@ -733,18 +730,22 @@ static void test_scaler_with_modifier_pipe(data_t *d,
> uint64_t modifier = modifiers[i];
>
> if (igt_plane_has_format_mod(plane, format, modifier))
> - check_scaling_pipe_plane_rot(d, plane,
> - format, modifier,
> - sf_plane,
> - is_clip_clamp,
> - is_upscale,
> - pipe, output,
> - IGT_ROTATION_0);
> + ret = check_scaling_pipe_plane_rot(d, plane,
> + format, modifier,
> + sf_plane,
> + is_clip_clamp,
> + is_upscale,
> + pipe, output,
> + IGT_ROTATION_0);
> + if (ret != 0)
> + return ret;
> }
> +
> }
> + return ret;
> }
>
> -static void test_scaler_with_rotation_pipe(data_t *d,
> +static int test_scaler_with_rotation_pipe(data_t *d,
> double sf_plane,
> bool is_clip_clamp,
> bool is_upscale,
> @@ -755,6 +756,7 @@ static void test_scaler_with_rotation_pipe(data_t *d,
> unsigned format = DRM_FORMAT_XRGB8888;
> uint64_t modifier = DRM_FORMAT_MOD_LINEAR;
> igt_plane_t *plane;
> + uint32_t ret;
>
> cleanup_crtc(d);
>
> @@ -768,18 +770,21 @@ static void test_scaler_with_rotation_pipe(data_t *d,
> igt_rotation_t rot = rotations[i];
>
> if (igt_plane_has_rotation(plane, rot))
> - check_scaling_pipe_plane_rot(d, plane,
> - format, modifier,
> - sf_plane,
> - is_clip_clamp,
> - is_upscale,
> - pipe, output,
> - rot);
> + ret = check_scaling_pipe_plane_rot(d, plane,
> + format, modifier,
> + sf_plane,
> + is_clip_clamp,
> + is_upscale,
> + pipe, output,
> + rot);
> + if (ret != 0)
> + return ret;
> }
> }
> + return ret;
> }
>
> -static void test_scaler_with_pixel_format_pipe(data_t *d, double sf_plane,
> +static int test_scaler_with_pixel_format_pipe(data_t *d, double sf_plane,
> bool is_clip_clamp,
> bool is_upscale,
> enum pipe pipe,
> @@ -788,6 +793,7 @@ static void test_scaler_with_pixel_format_pipe(data_t *d, double sf_plane,
> igt_display_t *display = &d->display;
> uint64_t modifier = DRM_FORMAT_MOD_LINEAR;
> igt_plane_t *plane;
> + uint32_t ret;
>
> cleanup_crtc(d);
>
> @@ -810,17 +816,21 @@ static void test_scaler_with_pixel_format_pipe(data_t *d, double sf_plane,
> if (test_format(d, &tested_formats, format) &&
> igt_plane_has_format_mod(plane, format, modifier) &&
> can_scale(d, format))
> - check_scaling_pipe_plane_rot(d, plane,
> - format, modifier,
> - sf_plane,
> - is_clip_clamp,
> - is_upscale,
> - pipe, output,
> - IGT_ROTATION_0);
> + ret = check_scaling_pipe_plane_rot(d, plane,
> + format, modifier,
> + sf_plane,
> + is_clip_clamp,
> + is_upscale,
> + pipe, output,
> + IGT_ROTATION_0);
> + if (ret != 0) {
> + igt_vec_fini(&tested_formats);
> + return ret;
> + }
> }
> -
> igt_vec_fini(&tested_formats);
> }
> + return ret;
> }
>
> static enum pipe
> @@ -1327,6 +1337,7 @@ static data_t data;
> igt_main_args("", long_opts, help_str, opt_handler, &data)
> {
> enum pipe pipe;
> + uint32_t ret;
>
> igt_fixture {
> data.drm_fd = drm_open_driver_master(DRIVER_ANY);
> @@ -1343,21 +1354,28 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
> igt_describe(scaler_with_pixel_format_tests[index].describe);
> igt_subtest_with_dynamic(scaler_with_pixel_format_tests[index].name) {
> for_each_pipe(&data.display, pipe) {
> - for_each_valid_output_on_pipe(&data.display, pipe, output) {
> - if (!pipe_output_combo_valid(&data.display, pipe, output))
> - continue;
> - if (get_num_scalers(&data.display, pipe) < 1)
> - continue;
> -
> - igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> -
> - test_scaler_with_pixel_format_pipe(&data,
> + igt_dynamic_f("pipe-%s", kmstest_pipe_name(pipe)) {
> + for_each_valid_output_on_pipe(&data.display, pipe, output) {
> + igt_info(" Trying on %s\n",
> + igt_output_name(output));
> + if (!pipe_output_combo_valid(&data.display, pipe, output))
> + continue;
> + if (get_num_scalers(&data.display, pipe) < 1)
> + continue;
> + ret = test_scaler_with_pixel_format_pipe(&data,
> scaler_with_pixel_format_tests[index].sf,
> false,
> scaler_with_pixel_format_tests[index].is_upscale,
> pipe, output);
> + if (ret == 0)
> + break;
> + igt_info("Required Scaling operation not supported on %s trying on next output\n",
> + igt_output_name(output));
> }
> - break;
> + igt_skip_on_f(ret == -ERANGE || ret == -EINVAL,
> + "Unsupported scaling operation in driver with return value %s\n",
> + (ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
> + igt_assert_eq(ret, 0);
> }
> }
> }
> @@ -1367,21 +1385,27 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
> igt_describe(scaler_with_rotation_tests[index].describe);
> igt_subtest_with_dynamic(scaler_with_rotation_tests[index].name) {
> for_each_pipe(&data.display, pipe) {
> - for_each_valid_output_on_pipe(&data.display, pipe, output) {
> - if (!pipe_output_combo_valid(&data.display, pipe, output))
> - continue;
> - if (get_num_scalers(&data.display, pipe) < 1)
> - continue;
> -
> - igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> -
> - test_scaler_with_rotation_pipe(&data,
> + igt_dynamic_f("pipe-%s", kmstest_pipe_name(pipe)) {
> + igt_info(" Trying on %s\n", igt_output_name(output));
> + for_each_valid_output_on_pipe(&data.display, pipe, output) {
> + if (!pipe_output_combo_valid(&data.display, pipe, output))
> + continue;
> + if (get_num_scalers(&data.display, pipe) < 1)
> + continue;
> + ret = test_scaler_with_rotation_pipe(&data,
> scaler_with_rotation_tests[index].sf,
> false,
> scaler_with_rotation_tests[index].is_upscale,
> pipe, output);
> + if (ret == 0)
> + break;
> + igt_info("Required Scaling operation not supported on %s trying on next output\n",
> + igt_output_name(output));
Add separate patch for additional debug prints. This patch should be
restricted to error codes.
Same feedback implies to next patch.
> }
> - break;
> + igt_skip_on_f(ret == -ERANGE || ret == -EINVAL,
> + "Unsupported scaling operation in driver with return value %s\n",
> + (ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
> + igt_assert_eq(ret, 0);
> }
> }
> }
> @@ -1391,21 +1415,27 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
> igt_describe(scaler_with_modifiers_tests[index].describe);
> igt_subtest_with_dynamic(scaler_with_modifiers_tests[index].name) {
> for_each_pipe(&data.display, pipe) {
> - for_each_valid_output_on_pipe(&data.display, pipe, output) {
> - if (!pipe_output_combo_valid(&data.display, pipe, output))
> - continue;
> - if (get_num_scalers(&data.display, pipe) < 1)
> - continue;
> -
> - igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> -
> - test_scaler_with_modifier_pipe(&data,
> + igt_dynamic_f("pipe-%s", kmstest_pipe_name(pipe)) {
> + for_each_valid_output_on_pipe(&data.display, pipe, output) {
> + igt_info(" Trying on %s\n", igt_output_name(output));
> + if (!pipe_output_combo_valid(&data.display, pipe, output))
> + continue;
> + if (get_num_scalers(&data.display, pipe) < 1)
> + continue;
> + ret = test_scaler_with_modifier_pipe(&data,
> scaler_with_modifiers_tests[index].sf,
> false,
> scaler_with_modifiers_tests[index].is_upscale,
> pipe, output);
> + if (ret == 0)
> + break;
> + igt_info("Required Scaling operation not supported on %s trying on next valid output\n",
> + igt_output_name(output));
> }
> - break;
> + igt_skip_on_f(ret == -ERANGE || ret == -EINVAL,
> + "Unsupported scaling operation in driver with return value %s\n",
> + (ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
> + igt_assert_eq(ret, 0);
> }
> }
> }
> @@ -1414,19 +1444,25 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
> igt_describe("Tests scaling with clipping and clamping, pixel formats.");
> igt_subtest_with_dynamic("plane-scaler-with-clipping-clamping-pixel-formats") {
> for_each_pipe(&data.display, pipe) {
> - for_each_valid_output_on_pipe(&data.display, pipe, output) {
> - if (!pipe_output_combo_valid(&data.display, pipe, output))
> - continue;
> - if (get_num_scalers(&data.display, pipe) < 1)
> - continue;
> -
> - igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> -
> - test_scaler_with_pixel_format_pipe(&data, 0.0, true,
> - false, pipe,
> - output);
> + igt_dynamic_f("pipe-%s", kmstest_pipe_name(pipe)) {
> + for_each_valid_output_on_pipe(&data.display, pipe, output) {
> + igt_info(" Trying on %s\n", igt_output_name(output));
> + if (!pipe_output_combo_valid(&data.display, pipe, output))
> + continue;
> + if (get_num_scalers(&data.display, pipe) < 1)
> + continue;
> + ret = test_scaler_with_pixel_format_pipe(&data, 0.0, true,
> + false, pipe,
> + output);
> + if (ret == 0)
> + break;
> + igt_info("Required Scaling operation not supported on %s trying on next valid output\n",
> + igt_output_name(output));
> }
> - break;
> + igt_skip_on_f(ret == -ERANGE || ret == -EINVAL,
> + "Unsupported scaling operation in driver with return value %s\n",
> + (ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
> + igt_assert_eq(ret, 0);
> }
> }
> }
> @@ -1434,19 +1470,26 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
> igt_describe("Tests scaling with clipping and clamping, rotation.");
> igt_subtest_with_dynamic("plane-scaler-with-clipping-clamping-rotation") {
> for_each_pipe(&data.display, pipe) {
> - for_each_valid_output_on_pipe(&data.display, pipe, output) {
> - if (!pipe_output_combo_valid(&data.display, pipe, output))
> - continue;
> - if (get_num_scalers(&data.display, pipe) < 1)
> - continue;
> -
> - igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> + igt_dynamic_f("pipe-%s", kmstest_pipe_name(pipe)) {
> + for_each_valid_output_on_pipe(&data.display, pipe, output) {
> + igt_info(" Trying on %s\n", igt_output_name(output));
> + if (!pipe_output_combo_valid(&data.display, pipe, output))
> + continue;
> + if (get_num_scalers(&data.display, pipe) < 1)
> + continue;
> + ret = test_scaler_with_rotation_pipe(&data, 0.0, true,
> + false, pipe,
> + output);
> + if (ret == 0)
> + break;
> + igt_info("Required Scaling operation not supported on %s trying on next valid output\n",
> + igt_output_name(output));
>
> - test_scaler_with_rotation_pipe(&data, 0.0, true,
> - false, pipe,
> - output);
> }
> - break;
> + igt_skip_on_f(ret == -ERANGE || ret == -EINVAL,
> + "Unsupported scaling operation in driver with return value %s\n",
> + (ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
> + igt_assert_eq(ret, 0);
> }
> }
> }
> @@ -1454,18 +1497,25 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
> igt_describe("Tests scaling with clipping and clamping, modifiers.");
> igt_subtest_with_dynamic("plane-scaler-with-clipping-clamping-modifiers") {
> for_each_pipe(&data.display, pipe) {
> - for_each_valid_output_on_pipe(&data.display, pipe, output) {
> - if (!pipe_output_combo_valid(&data.display, pipe, output))
> - continue;
> - if (get_num_scalers(&data.display, pipe) < 1)
> - continue;
> -
> - igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> - test_scaler_with_modifier_pipe(&data, 0.0, true,
> - false, pipe,
> - output);
> + igt_dynamic_f("pipe-%s", kmstest_pipe_name(pipe)) {
> + for_each_valid_output_on_pipe(&data.display, pipe, output) {
> + igt_info(" Trying on %s\n", igt_output_name(output));
> + if (!pipe_output_combo_valid(&data.display, pipe, output))
> + continue;
> + if (get_num_scalers(&data.display, pipe) < 1)
> + continue;
> + ret = test_scaler_with_modifier_pipe(&data, 0.0, true,
> + false, pipe,
> + output);
> + if (ret == 0)
> + break;
> + igt_info("Required Scaling operation not supported on %s trying on next valid output\n",
> + igt_output_name(output));
> }
> - break;
> + igt_skip_on_f(ret == -ERANGE || ret == -EINVAL,
> + "Unsupported scaling operation in driver with return value %s\n",
> + (ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
> + igt_assert_eq(ret, 0);
> }
> }
> }
next prev parent reply other threads:[~2024-08-30 7:09 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-26 4:56 [PATCH i-g-t 0/3] tests/kms_plane_scaling: Update scaling functions to retry on valid outputs Naladala Ramanaidu
2024-08-26 4:56 ` [PATCH i-g-t 1/3] tests/kms_plane_scaling: Update scaling functions to return error codes Naladala Ramanaidu
2024-08-30 7:09 ` Sharma, Swati2 [this message]
2024-08-26 4:56 ` [PATCH i-g-t 2/3] tests/kms_plane_scaling: Update Multi-plane scaling functions Naladala Ramanaidu
2024-08-26 4:56 ` [PATCH i-g-t 3/3] HAX patch do not merge Naladala Ramanaidu
2024-08-26 6:22 ` ✗ CI.xeBAT: failure for tests/kms_plane_scaling: Update scaling functions to retry on valid outputs Patchwork
2024-08-26 6:32 ` ✓ Fi.CI.BAT: success " Patchwork
2024-08-26 7:23 ` ✗ CI.xeFULL: failure " Patchwork
2024-08-27 4:05 ` ✗ Fi.CI.IGT: " Patchwork
2024-08-30 7:05 ` [PATCH i-g-t 0/3] " Sharma, Swati2
-- strict thread matches above, loose matches on Subject: below --
2024-08-26 4:53 [PATCH i-g-t 0/3] tests/kms_plane_scaling: Update scaling Naladala Ramanaidu
2024-08-26 4:53 ` [PATCH i-g-t 1/3] tests/kms_plane_scaling: Update scaling functions to return error codes Naladala Ramanaidu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=68c65065-7cdc-41ab-bf3c-cafc1774d195@intel.com \
--to=swati2.sharma@intel.com \
--cc=ankit.k.nautiyal@intel.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=karthik.b.s@intel.com \
--cc=kunal1.joshi@intel.com \
--cc=ramanaidu.naladala@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox