On Monday 08 September 2014 08:10 PM, Daniel Vetter wrote:
On Mon, Sep 08, 2014 at 05:14:23PM +0300, Ville Syrjälä wrote:
On Mon, Sep 08, 2014 at 05:02:43PM +0300, Ville Syrjälä wrote:
On Tue, Sep 09, 2014 at 07:14:16PM +0530, deepa...@linux.intel.com wrote:
From: Deepak S <deepa...@linux.intel.com>

In chv, we have two power wells Render & Media. We need to use
corresponsing forcewake count. If we dont follow this we are getting
error "*ERROR*: Timed out waiting for forcewake old ack to clear" due to
multiple entry into __vlv_force_wake_get.

Signed-off-by: Deepak S <deepa...@linux.intel.com>
---
  drivers/gpu/drm/i915/intel_lrc.c | 29 +++++++++++++++++++++++++----
  1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index bd1b28d..bafd38b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -300,8 +300,18 @@ static void execlists_elsp_write(struct intel_engine_cs 
*ring,
         * Instead, we do the runtime_pm_get/put when creating/destroying 
requests.
         */
        spin_lock_irqsave(&dev_priv->uncore.lock, flags);
-       if (dev_priv->uncore.forcewake_count++ == 0)
-               dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_ALL);
+       if (IS_CHERRYVIEW(dev_priv->dev)) {
+               if (dev_priv->uncore.fw_rendercount++ == 0)
+                       dev_priv->uncore.funcs.force_wake_get(dev_priv,
+                                                             FORCEWAKE_RENDER);
+               if (dev_priv->uncore.fw_mediacount++ == 0)
+                       dev_priv->uncore.funcs.force_wake_get(dev_priv,
+                                                             FORCEWAKE_MEDIA);
This will wake both wells. Is that needed or should we just pick one
based on the ring?
Also unlike the comment says runtime_pm_get() can't sleep since someone
must already be holding a reference, othwewise we surely can't go
writing any registers. So in theory we should be able to call
gen6_gt_force_wake_get() here, but maybe that would trigger a
might_sleep() warning. the current force wake code duplication (esp.
outside intel_uncore.c) is rather unfortunate and I'd like to see it
killed off. Maybe we just need to pull the rpm get/put outside
gen6_gt_force_wake_get()? I never really liked hiding it there anyway.
Yeah this is just broken design. And if you look at the other wheel to
track outstanding gpu work (requests) then it's not needed at all.

But I'm not sure what's the priority of the "rework execlists to use
requests" task is and when (if ever that will happen). Jesse is the
arbiter for this stuff anyway, so adding him.
-Daniel

hmm , agreed do we have a reworked execlist? The reason why added this, on chv 
when i enable execlist, due to incorrect forcewake count
is causing multiple entries to forcewake_get resulting in "*ERROR*: Timed out waiting for 
forcewake old ack to clear" "and Hang.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to