Re: [PATCH 3/7] drm/i915: Spin opportunistically in wait_for

From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 3/7] drm/i915: Spin opportunistically in wait_for
Date: Wed, 18 May 2016 14:08:53 +0300	[thread overview]
Message-ID: <87oa8362t6.fsf@gaia.fi.intel.com> (raw)
In-Reply-To: <20160518104704.GH30479@nuc-i3427.alporthouse.com>

Chris Wilson <chris@chris-wilson.co.uk> writes:

> [ text/plain ]
> On Wed, May 18, 2016 at 10:20:22AM +0200, Daniel Vetter wrote:
>> On Tue, May 17, 2016 at 06:43:24PM +0300, Mika Kuoppala wrote:
>> > Usually the condition we are after appears within very short time.
>> > Spin few times before going into sleep. With this approximately
>> > half of the wait_for in init path will take the fast path without
>> > sleeping.
>> > 
>> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> 
>> Some numbers on how much time this saved would be nice.
>> 
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> 
>> > ---
>> >  drivers/gpu/drm/i915/intel_drv.h | 14 ++++++++++----
>> >  1 file changed, 10 insertions(+), 4 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> > index 488141929a7a..c225605c727c 100644
>> > --- a/drivers/gpu/drm/i915/intel_drv.h
>> > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> > @@ -39,7 +39,7 @@
>> >  #include <drm/drm_atomic.h>
>> >  
>> >  /**
>> > - * _wait_for_ms - magic (register) wait macro
>> > + * __wait_for_ms - magic (register) wait macro
>> >   *
>> >   * Does the right thing for modeset paths when run under kdgb or similar atomic
>> >   * contexts. Note that it's important that we check the condition again after
>> > @@ -50,17 +50,22 @@
>> >   * drm_can_sleep() can be removed and in_atomic()/!in_atomic() asserts
>> >   * added.
>> >   */
>> > -#define _wait_for_ms(COND, TIMEOUT_MS, SLEEP_US) ({ \
>> > +#define __wait_for_ms(COND, TIMEOUT_MS, SLEEP_US, SPIN_COUNT) ({	\
>> >  	const unsigned long timeout__ =					\
>> >  		jiffies + msecs_to_jiffies(TIMEOUT_MS) + 1;		\
>> > +	unsigned int c__ = 0;						\
>> >  	int ret__ = 0;							\
>> > +									\
>> >  	while (!(COND)) {						\
>> >  		if (time_after(jiffies, timeout__)) {			\
>> >  			if (!(COND))					\
>> >  				ret__ = -ETIMEDOUT;			\
>> >  			break;						\
>> >  		}							\
>> > -		if ((SLEEP_US) && drm_can_sleep()) {			\
>> > +									\
>> > +		if (++c__ > (SPIN_COUNT) &&				\
>> > +		    (SLEEP_US) &&					\
>
> Could we kill SLEEP_US here in patch 0?

Patch 1? Yes we could.
-Mika

> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx