[PATCH 14/14] drm/i915/dsb: Use chained DSBs for LUT programming

[Manna, Animesh]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Tuesday, June 25, 2024 12:41 AM
> To: intel-gfx at lists.freedesktop.org
> Subject: [PATCH 14/14] drm/i915/dsb: Use chained DSBs for LUT
> programming
> 
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> In order to better handle the necessary DSB DEwake tricks let's switch over to
> using a chained DSB for the actual LUT programming.
> The CPU will start 'dsb_color_commit', which in turn will start the chained
> 'dsb_color_vblank'.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Reviewed-by: Animesh Manna <animesh.manna at intel.com>
 
> ---
>  drivers/gpu/drm/i915/display/intel_atomic.c   |  2 ++
>  drivers/gpu/drm/i915/display/intel_color.c    | 32 +++++++++++++++----
>  drivers/gpu/drm/i915/display/intel_display.c  |  1 +
>  .../drm/i915/display/intel_display_types.h    |  2 +-
>  4 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index 55ce71be41ec..12d6ed940751 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -277,6 +277,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>  	crtc_state->fb_bits = 0;
>  	crtc_state->update_planes = 0;
>  	crtc_state->dsb_color_vblank = NULL;
> +	crtc_state->dsb_color_commit = NULL;
> 
>  	return &crtc_state->uapi;
>  }
> @@ -311,6 +312,7 @@ intel_crtc_destroy_state(struct drm_crtc *crtc,
>  	struct intel_crtc_state *crtc_state = to_intel_crtc_state(state);
> 
>  	drm_WARN_ON(crtc->dev, crtc_state->dsb_color_vblank);
> +	drm_WARN_ON(crtc->dev, crtc_state->dsb_color_commit);
> 
>  	__drm_atomic_helper_crtc_destroy_state(&crtc_state->uapi);
>  	intel_crtc_free_hw_state(crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 27acbf92d60f..5d701f48351b 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1890,8 +1890,8 @@ void intel_color_commit_arm(const struct
> intel_crtc_state *crtc_state)
> 
>  	i915->display.funcs.color->color_commit_arm(crtc_state);
> 
> -	if (crtc_state->dsb_color_vblank)
> -		intel_dsb_commit(crtc_state->dsb_color_vblank, true);
> +	if (crtc_state->dsb_color_commit)
> +		intel_dsb_commit(crtc_state->dsb_color_commit, false);
>  }
> 
>  void intel_color_post_update(const struct intel_crtc_state *crtc_state) @@ -
> 1919,26 +1919,44 @@ 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_vblank = intel_dsb_prepare(state, crtc,
> INTEL_DSB_0, 1024);
> +	crtc_state->dsb_color_vblank = intel_dsb_prepare(state, crtc,
> +INTEL_DSB_1, 1024);
>  	if (!crtc_state->dsb_color_vblank)
>  		return;
> 
>  	i915->display.funcs.color->load_luts(crtc_state);
> 
>  	intel_dsb_finish(crtc_state->dsb_color_vblank);
> +
> +	crtc_state->dsb_color_commit = intel_dsb_prepare(state, crtc,
> INTEL_DSB_0, 16);
> +	if (!crtc_state->dsb_color_commit) {
> +		intel_dsb_cleanup(crtc_state->dsb_color_vblank);
> +		crtc_state->dsb_color_vblank = NULL;
> +		return;
> +	}
> +
> +	intel_dsb_chain(state, crtc_state->dsb_color_commit,
> +			crtc_state->dsb_color_vblank, true);
> +
> +	intel_dsb_finish(crtc_state->dsb_color_commit);
>  }
> 
>  void intel_color_cleanup_commit(struct intel_crtc_state *crtc_state)  {
> -	if (!crtc_state->dsb_color_vblank)
> -		return;
> +	if (crtc_state->dsb_color_commit) {
> +		intel_dsb_cleanup(crtc_state->dsb_color_commit);
> +		crtc_state->dsb_color_commit = NULL;
> +	}
> 
> -	intel_dsb_cleanup(crtc_state->dsb_color_vblank);
> -	crtc_state->dsb_color_vblank = NULL;
> +	if (crtc_state->dsb_color_vblank) {
> +		intel_dsb_cleanup(crtc_state->dsb_color_vblank);
> +		crtc_state->dsb_color_vblank = NULL;
> +	}
>  }
> 
>  void intel_color_wait_commit(const struct intel_crtc_state *crtc_state)  {
> +	if (crtc_state->dsb_color_commit)
> +		intel_dsb_wait(crtc_state->dsb_color_commit);
>  	if (crtc_state->dsb_color_vblank)
>  		intel_dsb_wait(crtc_state->dsb_color_vblank);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 05a2a6942000..d5e0fa5c78b5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7511,6 +7511,7 @@ static void intel_atomic_commit_tail(struct
> intel_atomic_state *state)
>  		 * FIXME get rid of this funny new->old swapping
>  		 */
>  		old_crtc_state->dsb_color_vblank =
> fetch_and_zero(&new_crtc_state->dsb_color_vblank);
> +		old_crtc_state->dsb_color_commit =
> +fetch_and_zero(&new_crtc_state->dsb_color_commit);
>  	}
> 
>  	/* Underruns don't always raise interrupts, so check manually */ diff
> --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index bd079cd77bda..f22de0495dd7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1397,7 +1397,7 @@ struct intel_crtc_state {
>  	enum transcoder mst_master_transcoder;
> 
>  	/* For DSB based color LUT updates */
> -	struct intel_dsb *dsb_color_vblank;
> +	struct intel_dsb *dsb_color_vblank, *dsb_color_commit;
> 
>  	u32 psr2_man_track_ctl;
> 
> --
> 2.44.2