From mboxrd@z Thu Jan  1 00:00:00 1970
From: Chris Wilson <chris@chris-wilson.co.uk>
Subject: Re: [PATCH 2/2] drm/i915: Align the retire_requests
	worker to the nearest second
Date: Fri, 05 Oct 2012 16:55:29 +0100
Message-ID: <275ffc$6saak7@fmsmga002.fm.intel.com>
References: <1349445188-16253-1-git-send-email-chris@chris-wilson.co.uk>
	<1349445188-16253-2-git-send-email-chris@chris-wilson.co.uk>
	<506EFA39.4040902@linux.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
Received: from mga11.intel.com (mga11.intel.com [192.55.52.93])
	by gabe.freedesktop.org (Postfix) with ESMTP id E89019EB29
	for <intel-gfx@lists.freedesktop.org>;
	Fri,  5 Oct 2012 08:55:57 -0700 (PDT)
In-Reply-To: <506EFA39.4040902@linux.intel.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
To: Arjan van de Ven <arjan@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
List-Id: intel-gfx@lists.freedesktop.org

On Fri, 05 Oct 2012 08:18:17 -0700, Arjan van de Ven <arjan@linux.intel.com> wrote:
> On 10/5/2012 6:53 AM, Chris Wilson 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.
> > 
> > Suggested-by: Arjan van de Ven <arjan@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Arjan van de Ven <arjan@linux.intel.com>
> > ---
> >  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;
> > +}
> 
> this is buggy
> 
> 
> > +
> >  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));
> 
> when used like this
> 
> 
> round_jiffies() rounds absolute jiffies towards the next second
> 
> round_jiffies_relative() already subtracts jiffies from the result, like
> the helper that you're trying to invent here does ;=)
> 
> doing that double up is a bad idea.

For some reason the example I read convinced me that
round_jiffies_relative() returned the absolute jiffie for the relative
delay so that we could put it straight into mod_timer().

Again we can use round up here as well.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre