On Mon, Nov 25, 2013 at 09:54:33AM -0800, Ben Widawsky wrote: > The initial implementation of this function used MMIO to write the PDPs. > Upon review it was determined (correctly) that the docs say to use LRI. > The issue is there are times where we want to do a synchronous write > (GPU reset). > > I've tested this, and it works. I've verified with as many people as > possible that it should work. > > This should fix the failing reset problems. > > Signed-off-by: Ben Widawsky <ben@xxxxxxxxxxxx> This makes me nervous, for two reasons: - Historically we've _always_ ended up in hilarity, bloodshed and tears when having different setup/teardown code in the different parts of the driver. Always. - Looking at the reset code we should be initializing the rings before we try to re-enable ppgtt. So if I don't misread stuff horribly this needs at least one giant comment explaining what's exactly going on here - either the hw is busted or we have some other issue. Aside: Mika noticed that we're loosing a bunch of the w/a settings since a few of the per-ring workarounds have been errornously put into init_clock_gating. We need to move those to the correct per-ring init function. This is even more important once we have per-ring reset. Cheers, Daniel > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 1a5272c..96dbf3d 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -197,12 +197,19 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr, > > /* Broadwell Page Directory Pointer Descriptors */ > static int gen8_write_pdp(struct intel_ring_buffer *ring, unsigned entry, > - uint64_t val) > + uint64_t val, bool synchronous) > { > + struct drm_i915_private *dev_priv = ring->dev->dev_private; > int ret; > > BUG_ON(entry >= 4); > > + if (synchronous) { > + I915_WRITE(GEN8_RING_PDP_UDW(ring, entry), val >> 32); > + I915_WRITE(GEN8_RING_PDP_LDW(ring, entry), (u32)val); > + return 0; > + } > + > ret = intel_ring_begin(ring, 6); > if (ret) > return ret; > @@ -236,7 +243,8 @@ static int gen8_ppgtt_enable(struct drm_device *dev) > for (i = used_pd - 1; i >= 0; i--) { > dma_addr_t addr = ppgtt->pd_dma_addr[i]; > for_each_ring(ring, dev_priv, j) { > - ret = gen8_write_pdp(ring, i, addr); > + ret = gen8_write_pdp(ring, i, addr, > + i915_reset_in_progress(&dev_priv->gpu_error)); > if (ret) > goto err_out; > } > -- > 1.8.4.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx