Message ID | 20200918010245.772724-3-jose.souza@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v3,1/3] drm/i915/display: Ignore IGNORE_PSR2_HW_TRACKING for platforms without sel fetch | expand |
On Thu, 2020-09-17 at 18:02 -0700, José Roberto de Souza wrote: > Another step towards PSR2 selective fetch, here programming plane > selective fetch registers and MAN_TRK_CTL enabling selective fetch > but > for now it is fetching the whole area of the planes. > The damaged area calculation will come as next and final step. > > v2: > - removed warn on when no plane is visible in state > - removed calculations using plane damaged area in > intel_psr2_program_plane_sel_fetch() > > v3: > - do not shift 16 positions the plane dst coordinates, only src is > shifted > > BSpec: 55229 > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 10 +- > drivers/gpu/drm/i915/display/intel_psr.c | 120 > ++++++++++++++++++- > drivers/gpu/drm/i915/display/intel_psr.h | 10 +- > drivers/gpu/drm/i915/display/intel_sprite.c | 3 + > 4 files changed, 134 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 5a9d933e425a..96bc515497c1 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -11812,6 +11812,9 @@ static void i9xx_update_cursor(struct > intel_plane *plane, > if (INTEL_GEN(dev_priv) >= 9) > skl_write_cursor_wm(plane, crtc_state); > > + if (!needs_modeset(crtc_state)) > + intel_psr2_program_plane_sel_fetch(plane, crtc_state, > plane_state, 0); > + > if (plane->cursor.base != base || > plane->cursor.size != fbc_ctl || > plane->cursor.cntl != cntl) { > @@ -12823,8 +12826,11 @@ static int intel_crtc_atomic_check(struct > intel_atomic_state *state, > > } > > - if (!mode_changed) > - intel_psr2_sel_fetch_update(state, crtc); > + if (!mode_changed) { > + ret = intel_psr2_sel_fetch_update(state, crtc); > + if (ret) > + return ret; > + } > > return 0; > } > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > b/drivers/gpu/drm/i915/display/intel_psr.c > index 02f74b0ddec1..deb0523f9f29 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -1166,6 +1166,41 @@ static void psr_force_hw_tracking_exit(struct > drm_i915_private *dev_priv) > intel_psr_exit(dev_priv); > } > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, > + const struct intel_crtc_state > *crtc_state, > + const struct intel_plane_state > *plane_state, > + int color_plane) > +{ > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > + enum pipe pipe = plane->pipe; > + u32 val; > + > + if (!crtc_state->enable_psr2_sel_fetch) > + return; > + > + /* > + * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for > gen11+, so > + * plane_state->ctl is the right value > + */ > + val = plane_state ? plane_state->ctl : 0; IMHO, skl_plane_ctl() might set other ctl bits, it would be better to have separated ctl bit value for "selective fetch ctl". > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane- > >id), val); > + if (!val || plane->id == PLANE_CURSOR) > + return; > + > + val = plane_state->uapi.dst.y1 << 16 | plane_state- > >uapi.dst.x1; > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane- > >id), val); > + > + val = plane_state->color_plane[color_plane].y << 16; > + val |= plane_state->color_plane[color_plane].x; > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane- > >id), > + val); > + > + /* Sizes are 0 based */ > + val = ((drm_rect_height(&plane_state->uapi.src) >> 16) - 1) << > 16; > + val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - 1; > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane- > >id), val); > +} > + > void intel_psr2_program_trans_man_trk_ctl(const struct > intel_crtc_state *crtc_state) > { > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > @@ -1180,16 +1215,91 @@ void > intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state > *crtc_st > crtc_state->psr2_man_track_ctl); > } > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state *state, > - struct intel_crtc *crtc) > +static void psr2_man_trk_ctl_calc(struct intel_crtc_state > *crtc_state, > + struct drm_rect *clip, bool > full_update) > +{ > + u32 val = PSR2_MAN_TRK_CTL_ENABLE; > + > + if (full_update) { > + val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME; > + goto exit; > + } > + > + if (clip->y1 == -1) > + goto exit; > + > + val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE; > + val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1); I think the start address block would be like this too. PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(DIV_ROUND_UP(clip->y1 , 4)); > + val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip- > >y2, 4) + 1); > +exit: > + crtc_state->psr2_man_track_ctl = val; > +} > + > +static void clip_area_update(struct drm_rect *overlap_damage_area, > + struct drm_rect *damage_area) > +{ > + if (overlap_damage_area->y1 == -1) { > + overlap_damage_area->y1 = damage_area->y1; > + overlap_damage_area->y2 = damage_area->y2; > + return; > + } > + > + if (damage_area->y1 < overlap_damage_area->y1) > + overlap_damage_area->y1 = damage_area->y1; > + > + if (damage_area->y2 > overlap_damage_area->y2) > + overlap_damage_area->y2 = damage_area->y2; > +} > + > +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_clip = { .y1 = -1 }; > + struct intel_plane *plane; > + bool full_update = false; > + int i, ret; > > if (!crtc_state->enable_psr2_sel_fetch) > - return; > + return 0; > + > + ret = drm_atomic_add_affected_planes(&state->base, &crtc- > >base); > + if (ret) > + return ret; > + > + for_each_oldnew_intel_plane_in_state(state, plane, > old_plane_state, > + new_plane_state, i) { > + struct drm_rect temp; > + > + if (new_plane_state->uapi.crtc != crtc_state- > >uapi.crtc) > + continue; > > - crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE | > - PSR2_MAN_TRK_CTL_SF_SINGLE_FUL > L_FRAME; > + /* > + * TODO: Not clear how to handle planes with negative > position, > + * also planes are not updated if they have a negative > X > + * position so for now doing a full update in this > cases > + */ > + if (new_plane_state->uapi.dst.y1 < 0 || > + new_plane_state->uapi.dst.x1 < 0) { > + full_update = true; > + break; > + } > + > + if (!new_plane_state->uapi.visible) > + continue; > + > + /* > + * For now doing a selective fetch in the whole plane > area, > + * optimizations will come in the future. > + */ > + temp.y1 = new_plane_state->uapi.dst.y1; > + temp.y2 = new_plane_state->uapi.dst.y2; > + clip_area_update(&pipe_clip, &temp); > + } > + > + psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update); > + return 0; > } > > /** > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h > b/drivers/gpu/drm/i915/display/intel_psr.h > index 6a83c8e682e6..3eca9dcec3c0 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.h > +++ b/drivers/gpu/drm/i915/display/intel_psr.h > @@ -15,6 +15,8 @@ struct intel_crtc_state; > struct intel_dp; > struct intel_crtc; > struct intel_atomic_state; > +struct intel_plane_state; > +struct intel_plane; > > #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv- > >psr.sink_support) > void intel_psr_init_dpcd(struct intel_dp *intel_dp); > @@ -45,8 +47,12 @@ void intel_psr_atomic_check(struct drm_connector > *connector, > struct drm_connector_state *old_state, > struct drm_connector_state *new_state); > void intel_psr_set_force_mode_changed(struct intel_dp *intel_dp); > -void intel_psr2_sel_fetch_update(struct intel_atomic_state *state, > - struct intel_crtc *crtc); > +int intel_psr2_sel_fetch_update(struct intel_atomic_state *state, > + struct intel_crtc *crtc); > void intel_psr2_program_trans_man_trk_ctl(const struct > intel_crtc_state *crtc_state); > +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, > + const struct intel_crtc_state > *crtc_state, > + const struct intel_plane_state > *plane_state, > + int color_plane); > > #endif /* __INTEL_PSR_H__ */ > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c > b/drivers/gpu/drm/i915/display/intel_sprite.c > index 63040cb0d4e1..f12425b90607 100644 > --- a/drivers/gpu/drm/i915/display/intel_sprite.c > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c > @@ -690,6 +690,9 @@ skl_program_plane(struct intel_plane *plane, > intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe, > plane_id), > (plane_state->color_plane[1].y << 16) > | plane_state->color_plane[1].x); > > + if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi)) > + intel_psr2_program_plane_sel_fetch(plane, crtc_state, > plane_state, color_plane); > + > /* > * The control register self-arms if the plane was previously > * disabled. Try to make the plane enable atomic by writing
On Wed, 2020-09-23 at 14:02 +0100, Mun, Gwan-gyeong wrote: > On Thu, 2020-09-17 at 18:02 -0700, José Roberto de Souza wrote: > > Another step towards PSR2 selective fetch, here programming plane > > selective fetch registers and MAN_TRK_CTL enabling selective fetch > > but > > for now it is fetching the whole area of the planes. > > The damaged area calculation will come as next and final step. > > > > v2: > > - removed warn on when no plane is visible in state > > - removed calculations using plane damaged area in > > intel_psr2_program_plane_sel_fetch() > > > > v3: > > - do not shift 16 positions the plane dst coordinates, only src is > > shifted > > > > BSpec: 55229 > > Cc: Gwan-gyeong Mun < > > gwan-gyeong.mun@intel.com > > > > > Cc: Ville Syrjälä < > > ville.syrjala@linux.intel.com > > > > > Signed-off-by: José Roberto de Souza < > > jose.souza@intel.com > > > > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 10 +- > > drivers/gpu/drm/i915/display/intel_psr.c | 120 > > ++++++++++++++++++- > > drivers/gpu/drm/i915/display/intel_psr.h | 10 +- > > drivers/gpu/drm/i915/display/intel_sprite.c | 3 + > > 4 files changed, 134 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 5a9d933e425a..96bc515497c1 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -11812,6 +11812,9 @@ static void i9xx_update_cursor(struct > > intel_plane *plane, > > if (INTEL_GEN(dev_priv) >= 9) > > skl_write_cursor_wm(plane, crtc_state); > > > > + if (!needs_modeset(crtc_state)) > > + intel_psr2_program_plane_sel_fetch(plane, crtc_state, > > plane_state, 0); > > + > > if (plane->cursor.base != base || > > plane->cursor.size != fbc_ctl || > > plane->cursor.cntl != cntl) { > > @@ -12823,8 +12826,11 @@ static int intel_crtc_atomic_check(struct > > intel_atomic_state *state, > > > > } > > > > - if (!mode_changed) > > - intel_psr2_sel_fetch_update(state, crtc); > > + if (!mode_changed) { > > + ret = intel_psr2_sel_fetch_update(state, crtc); > > + if (ret) > > + return ret; > > + } > > > > return 0; > > } > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > b/drivers/gpu/drm/i915/display/intel_psr.c > > index 02f74b0ddec1..deb0523f9f29 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -1166,6 +1166,41 @@ static void psr_force_hw_tracking_exit(struct > > drm_i915_private *dev_priv) > > intel_psr_exit(dev_priv); > > } > > > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, > > + const struct intel_crtc_state > > *crtc_state, > > + const struct intel_plane_state > > *plane_state, > > + int color_plane) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > + enum pipe pipe = plane->pipe; > > + u32 val; > > + > > + if (!crtc_state->enable_psr2_sel_fetch) > > + return; > > + > > + /* > > + * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for > > gen11+, so > > + * plane_state->ctl is the right value > > + */ > > + val = plane_state ? plane_state->ctl : 0; > > IMHO, skl_plane_ctl() might set other ctl bits, it would be better to > have separated ctl bit value for "selective fetch ctl". Like said all other bits are spares so can be set without issues but okay will a "plane_state->ctl & PLANE_SEL_FETCH_CTL_ENABLE". > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane- > > > id), val); > > > > + if (!val || plane->id == PLANE_CURSOR) > > + return; > > + > > + val = plane_state->uapi.dst.y1 << 16 | plane_state- > > > uapi.dst.x1; > > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane- > > > id), val); > > > > + > > + val = plane_state->color_plane[color_plane].y << 16; > > + val |= plane_state->color_plane[color_plane].x; > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane- > > > id), > > > > + val); > > + > > + /* Sizes are 0 based */ > > + val = ((drm_rect_height(&plane_state->uapi.src) >> 16) - 1) << > > 16; > > + val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - 1; > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane- > > > id), val); > > > > +} > > + > > void intel_psr2_program_trans_man_trk_ctl(const struct > > intel_crtc_state *crtc_state) > > { > > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > @@ -1180,16 +1215,91 @@ void > > intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state > > *crtc_st > > crtc_state->psr2_man_track_ctl); > > } > > > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state *state, > > - struct intel_crtc *crtc) > > +static void psr2_man_trk_ctl_calc(struct intel_crtc_state > > *crtc_state, > > + struct drm_rect *clip, bool > > full_update) > > +{ > > + u32 val = PSR2_MAN_TRK_CTL_ENABLE; > > + > > + if (full_update) { > > + val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME; > > + goto exit; > > + } > > + > > + if (clip->y1 == -1) > > + goto exit; > > + > > + val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE; > > + val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1); > > I think the start address block would be like this too. > PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(DIV_ROUND_UP(clip->y1 , 4)); This would not match for numbers that divide by 4. y pos 0 = region 1y pos 1 = region 1 y pos 4 = region 2 > > + val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip- > > > y2, 4) + 1); > > > > +exit: > > + crtc_state->psr2_man_track_ctl = val; > > +} > > + > > +static void clip_area_update(struct drm_rect *overlap_damage_area, > > + struct drm_rect *damage_area) > > +{ > > + if (overlap_damage_area->y1 == -1) { > > + overlap_damage_area->y1 = damage_area->y1; > > + overlap_damage_area->y2 = damage_area->y2; > > + return; > > + } > > + > > + if (damage_area->y1 < overlap_damage_area->y1) > > + overlap_damage_area->y1 = damage_area->y1; > > + > > + if (damage_area->y2 > overlap_damage_area->y2) > > + overlap_damage_area->y2 = damage_area->y2; > > +} > > + > > +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_clip = { .y1 = -1 }; > > + struct intel_plane *plane; > > + bool full_update = false; > > + int i, ret; > > > > if (!crtc_state->enable_psr2_sel_fetch) > > - return; > > + return 0; > > + > > + ret = drm_atomic_add_affected_planes(&state->base, &crtc- > > > base); > > > > + if (ret) > > + return ret; > > + > > + for_each_oldnew_intel_plane_in_state(state, plane, > > old_plane_state, > > + new_plane_state, i) { > > + struct drm_rect temp; > > + > > + if (new_plane_state->uapi.crtc != crtc_state- > > > uapi.crtc) > > > > + continue; > > > > - crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE | > > - PSR2_MAN_TRK_CTL_SF_SINGLE_FUL > > L_FRAME; > > + /* > > + * TODO: Not clear how to handle planes with negative > > position, > > + * also planes are not updated if they have a negative > > X > > + * position so for now doing a full update in this > > cases > > + */ > > + if (new_plane_state->uapi.dst.y1 < 0 || > > + new_plane_state->uapi.dst.x1 < 0) { > > + full_update = true; > > + break; > > + } > > + > > + if (!new_plane_state->uapi.visible) > > + continue; > > + > > + /* > > + * For now doing a selective fetch in the whole plane > > area, > > + * optimizations will come in the future. > > + */ > > + temp.y1 = new_plane_state->uapi.dst.y1; > > + temp.y2 = new_plane_state->uapi.dst.y2; > > + clip_area_update(&pipe_clip, &temp); > > + } > > + > > + psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update); > > + return 0; > > } > > > > /** > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h > > b/drivers/gpu/drm/i915/display/intel_psr.h > > index 6a83c8e682e6..3eca9dcec3c0 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.h > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h > > @@ -15,6 +15,8 @@ struct intel_crtc_state; > > struct intel_dp; > > struct intel_crtc; > > struct intel_atomic_state; > > +struct intel_plane_state; > > +struct intel_plane; > > > > #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv- > > > psr.sink_support) > > > > void intel_psr_init_dpcd(struct intel_dp *intel_dp); > > @@ -45,8 +47,12 @@ void intel_psr_atomic_check(struct drm_connector > > *connector, > > struct drm_connector_state *old_state, > > struct drm_connector_state *new_state); > > void intel_psr_set_force_mode_changed(struct intel_dp *intel_dp); > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state *state, > > - struct intel_crtc *crtc); > > +int intel_psr2_sel_fetch_update(struct intel_atomic_state *state, > > + struct intel_crtc *crtc); > > void intel_psr2_program_trans_man_trk_ctl(const struct > > intel_crtc_state *crtc_state); > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, > > + const struct intel_crtc_state > > *crtc_state, > > + const struct intel_plane_state > > *plane_state, > > + int color_plane); > > > > #endif /* __INTEL_PSR_H__ */ > > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c > > b/drivers/gpu/drm/i915/display/intel_sprite.c > > index 63040cb0d4e1..f12425b90607 100644 > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c > > @@ -690,6 +690,9 @@ skl_program_plane(struct intel_plane *plane, > > intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe, > > plane_id), > > (plane_state->color_plane[1].y << 16) > > > plane_state->color_plane[1].x); > > > > > > + if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi)) > > + intel_psr2_program_plane_sel_fetch(plane, crtc_state, > > plane_state, color_plane); > > + > > /* > > * The control register self-arms if the plane was previously > > * disabled. Try to make the plane enable atomic by writing
On Wed, 2020-09-23 at 10:10 -0700, José Roberto de Souza wrote: > On Wed, 2020-09-23 at 14:02 +0100, Mun, Gwan-gyeong wrote: > > On Thu, 2020-09-17 at 18:02 -0700, José Roberto de Souza wrote: > > > Another step towards PSR2 selective fetch, here programming plane > > > selective fetch registers and MAN_TRK_CTL enabling selective fetch > > > but > > > for now it is fetching the whole area of the planes. > > > The damaged area calculation will come as next and final step. > > > > > > v2: > > > - removed warn on when no plane is visible in state > > > - removed calculations using plane damaged area in > > > intel_psr2_program_plane_sel_fetch() > > > > > > v3: > > > - do not shift 16 positions the plane dst coordinates, only src is > > > shifted > > > > > > BSpec: 55229 > > > Cc: Gwan-gyeong Mun < > > > gwan-gyeong.mun@intel.com > > > > > > > > > Cc: Ville Syrjälä < > > > ville.syrjala@linux.intel.com > > > > > > > > > Signed-off-by: José Roberto de Souza < > > > jose.souza@intel.com > > > > > > > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 10 +- > > > drivers/gpu/drm/i915/display/intel_psr.c | 120 > > > ++++++++++++++++++- > > > drivers/gpu/drm/i915/display/intel_psr.h | 10 +- > > > drivers/gpu/drm/i915/display/intel_sprite.c | 3 + > > > 4 files changed, 134 insertions(+), 9 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > index 5a9d933e425a..96bc515497c1 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -11812,6 +11812,9 @@ static void i9xx_update_cursor(struct > > > intel_plane *plane, > > > if (INTEL_GEN(dev_priv) >= 9) > > > skl_write_cursor_wm(plane, crtc_state); > > > > > > + if (!needs_modeset(crtc_state)) > > > + intel_psr2_program_plane_sel_fetch(plane, crtc_state, > > > plane_state, 0); > > > + > > > if (plane->cursor.base != base || > > > plane->cursor.size != fbc_ctl || > > > plane->cursor.cntl != cntl) { > > > @@ -12823,8 +12826,11 @@ static int intel_crtc_atomic_check(struct > > > intel_atomic_state *state, > > > > > > } > > > > > > - if (!mode_changed) > > > - intel_psr2_sel_fetch_update(state, crtc); > > > + if (!mode_changed) { > > > + ret = intel_psr2_sel_fetch_update(state, crtc); > > > + if (ret) > > > + return ret; > > > + } > > > > > > return 0; > > > } > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > > b/drivers/gpu/drm/i915/display/intel_psr.c > > > index 02f74b0ddec1..deb0523f9f29 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > @@ -1166,6 +1166,41 @@ static void psr_force_hw_tracking_exit(struct > > > drm_i915_private *dev_priv) > > > intel_psr_exit(dev_priv); > > > } > > > > > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, > > > + const struct intel_crtc_state > > > *crtc_state, > > > + const struct intel_plane_state > > > *plane_state, > > > + int color_plane) > > > +{ > > > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > > > + enum pipe pipe = plane->pipe; > > > + u32 val; > > > + > > > + if (!crtc_state->enable_psr2_sel_fetch) > > > + return; > > > + > > > + /* > > > + * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for > > > gen11+, so > > > + * plane_state->ctl is the right value > > > + */ > > > + val = plane_state ? plane_state->ctl : 0; > > > > IMHO, skl_plane_ctl() might set other ctl bits, it would be better to > > have separated ctl bit value for "selective fetch ctl". > > Like said all other bits are spares so can be set without issues but okay will a "plane_state->ctl & PLANE_SEL_FETCH_CTL_ENABLE". Please take a look to the answer of your other comment bellow, with the change above can I have your rv-b? > > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane- > > > > id), val); > > > > > > + if (!val || plane->id == PLANE_CURSOR) > > > + return; > > > + > > > + val = plane_state->uapi.dst.y1 << 16 | plane_state- > > > > uapi.dst.x1; > > > > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane- > > > > id), val); > > > > > > + > > > + val = plane_state->color_plane[color_plane].y << 16; > > > + val |= plane_state->color_plane[color_plane].x; > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane- > > > > id), > > > > > > + val); > > > + > > > + /* Sizes are 0 based */ > > > + val = ((drm_rect_height(&plane_state->uapi.src) >> 16) - 1) << > > > 16; > > > + val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - 1; > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane- > > > > id), val); > > > > > > +} > > > + > > > void intel_psr2_program_trans_man_trk_ctl(const struct > > > intel_crtc_state *crtc_state) > > > { > > > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > > @@ -1180,16 +1215,91 @@ void > > > intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state > > > *crtc_st > > > crtc_state->psr2_man_track_ctl); > > > } > > > > > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state *state, > > > - struct intel_crtc *crtc) > > > +static void psr2_man_trk_ctl_calc(struct intel_crtc_state > > > *crtc_state, > > > + struct drm_rect *clip, bool > > > full_update) > > > +{ > > > + u32 val = PSR2_MAN_TRK_CTL_ENABLE; > > > + > > > + if (full_update) { > > > + val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME; > > > + goto exit; > > > + } > > > + > > > + if (clip->y1 == -1) > > > + goto exit; > > > + > > > + val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE; > > > + val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 + 1); > > > > I think the start address block would be like this too. > > PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(DIV_ROUND_UP(clip->y1 , 4)); > > This would not match for numbers that divide by 4. > y pos 0 = region 1y pos 1 = region 1 > y pos 4 = region 2 > > > > > + val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip- > > > > y2, 4) + 1); > > > > > > +exit: > > > + crtc_state->psr2_man_track_ctl = val; > > > +} > > > + > > > +static void clip_area_update(struct drm_rect *overlap_damage_area, > > > + struct drm_rect *damage_area) > > > +{ > > > + if (overlap_damage_area->y1 == -1) { > > > + overlap_damage_area->y1 = damage_area->y1; > > > + overlap_damage_area->y2 = damage_area->y2; > > > + return; > > > + } > > > + > > > + if (damage_area->y1 < overlap_damage_area->y1) > > > + overlap_damage_area->y1 = damage_area->y1; > > > + > > > + if (damage_area->y2 > overlap_damage_area->y2) > > > + overlap_damage_area->y2 = damage_area->y2; > > > +} > > > + > > > +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_clip = { .y1 = -1 }; > > > + struct intel_plane *plane; > > > + bool full_update = false; > > > + int i, ret; > > > > > > if (!crtc_state->enable_psr2_sel_fetch) > > > - return; > > > + return 0; > > > + > > > + ret = drm_atomic_add_affected_planes(&state->base, &crtc- > > > > base); > > > > > > + if (ret) > > > + return ret; > > > + > > > + for_each_oldnew_intel_plane_in_state(state, plane, > > > old_plane_state, > > > + new_plane_state, i) { > > > + struct drm_rect temp; > > > + > > > + if (new_plane_state->uapi.crtc != crtc_state- > > > > uapi.crtc) > > > > > > + continue; > > > > > > - crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE | > > > - PSR2_MAN_TRK_CTL_SF_SINGLE_FUL > > > L_FRAME; > > > + /* > > > + * TODO: Not clear how to handle planes with negative > > > position, > > > + * also planes are not updated if they have a negative > > > X > > > + * position so for now doing a full update in this > > > cases > > > + */ > > > + if (new_plane_state->uapi.dst.y1 < 0 || > > > + new_plane_state->uapi.dst.x1 < 0) { > > > + full_update = true; > > > + break; > > > + } > > > + > > > + if (!new_plane_state->uapi.visible) > > > + continue; > > > + > > > + /* > > > + * For now doing a selective fetch in the whole plane > > > area, > > > + * optimizations will come in the future. > > > + */ > > > + temp.y1 = new_plane_state->uapi.dst.y1; > > > + temp.y2 = new_plane_state->uapi.dst.y2; > > > + clip_area_update(&pipe_clip, &temp); > > > + } > > > + > > > + psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update); > > > + return 0; > > > } > > > > > > /** > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h > > > b/drivers/gpu/drm/i915/display/intel_psr.h > > > index 6a83c8e682e6..3eca9dcec3c0 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_psr.h > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h > > > @@ -15,6 +15,8 @@ struct intel_crtc_state; > > > struct intel_dp; > > > struct intel_crtc; > > > struct intel_atomic_state; > > > +struct intel_plane_state; > > > +struct intel_plane; > > > > > > #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv- > > > > psr.sink_support) > > > > > > void intel_psr_init_dpcd(struct intel_dp *intel_dp); > > > @@ -45,8 +47,12 @@ void intel_psr_atomic_check(struct drm_connector > > > *connector, > > > struct drm_connector_state *old_state, > > > struct drm_connector_state *new_state); > > > void intel_psr_set_force_mode_changed(struct intel_dp *intel_dp); > > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state *state, > > > - struct intel_crtc *crtc); > > > +int intel_psr2_sel_fetch_update(struct intel_atomic_state *state, > > > + struct intel_crtc *crtc); > > > void intel_psr2_program_trans_man_trk_ctl(const struct > > > intel_crtc_state *crtc_state); > > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, > > > + const struct intel_crtc_state > > > *crtc_state, > > > + const struct intel_plane_state > > > *plane_state, > > > + int color_plane); > > > > > > #endif /* __INTEL_PSR_H__ */ > > > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c > > > b/drivers/gpu/drm/i915/display/intel_sprite.c > > > index 63040cb0d4e1..f12425b90607 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c > > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c > > > @@ -690,6 +690,9 @@ skl_program_plane(struct intel_plane *plane, > > > intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe, > > > plane_id), > > > (plane_state->color_plane[1].y << 16) > > > > plane_state->color_plane[1].x); > > > > > > > > > + if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi)) > > > + intel_psr2_program_plane_sel_fetch(plane, crtc_state, > > > plane_state, color_plane); > > > + > > > /* > > > * The control register self-arms if the plane was previously > > > * disabled. Try to make the plane enable atomic by writing
On Wed, 2020-09-23 at 10:09 -0700, Souza, Jose wrote: > On Wed, 2020-09-23 at 10:10 -0700, José Roberto de Souza wrote: > > On Wed, 2020-09-23 at 14:02 +0100, Mun, Gwan-gyeong wrote: > > > On Thu, 2020-09-17 at 18:02 -0700, José Roberto de Souza wrote: > > > > Another step towards PSR2 selective fetch, here programming > > > > plane > > > > selective fetch registers and MAN_TRK_CTL enabling selective > > > > fetch > > > > but > > > > for now it is fetching the whole area of the planes. > > > > The damaged area calculation will come as next and final step. > > > > > > > > v2: > > > > - removed warn on when no plane is visible in state > > > > - removed calculations using plane damaged area in > > > > intel_psr2_program_plane_sel_fetch() > > > > > > > > v3: > > > > - do not shift 16 positions the plane dst coordinates, only src > > > > is > > > > shifted > > > > > > > > BSpec: 55229 > > > > Cc: Gwan-gyeong Mun < > > > > gwan-gyeong.mun@intel.com > > > > > > > > > > > > Cc: Ville Syrjälä < > > > > ville.syrjala@linux.intel.com > > > > > > > > > > > > Signed-off-by: José Roberto de Souza < > > > > jose.souza@intel.com > > > > > > > > > > > > --- > > > > drivers/gpu/drm/i915/display/intel_display.c | 10 +- > > > > drivers/gpu/drm/i915/display/intel_psr.c | 120 > > > > ++++++++++++++++++- > > > > drivers/gpu/drm/i915/display/intel_psr.h | 10 +- > > > > drivers/gpu/drm/i915/display/intel_sprite.c | 3 + > > > > 4 files changed, 134 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > index 5a9d933e425a..96bc515497c1 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > @@ -11812,6 +11812,9 @@ static void i9xx_update_cursor(struct > > > > intel_plane *plane, > > > > if (INTEL_GEN(dev_priv) >= 9) > > > > skl_write_cursor_wm(plane, crtc_state); > > > > > > > > + if (!needs_modeset(crtc_state)) > > > > + intel_psr2_program_plane_sel_fetch(plane, > > > > crtc_state, > > > > plane_state, 0); > > > > + > > > > if (plane->cursor.base != base || > > > > plane->cursor.size != fbc_ctl || > > > > plane->cursor.cntl != cntl) { > > > > @@ -12823,8 +12826,11 @@ static int > > > > intel_crtc_atomic_check(struct > > > > intel_atomic_state *state, > > > > > > > > } > > > > > > > > - if (!mode_changed) > > > > - intel_psr2_sel_fetch_update(state, crtc); > > > > + if (!mode_changed) { > > > > + ret = intel_psr2_sel_fetch_update(state, crtc); > > > > + if (ret) > > > > + return ret; > > > > + } > > > > > > > > return 0; > > > > } > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > > > b/drivers/gpu/drm/i915/display/intel_psr.c > > > > index 02f74b0ddec1..deb0523f9f29 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > @@ -1166,6 +1166,41 @@ static void > > > > psr_force_hw_tracking_exit(struct > > > > drm_i915_private *dev_priv) > > > > intel_psr_exit(dev_priv); > > > > } > > > > > > > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane > > > > *plane, > > > > + const struct > > > > intel_crtc_state > > > > *crtc_state, > > > > + const struct > > > > intel_plane_state > > > > *plane_state, > > > > + int color_plane) > > > > +{ > > > > + struct drm_i915_private *dev_priv = to_i915(plane- > > > > >base.dev); > > > > + enum pipe pipe = plane->pipe; > > > > + u32 val; > > > > + > > > > + if (!crtc_state->enable_psr2_sel_fetch) > > > > + return; > > > > + > > > > + /* > > > > + * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 > > > > for > > > > gen11+, so > > > > + * plane_state->ctl is the right value > > > > + */ > > > > + val = plane_state ? plane_state->ctl : 0; > > > > > > IMHO, skl_plane_ctl() might set other ctl bits, it would be > > > better to > > > have separated ctl bit value for "selective fetch ctl". > > > > Like said all other bits are spares so can be set without issues > > but okay will a "plane_state->ctl & PLANE_SEL_FETCH_CTL_ENABLE". > > Please take a look to the answer of your other comment bellow, with > the change above can I have your rv-b? The purpose and composition of bits of Register_SEL_FETCH_PLANE_CTL is different from Register_PLANE_CTL. And the Spares bits of Register_SEL_FETCH_PLANE_CTL might be used for other purpose. (And we current don't know the side effect of setting of Spares bit of Register_SEL_FETCH_PLANE_CTL ) therefor I recommend "Read and Modify" to SEL_FETCH_PLANE_CTL. > > > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, > > > > plane- > > > > > id), val); > > > > > > > > + if (!val || plane->id == PLANE_CURSOR) > > > > + return; > > > > + > > > > + val = plane_state->uapi.dst.y1 << 16 | plane_state- > > > > > uapi.dst.x1; > > > > > > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, > > > > plane- > > > > > id), val); > > > > > > > > + > > > > + val = plane_state->color_plane[color_plane].y << 16; > > > > + val |= plane_state->color_plane[color_plane].x; > > > > + intel_de_write_fw(dev_priv, > > > > PLANE_SEL_FETCH_OFFSET(pipe, plane- > > > > > id), > > > > > > > > + val); > > > > + > > > > + /* Sizes are 0 based */ > > > > + val = ((drm_rect_height(&plane_state->uapi.src) >> 16) > > > > - 1) << > > > > 16; > > > > + val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - > > > > 1; > > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, > > > > plane- > > > > > id), val); > > > > > > > > +} > > > > + > > > > void intel_psr2_program_trans_man_trk_ctl(const struct > > > > intel_crtc_state *crtc_state) > > > > { > > > > struct intel_crtc *crtc = to_intel_crtc(crtc_state- > > > > >uapi.crtc); > > > > @@ -1180,16 +1215,91 @@ void > > > > intel_psr2_program_trans_man_trk_ctl(const struct > > > > intel_crtc_state > > > > *crtc_st > > > > crtc_state->psr2_man_track_ctl); > > > > } > > > > > > > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state > > > > *state, > > > > - struct intel_crtc *crtc) > > > > +static void psr2_man_trk_ctl_calc(struct intel_crtc_state > > > > *crtc_state, > > > > + struct drm_rect *clip, bool > > > > full_update) > > > > +{ > > > > + u32 val = PSR2_MAN_TRK_CTL_ENABLE; > > > > + > > > > + if (full_update) { > > > > + val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME; > > > > + goto exit; > > > > + } > > > > + > > > > + if (clip->y1 == -1) > > > > + goto exit; > > > > + > > > > + val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE; > > > > + val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / > > > > 4 + 1); > > > > > > I think the start address block would be like this too. > > > PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(DIV_ROUND_UP(clip->y1 , > > > 4)); > > > > This would not match for numbers that divide by 4. > > y pos 0 = region 1y pos 1 = region 1 > > y pos 4 = region 2 > > yes. you are right. > > > > > > + val |= > > > > PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip- > > > > > y2, 4) + 1); > > > > > > > > +exit: > > > > + crtc_state->psr2_man_track_ctl = val; > > > > +} > > > > + > > > > +static void clip_area_update(struct drm_rect > > > > *overlap_damage_area, > > > > + struct drm_rect *damage_area) > > > > +{ > > > > + if (overlap_damage_area->y1 == -1) { > > > > + overlap_damage_area->y1 = damage_area->y1; > > > > + overlap_damage_area->y2 = damage_area->y2; > > > > + return; > > > > + } > > > > + > > > > + if (damage_area->y1 < overlap_damage_area->y1) > > > > + overlap_damage_area->y1 = damage_area->y1; > > > > + > > > > + if (damage_area->y2 > overlap_damage_area->y2) > > > > + overlap_damage_area->y2 = damage_area->y2; > > > > +} > > > > + > > > > +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_clip = { .y1 = -1 }; > > > > + struct intel_plane *plane; > > > > + bool full_update = false; > > > > + int i, ret; > > > > > > > > if (!crtc_state->enable_psr2_sel_fetch) > > > > - return; > > > > + return 0; > > > > + > > > > + ret = drm_atomic_add_affected_planes(&state->base, > > > > &crtc- > > > > > base); > > > > > > > > + if (ret) > > > > + return ret; > > > > + > > > > + for_each_oldnew_intel_plane_in_state(state, plane, > > > > old_plane_state, > > > > + new_plane_state, > > > > i) { > > > > + struct drm_rect temp; > > > > + > > > > + if (new_plane_state->uapi.crtc != crtc_state- > > > > > uapi.crtc) > > > > > > > > + continue; > > > > > > > > - crtc_state->psr2_man_track_ctl = > > > > PSR2_MAN_TRK_CTL_ENABLE | > > > > - PSR2_MAN_TRK_CTL_SF_SI > > > > NGLE_FUL > > > > L_FRAME; > > > > + /* > > > > + * TODO: Not clear how to handle planes with > > > > negative > > > > position, > > > > + * also planes are not updated if they have a > > > > negative > > > > X > > > > + * position so for now doing a full update in > > > > this > > > > cases > > > > + */ > > > > + if (new_plane_state->uapi.dst.y1 < 0 || > > > > + new_plane_state->uapi.dst.x1 < 0) { > > > > + full_update = true; > > > > + break; > > > > + } > > > > + > > > > + if (!new_plane_state->uapi.visible) > > > > + continue; > > > > + > > > > + /* > > > > + * For now doing a selective fetch in the whole > > > > plane > > > > area, > > > > + * optimizations will come in the future. > > > > + */ > > > > + temp.y1 = new_plane_state->uapi.dst.y1; > > > > + temp.y2 = new_plane_state->uapi.dst.y2; > > > > + clip_area_update(&pipe_clip, &temp); > > > > + } > > > > + > > > > + psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, > > > > full_update); > > > > + return 0; > > > > } > > > > > > > > /** > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h > > > > b/drivers/gpu/drm/i915/display/intel_psr.h > > > > index 6a83c8e682e6..3eca9dcec3c0 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.h > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h > > > > @@ -15,6 +15,8 @@ struct intel_crtc_state; > > > > struct intel_dp; > > > > struct intel_crtc; > > > > struct intel_atomic_state; > > > > +struct intel_plane_state; > > > > +struct intel_plane; > > > > > > > > #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv- > > > > > psr.sink_support) > > > > > > > > void intel_psr_init_dpcd(struct intel_dp *intel_dp); > > > > @@ -45,8 +47,12 @@ void intel_psr_atomic_check(struct > > > > drm_connector > > > > *connector, > > > > struct drm_connector_state > > > > *old_state, > > > > struct drm_connector_state > > > > *new_state); > > > > void intel_psr_set_force_mode_changed(struct intel_dp > > > > *intel_dp); > > > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state > > > > *state, > > > > - struct intel_crtc *crtc); > > > > +int intel_psr2_sel_fetch_update(struct intel_atomic_state > > > > *state, > > > > + struct intel_crtc *crtc); > > > > void intel_psr2_program_trans_man_trk_ctl(const struct > > > > intel_crtc_state *crtc_state); > > > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane > > > > *plane, > > > > + const struct > > > > intel_crtc_state > > > > *crtc_state, > > > > + const struct > > > > intel_plane_state > > > > *plane_state, > > > > + int color_plane); > > > > > > > > #endif /* __INTEL_PSR_H__ */ > > > > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c > > > > b/drivers/gpu/drm/i915/display/intel_sprite.c > > > > index 63040cb0d4e1..f12425b90607 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c > > > > @@ -690,6 +690,9 @@ skl_program_plane(struct intel_plane > > > > *plane, > > > > intel_de_write_fw(dev_priv, > > > > PLANE_AUX_OFFSET(pipe, > > > > plane_id), > > > > (plane_state- > > > > >color_plane[1].y << 16) > > > > > plane_state->color_plane[1].x); > > > > > > > > > > > > + if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi)) > > > > + intel_psr2_program_plane_sel_fetch(plane, > > > > crtc_state, > > > > plane_state, color_plane); > > > > + > > > > /* > > > > * The control register self-arms if the plane was > > > > previously > > > > * disabled. Try to make the plane enable atomic by > > > > writing
On Thu, 2020-09-24 at 14:27 +0100, Mun, Gwan-gyeong wrote: > On Wed, 2020-09-23 at 10:09 -0700, Souza, Jose wrote: > > On Wed, 2020-09-23 at 10:10 -0700, José Roberto de Souza wrote: > > > On Wed, 2020-09-23 at 14:02 +0100, Mun, Gwan-gyeong wrote: > > > > On Thu, 2020-09-17 at 18:02 -0700, José Roberto de Souza wrote: > > > > > Another step towards PSR2 selective fetch, here programming > > > > > plane > > > > > selective fetch registers and MAN_TRK_CTL enabling selective > > > > > fetch > > > > > but > > > > > for now it is fetching the whole area of the planes. > > > > > The damaged area calculation will come as next and final step. > > > > > > > > > > v2: > > > > > - removed warn on when no plane is visible in state > > > > > - removed calculations using plane damaged area in > > > > > intel_psr2_program_plane_sel_fetch() > > > > > > > > > > v3: > > > > > - do not shift 16 positions the plane dst coordinates, only src > > > > > is > > > > > shifted > > > > > > > > > > BSpec: 55229 > > > > > Cc: Gwan-gyeong Mun < > > > > > gwan-gyeong.mun@intel.com > > > > > > > > > > > > > > > > > > > > Cc: Ville Syrjälä < > > > > > ville.syrjala@linux.intel.com > > > > > > > > > > > > > > > > > > > > Signed-off-by: José Roberto de Souza < > > > > > jose.souza@intel.com > > > > > > > > > > > > > > > > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_display.c | 10 +- > > > > > drivers/gpu/drm/i915/display/intel_psr.c | 120 > > > > > ++++++++++++++++++- > > > > > drivers/gpu/drm/i915/display/intel_psr.h | 10 +- > > > > > drivers/gpu/drm/i915/display/intel_sprite.c | 3 + > > > > > 4 files changed, 134 insertions(+), 9 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > > index 5a9d933e425a..96bc515497c1 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > @@ -11812,6 +11812,9 @@ static void i9xx_update_cursor(struct > > > > > intel_plane *plane, > > > > > if (INTEL_GEN(dev_priv) >= 9) > > > > > skl_write_cursor_wm(plane, crtc_state); > > > > > > > > > > + if (!needs_modeset(crtc_state)) > > > > > + intel_psr2_program_plane_sel_fetch(plane, > > > > > crtc_state, > > > > > plane_state, 0); > > > > > + > > > > > if (plane->cursor.base != base || > > > > > plane->cursor.size != fbc_ctl || > > > > > plane->cursor.cntl != cntl) { > > > > > @@ -12823,8 +12826,11 @@ static int > > > > > intel_crtc_atomic_check(struct > > > > > intel_atomic_state *state, > > > > > > > > > > } > > > > > > > > > > - if (!mode_changed) > > > > > - intel_psr2_sel_fetch_update(state, crtc); > > > > > + if (!mode_changed) { > > > > > + ret = intel_psr2_sel_fetch_update(state, crtc); > > > > > + if (ret) > > > > > + return ret; > > > > > + } > > > > > > > > > > return 0; > > > > > } > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > index 02f74b0ddec1..deb0523f9f29 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > @@ -1166,6 +1166,41 @@ static void > > > > > psr_force_hw_tracking_exit(struct > > > > > drm_i915_private *dev_priv) > > > > > intel_psr_exit(dev_priv); > > > > > } > > > > > > > > > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane > > > > > *plane, > > > > > + const struct > > > > > intel_crtc_state > > > > > *crtc_state, > > > > > + const struct > > > > > intel_plane_state > > > > > *plane_state, > > > > > + int color_plane) > > > > > +{ > > > > > + struct drm_i915_private *dev_priv = to_i915(plane- > > > > > > base.dev); > > > > > > > > > > + enum pipe pipe = plane->pipe; > > > > > + u32 val; > > > > > + > > > > > + if (!crtc_state->enable_psr2_sel_fetch) > > > > > + return; > > > > > + > > > > > + /* > > > > > + * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 > > > > > for > > > > > gen11+, so > > > > > + * plane_state->ctl is the right value > > > > > + */ > > > > > + val = plane_state ? plane_state->ctl : 0; > > > > > > > > IMHO, skl_plane_ctl() might set other ctl bits, it would be > > > > better to > > > > have separated ctl bit value for "selective fetch ctl". > > > > > > Like said all other bits are spares so can be set without issues > > > but okay will a "plane_state->ctl & PLANE_SEL_FETCH_CTL_ENABLE". > > > > Please take a look to the answer of your other comment bellow, with > > the change above can I have your rv-b? > > The purpose and composition of bits of Register_SEL_FETCH_PLANE_CTL is > different from Register_PLANE_CTL. > And the Spares bits of Register_SEL_FETCH_PLANE_CTL might be used for > other purpose. > (And we current don't know the side effect of setting of Spares bit of > Register_SEL_FETCH_PLANE_CTL ) > therefor I recommend "Read and Modify" to SEL_FETCH_PLANE_CTL. Ah just remembered why I was using doing this, for cursor the bits that needs to be set is the first 5 bits. Anyways masking out other bits in v4. > > > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, > > > > > plane- > > > > > > id), val); > > > > > > > > > > + if (!val || plane->id == PLANE_CURSOR) > > > > > + return; > > > > > + > > > > > + val = plane_state->uapi.dst.y1 << 16 | plane_state- > > > > > > uapi.dst.x1; > > > > > > > > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, > > > > > plane- > > > > > > id), val); > > > > > > > > > > + > > > > > + val = plane_state->color_plane[color_plane].y << 16; > > > > > + val |= plane_state->color_plane[color_plane].x; > > > > > + intel_de_write_fw(dev_priv, > > > > > PLANE_SEL_FETCH_OFFSET(pipe, plane- > > > > > > id), > > > > > > > > > > + val); > > > > > + > > > > > + /* Sizes are 0 based */ > > > > > + val = ((drm_rect_height(&plane_state->uapi.src) >> 16) > > > > > - 1) << > > > > > 16; > > > > > + val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - > > > > > 1; > > > > > + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, > > > > > plane- > > > > > > id), val); > > > > > > > > > > +} > > > > > + > > > > > void intel_psr2_program_trans_man_trk_ctl(const struct > > > > > intel_crtc_state *crtc_state) > > > > > { > > > > > struct intel_crtc *crtc = to_intel_crtc(crtc_state- > > > > > > uapi.crtc); > > > > > > > > > > @@ -1180,16 +1215,91 @@ void > > > > > intel_psr2_program_trans_man_trk_ctl(const struct > > > > > intel_crtc_state > > > > > *crtc_st > > > > > crtc_state->psr2_man_track_ctl); > > > > > } > > > > > > > > > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state > > > > > *state, > > > > > - struct intel_crtc *crtc) > > > > > +static void psr2_man_trk_ctl_calc(struct intel_crtc_state > > > > > *crtc_state, > > > > > + struct drm_rect *clip, bool > > > > > full_update) > > > > > +{ > > > > > + u32 val = PSR2_MAN_TRK_CTL_ENABLE; > > > > > + > > > > > + if (full_update) { > > > > > + val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME; > > > > > + goto exit; > > > > > + } > > > > > + > > > > > + if (clip->y1 == -1) > > > > > + goto exit; > > > > > + > > > > > + val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE; > > > > > + val |= PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / > > > > > 4 + 1); > > > > > > > > I think the start address block would be like this too. > > > > PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(DIV_ROUND_UP(clip->y1 , > > > > 4)); > > > > > > This would not match for numbers that divide by 4. > > > y pos 0 = region 1y pos 1 = region 1 > > > y pos 4 = region 2 > > > > > yes. you are right. > > > > > + val |= > > > > > PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(DIV_ROUND_UP(clip- > > > > > > y2, 4) + 1); > > > > > > > > > > +exit: > > > > > + crtc_state->psr2_man_track_ctl = val; > > > > > +} > > > > > + > > > > > +static void clip_area_update(struct drm_rect > > > > > *overlap_damage_area, > > > > > + struct drm_rect *damage_area) > > > > > +{ > > > > > + if (overlap_damage_area->y1 == -1) { > > > > > + overlap_damage_area->y1 = damage_area->y1; > > > > > + overlap_damage_area->y2 = damage_area->y2; > > > > > + return; > > > > > + } > > > > > + > > > > > + if (damage_area->y1 < overlap_damage_area->y1) > > > > > + overlap_damage_area->y1 = damage_area->y1; > > > > > + > > > > > + if (damage_area->y2 > overlap_damage_area->y2) > > > > > + overlap_damage_area->y2 = damage_area->y2; > > > > > +} > > > > > + > > > > > +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_clip = { .y1 = -1 }; > > > > > + struct intel_plane *plane; > > > > > + bool full_update = false; > > > > > + int i, ret; > > > > > > > > > > if (!crtc_state->enable_psr2_sel_fetch) > > > > > - return; > > > > > + return 0; > > > > > + > > > > > + ret = drm_atomic_add_affected_planes(&state->base, > > > > > &crtc- > > > > > > base); > > > > > > > > > > + if (ret) > > > > > + return ret; > > > > > + > > > > > + for_each_oldnew_intel_plane_in_state(state, plane, > > > > > old_plane_state, > > > > > + new_plane_state, > > > > > i) { > > > > > + struct drm_rect temp; > > > > > + > > > > > + if (new_plane_state->uapi.crtc != crtc_state- > > > > > > uapi.crtc) > > > > > > > > > > + continue; > > > > > > > > > > - crtc_state->psr2_man_track_ctl = > > > > > PSR2_MAN_TRK_CTL_ENABLE | > > > > > - PSR2_MAN_TRK_CTL_SF_SI > > > > > NGLE_FUL > > > > > L_FRAME; > > > > > + /* > > > > > + * TODO: Not clear how to handle planes with > > > > > negative > > > > > position, > > > > > + * also planes are not updated if they have a > > > > > negative > > > > > X > > > > > + * position so for now doing a full update in > > > > > this > > > > > cases > > > > > + */ > > > > > + if (new_plane_state->uapi.dst.y1 < 0 || > > > > > + new_plane_state->uapi.dst.x1 < 0) { > > > > > + full_update = true; > > > > > + break; > > > > > + } > > > > > + > > > > > + if (!new_plane_state->uapi.visible) > > > > > + continue; > > > > > + > > > > > + /* > > > > > + * For now doing a selective fetch in the whole > > > > > plane > > > > > area, > > > > > + * optimizations will come in the future. > > > > > + */ > > > > > + temp.y1 = new_plane_state->uapi.dst.y1; > > > > > + temp.y2 = new_plane_state->uapi.dst.y2; > > > > > + clip_area_update(&pipe_clip, &temp); > > > > > + } > > > > > + > > > > > + psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, > > > > > full_update); > > > > > + return 0; > > > > > } > > > > > > > > > > /** > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h > > > > > b/drivers/gpu/drm/i915/display/intel_psr.h > > > > > index 6a83c8e682e6..3eca9dcec3c0 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.h > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h > > > > > @@ -15,6 +15,8 @@ struct intel_crtc_state; > > > > > struct intel_dp; > > > > > struct intel_crtc; > > > > > struct intel_atomic_state; > > > > > +struct intel_plane_state; > > > > > +struct intel_plane; > > > > > > > > > > #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv- > > > > > > psr.sink_support) > > > > > > > > > > void intel_psr_init_dpcd(struct intel_dp *intel_dp); > > > > > @@ -45,8 +47,12 @@ void intel_psr_atomic_check(struct > > > > > drm_connector > > > > > *connector, > > > > > struct drm_connector_state > > > > > *old_state, > > > > > struct drm_connector_state > > > > > *new_state); > > > > > void intel_psr_set_force_mode_changed(struct intel_dp > > > > > *intel_dp); > > > > > -void intel_psr2_sel_fetch_update(struct intel_atomic_state > > > > > *state, > > > > > - struct intel_crtc *crtc); > > > > > +int intel_psr2_sel_fetch_update(struct intel_atomic_state > > > > > *state, > > > > > + struct intel_crtc *crtc); > > > > > void intel_psr2_program_trans_man_trk_ctl(const struct > > > > > intel_crtc_state *crtc_state); > > > > > +void intel_psr2_program_plane_sel_fetch(struct intel_plane > > > > > *plane, > > > > > + const struct > > > > > intel_crtc_state > > > > > *crtc_state, > > > > > + const struct > > > > > intel_plane_state > > > > > *plane_state, > > > > > + int color_plane); > > > > > > > > > > #endif /* __INTEL_PSR_H__ */ > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c > > > > > b/drivers/gpu/drm/i915/display/intel_sprite.c > > > > > index 63040cb0d4e1..f12425b90607 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c > > > > > @@ -690,6 +690,9 @@ skl_program_plane(struct intel_plane > > > > > *plane, > > > > > intel_de_write_fw(dev_priv, > > > > > PLANE_AUX_OFFSET(pipe, > > > > > plane_id), > > > > > (plane_state- > > > > > > color_plane[1].y << 16) > > > > > > plane_state->color_plane[1].x); > > > > > > > > > > > > > > > + if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi)) > > > > > + intel_psr2_program_plane_sel_fetch(plane, > > > > > crtc_state, > > > > > plane_state, color_plane); > > > > > + > > > > > /* > > > > > * The control register self-arms if the plane was > > > > > previously > > > > > * disabled. Try to make the plane enable atomic by > > > > > writing
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 5a9d933e425a..96bc515497c1 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -11812,6 +11812,9 @@ static void i9xx_update_cursor(struct intel_plane *plane, if (INTEL_GEN(dev_priv) >= 9) skl_write_cursor_wm(plane, crtc_state); + if (!needs_modeset(crtc_state)) + intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, 0); + if (plane->cursor.base != base || plane->cursor.size != fbc_ctl || plane->cursor.cntl != cntl) { @@ -12823,8 +12826,11 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state, } - if (!mode_changed) - intel_psr2_sel_fetch_update(state, crtc); + if (!mode_changed) { + ret = intel_psr2_sel_fetch_update(state, crtc); + if (ret) + return ret; + } return 0; } diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 02f74b0ddec1..deb0523f9f29 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1166,6 +1166,41 @@ static void psr_force_hw_tracking_exit(struct drm_i915_private *dev_priv) intel_psr_exit(dev_priv); } +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state, + int color_plane) +{ + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); + enum pipe pipe = plane->pipe; + u32 val; + + if (!crtc_state->enable_psr2_sel_fetch) + return; + + /* + * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for gen11+, so + * plane_state->ctl is the right value + */ + val = plane_state ? plane_state->ctl : 0; + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id), val); + if (!val || plane->id == PLANE_CURSOR) + return; + + val = plane_state->uapi.dst.y1 << 16 | plane_state->uapi.dst.x1; + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane->id), val); + + val = plane_state->color_plane[color_plane].y << 16; + val |= plane_state->color_plane[color_plane].x; + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_OFFSET(pipe, plane->id), + val); + + /* Sizes are 0 based */ + val = ((drm_rect_height(&plane_state->uapi.src) >> 16) - 1) << 16; + val |= (drm_rect_width(&plane_state->uapi.src) >> 16) - 1; + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_SIZE(pipe, plane->id), val); +} + void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state) { struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); @@ -1180,16 +1215,91 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st crtc_state->psr2_man_track_ctl); } -void intel_psr2_sel_fetch_update(struct intel_atomic_state *state, - struct intel_crtc *crtc) +static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state, + struct drm_rect *clip, bool full_update) +{ + u32 val = PSR2_MAN_TRK_CTL_ENABLE; + + if (full_update) { + val |= PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME; + goto exit; + } + + if (clip->y1 == -1) + goto exit; + + 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); +exit: + crtc_state->psr2_man_track_ctl = val; +} + +static void clip_area_update(struct drm_rect *overlap_damage_area, + struct drm_rect *damage_area) +{ + if (overlap_damage_area->y1 == -1) { + overlap_damage_area->y1 = damage_area->y1; + overlap_damage_area->y2 = damage_area->y2; + return; + } + + if (damage_area->y1 < overlap_damage_area->y1) + overlap_damage_area->y1 = damage_area->y1; + + if (damage_area->y2 > overlap_damage_area->y2) + overlap_damage_area->y2 = damage_area->y2; +} + +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_clip = { .y1 = -1 }; + struct intel_plane *plane; + bool full_update = false; + int i, ret; if (!crtc_state->enable_psr2_sel_fetch) - return; + return 0; + + ret = drm_atomic_add_affected_planes(&state->base, &crtc->base); + if (ret) + return ret; + + for_each_oldnew_intel_plane_in_state(state, plane, old_plane_state, + new_plane_state, i) { + struct drm_rect temp; + + if (new_plane_state->uapi.crtc != crtc_state->uapi.crtc) + continue; - crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE | - PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME; + /* + * TODO: Not clear how to handle planes with negative position, + * also planes are not updated if they have a negative X + * position so for now doing a full update in this cases + */ + if (new_plane_state->uapi.dst.y1 < 0 || + new_plane_state->uapi.dst.x1 < 0) { + full_update = true; + break; + } + + if (!new_plane_state->uapi.visible) + continue; + + /* + * For now doing a selective fetch in the whole plane area, + * optimizations will come in the future. + */ + temp.y1 = new_plane_state->uapi.dst.y1; + temp.y2 = new_plane_state->uapi.dst.y2; + clip_area_update(&pipe_clip, &temp); + } + + psr2_man_trk_ctl_calc(crtc_state, &pipe_clip, full_update); + return 0; } /** diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h index 6a83c8e682e6..3eca9dcec3c0 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.h +++ b/drivers/gpu/drm/i915/display/intel_psr.h @@ -15,6 +15,8 @@ struct intel_crtc_state; struct intel_dp; struct intel_crtc; struct intel_atomic_state; +struct intel_plane_state; +struct intel_plane; #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support) void intel_psr_init_dpcd(struct intel_dp *intel_dp); @@ -45,8 +47,12 @@ void intel_psr_atomic_check(struct drm_connector *connector, struct drm_connector_state *old_state, struct drm_connector_state *new_state); void intel_psr_set_force_mode_changed(struct intel_dp *intel_dp); -void intel_psr2_sel_fetch_update(struct intel_atomic_state *state, - struct intel_crtc *crtc); +int intel_psr2_sel_fetch_update(struct intel_atomic_state *state, + struct intel_crtc *crtc); void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state); +void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state, + int color_plane); #endif /* __INTEL_PSR_H__ */ diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c index 63040cb0d4e1..f12425b90607 100644 --- a/drivers/gpu/drm/i915/display/intel_sprite.c +++ b/drivers/gpu/drm/i915/display/intel_sprite.c @@ -690,6 +690,9 @@ skl_program_plane(struct intel_plane *plane, intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe, plane_id), (plane_state->color_plane[1].y << 16) | plane_state->color_plane[1].x); + if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi)) + intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane); + /* * The control register self-arms if the plane was previously * disabled. Try to make the plane enable atomic by writing
Another step towards PSR2 selective fetch, here programming plane selective fetch registers and MAN_TRK_CTL enabling selective fetch but for now it is fetching the whole area of the planes. The damaged area calculation will come as next and final step. v2: - removed warn on when no plane is visible in state - removed calculations using plane damaged area in intel_psr2_program_plane_sel_fetch() v3: - do not shift 16 positions the plane dst coordinates, only src is shifted BSpec: 55229 Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 10 +- drivers/gpu/drm/i915/display/intel_psr.c | 120 ++++++++++++++++++- drivers/gpu/drm/i915/display/intel_psr.h | 10 +- drivers/gpu/drm/i915/display/intel_sprite.c | 3 + 4 files changed, 134 insertions(+), 9 deletions(-)