diff mbox series

[7/8] drm/i915/psr: Don't tell sink that main link will be active in PSR2

Message ID 20180920204327.3513-7-jose.souza@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/8] drm/i915/psr: Share PSR and PSR2 exit mask | expand

Commit Message

Souza, Jose Sept. 20, 2018, 8:43 p.m. UTC 
For PSR2 we don't have the option to keep main link enabled while
PSR2 is active, so don't configure sink DPCD with a wrong value.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Dhinakaran Pandiyan Sept. 25, 2018, 6:02 a.m. UTC | #1 
On Thu, 2018-09-20 at 13:43 -0700, José Roberto de Souza wrote:
> For PSR2 we don't have the option to keep main link enabled while
> PSR2 is active, so don't configure sink DPCD with a wrong value.
Is this what the DP spec says or an Intel HW restriction?

-DK

> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>s
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index cf9d6e965697..60cf6fd251d0 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -344,12 +344,13 @@ static void intel_psr_enable_sink(struct
> intel_dp *intel_dp)
>  		drm_dp_dpcd_writeb(&intel_dp->aux,
> DP_RECEIVER_ALPM_CONFIG,
>  				   DP_ALPM_ENABLE);
>  		dpcd_val |= DP_PSR_ENABLE_PSR2 |
> DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
> +	} else {
> +		if (dev_priv->psr.link_standby)
> +			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> +		if (INTEL_GEN(dev_priv) >= 8)
> +			dpcd_val |= DP_PSR_CRC_VERIFICATION;
>  	}
>  
> -	if (dev_priv->psr.link_standby)
> -		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> -	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
> -		dpcd_val |= DP_PSR_CRC_VERIFICATION;
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
>  
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0);
Souza, Jose Sept. 26, 2018, 5:46 p.m. UTC | #2 
On Mon, 2018-09-24 at 23:02 -0700, dhinakaran.pandiyan@gmail.com wrote:
> On Thu, 2018-09-20 at 13:43 -0700, José Roberto de Souza wrote:
> > For PSR2 we don't have the option to keep main link enabled while
> > PSR2 is active, so don't configure sink DPCD with a wrong value.
> 
> Is this what the DP spec says or an Intel HW restriction?

This is Intel HW restriction as there is no register in PSR2 CTL to
keep link in standby, DP spec states that link can be kept on or off
while PSR2 is active, there is just a new timming restriction to turn
link off.

> 
> -DK
> 
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>s
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index cf9d6e965697..60cf6fd251d0 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -344,12 +344,13 @@ static void intel_psr_enable_sink(struct
> > intel_dp *intel_dp)
> >  		drm_dp_dpcd_writeb(&intel_dp->aux,
> > DP_RECEIVER_ALPM_CONFIG,
> >  				   DP_ALPM_ENABLE);
> >  		dpcd_val |= DP_PSR_ENABLE_PSR2 |
> > DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
> > +	} else {
> > +		if (dev_priv->psr.link_standby)
> > +			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> > +		if (INTEL_GEN(dev_priv) >= 8)
> > +			dpcd_val |= DP_PSR_CRC_VERIFICATION;
> >  	}
> >  
> > -	if (dev_priv->psr.link_standby)
> > -		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> > -	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
> > -		dpcd_val |= DP_PSR_CRC_VERIFICATION;
> >  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
> >  
> >  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D0);
Souza, Jose Sept. 26, 2018, 5:49 p.m. UTC | #3 
On Wed, 2018-09-26 at 17:46 +0000, Souza, Jose wrote:
> On Mon, 2018-09-24 at 23:02 -0700, dhinakaran.pandiyan@gmail.com
> wrote:
> > On Thu, 2018-09-20 at 13:43 -0700, José Roberto de Souza wrote:
> > > For PSR2 we don't have the option to keep main link enabled while
> > > PSR2 is active, so don't configure sink DPCD with a wrong value.
> > 
> > Is this what the DP spec says or an Intel HW restriction?
> 
> This is Intel HW restriction as there is no register in PSR2 CTL to
> keep link in standby, DP spec states that link can be kept on or off
> while PSR2 is active, there is just a new timming restriction to turn
> link off.

I will update the commit message to make it clear:

For PSR2 there is no register to tell HW to keep main link enabled
while PSR2 is active, so don't configure sink DPCD with a
wrong value.


> 
> > 
> > -DK
> > 
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>s
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 9 +++++----
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index cf9d6e965697..60cf6fd251d0 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -344,12 +344,13 @@ static void intel_psr_enable_sink(struct
> > > intel_dp *intel_dp)
> > >  		drm_dp_dpcd_writeb(&intel_dp->aux,
> > > DP_RECEIVER_ALPM_CONFIG,
> > >  				   DP_ALPM_ENABLE);
> > >  		dpcd_val |= DP_PSR_ENABLE_PSR2 |
> > > DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
> > > +	} else {
> > > +		if (dev_priv->psr.link_standby)
> > > +			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> > > +		if (INTEL_GEN(dev_priv) >= 8)
> > > +			dpcd_val |= DP_PSR_CRC_VERIFICATION;
> > >  	}
> > >  
> > > -	if (dev_priv->psr.link_standby)
> > > -		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> > > -	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
> > > -		dpcd_val |= DP_PSR_CRC_VERIFICATION;
> > >  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
> > >  
> > >  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > > DP_SET_POWER_D0);
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index cf9d6e965697..60cf6fd251d0 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -344,12 +344,13 @@  static void intel_psr_enable_sink(struct intel_dp *intel_dp)
 		drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG,
 				   DP_ALPM_ENABLE);
 		dpcd_val |= DP_PSR_ENABLE_PSR2 | DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
+	} else {
+		if (dev_priv->psr.link_standby)
+			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
+		if (INTEL_GEN(dev_priv) >= 8)
+			dpcd_val |= DP_PSR_CRC_VERIFICATION;
 	}
 
-	if (dev_priv->psr.link_standby)
-		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
-	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
-		dpcd_val |= DP_PSR_CRC_VERIFICATION;
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
 
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);