On Tue, 2020-10-27 at 13:12 -0700, Souza, Jose wrote:
> On Tue, 2020-10-27 at 01:04 +0000, Souza, Jose wrote:
> > 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;
selective fetch area also can be affected by Selective Updated region.
therefore Selective Updated region rect should be calculated first and
then the plane's selective fetch area should be applied (intersected by
calculated SU.)
please check this implementation.
(https://patchwork.freedesktop.org/patch/404264/?series=84340&rev=1)
> > > > @@ -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.
>
> We can't use this function as it marks the whole plane area as
> damaged if there is no damaged clips.
> But for our use case this is bad as we add all planes of the
> pipe/CRTC to the state, so it would cause a full fetch of the planes
> without any
> flip/modification.
>
> > > > + 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
