On Wed, Sep 10, 2014 at 05:43:15PM +0100, Chris Wilson wrote:
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
> drivers/gpu/drm/i915/intel_display.c | 20 ++------------
> drivers/gpu/drm/i915/intel_lrc.c | 21 ++-------------
> drivers/gpu/drm/i915/intel_uncore.c | 52 +++++++++++++++---------------------
> 4 files changed, 27 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5f35048..a72d8b8 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4148,6 +4148,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;
> @@ -4162,6 +4163,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 794ad8f..fafd202 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7596,7 +7596,6 @@ static void hsw_disable_lcpll(struct drm_i915_private *dev_priv,
> static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
> {
> uint32_t val;
> - unsigned long irqflags;
>
> val = I915_READ(LCPLL_CTL);
>
> @@ -7607,19 +7606,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_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);
> + gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
>
> if (val & LCPLL_POWER_DOWN_ALLOW) {
> val &= ~LCPLL_POWER_DOWN_ALLOW;
> @@ -7649,11 +7637,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_irqsave(&dev_priv->uncore.lock, irqflags);
> - 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, irqflags);
> + 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 6f1dd00..aeaa1bc 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -243,7 +243,6 @@ static void execlists_submit_pair(struct intel_engine_cs *engine,
> struct drm_i915_private *dev_priv = engine->i915;
> uint64_t tmp;
> uint32_t desc[4];
> - unsigned long flags;
>
> /* XXX: You must always write both descriptors in the order below. */
>
> @@ -260,18 +259,7 @@ static void execlists_submit_pair(struct intel_engine_cs *engine,
> desc[1] = upper_32_bits(tmp);
> desc[0] = lower_32_bits(tmp);
>
> - /* 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 (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, engine->power_domains);
> I915_WRITE(RING_ELSP(engine), desc[1]);
> I915_WRITE(RING_ELSP(engine), desc[0]);
> I915_WRITE(RING_ELSP(engine), desc[3]);
> @@ -280,12 +268,7 @@ static void execlists_submit_pair(struct intel_engine_cs *engine,
>
> /* ELSP is a wo register, so use another nearby reg for posting instead */
> POSTING_READ(RING_EXECLIST_STATUS(engine));
> -
> - /* Release Force Wakeup (see the big comment above). */
> - spin_lock_irqsave(&dev_priv->uncore.lock, flags);
> - 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, engine->power_domains);
> }
>
> static u16 next_tag(struct intel_engine_cs *engine)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index c99d5ef..3b3d3e0 100644
> --- 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__))
> @@ -258,10 +260,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;
> @@ -271,16 +269,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)
> @@ -295,8 +287,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 gen6_force_wake_timer(unsigned long arg)
Looks like you forgot to kill the rpm_put() from the timer. And then we
also need to make sure the timer is approriately cancelled when
runtime suspending the device.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
