* [PATCH 2/2] drm/i915/bdw: Setup global hardware status page in execlists mode
@ 2014-10-23 15:24 Thomas Daniel
2014-10-23 15:41 ` Chris Wilson
2014-10-24 8:14 ` Daniel Vetter
0 siblings, 2 replies; 6+ messages in thread
From: Thomas Daniel @ 2014-10-23 15:24 UTC (permalink / raw)
To: intel-gfx; +Cc: shuang.he
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
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 666cb28..ad36d66 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -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) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915/bdw: Setup global hardware status page in execlists mode
2014-10-23 15:24 [PATCH 2/2] drm/i915/bdw: Setup global hardware status page in execlists mode Thomas Daniel
@ 2014-10-23 15:41 ` Chris Wilson
2014-10-24 8:14 ` Daniel Vetter
1 sibling, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2014-10-23 15:41 UTC (permalink / raw)
To: Thomas Daniel; +Cc: intel-gfx, shuang.he
On Thu, Oct 23, 2014 at 04:24:46PM +0100, Thomas Daniel wrote:
> 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.
If only there was already a patch on the list that fixed this.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915/bdw: Setup global hardware status page in execlists mode
2014-10-23 15:24 [PATCH 2/2] drm/i915/bdw: Setup global hardware status page in execlists mode Thomas Daniel
2014-10-23 15:41 ` Chris Wilson
@ 2014-10-24 8:14 ` Daniel Vetter
2014-10-24 8:30 ` Daniel, Thomas
1 sibling, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2014-10-24 8:14 UTC (permalink / raw)
To: Thomas Daniel; +Cc: intel-gfx, shuang.he
On Thu, Oct 23, 2014 at 04:24:46PM +0100, Thomas Daniel wrote:
> 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
> Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 666cb28..ad36d66 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -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));
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
> }
>
> if (ring->id == RCS && !ctx->rcs_initialized) {
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915/bdw: Setup global hardware status page in execlists mode
2014-10-24 8:14 ` Daniel Vetter
@ 2014-10-24 8:30 ` Daniel, Thomas
2014-10-24 8:48 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Daniel, Thomas @ 2014-10-24 8:30 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx@lists.freedesktop.org
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Friday, October 24, 2014 9:15 AM
> To: Daniel, Thomas
> Cc: intel-gfx@lists.freedesktop.org; shuang.he@linux.intel.com
> 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
> the HWS_PGA register? 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.
> -Daniel
>
> > }
> >
> > if (ring->id == RCS && !ctx->rcs_initialized) {
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915/bdw: Setup global hardware status page in execlists mode
2014-10-24 8:30 ` Daniel, Thomas
@ 2014-10-24 8:48 ` Daniel Vetter
2014-10-24 9:36 ` Chris Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2014-10-24 8:48 UTC (permalink / raw)
To: Daniel, Thomas; +Cc: intel-gfx@lists.freedesktop.org
On Fri, Oct 24, 2014 at 08:30:56AM +0000, Daniel, Thomas wrote:
>
>
> > -----Original Message-----
> > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> > Vetter
> > Sent: Friday, October 24, 2014 9:15 AM
> > To: Daniel, Thomas
> > Cc: intel-gfx@lists.freedesktop.org; shuang.he@linux.intel.com
> > 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
> > the HWS_PGA register? 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.
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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915/bdw: Setup global hardware status page in execlists mode
2014-10-24 8:48 ` Daniel Vetter
@ 2014-10-24 9:36 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2014-10-24 9:36 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx@lists.freedesktop.org
On Fri, Oct 24, 2014 at 10:48:43AM +0200, Daniel Vetter wrote:
> On Fri, Oct 24, 2014 at 08:30:56AM +0000, Daniel, Thomas wrote:
> >
> >
> > > -----Original Message-----
> > > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> > > Vetter
> > > Sent: Friday, October 24, 2014 9:15 AM
> > > To: Daniel, Thomas
> > > Cc: intel-gfx@lists.freedesktop.org; shuang.he@linux.intel.com
> > > 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
> > > the HWS_PGA register? 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.
>
> 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.
For reference, you can look at http://patchwork.freedesktop.org/patch/33175/
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-24 9:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-23 15:24 [PATCH 2/2] drm/i915/bdw: Setup global hardware status page in execlists mode Thomas Daniel
2014-10-23 15:41 ` Chris Wilson
2014-10-24 8:14 ` Daniel Vetter
2014-10-24 8:30 ` Daniel, Thomas
2014-10-24 8:48 ` Daniel Vetter
2014-10-24 9:36 ` Chris Wilson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox