Re: [PATCH RFC v2 06/20] drm: Introduce DRM_CLIENT_CAP_POST_BLEND_COLOR_PIPELINE

[Harry Wentland <harry.wentland@amd.com>]

On 2025-09-17 20:43, Nícolas F. R. A. Prado wrote:
> Introduce DRM_CLIENT_CAP_POST_BLEND_COLOR_PIPELINE which a DRM client
> can set to enable the usage of post-blend color pipelines instead of the
> now deprecated CRTC color management properties: "GAMMA_LUT",
> "DEGAMMA_LUT" and "CTM".
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>   drivers/gpu/drm/drm_atomic_uapi.c   | 15 +++++++++++++++
>   drivers/gpu/drm/drm_connector.c     |  1 +
>   drivers/gpu/drm/drm_crtc_internal.h |  1 +
>   drivers/gpu/drm/drm_ioctl.c         |  9 +++++++++
>   drivers/gpu/drm/drm_mode_object.c   |  9 +++++++++
>   include/drm/drm_file.h              |  7 +++++++
>   include/uapi/drm/drm.h              | 19 +++++++++++++++++++
>   7 files changed, 61 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 063c142fd9b656e228cfc660d005a3fbb4640d32..f5125fa3fa28ff2a6ff07fd7cf07d4bdf77ab738 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -452,6 +452,11 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>   	} else if (property == config->prop_vrr_enabled) {
>   		state->vrr_enabled = val;
>   	} else if (property == config->degamma_lut_property) {
> +		if (file_priv->post_blend_color_pipeline) {
> +			drm_dbg_atomic(dev,
> +				"Setting DEGAMMA_LUT CRTC property not permitted with DRM_CLIENT_CAP_POST_BLEND_COLOR_PIPELINE client cap\n");
> +			return -EINVAL;
> +		}
>   		ret = drm_property_replace_blob_from_id(dev,
>   					&state->degamma_lut,
>   					val,
> @@ -460,6 +465,11 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>   		state->color_mgmt_changed |= replaced;
>   		return ret;
>   	} else if (property == config->ctm_property) {
> +		if (file_priv->post_blend_color_pipeline) {
> +			drm_dbg_atomic(dev,
> +				"Setting CTM CRTC property not permitted with DRM_CLIENT_CAP_POST_BLEND_COLOR_PIPELINE client cap\n");
> +			return -EINVAL;
> +		}
>   		ret = drm_property_replace_blob_from_id(dev,
>   					&state->ctm,
>   					val,
> @@ -468,6 +478,11 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>   		state->color_mgmt_changed |= replaced;
>   		return ret;
>   	} else if (property == config->gamma_lut_property) {
> +		if (file_priv->post_blend_color_pipeline) {
> +			drm_dbg_atomic(dev,
> +				"Setting GAMMA_LUT CRTC property not permitted with DRM_CLIENT_CAP_POST_BLEND_COLOR_PIPELINE client cap\n");
> +			return -EINVAL;
> +		}
>   		ret = drm_property_replace_blob_from_id(dev,
>   					&state->gamma_lut,
>   					val,
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4d6dc9ebfdb5bc730b1aff7a184448af7b93f078..f58cfd2131139ff3e613adc4dbb9ddbedf724dc7 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -3440,6 +3440,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>   	 */
>   	ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic,
>   			file_priv->plane_color_pipeline,
> +			file_priv->post_blend_color_pipeline,
>   			(uint32_t __user *)(unsigned long)(out_resp->props_ptr),
>   			(uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr),
>   			&out_resp->count_props);
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index e3dbdcbfa385b940ec0b5476adde6146fe4afde1..c53f154e5392a10c326c844b7321666275f9ac02 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -169,6 +169,7 @@ void drm_mode_object_unregister(struct drm_device *dev,
>   				struct drm_mode_object *object);
>   int drm_mode_object_get_properties(struct drm_mode_object *obj, bool atomic,
>   				   bool plane_color_pipeline,
> +				   bool post_blend_color_pipeline,
>   				   uint32_t __user *prop_ptr,
>   				   uint64_t __user *prop_values,
>   				   uint32_t *arg_count_props);
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 01592d10e3465ddceddef94bc417f98d3ec12087..ea9600f5392f520a2b42ba7ef363d2f08ce19812 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -383,6 +383,15 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
>   			return -EINVAL;
>   		file_priv->plane_color_pipeline = req->value;
>   		break;
> +	case DRM_CLIENT_CAP_POST_BLEND_COLOR_PIPELINE:
> +		if (!file_priv->atomic)
> +			return -EINVAL;
> +		if (req->value > 1)
> +			return -EINVAL;
> +		if (!drm_core_check_feature(dev, DRIVER_POST_BLEND_COLOR_PIPELINE))
> +			return -EINVAL;
> +		file_priv->post_blend_color_pipeline = req->value;
> +		break;
>   	default:
>   		return -EINVAL;
>   	}
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index b45d501b10c868c6d9b7a5a8760eadbd7b372a6a..5e6c3de9456b997985142a68b9cef57771a58bdc 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -388,6 +388,7 @@ EXPORT_SYMBOL(drm_object_property_get_default_value);
>   /* helper for getconnector and getproperties ioctls */
>   int drm_mode_object_get_properties(struct drm_mode_object *obj, bool atomic,
>   				   bool plane_color_pipeline,
> +				   bool post_blend_color_pipeline,
>   				   uint32_t __user *prop_ptr,
>   				   uint64_t __user *prop_values,
>   				   uint32_t *arg_count_props)
> @@ -416,6 +417,13 @@ int drm_mode_object_get_properties(struct drm_mode_object *obj, bool atomic,
>   				continue;
>   		}
>   
> +		if (!post_blend_color_pipeline && obj->type == DRM_MODE_OBJECT_CRTC) {
> +			struct drm_crtc *crtc = obj_to_crtc(obj);
> +
> +			if (prop == crtc->color_pipeline_property)
> +				continue;
> +		}
> +
>   		if (*arg_count_props > count) {
>   			ret = __drm_object_property_get_value(obj, prop, &val);
>   			if (ret)
> @@ -475,6 +483,7 @@ int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data,
>   
>   	ret = drm_mode_object_get_properties(obj, file_priv->atomic,
>   			file_priv->plane_color_pipeline,
> +			file_priv->post_blend_color_pipeline,
>   			(uint32_t __user *)(unsigned long)(arg->props_ptr),
>   			(uint64_t __user *)(unsigned long)(arg->prop_values_ptr),
>   			&arg->count_props);
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index 1a3018e4a537b3341acb50187d47371f6b781b9d..42b9a43baa18079af8ec2ea5b1484b23c497beb0 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -213,6 +213,13 @@ struct drm_file {
>   	 */
>   	bool plane_color_pipeline;
>   
> +	/**
> +	 * @post_blend_color_pipeline:
> +	 *
> +	 * True if client understands post-blend color pipelines
> +	 */
> +	bool post_blend_color_pipeline;
> +
>   	/**
>   	 * @was_master:
>   	 *
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index c6c53e57958e951204154ce41a69696a6876f0e8..f9ac10b3e4876f71005a87dedefa4aed320566f0 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -927,6 +927,25 @@ struct drm_get_cap {
>    */
>   #define DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE	7
>   
> +/**
> + * DRM_CLIENT_CAP_POST_BLEND_COLOR_PIPELINE

DRM_CLIENT_CAP_CRTC_COLOR_PIPELINE might be better to align
terminology between pre- and post-blend pipelines. It would
also make it clear that this is about the color pipeline on
a drm_crtc (which pretty much by definition means post-
blending).

Harry

> + *
> + * If set to 1 the DRM core will allow setting the COLOR_PIPELINE
> + * property on a &drm_crtc, as well as drm_colorop properties.
> + *
> + * Setting of these crtc properties will be rejected when this client
> + * cap is set:
> + * - GAMMA_LUT
> + * - DEGAMMA_LUT
> + * - CTM
> + *
> + * The client must enable &DRM_CLIENT_CAP_ATOMIC first.
> + *
> + * This client cap can only be set if the driver sets the corresponding driver
> + * cap &DRM_CAP_POST_BLEND_COLOR_PIPELINE.
> + */
> +#define DRM_CLIENT_CAP_POST_BLEND_COLOR_PIPELINE	8
> +
>   /* DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
>   struct drm_set_client_cap {
>   	__u64 capability;
>