On Mon, 2020-10-26 at 21:40 +0000, Mun, Gwan-gyeong wrote:
> On Tue, 2020-10-13 at 16:01 -0700, 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.
> >
> > 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 | 54 +++++++++++++++++++++-
> > --
> > 1 file changed, 49 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 773a5b5fa078..0f1e9f0fa57f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1273,6 +1273,9 @@ int intel_psr2_sel_fetch_update(struct
> > intel_atomic_state *state,
> > 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;
> > + u32 num_clips;
> > + int j;
> >
> >
> >
> >
> > if (new_plane_state->uapi.crtc != crtc_state-
> > > uapi.crtc)
> > continue;
> > @@ -1291,13 +1294,54 @@ int intel_psr2_sel_fetch_update(struct
> > intel_atomic_state *state,
> > if (!new_plane_state->uapi.visible)
> > continue;
> >
> >
> >
> >
> > + sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> > + sel_fetch_area->y1 = -1;
> > +
> > + 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 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 (!drm_rect_equals(&new_plane_state->uapi.dst,
> > + &old_plane_state->uapi.dst)) {
> > + num_clips = 0;
> > + sel_fetch_area->y1 = new_plane_state-
> > > uapi.src.y1 >> 16;
> > + sel_fetch_area->y2 = new_plane_state-
> > > uapi.src.y2 >> 16;
> > + } 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 = new_plane_state-
> > > uapi.src.y1 >> 16;
> > + sel_fetch_area->y2 = new_plane_state-
> > > uapi.src.y2 >> 16;
> > + }
> > +
> why don't you use drm_atomic_helper_damage_merged() function here?
hum did not knew about this function, will take a look at as it does more than just merge all damaged areas.
> > + for (j = 0; j < num_clips; j++) {
> > + struct drm_rect damage_area;
> > +
> > + damage_area.y1 = damaged_clips[j].y1;
> > + damage_area.y2 = damaged_clips[j].y2;
> > + clip_area_update(sel_fetch_area, &damage_area);
> > + }
> > +
> > + /*
> > + * No page flip, no plane moviment or no damage areas,
> > so don't
> typo (moviment -> movement)
fixed
> > + * fetch any pixel from memory for this plane
> > + */
> > + if (sel_fetch_area->y1 == -1) {
> > + sel_fetch_area->y1 = 0;
> > + sel_fetch_area->y2 = 0;
> > + }
> > +
> > + /* Don't need to redraw plane damaged areas outside of
> > screen */
> > + j = sel_fetch_area->y2 + (new_plane_state->uapi.dst.y1
> > > > 16);
> src coordinates of the drm_plane_state are 16.16 fixed point but dst
> coordinates are not 16.16 fixed point.
> therefore we don't need to bit shift for dst.
> Because the sel_fetch_area seems based on src coordinates, in order to
> apply to dst coordinates here, it requires coordinate calculation.
thanks for catching this, also fixed the same issue patch 1.
> > + j = crtc_state->uapi.adjusted_mode.crtc_vdisplay - j;
> > + if (j < 0)
> > + sel_fetch_area->y2 += j;
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > temp = *sel_fetch_area;
> > temp.y1 += new_plane_state->uapi.dst.y1 >> 16;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
