[PATCH V8 04/36] lib/igt_kms: Introduce DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE
Borah, Chaitanya Kumar
chaitanya.kumar.borah at intel.com
Mon Jun 9 09:25:06 UTC 2025
Hi Alex,
+ Swati
> -----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Alex
> Hung
> Sent: Wednesday, April 30, 2025 7:04 AM
> To: igt-dev at lists.freedesktop.org
> Cc: alex.hung at amd.com; harry.wentland at amd.com
> Subject: [PATCH V8 04/36] lib/igt_kms: Introduce
> DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE
>
> From: Harry Wentland <harry.wentland at amd.com>
>
> Setting the COLOR_PIPELINE plane prop is only allowed when the
> DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set.
>
> Setting COLOR_RANGE and COLOR_ENCODING is only allowed when the
> DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is NOT set.
>
> v6:
> - Ignore COLOR_RANGE and COLOR_ENCODING plane properties
> when DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set
>
> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
> ---
> include/drm-uapi/drm.h | 15 +++++++++++++++
> lib/igt_kms.c | 32 ++++++++++++++++++++++++++++++++
> lib/igt_kms.h | 1 +
> tests/kms_properties.c | 22 ++++++++++++++++------
> 4 files changed, 64 insertions(+), 6 deletions(-)
>
> diff --git a/include/drm-uapi/drm.h b/include/drm-uapi/drm.h index
> 4e4f7c2c3..6b9af3348 100644
> --- a/include/drm-uapi/drm.h
> +++ b/include/drm-uapi/drm.h
> @@ -869,6 +869,21 @@ struct drm_get_cap {
> */
> #define DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT 6
>
> +/**
> + * DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE
> + *
> + * If set to 1, the DRM core will
> + * - expose plane COLOR_PIPELINE properties for pre-blending color
> management.
> + * - reject setting of these plane properties:
> + * - COLOR_ENCODING
> + * - COLOR_RANGE
> + *
> + * The client must enable &DRM_CLIENT_CAP_ATOMIC first.
> + *
> + * This capability is currently in development.
> + */
> +#define DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE 7
> +
> /* DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */ struct
> drm_set_client_cap {
> __u64 capability;
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 9b817c98f..9ebe22d1b 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -2981,6 +2981,9 @@ void igt_display_require(igt_display_t *display, int
> drm_fd)
> if (drmSetClientCap(drm_fd,
> LOCAL_DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT, 1) == 0)
> display->has_virt_cursor_plane = 1;
>
> + if (drmSetClientCap(drm_fd,
> DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE, 1) == 0)
> + display->has_plane_color_pipeline = 1;
> +
> plane_resources = drmModeGetPlaneResources(display->drm_fd);
> igt_assert(plane_resources);
>
> @@ -3608,6 +3611,29 @@ static uint32_t igt_plane_get_fb_id(igt_plane_t
> *plane)
> igt_assert_eq(r, 0); \
> }
>
> +static bool
> +igt_atomic_ignore_plane_prop(igt_pipe_t *pipe, uint32_t prop) {
> + igt_display_t *display = pipe->display;
> +
> + if (display->has_plane_color_pipeline) {
> + switch(prop) {
> + case IGT_PLANE_COLOR_ENCODING:
> + case IGT_PLANE_COLOR_RANGE:
> + return true;
> + default:
> + return false;
> + }
> + } else {
> + switch(prop) {
> + default:
> + return false;
> + }
> + }
> +
> + return false;
> +}
> +
> /*
> * Add position and fb changes of a plane to the atomic property set
> */
> @@ -3627,6 +3653,12 @@ igt_atomic_prepare_plane_commit(igt_plane_t
> *plane, igt_pipe_t *pipe,
> igt_plane_get_fb_id(plane));
>
> for (i = 0; i < IGT_NUM_PLANE_PROPS; i++) {
> + if (igt_atomic_ignore_plane_prop(pipe, i)) {
> + igt_debug("plane %s.%d: Ignoring property \"%s\" to
> 0x%"PRIx64"/%"PRIi64"\n",
> + kmstest_pipe_name(pipe->pipe), plane-
> >index, igt_plane_prop_names[i],
> + plane->values[i], plane->values[i]);
> continue;
> + }
> +
> if (!igt_plane_is_prop_changed(plane, i))
> continue;
>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h index 9b5a940f7..7531ae53b 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -503,6 +503,7 @@ struct igt_display {
> bool has_cursor_plane;
> bool is_atomic;
> bool has_virt_cursor_plane;
> + bool has_plane_color_pipeline;
> bool first_commit;
>
> uint64_t *modifiers;
> diff --git a/tests/kms_properties.c b/tests/kms_properties.c index
> 408e23578..01ec3bedc 100644
> --- a/tests/kms_properties.c
> +++ b/tests/kms_properties.c
> @@ -101,7 +101,7 @@ static void cleanup_pipe(igt_display_t *display, enum
> pipe pipe, igt_output_t *o }
>
> static bool ignore_property(uint32_t obj_type, uint32_t prop_flags,
> - const char *name, bool atomic)
> + const char *name, bool atomic, bool
> has_color_pipeline)
> {
> if (prop_flags & DRM_MODE_PROP_IMMUTABLE)
> return true;
> @@ -111,6 +111,16 @@ static bool ignore_property(uint32_t obj_type,
> uint32_t prop_flags,
> if (atomic && !strcmp(name, "DPMS"))
> return true;
> break;
> + case DRM_MODE_OBJECT_PLANE:
> + if (has_color_pipeline && !strcmp(name, "COLOR_RANGE")) {
> + printf("hwhw: ignoring COLOR_RANGE\n");
> + return true;
> + }
> + if (has_color_pipeline && !strcmp(name,
> "COLOR_ENCODING")) {
> + printf("hwhw: ignoring COLOR_ENCODING\n");
> + return true;
> + }
> + break;
> default:
> break;
> }
This is not enough for the legacy commit path to work. Further changes are needed[1].
That got me thinking. Should we set this capability within the colorop tests instead of enabling it globally?
Regards
Chaitanya
[1]
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 487906453..707d9b89e 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -2664,6 +2664,8 @@ static int get_drm_plane_type(int drm_fd, uint32_t plane_id)
static void igt_plane_reset(igt_plane_t *plane)
{
+ igt_display_t *display = plane->pipe->display;
+
/* Reset src coordinates. */
igt_plane_set_prop_value(plane, IGT_PLANE_SRC_X, 0);
igt_plane_set_prop_value(plane, IGT_PLANE_SRC_Y, 0);
@@ -2680,11 +2682,13 @@ static void igt_plane_reset(igt_plane_t *plane)
igt_plane_set_prop_value(plane, IGT_PLANE_FB_ID, 0);
igt_plane_set_prop_value(plane, IGT_PLANE_CRTC_ID, 0);
- if (igt_plane_has_prop(plane, IGT_PLANE_COLOR_ENCODING))
+ if (!display->has_plane_color_pipeline &&
+ igt_plane_has_prop(plane, IGT_PLANE_COLOR_ENCODING))
igt_plane_set_prop_enum(plane, IGT_PLANE_COLOR_ENCODING,
igt_color_encoding_to_str(IGT_COLOR_YCBCR_BT601));
- if (igt_plane_has_prop(plane, IGT_PLANE_COLOR_RANGE))
+ if (!display->has_plane_color_pipeline &&
+ igt_plane_has_prop(plane, IGT_PLANE_COLOR_RANGE))
igt_plane_set_prop_enum(plane, IGT_PLANE_COLOR_RANGE,
igt_color_range_to_str(IGT_COLOR_YCBCR_LIMITED_RANGE));
@@ -5549,10 +5553,12 @@ void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb)
igt_fb_set_position(fb, plane, 0, 0);
igt_fb_set_size(fb, plane, fb->width, fb->height);
- if (igt_plane_has_prop(plane, IGT_PLANE_COLOR_ENCODING))
+ if (!display->has_plane_color_pipeline &&
+ igt_plane_has_prop(plane, IGT_PLANE_COLOR_ENCODING))
igt_plane_set_prop_enum(plane, IGT_PLANE_COLOR_ENCODING,
igt_color_encoding_to_str(fb->color_encoding));
- if (igt_plane_has_prop(plane, IGT_PLANE_COLOR_RANGE))
+ if (!display->has_plane_color_pipeline &&
+ igt_plane_has_prop(plane, IGT_PLANE_COLOR_RANGE))
igt_plane_set_prop_enum(plane, IGT_PLANE_COLOR_RANGE,
igt_color_range_to_str(fb->color_range));
> @@ -164,7 +174,7 @@ static bool has_additional_test_lookup(uint32_t
> obj_type, const char *name,
>
> return false;
> }
> -static void test_properties(int fd, uint32_t type, uint32_t id, bool atomic)
> +static void test_properties(int fd, uint32_t type, uint32_t id, bool
> +atomic, bool has_color_pipeline)
> {
> drmModeObjectPropertiesPtr props =
> drmModeObjectGetProperties(fd, id, type); @@ -183,7
> +193,7 @@ static void test_properties(int fd, uint32_t type, uint32_t id, bool
> atomic)
>
> igt_assert(prop);
>
> - if (ignore_property(type, prop->flags, prop->name, atomic)) {
> + if (ignore_property(type, prop->flags, prop->name, atomic,
> +has_color_pipeline)) {
> igt_debug("Ignoring property \"%s\"\n", prop-
> >name);
>
> continue;
> @@ -230,7 +240,7 @@ static void run_plane_property_tests(igt_display_t
> *display, enum pipe pipe, igt
> igt_info("Testing plane properties on %s.#%d-%s (output:
> %s)\n",
> kmstest_pipe_name(pipe), plane->index,
> kmstest_plane_type_name(plane->type), output->name);
>
> - test_properties(display->drm_fd,
> DRM_MODE_OBJECT_PLANE, plane->drm_plane->plane_id, atomic);
> + test_properties(display->drm_fd,
> DRM_MODE_OBJECT_PLANE,
> +plane->drm_plane->plane_id, atomic, display->has_plane_color_pipeline);
> }
>
> cleanup_pipe(display, pipe, output, &fb); @@ -244,7 +254,7 @@
> static void run_crtc_property_tests(igt_display_t *display, enum pipe pipe,
> igt_
>
> igt_info("Testing crtc properties on %s (output: %s)\n",
> kmstest_pipe_name(pipe), output->name);
>
> - test_properties(display->drm_fd, DRM_MODE_OBJECT_CRTC, display-
> >pipes[pipe].crtc_id, atomic);
> + test_properties(display->drm_fd, DRM_MODE_OBJECT_CRTC,
> +display->pipes[pipe].crtc_id, atomic, false);
>
> cleanup_pipe(display, pipe, output, &fb); } @@ -258,7 +268,7 @@
> static void run_connector_property_tests(igt_display_t *display, enum pipe
> pipe,
>
> igt_info("Testing connector properties on output %s (pipe: %s)\n",
> output->name, kmstest_pipe_name(pipe));
>
> - test_properties(display->drm_fd, DRM_MODE_OBJECT_CONNECTOR,
> output->id, atomic);
> + test_properties(display->drm_fd, DRM_MODE_OBJECT_CONNECTOR,
> +output->id, atomic, false);
>
> if (pipe != PIPE_NONE)
> cleanup_pipe(display, pipe, output, &fb);
> --
> 2.43.0
More information about the igt-dev
mailing list