[Intel-gfx] [PATCH v2] drm/i915/psr: Split sel fetch plane configuration into arm and noarm

Souza, Jose jose.souza at intel.com
Fri Jan 27 14:00:16 UTC 2023


On Fri, 2023-01-27 at 10:27 +0200, Jouni Högander wrote:
> SEL_FETCH_CTL registers are armed immediately when plane is disabled.
> SEL_FETCH_* instances of plane configuration are used when doing
> selective update and normal plane register instances for full updates.
> Currently all SEL_FETCH_* registers are written as a part of noarm
> plane configuration. If noarm and arm plane configuration are not
> happening within same vblank we may end up having plane as a part of
> selective update before it's PLANE_SURF register is written.
> 
> Fix this by splitting plane selective fetch configuration into arm and
> noarm versions and call them accordingly. Write SEL_FETCH_CTL in arm
> version.

Does this helps to revert the set of SFF and CFF at the same time?

> 
> v2:
>  - drop color_plane parameter from arm part
>  - dev_priv -> i915 in arm part
> 
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: José Roberto de Souza <jose.souza at intel.com>
> Cc: Mika Kahola <mika.kahola at intel.com>
> Cc: Vinod Govindapillai <vinod.govindapillai at intel.com>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> Cc: Luca Coelho <luciano.coelho at intel.com>
> Signed-off-by: Jouni Högander <jouni.hogander at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cursor.c   |  3 +-
>  drivers/gpu/drm/i915/display/intel_psr.c      | 28 +++++++++++++------
>  drivers/gpu/drm/i915/display/intel_psr.h      |  6 +++-
>  .../drm/i915/display/skl_universal_plane.c    |  4 ++-
>  4 files changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
> index d190fa0d393b..ae9f0b6c92db 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -532,7 +532,8 @@ static void i9xx_cursor_update_arm(struct intel_plane *plane,
>  		skl_write_cursor_wm(plane, crtc_state);
>  
>  	if (plane_state)
> -		intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, 0);
> +		intel_psr2_program_plane_sel_fetch_arm(plane, crtc_state,
> +						       plane_state);
>  	else
>  		intel_psr2_disable_plane_sel_fetch(plane, crtc_state);

Missing rename intel_psr2_disable_plane_sel_fetch() to intel_psr2_disable_plane_sel_fetch_arm().

With this LGTM.
Reviewed-by: José Roberto de Souza <jose.souza at intel.com>


>  
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 7a72e15e6836..a3f4451eb66d 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1559,7 +1559,25 @@ void intel_psr2_disable_plane_sel_fetch(struct intel_plane *plane,
>  	intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id), 0);
>  }
>  
> -void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> +void intel_psr2_program_plane_sel_fetch_arm(struct intel_plane *plane,
> +					const struct intel_crtc_state *crtc_state,
> +					const struct intel_plane_state *plane_state)
> +{
> +	struct drm_i915_private *i915 = to_i915(plane->base.dev);
> +	enum pipe pipe = plane->pipe;
> +
> +	if (!crtc_state->enable_psr2_sel_fetch)
> +		return;
> +
> +	if (plane->id == PLANE_CURSOR)
> +		intel_de_write_fw(i915, PLANE_SEL_FETCH_CTL(pipe, plane->id),
> +				  plane_state->ctl);
> +	else
> +		intel_de_write_fw(i915, PLANE_SEL_FETCH_CTL(pipe, plane->id),
> +				  PLANE_SEL_FETCH_CTL_ENABLE);
> +}
> +
> +void intel_psr2_program_plane_sel_fetch_noarm(struct intel_plane *plane,
>  					const struct intel_crtc_state *crtc_state,
>  					const struct intel_plane_state *plane_state,
>  					int color_plane)
> @@ -1573,11 +1591,8 @@ void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
>  	if (!crtc_state->enable_psr2_sel_fetch)
>  		return;
>  
> -	if (plane->id == PLANE_CURSOR) {
> -		intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id),
> -				  plane_state->ctl);
> +	if (plane->id == PLANE_CURSOR)
>  		return;
> -	}
>  
>  	clip = &plane_state->psr2_sel_fetch_area;
>  
> @@ -1605,9 +1620,6 @@ void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
>  	val = (drm_rect_height(clip) - 1) << 16;
>  	val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - 1;
>  	intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane->id), val);
> -
> -	intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id),
> -			  PLANE_SEL_FETCH_CTL_ENABLE);
>  }
>  
>  void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state)
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
> index 2ac3a46cccc5..c87ae2e6ee6c 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -46,10 +46,14 @@ bool intel_psr_enabled(struct intel_dp *intel_dp);
>  int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>  				struct intel_crtc *crtc);
>  void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state);
> -void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> +void intel_psr2_program_plane_sel_fetch_noarm(struct intel_plane *plane,
>  					const struct intel_crtc_state *crtc_state,
>  					const struct intel_plane_state *plane_state,
>  					int color_plane);
> +void intel_psr2_program_plane_sel_fetch_arm(struct intel_plane *plane,
> +					const struct intel_crtc_state *crtc_state,
> +					const struct intel_plane_state *plane_state);
> +
>  void intel_psr2_disable_plane_sel_fetch(struct intel_plane *plane,
>  					const struct intel_crtc_state *crtc_state);
>  void intel_psr_pause(struct intel_dp *intel_dp);
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 9b172a1e90de..296da4bc297c 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1260,7 +1260,7 @@ icl_plane_update_noarm(struct intel_plane *plane,
>  	if (plane_state->force_black)
>  		icl_plane_csc_load_black(plane);
>  
> -	intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane);
> +	intel_psr2_program_plane_sel_fetch_noarm(plane, crtc_state, plane_state, color_plane);
>  }
>  
>  static void
> @@ -1287,6 +1287,8 @@ icl_plane_update_arm(struct intel_plane *plane,
>  	if (plane_state->scaler_id >= 0)
>  		skl_program_plane_scaler(plane, crtc_state, plane_state);
>  
> +	intel_psr2_program_plane_sel_fetch_arm(plane, crtc_state, plane_state);
> +
>  	/*
>  	 * The control register self-arms if the plane was previously
>  	 * disabled. Try to make the plane enable atomic by writing



More information about the Intel-gfx mailing list