drm/i915/bdw: Setup global hardware status page in execlists mode
Subsystems (2)
PATCH 2/2 drm/i915/bdw: Setup global hardware status page in execlists mode
Write HWS_PGA address even in execlists mode as the global hardware status page is still required. This address was previously uninitialized and HWSP writes would clobber whatever buffer happened to reside at GGTT address 0.
Issue: VIZ-2020
--- drivers/gpu/drm/i915/intel_lrc.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/gpu/drm/i915/intel_lrc.c +5 -0
@@ -1678,6 +1678,7 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,uint32_t context_size;struct intel_ringbuffer *ringbuf;int ret; +struct drm_i915_private *dev_priv = dev->dev_private;WARN_ON (ctx->legacy_hw_ctx.rcs_state !=NULL );if (ctx->engine[ring->id].state)
@@ -1750,6 +1751,10 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,if (ring->status_page.page_addr ==NULL )return -ENOMEM ; ring->status_page.obj = ctx_obj; + +I915_WRITE (RING_HWS_PGA (ring->mmio_base), + (u32 )ring->status_page.gfx_addr); +POSTING_READ (RING_HWS_PGA (ring->mmio_base)); }if (ring->id ==RCS && !ctx->rcs_initialized) {
Signature · 1 line
1.7.9.5
Quoted 4 lines · Thomas Daniel
Write
HWS_PGAaddress even in execlists mode as the global hardware status page is still required. This address was previously uninitialized and HWSP writes would clobber whatever buffer happened to reside at GGTT address 0.
If only there was already a patch on the list that fixed this. -Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
Quoted 24 lines · Thomas Daniel
Write
HWS_PGAaddress even in execlists mode as the global hardware status page is still required. This address was previously uninitialized and HWSP writes would clobber whatever buffer happened to reside at GGTT address 0.Issue: VIZ-2020
Signed-off-by Thomas Daniel <[email protected]>--- drivers/gpu/drm/i915/intel_lrc.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/gpu/drm/i915/intel_lrc.c +1
-0@@ -1678,6 +1678,7 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,uint32_t context_size;struct intel_ringbuffer *ringbuf;int ret; +struct drm_i915_private *dev_priv = dev->dev_private;WARN_ON (ctx->legacy_hw_ctx.rcs_state !=NULL );if (ctx->engine[ring->id].state)
drivers/gpu/drm/i915/intel_lrc.c int intel_lr_context_deferred_create(struct intel_context *ctx, · line 1757
Quoted 7 lines
drivers/gpu/drm/i915/intel_lrc.c +4
-0@@ -1750,6 +1751,10 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,if (ring->status_page.page_addr ==NULL )return -ENOMEM ; ring->status_page.obj = ctx_obj; + +I915_WRITE (RING_HWS_PGA (ring->mmio_base), + (u32 )ring->status_page.gfx_addr); +POSTING_READ (RING_HWS_PGA (ring->mmio_base));
So every time a random new contexts gets created we write a new value into the HWS_PGA register? Shouldn't this only be done when we set up the default/system context?
-Daniel
Quoted 10 lines
drivers/gpu/drm/i915/intel_lrc.c +0
-0}if (ring->id ==RCS && !ctx->rcs_initialized) {Signature · 6 lines
1.7.9.5 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Quoted 13 lines
-----Original Message-----
From: Daniel Vetter [mailto:[email protected]] On Behalf Of Daniel Vetter
Sent: Friday, October 24, 2014 9:15 AM
To: Daniel, Thomas
Cc: [email protected]; [email protected]
Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/bdw: Setup global hardware status page in execlists mode
So every time a random new contexts gets created we write a new value into theHWS_PGAregister? Shouldn't this only be done when we set up the default/system context?
The write is inside an if(ctx == ring->default_ctx) block. We only write when the default context is created.
Thomas.
Quoted 17 lines
-Daniel
Quoted 10 lines
drivers/gpu/drm/i915/intel_lrc.c +0
-1}if (ring->id ==RCS && !ctx->rcs_initialized) { --1.7.9.5
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Quoted 13 lines · Thomas
Quoted 13 lines
-----Original Message-----
From: Daniel Vetter [mailto:[email protected]] On Behalf Of Daniel Vetter
Sent: Friday, October 24, 2014 9:15 AM
To: Daniel, Thomas
Cc: [email protected]; [email protected]
Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/bdw: Setup global hardware status page in execlists mode
So every time a random new contexts gets created we write a new value into theHWS_PGAregister? Shouldn't this only be done when we set up the default/system context?
Quoted 2 lines
The write is inside an
if(ctx == ring->default_ctx)block. We only write when the default context is created.
Hm right, confusing diff context. And I guess we can't move that out to logical_ring_init since it must happen before we init the render ring?
I think extracting this entire block into a setup_global_hws or so helper function would be useful for clarity.
-Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Quoted 14 lines · Daniel Vetter
Quoted 13 lines · Thomas
Quoted 13 lines
-----Original Message-----
From: Daniel Vetter [mailto:[email protected]] On Behalf Of Daniel Vetter
Sent: Friday, October 24, 2014 9:15 AM
To: Daniel, Thomas
Cc: [email protected]; [email protected]
Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/bdw: Setup global hardware status page in execlists mode
So every time a random new contexts gets created we write a new value into theHWS_PGAregister? Shouldn't this only be done when we set up the default/system context?
Quoted 8 lines
Quoted 2 lines
The write is inside an
if(ctx == ring->default_ctx)block. We only write when the default context is created.Hm right, confusing diff context. And I guess we can't move that out to
logical_ring_initsince it must happen before we init the render ring?I think extracting this entire block into a
setup_global_hwsor so helper function would be useful for clarity.
For reference, you can look at http://patchwork.freedesktop.org/patch/33175/
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre