[Intel-gfx] [PATCH] drm/i915: Reset CSB read pointer when enabling contexts

[Daniel Vetter]

On Wed, Jan 07, 2015 at 04:09:30PM +0000, Thomas Daniel wrote:
> During a suspend/resume cycle the hardware Context Status Buffer write pointer
> is reset.  However since recent changes to the init sequence the software CSB
> read pointer is no longer reset.  This means that context status events are not
> handled correctly and new contexts are not written to the ELSP, resulting in an
> apparent GPU hang.
> 
> Pending further changes to the ring init code, just move the
> ring->next_context_status_buffer initialization into i915_gem_context_enable to
> fix this regression.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88097

Please cite the commit which introduced this regression (plus since it's
just the switch of defaults) also the commit which broke the execlist
code. And then add all the relevant ppl to the cc list of this patch.

> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Signed-off-by: Thomas Daniel <thomas.daniel at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c |    1 +
>  drivers/gpu/drm/i915/intel_lrc.c        |    1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index bf9778e..cc100c9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -412,6 +412,7 @@ int i915_gem_context_enable(struct drm_i915_private *dev_priv)
>  
>  	if (i915.enable_execlists) {
>  		for_each_ring(ring, dev_priv, i) {
> +			ring->next_context_status_buffer = 0;

Hm, shouldn't we instead move this to the ring->init_hw callback instead?
That was the split which broke things here after all, and imo it makes
more sense. Otherwise someone might reorder the setup sequence a bit and
get confused when the engine is enabled already but the sw side still has
bogus state ...

Thanks, Daniel

>  			if (ring->init_context) {
>  				ret = ring->init_context(ring,
>  						ring->default_context);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 7670a0f..4580267 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1394,7 +1394,6 @@ static int logical_ring_init(struct drm_device *dev, struct intel_engine_cs *rin
>  	INIT_LIST_HEAD(&ring->execlist_queue);
>  	INIT_LIST_HEAD(&ring->execlist_retired_req_list);
>  	spin_lock_init(&ring->execlist_lock);
> -	ring->next_context_status_buffer = 0;
>  
>  	ret = i915_cmd_parser_init_ring(ring);
>  	if (ret)
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch