> Subject: [v7 04/15] drm/i915/color: Create a transfer function color pipeline > > From: Chaitanya Kumar Borah <[email protected]> > > Add a color pipeline with three colorops in the sequence > > 1D LUT - 3x4 CTM - 1D LUT > > This pipeline can be used to do any color space conversion or HDR tone > mapping > > v2: Change namespace to drm_plane_colorop* > v3: Use simpler/pre-existing colorops for first iteration > v4: > - s/*_tf_*/*_color_* (Jani) > - Refactor to separate files (Jani) > - Add missing space in comment (Suraj) > - Consolidate patch that adds/attaches pipeline property > v5: > - Limit MAX_COLOR_PIPELINES to 2.(Suraj) > Increase it as and when we add more pipelines. > - Remove redundant initialization code (Suraj) > > Signed-off-by: Uma Shankar <[email protected]> > Signed-off-by: Chaitanya Kumar Borah <[email protected]> > --- > drivers/gpu/drm/i915/Makefile | 1 + > .../drm/i915/display/intel_color_pipeline.c | 97 +++++++++++++++++++ > .../drm/i915/display/intel_color_pipeline.h | 13 +++ > drivers/gpu/drm/xe/Makefile | 1 + > 4 files changed, 112 insertions(+) > create mode 100644 drivers/gpu/drm/i915/display/intel_color_pipeline.c > create mode 100644 drivers/gpu/drm/i915/display/intel_color_pipeline.h > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 7c19d5345d88..ca5c69d1cb08 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -241,6 +241,7 @@ i915-y += \ > display/intel_cmtg.o \ > display/intel_color.o \ > display/intel_colorop.o \ > + display/intel_color_pipeline.o \ > display/intel_combo_phy.o \ > display/intel_connector.o \ > display/intel_crtc.o \ > diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.c > b/drivers/gpu/drm/i915/display/intel_color_pipeline.c > new file mode 100644 > index 000000000000..1415f94dd3e3 > --- /dev/null > +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c > @@ -0,0 +1,97 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright © 2025 Intel Corporation > + */ > +#include "intel_colorop.h" > +#include "intel_color_pipeline.h" > +#include "intel_de.h" > +#include "intel_display_types.h" > +#include "skl_universal_plane.h" > + > +#define MAX_COLOR_PIPELINES 2 > +#define PLANE_DEGAMMA_SIZE 128 > +#define PLANE_GAMMA_SIZE 32 > + > +static > +int _intel_color_pipeline_plane_init(struct drm_plane *plane, struct > +drm_prop_enum_list *list) { > + struct intel_colorop *colorop; > + struct drm_device *dev = plane->dev; > + int ret; > + struct drm_colorop *prev_op; > + > + colorop = intel_colorop_create(INTEL_PLANE_CB_PRE_CSC_LUT); > + > + ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, > plane, > + PLANE_DEGAMMA_SIZE, > + > DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR, > + > DRM_COLOROP_FLAG_ALLOW_BYPASS); > + > + if (ret) > + return ret; > + > + list->type = colorop->base.base.id; > + list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", > +colorop->base.base.id); > + > + /* TODO: handle failures and clean up */ > + prev_op = &colorop->base; > + > + colorop = intel_colorop_create(INTEL_PLANE_CB_CSC); > + ret = drm_plane_colorop_ctm_3x4_init(dev, &colorop->base, plane, > + > DRM_COLOROP_FLAG_ALLOW_BYPASS); > + if (ret) > + return ret; > + > + drm_colorop_set_next_property(prev_op, &colorop->base); > + prev_op = &colorop->base; > + > + colorop = intel_colorop_create(INTEL_PLANE_CB_POST_CSC_LUT); > + ret = drm_plane_colorop_curve_1d_lut_init(dev, &colorop->base, > plane, > + PLANE_GAMMA_SIZE, > + > DRM_COLOROP_LUT1D_INTERPOLATION_LINEAR, > + > DRM_COLOROP_FLAG_ALLOW_BYPASS); > + if (ret) > + return ret; > + > + drm_colorop_set_next_property(prev_op, &colorop->base); > + > + return 0; > +} > + > +int intel_color_pipeline_plane_init(struct drm_plane *plane) { > + struct drm_device *dev = plane->dev; > + struct intel_display *display = to_intel_display(dev); > + struct drm_property *prop; > + struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES]; > + int len = 0; > + int ret; > + > + /* Currently expose pipeline only for HDR planes */ > + if (!icl_is_hdr_plane(display, to_intel_plane(plane)->id)) > + return 0; > + > + /* Add "Bypass" (i.e. NULL) pipeline */
I think we can do away with th "(i.e NULL)" part since the documentation at drm core does a good job explaining what BYPASS pipeline is With that fixed LGTM, Reviewed-by: Suraj Kandpal <[email protected]> > + pipelines[len].type = 0; > + pipelines[len].name = "Bypass"; > + len++; > + > + /* Add pipeline consisting of transfer functions */ > + ret = _intel_color_pipeline_plane_init(plane, &pipelines[len]); > + if (ret) > + return ret; > + len++; > + > + /* Create COLOR_PIPELINE property and attach */ > + prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC, > + "COLOR_PIPELINE", > + pipelines, len); > + if (!prop) > + return -ENOMEM; > + > + plane->color_pipeline_property = prop; > + > + drm_object_attach_property(&plane->base, prop, 0); > + > + return 0; > +} > diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.h > b/drivers/gpu/drm/i915/display/intel_color_pipeline.h > new file mode 100644 > index 000000000000..7f1d32bc9202 > --- /dev/null > +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.h > @@ -0,0 +1,13 @@ > +/* SPDX-License-Identifier: MIT */ > +/* > + * Copyright © 2025 Intel Corporation > + */ > + > +#ifndef __INTEL_COLOR_PIPELINE_H__ > +#define __INTEL_COLOR_PIPELINE_H__ > + > +struct drm_plane; > + > +int intel_color_pipeline_plane_init(struct drm_plane *plane); > + > +#endif /* __INTEL_COLOR_PIPELINE_H__ */ > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile index > 3420725c4ba8..89f922d745ba 100644 > --- a/drivers/gpu/drm/xe/Makefile > +++ b/drivers/gpu/drm/xe/Makefile > @@ -235,6 +235,7 @@ xe-$(CONFIG_DRM_XE_DISPLAY) += \ > i915-display/intel_cmtg.o \ > i915-display/intel_color.o \ > i915-display/intel_colorop.o \ > + i915-display/intel_color_pipeline.o \ > i915-display/intel_combo_phy.o \ > i915-display/intel_connector.o \ > i915-display/intel_crtc.o \ > -- > 2.50.1
