On 9/15/25 12:43, Nícolas F. R. A. Prado wrote:
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
Thanks for this. Let me go through the thread and come back.
+
+ 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.
