On Wed, 8 Nov 2023 11:36:32 -0500
Harry Wentland <harry.wentland@xxxxxxx> wrote:
> We're adding a new enum COLOR PIPELINE property. This
> property will have entries for each COLOR PIPELINE by
> referencing the DRM object ID of the first drm_colorop
> of the pipeline. 0 disables the entire COLOR PIPELINE.
I didn't find the call that actually creates that property, where is it?
> Userspace can use this to discover the available color
> pipelines, as well as set the desired one. The color
> pipelines are programmed via properties on the actual
> drm_colorop objects.
>
> Signed-off-by: Harry Wentland <harry.wentland@xxxxxxx>
> ---
> drivers/gpu/drm/drm_atomic.c | 46 +++++++++++++++++++++++
> drivers/gpu/drm/drm_atomic_state_helper.c | 5 +++
> drivers/gpu/drm/drm_atomic_uapi.c | 44 ++++++++++++++++++++++
> include/drm/drm_atomic_uapi.h | 2 +
> include/drm/drm_plane.h | 8 ++++
> 5 files changed, 105 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index ccf26b034433..cf3cb6d1239f 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1463,6 +1463,52 @@ drm_atomic_add_affected_planes(struct drm_atomic_state *state,
> }
> EXPORT_SYMBOL(drm_atomic_add_affected_planes);
>
> +/**
> + * drm_atomic_add_affected_colorops - add colorops for plane
> + * @state: atomic state
> + * @plane: DRM plane
> + *
> + * This function walks the current configuration and adds all colorops
> + * currently used by @plane to the atomic configuration @state. This is useful
> + * when an atomic commit also needs to check all currently enabled colorop on
> + * @plane, e.g. when changing the mode. It's also useful when re-enabling a plane
> + * to avoid special code to force-enable all colorops.
> + *
> + * Since acquiring a colorop state will always also acquire the w/w mutex of the
> + * current plane for that colorop (if there is any) adding all the colorop states for
> + * a plane will not reduce parallelism of atomic updates.
> + *
> + * Returns:
> + * 0 on success or can fail with -EDEADLK or -ENOMEM. When the error is EDEADLK
> + * then the w/w mutex code has detected a deadlock and the entire atomic
> + * sequence must be restarted. All other errors are fatal.
> + */
> +int
> +drm_atomic_add_affected_colorops(struct drm_atomic_state *state,
> + struct drm_plane *plane)
> +{
> + struct drm_colorop *colorop;
> + struct drm_colorop_state *colorop_state;
> +
> + WARN_ON(!drm_atomic_get_new_plane_state(state, plane));
> +
> + drm_dbg_atomic(plane->dev,
> + "Adding all current colorops for [plane:%d:%s] to %p\n",
> + plane->base.id, plane->name, state);
> +
> + drm_for_each_colorop(colorop, plane->dev) {
> + if (colorop->plane != plane)
> + continue;
> +
> + colorop_state = drm_atomic_get_colorop_state(state, colorop);
> + if (IS_ERR(colorop_state))
> + return PTR_ERR(colorop_state);
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_add_affected_colorops);
> +
> /**
> * drm_atomic_check_only - check whether a given config would work
> * @state: atomic configuration to check
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 784e63d70a42..3c5f2c8e33d0 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -267,6 +267,11 @@ void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state,
> plane_state->color_range = val;
> }
>
> + if (plane->color_pipeline_property) {
> + /* default is always NULL, i.e., bypass */
> + plane_state->color_pipeline = NULL;
> + }
> +
> if (plane->zpos_property) {
> if (!drm_object_property_get_default_value(&plane->base,
> plane->zpos_property,
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index a8f7a8a6639a..c6629fdaa114 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -256,6 +256,38 @@ drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state,
> }
> EXPORT_SYMBOL(drm_atomic_set_fb_for_plane);
>
> +
> +/**
> + * drm_atomic_set_colorop_for_plane - set colorop for plane
> + * @plane_state: atomic state object for the plane
> + * @colorop: colorop to use for the plane
> + *
> + * Changing the assigned framebuffer for a plane requires us to grab a reference
> + * to the new fb and drop the reference to the old fb, if there is one. This
> + * function takes care of all these details besides updating the pointer in the
> + * state object itself.
This paragraph does not seem to talk about this function.
> + */
> +void
> +drm_atomic_set_colorop_for_plane(struct drm_plane_state *plane_state,
> + struct drm_colorop *colorop)
> +{
> + struct drm_plane *plane = plane_state->plane;
> +
> + if (colorop)
> + drm_dbg_atomic(plane->dev,
> + "Set [COLOROP:%d] for [PLANE:%d:%s] state %p\n",
> + colorop->base.id, plane->base.id, plane->name,
> + plane_state);
> + else
> + drm_dbg_atomic(plane->dev,
> + "Set [NOCOLOROP] for [PLANE:%d:%s] state %p\n",
> + plane->base.id, plane->name, plane_state);
> +
> + plane_state->color_pipeline = colorop;
> +}
> +EXPORT_SYMBOL(drm_atomic_set_colorop_for_plane);
> +
> +
> /**
> * drm_atomic_set_crtc_for_connector - set CRTC for connector
> * @conn_state: atomic state object for the connector
> @@ -581,6 +613,16 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
> state->color_encoding = val;
> } else if (property == plane->color_range_property) {
> state->color_range = val;
> + } else if (property == plane->color_pipeline_property) {
> + /* find DRM colorop object */
> + struct drm_colorop *colorop = NULL;
> + colorop = drm_colorop_find(dev, file_priv, val);
> +
> + if (val && !colorop)
> + return -EACCES;
> +
> + /* set it on drm_plane_state */
> + drm_atomic_set_colorop_for_plane(state, colorop);
> } else if (property == config->prop_fb_damage_clips) {
> ret = drm_atomic_replace_property_blob_from_id(dev,
> &state->fb_damage_clips,
> @@ -647,6 +689,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> *val = state->color_encoding;
> } else if (property == plane->color_range_property) {
> *val = state->color_range;
> + } else if (property == plane->color_pipeline_property) {
> + *val = (state->color_pipeline) ? state->color_pipeline->base.id : 0;
> } else if (property == config->prop_fb_damage_clips) {
> *val = (state->fb_damage_clips) ?
> state->fb_damage_clips->base.id : 0;
> diff --git a/include/drm/drm_atomic_uapi.h b/include/drm/drm_atomic_uapi.h
> index 70a115d523cd..436315523326 100644
> --- a/include/drm/drm_atomic_uapi.h
> +++ b/include/drm/drm_atomic_uapi.h
> @@ -50,6 +50,8 @@ drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state,
> struct drm_crtc *crtc);
> void drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state,
> struct drm_framebuffer *fb);
> +void drm_atomic_set_colorop_for_plane(struct drm_plane_state *plane_state,
> + struct drm_colorop *colorop);
> int __must_check
> drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
> struct drm_crtc *crtc);
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 57bbd0cd73a9..e65074f266c0 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -745,6 +745,14 @@ struct drm_plane {
> */
> struct drm_property *color_range_property;
>
> + /**
> + * @color_pipeline_property:
> + *
> + * Optional "COLOR_PIPELINE" enum property for specifying
> + * a color pipeline to use on the plane.
> + */
> + struct drm_property *color_pipeline_property;
> +
> /**
> * @scaling_filter_property: property to apply a particular filter while
> * scaling.
Thanks,
pq
Attachment:
pgpTqSDDK3ytV.pgp
Description: OpenPGP digital signature
