Le 19/10/2025 à 11:16, Federico Amedeo Izzo via B4 Relay a écrit :
From: Federico Amedeo Izzo <federico-3FKkmSgw/[email protected]>

Add support for DSPP GC block in DPU driver for Qualcomm SoCs.
Expose the GAMMA_LUT DRM property, which is needed to enable
night light and basic screen color calibration.

I used LineageOS downstream kernel as a reference and found the LUT
format by trial-and-error on OnePlus 6.

Tested on oneplus-enchilada (sdm845-mainline 6.16-dev) and xiaomi-tissot
(msm8953-mainline 6.12/main).

Tested-by: David Heidelberg <[email protected]>  # Pixel 3 
(next-20251018)
Tested-by: Guido Günther <[email protected]> # on 
sdm845-shift-axolotl
Signed-off-by: Federico Amedeo Izzo <federico-3FKkmSgw/[email protected]>
---
DRM GAMMA_LUT support was missing on sdm845 and other Qualcomm SoCs using
DPU for CRTC. This is needed in userspace to enable features like Night
Light or basic color calibration.

I wrote this driver to enable Night Light on OnePlus 6, and after the
driver was working I found out it applies to the 29 different Qualcomm SoCs
that use the DPU display engine, including X1E for laptops.

I used the LineageOS downstream kernel as reference and found the correct
LUT format by trial-and-error on OnePlus 6.

This was my first Linux driver and it's been a great learning
experience.

The patch was reviewed by postmarketOS contributors here:
https://gitlab.com/sdm845-mainline/linux/-/merge_requests/137
During review the patch was tested successfully on hamoa (X1E).
---

Hi,

...

@@ -830,19 +862,40 @@ static void _dpu_crtc_setup_cp_blocks(struct drm_crtc 
*crtc)
                ctl = mixer[i].lm_ctl;
                dspp = mixer[i].hw_dspp;
- if (!dspp || !dspp->ops.setup_pcc)
+               if (!dspp)
                        continue;
- if (!state->ctm) {
-                       dspp->ops.setup_pcc(dspp, NULL);
-               } else {
-                       _dpu_crtc_get_pcc_coeff(state, &cfg);
-                       dspp->ops.setup_pcc(dspp, &cfg);
+               if (dspp->ops.setup_pcc) {
+                       if (!state->ctm) {
+                               dspp->ops.setup_pcc(dspp, NULL);
+                       } else {
+                               _dpu_crtc_get_pcc_coeff(state, &cfg);
+                               dspp->ops.setup_pcc(dspp, &cfg);
+                       }
+
+                       /* stage config flush mask */
+                       ctl->ops.update_pending_flush_dspp(ctl,
+                               mixer[i].hw_dspp->idx, DPU_DSPP_PCC);
                }
- /* stage config flush mask */
-               ctl->ops.update_pending_flush_dspp(ctl,
-                       mixer[i].hw_dspp->idx, DPU_DSPP_PCC);
+               if (dspp->ops.setup_gc) {
+                       if (!state->gamma_lut) {
+                               dspp->ops.setup_gc(dspp, NULL);
+                       } else {
+                               gc_lut = kzalloc(sizeof(*gc_lut), GFP_KERNEL);

The memory is already cleared in _dpu_crtc_get_gc_lut().
Eiher this could be changed into kmalloc(), or the memset in _dpu_crtc_get_gc_lut() could be removed.

+                               if (!gc_lut) {
+                                       DRM_ERROR("failed to allocate 
gc_lut\n");

usually,message related to memory allocation errors are not needed, because things are already verbose in such a case.

+                                       continue;
+                               }
+                               _dpu_crtc_get_gc_lut(state, gc_lut);
+                               dspp->ops.setup_gc(dspp, gc_lut);
+                               kfree(gc_lut);
+                       }
+
+                       /* stage config flush mask */
+                       ctl->ops.update_pending_flush_dspp(ctl,
+                               mixer[i].hw_dspp->idx, DPU_DSPP_GC);
+               }
        }
  }

...

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c
index 54b20faa0b69..7ebe7d8a5382 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c

...

@@ -63,6 +75,47 @@ static void dpu_setup_dspp_pcc(struct dpu_hw_dspp *ctx,
        DPU_REG_WRITE(&ctx->hw, base, PCC_EN);
  }
+static void dpu_setup_dspp_gc(struct dpu_hw_dspp *ctx,
+               struct dpu_hw_gc_lut *gc_lut)
+{
+       int i = 0;
+       u32 base, reg;
+
+       if (!ctx) {
+               DRM_ERROR("invalid ctx %pK\n", ctx);

ctx is known to be NULL here. So the message can be simplified.

+               return;
+       }
+
+       base = ctx->cap->sblk->gc.base;
+
+       if (!base) {
+               DRM_ERROR("invalid ctx %pK gc base 0x%x\n", ctx, base);

base is known to be NULL here. So the message can be simplified.

+               return;
+       }
+
+       if (!gc_lut) {
+               DRM_DEBUG_DRIVER("disable gc feature\n");
+               DPU_REG_WRITE(&ctx->hw, base, GC_DIS);
+               return;
+       }
+
+       reg = 0;
+       DPU_REG_WRITE(&ctx->hw, base + GC_C0_INDEX_OFF, reg);
+       DPU_REG_WRITE(&ctx->hw, base + GC_C1_INDEX_OFF, reg);
+       DPU_REG_WRITE(&ctx->hw, base + GC_C2_INDEX_OFF, reg);

Why not using 0 explicitly, instead of using reg here?

+
+       for (i = 0; i < PGC_TBL_LEN; i++) {
+               DPU_REG_WRITE(&ctx->hw, base + GC_C0_OFF, gc_lut->c0[i]);
+               DPU_REG_WRITE(&ctx->hw, base + GC_C1_OFF, gc_lut->c1[i]);
+               DPU_REG_WRITE(&ctx->hw, base + GC_C2_OFF, gc_lut->c2[i]);
+       }
+
+       DPU_REG_WRITE(&ctx->hw, base + GC_LUT_SWAP_OFF, BIT(0));
+
+       reg = GC_EN | ((gc_lut->flags & PGC_8B_ROUND) ? GC_8B_ROUND_EN : 0);
+       DPU_REG_WRITE(&ctx->hw, base, reg);
+}

...

CJ

Reply via email to