Re: [PATCH] drm/i915/vlv: Remove check for Old Ack during forcewake

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Sep 18, 2014 at 11:39:25AM +0300, Ville Syrjälä wrote:
> On Thu, Sep 18, 2014 at 06:51:50PM +0530, deepak.s@linux.intel.com wrote:
> > From: Deepak S <deepak.s@linux.intel.com>
> > 
> > Based on the HW team inputs. We can should not wait for the old ack,
> > Waiting for old ack might fail, when other forcewake came before the
> > present one is desserted.
> > 
> > for example, if forcewake bit 0 was set and before it could get cleared
> > forcewake bit 1 got set, HW eventually clear bit 0, when the bit 1
> > is cleared. i.e, bit 1 is still sent then forcewake bit 0 will still be
> > set.
> > 
> > Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_uncore.c | 10 ----------
> >  1 file changed, 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index 918b761..bcb0806 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -196,11 +196,6 @@ static void __vlv_force_wake_get(struct drm_i915_private *dev_priv,
> >  {
> >  	/* Check for Render Engine */
> >  	if (FORCEWAKE_RENDER & fw_engine) {
> > -		if (wait_for_atomic((__raw_i915_read32(dev_priv,
> > -						FORCEWAKE_ACK_VLV) &
> > -						FORCEWAKE_KERNEL) == 0,
> > -					FORCEWAKE_ACK_TIMEOUT_MS))
> > -			DRM_ERROR("Timed out: Render forcewake old ack to clear.\n");
> >  
> 
> Can we have a comment here? Something like this perhaps:
> 
> /*
>  * WaRsDontPollForAckOnClearingFWBits:vlv
>  * Hardware clears ack bits lazily (only when all ack
>  * bits become 0) so don't poll for individiual ack
>  * bits to be clear here.
>  */
> 
> Apart from that:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Shouldn't this be Cc: stable@vger.kernel.org?
-Daniel

> 
> >  		__raw_i915_write32(dev_priv, FORCEWAKE_VLV,
> >  				   _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
> > @@ -214,11 +209,6 @@ static void __vlv_force_wake_get(struct drm_i915_private *dev_priv,
> >  
> >  	/* Check for Media Engine */
> >  	if (FORCEWAKE_MEDIA & fw_engine) {
> > -		if (wait_for_atomic((__raw_i915_read32(dev_priv,
> > -						FORCEWAKE_ACK_MEDIA_VLV) &
> > -						FORCEWAKE_KERNEL) == 0,
> > -					FORCEWAKE_ACK_TIMEOUT_MS))
> > -			DRM_ERROR("Timed out: Media forcewake old ack to clear.\n");
> >  
> >  		__raw_i915_write32(dev_priv, FORCEWAKE_MEDIA_VLV,
> >  				   _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch