On 11/10/2014 4:38 PM, Mika Kuoppala wrote: > Arun Siluvery writes: > >> From: Michel Thierry >> >> 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 >> Signed-off-by: Michel Thierry >> Signed-off-by: Arun Siluvery >> --- >> 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