On Mon, 2020-07-06 at 16:43 -0700, José Roberto de Souza wrote:
> All GEN12 platforms supports PSR2 selective fetch but not all GEN12
> platforms supports PSR2 hardware tracking(aka RKL).
>
> This feature consists in software programming registers with the
> damaged area of each plane this way hardware will only fetch from
> memory those areas and sent the PSR2 selective update blocks to
> panel,
> saving even more power.
>
> But as initial step it is only enabling the full frame fetch at
> every flip, the actual selective fetch part will come in a future
> patch.
>
> Also this is only handling the page flip side, it is still completely
> missing frontbuffer modifications, that is why the
> enable_psr2_sel_fetch parameter was added.
>
> v3:
> - calling intel_psr2_sel_fetch_update() during the atomic check phase
> (Ville)
>
> BSpec: 55229
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 5 +
> .../drm/i915/display/intel_display_debugfs.c | 3 +
> .../drm/i915/display/intel_display_types.h | 3 +
> drivers/gpu/drm/i915/display/intel_psr.c | 95 ++++++++++++++++-
> --
> drivers/gpu/drm/i915/display/intel_psr.h | 5 +
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/i915_params.c | 5 +
> drivers/gpu/drm/i915/i915_params.h | 1 +
> 8 files changed, 105 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index dff7c17f3d2b..eb64e2ac773a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -12759,6 +12759,9 @@ static int intel_crtc_atomic_check(struct
> intel_atomic_state *state,
>
> }
>
> + if (!mode_changed)
> + intel_psr2_sel_fetch_update(state, crtc);
> +
> return 0;
> }
>
> @@ -15135,6 +15138,8 @@ static void commit_pipe_config(struct
> intel_atomic_state *state,
>
> if (new_crtc_state->update_pipe)
> intel_pipe_fastset(old_crtc_state,
> new_crtc_state);
> +
> + intel_psr2_program_trans_man_trk_ctl(new_crtc_state);
> }
>
> if (dev_priv->display.atomic_update_watermarks)
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 3644752cc5ec..15d963183bcf 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -417,6 +417,9 @@ static int i915_edp_psr_status(struct seq_file
> *m, void *data)
> su_blocks = su_blocks >>
> PSR2_SU_STATUS_SHIFT(frame);
> seq_printf(m, "%d\t%d\n", frame, su_blocks);
> }
> +
> + seq_printf(m, "PSR2 selective fetch: %s\n",
> + enableddisabled(psr-
> >psr2_sel_fetch_enabled));
> }
>
> unlock:
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index e8f809161c75..403a76214e71 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -931,6 +931,7 @@ struct intel_crtc_state {
>
> bool has_psr;
> bool has_psr2;
> + bool enable_psr2_sel_fetch;
> u32 dc3co_exitline;
>
> /*
> @@ -1073,6 +1074,8 @@ struct intel_crtc_state {
>
> /* For DSB related info */
> struct intel_dsb *dsb;
> +
> + u32 psr2_man_track_ctl;
> };
>
> enum intel_pipe_crc_source {
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index bf9e320c547d..d30a3560b794 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -553,6 +553,14 @@ static void hsw_activate_psr2(struct intel_dp
> *intel_dp)
> val |= EDP_PSR2_FAST_WAKE(7);
> }
>
> + if (dev_priv->psr.psr2_sel_fetch_enabled)
> + intel_de_write(dev_priv,
> + PSR2_MAN_TRK_CTL(dev_priv-
> >psr.transcoder),
> + PSR2_MAN_TRK_CTL_ENABLE);
> + else if (HAS_PSR2_SEL_FETCH(dev_priv))
> + intel_de_write(dev_priv,
> + PSR2_MAN_TRK_CTL(dev_priv-
> >psr.transcoder), 0);
> +
> /*
> * PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and BSpec
> is
> * recommending keep this bit unset while PSR2 is enabled.
> @@ -663,6 +671,38 @@ tgl_dc3co_exitline_compute_config(struct
> intel_dp *intel_dp,
> crtc_state->dc3co_exitline = crtc_vdisplay - exit_scanlines;
> }
>
> +static bool intel_psr2_sel_fetch_config_valid(struct intel_dp
> *intel_dp,
> + struct intel_crtc_state
> *crtc_state)
> +{
> + struct intel_atomic_state *state =
> to_intel_atomic_state(crtc_state->uapi.state);
> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> + struct intel_plane_state *plane_state;
> + struct intel_plane *plane;
> + int i;
> +
> + if (!dev_priv->params.enable_psr2_sel_fetch) {
> + drm_dbg_kms(&dev_priv->drm,
> + "PSR2 sel fetch not enabled, disabled by
> parameter\n");
> + return false;
> + }
> +
> + if (crtc_state->uapi.async_flip) {
> + drm_dbg_kms(&dev_priv->drm,
> + "PSR2 sel fetch not enabled, async flip
> enabled\n");
> + return false;
> + }
> +
> + for_each_new_intel_plane_in_state(state, plane, plane_state, i)
> {
> + if (plane_state->uapi.rotation != DRM_MODE_ROTATE_0) {
> + drm_dbg_kms(&dev_priv->drm,
> + "PSR2 sel fetch not enabled, plane
> rotated\n");
> + return false;
> + }
> + }
> +
> + return crtc_state->enable_psr2_sel_fetch = true;
> +}
> +
> static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> struct intel_crtc_state
> *crtc_state)
> {
> @@ -732,22 +772,17 @@ static bool intel_psr2_config_valid(struct
> intel_dp *intel_dp,
> return false;
> }
>
> - /*
> - * Some platforms lack PSR2 HW tracking and instead require
> manual
> - * tracking by software. In this case, the driver is required
> to track
> - * the areas that need updates and program hardware to send
> selective
> - * updates.
> - *
> - * So until the software tracking is implemented, PSR2 needs to
> be
> - * disabled for platforms without PSR2 HW tracking.
> - */
> - if (!HAS_PSR_HW_TRACKING(dev_priv)) {
> - drm_dbg_kms(&dev_priv->drm,
> - "No PSR2 HW tracking in the platform\n");
> - return false;
> + if (HAS_PSR2_SEL_FETCH(dev_priv)) {
> + if (!intel_psr2_sel_fetch_config_valid(intel_dp,
> crtc_state) &&
> + !HAS_PSR_HW_TRACKING(dev_priv)) {
> + drm_dbg_kms(&dev_priv->drm,
> + "PSR2 not enabled, selective fetch
> not valid and no HW tracking available\n");
> + return false;
> + }
> }
>
> - if (crtc_hdisplay > psr_max_h || crtc_vdisplay > psr_max_v) {
> + if (!crtc_state->enable_psr2_sel_fetch &&
> + (crtc_hdisplay > psr_max_h || crtc_vdisplay > psr_max_v)) {
> drm_dbg_kms(&dev_priv->drm,
> "PSR2 not enabled, resolution %dx%d > max
> supported %dx%d\n",
> crtc_hdisplay, crtc_vdisplay,
> @@ -898,6 +933,11 @@ static void intel_psr_enable_source(struct
> intel_dp *intel_dp,
> val |= EXITLINE_ENABLE;
> intel_de_write(dev_priv, EXITLINE(cpu_transcoder),
> val);
> }
> +
> + if (HAS_PSR_HW_TRACKING(dev_priv))
> + intel_de_rmw(dev_priv, CHICKEN_PAR1_1,
> IGNORE_PSR2_HW_TRACKING,
> + dev_priv->psr.psr2_sel_fetch_enabled ?
> + IGNORE_PSR2_HW_TRACKING : 0);
> }
>
> static void intel_psr_enable_locked(struct drm_i915_private
> *dev_priv,
> @@ -919,6 +959,7 @@ static void intel_psr_enable_locked(struct
> drm_i915_private *dev_priv,
> /* DC5/DC6 requires at least 6 idle frames */
> val = usecs_to_jiffies(intel_get_frame_time_us(crtc_state) *
> 6);
> dev_priv->psr.dc3co_exit_delay = val;
> + dev_priv->psr.psr2_sel_fetch_enabled = crtc_state-
> >enable_psr2_sel_fetch;
>
> /*
> * If a PSR error happened and the driver is reloaded, the
> EDP_PSR_IIR
> @@ -1115,6 +1156,32 @@ static void psr_force_hw_tracking_exit(struct
> drm_i915_private *dev_priv)
> intel_psr_exit(dev_priv);
> }
>
> +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);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + struct i915_psr *psr = &dev_priv->psr;
> +
> + if (!HAS_PSR2_SEL_FETCH(dev_priv) ||
> + !crtc_state->enable_psr2_sel_fetch)
> + return;
> +
> + intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(psr->transcoder),
> + crtc_state->psr2_man_track_ctl);
> +}
> +
> +void 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);
> +
> + if (!crtc_state->enable_psr2_sel_fetch)
> + return;
> +
> + crtc_state->psr2_man_track_ctl = PSR2_MAN_TRK_CTL_ENABLE |
> + PSR2_MAN_TRK_CTL_SF_SINGLE_FUL
> L_FRAME;
> +}
> +
> /**
> * intel_psr_update - Update PSR state
> * @intel_dp: Intel DP
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> b/drivers/gpu/drm/i915/display/intel_psr.h
> index b4515186d5f4..6a83c8e682e6 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -13,6 +13,8 @@ struct drm_connector_state;
> struct drm_i915_private;
> struct intel_crtc_state;
> struct intel_dp;
> +struct intel_crtc;
> +struct intel_atomic_state;
>
> #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv-
> >psr.sink_support)
> void intel_psr_init_dpcd(struct intel_dp *intel_dp);
> @@ -43,5 +45,8 @@ 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);
> +void intel_psr2_program_trans_man_trk_ctl(const struct
> intel_crtc_state *crtc_state);
>
> #endif /* __INTEL_PSR_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 2c2e88d49f3e..1f3fbec2f228 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -505,6 +505,7 @@ struct i915_psr {
> bool link_standby;
> bool colorimetry_support;
> bool psr2_enabled;
> + bool psr2_sel_fetch_enabled;
> u8 sink_sync_latency;
> ktime_t last_entry_attempt;
> ktime_t last_exit;
> @@ -1653,6 +1654,7 @@ IS_SUBPLATFORM(const struct drm_i915_private
> *i915,
> #define HAS_PSR(dev_priv) (INTEL_INFO(dev_priv)-
> >display.has_psr)
> #define HAS_PSR_HW_TRACKING(dev_priv) \
> (INTEL_INFO(dev_priv)->display.has_psr_hw_tracking)
> +#define HAS_PSR2_SEL_FETCH(dev_priv) (INTEL_GEN(dev_priv) >= 12)
> #define HAS_TRANSCODER(dev_priv, trans) ((INTEL_INFO(dev_priv)
> ->cpu_transcoder_mask & BIT(trans)) != 0)
>
> #define HAS_RC6(dev_priv) (INTEL_INFO(dev_priv)-
> >has_rc6)
> diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> index 8d8db9ff0a48..7f139ea4a90b 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -102,6 +102,11 @@ i915_param_named(psr_safest_params, bool, 0400,
> "is helpful to detect if PSR issues are related to bad values
> set in "
> " VBT. (0=use VBT parameters, 1=use safest parameters)");
>
> +i915_param_named_unsafe(enable_psr2_sel_fetch, bool, 0400,
> + "Enable PSR2 selective fetch "
> + "(0=disabled, 1=enabled) "
> + "Default: 0");
> +
> i915_param_named_unsafe(force_probe, charp, 0400,
> "Force probe the driver for specified devices. "
> "See CONFIG_DRM_I915_FORCE_PROBE for details.");
> diff --git a/drivers/gpu/drm/i915/i915_params.h
> b/drivers/gpu/drm/i915/i915_params.h
> index 53fb5ba8fbed..330c03e2b4f7 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -54,6 +54,7 @@ struct drm_printer;
> param(int, enable_fbc, -1, 0600) \
> param(int, enable_psr, -1, 0600) \
> param(bool, psr_safest_params, false, 0600) \
> + param(bool, enable_psr2_sel_fetch, false, 0600) \
> param(int, disable_power_well, -1, 0400) \
> param(int, enable_ips, 1, 0600) \
> param(int, invert_brightness, 0, 0600) \
As per the base implementation point of view, it looks good to me.
But while the mouse pointer moves on the screen, the mouse pointer
movement showed stuttering motion.
( gdm, weston and gnome-mutter on TGL).
When the PSR2 selective update is coming, we should fix this.
For now, it is not enabled by default (it can be enabled by boot
parameter (i915.enable_psr2_sel_fetch=1)),
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
