[Intel-gfx] [PATCH 3/6] drm/i915/display/psr: Consider other planes to damaged area calculation
Souza, Jose
jose.souza at intel.com
Tue Dec 1 17:44:22 UTC 2020
On Tue, 2020-12-01 at 17:33 +0000, Mun, Gwan-gyeong wrote:
> 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 at linux.intel.com>
> > > > Cc: Gwan-gyeong Mun <gwan-gyeong.mun at intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > > ---
> > > > 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 the planes bellow don't have damaged areas or if it don't have any overlap with other planes we should not include it do the SU region to save
power.
> > > > + 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.
Will try it, anything need to be enabled or configure to this to work?
Any instructions that you can share will save time for me.
> >
> > > > /* 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;
More information about the Intel-gfx
mailing list