[Intel-gfx] [PATCH v2 1/6] drm/i915/display/psr: Calculate selective fetch plane registers

[Mun, Gwan-gyeong]

On Tue, 2020-10-27 at 16:45 -0700, José Roberto de Souza wrote:
> Add the calculations to set plane selective fetch registers depending
> in the value of the area damaged.
> It is still using the whole plane area as damaged but that will
> change
> in next patches.
> 
> v2:
> - fixed new_plane_state->uapi.dst.y2 typo in
> intel_psr2_sel_fetch_update()
> - do not shifthing new_plane_state->uapi.dst only src is in 16.16
> format
> 
> BSpec: 55229
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
>  .../drm/i915/display/intel_display_types.h    |  2 ++
>  drivers/gpu/drm/i915/display/intel_psr.c      | 22 ++++++++++++++---
> --
>  2 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index f6f0626649e0..3f2707d882cc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -604,6 +604,8 @@ struct intel_plane_state {
>  	u32 planar_slave;
>  
>  	struct drm_intel_sprite_colorkey ckey;
> +
> +	struct drm_rect psr2_sel_fetch_area;
>  };
>  
>  struct intel_initial_plane_config {
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index b2544102e7b1..6dead51d7a81 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1187,6 +1187,7 @@ void intel_psr2_program_plane_sel_fetch(struct
> intel_plane *plane,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  	enum pipe pipe = plane->pipe;
> +	const struct drm_rect *clip;
>  	u32 val;
>  
>  	if (!crtc_state->enable_psr2_sel_fetch)
> @@ -1198,16 +1199,20 @@ void
> intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
>  	if (!val || plane->id == PLANE_CURSOR)
>  		return;
>  
> -	val = plane_state->uapi.dst.y1 << 16 | plane_state-
> >uapi.dst.x1;
> +	clip = &plane_state->psr2_sel_fetch_area;
> +
> +	val = (clip->y1 + plane_state->uapi.dst.y1) << 16;
> +	val |= plane_state->uapi.dst.x1;
>  	intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane-
> >id), val);
>  
> -	val = plane_state->color_plane[color_plane].y << 16;
> +	/* TODO: consider tiling and auxiliary surfaces */
> +	val = (clip->y1 + plane_state->color_plane[color_plane].y) <<
> 16;
>  	val |= plane_state->color_plane[color_plane].x;
>  	intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane-
> >id),
>  			  val);
>  
>  	/* Sizes are 0 based */
> -	val = ((drm_rect_height(&plane_state->uapi.src) >> 16) - 1) <<
> 16;
> +	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);
>  }
> @@ -1281,7 +1286,7 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
>  
>  	for_each_oldnew_intel_plane_in_state(state, plane,
> old_plane_state,
>  					     new_plane_state, i) {
> -		struct drm_rect temp;
> +		struct drm_rect *sel_fetch_area, temp;
>  
>  		if (new_plane_state->uapi.crtc != crtc_state-
> >uapi.crtc)
>  			continue;
> @@ -1304,8 +1309,13 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
>  		 * For now doing a selective fetch in the whole plane
> area,
>  		 * optimizations will come in the future.
>  		 */
> -		temp.y1 = new_plane_state->uapi.dst.y1;
> -		temp.y2 = new_plane_state->uapi.dst.y2;
> +		sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> +		sel_fetch_area->y1 = new_plane_state->uapi.src.y1 >>
> 16;
> +		sel_fetch_area->y2 = new_plane_state->uapi.src.y2 >>
> 16;
> +
> +		temp = *sel_fetch_area;
> +		temp.y1 += new_plane_state->uapi.dst.y1;
> +		temp.y2 += new_plane_state->uapi.dst.y2;
when the userspace call drmModeSetPlane(), src_y and crtc_y can be
differ.
(drm core uses drm_rect_translate_to() utility function for apply
coordinate traslate to dst from src. )

In my opinion, we can calculate like as,
int coord_trans = new_plane_state->uapi.dst.y1 - new_plane_state-
>uapi.src.y1 >> 16;
temp = *sel_fetch_area; // assunes that sel_fetch_area has damage area.
temp.y1 += coord_trans;
temp.y2 += coord_trans;

>  		clip_area_update(&pipe_clip, &temp);

As we don't check and calculate plane rotation and scale for dst
coordinates here.
so can you add checking of plane scale or rotation? and if the plane
has a scale factor or rotation, for now, we should not apply PSR SU.
(if plane scale or rotation are used, dst might differ. if we don't
apply PSR SU, we don't need to call clip_area_update() here. )
>  	}
>