| Message ID | 1345821981-8208-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, 24 Aug 2012 17:26:20 +0200 Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > For some odd reasons, the vlv forcewake code is rather different from > all other platforms, with no clear justification. Adjust things: > > - Don't check whether the gt is awake already (and bail out early), we > need to grab a forcewake anyway. Otherwise the chip might go to > sleep too early. And this would also screw up our forcewake > accounting. > - Like all other platforms, check whether the gt has cleared the > forcewake bit in the _ACK register before setting it again. > - Use _MASKED_BIT_ENABLE/DISABLE macros > - Only use bit0 of the forcewake reg, not all 16 bits. > - check the gtfifodb reg like on all other platforms in _put. > - Drop the POSTING_READs for consistency. > > v2: Failure to git add ... again. > > v3: Fixup the spelling fail a bit. > > Tested-by: "Purushothaman, Vijay A" <vijay.a.purushothaman@intel.com> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch> I'd like to see the FORCEWAKE patches I've recently submitted get merged, and the same magic in those get applied here. Regarding docs, I can't find the right ones to verify any of the forcewake stuff, so no r-b. Maybe Jesse can help. For an a-b I'd like you to take the forcewake fixes, and considering squashing the vlv specific functions into the main functions, since you've removed most of the differences anyway. In any case, it is: Tested-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/intel_pm.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index c0721ff..1a197da 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4075,12 +4075,10 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv) > > static void vlv_force_wake_get(struct drm_i915_private *dev_priv) > { > - /* Already awake? */ > - if ((I915_READ(0x130094) & 0xa1) == 0xa1) > - return; > + if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1) == 0, 500)) > + DRM_ERROR("Force wake wait timed out\n"); WTF? > > - I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffffffff); > - POSTING_READ(FORCEWAKE_VLV); > + I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_ENABLE(1)); > > if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1), 500)) > DRM_ERROR("Force wake wait timed out\n"); > @@ -4090,9 +4088,9 @@ 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, 0xffff0000); > - /* FIXME: confirm VLV behavior with Punit folks */ > - POSTING_READ(FORCEWAKE_VLV); > + I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(1)); > + /* The below doubles as a POSTING_READ */ > + gen6_gt_check_fifodbg(dev_priv); > } > > void intel_gt_init(struct drm_device *dev)
On Sun, Sep 02, 2012 at 03:43:39PM -0700, Ben Widawsky wrote: > On Fri, 24 Aug 2012 17:26:20 +0200 > Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > > For some odd reasons, the vlv forcewake code is rather different from > > all other platforms, with no clear justification. Adjust things: > > > > - Don't check whether the gt is awake already (and bail out early), we > > need to grab a forcewake anyway. Otherwise the chip might go to > > sleep too early. And this would also screw up our forcewake > > accounting. > > - Like all other platforms, check whether the gt has cleared the > > forcewake bit in the _ACK register before setting it again. > > - Use _MASKED_BIT_ENABLE/DISABLE macros > > - Only use bit0 of the forcewake reg, not all 16 bits. > > - check the gtfifodb reg like on all other platforms in _put. > > - Drop the POSTING_READs for consistency. > > > > v2: Failure to git add ... again. > > > > v3: Fixup the spelling fail a bit. > > > > Tested-by: "Purushothaman, Vijay A" <vijay.a.purushothaman@intel.com> > > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > I'd like to see the FORCEWAKE patches I've recently submitted get > merged, and the same magic in those get applied here. > > Regarding docs, I can't find the right ones to verify any of the > forcewake stuff, so no r-b. Maybe Jesse can help. For an a-b I'd like > you to take the forcewake fixes, and considering squashing the vlv > specific functions into the main functions, since you've removed most of > the differences anyway. I've figured I'll just apply it and let Jesse yell it at me ;-) After all, this patch has been floating for a few weeks already in some form or another, and seems to no break 2 of the 3 vlv machines we have access too ... > In any case, it is: > Tested-by: Ben Widawsky <ben@bwidawsk.net> Thanks, I've merged both patches for -next. -Daniel > > > > > --- > > drivers/gpu/drm/i915/intel_pm.c | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index c0721ff..1a197da 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -4075,12 +4075,10 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv) > > > > static void vlv_force_wake_get(struct drm_i915_private *dev_priv) > > { > > - /* Already awake? */ > > - if ((I915_READ(0x130094) & 0xa1) == 0xa1) > > - return; > > + if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1) == 0, 500)) > > + DRM_ERROR("Force wake wait timed out\n"); > > WTF? I hope that wtf is for the old code, not the new ... > > > > > - I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffffffff); > > - POSTING_READ(FORCEWAKE_VLV); > > + I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_ENABLE(1)); > > > > if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1), 500)) > > DRM_ERROR("Force wake wait timed out\n"); > > @@ -4090,9 +4088,9 @@ 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, 0xffff0000); > > - /* FIXME: confirm VLV behavior with Punit folks */ > > - POSTING_READ(FORCEWAKE_VLV); > > + I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(1)); > > + /* The below doubles as a POSTING_READ */ > > + gen6_gt_check_fifodbg(dev_priv); > > } > > > > void intel_gt_init(struct drm_device *dev) > > > > -- > Ben Widawsky, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c0721ff..1a197da 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4075,12 +4075,10 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv) static void vlv_force_wake_get(struct drm_i915_private *dev_priv) { - /* Already awake? */ - if ((I915_READ(0x130094) & 0xa1) == 0xa1) - return; + if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1) == 0, 500)) + DRM_ERROR("Force wake wait timed out\n"); - I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffffffff); - POSTING_READ(FORCEWAKE_VLV); + I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_ENABLE(1)); if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1), 500)) DRM_ERROR("Force wake wait timed out\n"); @@ -4090,9 +4088,9 @@ 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, 0xffff0000); - /* FIXME: confirm VLV behavior with Punit folks */ - POSTING_READ(FORCEWAKE_VLV); + I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(1)); + /* The below doubles as a POSTING_READ */ + gen6_gt_check_fifodbg(dev_priv); } void intel_gt_init(struct drm_device *dev)