> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.bo...@intel.com>
> Sent: Tuesday, April 8, 2025 4:30 PM
> To: intel...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: ville.syrj...@linux.intel.com; Shankar, Uma <uma.shan...@intel.com>;
> Borah, Chaitanya Kumar <chaitanya.kumar.bo...@intel.com>; Manna, Animesh
> <animesh.ma...@intel.com>
> Subject: [PATCH v2 08/11] drm/i915: use GOSUB to program doubled buffered
Nit: s/doubled/double
Also add display as prefix in header (drm/i915/display)
Overall changes look good to me.
Reviewed-by: Uma Shankar <uma.shan...@intel.com>
> LUT registers
>
> 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
>
> v2: Refactor code to simplify commit completion flow.
> Add some helpers along the way (Ville)
>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.bo...@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_color.c | 39 ++++++++++++++-----
> drivers/gpu/drm/i915/display/intel_color.h | 2 +
> drivers/gpu/drm/i915/display/intel_display.c | 24 +++++++-----
> .../drm/i915/display/intel_display_device.h | 1 +
> 4 files changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index bb2da3a53e9c..ac00b86798fb 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1965,6 +1965,25 @@ void intel_color_modeset(const struct
> intel_crtc_state *crtc_state)
> }
> }
>
> +bool intel_color_uses_dsb(const struct intel_crtc_state *crtc_state) {
> + return crtc_state->dsb_color;
> +}
> +
> +bool intel_color_uses_chained_dsb(const struct intel_crtc_state
> +*crtc_state) {
> + struct intel_display *display = to_intel_display(crtc_state);
> +
> + return crtc_state->dsb_color &&
> !HAS_DOUBLE_BUFFERED_LUT(display);
> +}
> +
> +bool intel_color_uses_gosub_dsb(const struct intel_crtc_state
> +*crtc_state) {
> + struct intel_display *display = to_intel_display(crtc_state);
> +
> + return crtc_state->dsb_color && HAS_DOUBLE_BUFFERED_LUT(display);
> }
> +
> void intel_color_prepare_commit(struct intel_atomic_state *state,
> struct intel_crtc *crtc)
> {
> @@ -1982,20 +2001,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 (!crtc_state->dsb_color)
> + 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 (!intel_color_uses_dsb(crtc_state))
> return;
>
> display->funcs.color->load_luts(crtc_state);
>
> - if (crtc_state->use_dsb) {
> + if (crtc_state->use_dsb && intel_color_uses_chained_dsb(crtc_state)) {
> 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 (intel_color_uses_gosub_dsb(crtc_state))
> + 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) @@ -
> 2012,11 +2038,6 @@ void intel_color_wait_commit(const struct intel_crtc_state
> *crtc_state)
> intel_dsb_wait(crtc_state->dsb_color);
> }
>
> -bool intel_color_uses_dsb(const struct intel_crtc_state *crtc_state) -{
> - return crtc_state->dsb_color;
> -}
> -
> static bool intel_can_preload_luts(struct intel_atomic_state *state,
> struct intel_crtc *crtc)
> {
> diff --git a/drivers/gpu/drm/i915/display/intel_color.h
> b/drivers/gpu/drm/i915/display/intel_color.h
> index 9d66457c1e89..bf7a12ce9df0 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.h
> +++ b/drivers/gpu/drm/i915/display/intel_color.h
> @@ -24,6 +24,8 @@ void intel_color_prepare_commit(struct intel_atomic_state
> *state,
> struct intel_crtc *crtc);
> void intel_color_cleanup_commit(struct intel_crtc_state *crtc_state); bool
> intel_color_uses_dsb(const struct intel_crtc_state *crtc_state);
> +bool intel_color_uses_chained_dsb(const struct intel_crtc_state
> +*crtc_state); bool intel_color_uses_gosub_dsb(const struct
> +intel_crtc_state *crtc_state);
> void intel_color_wait_commit(const struct intel_crtc_state *crtc_state);
> void
> intel_color_commit_noarm(struct intel_dsb *dsb,
> const 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 c940ffb500e0..58a84829fe58 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7230,20 +7230,24 @@ static void intel_atomic_dsb_finish(struct
> intel_atomic_state *state,
> if (DISPLAY_VER(display) >= 9)
> skl_detach_scalers(new_crtc_state->dsb_commit,
> new_crtc_state);
> -
> - if (!new_crtc_state->dsb_color) {
> - 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);
> - }
> }
>
> - if (new_crtc_state->dsb_color)
> + if (intel_color_uses_chained_dsb(new_crtc_state))
> intel_dsb_chain(state, new_crtc_state->dsb_commit,
> new_crtc_state->dsb_color, true);
> + else if (intel_color_uses_gosub_dsb(new_crtc_state))
> + intel_dsb_gosub(new_crtc_state->dsb_commit,
> + new_crtc_state->dsb_color);
> +
> + if (new_crtc_state->use_dsb &&
> !intel_color_uses_chained_dsb(new_crtc_state)) {
> + 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);
> + }
>
> 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
