[PATCH i-g-t v6 2/2] tests/kms_async_flips: use in_formats_async for async modifiers
Borah, Chaitanya Kumar
chaitanya.kumar.borah at intel.com
Tue Mar 25 09:04:46 UTC 2025
> -----Original Message-----
> From: Reddy Guddati, Santhosh <santhosh.reddy.guddati at intel.com>
> Sent: Wednesday, March 19, 2025 10:59 AM
> To: igt-dev at lists.freedesktop.org
> Cc: Murthy, Arun R <arun.r.murthy at intel.com>; B S, Karthik
> <karthik.b.s at intel.com>; Borah, Chaitanya Kumar
> <chaitanya.kumar.borah at intel.com>; Reddy Guddati, Santhosh
> <santhosh.reddy.guddati at intel.com>
> Subject: [PATCH i-g-t v6 2/2] tests/kms_async_flips: use in_formats_async for
> async modifiers
>
> Utilise IN_FORMATS_ASYNC property exposed to get the list of async
> supported modifier/format pair and improve the test coverage by iterating
> through all the supported modifier format pairs.
>
> V2: Improve run_test_with_modifiers to set data formats based on async
> formats.
> Update make_fb to use data formats instead of hard coded format
>
> V3: Update commit message, remove complicated iterations (Chaitanya)
>
> V4: Reduce the format+modifier combinations to reduce time needed to
> execute the tests. (Chaitanya)
>
> V5: Remove unnecessary check for reduced formats. (Chaitanya)
>
> V6: Introduce new test for all formats and modifiers, executing only one
> flip to reduce overall time.
> Update test_with_modifiers to use async modifiers and formats X8888.
>
> Signed-off-by: Santhosh Reddy Guddati <santhosh.reddy.guddati at intel.com>
> ---
> tests/kms_async_flips.c | 116 ++++++++++++++++++++++++++++++++++++++-
> -
> 1 file changed, 112 insertions(+), 4 deletions(-)
>
> diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c index
> da426f753..165900e09 100644
> --- a/tests/kms_async_flips.c
> +++ b/tests/kms_async_flips.c
> @@ -36,6 +36,7 @@
> #include "igt.h"
> #include "igt_aux.h"
> #include "igt_psr.h"
> +#include "igt_vec.h"
> #include <sys/ioctl.h>
> #include <sys/time.h>
> #include <poll.h>
> @@ -85,6 +86,9 @@
> *
> * SUBTEST: async-flip-suspend-resume
> * Description: Verify the async flip functionality with suspend and resume
> cycle
> + *
> + * SUBTEST: async-flip-with-all-modifiers-formats
> + * Description: Verify the async flip functionality with all supported
> + modifiers and formats
> */
>
> #define CURSOR_POS 128
> @@ -122,8 +126,15 @@ typedef struct {
> bool allow_fail;
> struct buf_ops *bops;
> bool atomic_path;
> + unsigned int plane_format;
> + bool async_mod_formats;
> } data_t;
>
> +struct format_mod {
> + uint64_t modifier;
> + uint32_t format;
> +};
> +
> static void flip_handler(int fd_, unsigned int sequence, unsigned int tv_sec,
> unsigned int tv_usec, void *_data)
> {
> @@ -193,7 +204,7 @@ static void make_fb(data_t *data, struct igt_fb *fb,
>
> rec_width = width / (NUM_FBS * 2);
>
> - igt_create_color_fb(data->drm_fd, width, height,
> DRM_FORMAT_XRGB8888,
> + igt_create_color_fb(data->drm_fd, width, height, data->plane_format,
> data->modifier, 0.0, 0.0, 0.5, fb);
>
> cr = igt_get_cairo_ctx(data->drm_fd, fb); @@ -348,10 +359,19 @@
> static void test_async_flip(data_t *data)
> data->suspend_resume = false;
>
> igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> SUSPEND_TEST_NONE);
> }
> +
> + /* Reduce test execution for all formats and modifiers. One
> flip should suffice */
We will need to flip at least twice because Intel HW converts the first async to sync.
So we have to add data->async_mod_formats to the code that does the extra flip.
int flags = DRM_MODE_PAGE_FLIP_ASYNC | DRM_MODE_PAGE_FLIP_EVENT;
- if (data->alternate_sync_async) {
+ if (data->alternate_sync_async || data->async_mod_formats) {
flags &= ~DRM_MODE_PAGE_FLIP_ASYNC;
> + if (data->async_mod_formats) {
> + igt_assert_f(ret == 0, "Async flip failed with %s
> modifier and %s format",
> + igt_fb_modifier_name(data->modifier),
> + igt_format_str(data->plane_format));
> + break;
> + }
> +
> frame++;
> } while (diff.tv_sec < RUN_TIME);
>
> - if (!data->alternate_sync_async) {
> + if (!data->alternate_sync_async && !data->async_mod_formats) {
> fps = frame * 1000 / RUN_TIME;
> igt_assert_f((fps / 1000) > (data->refresh_rate *
> MIN_FLIPS_PER_FRAME),
> "FPS should be significantly higher than the refresh
> rate\n"); @@ -710,17 +730,99 @@ static void run_test(data_t *data, void
> (*test)(data_t *))
> }
> }
>
> +static bool skip_async_format_mod(data_t *data,
> + uint32_t format, uint64_t modifier,
> + struct igt_vec *tested_formats)
> +{
> + /* igt doesn't know how to sw generate UBWC: */
> + if (is_msm_device(data->drm_fd) &&
> + modifier == DRM_FORMAT_MOD_QCOM_COMPRESSED)
> + return true;
> +
> + /* VEBOX just hangs with an actual 10bpc format */
> + if (igt_fb_is_gen12_mc_ccs_modifier(modifier) &&
> + igt_reduce_format(format) == DRM_FORMAT_XRGB2101010)
> + return true;
> +
> + /* test each format "class" only once in non-extended tests */
> + struct format_mod rf = {
> + .format = igt_reduce_format(format),
> + .modifier = modifier,
> + };
> +
> + if (igt_vec_index(tested_formats, &rf) >= 0)
> + return true;
> +
> + igt_vec_push(tested_formats, &rf);
> +
> + return false;
> +}
> +
> +static void run_test_with_async_format_modifiers(data_t *data, void
> +(*test)(data_t *)) {
> + struct igt_vec tested_formats;
> +
> + igt_vec_init(&tested_formats, sizeof(struct format_mod));
> +
> + for_each_pipe_with_valid_output(&data->display, data->pipe, data-
> >output) {
> + test_init(data);
> + for (int i = 0; i < data->plane->async_format_mod_count; i++) {
> + struct format_mod f = {
> + .format = data->plane->async_formats[i],
> + .modifier = data->plane->async_modifiers[i],
> + };
> +
> + if (skip_async_format_mod(data, f.format, f.modifier,
> &tested_formats)) {
> + igt_debug("Skipping format "
> IGT_FORMAT_FMT " / modifier "
> + IGT_MODIFIER_FMT " on %s.%u\n",
> + IGT_FORMAT_ARGS(f.format),
> + IGT_MODIFIER_ARGS(f.modifier),
> + kmstest_pipe_name(data->pipe),
> + data->plane->index);
> + continue;
> + }
> +
> + /* C8 format is only supported with legacy LUT,
> skipping it */
> + if (f.format == DRM_FORMAT_C8)
> + continue;
We can remove this as kernel is not advertising it anymore.
https://patchwork.freedesktop.org/patch/643629/?series=140935&rev=9
> +
> + data->allow_fail = true;
We should not need this anymore as we have the exact list of modifiers we support.
Worthy checking if we need this for other tests too.
> + data->modifier = f.modifier;
> + data->plane_format = f.format;
> + data->async_mod_formats = true;
> +
> + igt_dynamic_f("pipe-%s-%s-%s-%s",
> kmstest_pipe_name(data->pipe),
> + data->output->name,
> + igt_fb_modifier_name(data->modifier),
> + igt_format_str(data->plane_format)) {
> + /*
> + * FIXME: joiner+async flip is busted
> currently in KMD.
> + * Remove this check once the issues are
> fixed in KMD.
> + */
> + igt_skip_on_f(is_joiner_mode(data-
> >drm_fd,
> + data-
> >output),
> + "Skipping, async flip not
> supported "
> + "on joiner mode\n");
> + test_init_fbs(data);
> + test(data);
> + }
> + }
> + }
> +
> + igt_vec_fini(&tested_formats);
> +}
> +
> static void run_test_with_modifiers(data_t *data, void (*test)(data_t *)) {
> for_each_pipe_with_valid_output(&data->display, data->pipe, data-
> >output) {
> test_init(data);
>
> - for (int i = 0; i < data->plane->format_mod_count; i++) {
> + for (int i = 0; i < data->plane-> async_format_mod_count; i++) {
> if (data->plane->formats[i] !=
> DRM_FORMAT_XRGB8888)
> continue;
>
> data->allow_fail = true;
> - data->modifier = data->plane->modifiers[i];
> + data->modifier = data->plane->async_modifiers[i];
>
> igt_dynamic_f("pipe-%s-%s-%s",
> kmstest_pipe_name(data->pipe),
> data->output->name,
> @@ -757,6 +859,7 @@ igt_main
>
> if (is_intel_device(data.drm_fd))
> data.bops = buf_ops_create(data.drm_fd);
> + data.plane_format = DRM_FORMAT_XRGB8888;
> }
>
> igt_describe("Verify the async flip functionality and the fps during
> async flips"); @@ -896,6 +999,11 @@ igt_main
> run_test(&data, test_async_flip);
> }
>
> + igt_describe("Verify async flip with all supported modifier and format
> combinations");
> + igt_subtest_with_dynamic("async-flip-with-all-modifiers-formats") {
> + run_test_with_async_format_modifiers(&data,
> test_async_flip);
> + }
> +
> igt_fixture {
> for (i = 0; i < NUM_FBS; i++)
> igt_remove_fb(data.drm_fd, &data.bufs[i]);
Now to some over-all design. First of all, let's separate out the changes affecting existing tests and the ones that adds a new test.
In addition to that, since we expect this property available for all intel devices, we should trigger a failure if the property is not present.
Also, we don't really have a fallback if the variables like async_format_mod_count are not initialized.
Regards
Chaitanya
> --
> 2.34.1
More information about the igt-dev
mailing list