| Message ID | 1414516762-27254-1-git-send-email-arun.siluvery@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Arun Siluvery <arun.siluvery@linux.intel.com> writes: > From: Michel Thierry <michel.thierry@intel.com> > > Following the legacy ring submission example, update the > ring->init_context() hook to support the execlist submission mode. > > v2: update to use the new workaround macros and cleanup unused code. > This takes care of both bdw and chv workarounds. > > v2.1: Add missing call to init_context() during deferred context creation. > > Issue: VIZ-4092 > Issue: GMIN-3475 > Change-Id: Ie3d093b2542ab0e2a44b90460533e2f979788d6c > Cc: Deepak S <deepak.s@intel.com> > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_gem_context.c | 2 +- > drivers/gpu/drm/i915/intel_lrc.c | 46 ++++++++++++++++++++++++++++++++- > drivers/gpu/drm/i915/intel_ringbuffer.c | 5 ++-- > drivers/gpu/drm/i915/intel_ringbuffer.h | 4 ++- > 4 files changed, 52 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index a5221d8..a37668f 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -629,7 +629,7 @@ done: > > if (uninitialized) { > if (ring->init_context) { > - ret = ring->init_context(ring); > + ret = ring->init_context(ring->buffer); > if (ret) > DRM_ERROR("ring init context: %d\n", ret); > } > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index cd74e5c..f3efdbd 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -991,6 +991,43 @@ int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, int num_dwords) > return 0; > } > > +static int intel_logical_ring_workarounds_emit(struct intel_ringbuffer *ringbuf) > +{ > + int ret, i; > + struct intel_engine_cs *ring = ringbuf->ring; > + struct drm_device *dev = ring->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct i915_workarounds *w = &dev_priv->workarounds; > + > + if (WARN_ON(w->count == 0)) > + return 0; > + > + ring->gpu_caches_dirty = true; > + ret = logical_ring_flush_all_caches(ringbuf); > + if (ret) > + return ret; > + > + ret = intel_logical_ring_begin(ringbuf, w->count * 2 + 2); > + if (ret) > + return ret; > + > + intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(w->count)); > + for (i = 0; i < w->count; i++) { > + intel_logical_ring_emit(ringbuf, w->reg[i].addr); > + intel_logical_ring_emit(ringbuf, w->reg[i].value); > + } > + intel_logical_ring_emit(ringbuf, MI_NOOP); > + > + intel_logical_ring_advance(ringbuf); > + > + ring->gpu_caches_dirty = true; > + ret = logical_ring_flush_all_caches(ringbuf); > + if (ret) > + return ret; > + > + return 0; > +} > + > static int gen8_init_common_ring(struct intel_engine_cs *ring) > { > struct drm_device *dev = ring->dev; > @@ -1034,7 +1071,7 @@ static int gen8_init_render_ring(struct intel_engine_cs *ring) > > I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING)); > > - return ret; > + return init_workarounds_ring(ring); > } > > static int gen8_emit_bb_start(struct intel_ringbuffer *ringbuf, > @@ -1282,6 +1319,7 @@ static int logical_render_ring_init(struct drm_device *dev) > ring->irq_keep_mask |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT; > > ring->init = gen8_init_render_ring; > + ring->init_context = intel_logical_ring_workarounds_emit; > ring->cleanup = intel_fini_pipe_control; > ring->get_seqno = gen8_get_seqno; > ring->set_seqno = gen8_set_seqno; > @@ -1745,6 +1783,12 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, > } > > if (ring->id == RCS && !ctx->rcs_initialized) { > + if (ring->init_context) { > + ret = ring->init_context(ringbuf); > + if (ret) > + DRM_ERROR("ring init context: %d\n", ret); > + } > + > ret = intel_lr_context_render_state_init(ring, ctx); > if (ret) { > DRM_ERROR("Init render state failed: %d\n", ret); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 368b20a..70442fa 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -665,9 +665,10 @@ err: > return ret; > } > > -static int intel_ring_workarounds_emit(struct intel_engine_cs *ring) > +static int intel_ring_workarounds_emit(struct intel_ringbuffer *ringbuf) > { > int ret, i; > + struct intel_engine_cs *ring = ringbuf->ring; > struct drm_device *dev = ring->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct i915_workarounds *w = &dev_priv->workarounds; > @@ -806,7 +807,7 @@ static int chv_init_workarounds(struct intel_engine_cs *ring) > return 0; > } > > -static int init_workarounds_ring(struct intel_engine_cs *ring) > +int init_workarounds_ring(struct intel_engine_cs *ring) > { > struct drm_device *dev = ring->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 96479c8..a69ef8b 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -148,7 +148,7 @@ struct intel_engine_cs { > > int (*init)(struct intel_engine_cs *ring); > > - int (*init_context)(struct intel_engine_cs *ring); > + int (*init_context)(struct intel_ringbuffer *ringbuf); What is the rationale of this? This seems odd. Why not: int (*init_context)(struct intel_engine_cs *ring, struct intel_context *ctx); -Mika > void (*write_tail)(struct intel_engine_cs *ring, > u32 value); > @@ -424,6 +424,8 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev); > u64 intel_ring_get_active_head(struct intel_engine_cs *ring); > void intel_ring_setup_status_page(struct intel_engine_cs *ring); > > +int init_workarounds_ring(struct intel_engine_cs *ring); > + > static inline u32 intel_ring_get_tail(struct intel_ringbuffer *ringbuf) > { > return ringbuf->tail; > -- > 2.1.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 11/10/2014 4:38 PM, Mika Kuoppala wrote: > Arun Siluvery <arun.siluvery@linux.intel.com> writes: > >> From: Michel Thierry <michel.thierry@intel.com> >> >> Following the legacy ring submission example, update the >> ring->init_context() hook to support the execlist submission mode. >> >> v2: update to use the new workaround macros and cleanup unused code. >> This takes care of both bdw and chv workarounds. >> >> v2.1: Add missing call to init_context() during deferred context creation. >> >> Issue: VIZ-4092 >> Issue: GMIN-3475 >> Change-Id: Ie3d093b2542ab0e2a44b90460533e2f979788d6c >> Cc: Deepak S <deepak.s@intel.com> >> Signed-off-by: Michel Thierry <michel.thierry@intel.com> >> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> >> --- >> drivers/gpu/drm/i915/i915_gem_context.c | 2 +- >> drivers/gpu/drm/i915/intel_lrc.c | 46 ++++++++++++++++++++++++++++++++- >> drivers/gpu/drm/i915/intel_ringbuffer.c | 5 ++-- >> drivers/gpu/drm/i915/intel_ringbuffer.h | 4 ++- >> 4 files changed, 52 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c >> index a5221d8..a37668f 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_context.c >> +++ b/drivers/gpu/drm/i915/i915_gem_context.c >> @@ -629,7 +629,7 @@ done: >> >> if (uninitialized) { >> if (ring->init_context) { >> - ret = ring->init_context(ring); >> + ret = ring->init_context(ring->buffer); >> if (ret) >> DRM_ERROR("ring init context: %d\n", ret); >> } >> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c >> index cd74e5c..f3efdbd 100644 >> --- a/drivers/gpu/drm/i915/intel_lrc.c >> +++ b/drivers/gpu/drm/i915/intel_lrc.c >> @@ -991,6 +991,43 @@ int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, int num_dwords) >> return 0; >> } >> >> +static int intel_logical_ring_workarounds_emit(struct intel_ringbuffer *ringbuf) >> +{ >> + int ret, i; >> + struct intel_engine_cs *ring = ringbuf->ring; >> + struct drm_device *dev = ring->dev; >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + struct i915_workarounds *w = &dev_priv->workarounds; >> + >> + if (WARN_ON(w->count == 0)) >> + return 0; >> + >> + ring->gpu_caches_dirty = true; >> + ret = logical_ring_flush_all_caches(ringbuf); >> + if (ret) >> + return ret; >> + >> + ret = intel_logical_ring_begin(ringbuf, w->count * 2 + 2); >> + if (ret) >> + return ret; >> + >> + intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(w->count)); >> + for (i = 0; i < w->count; i++) { >> + intel_logical_ring_emit(ringbuf, w->reg[i].addr); >> + intel_logical_ring_emit(ringbuf, w->reg[i].value); >> + } >> + intel_logical_ring_emit(ringbuf, MI_NOOP); >> + >> + intel_logical_ring_advance(ringbuf); >> + >> + ring->gpu_caches_dirty = true; >> + ret = logical_ring_flush_all_caches(ringbuf); >> + if (ret) >> + return ret; >> + >> + return 0; >> +} >> + >> static int gen8_init_common_ring(struct intel_engine_cs *ring) >> { >> struct drm_device *dev = ring->dev; >> @@ -1034,7 +1071,7 @@ static int gen8_init_render_ring(struct intel_engine_cs *ring) >> >> I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING)); >> >> - return ret; >> + return init_workarounds_ring(ring); >> } >> >> static int gen8_emit_bb_start(struct intel_ringbuffer *ringbuf, >> @@ -1282,6 +1319,7 @@ static int logical_render_ring_init(struct drm_device *dev) >> ring->irq_keep_mask |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT; >> >> ring->init = gen8_init_render_ring; >> + ring->init_context = intel_logical_ring_workarounds_emit; >> ring->cleanup = intel_fini_pipe_control; >> ring->get_seqno = gen8_get_seqno; >> ring->set_seqno = gen8_set_seqno; >> @@ -1745,6 +1783,12 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, >> } >> >> if (ring->id == RCS && !ctx->rcs_initialized) { >> + if (ring->init_context) { >> + ret = ring->init_context(ringbuf); >> + if (ret) >> + DRM_ERROR("ring init context: %d\n", ret); >> + } >> + >> ret = intel_lr_context_render_state_init(ring, ctx); >> if (ret) { >> DRM_ERROR("Init render state failed: %d\n", ret); >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c >> index 368b20a..70442fa 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c >> @@ -665,9 +665,10 @@ err: >> return ret; >> } >> >> -static int intel_ring_workarounds_emit(struct intel_engine_cs *ring) >> +static int intel_ring_workarounds_emit(struct intel_ringbuffer *ringbuf) >> { >> int ret, i; >> + struct intel_engine_cs *ring = ringbuf->ring; >> struct drm_device *dev = ring->dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct i915_workarounds *w = &dev_priv->workarounds; >> @@ -806,7 +807,7 @@ static int chv_init_workarounds(struct intel_engine_cs *ring) >> return 0; >> } >> >> -static int init_workarounds_ring(struct intel_engine_cs *ring) >> +int init_workarounds_ring(struct intel_engine_cs *ring) >> { >> struct drm_device *dev = ring->dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h >> index 96479c8..a69ef8b 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h >> @@ -148,7 +148,7 @@ struct intel_engine_cs { >> >> int (*init)(struct intel_engine_cs *ring); >> >> - int (*init_context)(struct intel_engine_cs *ring); >> + int (*init_context)(struct intel_ringbuffer *ringbuf); > > What is the rationale of this? This seems odd. > > Why not: > > int (*init_context)(struct intel_engine_cs *ring, > struct intel_context *ctx); > > -Mika > We want to have the emit function for both legacy and execlist submission modes (intel_ring_workarounds_emit & intel_logical_ring_workarounds_emit). It's easy to get the ring from a ringbuf, but not the other way around. -Michel
On Tue, Nov 11, 2014 at 11:34:58AM +0000, Michel Thierry wrote: > On 11/10/2014 4:38 PM, Mika Kuoppala wrote: > >>- int (*init_context)(struct intel_engine_cs *ring); > >>+ int (*init_context)(struct intel_ringbuffer *ringbuf); > >What is the rationale of this? This seems odd. > > > >Why not: > > > >int (*init_context)(struct intel_engine_cs *ring, > > struct intel_context *ctx); > > > >-Mika > > We want to have the emit function for both legacy and execlist submission > modes (intel_ring_workarounds_emit & intel_logical_ring_workarounds_emit). > It's easy to get the ring from a ringbuf, but not the other way around. ctx already has the ringbuf pointer for execlist that you are looking for. And for legacy contexts there's only one, per-engine. So from that quick look Mika's interface proposal does make more sense to me. -Daneil
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index a5221d8..a37668f 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -629,7 +629,7 @@ done: if (uninitialized) { if (ring->init_context) { - ret = ring->init_context(ring); + ret = ring->init_context(ring->buffer); if (ret) DRM_ERROR("ring init context: %d\n", ret); } diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index cd74e5c..f3efdbd 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -991,6 +991,43 @@ int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, int num_dwords) return 0; } +static int intel_logical_ring_workarounds_emit(struct intel_ringbuffer *ringbuf) +{ + int ret, i; + struct intel_engine_cs *ring = ringbuf->ring; + struct drm_device *dev = ring->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct i915_workarounds *w = &dev_priv->workarounds; + + if (WARN_ON(w->count == 0)) + return 0; + + ring->gpu_caches_dirty = true; + ret = logical_ring_flush_all_caches(ringbuf); + if (ret) + return ret; + + ret = intel_logical_ring_begin(ringbuf, w->count * 2 + 2); + if (ret) + return ret; + + intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(w->count)); + for (i = 0; i < w->count; i++) { + intel_logical_ring_emit(ringbuf, w->reg[i].addr); + intel_logical_ring_emit(ringbuf, w->reg[i].value); + } + intel_logical_ring_emit(ringbuf, MI_NOOP); + + intel_logical_ring_advance(ringbuf); + + ring->gpu_caches_dirty = true; + ret = logical_ring_flush_all_caches(ringbuf); + if (ret) + return ret; + + return 0; +} + static int gen8_init_common_ring(struct intel_engine_cs *ring) { struct drm_device *dev = ring->dev; @@ -1034,7 +1071,7 @@ static int gen8_init_render_ring(struct intel_engine_cs *ring) I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING)); - return ret; + return init_workarounds_ring(ring); } static int gen8_emit_bb_start(struct intel_ringbuffer *ringbuf, @@ -1282,6 +1319,7 @@ static int logical_render_ring_init(struct drm_device *dev) ring->irq_keep_mask |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT; ring->init = gen8_init_render_ring; + ring->init_context = intel_logical_ring_workarounds_emit; ring->cleanup = intel_fini_pipe_control; ring->get_seqno = gen8_get_seqno; ring->set_seqno = gen8_set_seqno; @@ -1745,6 +1783,12 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, } if (ring->id == RCS && !ctx->rcs_initialized) { + if (ring->init_context) { + ret = ring->init_context(ringbuf); + if (ret) + DRM_ERROR("ring init context: %d\n", ret); + } + ret = intel_lr_context_render_state_init(ring, ctx); if (ret) { DRM_ERROR("Init render state failed: %d\n", ret); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 368b20a..70442fa 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -665,9 +665,10 @@ err: return ret; } -static int intel_ring_workarounds_emit(struct intel_engine_cs *ring) +static int intel_ring_workarounds_emit(struct intel_ringbuffer *ringbuf) { int ret, i; + struct intel_engine_cs *ring = ringbuf->ring; struct drm_device *dev = ring->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct i915_workarounds *w = &dev_priv->workarounds; @@ -806,7 +807,7 @@ static int chv_init_workarounds(struct intel_engine_cs *ring) return 0; } -static int init_workarounds_ring(struct intel_engine_cs *ring) +int init_workarounds_ring(struct intel_engine_cs *ring) { struct drm_device *dev = ring->dev; struct drm_i915_private *dev_priv = dev->dev_private; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 96479c8..a69ef8b 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -148,7 +148,7 @@ struct intel_engine_cs { int (*init)(struct intel_engine_cs *ring); - int (*init_context)(struct intel_engine_cs *ring); + int (*init_context)(struct intel_ringbuffer *ringbuf); void (*write_tail)(struct intel_engine_cs *ring, u32 value); @@ -424,6 +424,8 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev); u64 intel_ring_get_active_head(struct intel_engine_cs *ring); void intel_ring_setup_status_page(struct intel_engine_cs *ring); +int init_workarounds_ring(struct intel_engine_cs *ring); + static inline u32 intel_ring_get_tail(struct intel_ringbuffer *ringbuf) { return ringbuf->tail;