On Thu, Jun 25, 2020 at 06:01:50PM -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.
>
> 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 | 3 +
> .../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, 103 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index b66008b80589..eb3a4f317b01 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15114,6 +15114,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)
> @@ -15155,6 +15157,7 @@ static void intel_update_crtc(struct intel_atomic_state *state,
> intel_color_load_luts(new_crtc_state);
>
> intel_pre_plane_update(state, crtc);
> + intel_psr2_sel_fetch_update(state, crtc);
You seem to be modifying the crtc state here. No good. Ideally the state
should be const for the whole programming step.
>
> if (new_crtc_state->update_pipe)
> intel_encoders_update_pipe(state, crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index d1cb48b3f462..4c9591f7ed92 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 4b0aaa3081c9..44c98ae3964e 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 611cb8d74811..078987a878b0 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_FULL_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 9aad3ec979bd..038bd57e429e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -503,6 +503,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;
> @@ -1651,6 +1652,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 a7b61e6ec508..da686f8bcb09 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 helpfull to detect if PSR issues are related to bad values set in "
> " VBT. (0=use VBT paramters, 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) \
> --
> 2.27.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
