Re: [PATCH] drm/i915: Initialize workarounds in logical ring mode too

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx