I've tested this patch with SelectiveFetch / PSR Selective Update IGT test. igt patch : Add FB_DAMAGE_CLIPS prop and new test for Selective fetch : https://patchwork.freedesktop.org/series/84696/ (this igt patch is under reviewing) As per bspec, when I checked this patch by code level, it looked having no issues. But it showed that some tests failed. Basically, this feature is not enabled by default and other under- development features (like DSC for PSR and DP pannel replay) are dependant on this feature. therefore I suggest land it first and send another patch for fixing failing SF igt test. Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx> - Passed cases IGT-Version: 1.25-g9e36faf6 (x86_64) (Linux: 5.10.0-DRM-TIP+ x86_64) Starting subtest: primary-plane-update-sf-dmg-area-1 Subtest primary-plane-update-sf-dmg-area-1: SUCCESS (0.477s) Starting subtest: primary-plane-update-sf-dmg-area-2 Subtest primary-plane-update-sf-dmg-area-2: SUCCESS (0.499s) Starting subtest: primary-plane-update-sf-dmg-area-3 Subtest primary-plane-update-sf-dmg-area-3: SUCCESS (0.497s) Starting subtest: primary-plane-update-sf-dmg-area-4 Subtest primary-plane-update-sf-dmg-area-4: SUCCESS (0.483s) Starting subtest: primary-plane-update-sf-dmg-area-5 Subtest primary-plane-update-sf-dmg-area-5: SUCCESS (0.485s) Starting subtest: overlay-plane-update-sf-dmg-area-1 Subtest overlay-plane-update-sf-dmg-area-1: SUCCESS (0.488s) Starting subtest: overlay-plane-update-sf-dmg-area-2 Subtest overlay-plane-update-sf-dmg-area-2: SUCCESS (0.672s) Starting subtest: overlay-plane-update-sf-dmg-area-3 Subtest overlay-plane-update-sf-dmg-area-3: SUCCESS (0.530s) Starting subtest: overlay-plane-update-sf-dmg-area-4 Subtest overlay-plane-update-sf-dmg-area-4: SUCCESS (0.531s) Starting subtest: overlay-plane-update-sf-dmg-area-5 Subtest overlay-plane-update-sf-dmg-area-5: SUCCESS (0.531s) Starting subtest: cursor-plane-update-sf Subtest cursor-plane-update-sf: SUCCESS (0.513s) Starting subtest: plane-move-sf-dmg-area-0 Subtest plane-move-sf-dmg-area-0: SUCCESS (0.672s) Starting subtest: plane-move-sf-dmg-area-1 Subtest plane-move-sf-dmg-area-1: SUCCESS (0.672s) Starting subtest: plane-move-sf-dmg-area-2 Subtest plane-move-sf-dmg-area-2: SUCCESS (0.531s) Starting subtest: plane-move-sf-dmg-area-3 Subtest plane-move-sf-dmg-area-3: SUCCESS (0.674s) Starting subtest: overlay-primary-update-sf-dmg-area-1 Subtest overlay-primary-update-sf-dmg-area-1: SUCCESS (0.608s) Starting subtest: overlay-primary-update-sf-dmg-area-2 Subtest overlay-primary-update-sf-dmg-area-2: SUCCESS (0.605s) Starting subtest: overlay-primary-update-sf-dmg-area-3 Subtest overlay-primary-update-sf-dmg-area-3: SUCCESS (0.606s) - Failed test subtest: overlay-primary-update-sf-dmg-area-4 Br, G.G. On Fri, 2020-12-18 at 10:46 -0800, 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. > > Now that we are not complete fetching each plane, there is another > loop needed as all the plane areas that intersect with the pipe > damaged area needs to be fetched from memory so the complete blending > of all planes can happen. > > v2: > - do not shifting new_plane_state->uapi.dst only src is in 16.16 > format > > v4: > - setting plane selective fetch area using the whole pipe damage area > - mark the whole plane area damaged if plane visibility or alpha > changed > > v5: > - taking in consideration src.y1 in the damage coordinates > - adding to the pipe damaged area planes that were visible but are > invisible in the new state > > v6: > - consider old state plane coordinates when visibility changes or it > moved to calculate damaged area > - remove from damaged area the portion not in src clip > > v7: > - intersec every damage clip with src to minimize damaged area > > v8: > - adjust pipe_damaged area to 4 lines grouping > - adjust calculation now that is understood that uapi.src is the > framebuffer coordinates that plane will start to fetch from > > v9: > - Only add plane dst or src to damaged_area if visible > - Early skip plane damage calculation if it was not visible in old > and > new state > > 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 | 113 ++++++++++++++++++++- > -- > 1 file changed, 99 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > b/drivers/gpu/drm/i915/display/intel_psr.c > index d9a395c486d3..f5b9519b3756 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -1242,9 +1242,11 @@ static void psr2_man_trk_ctl_calc(struct > intel_crtc_state *crtc_state, > if (clip->y1 == -1) > goto exit; > > + drm_WARN_ON(crtc_state->uapi.crtc->dev, clip->y1 % 4 || clip- > >y2 % 4); > + > val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE; > val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1); > - val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip- > >y2, 4) + 1); > + val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip->y2 / 4 + 1); > exit: > crtc_state->psr2_man_track_ctl = val; > } > @@ -1269,8 +1271,8 @@ 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 drm_rect pipe_clip = { .x1 = 0, .y1 = -1, .x2 = INT_MAX, > .y2 = -1 }; > struct intel_plane_state *new_plane_state, *old_plane_state; > - struct drm_rect pipe_clip = { .y1 = -1 }; > struct intel_plane *plane; > bool full_update = false; > int i, ret; > @@ -1282,13 +1284,25 @@ int intel_psr2_sel_fetch_update(struct > intel_atomic_state *state, > if (ret) > return ret; > > + /* > + * Calculate minimal selective fetch area of each plane and > calculate > + * the pipe damaged area. > + * In the next loop the plane selective fetch area will > actually be set > + * using whole pipe damaged area. > + */ > 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_rect src, damaged_area = { .y1 = -1 }; > + struct drm_mode_rect *damaged_clips; > + u32 num_clips, j; > > if (new_plane_state->uapi.crtc != crtc_state- > >uapi.crtc) > continue; > > + if (!new_plane_state->uapi.visible && > + !old_plane_state->uapi.visible) > + continue; > + > /* > * TODO: Not clear how to handle planes with negative > position, > * also planes are not updated if they have a negative > X > @@ -1300,23 +1314,94 @@ int intel_psr2_sel_fetch_update(struct > intel_atomic_state *state, > break; > } > > - if (!new_plane_state->uapi.visible) > - continue; > + 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 visibility or plane moved, mark the whole plane > area as > + * damaged as it needs to be complete redraw in the new > and old > + * 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 (new_plane_state->uapi.visible != old_plane_state- > >uapi.visible || > + !drm_rect_equals(&new_plane_state->uapi.dst, > + &old_plane_state->uapi.dst)) { > + if (old_plane_state->uapi.visible) { > + damaged_area.y1 = old_plane_state- > >uapi.dst.y1; > + damaged_area.y2 = old_plane_state- > >uapi.dst.y2; > + clip_area_update(&pipe_clip, > &damaged_area); > + } > + > + if (new_plane_state->uapi.visible) { > + damaged_area.y1 = new_plane_state- > >uapi.dst.y1; > + damaged_area.y2 = new_plane_state- > >uapi.dst.y2; > + clip_area_update(&pipe_clip, > &damaged_area); > + } > + continue; > + } else if (new_plane_state->uapi.alpha != > old_plane_state->uapi.alpha || > + (!num_clips && > + new_plane_state->uapi.fb != > old_plane_state->uapi.fb)) { > + /* > + * If the plane don't have damaged areas but > the > + * framebuffer changed or alpha changed, mark > the whole > + * plane area as damaged. > + */ > + damaged_area.y1 = new_plane_state->uapi.dst.y1; > + damaged_area.y2 = new_plane_state->uapi.dst.y2; > + clip_area_update(&pipe_clip, &damaged_area); > + continue; > + } > + > + drm_rect_fp_to_int(&src, &new_plane_state->uapi.src); > + damaged_clips = > drm_plane_get_damage_clips(&new_plane_state->uapi); > + > + for (j = 0; j < num_clips; j++) { > + struct drm_rect clip; > + > + clip.x1 = damaged_clips[j].x1; > + clip.y1 = damaged_clips[j].y1; > + clip.x2 = damaged_clips[j].x2; > + clip.y2 = damaged_clips[j].y2; > + if (drm_rect_intersect(&clip, &src)) > + clip_area_update(&damaged_area, &clip); > + } > > - temp = *sel_fetch_area; > - temp.y1 += new_plane_state->uapi.dst.y1; > - temp.y2 += new_plane_state->uapi.dst.y2; > - clip_area_update(&pipe_clip, &temp); > + if (damaged_area.y1 == -1) > + continue; > + > + damaged_area.y1 += new_plane_state->uapi.dst.y1 - > src.y1; > + damaged_area.y2 += new_plane_state->uapi.dst.y1 - > src.y1; > + clip_area_update(&pipe_clip, &damaged_area); > + } > + > + if (full_update) > + goto skip_sel_fetch_set_loop; > + > + /* It must be aligned to 4 lines */ > + pipe_clip.y1 -= pipe_clip.y1 % 4; > + if (pipe_clip.y2 % 4) > + pipe_clip.y2 = ((pipe_clip.y2 / 4) + 1) * 4; > + > + /* > + * Now that we have the pipe damaged area check if it intersect > with > + * every plane, if it does set the plane selective fetch area. > + */ > + for_each_oldnew_intel_plane_in_state(state, plane, > old_plane_state, > + new_plane_state, i) { > + struct drm_rect *sel_fetch_area, inter; > + > + if (new_plane_state->uapi.crtc != crtc_state->uapi.crtc > || > + !new_plane_state->uapi.visible) > + continue; > + > + inter = pipe_clip; > + if (!drm_rect_intersect(&inter, &new_plane_state- > >uapi.dst)) > + continue; > + > + sel_fetch_area = &new_plane_state->psr2_sel_fetch_area; > + sel_fetch_area->y1 = inter.y1 - new_plane_state- > >uapi.dst.y1; > + sel_fetch_area->y2 = inter.y2 - new_plane_state- > >uapi.dst.y1; > } > > +skip_sel_fetch_set_loop: > psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update); > return 0; > } _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx