On 12/12/2025 8:38 PM, Ville Syrjälä wrote:
On Wed, Dec 03, 2025 at 02:22:10PM +0530, Uma Shankar wrote:
From: Chaitanya Kumar Borah <[email protected]>
Add helpers to program the 3D LUT registers and arm them.
LUT_3D_READY in LUT_3D_CLT is cleared off by the HW once
the LUT buffer is loaded into it's internal working RAM.
So by the time we try to load/commit new values, we expect
it to be cleared off. If not, log an error and return
without writing new values. Do it only when writing with MMIO.
There is no way to read register within DSB execution.
v2:
- Add information regarding LUT_3D_READY to commit message (Jani)
- Log error instead of a drm_warn and return without committing changes
if 3DLUT HW is not ready to accept new values.
- Refactor intel_color_crtc_has_3dlut()
Also remove Gen10 check (Suraj)
v3:
- Addressed review comments (Suraj)
Reviewed-by: Suraj Kandpal <[email protected]>
Signed-off-by: Chaitanya Kumar Borah <[email protected]>
Signed-off-by: Uma Shankar <[email protected]>
---
drivers/gpu/drm/i915/display/intel_color.c | 78 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_color.h | 4 +
.../drm/i915/display/intel_color_pipeline.c | 29 +++++--
.../drm/i915/display/intel_color_pipeline.h | 3 +-
.../drm/i915/display/intel_display_limits.h | 1 +
.../drm/i915/display/intel_display_types.h | 2 +-
drivers/gpu/drm/i915/display/intel_plane.c | 2 +
7 files changed, 112 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c
b/drivers/gpu/drm/i915/display/intel_color.c
index 08f3b5b47b8e..e7950655434b 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -4062,6 +4062,52 @@ xelpd_plane_load_luts(struct intel_dsb *dsb, const
struct intel_plane_state *pla
xelpd_program_plane_post_csc_lut(dsb, plane_state);
}
+static u32 glk_3dlut_10(const struct drm_color_lut32 *color)
+{
+ return REG_FIELD_PREP(LUT_3D_DATA_RED_MASK,
drm_color_lut32_extract(color->red, 10)) |
+ REG_FIELD_PREP(LUT_3D_DATA_GREEN_MASK,
drm_color_lut32_extract(color->green, 10)) |
+ REG_FIELD_PREP(LUT_3D_DATA_BLUE_MASK,
drm_color_lut32_extract(color->blue, 10));
+}
+
+static void glk_load_lut_3d(struct intel_dsb *dsb,
+ struct intel_crtc *crtc,
+ const struct drm_property_blob *blob)
+{
+ struct intel_display *display = to_intel_display(crtc->base.dev);
+ const struct drm_color_lut32 *lut = blob->data;
+ int i, lut_size = drm_color_lut32_size(blob);
+ enum pipe pipe = crtc->pipe;
+
+ if (!dsb && intel_de_read(display, LUT_3D_CTL(pipe)) & LUT_3D_READY) {
+ drm_err(display->drm, "[CRTC:%d:%s] 3D LUT not ready, not loading
LUTs\n",
+ crtc->base.base.id, crtc->base.name);
+ return;
Just ran into this while perusing the code...
This check could be implemented exactly like intel_vrr_check_push_sent()
so that it works for both the DSB and non-DSB paths.
We did discuss this briefly[1], but went on with this as a first step.
My main concern was if it is a good idea to poll for a bit in the middle
of a commit. I understand that this is done for TRANS_PUSH_SEND but that
is the last thing we do for a commit.
The 'return' should
just get nuked IMO.
So just move ahead and program irrespective?
+void intel_color_plane_commit_arm(struct intel_dsb *dsb,
+ const struct intel_plane_state *plane_state)
+{
+ struct intel_display *display = to_intel_display(plane_state);
+ struct intel_crtc *crtc = to_intel_crtc(plane_state->uapi.crtc);
+
+ if (crtc && intel_color_crtc_has_3dlut(display, crtc->pipe))
+ glk_lut_3d_commit(dsb, crtc, !!plane_state->hw.lut_3d);
^^^^^^^^^^^^
And this looks like a pretty major fail. Why is the 3D LUT stored in
the *plane* state when it's a pipe level thing?
With DISPLAY_VER(display) >= 35, 3DLUT can be attached to a plane.
(Bits[23:22] in 3DLUT_CTL). This is the only way we are exposing the HW
to the userspace right now (through the new plane color pipeline uapi).
Therefore, it lies in the plane state.
However, there are (soonish)plans to adopt the color pipeline for crtcs
too. Once that happens, it needs to be handled a bit more carefully. A
potential approach is to allow userspace to program the block with a
first come first served semantics and fail the commit if it tries to set
3DLUT both on plane and crtc in the same commit.
[1]
https://lore.kernel.org/intel-gfx/[email protected]/
==
Chaitanya
