On Wed, 2020-12-16 at 06:40 -0800, Souza, Jose wrote:
> On Wed, 2020-12-16 at 14:10 +0000, Mun, Gwan-gyeong wrote:
> > On Wed, 2020-12-16 at 05:17 -0800, Souza, Jose wrote:
> > > On Wed, 2020-12-16 at 10:29 +0000, Mun, Gwan-gyeong wrote:
> > > > On Tue, 2020-12-15 at 05:14 -0800, Souza, Jose wrote:
> > > > > On Tue, 2020-12-15 at 13:02 +0000, Mun, Gwan-gyeong wrote:
> > > > > > On Mon, 2020-12-14 at 09:49 -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.
> > > > > > >
> > > > > > Hi,
> > > > > > > v2:
> > > > > > > - do not shifthing new_plane_state->uapi.dst only src is
> > > > > > > in
> > > > > > > 16.16
> > > > > > Typo on commit message. shifthing -> shifting
> > > > > > > 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
> > > > > > >
> > > > > > > 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 | 98
> > > > > > > +++++++++++++++++++++-
> > > > > > > --
> > > > > > > 1 file changed, 86 insertions(+), 12 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > > > index d9a395c486d3..7daed098fa74 100644
> > > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > > > @@ -1269,8 +1269,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,9 +1282,17 @@ 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 = { .x1 = 0,
> > > > > > > .y1 =
> > > > > > > -1, .x2 = INT_MAX, .y2 = -1 };
> > > > > > > + struct drm_mode_rect *damaged_clips;
> > > > > > > + u32 num_clips, j;
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > if (new_plane_state->uapi.crtc != crtc_state-
> > > > > > > > uapi.crtc)
> > > > > > > continue;
> > > > > > > @@ -1300,23 +1308,89 @@ int
> > > > > > > intel_psr2_sel_fetch_update(struct
> > > > > > > intel_atomic_state *state,
> > > > > > > break;
> > > > > > > }
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > - if (!new_plane_state->uapi.visible)
> > > > > > > - continue;
> > > > > > > + drm_rect_convert_16_16_to_regular(&new_plane_st
> > > > > > > ate-
> > > > > > > > uapi.src, &src);
> > > > > > > + 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 plane moved, mark the whole
> > > > > > > plane
> > > > > > > area as
> > > > > > > + * damaged as it needs to be complete redraw in
> > > > > > > the new
> > > > > > > and old
> > > > > > > + * position.
> > > > > > > */
> > > > > > > + if (new_plane_state->uapi.visible !=
> > > > > > > old_plane_state-
> > > > > > > > uapi.visible ||
> > > > > > > + !drm_rect_equals(&new_plane_state-
> > > > > > > > uapi.dst,
> > > > > > > + &old_plane_state-
> > > > > > > > uapi.dst)) {
> > > > > > > + damaged_area.y1 = old_plane_state-
> > > > > > > > uapi.src.y1
> > > > > > > > > 16;
> > > > > > > + damaged_area.y1 = old_plane_state-
> > > > > > > > uapi.src.y2
> > > > > > > > > 16;
> > > > > > > + damaged_area.y1 += old_plane_state-
> > > > > > > > uapi.dst.y1;
> > > > > > > + damaged_area.y2 += old_plane_state-
> > > > > > > > uapi.dst.y1;
> > > > > > > + clip_area_update(&pipe_clip,
> > > > > > > &damaged_area);
> > > > > > > +
> > > > > > > + num_clips = 0;
> > > > > > > + damaged_area.y1 = src.y1;
> > > > > > > + damaged_area.y2 = src.y2;
> > > > > > 1. Visibility change case
> > > > > > The pipe damaged area (The Selective Update region) needs
> > > > > > to
> > > > > > accumulate being visible plane's dst between old and new
> > > > > > plane's
> > > > > > dst.
> > > > > >
> > > > > > if you are implementing this scenario, the code shoule be
> > > > > > like
> > > > > > this,
> > > > > >
> > > > > > if (new_plane_state->uapi.visible != old_plane_state-
> > > > > > > uapi.visible)
> > > > > > {
> > > > > > if (new_plane_state->uapi.visible) {
> > > > > > damaged_area.y1 = new_plane_state->uapi.dst.y1;
> > > > > > damaged_area.y2 = new_plane_state->uapi.dst.y2;
> > > > > > } else {
> > > > > > 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);
> > > > > > continue;
> > > > > > }
> > > > >
> > > > > The current code does exactly above but discards the clipped
> > > > > in
> > > > > uapi.src.
> > > > >
> > > > in order to accumlate the pipe damage area, we don't need to
> > > > use
> > > > and
> > > > calculate src coordinates on move and visibility change
> > > > scenarios.
> > > > we can easily accumalates than additional calculation.
> > >
> > > Using uapi.dst will cause additional and unnecessary area be
> > > included
> > > to the pipe damaged area, imagine that a plane have src.x1 = 50,
> > > src.y1 = 100,
> > > ... the plane invisible/cropped area don't need to be added to
> > > the
> > > pipe damaged area.
> > >
> > easist way to get damaged area for crtc is just using uapi.dst
> > but because fb damage clip follows uapi.src coordinates, we have to
> > translate fb damage clip's coordinates to dst coordinates for
> > accumulating crtc damage region.
> >
> > therefore can you explain what is the exact burden of using
> > uapi.dst
> > for calculating crtc damage area?
> >
>
> I hope this ASCI drawing do not break :P
>
> > ----------------------|
> > |
> > Pipe area |
> > ----------------| |
> > whole plane A | |
> > |----------| |
> > | src | |
> > | visible | |
> ------------------------
>
> Using dst would add the whole plane A area to the pipe damaged area
> while using src will only add the visible area.
>
if you expect to be shown the src visible area to pipe area,
(that means the dst area has the same width and height, but the x1, y1
will be based on crtc coordinates.)
in the crte point of view, the src visible area is the dst area.
(but because the src area use fb based coordinates, we should use dst)
and if you are expecting handling obscured area by other planes, (for
optimizing)
we should use dst too.
we only need to use src for handling fb damaged clip rects.
but after accumulating the fb damaged clip rects, it also has to be
translated to crtc coordinates from fb (src ) coordinates.
> >
> > > > > > 2. Move case
> > > > > > The pipe damaged area (The Selective Update region) needs
> > > > > > to
> > > > > > accumulate both old and new plane's dst
> > > > >
> > > > > same
> > > > >
> > > > > > if you are implementing this scenario, the code shoule be
> > > > > > like
> > > > > > this,
> > > > > >
> > > > > > else if (!drm_rect_equals(&new_plane_state->uapi.dst,
> > > > > > &old_plane_state-
> > > > > > > uapi.dst)) {
> > > > > > 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);
> > > > > >
> > > > > > 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);
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > continue;
> > > > > > }
> > > > > > > + } else if (new_plane_state->uapi.alpha !=
> > > > > > > old_plane_state->uapi.alpha) {
> > > > > > > + num_clips = 0;
> > > > > > > + damaged_area.y1 = src.y1;
> > > > > > > + damaged_area.y2 = src.y2;
> > > > > > 3. alpha has changed ( because alpha handling section is
> > > > > > not
> > > > > > optimized,
> > > > > > it can be treated with the move section)
> > > > Yes, you are right, there was a misunderstanding of the alpha
> > > > scenario
> > > > of me.
> > > > > no need to handle like this for alpha, if the plane moved if
> > > > > will
> > > > > be
> > > > > handled in the "if" above with or without alpha change, if it
> > > > > did
> > > > > not
> > > > > moved but
> > > > > alpha change no need to add the old state coordinate to the
> > > > > pipe
> > > > > clip.
> > > > >
> > > > > > else if (new_plane_state->uapi.alpha != old_plane_state-
> > > > > > > uapi.alpha) {
> > > > > > 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);
> > > > > >
> > > > > > 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);
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > continue;
> > > > > > }
> > > > > > > + } 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.
> > > > > > > + */
> > > > > > > + damaged_area.y1 = src.y1;
> > > > > > > + damaged_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.
> > > > > > */
> > > > > > 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;
> > > > > > }
> > > > > > > + for (j = 0; j < num_clips; j++) {
> > > > > > > + struct drm_rect clip;
> > > > > > > +
> > > > > > > + clip.y1 = damaged_clips[j].y1;
> > > > > > > + clip.y2 = damaged_clips[j].y2;
> > > > > > > + clip_area_update(&damaged_area, &clip);
> > > > > > > + }
> > > > > > > +
> > > > > > > + if (!drm_rect_intersect(&damaged_area, &src))
> > > > > > > + continue;
> > > > > > > +
> > > > > > > + damaged_area.y1 += new_plane_state-
> > > > > > > > uapi.dst.y1;
> > > > > > > + damaged_area.y2 += new_plane_state-
> > > > > > > > uapi.dst.y1;
> > > > > > > + clip_area_update(&pipe_clip, &damaged_area);
> > > > > > > + }
> > > > > > > +
> > > > > > else if (num_clips) {
> > > > > > struct drm_rect plane_damaged_area;
> > > > > > plane_damaged_area.x1 = src.x1;
> > > > > > plane_damaged_area.x2 = src.x2;
> > > > > > plane_damaged_area.y1 = 0;
> > > > > > plane_damaged_area.y2 = 0;
> > > > > >
> > > > > > for (j = 0; j < num_clips; j++) {
> > > > > > struct drm_rect clip;
> > > > > >
> > > > > > clip.x1 = damaged_clips[j].x1;
> > > > > > clip.x2 = damaged_clips[j].x2;
> > > > > > clip.y1 = damaged_clips[j].y1;
> > > > > > clip.y2 = damaged_clips[j].y2;
> > > > > >
> > > > > > if (drm_rect_intersect(&clip, &src)) {
> > > > > > clip_area_update(&plane_damaged_area, &clip);
> > > > > > }
> > > > >
> > > > > Call intersect at every clip? that don't look optimized.
> > > > >
> > > > when the clip_area_update() is called it just accumulates the
> > > > input
> > > > rects to one output rect.
> > > > it might lead to accumulating unneeded excessive update rect
> > > > region
> > >
> > > The excessive rect region will be removed once after sum all
> > > clips in
> > > the current approach.
> > >
> > on the link's 2nd page,
> > we only need a red Selective Fetch area, but your patch seems
> > Selective
> > Fetch area as green rect.
> > https://docs.google.com/presentation/d/1gOKE4JnC97RzRg15ZM8IaQzHnifIxwvwP_UbOoGFE9E/edit?usp=sharing
>
> Okay, now I got it.
> Will update this but please take a look to the other answers so we do
> only one more version.
>
>
> > > > > > }
> > > > > >
> > > > > > if (drm_rect_visible(plane_damaged_area)) {
> > > > > > plane_damaged_area.y1 = plane_damaged_area.y1 - src.y1
> > > > > > +
> > > > > > new_plane_state->uapi.dst.y1;
> > > > > > plane_damaged_area.y2 = plane_damaged_area.y2 - src.y1
> > > > > > +
> > > > > > new_plane_state->uapi.dst.y1;
> > > > > >
> > > > > > clip_area_update(&pipe_clip, &plane_damaged_area);
> > > > > > continue;
> > > > > > }
> > > > > > }
> > > > >
> > > > > Current code sum all the damage clips, if the sum of the
> > > > > damage
> > > > > clips
> > > > > insect with the src area it is translated to pipe coordinates
> > > > > and
> > > > > called
> > > > > clip_area_update() to update pipe_clip.
> > > > >
> > > > > > the calculation and translating cooridinates is alreay
> > > > > > implemented
> > > > > > here
> > > > > > (
> > > > > > https://patchwork.freedesktop.org/patch/404264/?series=84340&rev=1
> > > > > > it has commetns which explains scenarios.)
> > > > > > > + 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, src;
> > > > > > > +
> > > > > > > + 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;
> > > > > > > +
> > > > > > > + drm_rect_convert_16_16_to_regular(&new_plane_st
> > > > > > > ate-
> > > > > > > > uapi.src, &src);
> > > > > > > +
> > > > > > > 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;
> > > > > > > + sel_fetch_area->y1 = inter.y1 -
> > > > > > > new_plane_state-
> > > > > > > > uapi.dst.y1;
> > > > > > > + sel_fetch_area->y2 = inter.y2 -
> > > > > > > new_plane_state-
> > > > > > > > uapi.dst.y1;
> > > > > > 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;
> > > > >
> > > > > sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
> > > > > +
> > > > > drm_rect_intersect(sel_fetch_area, &src);
> > > > > does a the same job and is easier to understand(translation +
> > > > > clip)
> > > > >
> > > > when the src's x1 and y1 indicates over than 0, does it work?
> > >
> > > It will, we use it for scenarios like that in this function.
> > >
> > >
> > here is the problematic case,
> > if we assume src indicates 50 x 50 +10 +10
> > ,fb damage rect indicates 5 x 5 +55 +55
> > and dst indicates 50 x 50 +100 +100
> >
> > 1) if whole plane src area needs to be updated
> > inter: 50 x 50 +100 +100
> > sel_fetch_area->y1 = 100 - 100 => 0
> > sel_fetch_area->y2 = 100 - 100 => 0
> >
> >
> > drm_rect_intersect(sel_fetch_area, &src);
> > => sel_fetch_area does not fit whole src area.
>
> Your values are wrong.
>
I used DRM_RECT_FMT style.
#define DRM_RECT_FMT "%dx%d%+d%+d"
#define DRM_RECT_ARG(r) drm_rect_width(r), drm_rect_height(r), (r)->x1,
(r)->y1
> # 50 x 50 +10 +10
>
> src.x1 = 50
> src.y1 = 50
> src.x2 = 60
> src.y2 = 60
>
src.x1 = 10
src.y1 = 10
src.x2 = 60
src.y2 = 60
> # 5 x 5 +55 +55. * not used in example 1
> damaged.x1 = 5
> damaged.y1 = 5
> damaged.x2 = 60
> damaged.y2 = 60
>
damaged.x1 = 55
damaged.y1 = 55
damaged.x2 = 60
damaged.y2 = 60
> # 50 x 50 +100 +100
> dst.x1 = 50
> dst.y1 = 50
> dst.x2 = 150
> dst.y2 = 150
>
dst.x1 = 100
dst.y1 = 100
dst.x2 = 150
dst.y2 = 150
therefore below result seems to wrong result.
> ####
>
> 1) if whole plane src are needs to be updated
> pipe_clip.x1 = src.x1 + dst.x1 = 100
> pipe_clip.y2 = src.y1 + dst.y1 = 100
> pipe_clip.x2 = src.x2 + dst.y1 = 110
> pipe_clip.y2 = src.y2 + dst.y2 = 110
>
> # drm_rect_intersect(&inter, &new_plane_state->uapi.dst)
> iter.x1 = 100
> iter.y1 = 100
> iter.x2 = 110
> iter.y2 = 110
>
> # sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
> sel_fetch_area.y1 = 100 - 50 = 50
> sel_fetch_area.y2 = 110 - 50 = 60
>
> drm_rect_intersect(sel_fetch_area, &src);
>
> sel_fetch_area.y1 = 50
> sel_fetch_area.y2 = 60
>
> > 2) if only the plane fb damage needs to be updated
> > translate fb damage's coordinates to crtc coordinates
> > 5 x 5 +55 +55 => 5 x 5 +105 +105
> > inter: 5 x 5 +105 +105
> > sel_fetch_area->y1 = 105 - 100 => 5
> > sel_fetch_area->y2 = 105 - 100 => 5
> >
> > drm_rect_intersect(sel_fetch_area, &src);
> > => sel_fetch_area does not fit to src's damage area.
>
> Let me know if this case is broken after use the right values.
>
> > > > > > > + sel_fetch_area->x1 = src.x1;
> > > > > > > + sel_fetch_area->x2 = src.x2;
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > - 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);
> > > > > > > + drm_rect_intersect(sel_fetch_area, &src);
> > > > > > why this line is needed?
> > > > >
> > > > > explained above.
> > > > >
> > > > >
> > > > > > > }
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > +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
