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?
> + 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)
> + * 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.
> + 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
