On Thu, 2025-08-14 at 21:50 -0600, Alex Hung wrote:
> From: Harry Wentland <harry.wentl...@amd.com>
>
> With the introduction of the pre-blending color pipeline we
> can no longer have color operations that don't have a clear
> position in the color pipeline. We deprecate all existing
> plane properties. For upstream drivers those are:
> - COLOR_ENCODING
> - COLOR_RANGE
>
> Drivers are expected to ignore these properties when
> programming the HW. DRM clients that register with
> DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE will not be allowed to
> set the COLOR_ENCODING and COLOR_RANGE properties.
>
> Setting of the COLOR_PIPELINE plane property or drm_colorop
> properties is only allowed for userspace that sets this
> client cap.
>
> Reviewed-by: Simon Ser <cont...@emersion.fr>
> Signed-off-by: Alex Hung <alex.h...@amd.com>
> Signed-off-by: Harry Wentland <harry.wentl...@amd.com>
> Reviewed-by: Daniel Stone <dani...@collabora.com>
> Reviewed-by: Melissa Wen <m...@igalia.com>
> ---
> v11
> - Skip color_encoding/range_property in
> drm_mode_object_get_properties
> when plane_color_pipeline is present (Harry Wentland)
>
> V9:
> - Fix typo in commit description (Shengyu Qu)
>
> v8:
> - Disallow setting of COLOR_RANGE and COLOR_ENCODING when
> DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set
>
> v5:
> - Fix kernel docs
>
> v4:
> - Don't block setting of COLOR_RANGE and COLOR_ENCODING
> when client cap is set
>
> drivers/gpu/drm/drm_connector.c | 1 +
> drivers/gpu/drm/drm_crtc_internal.h | 1 +
> drivers/gpu/drm/drm_ioctl.c | 7 +++++++
> drivers/gpu/drm/drm_mode_object.c | 18 ++++++++++++++++++
> include/drm/drm_file.h | 7 +++++++
> include/uapi/drm/drm.h | 15 +++++++++++++++
> 6 files changed, 49 insertions(+)
[..]
> @@ -399,6 +401,21 @@ int drm_mode_object_get_properties(struct
> drm_mode_object *obj, bool atomic,
> if ((prop->flags & DRM_MODE_PROP_ATOMIC) && !atomic)
> continue;
>
> + if (plane_color_pipeline && obj->type ==
> DRM_MODE_OBJECT_PLANE) {
> + struct drm_plane *plane = obj_to_plane(obj);
> +
> + if (prop == plane->color_encoding_property
> ||
> + prop == plane->color_range_property)
> + continue;
> + }
A feedback that came up when discussing post-blend colorops [1] which
is also applicable here, is that there should be a driver cap in
addition to the client cap, and that the client cap should only be
possible to set if the driver cap is set. Without that, if the driver
doesn't support color pipelines, the client will effectively and
unintentionally disable color operations without any replacement when
setting the client cap.
Another suggestion was to keep the deprecated properties (in this case
COLOR_RANGE and COLOR_ENCODING) available but read-only so that
drm_info can display them and so that graceful handover from colorop-
unaware to colorop-aware clients can happen.
[1]
https://lore.kernel.org/dri-devel/20250822-mtk-post-blend-color-pipeline-v1-0-a9446d4ac...@collabora.com/T/#m830e2f87ca82b1f8da7377d0c55c557fb070c2dd
> +
> + if (!plane_color_pipeline && obj->type ==
> DRM_MODE_OBJECT_PLANE) {
> + struct drm_plane *plane = obj_to_plane(obj);
> +
> + if (prop == plane->color_pipeline_property)
> + continue;
> + }
> +
> if (*arg_count_props > count) {
> ret = __drm_object_property_get_value(obj,
> prop, &val);
> if (ret)
>
[..]
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 3cd5cf15e3c9..27cc159c1d27 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -906,6 +906,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 allow setting the COLOR_PIPELINE
> + * property on a &drm_plane, as well as drm_colorop properties.
> + *
> + * Setting of these plane properties will be rejected when this
> client
> + * cap is set:
> + * - COLOR_ENCODING
> + * - COLOR_RANGE
> + *
> + * The client must enable &DRM_CLIENT_CAP_ATOMIC first.
> + */
> +#define DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE 7
> +
> /* DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
> struct drm_set_client_cap {
> __u64 capability;
One other thing pointed out was that these deprecated properties are
not actually rejected, but simply unlisted in the current
implementation, contrary to the documentation above. But if we do make
them read-only we'll need to revert back to the implementation on the
previous version, and then the documentation can stay as is.
--
Thanks,
Nícolas
