Re: [PATCH i-g-t 1/5] tests/kms_plane_scaling: Update the single plane scaling function arguments

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

On 7/15/2024 9:16 PM, Naladala Ramanaidu wrote:
> Update the helper test_scaler_with_modifier_pipe to use a scaling_factor
> and is_clip_clamp flag instead of explicit width and height parameters.
> This change simplifies the function interfaces and allows for testing
> scenarios, where we need to recalculate the width and height based on the
> display mode.Adjusted all function calls to match new argument order.
>
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala@intel.com>
> ---
>   tests/kms_plane_scaling.c | 61 ++++++++++++++++++++++-----------------
>   1 file changed, 35 insertions(+), 26 deletions(-)
>
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index 3f63d3cf4..f5efd13ef 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -569,7 +569,8 @@ static void cleanup_crtc(data_t *data)
>   static void check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane,
>   					 uint32_t pixel_format,
>   					 uint64_t modifier,
> -					 int width, int height,
> +					 double sf_plane1,
> +					 bool is_clip_clamp,
>   					 bool is_upscale,
>   					 enum pipe pipe,
>   					 igt_output_t *output,
> @@ -579,9 +580,18 @@ static void check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane,
>   	drmModeModeInfo *mode;
>   	int commit_ret;
>   	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_plane1);
> +		height = get_height(mode, sf_plane1);
> +	}
> +
>   	if (is_upscale) {
>   		w = width;
>   		h = height;
> @@ -693,7 +703,8 @@ static const uint64_t modifiers[] = {
>   };
>   
>   static void test_scaler_with_modifier_pipe(data_t *d,
> -					   int width, int height,
> +					   double sf_plane1,
> +					   bool is_clip_clamp,
>   					   bool is_upscale,
>   					   enum pipe pipe,
>   					   igt_output_t *output)
> @@ -716,7 +727,8 @@ static void test_scaler_with_modifier_pipe(data_t *d,
>   			if (igt_plane_has_format_mod(plane, format, modifier))
>   				check_scaling_pipe_plane_rot(d, plane,
>   							     format, modifier,
> -							     width, height,
> +							     sf_plane1,
> +							     is_clip_clamp,
>   							     is_upscale,
>   							     pipe, output,
>   							     IGT_ROTATION_0);
> @@ -725,7 +737,8 @@ static void test_scaler_with_modifier_pipe(data_t *d,
>   }
>   
>   static void test_scaler_with_rotation_pipe(data_t *d,
> -					   int width, int height,
> +					   double sf_plane1,
> +					   bool is_clip_clamp,
>   					   bool is_upscale,
>   					   enum pipe pipe,
>   					   igt_output_t *output)
> @@ -749,7 +762,8 @@ static void test_scaler_with_rotation_pipe(data_t *d,
>   			if (igt_plane_has_rotation(plane, rot))
>   				check_scaling_pipe_plane_rot(d, plane,
>   							     format, modifier,
> -							     width, height,
> +							     sf_plane1,
> +							     is_clip_clamp,
>   							     is_upscale,
>   							     pipe, output,
>   							     rot);
> @@ -757,8 +771,9 @@ static void test_scaler_with_rotation_pipe(data_t *d,
>   	}
>   }
>   
> -static void test_scaler_with_pixel_format_pipe(data_t *d, int width, int height, bool is_upscale,
> -					       enum pipe pipe, igt_output_t *output)
> +static void test_scaler_with_pixel_format_pipe(data_t *d, double sf_plane1,
> +		bool is_clip_clamp, bool is_upscale, enum pipe pipe,
> +		igt_output_t *output)

Align with parenthesis.

Otherwise looks good to me.

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


>   {
>   	igt_display_t *display = &d->display;
>   	uint64_t modifier = DRM_FORMAT_MOD_LINEAR;
> @@ -787,7 +802,7 @@ static void test_scaler_with_pixel_format_pipe(data_t *d, int width, int height,
>   			    can_scale(d, format))
>   			    check_scaling_pipe_plane_rot(d, plane,
>   							 format, modifier,
> -							 width, height,
> +							 sf_plane1, is_clip_clamp,
>   							 is_upscale,
>   							 pipe, output, IGT_ROTATION_0);
>   		}
> @@ -1307,11 +1322,10 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>   							continue;
>   
>   						igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> -							drmModeModeInfo *mode = igt_output_get_mode(output);
>   
>   							test_scaler_with_pixel_format_pipe(&data,
> -								get_width(mode, scaler_with_pixel_format_tests[index].sf),
> -								get_height(mode, scaler_with_pixel_format_tests[index].sf),
> +								scaler_with_pixel_format_tests[index].sf,
> +								false,
>   								scaler_with_pixel_format_tests[index].is_upscale,
>   								pipe, output);
>   						}
> @@ -1332,11 +1346,10 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>   							continue;
>   
>   						igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> -							drmModeModeInfo *mode = igt_output_get_mode(output);
>   
>   							test_scaler_with_rotation_pipe(&data,
> -								get_width(mode, scaler_with_rotation_tests[index].sf),
> -								get_height(mode, scaler_with_rotation_tests[index].sf),
> +								scaler_with_rotation_tests[index].sf,
> +								false,
>   								scaler_with_rotation_tests[index].is_upscale,
>   								pipe, output);
>   						}
> @@ -1357,11 +1370,10 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>   							continue;
>   
>   						igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> -							drmModeModeInfo *mode = igt_output_get_mode(output);
>   
>   							test_scaler_with_modifier_pipe(&data,
> -								get_width(mode, scaler_with_modifiers_tests[index].sf),
> -								get_height(mode, scaler_with_modifiers_tests[index].sf),
> +								scaler_with_modifiers_tests[index].sf,
> +								false,
>   								scaler_with_modifiers_tests[index].is_upscale,
>   								pipe, output);
>   						}
> @@ -1381,10 +1393,9 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>   						continue;
>   
>   					igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> -						drmModeModeInfo *mode = igt_output_get_mode(output);
>   
> -						test_scaler_with_pixel_format_pipe(&data, mode->hdisplay + 100,
> -							mode->vdisplay + 100, false, pipe, output);
> +						test_scaler_with_pixel_format_pipe(&data, 0.0, true,
> +								false, pipe, output);
>   					}
>   					break;
>   				}
> @@ -1401,10 +1412,9 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>   						continue;
>   
>   					igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> -						drmModeModeInfo *mode = igt_output_get_mode(output);
>   
> -						test_scaler_with_rotation_pipe(&data, mode->hdisplay + 100,
> -							mode->vdisplay + 100, false, pipe, output);
> +						test_scaler_with_rotation_pipe(&data, 0.0, true,
> +								false, pipe, output);
>   					}
>   					break;
>   				}
> @@ -1421,9 +1431,8 @@ igt_main_args("", long_opts, help_str, opt_handler, &data)
>   						continue;
>   
>   					igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> -						drmModeModeInfo *mode = igt_output_get_mode(output);
> -						test_scaler_with_modifier_pipe(&data, mode->hdisplay + 100,
> -							mode->vdisplay + 100, false, pipe, output);
> +						test_scaler_with_modifier_pipe(&data, 0.0, true,
> +								false, pipe, output);
>   					}
>   					break;
>   				}