[Intel-gfx] [PATCH 3/6] drm/i915/display/psr: Consider other planes to damaged area calculation

[Mun, Gwan-gyeong]

On Tue, 2020-10-13 at 16:01 -0700, José Roberto de Souza wrote:
> Planes can individually have transparent, move or have visibility
> changed if any of those happens, planes bellow it will be visible or
> have more pixels of it visible than before.
> 
> This patch is taking care of this case for selective fetch by adding
> to each plane damaged area all the intersections of planes above it
> that matches with the characteristics described above.
> 
> There still some room from improvements here but at least this
> initial
> version will take care of display what is expected saving some memory
> reads.
> 
> 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 | 62
> ++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 0f1e9f0fa57f..91ba97bf609b 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1253,11 +1253,38 @@ static void clip_area_update(struct drm_rect
> *overlap_damage_area,
>  		overlap_damage_area->y2 = damage_area->y2;
>  }
>  
> +/* Update plane damage area if planes above moved or have alpha */
> +static void pipe_dirty_areas_set(struct intel_plane_state
> *plane_state,
> +				 struct intel_plane *plane,
> +				 const struct drm_rect
> *pipe_dirty_areas,
> +				 struct drm_rect *sel_fetch_area)
> +{
> +	enum plane_id i;
> +
> +	for (i = PLANE_CURSOR; i > plane->id; i--) {
> +		int j;
> +
> +		for (j = 0; j < 2; j++) {
> +			struct drm_rect r = pipe_dirty_areas[i * 2 +
> j];
> +
> +			if (!drm_rect_width(&r))
> +				continue;
> +			if (!drm_rect_intersect(&r, &plane_state-
> >uapi.dst))
> +				continue;
> +
> +			r.y1 -= plane_state->uapi.dst.y1;
> +			r.y2 -= plane_state->uapi.dst.y1;
typo of y2?
> +			clip_area_update(sel_fetch_area, &r);
sel_fetch_area has plane coordinates, but it tried to apply dst
coordinates.
> +		}
> +	}
> +}
> +
>  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 intel_plane_state *new_plane_state, *old_plane_state;
> +	struct drm_rect pipe_dirty_areas[I915_MAX_PLANES * 2] = {};
>  	struct drm_rect pipe_clip = { .y1 = -1 };
>  	struct intel_plane *plane;
>  	bool full_update = false;
> @@ -1270,6 +1297,38 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * Mark all the areas where there is a plane that matches one
> of this:
> +	 * - transparent
> +	 * - moved
> +	 * - visibility changed
> +	 * In all those cases, planes bellow it will need to be redraw.
> +	 */
> +	for_each_oldnew_intel_plane_in_state(state, plane,
> old_plane_state,
> +					     new_plane_state, i) {
> +		bool alpha, flip, dirty;
> +
> +		if (new_plane_state->uapi.crtc != crtc_state-
> >uapi.crtc)
> +			continue;
> +
> +		alpha = new_plane_state->uapi.alpha !=
> DRM_BLEND_ALPHA_OPAQUE;
> +		alpha |= old_plane_state->uapi.alpha !=
> DRM_BLEND_ALPHA_OPAQUE;
> +		flip = new_plane_state->uapi.fb != old_plane_state-
> >uapi.fb;
> +		dirty = alpha && flip;
> +
> +		dirty |= !drm_rect_equals(&new_plane_state->uapi.dst,
> +					  &old_plane_state->uapi.dst);
> +		dirty |= new_plane_state->uapi.visible !=
> +			 old_plane_state->uapi.visible;
> +		if (!dirty)
> +			continue;
> +
> +		if (old_plane_state->uapi.visible)
> +			pipe_dirty_areas[plane->id * 2] =
> old_plane_state->uapi.dst;
> +		if (new_plane_state->uapi.visible)
> +			pipe_dirty_areas[plane->id * 2 + 1] =
> new_plane_state->uapi.dst;
> +	}
> +
>  	for_each_oldnew_intel_plane_in_state(state, plane,
> old_plane_state,
>  					     new_plane_state, i) {
>  		struct drm_rect *sel_fetch_area, temp;
> @@ -1337,6 +1396,9 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
>  			sel_fetch_area->y2 = 0;
>  		}
>  
> +		pipe_dirty_areas_set(new_plane_state, plane,
> pipe_dirty_areas,
> +				     sel_fetch_area);
> +
In my humble opinion, plane_state's alpha and visibility might affect
to PSR Selective Update, but it would not affect to damage region of
the Plane Selective Fetch program. therefore if we want to calculate
with changing of alpha and visibility, we need to separate the data
structure of clip rects and calculating clip region to "plane's
selective fetch" and "PSR's selective update".
>  		/* Don't need to redraw plane damaged areas outside of
> screen */
>  		j = sel_fetch_area->y2 + (new_plane_state->uapi.dst.y1
> >> 16);
>  		j = crtc_state->uapi.adjusted_mode.crtc_vdisplay - j;