Re: [PATCH 10/18] drm/i915: Consolidate intel_pre_commit_crtc_state()

["Nautiyal, Ankit K" <ankit.k.nautiyal@xxxxxxxxx>]

On 12/11/2024 2:39 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> We have approximately two copies of pre_commit_crtc_state(),
> one in the DSB code, the other in the vblank evasion code.
> Combine them into one. The slight difference between the two
> is that vblank evasion doesn't have a full atomic state (when
> called from the legacy cursor code), so it gets the old and
> new crtc state passed in by hand.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>


> ---
>   drivers/gpu/drm/i915/display/intel_dsb.c    | 37 +++++++-------------
>   drivers/gpu/drm/i915/display/intel_vblank.c | 38 +++++++++++++++------
>   drivers/gpu/drm/i915/display/intel_vblank.h |  5 +++
>   3 files changed, 45 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index d755825edb35..673f75703785 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -109,25 +109,6 @@ static bool pre_commit_is_vrr_active(struct intel_atomic_state *state,
>   	return old_crtc_state->vrr.enable && !intel_crtc_vrr_disabling(state, crtc);
>   }
>   
> -static const struct intel_crtc_state *
> -pre_commit_crtc_state(struct intel_atomic_state *state,
> -		      struct intel_crtc *crtc)
> -{
> -	const struct intel_crtc_state *old_crtc_state =
> -		intel_atomic_get_old_crtc_state(state, crtc);
> -	const struct intel_crtc_state *new_crtc_state =
> -		intel_atomic_get_new_crtc_state(state, crtc);
> -
> -	/*
> -	 * During fastsets/etc. the transcoder is still
> -	 * running with the old timings at this point.
> -	 */
> -	if (intel_crtc_needs_modeset(new_crtc_state))
> -		return new_crtc_state;
> -	else
> -		return old_crtc_state;
> -}
> -
>   static int dsb_vblank_delay(const struct intel_crtc_state *crtc_state)
>   {
>   	return intel_mode_vblank_delay(&crtc_state->hw.adjusted_mode);
> @@ -136,7 +117,8 @@ static int dsb_vblank_delay(const struct intel_crtc_state *crtc_state)
>   static int dsb_vtotal(struct intel_atomic_state *state,
>   		      struct intel_crtc *crtc)
>   {
> -	const struct intel_crtc_state *crtc_state = pre_commit_crtc_state(state, crtc);
> +	const struct intel_crtc_state *crtc_state =
> +		intel_pre_commit_crtc_state(state, crtc);
>   
>   	if (pre_commit_is_vrr_active(state, crtc))
>   		return intel_vrr_vmax_vtotal(crtc_state);
> @@ -147,7 +129,8 @@ static int dsb_vtotal(struct intel_atomic_state *state,
>   static int dsb_dewake_scanline_start(struct intel_atomic_state *state,
>   				     struct intel_crtc *crtc)
>   {
> -	const struct intel_crtc_state *crtc_state = pre_commit_crtc_state(state, crtc);
> +	const struct intel_crtc_state *crtc_state =
> +		intel_pre_commit_crtc_state(state, crtc);
>   	struct drm_i915_private *i915 = to_i915(state->base.dev);
>   	unsigned int latency = skl_watermark_max_latency(i915, 0);
>   
> @@ -158,7 +141,8 @@ static int dsb_dewake_scanline_start(struct intel_atomic_state *state,
>   static int dsb_dewake_scanline_end(struct intel_atomic_state *state,
>   				   struct intel_crtc *crtc)
>   {
> -	const struct intel_crtc_state *crtc_state = pre_commit_crtc_state(state, crtc);
> +	const struct intel_crtc_state *crtc_state =
> +		intel_pre_commit_crtc_state(state, crtc);
>   
>   	return intel_mode_vdisplay(&crtc_state->hw.adjusted_mode);
>   }
> @@ -166,7 +150,8 @@ static int dsb_dewake_scanline_end(struct intel_atomic_state *state,
>   static int dsb_scanline_to_hw(struct intel_atomic_state *state,
>   			      struct intel_crtc *crtc, int scanline)
>   {
> -	const struct intel_crtc_state *crtc_state = pre_commit_crtc_state(state, crtc);
> +	const struct intel_crtc_state *crtc_state =
> +		intel_pre_commit_crtc_state(state, crtc);
>   	int vtotal = dsb_vtotal(state, crtc);
>   
>   	return (scanline + vtotal - intel_crtc_scanline_offset(crtc_state)) % vtotal;
> @@ -531,7 +516,8 @@ void intel_dsb_vblank_evade(struct intel_atomic_state *state,
>   			    struct intel_dsb *dsb)
>   {
>   	struct intel_crtc *crtc = dsb->crtc;
> -	const struct intel_crtc_state *crtc_state = pre_commit_crtc_state(state, crtc);
> +	const struct intel_crtc_state *crtc_state =
> +		intel_pre_commit_crtc_state(state, crtc);
>   	/* FIXME calibrate sensibly */
>   	int latency = intel_usecs_to_scanlines(&crtc_state->hw.adjusted_mode, 20);
>   	int vblank_delay = dsb_vblank_delay(crtc_state);
> @@ -623,7 +609,8 @@ void intel_dsb_wait_vblank_delay(struct intel_atomic_state *state,
>   				 struct intel_dsb *dsb)
>   {
>   	struct intel_crtc *crtc = dsb->crtc;
> -	const struct intel_crtc_state *crtc_state = pre_commit_crtc_state(state, crtc);
> +	const struct intel_crtc_state *crtc_state =
> +		intel_pre_commit_crtc_state(state, crtc);
>   	int usecs = intel_scanlines_to_usecs(&crtc_state->hw.adjusted_mode,
>   					     dsb_vblank_delay(crtc_state)) + 1;
>   
> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c b/drivers/gpu/drm/i915/display/intel_vblank.c
> index ee33a49d9873..3cf3cff0687a 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> @@ -602,6 +602,32 @@ int intel_mode_vblank_delay(const struct drm_display_mode *mode)
>   	return intel_mode_vblank_start(mode) - intel_mode_vdisplay(mode);
>   }
>   
> +static const struct intel_crtc_state *
> +pre_commit_crtc_state(const struct intel_crtc_state *old_crtc_state,
> +		      const struct intel_crtc_state *new_crtc_state)
> +{
> +	/*
> +	 * During fastsets/etc. the transcoder is still
> +	 * running with the old timings at this point.
> +	 */
> +	if (intel_crtc_needs_modeset(new_crtc_state))
> +		return new_crtc_state;
> +	else
> +		return old_crtc_state;
> +}
> +
> +const struct intel_crtc_state *
> +intel_pre_commit_crtc_state(struct intel_atomic_state *state,
> +			    struct intel_crtc *crtc)
> +{
> +	const struct intel_crtc_state *old_crtc_state =
> +		intel_atomic_get_old_crtc_state(state, crtc);
> +	const struct intel_crtc_state *new_crtc_state =
> +		intel_atomic_get_new_crtc_state(state, crtc);
> +
> +	return pre_commit_crtc_state(old_crtc_state, new_crtc_state);
> +}
> +
>   void intel_vblank_evade_init(const struct intel_crtc_state *old_crtc_state,
>   			     const struct intel_crtc_state *new_crtc_state,
>   			     struct intel_vblank_evade_ctx *evade)
> @@ -617,16 +643,8 @@ void intel_vblank_evade_init(const struct intel_crtc_state *old_crtc_state,
>   				  display->platform.cherryview) &&
>   		intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI);
>   
> -	/*
> -	 * During fastsets/etc. the transcoder is still
> -	 * running with the old timings at this point.
> -	 *
> -	 * TODO: maybe just use the active timings here?
> -	 */
> -	if (intel_crtc_needs_modeset(new_crtc_state))
> -		crtc_state = new_crtc_state;
> -	else
> -		crtc_state = old_crtc_state;
> +	/* TODO: maybe just use the active timings here? */
> +	crtc_state = pre_commit_crtc_state(old_crtc_state, new_crtc_state);
>   
>   	adjusted_mode = &crtc_state->hw.adjusted_mode;
>   
> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.h b/drivers/gpu/drm/i915/display/intel_vblank.h
> index 12c8e1f829a4..21fbb08d61d5 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.h
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.h
> @@ -11,6 +11,7 @@
>   
>   struct drm_crtc;
>   struct drm_display_mode;
> +struct intel_atomic_state;
>   struct intel_crtc;
>   struct intel_crtc_state;
>   
> @@ -43,4 +44,8 @@ void intel_crtc_update_active_timings(const struct intel_crtc_state *crtc_state,
>   				      bool vrr_enable);
>   int intel_crtc_scanline_offset(const struct intel_crtc_state *crtc_state);
>   
> +const struct intel_crtc_state *
> +intel_pre_commit_crtc_state(struct intel_atomic_state *state,
> +			    struct intel_crtc *crtc);
> +
>   #endif /* __INTEL_VBLANK_H__ */