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

From: "Souza, Jose" <jose.souza@intel.com>
To: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"dhinakaran.pandiyan@gmail.com" <dhinakaran.pandiyan@gmail.com>
Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Subject: Re: [PATCH v2 1/9] drm/i915/psr: Move specific HSW+ WARN_ON to HSW+ function
Date: Mon, 30 Apr 2018 22:54:11 +0000	[thread overview]
Message-ID: <579376ca9bb221cd50b6ce856a6d98a2b1ba852b.camel@intel.com> (raw)
In-Reply-To: <1890796.p13hcCd3MI@dk>

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);
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx