[v2 02/25] drm: Add support for 3x3 CTM

[Shankar, Uma]

> -----Original Message-----
> From: dri-devel <dri-devel-bounces at lists.freedesktop.org> On Behalf Of Dmitry
> Baryshkov
> Sent: Saturday, November 30, 2024 3:08 PM
> To: Shankar, Uma <uma.shankar at intel.com>
> Cc: dri-devel at lists.freedesktop.org; intel-gfx at lists.freedesktop.org; intel-
> xe at lists.freedesktop.org; ville.syrjala at linux.intel.com;
> harry.wentland at amd.com; pekka.paalanen at haloniitty.fi;
> sebastian.wick at redhat.com; jadahl at redhat.com; mwen at igalia.com;
> contact at emersion.fr; Kumar, Naveen1 <naveen1.kumar at intel.com>; Borah,
> Chaitanya Kumar <chaitanya.kumar.borah at intel.com>
> Subject: Re: [v2 02/25] drm: Add support for 3x3 CTM
> 
> On Tue, Nov 26, 2024 at 06:57:07PM +0530, Uma Shankar wrote:
> > From: Chaitanya Kumar Borah <chaitanya.kumar.borah at intel.com>
> >
> > Add support for 3x3 Color Transformation Matrices in Color Pipeline.
> >
> > Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah at intel.com>
> > Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> > ---
> >  drivers/gpu/drm/drm_atomic.c      |  3 +++
> >  drivers/gpu/drm/drm_atomic_uapi.c |  3 +++
> >  drivers/gpu/drm/drm_colorop.c     | 29 +++++++++++++++++++++++++++++
> >  include/drm/drm_colorop.h         |  2 ++
> >  include/uapi/drm/drm_mode.h       |  8 ++++++++
> >  5 files changed, 45 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c
> > b/drivers/gpu/drm/drm_atomic.c index 3161e2ab4efc..8a75f4a0637a 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -802,6 +802,9 @@ static void drm_atomic_colorop_print_state(struct
> drm_printer *p,
> >  		drm_printf(p, "\tinterpolation=%s\n",
> drm_get_colorop_lut1d_interpolation_name(colorop->lut1d_interpolation));
> >  		drm_printf(p, "\tdata blob id=%d\n", state->data ? state->data-
> >base.id : 0);
> >  		break;
> > +	case DRM_COLOROP_CTM_3X3:
> > +		drm_printf(p, "\tdata blob id=%d\n", state->data ? state->data-
> >base.id : 0);
> > +		break;
> >  	case DRM_COLOROP_CTM_3X4:
> >  		drm_printf(p, "\tdata blob id=%d\n", state->data ? state->data-
> >base.id : 0);
> >  		break;
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index d6c71135f290..ba68358c0ffe 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -708,6 +708,9 @@ static int drm_atomic_color_set_data_property(struct
> drm_colorop *colorop,
> >  	case DRM_COLOROP_CTM_3X4:
> >  		size = sizeof(struct drm_color_ctm_3x4);
> >  		break;
> > +	case DRM_COLOROP_CTM_3X3:
> > +		size = sizeof(struct drm_color_ctm);
> > +		break;
> >  	case DRM_COLOROP_3D_LUT:
> >  		index = state->lut_3d_mode_index;
> >  		if (index >= (state->lut_3d_modes->length / sizeof(struct
> > drm_mode_3dlut_mode))) diff --git a/drivers/gpu/drm/drm_colorop.c
> > b/drivers/gpu/drm/drm_colorop.c index fd1cd934df48..a427cfc5bbbc
> > 100644
> > --- a/drivers/gpu/drm/drm_colorop.c
> > +++ b/drivers/gpu/drm/drm_colorop.c
> > @@ -315,6 +315,35 @@ int drm_colorop_ctm_3x4_init(struct drm_device
> > *dev, struct drm_colorop *colorop  }
> > EXPORT_SYMBOL(drm_colorop_ctm_3x4_init);
> >
> > +/**
> > + * drm_colorop_ctm_3x3 - Initialize a DRM_COLOROP_CTM_3X3
> 
> Please build with W=1 to trigger kerneldoc warnings. Here the prototype in the
> doc comment doesn't match the actual function name.

Thanks Dmitry for the feedback. Yeah we missed it.
Will fix it in the next version.

Regards,
Uma Shankar

> > + *
> > + * @dev: DRM device
> > + * @colorop: The drm_colorop object to initialize
> > + * @plane: The associated drm_plane
> > + * @allow_bypass: true if BYPASS property should be created, false if bypass of
> > + *                this colorop is not possible
> > + * @return zero on success, -E value on failure  */ int
> > +drm_colorop_ctm_3x3_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> > +			     struct drm_plane *plane, bool allow_bypass) {
> > +	int ret;
> > +
> > +	ret = drm_colorop_init(dev, colorop, plane, DRM_COLOROP_CTM_3X3,
> allow_bypass);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = drm_colorop_create_data_prop(dev, colorop);
> > +	if (ret)
> > +		return ret;
> > +
> > +	drm_colorop_reset(colorop);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_colorop_ctm_3x3_init);
> > +
> >  /**
> >   * drm_colorop_mult_init - Initialize a DRM_COLOROP_MULTIPLIER
> >   *
> > diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
> > index bf5117f30c80..56b51fafebdf 100644
> > --- a/include/drm/drm_colorop.h
> > +++ b/include/drm/drm_colorop.h
> > @@ -377,6 +377,8 @@ int drm_colorop_curve_1d_lut_init(struct drm_device
> *dev, struct drm_colorop *co
> >  				  struct drm_plane *plane, uint32_t lut_size,
> >  				  enum drm_colorop_lut1d_interpolation_type
> lut1d_interpolation,
> >  				  bool allow_bypass);
> > +int drm_colorop_ctm_3x3_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> > +			     struct drm_plane *plane, bool allow_bypass);
> >  int drm_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> >  			     struct drm_plane *plane, bool allow_bypass);  int
> > drm_colorop_mult_init(struct drm_device *dev, struct drm_colorop
> > *colorop, diff --git a/include/uapi/drm/drm_mode.h
> > b/include/uapi/drm/drm_mode.h index 290c2e32f692..a4a7ab689631 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -905,6 +905,14 @@ enum drm_colorop_type {
> >  	 */
> >  	DRM_COLOROP_CTM_3X4,
> >
> > +	/**
> > +	 * @DRM_COLOROP_CTM_3X3:
> > +	 *
> > +	 * A 3x3 matrix. Its values are specified via the
> > +	 * &drm_color_ctm struct provided via the DATA property.
> > +	 */
> > +	DRM_COLOROP_CTM_3X3,
> > +
> >  	/**
> >  	 * @DRM_COLOROP_MULTIPLIER:
> >  	 *
> > --
> > 2.42.0
> >
> 
> --
> With best wishes
> Dmitry