| Message ID | 1357890833-8523-1-git-send-email-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 11, 2013 at 09:53:53AM +0200, Jani Nikula wrote: > We stopped reading FORCEWAKE for posting reads in > > commit 8dee3eea3ccd3b6c00a8d3a08dd715d6adf737dd > Author: Ben Widawsky <ben@bwidawsk.net> > Date: Sat Sep 1 22:59:50 2012 -0700 > > drm/i915: Never read FORCEWAKE > > and started using something from the same cacheline instead. This was true > for ECOBUS, but not for GTFIFODBG. Add posting reads on ECOBUS where > GTFIFODBG was relied on. > > In true cargo cult spirit, add posting reads for FORCEWAKE_VLV writes as > well. > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=52411 > Reported-and-tested-by: Alexander Bersenev <bay@hackerdom.ru> > CC: Ben Widawsky <ben@bwidawsk.net> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> Am I the only skeptic about what is actually fixing the bug here? Without WC MMIO, the cacheline we readback from makes no difference. ie. I put ECOBUS there for a warm and fuzzy feeling, nothing else. > --- > drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index abfff29..06b75a1 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4122,7 +4122,8 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv) > static void __gen6_gt_force_wake_mt_reset(struct drm_i915_private *dev_priv) > { > I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(0xffff)); > - POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */ > + /* something from same cacheline, but !FORCEWAKE_MT */ > + POSTING_READ(ECOBUS); > } > > static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv) > @@ -4139,7 +4140,8 @@ static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv) > DRM_ERROR("Timed out waiting for forcewake old ack to clear.\n"); > > I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL)); > - POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */ > + /* something from same cacheline, but !FORCEWAKE_MT */ > + POSTING_READ(ECOBUS); > > if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1), > FORCEWAKE_ACK_TIMEOUT_MS)) > @@ -4176,14 +4178,16 @@ void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv) > static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv) > { > I915_WRITE_NOTRACE(FORCEWAKE, 0); > - /* gen6_gt_check_fifodbg doubles as the POSTING_READ */ > + /* something from same cacheline, but !FORCEWAKE */ > + POSTING_READ(ECOBUS); > gen6_gt_check_fifodbg(dev_priv); > } > > static void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv) > { > I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL)); > - /* gen6_gt_check_fifodbg doubles as the POSTING_READ */ > + /* something from same cacheline, but !FORCEWAKE_MT */ > + POSTING_READ(ECOBUS); > gen6_gt_check_fifodbg(dev_priv); > } > > @@ -4223,6 +4227,8 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv) > static void vlv_force_wake_reset(struct drm_i915_private *dev_priv) > { > I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(0xffff)); > + /* something from same cacheline, but !FORCEWAKE_VLV */ > + POSTING_READ(FORCEWAKE_ACK_VLV); > } > > static void vlv_force_wake_get(struct drm_i915_private *dev_priv) > @@ -4243,7 +4249,8 @@ static void vlv_force_wake_get(struct drm_i915_private *dev_priv) > static void vlv_force_wake_put(struct drm_i915_private *dev_priv) > { > I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL)); > - /* The below doubles as a POSTING_READ */ > + /* something from same cacheline, but !FORCEWAKE_VLV */ > + POSTING_READ(FORCEWAKE_ACK_VLV); > gen6_gt_check_fifodbg(dev_priv); > } > > -- > 1.7.9.5 >
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index abfff29..06b75a1 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4122,7 +4122,8 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv) static void __gen6_gt_force_wake_mt_reset(struct drm_i915_private *dev_priv) { I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(0xffff)); - POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */ + /* something from same cacheline, but !FORCEWAKE_MT */ + POSTING_READ(ECOBUS); } static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv) @@ -4139,7 +4140,8 @@ static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv) DRM_ERROR("Timed out waiting for forcewake old ack to clear.\n"); I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL)); - POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */ + /* something from same cacheline, but !FORCEWAKE_MT */ + POSTING_READ(ECOBUS); if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1), FORCEWAKE_ACK_TIMEOUT_MS)) @@ -4176,14 +4178,16 @@ void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv) static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv) { I915_WRITE_NOTRACE(FORCEWAKE, 0); - /* gen6_gt_check_fifodbg doubles as the POSTING_READ */ + /* something from same cacheline, but !FORCEWAKE */ + POSTING_READ(ECOBUS); gen6_gt_check_fifodbg(dev_priv); } static void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv) { I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL)); - /* gen6_gt_check_fifodbg doubles as the POSTING_READ */ + /* something from same cacheline, but !FORCEWAKE_MT */ + POSTING_READ(ECOBUS); gen6_gt_check_fifodbg(dev_priv); } @@ -4223,6 +4227,8 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv) static void vlv_force_wake_reset(struct drm_i915_private *dev_priv) { I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(0xffff)); + /* something from same cacheline, but !FORCEWAKE_VLV */ + POSTING_READ(FORCEWAKE_ACK_VLV); } static void vlv_force_wake_get(struct drm_i915_private *dev_priv) @@ -4243,7 +4249,8 @@ static void vlv_force_wake_get(struct drm_i915_private *dev_priv) static void vlv_force_wake_put(struct drm_i915_private *dev_priv) { I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(FORCEWAKE_KERNEL)); - /* The below doubles as a POSTING_READ */ + /* something from same cacheline, but !FORCEWAKE_VLV */ + POSTING_READ(FORCEWAKE_ACK_VLV); gen6_gt_check_fifodbg(dev_priv); }
We stopped reading FORCEWAKE for posting reads in commit 8dee3eea3ccd3b6c00a8d3a08dd715d6adf737dd Author: Ben Widawsky <ben@bwidawsk.net> Date: Sat Sep 1 22:59:50 2012 -0700 drm/i915: Never read FORCEWAKE and started using something from the same cacheline instead. This was true for ECOBUS, but not for GTFIFODBG. Add posting reads on ECOBUS where GTFIFODBG was relied on. In true cargo cult spirit, add posting reads for FORCEWAKE_VLV writes as well. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=52411 Reported-and-tested-by: Alexander Bersenev <bay@hackerdom.ru> CC: Ben Widawsky <ben@bwidawsk.net> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)