From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: Convert the forcewake worker into a timer func Date: Fri, 28 Feb 2014 11:16:33 +0200 Message-ID: <20140228091633.GO3852@intel.com> References: <20140224203148.GB14674@bwidawsk.net> <1393578158-2112-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id DB7E6FBB81 for ; Fri, 28 Feb 2014 01:16:37 -0800 (PST) Content-Disposition: inline In-Reply-To: <1393578158-2112-1-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org, Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Fri, Feb 28, 2014 at 09:02:38AM +0000, Chris Wilson wrote: > We don't want to suffer scheduling delay when turning off the GPU after > waking it up to touch registers. Ideally, we only want to keep the GPU > awake for the register access sequence, with a single forcewake dance on > the first access and release immediately after the last. We set a timer > on the first access so that we only dance once and on the next scheduler > tick, we drop the forcewake again. > = > This moves the cleanup routine from the common i915 workqueue to a timer > func so that we don't anger powertop, and drop the forcewake again > quicker. > = > Signed-off-by: Chris Wilson > Cc: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_uncore.c | 15 ++++++--------- > 2 files changed, 7 insertions(+), 10 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_= drv.h > index eb31c45..9416b03 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -508,7 +508,7 @@ struct intel_uncore { > unsigned fw_rendercount; > unsigned fw_mediacount; > = > - struct delayed_work force_wake_work; > + struct timer_list force_wake_timer; > }; > = > #define DEV_INFO_FOR_EACH_FLAG(func, sep) \ > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/i= ntel_uncore.c > index c628414..140a9f0 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -289,10 +289,8 @@ void vlv_force_wake_put(struct drm_i915_private *dev= _priv, > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > } > = > -static void gen6_force_wake_work(struct work_struct *work) > +static void gen6_force_wake_timer(struct drm_i915_private *dev_priv) > { > - struct drm_i915_private *dev_priv =3D > - container_of(work, typeof(*dev_priv), uncore.force_wake_work.work); > unsigned long irqflags; > = > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > @@ -405,9 +403,8 @@ void gen6_gt_force_wake_put(struct drm_i915_private *= dev_priv, int fw_engine) > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > if (--dev_priv->uncore.forcewake_count =3D=3D 0) { > dev_priv->uncore.forcewake_count++; > - mod_delayed_work(dev_priv->wq, > - &dev_priv->uncore.force_wake_work, > - 1); > + mod_timer_pinned(&dev_priv->uncore.force_wake_timer, > + jiffies + 1); > } > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); Should we use the timer to delay the forcewake release from register read/write functions too? Or maybe do it only for register read/write since gen6_gt_force_wake_get/put are rather rare and I wouldn't expect significant forcewake ping-pong after the put. > = > @@ -681,8 +678,8 @@ void intel_uncore_init(struct drm_device *dev) > { > struct drm_i915_private *dev_priv =3D dev->dev_private; > = > - INIT_DELAYED_WORK(&dev_priv->uncore.force_wake_work, > - gen6_force_wake_work); > + setup_timer(&dev_priv->uncore.force_wake_timer, > + gen6_force_wake_timer, (unsigned long)dev_priv); > = > if (IS_VALLEYVIEW(dev)) { > dev_priv->uncore.funcs.force_wake_get =3D __vlv_force_wake_get; > @@ -794,7 +791,7 @@ void intel_uncore_fini(struct drm_device *dev) > { > struct drm_i915_private *dev_priv =3D dev->dev_private; > = > - flush_delayed_work(&dev_priv->uncore.force_wake_work); > + del_timer_sync(&dev_priv->uncore.force_wake_timer); > = > /* Paranoia: make sure we have disabled everything before we exit. */ > intel_uncore_sanitize(dev); > -- = > 1.7.9.5 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC