diff mbox

[v2,1/9] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function

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

Commit Message

Souza, Jose April 18, 2018, 10:43 p.m. UTC
It was reading some random register in VLV and CHV.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---

No changes from v1.

 drivers/gpu/drm/i915/intel_psr.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Dhinakaran Pandiyan April 26, 2018, 12:41 a.m. UTC | #1
On Wednesday, April 18, 2018 3:43:03 PM PDT José Roberto de Souza wrote:
> It was reading some random register in VLV and CHV.
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> 
> No changes from v1.
> 
>  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 69a5b276f4d8..659180656f5b 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -424,6 +424,11 @@ static void hsw_psr_activate(struct intel_dp *intel_dp)
> struct drm_device *dev = dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> 
> +	if (dev_priv->psr.psr2_enabled)
> +		WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
> +	else
> +		WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> +

Why not move the checks under hsw_activate_psr2 and hsw_activate_psr1?  This 
is just duplicating another  psr2_enabled branch that is right below.

>  	/* On HSW+ after we enable PSR on source it will activate it
>  	 * as soon as it match configure idle_frame count. So
>  	 * we just actually enable it here on activation time.
> @@ -549,10 +554,6 @@ static void intel_psr_activate(struct intel_dp
> *intel_dp) struct drm_device *dev = intel_dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> 
> -	if (dev_priv->psr.psr2_enabled)
> -		WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
> -	else
> -		WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
>  	WARN_ON(dev_priv->psr.active);
>  	lockdep_assert_held(&dev_priv->psr.lock);
Souza, Jose April 30, 2018, 10:54 p.m. UTC | #2
On Wed, 2018-04-25 at 17:41 -0700, Dhinakaran Pandiyan wrote:
> On Wednesday, April 18, 2018 3:43:03 PM PDT José Roberto de Souza

> wrote:

> > It was reading some random register in VLV and CHV.

> > 

> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> > ---

> > 

> > No changes from v1.

> > 

> >  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 69a5b276f4d8..659180656f5b

> > 100644

> > --- a/drivers/gpu/drm/i915/intel_psr.c

> > +++ b/drivers/gpu/drm/i915/intel_psr.c

> > @@ -424,6 +424,11 @@ static void hsw_psr_activate(struct intel_dp

> > *intel_dp)

> > struct drm_device *dev = dig_port->base.base.dev;

> >  	struct drm_i915_private *dev_priv = to_i915(dev);

> > 

> > +	if (dev_priv->psr.psr2_enabled)

> > +		WARN_ON(I915_READ(EDP_PSR2_CTL) &

> > EDP_PSR2_ENABLE);

> > +	else

> > +		WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);

> > +

> 

> Why not move the checks under hsw_activate_psr2 and

> hsw_activate_psr1?  This 

> is just duplicating another  psr2_enabled branch that is right below.


Okay, done.

> 

> >  	/* On HSW+ after we enable PSR on source it will activate

> > it

> >  	 * as soon as it match configure idle_frame count. So

> >  	 * we just actually enable it here on activation time.

> > @@ -549,10 +554,6 @@ static void intel_psr_activate(struct intel_dp

> > *intel_dp) struct drm_device *dev = intel_dig_port->base.base.dev;

> >  	struct drm_i915_private *dev_priv = to_i915(dev);

> > 

> > -	if (dev_priv->psr.psr2_enabled)

> > -		WARN_ON(I915_READ(EDP_PSR2_CTL) &

> > EDP_PSR2_ENABLE);

> > -	else

> > -		WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);

> >  	WARN_ON(dev_priv->psr.active);

> >  	lockdep_assert_held(&dev_priv->psr.lock);

> 

>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 69a5b276f4d8..659180656f5b 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -424,6 +424,11 @@  static void hsw_psr_activate(struct intel_dp *intel_dp)
 	struct drm_device *dev = dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
+	if (dev_priv->psr.psr2_enabled)
+		WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
+	else
+		WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
+
 	/* On HSW+ after we enable PSR on source it will activate it
 	 * as soon as it match configure idle_frame count. So
 	 * we just actually enable it here on activation time.
@@ -549,10 +554,6 @@  static void intel_psr_activate(struct intel_dp *intel_dp)
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
-	if (dev_priv->psr.psr2_enabled)
-		WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
-	else
-		WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
 	WARN_ON(dev_priv->psr.active);
 	lockdep_assert_held(&dev_priv->psr.lock);