All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Mun, Gwan-gyeong" <gwan-gyeong.mun@intel.com>
To: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Souza, Jose" <jose.souza@intel.com>
Subject: Re: [Intel-gfx] [PATCH v9 2/5] drm/i915/display/psr: Use plane damage clips to calculate damaged area
Date: Wed, 23 Dec 2020 11:53:45 +0000	[thread overview]
Message-ID: <7b2a48af1b56af9622b8b6be580ccce148c48e44.camel@intel.com> (raw)
In-Reply-To: <20201218184701.111857-2-jose.souza@intel.com>

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@intel.com>

- 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@linux.intel.com>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2020-12-23 11:53 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-18 18:46 [PATCH v9 1/5] drm: Add function to convert rect in 16.16 fixed format to regular format José Roberto de Souza
2020-12-18 18:46 ` [Intel-gfx] " José Roberto de Souza
2020-12-18 18:46 ` [Intel-gfx] [PATCH v9 2/5] drm/i915/display/psr: Use plane damage clips to calculate damaged area José Roberto de Souza
2020-12-23 11:53   ` Mun, Gwan-gyeong [this message]
2020-12-18 18:46 ` [Intel-gfx] [PATCH v9 3/5] drm/i915/display: Split and export main surface calculation from skl_check_main_surface() José Roberto de Souza
2020-12-18 18:47 ` [Intel-gfx] [PATCH v9 4/5] drm/i915/display/psr: Program plane's calculated offset to plane SF register José Roberto de Souza
2020-12-18 18:47 ` [Intel-gfx] [PATCH v9 5/5] HAX/DO_NOT_MERGE_IT: drm/i915/display: Enable PSR2 selective fetch for testing José Roberto de Souza
2020-12-18 20:56 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v9,1/5] drm: Add function to convert rect in 16.16 fixed format to regular format Patchwork
2020-12-18 21:24 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-12-21 10:45 ` [PATCH v9 1/5] " Mun, Gwan-gyeong
2020-12-21 10:45   ` [Intel-gfx] " Mun, Gwan-gyeong
2020-12-23 14:48 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [v9,1/5] drm: Add function to convert rect in 16.16 fixed format to regular format (rev2) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7b2a48af1b56af9622b8b6be580ccce148c48e44.camel@intel.com \
    --to=gwan-gyeong.mun@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jose.souza@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.