Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Sharma, Swati2" <swati2.sharma@intel.com>
To: Naladala Ramanaidu <ramanaidu.naladala@intel.com>,
	igt-dev@lists.freedesktop.org
Subject: Re: [PATCH i-g-t v1 1/5] tests/kms_plane_scaling: Update scaling functions to return error codes
Date: Tue, 10 Sep 2024 12:36:12 +0530	[thread overview]
Message-ID: <5e6be7cd-85e9-43b9-a025-662854df1458@intel.com> (raw)
In-Reply-To: <20240905145038.1569226-2-ramanaidu.naladala@intel.com>

Hi Ramanaidu,


On 05-Sep-24 8:20 PM, 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.
> 
> v1: fix crash issues.  (Swati)
> 
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala@intel.com>
> ---
>   tests/kms_plane_scaling.c | 263 +++++++++++++++++++++-----------------
>   1 file changed, 148 insertions(+), 115 deletions(-)
> 
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index 42eb67409..5c5837a51 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -566,15 +566,16 @@ static void cleanup_crtc(data_t *data)
>   	cleanup_fbs(data);
>   }
>   
> -static void check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane,
> -					 uint32_t pixel_format,
> -					 uint64_t modifier,
> -					 double sf_plane,
> -					 bool is_clip_clamp,
> -					 bool is_upscale,
> -					 enum pipe pipe,
> -					 igt_output_t *output,
> -					 igt_rotation_t rot)
> +static uint32_t
> +check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane,
> +			     uint32_t pixel_format,
> +			     uint64_t modifier,
> +			     double sf_plane,
> +			     bool is_clip_clamp,
> +			     bool is_upscale,
> +			     enum pipe pipe,
> +			     igt_output_t *output,
> +			     igt_rotation_t rot)
>   {
>   	igt_display_t *display = &d->display;
>   	drmModeModeInfo *mode;
> @@ -634,11 +635,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 +707,18 @@ 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 uint32_t
> +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,28 +732,33 @@ 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,
> -					   double sf_plane,
> -					   bool is_clip_clamp,
> -					   bool is_upscale,
> -					   enum pipe pipe,
> -					   igt_output_t *output)
> +static uint32_t
> +test_scaler_with_rotation_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;
>   	uint64_t modifier = DRM_FORMAT_MOD_LINEAR;
>   	igt_plane_t *plane;
> +	uint32_t ret;
>   
>   	cleanup_crtc(d);
>   
> @@ -768,26 +772,31 @@ 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,
> -					       bool is_clip_clamp,
> -					       bool is_upscale,
> -					       enum pipe pipe,
> -					       igt_output_t *output)
> +static uint32_t
> +test_scaler_with_pixel_format_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;
>   	uint64_t modifier = DRM_FORMAT_MOD_LINEAR;
>   	igt_plane_t *plane;
> +	uint32_t ret;
>   
>   	cleanup_crtc(d);
>   
> @@ -810,17 +819,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 +1340,7 @@ static data_t data;
>   igt_main_args("", long_opts, help_str, opt_handler, &data)
>   {
>   	enum pipe pipe;
> +	uint32_t ret = -EINVAL;
>   
>   	igt_fixture {
>   		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
> @@ -1343,21 +1357,23 @@ 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) {
> +							if (!pipe_output_combo_valid(&data.display, pipe, output))
> +								continue;
> +							if (get_num_scalers(&data.display, pipe) < 1)
> +								continue;

Add a new line here.

With this fixed.
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>

> +							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;
>   						}
> -						break;
> +						igt_skip_on_f(ret == -ERANGE || ret == -EINVAL,
> +							      "Unsupported scaling operation in driver with return value %s\n",
> +							      (ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
>   					}
>   				}
>   			}
> @@ -1367,21 +1383,24 @@ 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)) {
> +						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;
>   						}
> -						break;
> +						igt_skip_on_f(ret == -ERANGE || ret == -EINVAL,
> +							      "Unsupported scaling operation in driver with return value %s\n",
> +							      (ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
>   					}
>   				}
>   			}
> @@ -1391,21 +1410,24 @@ 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) {
> +							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;
>   						}
> -						break;
> +						igt_skip_on_f(ret == -ERANGE || ret == -EINVAL,
> +							      "Unsupported scaling operation in driver with return value %s\n",
> +							      (ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
>   					}
>   				}
>   			}
> @@ -1414,39 +1436,46 @@ 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)) {
> +				igt_dynamic_f("pipe-%s", kmstest_pipe_name(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;
>   
> -						test_scaler_with_pixel_format_pipe(&data, 0.0, true,
> -										   false, pipe,
> -										   output);
> +						ret = test_scaler_with_pixel_format_pipe(&data, 0.0, true,
> +											 false, pipe,
> +											 output);
> +						if (ret == 0)
> +							break;
>   					}
> -					break;
> +					igt_skip_on_f(ret == -ERANGE || ret == -EINVAL,
> +						      "Unsupported scaling operation in driver with return value %s\n",
> +						      (ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
>   				}
>   			}
>   		}
>   
> +
>   		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) {
> +						if (!pipe_output_combo_valid(&data.display, pipe, output))
> +							continue;
> +						if (get_num_scalers(&data.display, pipe) < 1)
> +							continue;
>   
> -						test_scaler_with_rotation_pipe(&data, 0.0, true,
> -										false, pipe,
> -										output);
> +						ret = test_scaler_with_rotation_pipe(&data, 0.0, true,
> +										     false, pipe,
> +										     output);
> +						if (ret == 0)
> +							break;
>   					}
> -					break;
> +					igt_skip_on_f(ret == -ERANGE || ret == -EINVAL,
> +						      "Unsupported scaling operation in driver with return value %s\n",
> +						      (ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
>   				}
>   			}
>   		}
> @@ -1454,18 +1483,22 @@ 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", kmstest_pipe_name(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);
> +						ret = test_scaler_with_modifier_pipe(&data, 0.0, true,
> +										     false, pipe,
> +										     output);
> +						if (ret == 0)
> +							break;
>   					}
> -					break;
> +					igt_skip_on_f(ret == -ERANGE || ret == -EINVAL,
> +						      "Unsupported scaling operation in driver with return value %s\n",
> +						      (ret == -EINVAL) ? "-EINVAL" : "-ERANGE");
>   				}
>   			}
>   		}

  reply	other threads:[~2024-09-10  7:06 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-05 14:50 [PATCH i-g-t v1 0/5] tests/kms_plane_scaling: Update scaling functions to retry on valid outputs Naladala Ramanaidu
2024-09-05 14:50 ` [PATCH i-g-t v1 1/5] tests/kms_plane_scaling: Update scaling functions to return error codes Naladala Ramanaidu
2024-09-10  7:06   ` Sharma, Swati2 [this message]
2024-09-05 14:50 ` [PATCH i-g-t v1 2/5] tests/kms_plane_scaling: Add informative logs for single plane scaling operations Naladala Ramanaidu
2024-09-10  7:42   ` Sharma, Swati2
2024-09-05 14:50 ` [PATCH i-g-t v1 3/5] tests/kms_plane_scaling: Return error code in test_planes_scaling_combo Naladala Ramanaidu
2024-09-10  8:09   ` Sharma, Swati2
2024-09-05 14:50 ` [PATCH i-g-t v1 4/5] tests/kms_plane_scaling: Add debug logs for output scaling attempts Naladala Ramanaidu
2024-09-10  8:12   ` Sharma, Swati2
2024-09-05 14:50 ` [PATCH i-g-t v1 5/5] HAX patch do not merge Naladala Ramanaidu
2024-09-05 23:37 ` ✗ CI.xeBAT: failure for tests/kms_plane_scaling: Update scaling functions to retry on valid outputs (rev2) Patchwork
2024-09-08  3:33 ` ✗ CI.xeFULL: " Patchwork
2024-09-10 10:57 ` ✗ CI.xeBAT: " Patchwork
2024-09-10 12:02 ` ✗ CI.xeFULL: " Patchwork
2024-09-10 23:03 ` ✗ CI.xeBAT: failure for tests/kms_plane_scaling: Update scaling functions to retry on valid outputs (rev3) Patchwork
2024-09-11  1:04 ` ✓ Fi.CI.IGT: success for tests/kms_plane_scaling: Update scaling functions to retry on valid outputs (rev2) Patchwork
2024-09-11  1:18 ` ✗ CI.xeFULL: failure for tests/kms_plane_scaling: Update scaling functions to retry on valid outputs (rev3) Patchwork

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=5e6be7cd-85e9-43b9-a025-662854df1458@intel.com \
    --to=swati2.sharma@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --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