Re: [PATCH v9 2/5] drm/i915/display/psr: Use plane damage clips to calculate damaged area

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I've tested this patch with SelectiveFetch / PSR Selective Update IGT
test.

igt patch : Add FB_DAMAGE_CLIPS prop and new test for Selective fetch
: https://patchwork.freedesktop.org/series/84696/  (this igt patch is
under reviewing)

As per bspec, when I checked this patch by code level, it looked having
no issues.
But it showed that some tests failed.
Basically, this feature is not enabled by default and other under-
development features (like DSC for PSR and DP pannel replay) are
dependant on this feature.
therefore I suggest land it first and send another patch for fixing
failing SF igt test.

Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>

- Passed cases
IGT-Version: 1.25-g9e36faf6 (x86_64) (Linux: 5.10.0-DRM-TIP+ x86_64)
Starting subtest: primary-plane-update-sf-dmg-area-1
Subtest primary-plane-update-sf-dmg-area-1: SUCCESS (0.477s)
Starting subtest: primary-plane-update-sf-dmg-area-2
Subtest primary-plane-update-sf-dmg-area-2: SUCCESS (0.499s)
Starting subtest: primary-plane-update-sf-dmg-area-3
Subtest primary-plane-update-sf-dmg-area-3: SUCCESS (0.497s)
Starting subtest: primary-plane-update-sf-dmg-area-4
Subtest primary-plane-update-sf-dmg-area-4: SUCCESS (0.483s)
Starting subtest: primary-plane-update-sf-dmg-area-5
Subtest primary-plane-update-sf-dmg-area-5: SUCCESS (0.485s)
Starting subtest: overlay-plane-update-sf-dmg-area-1
Subtest overlay-plane-update-sf-dmg-area-1: SUCCESS (0.488s)
Starting subtest: overlay-plane-update-sf-dmg-area-2
Subtest overlay-plane-update-sf-dmg-area-2: SUCCESS (0.672s)
Starting subtest: overlay-plane-update-sf-dmg-area-3
Subtest overlay-plane-update-sf-dmg-area-3: SUCCESS (0.530s)
Starting subtest: overlay-plane-update-sf-dmg-area-4
Subtest overlay-plane-update-sf-dmg-area-4: SUCCESS (0.531s)
Starting subtest: overlay-plane-update-sf-dmg-area-5
Subtest overlay-plane-update-sf-dmg-area-5: SUCCESS (0.531s)
Starting subtest: cursor-plane-update-sf
Subtest cursor-plane-update-sf: SUCCESS (0.513s)
Starting subtest: plane-move-sf-dmg-area-0
Subtest plane-move-sf-dmg-area-0: SUCCESS (0.672s)
Starting subtest: plane-move-sf-dmg-area-1
Subtest plane-move-sf-dmg-area-1: SUCCESS (0.672s)
Starting subtest: plane-move-sf-dmg-area-2
Subtest plane-move-sf-dmg-area-2: SUCCESS (0.531s)
Starting subtest: plane-move-sf-dmg-area-3
Subtest plane-move-sf-dmg-area-3: SUCCESS (0.674s)
Starting subtest: overlay-primary-update-sf-dmg-area-1
Subtest overlay-primary-update-sf-dmg-area-1: SUCCESS (0.608s)
Starting subtest: overlay-primary-update-sf-dmg-area-2
Subtest overlay-primary-update-sf-dmg-area-2: SUCCESS (0.605s)
Starting subtest: overlay-primary-update-sf-dmg-area-3
Subtest overlay-primary-update-sf-dmg-area-3: SUCCESS (0.606s)

- Failed test
subtest: overlay-primary-update-sf-dmg-area-4

Br,
G.G.

On Fri, 2020-12-18 at 10:46 -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
> damaged area needs to be fetched from memory so the complete blending
> of all planes can happen.
> 
> v2:
> - do not shifting 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
> 
> v6:
> - consider old state plane coordinates when visibility changes or it
> moved to calculate damaged area
> - remove from damaged area the portion not in src clip
> 
> v7:
> - intersec every damage clip with src to minimize damaged area
> 
> v8:
> - adjust pipe_damaged area to 4 lines grouping
> - adjust calculation now that is understood that uapi.src is the
> framebuffer coordinates that plane will start to fetch from
> 
> v9:
> - Only add plane dst or src to damaged_area if visible
> - Early skip plane damage calculation if it was not visible in old
> and
> new state
> 
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 113 ++++++++++++++++++++-
> --
>  1 file changed, 99 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..f5b9519b3756 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1242,9 +1242,11 @@ static void psr2_man_trk_ctl_calc(struct
> intel_crtc_state *crtc_state,
>  	if (clip->y1 == -1)
>  		goto exit;
>  
> +	drm_WARN_ON(crtc_state->uapi.crtc->dev, clip->y1 % 4 || clip-
> >y2 % 4);
> +
>  	val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
>  	val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1);
> -	val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip-
> >y2, 4) + 1);
> +	val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2 / 4 + 1);
>  exit:
>  	crtc_state->psr2_man_track_ctl = val;
>  }
> @@ -1269,8 +1271,8 @@ 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;
> @@ -1282,13 +1284,25 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * 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_rect src, damaged_area = { .y1 = -1 };
> +		struct drm_mode_rect *damaged_clips;
> +		u32 num_clips, j;
>  
>  		if (new_plane_state->uapi.crtc != crtc_state-
> >uapi.crtc)
>  			continue;
>  
> +		if (!new_plane_state->uapi.visible &&
> +		    !old_plane_state->uapi.visible)
> +			continue;
> +
>  		/*
>  		 * TODO: Not clear how to handle planes with negative
> position,
>  		 * also planes are not updated if they have a negative
> X
> @@ -1300,23 +1314,94 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
>  			break;
>  		}
>  
> -		if (!new_plane_state->uapi.visible)
> -			continue;
> +		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 plane moved, mark the whole plane
> area as
> +		 * damaged as it needs to be complete redraw in the new
> and old
> +		 * 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 ||
> +		    !drm_rect_equals(&new_plane_state->uapi.dst,
> +				     &old_plane_state->uapi.dst)) {
> +			if (old_plane_state->uapi.visible) {
> +				damaged_area.y1 = old_plane_state-
> >uapi.dst.y1;
> +				damaged_area.y2 = old_plane_state-
> >uapi.dst.y2;
> +				clip_area_update(&pipe_clip,
> &damaged_area);
> +			}
> +
> +			if (new_plane_state->uapi.visible) {
> +				damaged_area.y1 = new_plane_state-
> >uapi.dst.y1;
> +				damaged_area.y2 = new_plane_state-
> >uapi.dst.y2;
> +				clip_area_update(&pipe_clip,
> &damaged_area);
> +			}
> +			continue;
> +		} else if (new_plane_state->uapi.alpha !=
> old_plane_state->uapi.alpha ||
> +			   (!num_clips &&
> +			    new_plane_state->uapi.fb !=
> old_plane_state->uapi.fb)) {
> +			/*
> +			 * If the plane don't have damaged areas but
> the
> +			 * framebuffer changed or alpha changed, mark
> the whole
> +			 * plane area as damaged.
> +			 */
> +			damaged_area.y1 = new_plane_state->uapi.dst.y1;
> +			damaged_area.y2 = new_plane_state->uapi.dst.y2;
> +			clip_area_update(&pipe_clip, &damaged_area);
> +			continue;
> +		}
> +
> +		drm_rect_fp_to_int(&src, &new_plane_state->uapi.src);
> +		damaged_clips =
> drm_plane_get_damage_clips(&new_plane_state->uapi);
> +
> +		for (j = 0; j < num_clips; j++) {
> +			struct drm_rect clip;
> +
> +			clip.x1 = damaged_clips[j].x1;
> +			clip.y1 = damaged_clips[j].y1;
> +			clip.x2 = damaged_clips[j].x2;
> +			clip.y2 = damaged_clips[j].y2;
> +			if (drm_rect_intersect(&clip, &src))
> +				clip_area_update(&damaged_area, &clip);
> +		}
>  
> -		temp = *sel_fetch_area;
> -		temp.y1 += new_plane_state->uapi.dst.y1;
> -		temp.y2 += new_plane_state->uapi.dst.y2;
> -		clip_area_update(&pipe_clip, &temp);
> +		if (damaged_area.y1 == -1)
> +			continue;
> +
> +		damaged_area.y1 += new_plane_state->uapi.dst.y1 -
> src.y1;
> +		damaged_area.y2 += new_plane_state->uapi.dst.y1 -
> src.y1;
> +		clip_area_update(&pipe_clip, &damaged_area);
> +	}
> +
> +	if (full_update)
> +		goto skip_sel_fetch_set_loop;
> +
> +	/* It must be aligned to 4 lines */
> +	pipe_clip.y1 -= pipe_clip.y1 % 4;
> +	if (pipe_clip.y2 % 4)
> +		pipe_clip.y2 = ((pipe_clip.y2 / 4) + 1) * 4;
> +
> +	/*
> +	 * 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;
> +		sel_fetch_area->y2 = inter.y2 - new_plane_state-
> >uapi.dst.y1;
>  	}
>  
> +skip_sel_fetch_set_loop:
>  	psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update);
>  	return 0;
>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux