On Mon, Aug 31, 2020 at 06:09:23PM -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.
>
> BSpec: 55229
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 10 +-
> .../drm/i915/display/intel_display_types.h | 2 +
> drivers/gpu/drm/i915/display/intel_psr.c | 129 +++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_psr.h | 10 +-
> drivers/gpu/drm/i915/display/intel_sprite.c | 3 +
> 5 files changed, 145 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index c8b1dd1a9e46..865486e89915 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -11799,6 +11799,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) {
> @@ -12810,8 +12813,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_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 9349b15afff6..2138bb0f1587 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -586,6 +586,8 @@ struct intel_plane_state {
> u32 planar_slave;
>
> struct drm_intel_sprite_colorkey ckey;
> +
> + struct drm_rect psr2_sel_fetch_area;
> };
>
> struct intel_initial_plane_config {
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 6698d0209879..b60ea133a527 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1173,6 +1173,46 @@ 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);
> + const struct drm_rect *clip;
> + enum pipe pipe = plane->pipe;
> + u32 val;
> +
> + if (!plane_state || !dev_priv->psr.psr2_sel_fetch_enabled)
> + return;
> +
> + /*
> + * skl_plane_ctl_crtc()/i9xx_cursor_ctl_crtc() return 0 for gen11+, so
> + * plane_state->ctl is the right value
> + */
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id), plane_state->ctl);
> + if (!plane_state->ctl || plane->id == PLANE_CURSOR)
> + return;
> +
> + clip = &plane_state->psr2_sel_fetch_area;
> +
> + val = (clip->y1 + plane_state->uapi.crtc_y) << 16;
crtc_x/y are the raw values usrspace gave us. That is definitely not
what we should be looking at.
As the first step I think these functions should just program the
registers with *exactly* the same values as we program into the
normal plane register. That gets us to the point where we're
actually programming something into the register without having to
complicate things with calculating the selective fetch area.
> + val |= plane_state->uapi.crtc_x;
> + intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_POS(pipe, plane->id),
> + val);
> +
> + val = (clip->y1 + 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(clip) - 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);
> @@ -1187,17 +1227,96 @@ 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;
> + }
> +
> + drm_WARN_ON_ONCE(crtc_state->uapi.crtc->dev, clip->y1 == -1);
> +
> + 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 drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + 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 (!dev_priv->psr.psr2_sel_fetch_enabled)
> - 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 *plane_sel_fetch_area, temp;
>
> - crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
> - PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> + if (new_plane_state->uapi.crtc != crtc_state->uapi.crtc)
> + continue;
> +
> + /*
> + * 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.crtc_y < 0 ||
> + new_plane_state->uapi.crtc_x < 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.
> + */
> + plane_sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> + plane_sel_fetch_area->y1 = new_plane_state->uapi.src.y1 >> 16;
> + plane_sel_fetch_area->y2 = new_plane_state->uapi.src.y2 >> 16;
> +
> + temp = *plane_sel_fetch_area;
> + temp.y1 += new_plane_state->uapi.crtc_y;
> + temp.y2 += new_plane_state->uapi.crtc_y;
> + 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 1797a06cfd60..24ee9b08ec4a 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
> --
> 2.28.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
