On Wed, 8 Nov 2023 11:36:32 -0500
Harry Wentland <harry.wentl...@amd.com> 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.wentl...@amd.com>
> ---
>  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: pgpDsxPec467h.pgp
Description: OpenPGP digital signature

Reply via email to