* [PATCH] drm/i915/vlv: Remove check for Old Ack during forcewake @ 2014-09-18 13:21 deepak.s 2014-09-18 8:39 ` Ville Syrjälä 0 siblings, 1 reply; 6+ messages in thread From: deepak.s @ 2014-09-18 13:21 UTC (permalink / raw) To: intel-gfx; +Cc: daniel.vetter, ille.syrjala 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"); __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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/vlv: Remove check for Old Ack during forcewake 2014-09-18 13:21 [PATCH] drm/i915/vlv: Remove check for Old Ack during forcewake deepak.s @ 2014-09-18 8:39 ` Ville Syrjälä 2014-09-18 11:53 ` Daniel Vetter 2014-09-20 3:29 ` Deepak S 0 siblings, 2 replies; 6+ messages in thread From: Ville Syrjälä @ 2014-09-18 8:39 UTC (permalink / raw) To: deepak.s; +Cc: daniel.vetter, intel-gfx 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> > __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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/vlv: Remove check for Old Ack during forcewake 2014-09-18 8:39 ` Ville Syrjälä @ 2014-09-18 11:53 ` Daniel Vetter 2014-09-18 13:08 ` Ville Syrjälä 2014-09-20 3:29 ` Deepak S 1 sibling, 1 reply; 6+ messages in thread From: Daniel Vetter @ 2014-09-18 11:53 UTC (permalink / raw) To: Ville Syrjälä; +Cc: daniel.vetter, intel-gfx 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/vlv: Remove check for Old Ack during forcewake 2014-09-18 11:53 ` Daniel Vetter @ 2014-09-18 13:08 ` Ville Syrjälä 2014-09-20 3:23 ` Deepak S 0 siblings, 1 reply; 6+ messages in thread From: Ville Syrjälä @ 2014-09-18 13:08 UTC (permalink / raw) To: Daniel Vetter; +Cc: daniel.vetter, intel-gfx On Thu, Sep 18, 2014 at 01:53:13PM +0200, Daniel Vetter wrote: > 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? Perhaps. But on the other hand LRI to display registers is busted on VLV so the ddx doesn't do scanline waits which means it doesn't poke at the forcewake bits either. So only the KERNEL bit gets poked and so it should be quite impossible to encounter this issue with current userspace. Which probably explains why no one has ever reported it, or at least I don't remember seeing such a report myself. > -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 -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/vlv: Remove check for Old Ack during forcewake 2014-09-18 13:08 ` Ville Syrjälä @ 2014-09-20 3:23 ` Deepak S 0 siblings, 0 replies; 6+ messages in thread From: Deepak S @ 2014-09-20 3:23 UTC (permalink / raw) To: Ville Syrjälä, Daniel Vetter Cc: deepak.s, daniel.vetter, intel-gfx, deepak.s On Thursday 18 September 2014 06:38 PM, Ville Syrjälä wrote: > On Thu, Sep 18, 2014 at 01:53:13PM +0200, Daniel Vetter wrote: >> 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? > Perhaps. But on the other hand LRI to display registers is busted on > VLV so the ddx doesn't do scanline waits which means it doesn't > poke at the forcewake bits either. So only the KERNEL bit gets poked > and so it should be quite impossible to encounter this issue with > current userspace. Which probably explains why no one has ever > reported it, or at least I don't remember seeing such a report > myself. > >> -Daniel Your right, with current userspace we should not see the issue. But if user space starts using other forcewake bits, we might endup hanging the system due to wait for OLD Ack. Advice from HW team was to avoid this check. Thanks Deepak >>>> __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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/vlv: Remove check for Old Ack during forcewake 2014-09-18 8:39 ` Ville Syrjälä 2014-09-18 11:53 ` Daniel Vetter @ 2014-09-20 3:29 ` Deepak S 1 sibling, 0 replies; 6+ messages in thread From: Deepak S @ 2014-09-20 3:29 UTC (permalink / raw) To: Ville Syrjälä, deepak.s; +Cc: daniel.vetter, intel-gfx On Thursday 18 September 2014 02:09 PM, 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> Thanks Ville, I will change the commit message. >> __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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-19 3:33 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-18 13:21 [PATCH] drm/i915/vlv: Remove check for Old Ack during forcewake deepak.s 2014-09-18 8:39 ` Ville Syrjälä 2014-09-18 11:53 ` Daniel Vetter 2014-09-18 13:08 ` Ville Syrjälä 2014-09-20 3:23 ` Deepak S 2014-09-20 3:29 ` Deepak S
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox