| Message ID | 1459900669-31740-5-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: > An oversight is that when we wrap the seqno, we need to reset the hw > semaphore counters to 0. We did this for gen6 and gen7 and forgot to do > so for the new implementation required for gen8 (legacy). > > 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/intel_ringbuffer.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index fb304df8085d..c7023d6ca0b7 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -2564,6 +2564,14 @@ void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno) > } > memset(engine->semaphore.sync_seqno, 0, > sizeof(engine->semaphore.sync_seqno)); > + if (dev_priv->semaphore_obj) { > + struct drm_i915_gem_object *obj = dev_priv->semaphore_obj; > + struct page *page = i915_gem_object_get_dirty_page(obj, 0); > + uint64_t *semaphores = kmap(page); > + memset(semaphores + engine->id * I915_NUM_ENGINES, 0, > + sizeof(*semaphores) * I915_NUM_ENGINES); There is i915_semaphore_seqno_size define which the GEN8_WAIT_OFFSET and GEN8_SIGNAL_OFFSET use. So rather use that to stay consistent. Other than that, Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> PS. The existing semaphore code could use some cleanup, adding to backlog. > + kunmap(page); > + } > > engine->set_seqno(engine, seqno); >
On Wed, Apr 06, 2016 at 12:58:43PM +0300, Joonas Lahtinen wrote: > On ke, 2016-04-06 at 00:57 +0100, Chris Wilson wrote: > > An oversight is that when we wrap the seqno, we need to reset the hw > > semaphore counters to 0. We did this for gen6 and gen7 and forgot to do > > so for the new implementation required for gen8 (legacy). > > > > 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/intel_ringbuffer.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index fb304df8085d..c7023d6ca0b7 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -2564,6 +2564,14 @@ void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno) > > } > > memset(engine->semaphore.sync_seqno, 0, > > sizeof(engine->semaphore.sync_seqno)); > > + if (dev_priv->semaphore_obj) { > > + struct drm_i915_gem_object *obj = dev_priv->semaphore_obj; > > + struct page *page = i915_gem_object_get_dirty_page(obj, 0); > > + uint64_t *semaphores = kmap(page); > > + memset(semaphores + engine->id * I915_NUM_ENGINES, 0, > > + sizeof(*semaphores) * I915_NUM_ENGINES); > > There is i915_semaphore_seqno_size define which the GEN8_WAIT_OFFSET > and GEN8_SIGNAL_OFFSET use. So rather use that to stay consistent. I didn't want the size, I wanted the type. What I really wanted were sensible macros... I can change this to i915_semaphore_seqno_size (even though that is not the right name) and void *. > Other than that, > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > PS. The existing semaphore code could use some cleanup, adding to > backlog. The existing semaphore code is 3 patches from working... 1. fix hw-id 2. fix signaling 3. reset pd after wait 4. enable/profit -Chris
On ke, 2016-04-06 at 11:10 +0100, Chris Wilson wrote: > On Wed, Apr 06, 2016 at 12:58:43PM +0300, Joonas Lahtinen wrote: > > > > On ke, 2016-04-06 at 00:57 +0100, Chris Wilson wrote: > > > > > > An oversight is that when we wrap the seqno, we need to reset the hw > > > semaphore counters to 0. We did this for gen6 and gen7 and forgot to do > > > so for the new implementation required for gen8 (legacy). > > > > > > 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/intel_ringbuffer.c | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > > > index fb304df8085d..c7023d6ca0b7 100644 > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > > @@ -2564,6 +2564,14 @@ void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno) > > > } > > > memset(engine->semaphore.sync_seqno, 0, > > > sizeof(engine->semaphore.sync_seqno)); > > > + if (dev_priv->semaphore_obj) { > > > + struct drm_i915_gem_object *obj = dev_priv->semaphore_obj; > > > + struct page *page = i915_gem_object_get_dirty_page(obj, 0); > > > + uint64_t *semaphores = kmap(page); > > > + memset(semaphores + engine->id * I915_NUM_ENGINES, 0, > > > + sizeof(*semaphores) * I915_NUM_ENGINES); > > There is i915_semaphore_seqno_size define which the GEN8_WAIT_OFFSET > > and GEN8_SIGNAL_OFFSET use. So rather use that to stay consistent. > I didn't want the size, I wanted the type. What I really wanted were > sensible macros... The macros could use some cleanup is what I meant. > > I can change this to i915_semaphore_seqno_size (even though that is not > the right name) and void *. > > > > > Other than that, > > > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > > > PS. The existing semaphore code could use some cleanup, adding to > > backlog. > The existing semaphore code is 3 patches from working... > 1. fix hw-id > 2. fix signaling > 3. reset pd after wait > 4. enable/profit > -Chris >
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index fb304df8085d..c7023d6ca0b7 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2564,6 +2564,14 @@ void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno) } memset(engine->semaphore.sync_seqno, 0, sizeof(engine->semaphore.sync_seqno)); + if (dev_priv->semaphore_obj) { + struct drm_i915_gem_object *obj = dev_priv->semaphore_obj; + struct page *page = i915_gem_object_get_dirty_page(obj, 0); + uint64_t *semaphores = kmap(page); + memset(semaphores + engine->id * I915_NUM_ENGINES, 0, + sizeof(*semaphores) * I915_NUM_ENGINES); + kunmap(page); + } engine->set_seqno(engine, seqno);
An oversight is that when we wrap the seqno, we need to reset the hw semaphore counters to 0. We did this for gen6 and gen7 and forgot to do so for the new implementation required for gen8 (legacy). 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/intel_ringbuffer.c | 8 ++++++++ 1 file changed, 8 insertions(+)