On Mon, Apr 07, 2025 at 07:53:56PM +0530, Chaitanya Kumar Borah wrote:
> With addition of double buffered GAMMA registers in PTL, we can now
> program them in the active region. Use GOSUB instruction of DSB to
> program them.
>
> It is done in the following steps:
> 1. intel_color_prepare_commit()
> - If the platform supports, prepare a dsb instance (dsb_color)
> hooked to DSB0.
> - Add all the register write instructions to dsb_color through
> the load_lut() hook
> - Do not add the vrr_send_push() logic to the buffer as it
> should be taken care by dsb_commit instance of DSB0
> - Finish preparation of the buffer by aligning it to 64 bit
>
> 2. intel_atomic_dsb_finish()
> - Add the gosub instruction into the dsb_commit instance of DSB0
> using intel_dsb_gosub()
> - If needed, add the vrr_send_push() logic to dsb_commit after
> it
>
> Signed-off-by: Chaitanya Kumar Borah <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_color.c | 13 ++++++++---
> drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++++++++---
> .../drm/i915/display/intel_display_device.h | 1 +
> 3 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index bb2da3a53e9c..49429404bd82 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1982,20 +1982,27 @@ void intel_color_prepare_commit(struct
> intel_atomic_state *state,
> if (!crtc_state->pre_csc_lut && !crtc_state->post_csc_lut)
> return;
>
> - crtc_state->dsb_color = intel_dsb_prepare(state, crtc, INTEL_DSB_1,
> 1024);
> + if (HAS_DOUBLE_BUFFERED_LUT(display))
> + crtc_state->dsb_color = intel_dsb_prepare(state, crtc,
> INTEL_DSB_0, 1024);
> + else
> + crtc_state->dsb_color = intel_dsb_prepare(state, crtc,
> INTEL_DSB_1, 1024);
> +
> if (!crtc_state->dsb_color)
> return;
>
> display->funcs.color->load_luts(crtc_state);
>
> - if (crtc_state->use_dsb) {
> + if (crtc_state->use_dsb && !HAS_DOUBLE_BUFFERED_LUT(display)) {
> intel_vrr_send_push(crtc_state->dsb_color, crtc_state);
> intel_dsb_wait_vblank_delay(state, crtc_state->dsb_color);
> intel_vrr_check_push_sent(crtc_state->dsb_color, crtc_state);
> intel_dsb_interrupt(crtc_state->dsb_color);
> }
>
> - intel_dsb_finish(crtc_state->dsb_color);
> + if (HAS_DOUBLE_BUFFERED_LUT(display))
> + intel_dsb_gosub_finish(crtc_state->dsb_color);
> + else
> + intel_dsb_finish(crtc_state->dsb_color);
> }
>
> void intel_color_cleanup_commit(struct intel_crtc_state *crtc_state)
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 69c1790199d3..85e28b4c9e66 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7239,9 +7239,25 @@ static void intel_atomic_dsb_finish(struct
> intel_atomic_state *state,
> }
> }
>
> - if (new_crtc_state->dsb_color)
> - intel_dsb_chain(state, new_crtc_state->dsb_commit,
> - new_crtc_state->dsb_color, true);
> + if (new_crtc_state->dsb_color) {
> + if (HAS_DOUBLE_BUFFERED_LUT(display)) {
> + intel_dsb_gosub(new_crtc_state->dsb_commit,
> + new_crtc_state->dsb_color);
> +
> + if (new_crtc_state->use_dsb) {
> +
> intel_dsb_wait_vblanks(new_crtc_state->dsb_commit, 1);
> +
> + intel_vrr_send_push(new_crtc_state->dsb_commit,
> new_crtc_state);
> + intel_dsb_wait_vblank_delay(state,
> new_crtc_state->dsb_commit);
> +
> intel_vrr_check_push_sent(new_crtc_state->dsb_commit,
> + new_crtc_state);
> + intel_dsb_interrupt(new_crtc_state->dsb_commit);
> + }
> + } else {
> + intel_dsb_chain(state, new_crtc_state->dsb_commit,
> + new_crtc_state->dsb_color, true);
> + }
> + }
The logic around the commit completion is getting very messy (it already
was pretty bad though).
maybe something like this:
bool intel_color_use_chained_dsb()
{
return dsb_color && !HAS_DOUBLE_BUFFERED_LUT;
}
if (use_dsb) {
do pipe/plane programming
}
if (use_chained_dsb())
dsb_chain();
else if (dsb_color)
dsb_gosub();
if (use_dsb && !use_chained_dsb() {
do commit completion
}
>
> intel_dsb_finish(new_crtc_state->dsb_commit);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
> b/drivers/gpu/drm/i915/display/intel_display_device.h
> index 368b0d3417c2..14943b47824b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -157,6 +157,7 @@ struct intel_display_platforms {
> #define HAS_DMC(__display)
> (DISPLAY_RUNTIME_INFO(__display)->has_dmc)
> #define HAS_DMC_WAKELOCK(__display) (DISPLAY_VER(__display) >= 20)
> #define HAS_DOUBLE_BUFFERED_M_N(__display) (DISPLAY_VER(__display) >= 9 ||
> (__display)->platform.broadwell)
> +#define HAS_DOUBLE_BUFFERED_LUT(__display) (DISPLAY_VER(__display) >= 30)
> #define HAS_DOUBLE_WIDE(__display) (DISPLAY_VER(__display) < 4)
> #define HAS_DP20(__display) ((__display)->platform.dg2 ||
> DISPLAY_VER(__display) >= 14)
> #define HAS_DPT(__display) (DISPLAY_VER(__display) >= 13)
> --
> 2.25.1
--
Ville Syrjälä
Intel
