| Message ID | 1459900669-31740-7-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On ke, 2016-04-06 at 00:57 +0100, Chris Wilson wrote: > Having fixed the tracking of the engine's last_submitted_seqno, we can > now rely on it for detecting when the engine is idle (and not have to > touch the requests pointer). > > Testcase: igt/gem_exec_whisper > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index c85eb8dec2dc..a56be4f92264 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2805,8 +2805,7 @@ static void gen8_disable_vblank(struct drm_device *dev, unsigned int pipe) > static bool > ring_idle(struct intel_engine_cs *engine, u32 seqno) > { > - return (list_empty(&engine->request_list) || > - i915_seqno_passed(seqno, engine->last_submitted_seqno)); > + return i915_seqno_passed(seqno, engine->last_submitted_seqno); > } > > static bool
Chris Wilson <chris@chris-wilson.co.uk> writes: > [ text/plain ] > Having fixed the tracking of the engine's last_submitted_seqno, we can > now rely on it for detecting when the engine is idle (and not have to > touch the requests pointer). > > Testcase: igt/gem_exec_whisper > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index c85eb8dec2dc..a56be4f92264 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2805,8 +2805,7 @@ static void gen8_disable_vblank(struct drm_device *dev, unsigned int pipe) > static bool > ring_idle(struct intel_engine_cs *engine, u32 seqno) > { > - return (list_empty(&engine->request_list) || > - i915_seqno_passed(seqno, engine->last_submitted_seqno)); > + return i915_seqno_passed(seqno, engine->last_submitted_seqno); My concern here is that we should move the write for the last_submitted_seqno before we push the work into ring. To guarantee that the driver never sees a seqno > last_submitted_seqno on particular ring. -Mika > } > > static bool > -- > 2.8.0.rc3
On Wed, Apr 06, 2016 at 12:32:29PM +0300, Mika Kuoppala wrote: > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > [ text/plain ] > > Having fixed the tracking of the engine's last_submitted_seqno, we can > > now rely on it for detecting when the engine is idle (and not have to > > touch the requests pointer). > > > > Testcase: igt/gem_exec_whisper > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_irq.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > index c85eb8dec2dc..a56be4f92264 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -2805,8 +2805,7 @@ static void gen8_disable_vblank(struct drm_device *dev, unsigned int pipe) > > static bool > > ring_idle(struct intel_engine_cs *engine, u32 seqno) > > { > > - return (list_empty(&engine->request_list) || > > - i915_seqno_passed(seqno, engine->last_submitted_seqno)); > > + return i915_seqno_passed(seqno, engine->last_submitted_seqno); > > My concern here is that we should move the write for the > last_submitted_seqno before we push the work into ring. > > To guarantee that the driver never sees a seqno > last_submitted_seqno > on particular ring. Reasonable request. You'll want a smp_store_mb() on the last_submitted_seqno as well with a comment /* barrier for hangcheck */ In return what's up with the Braswell? -Chris
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index c85eb8dec2dc..a56be4f92264 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2805,8 +2805,7 @@ static void gen8_disable_vblank(struct drm_device *dev, unsigned int pipe) static bool ring_idle(struct intel_engine_cs *engine, u32 seqno) { - return (list_empty(&engine->request_list) || - i915_seqno_passed(seqno, engine->last_submitted_seqno)); + return i915_seqno_passed(seqno, engine->last_submitted_seqno); } static bool
Having fixed the tracking of the engine's last_submitted_seqno, we can now rely on it for detecting when the engine is idle (and not have to touch the requests pointer). Testcase: igt/gem_exec_whisper Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)