> Subject: [v6 05/16] 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
>
> 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 | 101 ++++++++++++++++++
> .../drm/i915/display/intel_color_pipeline.h | 13 +++
> drivers/gpu/drm/xe/Makefile | 1 +
> 4 files changed, 116 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 b2f591e156cb..8e1875538831 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -239,6 +239,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..b6e6ebdd0dff
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_color_pipeline.c
> @@ -0,0 +1,101 @@
> +// 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 5
Make this NUM_COLOR_PIPELINES 2
No point in attaching pipeline[5] when you only use also it does not
Actually reflect any H/W limitations
When more pipelines are added it can be changed accordingly
> +#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 */
> + 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);
> +
> + /* TODO check if needed */
> + if (plane->state)
> + plane->state->color_pipeline = NULL;
I know you have added a TODO here but
I don’t get why is this added should this not be happening during some
Atomic_duplicate_state or something.
Also it should be *TODO:
Regards,
Suraj Kandpal
> +
> + 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 c89cafbd00fa..dfb8f00940ad 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -241,6 +241,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
