From mboxrd@z Thu Jan  1 00:00:00 1970
From: Arjan van de Ven <arjan@linux.intel.com>
Subject: Re: [PATCH 2/2] drm/i915: Align the retire_requests
 worker to the nearest second
Date: Fri, 05 Oct 2012 08:18:17 -0700
Message-ID: <506EFA39.4040902@linux.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>
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 mga14.intel.com (mga14.intel.com [143.182.124.37])
	by gabe.freedesktop.org (Postfix) with ESMTP id 244B19E7F7
	for <intel-gfx@lists.freedesktop.org>;
	Fri,  5 Oct 2012 08:18:19 -0700 (PDT)
In-Reply-To: <1349445188-16253-2-git-send-email-chris@chris-wilson.co.uk>
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: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
List-Id: intel-gfx@lists.freedesktop.org

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.