| Message ID | 1414077886-19017-1-git-send-email-thomas.daniel@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
> -----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
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
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
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) {
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(+)