Re: [PATCH v3 1/2] drm/i915: Initial implementation of PSR2 selective fetch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux