[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <acd144d5-9bf7-4661-9f3a-5b55a8132b10@collabora.com>
Date: Wed, 10 Dec 2025 18:00:10 -0300
From: Ariel D'Alessandro <ariel.dalessandro@...labora.com>
To: Louis Chauvet <louis.chauvet@...tlin.com>,
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>,
Haneen Mohammed <hamohammed.sa@...il.com>,
Melissa Wen <melissa.srw@...il.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, leandro.ribeiro@...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 v2 04/20] drm/crtc: Add COLOR_PIPELINE property
Louis,
On 9/19/25 9:43 AM, Louis Chauvet wrote:
>
>
> Le 18/09/2025 à 02:43, Nícolas F. R. A. Prado a écrit :
>> Add a COLOR_PIPELINE property to the CRTC to allow userspace to set a
>> post-blend color pipeline analogously to how pre-blend color pipelines
>> are set on planes.
>>
>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@...labora.com>
>> ---
>> drivers/gpu/drm/drm_atomic_uapi.c | 49 +++++++++++++++++++++++++++++
>> ++++++----
>> drivers/gpu/drm/drm_crtc.c | 33 ++++++++++++++++++++++++++
>> include/drm/drm_atomic_uapi.h | 2 ++
>> include/drm/drm_crtc.h | 11 +++++++++
>> 4 files changed, 91 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/
>> drm_atomic_uapi.c
>> index
>> b7cc6945864274bedd21dd5b73494f9aae216888..063c142fd9b656e228cfc660d005a3fbb4640d32 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -287,6 +287,33 @@ drm_atomic_set_colorop_for_plane(struct
>> drm_plane_state *plane_state,
>> }
>> EXPORT_SYMBOL(drm_atomic_set_colorop_for_plane);
>> +/**
>> + * drm_atomic_set_colorop_for_crtc - set colorop for crtc
>> + * @crtc_state: atomic state object for the crtc
>> + * @colorop: colorop to use for the crtc
>> + *
>> + * Helper function to select the color pipeline on a crtc by setting
>> + * it to the first drm_colorop element of the pipeline.
>> + */
>> +void
>> +drm_atomic_set_colorop_for_crtc(struct drm_crtc_state *crtc_state,
>> + struct drm_colorop *colorop)
>> +{
>> + struct drm_crtc *crtc = crtc_state->crtc;
>> +
>> + if (colorop)
>> + drm_dbg_atomic(crtc->dev,
>> + "Set [COLOROP:%d] for [CRTC:%d:%s] state %p\n",
>> + colorop->base.id, crtc->base.id, crtc->name,
>> + crtc_state);
>> + else
>> + drm_dbg_atomic(crtc->dev,
>> + "Set [NOCOLOROP] for [CRTC:%d:%s] state %p\n",
>> + crtc->base.id, crtc->name, crtc_state);
>> +
>> + crtc_state->color_pipeline = colorop;
>> +}
>> +EXPORT_SYMBOL(drm_atomic_set_colorop_for_crtc);
>> /**
>> * drm_atomic_set_crtc_for_connector - set CRTC for connector
>> @@ -396,8 +423,8 @@ static s32 __user
>> *get_out_fence_for_connector(struct drm_atomic_state *state,
>> }
>> static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>> - struct drm_crtc_state *state, struct drm_property *property,
>> - uint64_t val)
>> + struct drm_crtc_state *state, struct drm_file *file_priv,
>> + struct drm_property *property, uint64_t val)
>> {
>> struct drm_device *dev = crtc->dev;
>> struct drm_mode_config *config = &dev->mode_config;
>> @@ -406,7 +433,17 @@ static int drm_atomic_crtc_set_property(struct
>> drm_crtc *crtc,
>> if (property == config->prop_active)
>> state->active = val;
>> - else if (property == config->prop_mode_id) {
>> + else if (property == crtc->color_pipeline_property) {
>> + /* find DRM colorop object */
>> + struct drm_colorop *colorop = NULL;
>> +
>> + colorop = drm_colorop_find(dev, file_priv, val);
>> +
>> + if (val && !colorop)
>> + return -EACCES;
>> +
>> + drm_atomic_set_colorop_for_crtc(state, colorop);
>
> I don't know if this is needed, but you added a warning/early return for
> ctm/degamma_lut if file_priv->post_blend_color_pipeline is true.
>
> Does it make sense to add this?
>
> if (!file_priv->post_blend_color_pipeline) {
> drm_dbg_atomic(dev,
> "Setting COLOR_PIPELINE property not permitted without
> DRM_CLIENT_CAP_POST_BLEND_COLOR_PIPELINE client cap\n");
> return -EINVAL;
> }
Indeed that makes sense. Will add in v3.
Thanks,
--
Ariel D'Alessandro
Software Engineer
Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718
Powered by blists - more mailing lists