Re: [PATCH v2] drm/i915: Reset CSB read pointer in ring init

[Dave Gordon <david.s.gordon@intel.com>]

On 09/01/15 11:09, Daniel, Thomas wrote:
> A previous commit enabled execlists by default:
> 
>        commit 27401d126b5b ("drm/i915/bdw: Enable execlists by default where supported")
> 
> This allowed routine testing of execlists which exposed a regression when
> resuming from suspend.  The cause was tracked down the to recent changes to the
> ring init sequence:
> 
>        commit 35a57ffbb108 ("drm/i915: Only init engines once")
> 
> During a suspend/resume cycle the hardware Context Status Buffer write pointer
> is reset.  However since the 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 gen8_init_common_ring to
> fix this regression.
> 
> v2: Moved init into gen8_init_common_ring rather than context_enable after
> feedback from Daniel Vetter.  Updated commit msg to reflect this and also cite
> commits related to the regression.  Fixed bz link to correct bug.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88096
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 7670a0f..e405b61 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1137,6 +1137,7 @@ static int gen8_init_common_ring(struct intel_engine_cs *ring)
>  		   _MASKED_BIT_DISABLE(GFX_REPLAY_MODE) |
>  		   _MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE));
>  	POSTING_READ(RING_MODE_GEN7(ring));
> +	ring->next_context_status_buffer = 0;
>  	DRM_DEBUG_DRIVER("Execlists enabled for %s\n", ring->name);
>  
>  	memset(&ring->hangcheck, 0, sizeof(ring->hangcheck));
> @@ -1394,7 +1395,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)
> 

LGTM.
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx