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

[Souza, Jose]

On Mon, 2020-12-14 at 11:00 +0000, Mun, Gwan-gyeong wrote:
> On Sun, 2020-12-13 at 10:39 -0800, 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.
> > 
> > Now that we are not complete fetching each plane, there is another
> > loop needed as all the plane areas that intersect with the pipe
> > damadged area needs to be fetched from memory so the complete
> > blending
> > of all planes can happen.
> > 
> > v2:
> > - do not shifthing new_plane_state->uapi.dst only src is in 16.16
> > format
> > 
> > v4:
> > - setting plane selective fetch area using the whole pipe damage area
> > - mark the whole plane area damaged if plane visibility or alpha
> > changed
> > 
> > v5:
> > - taking in consideration src.y1 in the damage coordinates
> > - adding to the pipe damaged area planes that were visible but are
> > invisible in the new state
> > 
> > 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 | 93 ++++++++++++++++++++
> > ----
> >  1 file changed, 79 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index d9a395c486d3..b256184821da 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1269,11 +1269,11 @@ int intel_psr2_sel_fetch_update(struct
> > intel_atomic_state *state,
> >  				struct intel_crtc *crtc)
> >  {
> >  	struct intel_crtc_state *crtc_state =
> > intel_atomic_get_new_crtc_state(state, crtc);
> > +	struct drm_rect pipe_clip = { .x1 = 0, .y1 = -1, .x2 = INT_MAX,
> > .y2 = -1 };
> >  	struct intel_plane_state *new_plane_state, *old_plane_state;
> > -	struct drm_rect pipe_clip = { .y1 = -1 };
> >  	struct intel_plane *plane;
> >  	bool full_update = false;
> > -	int i, ret;
> > +	int i, src_y1, src_y2, ret;
> >  
> > 
> > 
> > 
> >  	if (!crtc_state->enable_psr2_sel_fetch)
> >  		return 0;
> > @@ -1282,9 +1282,21 @@ int intel_psr2_sel_fetch_update(struct
> > intel_atomic_state *state,
> >  	if (ret)
> >  		return ret;
> >  
> > 
> > 
> > 
> > +	src_y1 = new_plane_state->uapi.src.y1 >> 16;
> > +	src_y2 = new_plane_state->uapi.src.y2 >> 16;
> > +
> > +	/*
> > +	 * Calculate minimal selective fetch area of each plane and
> > calculate
> > +	 * the pipe damaged area.
> > +	 * In the next loop the plane selective fetch area will
> > actually be set
> > +	 * using whole pipe damaged area.
> > +	 */
> >  	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;
> > +		struct drm_rect sel_fetch_area = { .y1 = -1 };
> > +		u32 num_clips;
> > +		int j;
> >  
> > 
> > 
> > 
> >  		if (new_plane_state->uapi.crtc != crtc_state-
> > > uapi.crtc)
> >  			continue;
> > @@ -1300,23 +1312,76 @@ int intel_psr2_sel_fetch_update(struct
> > intel_atomic_state *state,
> >  			break;
> >  		}
> >  
> > 
> > 
> > 
> > -		if (!new_plane_state->uapi.visible)
> > -			continue;
> > +		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 visibility or alpha changed or 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 (new_plane_state->uapi.visible != old_plane_state-
> > > uapi.visible ||
> > +		    new_plane_state->uapi.alpha != old_plane_state-
> > > uapi.alpha ||
> > +		    !drm_rect_equals(&new_plane_state->uapi.dst,
> > +				     &old_plane_state->uapi.dst)) {
> > +			num_clips = 0;
> > +			sel_fetch_area.y1 = src_y1;
> > +			sel_fetch_area.y2 = src_y2;
> > +		} 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 = src_y1;
> > +			sel_fetch_area.y2 = src_y2;
> > +		}
> > +
> > +		for (j = 0; j < num_clips; j++) {
> > +			struct drm_rect damage_area;
> > +
> > +			damage_area.y1 = damaged_clips[j].y1 + src_y1;
> > +			damage_area.y2 = damaged_clips[j].y2 + src_y1;
> > +			clip_area_update(&sel_fetch_area,
> > &damage_area);
> > +		}
> > +
> > +		/* No damaged area in this plane */
> > +		if (sel_fetch_area.y1 == -1) {
> > +			sel_fetch_area.y1 = 0;
> > +			sel_fetch_area.y2 = 0;
> > +		}
> > 
> 1. Visibility change case
>  The pipe damaged area (The Selective Update region) needs to
> accumulate both old and new plane's dst
> 
> 2. Move case
>  The pipe damaged area (The Selective Update region) needs to
> accumulate both old and new plane's dst
> 
> 3. damage case
>  if the damage clips don't intersect src we don't need to accumalate it
> to merged damage area.

Makes sense, doing the changes.

> 
> we can calculate the SelectiveUpdate area with plane's dst and only the
> damage area need to be translated to dst coordinates.
> 
> > -		temp = *sel_fetch_area;
> > -		temp.y1 += new_plane_state->uapi.dst.y1;
> > -		temp.y2 += new_plane_state->uapi.dst.y2;
> for translating to  dst coordinates from src coordinates, 
> we should do like this 
> dst_damaged_area.y1 = src_damaged_area.y1 - plane's src.y1 + plane's
> dst.y1;
> > -		clip_area_update(&pipe_clip, &temp);
> > +		sel_fetch_area.y1 += new_plane_state->uapi.dst.y1;
> > +		sel_fetch_area.y2 += new_plane_state->uapi.dst.y1;
> > +		clip_area_update(&pipe_clip, &sel_fetch_area);
> > +	}
> > +
> > +	if (full_update)
> > +		goto skip_sel_fetch_set_loop;
> > +
> > +	/*
> > +	 * Now that we have the pipe damaged area check if it intersect
> > with
> > +	 * every plane, if it does set the plane selective fetch area.
> > +	 */
> > +	for_each_oldnew_intel_plane_in_state(state, plane,
> > old_plane_state,
> > +					     new_plane_state, i) {
> > +		struct drm_rect *sel_fetch_area, inter;
> > +
> > +		if (new_plane_state->uapi.crtc != crtc_state->uapi.crtc 
> > > > 
> > +		    !new_plane_state->uapi.visible)
> > +			continue;
> > +
> > +		inter = pipe_clip;
> > +		if (!drm_rect_intersect(&inter, &new_plane_state-
> > > uapi.dst))
> > +			continue;
> > +
> > +		sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> > +		sel_fetch_area->y1 = inter.y1 - new_plane_state-
> > > uapi.dst.y1 + src_y1;
> > +		sel_fetch_area->y2 = inter.y2 - new_plane_state-
> > > uapi.dst.y1 + src_y1;
> >  	}
> >  
> > 
> > 
> > 
> > +skip_sel_fetch_set_loop:
> >  	psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update);
> >  	return 0;
> >  }