> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@xxxxxxxxx>
> Sent: Wednesday, December 11, 2024 11:49 PM
> To: intel-xe@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Shankar, Uma <uma.shankar@xxxxxxxxx>; ville.syrjala@xxxxxxxxxxxxxxx;
> Borah, Chaitanya Kumar <chaitanya.kumar.borah@xxxxxxxxx>
> Subject: [RFC PATCH] drm/xe/display: Program double buffered LUT registers
>
> From PTL, LUT registers are made double buffered. This helps us to program them
> in the active region without any concern of tearing.
> This particulary helps in case of displays with high refresh rates where vblank
> periods are shorter.
>
> This patch tries to incorporates LUT programming to the noarm commit path for
> PTL without making significant changes to the color callback framework itself.
> DSB0 is still used to program to non LUT color registers (for ex. CTM). However, it
> does not chain DSB1 to program the LUT registers. Instead, it is programmed
> through intel_pre_update_crtc path.
>
> LUT programming is also disabled in the vblank worker.
Approach Looks Good to me. But we can still use DSB to program the same in active or
Is there any limitation ?
Regards,
Uma Shankar
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_color.c | 28 +++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_crtc.c | 4 ++-
> drivers/gpu/drm/i915/display/intel_display.c | 2 +-
> 3 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 7cd902bbd244..513b2718bf5a 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1911,6 +1911,16 @@ static void chv_load_luts(const struct intel_crtc_state
> *crtc_state)
> crtc_state->cgm_mode);
> }
>
> +static void ptl_color_commit_noarm(struct intel_dsb *dsb,
> + const struct intel_crtc_state *crtc_state) {
> + icl_load_csc_matrix(dsb, crtc_state);
> + if (crtc_state->preload_luts || intel_crtc_needs_modeset(crtc_state) ||
> dsb)
> + return;
> +
> + icl_load_luts(crtc_state);
> +}
> +
> void intel_color_load_luts(const struct intel_crtc_state *crtc_state) {
> struct intel_display *display = to_intel_display(crtc_state); @@ -1980,6
> +1990,9 @@ void intel_color_prepare_commit(struct intel_atomic_state *state,
> if (!crtc_state->pre_csc_lut && !crtc_state->post_csc_lut)
> return;
>
> + if (DISPLAY_VER(display) >= 30)
> + return;
> +
> crtc_state->dsb_color_vblank = intel_dsb_prepare(state, crtc,
> INTEL_DSB_1, 1024);
> if (!crtc_state->dsb_color_vblank)
> return;
> @@ -3842,6 +3855,17 @@ static const struct intel_color_funcs i9xx_color_funcs
> = {
> .get_config = i9xx_get_config,
> };
>
> +static const struct intel_color_funcs ptl_color_funcs = {
> + .color_check = icl_color_check,
> + .color_commit_noarm = ptl_color_commit_noarm,
> + .color_commit_arm = icl_color_commit_arm,
> + .load_luts = icl_load_luts,
> + .read_luts = icl_read_luts,
> + .lut_equal = icl_lut_equal,
> + .read_csc = icl_read_csc,
> + .get_config = skl_get_config,
> +};
> +
> static const struct intel_color_funcs tgl_color_funcs = {
> .color_check = icl_color_check,
> .color_commit_noarm = icl_color_commit_noarm, @@ -3989,7 +4013,9
> @@ void intel_color_init_hooks(struct intel_display *display)
> else
> display->funcs.color = &i9xx_color_funcs;
> } else {
> - if (DISPLAY_VER(display) >= 12)
> + if (DISPLAY_VER(display) >= 30)
> + display->funcs.color = &ptl_color_funcs;
> + else if (DISPLAY_VER(display) >= 12)
> display->funcs.color = &tgl_color_funcs;
> else if (DISPLAY_VER(display) == 11)
> display->funcs.color = &icl_color_funcs; diff --git
> a/drivers/gpu/drm/i915/display/intel_crtc.c
> b/drivers/gpu/drm/i915/display/intel_crtc.c
> index a2c528d707f4..cb02af401085 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -429,10 +429,12 @@ static void intel_crtc_vblank_work(struct
> kthread_work *base)
> struct intel_crtc_state *crtc_state =
> container_of(work, typeof(*crtc_state), vblank_work);
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> + struct intel_display *display = to_intel_display(crtc_state);
>
> trace_intel_crtc_vblank_work_start(crtc);
>
> - intel_color_load_luts(crtc_state);
> + if (DISPLAY_VER(display) < 30)
> + intel_color_load_luts(crtc_state);
>
> if (crtc_state->uapi.event) {
> spin_lock_irq(&crtc->base.dev->event_lock);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 35c8904ecf44..a0bcffe470e5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7203,7 +7203,7 @@ static void intel_pre_update_crtc(struct
> intel_atomic_state *state,
>
> if (!modeset &&
> intel_crtc_needs_color_update(new_crtc_state) &&
> - !new_crtc_state->use_dsb)
> + (!new_crtc_state->use_dsb || !new_crtc_state->dsb_color_vblank))
> intel_color_commit_noarm(NULL, new_crtc_state);
>
> if (!new_crtc_state->use_dsb)
> --
> 2.25.1
