[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <22a8894a-6eb9-4f7a-b485-6259c3abc300@bootlin.com>
Date: Fri, 5 Sep 2025 19:55:52 +0200
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: Nícolas F. R. A. Prado <nfraprado@...labora.com>,
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>
Cc: Alex Hung <alex.hung@....com>, wayland-devel@...ts.freedesktop.org,
harry.wentland@....com, leo.liu@....com, ville.syrjala@...ux.intel.com,
pekka.paalanen@...labora.com, contact@...rsion.fr, mwen@...lia.com,
jadahl@...hat.com, sebastian.wick@...hat.com, shashank.sharma@....com,
agoins@...dia.com, joshua@...ggi.es, mdaenzer@...hat.com, aleixpol@....org,
xaver.hugl@...il.com, victoria@...tem76.com, uma.shankar@...el.com,
quic_naseer@...cinc.com, quic_cbraga@...cinc.com, quic_abhinavk@...cinc.com,
marcan@...can.st, Liviu.Dudau@....com, sashamcintosh@...gle.com,
chaitanya.kumar.borah@...el.com, mcanal@...lia.com, kernel@...labora.com,
daniels@...labora.com, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, Simona Vetter <simona.vetter@...ll.ch>
Subject: Re: [PATCH RFC 3/5] drm/mediatek: Support post-blend colorops for
gamma and ctm
Le 22/08/2025 à 20:36, Nícolas F. R. A. Prado a écrit :
> Allow configuring the gamma and ccorr blocks through the post-blend
> color pipeline API instead of the GAMMA_LUT and CTM properties.
>
> In order to achieve this, initialize the color pipeline property and
> colorops on the CRTC based on the DDP components available in the CRTC
> path. Then introduce a struct mtk_drm_colorop that extends drm_colorop
> and tracks the mtk_ddp_comp that implements it in hardware, and include
> new ddp_comp helper functions for setting gamma and ctm through the new
> API. These helpers will then be called during commit flush for every
> updated colorop if the DRM client supports the post-blend color pipeline
> API.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@...labora.com>
> ---
> drivers/gpu/drm/mediatek/mtk_crtc.c | 211 +++++++++++++++++++++++++++++++-
> drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 2 +
> 2 files changed, 208 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
> index bc7527542fdc6fb89fc36794cee7d6dc26f7dcce..80ed061de1af31916d814f29f9111973cffd10dd 100644
> --- a/drivers/gpu/drm/mediatek/mtk_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
> @@ -82,6 +82,12 @@ struct mtk_crtc_state {
> unsigned int pending_vrefresh;
> };
>
> +struct mtk_drm_colorop {
> + struct drm_colorop colorop;
> + struct mtk_ddp_comp *comp;
> + uint32_t data_id;
> +};
> +
@Alex: This is exactly a case where I think the current
drm_colorop_pipeline_destroy will break.
> static inline struct mtk_crtc *to_mtk_crtc(struct drm_crtc *c)
> {
> return container_of(c, struct mtk_crtc, base);
> @@ -92,6 +98,11 @@ static inline struct mtk_crtc_state *to_mtk_crtc_state(struct drm_crtc_state *s)
> return container_of(s, struct mtk_crtc_state, base);
> }
>
> +static inline struct mtk_drm_colorop *to_mtk_colorop(struct drm_colorop *colorop)
> +{
> + return container_of(colorop, struct mtk_drm_colorop, colorop);
> +}
> +
> static void mtk_crtc_finish_page_flip(struct mtk_crtc *mtk_crtc)
> {
> struct drm_crtc *crtc = &mtk_crtc->base;
> @@ -125,6 +136,19 @@ static void mtk_drm_finish_page_flip(struct mtk_crtc *mtk_crtc)
> spin_unlock_irqrestore(&mtk_crtc->config_lock, flags);
> }
>
> +static void mtk_drm_colorop_pipeline_destroy(struct drm_device *dev)
> +{
> + struct drm_mode_config *config = &dev->mode_config;
> + struct drm_colorop *colorop, *next;
> + struct mtk_drm_colorop *mtk_colorop;
> +
> + list_for_each_entry_safe(colorop, next, &config->colorop_list, head) {
> + drm_colorop_cleanup(colorop);
> + mtk_colorop = to_mtk_colorop(colorop);
> + kfree(mtk_colorop);
> + }
> +}
> +
> static void mtk_crtc_destroy(struct drm_crtc *crtc)
> {
> struct mtk_crtc *mtk_crtc = to_mtk_crtc(crtc);
> @@ -146,6 +170,8 @@ static void mtk_crtc_destroy(struct drm_crtc *crtc)
> mtk_ddp_comp_unregister_vblank_cb(comp);
> }
>
> + mtk_drm_colorop_pipeline_destroy(crtc->dev);
> +
> drm_crtc_cleanup(crtc);
> }
>
> @@ -854,20 +880,103 @@ static void mtk_crtc_atomic_begin(struct drm_crtc *crtc,
> }
> }
>
> +static bool colorop_data_update_flush_status(struct drm_colorop_state *colorop_state)
> +{
> + struct drm_colorop *colorop = colorop_state->colorop;
> + struct mtk_drm_colorop *mtk_colorop = to_mtk_colorop(colorop);
> + struct drm_property_blob *data_blob = colorop_state->data;
> + uint32_t data_id = colorop_state->bypass ? 0 : data_blob->base.id;
> + bool needs_flush = mtk_colorop->data_id != data_id;
> +
> + mtk_colorop->data_id = data_id;
> +
> + return needs_flush;
> +}
> +
> +static void mtk_crtc_ddp_comp_apply_colorop(struct drm_colorop_state *colorop_state)
> +{
> + struct drm_colorop *colorop = colorop_state->colorop;
> + struct mtk_drm_colorop *mtk_colorop = to_mtk_colorop(colorop);
> + struct drm_property_blob *data_blob = colorop_state->data;
> + struct mtk_ddp_comp *ddp_comp = mtk_colorop->comp;
> + struct drm_color_ctm_3x4 *ctm = NULL;
> + struct drm_color_lut32 *lut = NULL;
> +
> + switch (colorop->type) {
> + case DRM_COLOROP_1D_LUT:
> + if (!colorop_data_update_flush_status(colorop_state))
> + return;
> +
> + if (!colorop_state->bypass)
> + lut = (struct drm_color_lut32 *)data_blob->data;
> +
> + ddp_comp->funcs->gamma_set_color_pipeline(ddp_comp->dev, lut);
> + break;
> + case DRM_COLOROP_CTM_3X4:
> + if (!colorop_data_update_flush_status(colorop_state))
> + return;
> +
> + if (!colorop_state->bypass)
> + ctm = (struct drm_color_ctm_3x4 *)data_blob->data;
> +
> + ddp_comp->funcs->ctm_set_color_pipeline(ddp_comp->dev, ctm);
> + break;
> + default:
> + /* If this happens the driver is broken */
> + drm_WARN(colorop->dev, 1,
> + "Trying to atomic flush COLOROP of type unsupported by driver: %d\n",
> + colorop->type);
> + break;
> + }
> +}
> +
> static void mtk_crtc_atomic_flush(struct drm_crtc *crtc,
> struct drm_atomic_state *state)
> {
> struct mtk_crtc *mtk_crtc = to_mtk_crtc(crtc);
> + struct drm_colorop_state *new_colorop_state;
> + struct drm_colorop *colorop;
> int i;
>
> - if (crtc->state->color_mgmt_changed)
> - for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> - mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
> - mtk_ddp_ctm_set(mtk_crtc->ddp_comp[i], crtc->state);
> - }
> + if (state->post_blend_color_pipeline) {
> + for_each_new_colorop_in_state(state, colorop, new_colorop_state, i)
> + mtk_crtc_ddp_comp_apply_colorop(new_colorop_state);
> + } else {
> + if (crtc->state->color_mgmt_changed)
> + for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> + mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
> + mtk_ddp_ctm_set(mtk_crtc->ddp_comp[i], crtc->state);
> + }
> + }
> mtk_crtc_update_config(mtk_crtc, !!mtk_crtc->event);
> }
>
> +static int mtk_crtc_atomic_check(struct drm_crtc *crtc,
> + struct drm_atomic_state *state)
> +{
> + struct drm_colorop_state *new_colorop_state;
> + struct drm_colorop *colorop;
> + int i;
> +
> + for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
> + switch (colorop->type) {
> + case DRM_COLOROP_1D_LUT:
> + case DRM_COLOROP_CTM_3X4:
> + if (!new_colorop_state->bypass && !new_colorop_state->data) {
> + drm_dbg_atomic(crtc->dev,
> + "Missing required DATA property for COLOROP:%d\n",
> + colorop->base.id);
> + return -EINVAL;
> + }
> + break;
> + default:
> + break;
> + }
> + }
> +
> + return 0;
> +}
> +
> static const struct drm_crtc_funcs mtk_crtc_funcs = {
> .set_config = drm_atomic_helper_set_config,
> .page_flip = drm_atomic_helper_page_flip,
> @@ -885,6 +994,7 @@ static const struct drm_crtc_helper_funcs mtk_crtc_helper_funcs = {
> .mode_valid = mtk_crtc_mode_valid,
> .atomic_begin = mtk_crtc_atomic_begin,
> .atomic_flush = mtk_crtc_atomic_flush,
> + .atomic_check = mtk_crtc_atomic_check,
> .atomic_enable = mtk_crtc_atomic_enable,
> .atomic_disable = mtk_crtc_atomic_disable,
> };
> @@ -987,6 +1097,95 @@ struct device *mtk_crtc_dma_dev_get(struct drm_crtc *crtc)
> return mtk_crtc->dma_dev;
> }
>
> +#define MAX_COLOR_PIPELINE_OPS 2
> +#define MAX_COLOR_PIPELINES 1
> +
> +static int mtk_colorop_init(struct mtk_crtc *mtk_crtc,
> + struct mtk_drm_colorop *mtk_colorop,
> + struct mtk_ddp_comp *ddp_comp)
> +{
> + struct drm_colorop *colorop = &mtk_colorop->colorop;
> + int ret = 0;
> +
> + if (ddp_comp->funcs->gamma_set_color_pipeline) {
> + unsigned int lut_sz = mtk_ddp_gamma_get_lut_size(ddp_comp);
> +
> + ret = drm_crtc_colorop_curve_1d_lut_init(mtk_crtc->base.dev, colorop,
> + &mtk_crtc->base,
> + lut_sz,
> + DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR,
> + DRM_COLOROP_FLAG_ALLOW_BYPASS);
> + } else if (ddp_comp->funcs->ctm_set_color_pipeline) {
> + ret = drm_crtc_colorop_ctm_3x4_init(mtk_crtc->base.dev,
> + colorop,
> + &mtk_crtc->base,
> + DRM_COLOROP_FLAG_ALLOW_BYPASS);
> + }
> +
> + mtk_colorop->comp = ddp_comp;
> +
> + return ret;
> +}
> +
> +static int mtk_crtc_init_post_blend_color_pipeline(struct mtk_crtc *mtk_crtc,
> + unsigned int gamma_lut_size)
> +{
> + struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES];
> + struct drm_colorop *ops[MAX_COLOR_PIPELINE_OPS];
> + struct mtk_drm_colorop *mtk_colorop;
> + unsigned int num_pipelines = 0;
> + unsigned int op_idx = 0;
> + int ret;
> +
> + memset(ops, 0, sizeof(ops));
> +
> + for (unsigned int i = 0;
> + i < mtk_crtc->ddp_comp_nr && op_idx < MAX_COLOR_PIPELINE_OPS;
> + i++) {
> + struct mtk_ddp_comp *ddp_comp = mtk_crtc->ddp_comp[i];
> +
> + if (!(ddp_comp->funcs->gamma_set_color_pipeline ||
> + ddp_comp->funcs->ctm_set_color_pipeline))
> + continue;
> +
> + mtk_colorop = kzalloc(sizeof(struct mtk_drm_colorop), GFP_KERNEL);
> + if (!mtk_colorop) {
> + ret = -ENOMEM;
> + goto cleanup;
> + }
> +
> + ops[op_idx] = &mtk_colorop->colorop;
> +
> + ret = mtk_colorop_init(mtk_crtc, mtk_colorop, ddp_comp);
> + if (ret)
> + goto cleanup;
> +
> + if (op_idx > 0)
> + drm_colorop_set_next_property(ops[op_idx-1], ops[op_idx]);
> +
> + op_idx++;
> + }
> +
> + if (op_idx > 0) {
> + pipelines[0].type = ops[0]->base.id;
> + pipelines[0].name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ops[0]->base.id);
> + num_pipelines = 1;
> + }
> +
> + /* Create COLOR_PIPELINE property and attach */
> + drm_crtc_create_color_pipeline_property(&mtk_crtc->base, pipelines, num_pipelines);
> +
> + return 0;
> +
> +cleanup:
> + if (ret == -ENOMEM)
> + drm_err(mtk_crtc->base.dev, "KMS: Failed to allocate colorop\n");
> +
> + mtk_drm_colorop_pipeline_destroy(mtk_crtc->base.dev);
> +
> + return ret;
> +}
> +
> int mtk_crtc_create(struct drm_device *drm_dev, const unsigned int *path,
> unsigned int path_len, int priv_data_index,
> const struct mtk_drm_route *conn_routes,
> @@ -1103,6 +1302,8 @@ int mtk_crtc_create(struct drm_device *drm_dev, const unsigned int *path,
> if (ret < 0)
> return ret;
>
> + mtk_crtc_init_post_blend_color_pipeline(mtk_crtc, gamma_lut_size);
> +
> if (gamma_lut_size)
> drm_mode_crtc_set_gamma_size(&mtk_crtc->base, gamma_lut_size);
> drm_crtc_enable_color_mgmt(&mtk_crtc->base, 0, has_ctm, gamma_lut_size);
> diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
> index 7289b3dcf22f22f344016beee0c7c144cf7b93c8..554c3cc8ad7b266b8b8eee74ceb8f7383fe2f8df 100644
> --- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
> @@ -75,10 +75,12 @@ struct mtk_ddp_comp_funcs {
> unsigned int (*gamma_get_lut_size)(struct device *dev);
> void (*gamma_set)(struct device *dev,
> struct drm_crtc_state *state);
> + void (*gamma_set_color_pipeline)(struct device *dev, struct drm_color_lut32 *lut);
> void (*bgclr_in_on)(struct device *dev);
> void (*bgclr_in_off)(struct device *dev);
> void (*ctm_set)(struct device *dev,
> struct drm_crtc_state *state);
> + void (*ctm_set_color_pipeline)(struct device *dev, struct drm_color_ctm_3x4 *ctm);
> struct device * (*dma_dev_get)(struct device *dev);
> u32 (*get_blend_modes)(struct device *dev);
> const u32 *(*get_formats)(struct device *dev);
>
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Powered by blists - more mailing lists