Re: [PATCH 02/13] drm/i915: Don't need a timer to wake us up

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Jan 08, 2016 at 11:29:41AM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Looks like the sleeping loop in __i915_wait_request can be
> simplified by using io_schedule_timeout instead of setting
> up and destroying a timer.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

io_schedule_timeout was only added in

commit 9cff8adeaa34b5d2802f03f89803da57856b3b72
Author: NeilBrown <neilb@suse.de>
Date:   Fri Feb 13 15:49:17 2015 +1100

    sched: Prevent recursion in io_schedule()

(well the EXPORT_SYMBOL for it), that was iirc why this was open-coded.
Please add this to your commit message.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 28 ++++++++--------------------
>  1 file changed, 8 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6c60e04fc09c..de98dc41fb9f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1135,11 +1135,6 @@ i915_gem_check_wedge(struct i915_gpu_error *error,
>  	return 0;
>  }
>  
> -static void fake_irq(unsigned long data)
> -{
> -	wake_up_process((struct task_struct *)data);
> -}
> -
>  static bool missed_irq(struct drm_i915_private *dev_priv,
>  		       struct intel_engine_cs *ring)
>  {
> @@ -1291,7 +1286,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
>  	}
>  
>  	for (;;) {
> -		struct timer_list timer;
> +		long sched_timeout;
>  
>  		prepare_to_wait(&ring->irq_queue, &wait, state);
>  
> @@ -1321,21 +1316,14 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
>  			break;
>  		}
>  
> -		timer.function = NULL;
> -		if (timeout || missed_irq(dev_priv, ring)) {
> -			unsigned long expire;
> -
> -			setup_timer_on_stack(&timer, fake_irq, (unsigned long)current);
> -			expire = missed_irq(dev_priv, ring) ? jiffies + 1 : timeout_expire;
> -			mod_timer(&timer, expire);
> -		}
> -
> -		io_schedule();
> +		if (timeout)
> +			sched_timeout = timeout_expire - jiffies;
> +		else if (missed_irq(dev_priv, ring))
> +			sched_timeout = 1;
> +		else
> +			sched_timeout = MAX_SCHEDULE_TIMEOUT;
>  
> -		if (timer.function) {
> -			del_singleshot_timer_sync(&timer);
> -			destroy_timer_on_stack(&timer);
> -		}
> +		io_schedule_timeout(sched_timeout);
>  	}
>  	if (!irq_test_in_progress)
>  		ring->irq_put(ring);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx