[Intel-gfx] [PATCH 2/6] drm/i915/display/psr: Use plane damage clips to calculate damaged area

[Mun, Gwan-gyeong]

On Tue, 2020-10-13 at 16:01 -0700, José Roberto de Souza wrote:
> Now using plane damage clips property to calcualte the damaged area.
> Selective fetch only supports one region to be fetched so software
> needs to calculate a bounding box around all damage clips.
> 
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 54 +++++++++++++++++++++-
> --
>  1 file changed, 49 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 773a5b5fa078..0f1e9f0fa57f 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1273,6 +1273,9 @@ 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 *sel_fetch_area, temp;
> +		struct drm_mode_rect *damaged_clips;
> +		u32 num_clips;
> +		int j;
>  
>  		if (new_plane_state->uapi.crtc != crtc_state-
> >uapi.crtc)
>  			continue;
> @@ -1291,13 +1294,54 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
>  		if (!new_plane_state->uapi.visible)
>  			continue;
>  
> +		sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> +		sel_fetch_area->y1 = -1;
> +
> +		damaged_clips =
> drm_plane_get_damage_clips(&new_plane_state->uapi);
> +		num_clips =
> drm_plane_get_damage_clips_count(&new_plane_state->uapi);
> +
>  		/*
> -		 * For now doing a selective fetch in the whole plane
> area,
> -		 * optimizations will come in the future.
> +		 * If plane moved, mark the whole plane area as damaged
> as it
> +		 * needs to be complete redraw in the new position.
>  		 */
> -		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;
> +		if (!drm_rect_equals(&new_plane_state->uapi.dst,
> +				     &old_plane_state->uapi.dst)) {
> +			num_clips = 0;
> +			sel_fetch_area->y1 = new_plane_state-
> >uapi.src.y1 >> 16;
> +			sel_fetch_area->y2 = new_plane_state-
> >uapi.src.y2 >> 16;
> +		} else if (!num_clips && new_plane_state->uapi.fb !=
> +			   old_plane_state->uapi.fb) {
> +			/*
> +			 * If the plane don't have damage areas but the
> +			 * framebuffer changed, mark the whole plane
> area as
> +			 * damaged.
> +			 */
> +			sel_fetch_area->y1 = new_plane_state-
> >uapi.src.y1 >> 16;
> +			sel_fetch_area->y2 = new_plane_state-
> >uapi.src.y2 >> 16;
> +		}
> +
why don't you use drm_atomic_helper_damage_merged() function here?
> +		for (j = 0; j < num_clips; j++) {
> +			struct drm_rect damage_area;
> +
> +			damage_area.y1 = damaged_clips[j].y1;
> +			damage_area.y2 = damaged_clips[j].y2;
> +			clip_area_update(sel_fetch_area, &damage_area);
> +		}
> +
> +		/*
> +		 * No page flip, no plane moviment or no damage areas,
> so don't
typo (moviment -> movement)
> +		 * fetch any pixel from memory for this plane
> +		 */
> +		if (sel_fetch_area->y1 == -1) {
> +			sel_fetch_area->y1 = 0;
> +			sel_fetch_area->y2 = 0;
> +		}
> +
> +		/* Don't need to redraw plane damaged areas outside of
> screen */
> +		j = sel_fetch_area->y2 + (new_plane_state->uapi.dst.y1
> >> 16);
src coordinates of the drm_plane_state are 16.16 fixed point but dst
coordinates are not 16.16 fixed point.
therefore we don't need to bit shift for dst.
Because the sel_fetch_area seems based on src coordinates, in order to
apply to dst coordinates here,  it requires coordinate calculation. 
> +		j = crtc_state->uapi.adjusted_mode.crtc_vdisplay - j;
> +		if (j < 0)
> +			sel_fetch_area->y2 += j;
>  
>  		temp = *sel_fetch_area;
>  		temp.y1 += new_plane_state->uapi.dst.y1 >> 16;