Re: [PATCH 6/6] drm/i915/psr: Enable aux frame sync in source

Pandiyan, Dhinakaran
2018-03-16T01:34:41Z

On Wed, 2018-03-14 at 15:36 -0700, José Roberto de Souza wrote:
> Even with GTC not enabled lets send the aux frame sync.

If this was never enabled on the source side, why do we place a
requirement on the sink to support aux frame sync?

> Hardware is going to send dummy values but this way we can get rid of
> this workarround in PSR exit: 'drm/i915/psr: disable aux_frame_sync
> on psr2 exit'.
> Also moving the line disabling aux frame sync in sink to after report
> that PSR2 has exit to avoid.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_psr.c | 15 ++++++---------
> 2 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e9fc1722c0fb..5a2364656aa5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4132,6 +4132,7 @@ enum {
> #define EDP_PSR2_CTL _MMIO(0x6f900)
> #define EDP_PSR2_ENABLE (1<<31)
> #define EDP_SU_TRACK_ENABLE (1<<30)
> +#define EDP_AUX_FRAME_SYNC_ENABLE (1<<27)
> #define EDP_Y_COORDINATE_VALID (1<<26)
> #define EDP_Y_COORDINATE_ENABLE (1<<25)
> #define EDP_MAX_SU_DISABLE_TIME(t) ((t)<<20)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index d622e37894d4..7aab66b5bc91 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -418,6 +418,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
> * good enough. */
> val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
> val |= EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE;
> + val |= EDP_AUX_FRAME_SYNC_ENABLE;
>
> if (drm_dp_dpcd_readb(&intel_dp->aux,
> DP_SYNCHRONIZATION_LATENCY_IN_SINK,
> @@ -715,11 +716,6 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> i915_reg_t psr_status;
> u32 psr_status_mask;
>
> - if (dev_priv->psr.psr2_enabled)
> - drm_dp_dpcd_writeb(&intel_dp->aux,
> - DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
> - 0);
> -
> if (dev_priv->psr.psr2_enabled) {
> psr_status = EDP_PSR2_STATUS;
> psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> @@ -742,6 +738,11 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> 2000))
> DRM_ERROR("Timed out waiting for PSR Idle State\n");
>
> + if (dev_priv->psr.psr2_enabled)
> + drm_dp_dpcd_writeb(&intel_dp->aux,
> + DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
> + 0);
> +
> dev_priv->psr.active = false;
> } else {
> if (dev_priv->psr.psr2_enabled)
> @@ -863,10 +864,6 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
> return;
>
> if (HAS_DDI(dev_priv)) {
> - if (dev_priv->psr.psr2_enabled)
> - drm_dp_dpcd_writeb(&intel_dp->aux,
> - DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
> - 0);
> if (dev_priv->psr.psr2_enabled) {
> val = I915_READ(EDP_PSR2_CTL);
> WARN_ON(!(val & EDP_PSR2_ENABLE));