drm/i915: Assert that runtime pm is active on user fw access

On user forcewake access, assert that runtime pm reference is held.
Fix and cleanup the callsites accordingly.

v2: Remove intel_runtime_pm_get() rebasehap (Deepak)

v3: use drivers own runtime state tracking as pm_runtime_active()
    will return wrong results when we are in resume callchain (Mika)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Deepak S <deepak.s@linux.intel.com> (v2)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7952f313dda29496cc3cef4ebb78f5898589836d..ac517eb07c00ca13bc678d87f6c3e869f9e5b630 100644 (file)
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4339,6 +4339,7 @@ static int i915_forcewake_open(struct inode *inode, struct file *file)
        if (INTEL_INFO(dev)->gen < 6)
                return 0;
 
+       intel_runtime_pm_get(dev_priv);
        gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
 
        return 0;
@@ -4353,6 +4354,7 @@ static int i915_forcewake_release(struct inode *inode, struct file *file)
                return 0;
 
        gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
+       intel_runtime_pm_put(dev_priv);
 
        return 0;
 }

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c5cbcd74387a6713724ef2c66b1873265f83e87b..61b123f1a6527a9de89836cbc37fb623529df873 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7870,19 +7870,8 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
        /*
         * Make sure we're not on PC8 state before disabling PC8, otherwise
         * we'll hang the machine. To prevent PC8 state, just enable force_wake.
-        *
-        * The other problem is that hsw_restore_lcpll() is called as part of
-        * the runtime PM resume sequence, so we can't just call
-        * gen6_gt_force_wake_get() because that function calls
-        * intel_runtime_pm_get(), and we can't change the runtime PM refcount
-        * while we are on the resume sequence. So to solve this problem we have
-        * to call special forcewake code that doesn't touch runtime PM and
-        * doesn't enable the forcewake delayed work.
         */
-       spin_lock_irq(&dev_priv->uncore.lock);
-       if (dev_priv->uncore.forcewake_count++ == 0)
-               dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_ALL);
-       spin_unlock_irq(&dev_priv->uncore.lock);
+       gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
 
        if (val & LCPLL_POWER_DOWN_ALLOW) {
                val &= ~LCPLL_POWER_DOWN_ALLOW;
@@ -7912,11 +7901,7 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
                        DRM_ERROR("Switching back to LCPLL failed\n");
        }
 
-       /* See the big comment above. */
-       spin_lock_irq(&dev_priv->uncore.lock);
-       if (--dev_priv->uncore.forcewake_count == 0)
-               dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL);
-       spin_unlock_irq(&dev_priv->uncore.lock);
+       gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
 }
 
 /*

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index a5f614de08282b84678e9fb65d084bbb0bd74fab..49c7d862f8a5672a1e298d3ff84b26a6b719c988 100644 (file)
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -283,7 +283,6 @@ static void execlists_elsp_write(struct intel_engine_cs *ring,
        struct drm_i915_private *dev_priv = dev->dev_private;
        uint64_t temp = 0;
        uint32_t desc[4];
-       unsigned long flags;
 
        /* XXX: You must always write both descriptors in the order below. */
        if (ctx_obj1)
@@ -297,63 +296,17 @@ static void execlists_elsp_write(struct intel_engine_cs *ring,
        desc[3] = (u32)(temp >> 32);
        desc[2] = (u32)temp;
 
-       /* Set Force Wakeup bit to prevent GT from entering C6 while ELSP writes
-        * are in progress.
-        *
-        * The other problem is that we can't just call gen6_gt_force_wake_get()
-        * because that function calls intel_runtime_pm_get(), which might sleep.
-        * Instead, we do the runtime_pm_get/put when creating/destroying requests.
-        */
-       spin_lock_irqsave(&dev_priv->uncore.lock, flags);
-       if (IS_CHERRYVIEW(dev) || INTEL_INFO(dev)->gen >= 9) {
-               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);
-               if (INTEL_INFO(dev)->gen >= 9) {
-                       if (dev_priv->uncore.fw_blittercount++ == 0)
-                               dev_priv->uncore.funcs.force_wake_get(dev_priv,
-                                                       FORCEWAKE_BLITTER);
-               }
-       } else {
-               if (dev_priv->uncore.forcewake_count++ == 0)
-                       dev_priv->uncore.funcs.force_wake_get(dev_priv,
-                                                             FORCEWAKE_ALL);
-       }
-       spin_unlock_irqrestore(&dev_priv->uncore.lock, flags);
-
+       gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
        I915_WRITE(RING_ELSP(ring), desc[1]);
        I915_WRITE(RING_ELSP(ring), desc[0]);
        I915_WRITE(RING_ELSP(ring), desc[3]);
+
        /* The context is automatically loaded after the following */
        I915_WRITE(RING_ELSP(ring), desc[2]);
 
        /* ELSP is a wo register, so use another nearby reg for posting instead */
        POSTING_READ(RING_EXECLIST_STATUS(ring));
-
-       /* Release Force Wakeup (see the big comment above). */
-       spin_lock_irqsave(&dev_priv->uncore.lock, flags);
-       if (IS_CHERRYVIEW(dev) || INTEL_INFO(dev)->gen >= 9) {
-               if (--dev_priv->uncore.fw_rendercount == 0)
-                       dev_priv->uncore.funcs.force_wake_put(dev_priv,
-                                                             FORCEWAKE_RENDER);
-               if (--dev_priv->uncore.fw_mediacount == 0)
-                       dev_priv->uncore.funcs.force_wake_put(dev_priv,
-                                                             FORCEWAKE_MEDIA);
-               if (INTEL_INFO(dev)->gen >= 9) {
-                       if (--dev_priv->uncore.fw_blittercount == 0)
-                               dev_priv->uncore.funcs.force_wake_put(dev_priv,
-                                                       FORCEWAKE_BLITTER);
-               }
-       } else {
-               if (--dev_priv->uncore.forcewake_count == 0)
-                       dev_priv->uncore.funcs.force_wake_put(dev_priv,
-                                                             FORCEWAKE_ALL);
-       }
-
-       spin_unlock_irqrestore(&dev_priv->uncore.lock, flags);
+       gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
 }
 
 static int execlists_update_context(struct drm_i915_gem_object *ctx_obj,

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index b39ed7968383c0b24eab080089e75e41910a9bad..974881e23ffaddd4d6fe2a4c3c41dc2b1e3a076a 100644 (file)
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -24,6 +24,8 @@
 #include "i915_drv.h"
 #include "intel_drv.h"
 
+#include <linux/pm_runtime.h>
+
 #define FORCEWAKE_ACK_TIMEOUT_MS 2
 
 #define __raw_i915_read8(dev_priv__, reg__) readb((dev_priv__)->regs + (reg__))
@@ -247,10 +249,6 @@ static void __vlv_force_wake_put(struct drm_i915_private *dev_priv,
 
 static void vlv_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
 {
-       unsigned long irqflags;
-
-       spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
        if (fw_engine & FORCEWAKE_RENDER &&
            dev_priv->uncore.fw_rendercount++ != 0)
                fw_engine &= ~FORCEWAKE_RENDER;
@@ -260,16 +258,10 @@ static void vlv_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
 
        if (fw_engine)
                dev_priv->uncore.funcs.force_wake_get(dev_priv, fw_engine);
-
-       spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void vlv_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
 {
-       unsigned long irqflags;
-
-       spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
        if (fw_engine & FORCEWAKE_RENDER) {
                WARN_ON(!dev_priv->uncore.fw_rendercount);
                if (--dev_priv->uncore.fw_rendercount != 0)
@@ -284,8 +276,6 @@ static void vlv_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
 
        if (fw_engine)
                dev_priv->uncore.funcs.force_wake_put(dev_priv, fw_engine);
-
-       spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void __gen9_gt_force_wake_mt_reset(struct drm_i915_private *dev_priv)
@@ -380,10 +370,6 @@ __gen9_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
 static void
 gen9_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
 {
-       unsigned long irqflags;
-
-       spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
        if (FORCEWAKE_RENDER & fw_engine) {
                if (dev_priv->uncore.fw_rendercount++ == 0)
                        dev_priv->uncore.funcs.force_wake_get(dev_priv,
@@ -401,17 +387,11 @@ gen9_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
                        dev_priv->uncore.funcs.force_wake_get(dev_priv,
                                                        FORCEWAKE_BLITTER);
        }
-
-       spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
 gen9_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
 {
-       unsigned long irqflags;
-
-       spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
        if (FORCEWAKE_RENDER & fw_engine) {
                WARN_ON(dev_priv->uncore.fw_rendercount == 0);
                if (--dev_priv->uncore.fw_rendercount == 0)
@@ -432,8 +412,6 @@ gen9_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
                        dev_priv->uncore.funcs.force_wake_put(dev_priv,
                                                        FORCEWAKE_BLITTER);
        }
-
-       spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void gen6_force_wake_timer(unsigned long arg)
@@ -562,19 +540,20 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine)
        if (!dev_priv->uncore.funcs.force_wake_get)
                return;
 
-       intel_runtime_pm_get(dev_priv);
+       WARN_ON(dev_priv->pm.suspended);
 
-       /* Redirect to Gen9 specific routine */
-       if (IS_GEN9(dev_priv->dev))
-               return gen9_force_wake_get(dev_priv, fw_engine);
+       spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-       /* Redirect to VLV specific routine */
-       if (IS_VALLEYVIEW(dev_priv->dev))
-               return vlv_force_wake_get(dev_priv, fw_engine);
+       if (IS_GEN9(dev_priv->dev)) {
+               gen9_force_wake_get(dev_priv, fw_engine);
+       } else if (IS_VALLEYVIEW(dev_priv->dev)) {
+               vlv_force_wake_get(dev_priv, fw_engine);
+       } else {
+               if (dev_priv->uncore.forcewake_count++ == 0)
+                       dev_priv->uncore.funcs.force_wake_get(dev_priv,
+                                                             FORCEWAKE_ALL);
+       }
 
-       spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-       if (dev_priv->uncore.forcewake_count++ == 0)
-               dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_ALL);
        spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
@@ -588,31 +567,22 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
        if (!dev_priv->uncore.funcs.force_wake_put)
                return;
 
-       /* Redirect to Gen9 specific routine */
+       spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
        if (IS_GEN9(dev_priv->dev)) {
                gen9_force_wake_put(dev_priv, fw_engine);
-               goto out;
-       }
-
-       /* Redirect to VLV specific routine */
-       if (IS_VALLEYVIEW(dev_priv->dev)) {
+       } else if (IS_VALLEYVIEW(dev_priv->dev)) {
                vlv_force_wake_put(dev_priv, fw_engine);
-               goto out;
-       }
-
-       spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-       WARN_ON(!dev_priv->uncore.forcewake_count);
-
-       if (--dev_priv->uncore.forcewake_count == 0) {
-               dev_priv->uncore.forcewake_count++;
-               mod_timer_pinned(&dev_priv->uncore.force_wake_timer,
-                                jiffies + 1);
+       } else {
+               WARN_ON(!dev_priv->uncore.forcewake_count);
+               if (--dev_priv->uncore.forcewake_count == 0) {
+                       dev_priv->uncore.forcewake_count++;
+                       mod_timer_pinned(&dev_priv->uncore.force_wake_timer,
+                                        jiffies + 1);
+               }
        }
 
        spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
-
-out:
-       intel_runtime_pm_put(dev_priv);
 }
 
 void assert_force_wake_inactive(struct drm_i915_private *dev_priv)