[PATCH i-g-t v4 1/2] lib/igt_kms: Add support to retrieve async modifiers and formats
Borah, Chaitanya Kumar
chaitanya.kumar.borah at intel.com
Tue Mar 4 17:05:51 UTC 2025
Hello Santhosh,
> -----Original Message-----
> From: Reddy Guddati, Santhosh <santhosh.reddy.guddati at intel.com>
> Sent: Thursday, February 27, 2025 9:46 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 v4 1/2] lib/igt_kms: Add support to retrieve async
> modifiers and formats
>
> Parse "IN_FORMATS_ASYNC" plane property to identify supported format
> modifier pairs for async flips
>
> V2: Add new fields async_formats and reset idx.
>
> V3: Improve commit message , remove unused declaration (Chaitanya)
> Introduce structure to hold a format modifier and its
> associated format list
>
> Signed-off-by: Santhosh Reddy Guddati <santhosh.reddy.guddati at intel.com>
> ---
> lib/igt_kms.c | 57
> ++++++++++++++++++++++++++++++++++++++++++++++++++-
> lib/igt_kms.h | 17 ++++++++++++++-
> 2 files changed, 72 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c index cc3bb3ae7..c195d7b2d 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -701,6 +701,7 @@ const char * const
> igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> [IGT_PLANE_FB_DAMAGE_CLIPS] = "FB_DAMAGE_CLIPS",
> [IGT_PLANE_SCALING_FILTER] = "SCALING_FILTER",
> [IGT_PLANE_SIZE_HINTS] = "SIZE_HINTS",
> + [IGT_PLANE_IN_FORMATS_ASYNC] = "IN_FORMATS_ASYNC",
> };
>
> const char * const igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = { @@ -
> 5758,11 +5759,14 @@ static int igt_count_plane_format_mod(const struct
> drm_format_modifier_blob *blo
>
> static void igt_fill_plane_format_mod(igt_display_t *display, igt_plane_t
> *plane) {
> - const struct drm_format_modifier_blob *blob_data;
> + const struct drm_format_modifier_blob *blob_data,
> *async_blob_data;
> + const struct drm_format_modifier *async_modifiers;
> drmModePropertyBlobPtr blob;
> uint64_t blob_id;
> int idx = 0;
> int count;
> + int async_count, k;
> + const uint32_t *async_formats;
>
> if (!igt_plane_has_prop(plane, IGT_PLANE_IN_FORMATS)) {
> drmModePlanePtr p = plane->drm_plane; @@ -5822,6
> +5826,57 @@ static void igt_fill_plane_format_mod(igt_display_t *display,
> igt_plane_t *plane
> }
>
> igt_assert_eq(idx, plane->format_mod_count);
> +
> + if (!igt_plane_has_prop(plane, IGT_PLANE_IN_FORMATS_ASYNC))
> + return;
> +
> + blob_id = igt_plane_get_prop(plane,
> IGT_PLANE_IN_FORMATS_ASYNC);
> + blob = drmModeGetPropertyBlob(display->drm_fd, blob_id);
> +
> + if (!blob)
> + return;
> +
> + async_blob_data = (const struct drm_format_modifier_blob *)blob-
> >data;
> + async_count = igt_count_plane_format_mod(async_blob_data);
> +
> + if (!async_count)
> + return;
> +
> + plane->format_mod_pair = calloc(async_count, sizeof(plane-
> >format_mod_pair[0]));
> + igt_assert(plane->format_mod_pair);
> +
> + plane->async_format_mod_count = async_blob_data-
> >count_modifiers;
> + async_modifiers = modifiers_ptr(async_blob_data);
> + async_formats = formats_ptr(async_blob_data);
> +
> + for (int i = 0; i < async_blob_data->count_modifiers; i++) {
> + plane->format_mod_pair[i].modifier =
> async_modifiers[i].modifier;
> + plane->format_mod_pair[i].list.count = 0;
> +
> + // count the number of formats supported by this modifier
> + for (int j = 0; j < 64; j++) {
> + if (async_modifiers[i].formats & (1ULL << j))
> + plane->format_mod_pair[i].list.count++;
> + }
> +
> + // Allocate memory for the formats array
> + plane->format_mod_pair[i].list.formats = calloc(
> + plane-
> >format_mod_pair[i].list.count,
> + sizeof(plane-
> >format_mod_pair[i].list.formats[0])
> + );
> + igt_assert(plane->format_mod_pair[i].list.formats);
> +
> + k = 0;
> + //Fill the formats array
> + for (int j = 0; j < 64; j++) {
> + if (async_modifiers[i].formats & (1ULL << j)) {
> + plane->format_mod_pair[i].list.formats[k] =
> +
> async_formats[async_modifiers[i].offset + j];
> + k++;
> + }
> + }
> +
> + }
> }
>
> /**
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h index 27b545f52..111b61d7c 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -365,7 +365,8 @@ enum igt_atomic_plane_properties {
> IGT_PLANE_HOTSPOT_X,
> IGT_PLANE_HOTSPOT_Y,
> IGT_PLANE_SIZE_HINTS,
> - IGT_NUM_PLANE_PROPS
> + IGT_PLANE_IN_FORMATS_ASYNC,
> + IGT_NUM_PLANE_PROPS,
> };
>
> /**
> @@ -405,6 +406,16 @@ static inline bool
> igt_rotation_90_or_270(igt_rotation_t rotation)
> return rotation & (IGT_ROTATION_90 | IGT_ROTATION_270); }
>
> +typedef struct {
> + uint32_t *formats;
> + int count;
> +} async_format_list;
> +
> +typedef struct {
> + uint64_t modifier;
> + async_format_list list;
> +} igt_async_format_mod_t;
> +
> typedef struct igt_plane {
> /*< private >*/
> igt_pipe_t *pipe;
> @@ -438,8 +449,12 @@ typedef struct igt_plane {
> uint64_t *modifiers;
> uint32_t *formats;
> int format_mod_count;
> +
> + igt_async_format_mod_t *format_mod_pair;
> + int async_format_mod_count;
> } igt_plane_t;
>
While this should work, I think we should keep the implementation similar to the sync format modifier list.
You can have a look at the commit "lib: Parse plane IN_FORMATS blobifiers into a nicer form"[1] which implements it as
arrays of format-modifier tuples. (We can later think of implementing both as hash tables)
This should also simplify your loops in the second patch.
I also came across this commit "tests/kms_plane: Don't test all format+modifier combinations" [2] which tried
to reduce the time needed to execute the test by dividing the pixel formats into some "classes". While we have
a much smaller subset to test, it is still a good idea to profile the time taken to execute the test. If it exceeds the igt_runner
watchdog, we can implement something similar.
Also avoid "//" for comments.
Regards
Chaitanya
[1] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/commit/98f7614bd725afaae48f7b70d18329149075661b
[2] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/commit/1e7d15a65df0c1e21912e682b7cbc47d744b270b
> +
> /*
> * This struct represents a hardware pipe
> *
> --
> 2.34.1
More information about the igt-dev
mailing list