[PATCH i-g-t v1 2/2] tests/kms_plane_scaling: Simplify pixel format test flow
Samala, Pranay
pranay.samala at intel.com
Wed Jul 16 07:30:52 UTC 2025
Hi Rama,
> -----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Naladala
> Ramanaidu
> Sent: Wednesday, July 16, 2025 3:38 AM
> To: igt-dev at lists.freedesktop.org
> Cc: Joshi, Kunal1 <kunal1.joshi at intel.com>; Sharma, Swati2
> <swati2.sharma at intel.com>; B S, Karthik <karthik.b.s at intel.com>; Naladala,
> Ramanaidu <ramanaidu.naladala at intel.com>
> Subject: [PATCH i-g-t v1 2/2] tests/kms_plane_scaling: Simplify pixel format test
> flow
>
> This patch improves the structure and clarity of the scaler pixel format tests by
> adopting a more modular and scalable approach.
> Rather than handling all pixel format variations within a single test block, the tests
> are now divided into distinct subtests for each pixel format model.
>
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala at intel.com>
> ---
> tests/kms_plane_scaling.c | 124 ++++++++++++++++++++++++++++----------
> 1 file changed, 91 insertions(+), 33 deletions(-)
>
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c index
> 63d35d76b..922d984a0 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -89,13 +89,25 @@
> */
>
> /**
> - * SUBTEST: plane-scaler-unity-scaling-with-pixel-format
> + * SUBTEST: plane-scaler-unity-scaling-with-pixel-format-rgb
> + * Description: Tests scaling with pixel formats, unity scaling.
> + *
> + * SUBTEST: plane-scaler-unity-scaling-with-pixel-format-packed
> + * Description: Tests scaling with pixel formats, unity scaling.
> + *
> + * SUBTEST: plane-scaler-unity-scaling-with-pixel-format-planar
> * Description: Tests scaling with pixel formats, unity scaling.
> *
> * SUBTEST: plane-scaler-with-clipping-clamping-pixel-formats
> * Description: Tests scaling with clipping and clamping, pixel formats.
> *
> - * SUBTEST: plane-upscale-%s-with-pixel-format
> + * SUBTEST: plane-upscale-%s-with-pixel-format-rgb
> + * Description: Tests upscaling with pixel formats %arg[1].
> + *
> + * SUBTEST: plane-upscale-%s-with-pixel-format-packed
> + * Description: Tests upscaling with pixel formats %arg[1].
> + *
> + * SUBTEST: plane-upscale-%s-with-pixel-format-planar
> * Description: Tests upscaling with pixel formats %arg[1].
> *
> * arg[1]:
> @@ -105,7 +117,13 @@
> */
>
> /**
> - * SUBTEST: plane-downscale-factor-%s-with-pixel-format
> + * SUBTEST: plane-downscale-factor-%s-with-pixel-format-rgb
> + * Description: Tests downscaling with pixel formats for %arg[1] scaling factor.
> + *
> + * SUBTEST: plane-downscale-factor-%s-with-pixel-format-packed
> + * Description: Tests downscaling with pixel formats for %arg[1] scaling factor.
> + *
> + * SUBTEST: plane-downscale-factor-%s-with-pixel-format-planar
> * Description: Tests downscaling with pixel formats for %arg[1] scaling factor.
> *
> * arg[1]:
> @@ -204,6 +222,7 @@ typedef struct {
> igt_display_t display;
> struct igt_fb fb[4];
> bool extended;
> + const char *pixel_format_type;
> } data_t;
>
> struct invalid_paramtests {
> @@ -815,7 +834,14 @@ 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))
> + can_scale(d, format)) {
> + if (strcmp(d->pixel_format_type, "packed") == 0)
> {
> + if (!igt_format_is_packedyuv(format))
> + continue;
> + } else if (strcmp(d->pixel_format_type, "planar")
> == 0) {
> + if (!igt_format_is_planaryuv(format))
> + continue;
> + }
Imho, add an explicit "rgb" check to skip YUV formats for rgb subtests.
This avoids accidentally running YUV formats in the rgb subtest.
else if (strcmp(d->pixel_format_type, "rgb") == 0) {
if (igt_format_is_packedyuv(format) || igt_format_is_planaryuv(format))
continue;
> ret = check_scaling_pipe_plane_rot(d, plane,
> format,
> modifier,
> sf_plane,
> @@ -823,9 +849,10 @@ test_scaler_with_pixel_format_pipe(data_t *d, double
> sf_plane,
> is_upscale,
> pipe, output,
>
> IGT_ROTATION_0);
> - if (ret != 0) {
> - igt_vec_fini(&tested_formats);
> - return ret;
> + if (ret != 0) {
> + igt_vec_fini(&tested_formats);
> + return ret;
> + }
> }
> }
> igt_vec_fini(&tested_formats);
> @@ -1324,6 +1351,62 @@ pipe_output_combo_valid(igt_display_t *display,
> return ret;
> }
>
> +static void
> +scaler_pixel_format(data_t *d, uint32_t index) {
> + uint32_t ret = -EINVAL;
> + uint32_t z;
> + char subtest_name[128];
> + enum pipe pipe;
> + igt_output_t *output;
> +
> + const char *append[3] = {
> + "rgb",
> + "planar",
> + "packed"
> + };
Consider using an enum for pixel_format_type may something as below:
typedef enum {
PIXEL_FORMAT_RGB,
PIXEL_FORMAT_PLANAR,
PIXEL_FORMAT_PACKED
} pixel_format_type;
This will also reduce string comparison in every loop, and may be "append"
and the subtest name generation logic could be reused in future.
> +
> + for (z = 0; z < ARRAY_SIZE(append); z++) {
> + snprintf(subtest_name, sizeof(subtest_name), "%s-%s",
> + scaler_with_pixel_format_tests[index].name,
> + append[z]);
> +
> + igt_subtest_with_dynamic(subtest_name) {
> + d->pixel_format_type = append[z];
> +
> + for_each_pipe(&d->display, pipe) {
> + igt_dynamic_f("pipe-%s",
> + kmstest_pipe_name(pipe)) {
> + for_each_valid_output_on_pipe(&d-
> >display,
> + pipe,
> + output) {
> + igt_info("Trying on %s\n",
> +
> igt_output_name(output));
> + if
> (!pipe_output_combo_valid(&d->display,
> +
> pipe, output))
> + continue;
> + if (get_num_scalers(&d-
> >display, pipe) < 1)
> + continue;
> +
> + ret =
> test_scaler_with_pixel_format_pipe(d,
> +
> 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));
> + }
> + igt_skip_on_f(ret == -ERANGE || ret ==
> -EINVAL,
> + "Unsupported scaling
> operation in driver with return value %s\n",
> + (ret == -EINVAL) ? "-
> EINVAL" : "-ERANGE");
> + }
> + }
> + }
> + }
> +}
> +
> static int opt_handler(int opt, int opt_index, void *_data) {
> data_t *data = _data;
> @@ -1365,32 +1448,7 @@ igt_main_args("", long_opts, help_str, opt_handler,
> &data)
>
> for (int index = 0; index <
> ARRAY_SIZE(scaler_with_pixel_format_tests); index++) {
>
> 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) {
> - 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));
> - }
> - igt_skip_on_f(ret == -ERANGE
> || ret == -EINVAL,
> - "Unsupported
> scaling operation in driver with return value %s\n",
> - (ret == -EINVAL) ?
> "-EINVAL" : "-ERANGE");
> - }
> - }
> - }
> + scaler_pixel_format(&data, index);
> }
>
> for (int index = 0; index <
> ARRAY_SIZE(scaler_with_rotation_tests); index++) {
> --
> 2.43.0
More information about the igt-dev
mailing list