RE: [PATCH 09/28] drm: Add Color ops capability property

2024-02-13 Thread Shankar, Uma



> -Original Message-
> From: Sebastian Wick 
> Sent: Tuesday, February 13, 2024 5:35 PM
> To: Shankar, Uma 
> Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
> ville.syrj...@linux.intel.com; pekka.paala...@collabora.com;
> cont...@emersion.fr; harry.wentl...@amd.com; m...@igalia.com;
> jad...@redhat.com; shashank.sha...@amd.com; ago...@nvidia.com;
> jos...@froggi.es; mdaen...@redhat.com; aleix...@kde.org;
> xaver.h...@gmail.com; victo...@system76.com; dan...@ffwll.ch;
> quic_nas...@quicinc.com; quic_cbr...@quicinc.com;
> quic_abhin...@quicinc.com; arthurgri...@riseup.net; mar...@marcan.st;
> liviu.du...@arm.com; sashamcint...@google.com; s...@poorly.run
> Subject: Re: [PATCH 09/28] drm: Add Color ops capability property
> 
> On Tue, Feb 13, 2024 at 12:18:16PM +0530, Uma Shankar wrote:
> > Add capability property which a colorop can expose it's hardware's
> > abilities. It's a blob property that can be filled with respective
> > data structures depending on the colorop. The user space is expected
> > to read this property and program the colorop accordingly.
> >
> > Signed-off-by: Uma Shankar 
> > Signed-off-by: Chaitanya Kumar Borah 
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c |  3 +++
> >  include/drm/drm_colorop.h | 13 +
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 9f6a3a1c8020..95f1df73209c 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -770,6 +770,9 @@ drm_atomic_colorop_get_property(struct drm_colorop
> *colorop,
> > *val = state->curve_1d_type;
> > } else if (property == colorop->data_property) {
> > *val = (state->data) ? state->data->base.id : 0;
> > +   } else if (property == colorop->hw_caps_property) {
> > +   *val = state->hw_caps ?
> > +   state->hw_caps->base.id : 0;
> > } else {
> > return -EINVAL;
> > }
> > diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
> > index 5b8c36538491..f417e109c40a 100644
> > --- a/include/drm/drm_colorop.h
> > +++ b/include/drm/drm_colorop.h
> > @@ -59,6 +59,12 @@ struct drm_colorop_state {
> >  */
> > enum drm_colorop_curve_1d_type curve_1d_type;
> >
> > +   /**
> > +* @hw_caps:
> > +*
> > +*/
> > +   struct drm_property_blob *hw_caps;
> > +
> 
> Is this supposed to be generic for any colorop or specifically for
> DRM_COLOROP_1D_LUT?

We have intentionally kept it generic so that it can be used for any kind
of hardware color block (1D LUT, 3D LUT etc.). Differentiation can be done
by using the Color op type.

Regards,
Uma Shankar

> > /**
> >  * @data:
> >  *
> > @@ -167,6 +173,13 @@ struct drm_colorop {
> >  */
> > struct drm_property *bypass_property;
> >
> > +   /**
> > +* @hwlut_caps_property:
> > +*
> > +* Property to expose hardware lut capbilities.
> > +*/
> > +   struct drm_property *hw_caps_property;
> > +
> > /**
> >  * @curve_1d_type:
> >  *
> > --
> > 2.42.0
> >



Re: [PATCH 09/28] drm: Add Color ops capability property

2024-02-13 Thread Sebastian Wick
On Tue, Feb 13, 2024 at 12:18:16PM +0530, Uma Shankar wrote:
> Add capability property which a colorop can expose it's
> hardware's abilities. It's a blob property that can be
> filled with respective data structures depending on the
> colorop. The user space is expected to read this property
> and program the colorop accordingly.
> 
> Signed-off-by: Uma Shankar 
> Signed-off-by: Chaitanya Kumar Borah 
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  3 +++
>  include/drm/drm_colorop.h | 13 +
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 9f6a3a1c8020..95f1df73209c 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -770,6 +770,9 @@ drm_atomic_colorop_get_property(struct drm_colorop 
> *colorop,
>   *val = state->curve_1d_type;
>   } else if (property == colorop->data_property) {
>   *val = (state->data) ? state->data->base.id : 0;
> + } else if (property == colorop->hw_caps_property) {
> + *val = state->hw_caps ?
> + state->hw_caps->base.id : 0;
>   } else {
>   return -EINVAL;
>   }
> diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
> index 5b8c36538491..f417e109c40a 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -59,6 +59,12 @@ struct drm_colorop_state {
>*/
>   enum drm_colorop_curve_1d_type curve_1d_type;
>  
> + /**
> +  * @hw_caps:
> +  *
> +  */
> + struct drm_property_blob *hw_caps;
> +

Is this supposed to be generic for any colorop or specifically for
DRM_COLOROP_1D_LUT?

>   /**
>* @data:
>*
> @@ -167,6 +173,13 @@ struct drm_colorop {
>*/
>   struct drm_property *bypass_property;
>  
> + /**
> +  * @hwlut_caps_property:
> +  *
> +  * Property to expose hardware lut capbilities.
> +  */
> + struct drm_property *hw_caps_property;
> +
>   /**
>* @curve_1d_type:
>*
> -- 
> 2.42.0
>