| Message ID | 20201213183930.349592-3-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5,1/6] drm/damage_helper: Check if damage clips has valid values | expand |
On Sun, 2020-12-13 at 10:39 -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 > damadged area needs to be fetched from memory so the complete > blending > of all planes can happen. > > v2: > - do not shifthing 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 > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_psr.c | 93 ++++++++++++++++++++ > ---- > 1 file changed, 79 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..b256184821da 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -1269,11 +1269,11 @@ 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; > + int i, src_y1, src_y2, ret; > > if (!crtc_state->enable_psr2_sel_fetch) > return 0; > @@ -1282,9 +1282,21 @@ int intel_psr2_sel_fetch_update(struct > intel_atomic_state *state, > if (ret) > return ret; > > + src_y1 = new_plane_state->uapi.src.y1 >> 16; > + src_y2 = new_plane_state->uapi.src.y2 >> 16; > + > + /* > + * 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_mode_rect *damaged_clips; > + struct drm_rect sel_fetch_area = { .y1 = -1 }; > + u32 num_clips; > + int j; > > if (new_plane_state->uapi.crtc != crtc_state- > >uapi.crtc) > continue; > @@ -1300,23 +1312,76 @@ int intel_psr2_sel_fetch_update(struct > intel_atomic_state *state, > break; > } > > - if (!new_plane_state->uapi.visible) > - continue; > + 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 visibility or alpha changed or 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 (new_plane_state->uapi.visible != old_plane_state- > >uapi.visible || > + new_plane_state->uapi.alpha != old_plane_state- > >uapi.alpha || > + !drm_rect_equals(&new_plane_state->uapi.dst, > + &old_plane_state->uapi.dst)) { > + num_clips = 0; > + sel_fetch_area.y1 = src_y1; > + sel_fetch_area.y2 = src_y2; > + } 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 = src_y1; > + sel_fetch_area.y2 = src_y2; > + } > + > + for (j = 0; j < num_clips; j++) { > + struct drm_rect damage_area; > + > + damage_area.y1 = damaged_clips[j].y1 + src_y1; > + damage_area.y2 = damaged_clips[j].y2 + src_y1; > + clip_area_update(&sel_fetch_area, > &damage_area); > + } > + > + /* No damaged area in this plane */ > + if (sel_fetch_area.y1 == -1) { > + sel_fetch_area.y1 = 0; > + sel_fetch_area.y2 = 0; > + } > 1. Visibility change case The pipe damaged area (The Selective Update region) needs to accumulate both old and new plane's dst 2. Move case The pipe damaged area (The Selective Update region) needs to accumulate both old and new plane's dst 3. damage case if the damage clips don't intersect src we don't need to accumalate it to merged damage area. we can calculate the SelectiveUpdate area with plane's dst and only the damage area need to be translated to dst coordinates. > - temp = *sel_fetch_area; > - temp.y1 += new_plane_state->uapi.dst.y1; > - temp.y2 += new_plane_state->uapi.dst.y2; for translating to dst coordinates from src coordinates, we should do like this dst_damaged_area.y1 = src_damaged_area.y1 - plane's src.y1 + plane's dst.y1; > - clip_area_update(&pipe_clip, &temp); > + sel_fetch_area.y1 += new_plane_state->uapi.dst.y1; > + sel_fetch_area.y2 += new_plane_state->uapi.dst.y1; > + clip_area_update(&pipe_clip, &sel_fetch_area); > + } > + > + if (full_update) > + goto skip_sel_fetch_set_loop; > + > + /* > + * 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 + src_y1; > + sel_fetch_area->y2 = inter.y2 - new_plane_state- > >uapi.dst.y1 + src_y1; > } > > +skip_sel_fetch_set_loop: > psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update); > return 0; > }
On Mon, 2020-12-14 at 11:00 +0000, Mun, Gwan-gyeong wrote: > On Sun, 2020-12-13 at 10:39 -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 > > damadged area needs to be fetched from memory so the complete > > blending > > of all planes can happen. > > > > v2: > > - do not shifthing 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 > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_psr.c | 93 ++++++++++++++++++++ > > ---- > > 1 file changed, 79 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..b256184821da 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -1269,11 +1269,11 @@ 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; > > + int i, src_y1, src_y2, ret; > > > > > > > > > > if (!crtc_state->enable_psr2_sel_fetch) > > return 0; > > @@ -1282,9 +1282,21 @@ int intel_psr2_sel_fetch_update(struct > > intel_atomic_state *state, > > if (ret) > > return ret; > > > > > > > > > > + src_y1 = new_plane_state->uapi.src.y1 >> 16; > > + src_y2 = new_plane_state->uapi.src.y2 >> 16; > > + > > + /* > > + * 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_mode_rect *damaged_clips; > > + struct drm_rect sel_fetch_area = { .y1 = -1 }; > > + u32 num_clips; > > + int j; > > > > > > > > > > if (new_plane_state->uapi.crtc != crtc_state- > > > uapi.crtc) > > continue; > > @@ -1300,23 +1312,76 @@ int intel_psr2_sel_fetch_update(struct > > intel_atomic_state *state, > > break; > > } > > > > > > > > > > - if (!new_plane_state->uapi.visible) > > - continue; > > + 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 visibility or alpha changed or 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 (new_plane_state->uapi.visible != old_plane_state- > > > uapi.visible || > > + new_plane_state->uapi.alpha != old_plane_state- > > > uapi.alpha || > > + !drm_rect_equals(&new_plane_state->uapi.dst, > > + &old_plane_state->uapi.dst)) { > > + num_clips = 0; > > + sel_fetch_area.y1 = src_y1; > > + sel_fetch_area.y2 = src_y2; > > + } 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 = src_y1; > > + sel_fetch_area.y2 = src_y2; > > + } > > + > > + for (j = 0; j < num_clips; j++) { > > + struct drm_rect damage_area; > > + > > + damage_area.y1 = damaged_clips[j].y1 + src_y1; > > + damage_area.y2 = damaged_clips[j].y2 + src_y1; > > + clip_area_update(&sel_fetch_area, > > &damage_area); > > + } > > + > > + /* No damaged area in this plane */ > > + if (sel_fetch_area.y1 == -1) { > > + sel_fetch_area.y1 = 0; > > + sel_fetch_area.y2 = 0; > > + } > > > 1. Visibility change case > The pipe damaged area (The Selective Update region) needs to > accumulate both old and new plane's dst > > 2. Move case > The pipe damaged area (The Selective Update region) needs to > accumulate both old and new plane's dst > > 3. damage case > if the damage clips don't intersect src we don't need to accumalate it > to merged damage area. Makes sense, doing the changes. > > we can calculate the SelectiveUpdate area with plane's dst and only the > damage area need to be translated to dst coordinates. > > > - temp = *sel_fetch_area; > > - temp.y1 += new_plane_state->uapi.dst.y1; > > - temp.y2 += new_plane_state->uapi.dst.y2; > for translating to dst coordinates from src coordinates, > we should do like this > dst_damaged_area.y1 = src_damaged_area.y1 - plane's src.y1 + plane's > dst.y1; > > - clip_area_update(&pipe_clip, &temp); > > + sel_fetch_area.y1 += new_plane_state->uapi.dst.y1; > > + sel_fetch_area.y2 += new_plane_state->uapi.dst.y1; > > + clip_area_update(&pipe_clip, &sel_fetch_area); > > + } > > + > > + if (full_update) > > + goto skip_sel_fetch_set_loop; > > + > > + /* > > + * 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 + src_y1; > > + sel_fetch_area->y2 = inter.y2 - new_plane_state- > > > uapi.dst.y1 + src_y1; > > } > > > > > > > > > > +skip_sel_fetch_set_loop: > > psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update); > > return 0; > > }
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index d9a395c486d3..b256184821da 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1269,11 +1269,11 @@ 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; + int i, src_y1, src_y2, ret; if (!crtc_state->enable_psr2_sel_fetch) return 0; @@ -1282,9 +1282,21 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state, if (ret) return ret; + src_y1 = new_plane_state->uapi.src.y1 >> 16; + src_y2 = new_plane_state->uapi.src.y2 >> 16; + + /* + * 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_mode_rect *damaged_clips; + struct drm_rect sel_fetch_area = { .y1 = -1 }; + u32 num_clips; + int j; if (new_plane_state->uapi.crtc != crtc_state->uapi.crtc) continue; @@ -1300,23 +1312,76 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state, break; } - if (!new_plane_state->uapi.visible) - continue; + 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 visibility or alpha changed or 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 (new_plane_state->uapi.visible != old_plane_state->uapi.visible || + new_plane_state->uapi.alpha != old_plane_state->uapi.alpha || + !drm_rect_equals(&new_plane_state->uapi.dst, + &old_plane_state->uapi.dst)) { + num_clips = 0; + sel_fetch_area.y1 = src_y1; + sel_fetch_area.y2 = src_y2; + } 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 = src_y1; + sel_fetch_area.y2 = src_y2; + } + + for (j = 0; j < num_clips; j++) { + struct drm_rect damage_area; + + damage_area.y1 = damaged_clips[j].y1 + src_y1; + damage_area.y2 = damaged_clips[j].y2 + src_y1; + clip_area_update(&sel_fetch_area, &damage_area); + } + + /* No damaged area in this plane */ + if (sel_fetch_area.y1 == -1) { + sel_fetch_area.y1 = 0; + sel_fetch_area.y2 = 0; + } - 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); + sel_fetch_area.y1 += new_plane_state->uapi.dst.y1; + sel_fetch_area.y2 += new_plane_state->uapi.dst.y1; + clip_area_update(&pipe_clip, &sel_fetch_area); + } + + if (full_update) + goto skip_sel_fetch_set_loop; + + /* + * 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 + src_y1; + sel_fetch_area->y2 = inter.y2 - new_plane_state->uapi.dst.y1 + src_y1; } +skip_sel_fetch_set_loop: psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update); return 0; }
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 damadged area needs to be fetched from memory so the complete blending of all planes can happen. v2: - do not shifthing 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 Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/display/intel_psr.c | 93 ++++++++++++++++++++---- 1 file changed, 79 insertions(+), 14 deletions(-)