On Tue, 2020-10-27 at 10:25 -0700, Souza, Jose wrote:
> On Tue, 2020-10-27 at 13:34 +0000, Mun, Gwan-gyeong wrote:
> > On Tue, 2020-10-13 at 16:01 -0700, José Roberto de Souza wrote:
> > > Planes can individually have transparent, move or have visibility
> > > changed if any of those happens, planes bellow it will be visible
> > > or
> > > have more pixels of it visible than before.
> > >
> > > This patch is taking care of this case for selective fetch by
> > > adding
> > > to each plane damaged area all the intersections of planes above
> > > it
> > > that matches with the characteristics described above.
> > >
> > > There still some room from improvements here but at least this
> > > initial
> > > version will take care of display what is expected saving some
> > > memory
> > > reads.
> > >
> > > 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 | 62
> > > ++++++++++++++++++++++++
> > > 1 file changed, 62 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 0f1e9f0fa57f..91ba97bf609b 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -1253,11 +1253,38 @@ static void clip_area_update(struct
> > > drm_rect
> > > *overlap_damage_area,
> > > overlap_damage_area->y2 = damage_area->y2;
> > > }
> > >
> > >
> > >
> > >
> > > +/* Update plane damage area if planes above moved or have alpha
> > > */
> > > +static void pipe_dirty_areas_set(struct intel_plane_state
> > > *plane_state,
> > > + struct intel_plane *plane,
> > > + const struct drm_rect
> > > *pipe_dirty_areas,
> > > + struct drm_rect *sel_fetch_area)
> > > +{
> > > + enum plane_id i;
> > > +
> > > + for (i = PLANE_CURSOR; i > plane->id; i--) {
> > > + int j;
> > > +
> > > + for (j = 0; j < 2; j++) {
> > > + struct drm_rect r = pipe_dirty_areas[i * 2 +
> > > j];
> > > +
> > > + if (!drm_rect_width(&r))
> > > + continue;
> > > + if (!drm_rect_intersect(&r, &plane_state-
> > > > uapi.dst))
> > > + continue;
> > > +
> > > + r.y1 -= plane_state->uapi.dst.y1;
> > > + r.y2 -= plane_state->uapi.dst.y1;
> > typo of y2?
>
> Not a typo in this case.
>
> > > + clip_area_update(sel_fetch_area, &r);
> > sel_fetch_area has plane coordinates, but it tried to apply dst
> > coordinates.
>
> the subtraction above is converting pipe/dst coordinates to the
> current plane coordinates.
>
> > > + }
> > > + }
> > > +}
> > > +
> > > 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 intel_plane_state *new_plane_state, *old_plane_state;
> > > + struct drm_rect pipe_dirty_areas[I915_MAX_PLANES * 2] = {};
> > > struct drm_rect pipe_clip = { .y1 = -1 };
> > > struct intel_plane *plane;
> > > bool full_update = false;
> > > @@ -1270,6 +1297,38 @@ int intel_psr2_sel_fetch_update(struct
> > > intel_atomic_state *state,
> > > if (ret)
> > > return ret;
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > + /*
> > > + * Mark all the areas where there is a plane that matches one
> > > of this:
> > > + * - transparent
> > > + * - moved
> > > + * - visibility changed
> > > + * In all those cases, planes bellow it will need to be redraw.
> > > + */
(for future visibility and fully obscured plane checking) we
can traverse from top to down, by adding and using
for_each_oldnew_intel_plane_in_state_reverse()
> > > + for_each_oldnew_intel_plane_in_state(state, plane,
> > > old_plane_state,
> > > + new_plane_state, i) {
> > > + bool alpha, flip, dirty;
> > > +
> > > + if (new_plane_state->uapi.crtc != crtc_state-
> > > > uapi.crtc)
> > > + continue;
> > > +
> > > + alpha = new_plane_state->uapi.alpha !=
> > > DRM_BLEND_ALPHA_OPAQUE;
> > > + alpha |= old_plane_state->uapi.alpha !=
> > > DRM_BLEND_ALPHA_OPAQUE;
> > > + flip = new_plane_state->uapi.fb != old_plane_state-
> > > > uapi.fb;
> > > + dirty = alpha && flip;
> > > +
> > > + dirty |= !drm_rect_equals(&new_plane_state->uapi.dst,
> > > + &old_plane_state->uapi.dst);
> > > + dirty |= new_plane_state->uapi.visible !=
> > > + old_plane_state->uapi.visible;
> > > + if (!dirty)
> > > + continue;
> > > +
if we can calculate SU region here, we don't need to store all of the
dirty areas.
> > > + if (old_plane_state->uapi.visible)
> > > + pipe_dirty_areas[plane->id * 2] =
> > > old_plane_state->uapi.dst;
> > > + if (new_plane_state->uapi.visible)
> > > + pipe_dirty_areas[plane->id * 2 + 1] =
> > > new_plane_state->uapi.dst;
> > > + }
> > > +
> > > for_each_oldnew_intel_plane_in_state(state, plane,
> > > old_plane_state,
> > > new_plane_state, i) {
> > > struct drm_rect *sel_fetch_area, temp;
> > > @@ -1337,6 +1396,9 @@ int intel_psr2_sel_fetch_update(struct
> > > intel_atomic_state *state,
> > > sel_fetch_area->y2 = 0;
> > > }
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > + pipe_dirty_areas_set(new_plane_state, plane,
> > > pipe_dirty_areas,
> > > + sel_fetch_area);
> > > +
> > In my humble opinion, plane_state's alpha and visibility might
> > affect
> > to PSR Selective Update, but it would not affect to damage region
> > of
> > the Plane Selective Fetch program. therefore if we want to
> > calculate
> > with changing of alpha and visibility, we need to separate the data
> > structure of clip rects and calculating clip region to "plane's
> > selective fetch" and "PSR's selective update".
>
> Why separate? We need one clip region to program selective fetch
> plane registers.
> Like said in the commit description, this could be optimized in
> future like to check if pixels changed in the damaged overlap area
> with a plane with
> alpha but that will make things really complicated so for now keeping
> this more simple approach, compositors are not even supporting damage
> area yet.
weston (wayland reference compositor) supports damage area feature.
>
> > > /* Don't need to redraw plane damaged areas outside of
> > > screen */
> > > j = sel_fetch_area->y2 + (new_plane_state->uapi.dst.y1
> > > > > 16);
> > > j = crtc_state->uapi.adjusted_mode.crtc_vdisplay - j;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
