On 06/11/14 14:09, Daniel Vetter wrote: > On Wed, Nov 05, 2014 at 05:30:52PM +0200, Mika Kuoppala wrote: >> This reverts commit 5cb13c07dae73380d8b3ddc792740487b8742938. >> >> While the relevance for WaRsDontPollForAckOnClearingFWBits is under >> investigation, revert this as regression. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85684 >> Tested-by: Tested-by: lu hua <huax.lu@xxxxxxxxx> >> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Cc: S, Deepak <deepak.s@xxxxxxxxx> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> > > Queued for -next, thanks for the patch. > -Daniel > Hi, while looking at the (semi-generic) force wake code in the function gen6_gt_force_wake_get(), I noticed that if the platform doesn't provide a low-level forcewake register-access function, then we omit the PM get/put calls as well. > void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine) > { > unsigned long irqflags; > > if (!dev_priv->uncore.funcs.force_wake_get) > return; > > intel_runtime_pm_get(dev_priv); > > /* Redirect to Gen9 specific routine */ > if (IS_GEN9(dev_priv->dev)) > return gen9_force_wake_get(dev_priv, fw_engine); > > /* Redirect to VLV specific routine */ > if (IS_VALLEYVIEW(dev_priv->dev)) > return vlv_force_wake_get(dev_priv, fw_engine); > > 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); > } Does anyone know whether this is intentional, or just an accident of the current implementation? Presumably we don't have any platforms where this applies, but we is it plausible that some future device might want the runtime PM effects without actually having h/w support for forcewake? Alternatively, why do we need the runtime PM calls in here? I can see Paolo's commit "c8c8fb3 drm/i915: add some runtime PM get/put calls" added lots of get/put pairs round various sequences of operations in debugfs etc, so why do we also need them at this inner level? Is it just to catch any cases where the caller should have (but didn't) deal with runtime PM already? Thanks, Dave _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
