> -----Original Message-----
> From: Dmitry Baryshkov <dmitry.barysh...@linaro.org>
> Sent: Saturday, November 30, 2024 3:17 PM
> To: Shankar, Uma <uma.shan...@intel.com>
> Cc: dri-devel@lists.freedesktop.org; intel-...@lists.freedesktop.org; intel-
> x...@lists.freedesktop.org; ville.syrj...@linux.intel.com;
> harry.wentl...@amd.com; pekka.paala...@haloniitty.fi;
> sebastian.w...@redhat.com; jad...@redhat.com; m...@igalia.com;
> cont...@emersion.fr; Kumar, Naveen1 <naveen1.ku...@intel.com>; Borah,
> Chaitanya Kumar <chaitanya.kumar.bo...@intel.com>
> Subject: Re: [v2 09/25] drm: Add helper to initialize segmented 1D LUT
>
> On Tue, Nov 26, 2024 at 06:57:14PM +0530, Uma Shankar wrote:
> > From: Chaitanya Kumar Borah <chaitanya.kumar.bo...@intel.com>
> >
> > Add helper to initialize 1D segmented LUT
> >
> > Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.bo...@intel.com>
> > Signed-off-by: Uma Shankar <uma.shan...@intel.com>
> > ---
> > drivers/gpu/drm/drm_colorop.c | 27 ++++++++++++++++++++++++++-
> > include/drm/drm_colorop.h | 4 ++++
> > 2 files changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_colorop.c
> > b/drivers/gpu/drm/drm_colorop.c index 111517c08216..871d5660e3b2
> > 100644
> > --- a/drivers/gpu/drm/drm_colorop.c
> > +++ b/drivers/gpu/drm/drm_colorop.c
> > @@ -104,7 +104,6 @@ static int drm_create_colorop_capability_prop(struct
> drm_device *dev,
> > return 0;
> > }
> >
> > -__maybe_unused
>
> Squash all three patches so that you don't have to play with __maybe_unused.
> Then please expand commit message to describe the problem that you are
> solving.
Idea was to logically separate for ease of review. But we can squash if that
looks better.
> > static int drm_colorop_lutcaps_init(struct drm_colorop *colorop,
> > struct drm_plane *plane,
> > const struct drm_color_lut_range *ranges,
> @@ -338,6 +337,32
> > @@ int drm_colorop_curve_1d_lut_init(struct drm_device *dev, struct
> > drm_colorop *co } EXPORT_SYMBOL(drm_colorop_curve_1d_lut_init);
> >
>
> Missing kerneldoc
Sure, will update and add it.
Regards,
Uma Shankar
> > +int drm_colorop_curve_1d_lut_multseg_init(struct drm_device *dev, struct
> drm_colorop *colorop,
> > + struct drm_plane *plane,
> > + const struct drm_color_lut_range
> *ranges,
> > + size_t length, bool allow_bypass) {
> > + int ret;
> > +
> > + ret = drm_colorop_init(dev, colorop, plane,
> DRM_COLOROP_1D_LUT_MULTSEG, allow_bypass);
> > + if (ret)
> > + return ret;
> > +
> > + ret = drm_colorop_lutcaps_init(colorop, plane, ranges, length);
> > + if (ret)
> > + return ret;
> > +
> > + /* data */
> > + ret = drm_colorop_create_data_prop(dev, colorop);
> > + if (ret)
> > + return ret;
> > +
> > + drm_colorop_reset(colorop);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(drm_colorop_curve_1d_lut_multseg_init);
> > +
> > int drm_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop
> *colorop,
> > struct drm_plane *plane, bool allow_bypass) {
> > diff --
> git
> > a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h index
> > 58e5b87c6d56..9268fffd04f6 100644
> > --- a/include/drm/drm_colorop.h
> > +++ b/include/drm/drm_colorop.h
> > @@ -391,6 +391,10 @@ int drm_colorop_curve_1d_lut_init(struct drm_device
> *dev, struct drm_colorop *co
> > 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_curve_1d_lut_multseg_init(struct drm_device *dev, struct
> drm_colorop *colorop,
> > + struct drm_plane *plane,
> > + const struct drm_color_lut_range
> *ranges,
> > + size_t length, 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,
> > --
> > 2.42.0
> >
>
> --
> With best wishes
> Dmitry
