diff mbox

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

Message ID 20180314223617.20122-6-jose.souza@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Souza, Jose March 14, 2018, 10:36 p.m. UTC 
Even with GTC not enabled lets send the 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(-)

Comments

Rodrigo Vivi March 16, 2018, 12:31 a.m. UTC | #1 
On Wed, Mar 14, 2018 at 03:36:17PM -0700, José Roberto de Souza wrote:
> Even with GTC not enabled lets send the 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)

This shows reserved here for me. I believe we should just drop this patch.

>  #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));
> -- 
> 2.16.2
>
Dhinakaran Pandiyan March 16, 2018, 1:34 a.m. UTC | #2 
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));
diff mbox

Patch

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));