On Fri, 05 Oct 2012, Chris Wilson <[email protected]> wrote: > By using round_jiffies() we can align the wakeup of our worker to the > nearest second in order to batch wakeups and reduce system load, which > is useful for unimportant coarse tasks like our retire_requests.
Is there a reason not to just use INIT_DELAYED_WORK_DEFERRABLE()? Come to think of it, same with deferrable timer in patch 1/2. > Suggested-by: Arjan van de Ven <[email protected]> > Signed-off-by: Chris Wilson <[email protected]> > Cc: Arjan van de Ven <[email protected]> > --- > drivers/gpu/drm/i915/i915_gem.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 8e05d53..706f481 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2084,6 +2084,11 @@ i915_gem_next_request_seqno(struct intel_ring_buffer > *ring) > return ring->outstanding_lazy_request; > } > > +static unsigned long round_jiffies_delay(unsigned long delay) > +{ > + return round_jiffies_relative(delay) - jiffies; > +} Hmm, is it possible that would end up negative if someone reuses that with a small delay? An observation: there's a bunch of calls elsewhere in kernel to queue_delayed_work() with the delay wrapped in round_jiffies() or round_jiffies_relative(). The former at least gets queued within expected tolerance (though likely not on full second), but how could the code using the latter ever work?! I guess a function like yours could be useful in generic code. BR, Jani. > + > int > i915_add_request(struct intel_ring_buffer *ring, > struct drm_file *file, > @@ -2155,7 +2160,8 @@ i915_add_request(struct intel_ring_buffer *ring, > } > if (was_empty) { > queue_delayed_work(dev_priv->wq, > - &dev_priv->mm.retire_work, HZ); > + &dev_priv->mm.retire_work, > + round_jiffies_delay(HZ)); > intel_mark_busy(dev_priv->dev); > } > } > @@ -2346,7 +2352,8 @@ i915_gem_retire_work_handler(struct work_struct *work) > > /* Come back later if the device is busy... */ > if (!mutex_trylock(&dev->struct_mutex)) { > - queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, HZ); > + queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, > + round_jiffies_delay(HZ)); > return; > } > > @@ -2364,7 +2371,8 @@ i915_gem_retire_work_handler(struct work_struct *work) > } > > if (!dev_priv->mm.suspended && !idle) > - queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, HZ); > + queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, > + round_jiffies_delay(HZ)); > if (idle) > intel_mark_idle(dev); > > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
