[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2fc9635430e6af0c175d6d019b7cd1598034892c.camel@collabora.com>
Date: Fri, 06 Feb 2026 09:05:11 -0500
From: Nícolas "F. R. A. Prado" <nfraprado@...labora.com>
To: Pekka Paalanen <pekka.paalanen@...labora.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
<mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David Airlie
<airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Chun-Kuang Hu
<chunkuang.hu@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>, Matthias
Brugger <matthias.bgg@...il.com>, AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, daniels@...labora.com,
ariel.dalessandro@...labora.com, kernel@...labora.com
Subject: Re: [PATCH 09/11] drm/colorop: Introduce 3x3 Matrix
On Fri, 2026-02-06 at 11:27 +0200, Pekka Paalanen wrote:
> On Tue, 23 Dec 2025 16:44:50 -0300
> Nícolas F. R. A. Prado <nfraprado@...labora.com> wrote:
>
> > Introduce a 3x3 Matrix colorop analogous to the 3x4 Matrix colorop,
> > with
> > the difference of not supporting offset coefficients.
> >
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@...labora.com>
> > ---
> > drivers/gpu/drm/drm_atomic.c | 1 +
> > drivers/gpu/drm/drm_atomic_uapi.c | 3 +++
> > drivers/gpu/drm/drm_colorop.c | 21 +++++++++++++++++++++
> > include/drm/drm_colorop.h | 3 +++
> > include/uapi/drm/drm_mode.h | 16 ++++++++++++++++
> > 5 files changed, 44 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c
> > b/drivers/gpu/drm/drm_atomic.c
> > index 6d3ea8056b60..bf4a31c02b70 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -802,6 +802,7 @@ static void
> > drm_atomic_colorop_print_state(struct drm_printer *p,
> > drm_printf(p, "\tdata blob id=%d\n", state->data ?
> > state->data->base.id : 0);
> > break;
> > case DRM_COLOROP_CTM_3X4:
> > + case DRM_COLOROP_CTM_3X3:
> > drm_printf(p, "\tdata blob id=%d\n", state->data ?
> > state->data->base.id : 0);
> > break;
> > case DRM_COLOROP_MULTIPLIER:
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 7320db4b8489..7a70e894a2ef 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -709,6 +709,9 @@ static int
> > drm_atomic_color_set_data_property(struct drm_colorop *colorop,
> > size = colorop->size * colorop->size * colorop-
> > >size *
> > sizeof(struct drm_color_lut32);
> > break;
> > + case DRM_COLOROP_CTM_3X3:
> > + size = sizeof(struct drm_color_ctm);
> > + break;
> > default:
> > /* should never get here */
> > return -EINVAL;
> > diff --git a/drivers/gpu/drm/drm_colorop.c
> > b/drivers/gpu/drm/drm_colorop.c
> > index a19e03fb9c7c..51c1a0726dfa 100644
> > --- a/drivers/gpu/drm/drm_colorop.c
> > +++ b/drivers/gpu/drm/drm_colorop.c
> > @@ -68,6 +68,7 @@ static const struct drm_prop_enum_list
> > drm_colorop_type_enum_list[] = {
> > { DRM_COLOROP_CTM_3X4, "3x4 Matrix"},
> > { DRM_COLOROP_MULTIPLIER, "Multiplier"},
> > { DRM_COLOROP_3D_LUT, "3D LUT"},
> > + { DRM_COLOROP_CTM_3X3, "3x3 Matrix"},
> > };
> >
> > static const char * const colorop_curve_1d_type_names[] = {
> > @@ -377,6 +378,26 @@ int drm_plane_colorop_ctm_3x4_init(struct
> > drm_device *dev, struct drm_colorop *c
> > }
> > EXPORT_SYMBOL(drm_plane_colorop_ctm_3x4_init);
> >
> > +int drm_plane_colorop_ctm_3x3_init(struct drm_device *dev, struct
> > drm_colorop *colorop,
> > + struct drm_plane *plane, const
> > struct drm_colorop_funcs *funcs,
> > + uint32_t flags)
> > +{
> > + int ret;
> > +
> > + ret = drm_plane_colorop_init(dev, colorop, plane, funcs,
> > DRM_COLOROP_CTM_3X3, flags);
> > + 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_plane_colorop_ctm_3x3_init);
> > +
> > /**
> > * drm_plane_colorop_mult_init - Initialize a
> > DRM_COLOROP_MULTIPLIER
> > *
> > diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
> > index 8ec98521607d..ee7fa0eb5dbf 100644
> > --- a/include/drm/drm_colorop.h
> > +++ b/include/drm/drm_colorop.h
> > @@ -426,6 +426,9 @@ int drm_plane_colorop_curve_1d_lut_init(struct
> > drm_device *dev, struct drm_color
> > int drm_plane_colorop_ctm_3x4_init(struct drm_device *dev, struct
> > drm_colorop *colorop,
> > struct drm_plane *plane, const
> > struct drm_colorop_funcs *funcs,
> > uint32_t flags);
> > +int drm_plane_colorop_ctm_3x3_init(struct drm_device *dev, struct
> > drm_colorop *colorop,
> > + struct drm_plane *plane, const
> > struct drm_colorop_funcs *funcs,
> > + uint32_t flags);
> > int drm_plane_colorop_mult_init(struct drm_device *dev, struct
> > drm_colorop *colorop,
> > struct drm_plane *plane, const
> > struct drm_colorop_funcs *funcs,
> > uint32_t flags);
> > diff --git a/include/uapi/drm/drm_mode.h
> > b/include/uapi/drm/drm_mode.h
> > index cbbbfc1dfe2b..b894b19eb9f8 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -964,6 +964,22 @@ enum drm_colorop_type {
> > * color = lut3d[index]
> > */
> > DRM_COLOROP_3D_LUT,
> > +
> > + /**
> > + * @DRM_COLOROP_CTM_3X3:
> > + *
> > + * enum string "3x3 Matrix"
> > + *
> > + * A 3x3 matrix. Its values are specified via the
> > + * &drm_color_ctm struct provided via the DATA property.
> > + *
> > + * The DATA blob is a float[9]:
> > + * out matrix in
> > + * | R | | 0 1 2 | | R |
> > + * | G | = | 3 4 5 | x | G |
> > + * | B | | 6 7 8 | | B |
> > + */
> > + DRM_COLOROP_CTM_3X3,
> > };
> >
> > /**
> >
>
> Acked-by: Pekka Paalanen <pekka.paalanen@...labora.com>
>
> FWIW, the statement "The DATA blob is a float[9]" is incorrect, but
> the
> same wording exists already with the DRM_COLOROP_CTM_3X4. The data
> type
> is not a float, nor is it a float reinterpreted as __u64 (they are
> not
> even the same size). Documentation for the structs explains the
> correct
> type, which is a fixed-point number.
>
> The struct types document also the layout, so there is no need to
> repeat the layout here. It may be worth mentioning that the blob must
> contain exactly one instance of the struct.
Makes sense, thanks! I'll fix that in v2.
--
Thanks,
Nícolas
Powered by blists - more mailing lists