RE: [v6 05/16] drm/i915/color: Create a transfer function color pipeline
From: Kandpal, Suraj <hidden>
Date: 2025-11-18 06:23:32
Also in:
intel-gfx, intel-xe
quoted hunk ↗ jump to hunk
Subject: [v6 05/16] drm/i915/color: Create a transfer function color pipeline From: Chaitanya Kumar Borah <redacted> 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 <redacted> Signed-off-by: Chaitanya Kumar Borah <redacted> --- 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.hdiff --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.cb/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
quoted hunk ↗ jump to hunk
+ + return 0; +}diff --git a/drivers/gpu/drm/i915/display/intel_color_pipeline.hb/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