diff mbox series

[2/3] drm/i915: Flush the tasklet when checking for idle

Message ID 20180914080017.30308-2-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [1/3] drm/i915: Limit the backpressure for i915_request allocation | expand

Commit Message

Chris Wilson Sept. 14, 2018, 8 a.m. UTC
In order to reduce latency when checking for idle we kick the tasklet
directly. Sometimes this is not enough as it is queued on another cpu
and so to improve the accuracy of this idle-check (and so to reduce
latency overall by avoiding another pass, or worse declaring a timeout!)
wait for the tasklet to complete.

References: https://bugs.freedesktop.org/show_bug.cgi?id=107916
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Tvrtko Ursulin Sept. 14, 2018, 10:21 a.m. UTC | #1
On 14/09/2018 09:00, Chris Wilson wrote:
> In order to reduce latency when checking for idle we kick the tasklet
> directly. Sometimes this is not enough as it is queued on another cpu
> and so to improve the accuracy of this idle-check (and so to reduce
> latency overall by avoiding another pass, or worse declaring a timeout!)
> wait for the tasklet to complete.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=107916
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_engine_cs.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 10cd051ba29e..217ed3ee1cab 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -990,6 +990,9 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine)
>   		}
>   		local_bh_enable();
>   
> +		/* Otherwise flush the tasklet if it was on another cpu */
> +		tasklet_unlock_wait(t);

That's one bizarre api! I was expecting it to mess up the state here but 
nope, apparently it is actually what one would expect to be named 
tasklet_sync.

> +
>   		if (READ_ONCE(engine->execlists.active))
>   			return false;
>   	}
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
Chris Wilson Sept. 14, 2018, 11:40 a.m. UTC | #2
Quoting Tvrtko Ursulin (2018-09-14 11:21:07)
> 
> On 14/09/2018 09:00, Chris Wilson wrote:
> > In order to reduce latency when checking for idle we kick the tasklet
> > directly. Sometimes this is not enough as it is queued on another cpu
> > and so to improve the accuracy of this idle-check (and so to reduce
> > latency overall by avoiding another pass, or worse declaring a timeout!)
> > wait for the tasklet to complete.
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=107916
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Cc: Michel Thierry <michel.thierry@intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_engine_cs.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> > index 10cd051ba29e..217ed3ee1cab 100644
> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > @@ -990,6 +990,9 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine)
> >               }
> >               local_bh_enable();
> >   
> > +             /* Otherwise flush the tasklet if it was on another cpu */
> > +             tasklet_unlock_wait(t);
> 
> That's one bizarre api! I was expecting it to mess up the state here but 
> nope, apparently it is actually what one would expect to be named 
> tasklet_sync.
> 
> > +
> >               if (READ_ONCE(engine->execlists.active))
> >                       return false;
> >       }
> > 
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Ta, first two applied. I'm in the market for a victim for number 3.
Mika!
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 10cd051ba29e..217ed3ee1cab 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -990,6 +990,9 @@  bool intel_engine_is_idle(struct intel_engine_cs *engine)
 		}
 		local_bh_enable();
 
+		/* Otherwise flush the tasklet if it was on another cpu */
+		tasklet_unlock_wait(t);
+
 		if (READ_ONCE(engine->execlists.active))
 			return false;
 	}