| Message ID | 1372849014-4196-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 03, 2013 at 12:56:54PM +0200, Daniel Vetter wrote: > This fixes gpu reset on my gm45 - without this patch the bsd thing is > forever stuck since the seqno updates never reach the status page. > > Tbh I have no idea how this ever worked without rewriting the hws > registers after a gpu reset. > > To satisfy my OCD also give the functions a bit more consistent names: > - Use status_page everywhere, also for the physical addressed one. > - Use init for the allocation part and setup for the register setup > part consistently. > > Long term I'd really like to share the hw init parts completely > between gpu reset, resume and driver load, i.e. to call > i915_gem_init_hw instead of the individual pieces we might need. > > v2: Add the missing paragraph to the commit message about what bug > exactly this patch here fixes. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65495 > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Lots of low hanging fruit here for designating the phases of initialisation and what is required for reinitialisation, definitely requires a good renaming session and cleanup. Well, I was hoping that this would also help bug 64725, but alas upon resume we do the full init, rather than the shortcut for reset. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Wed, Jul 03, 2013 at 12:13:44PM +0100, Chris Wilson wrote: > On Wed, Jul 03, 2013 at 12:56:54PM +0200, Daniel Vetter wrote: > > This fixes gpu reset on my gm45 - without this patch the bsd thing is > > forever stuck since the seqno updates never reach the status page. > > > > Tbh I have no idea how this ever worked without rewriting the hws > > registers after a gpu reset. > > > > To satisfy my OCD also give the functions a bit more consistent names: > > - Use status_page everywhere, also for the physical addressed one. > > - Use init for the allocation part and setup for the register setup > > part consistently. > > > > Long term I'd really like to share the hw init parts completely > > between gpu reset, resume and driver load, i.e. to call > > i915_gem_init_hw instead of the individual pieces we might need. > > > > v2: Add the missing paragraph to the commit message about what bug > > exactly this patch here fixes. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65495 > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Lots of low hanging fruit here for designating the phases of > initialisation and what is required for reinitialisation, definitely > requires a good renaming session and cleanup. > > Well, I was hoping that this would also help bug 64725, but alas upon > resume we do the full init, rather than the shortcut for reset. > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Picked up for -fixes, thanks for the review. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index e51ab55..18ca76e 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -379,6 +379,17 @@ u32 intel_ring_get_active_head(struct intel_ring_buffer *ring) return I915_READ(acthd_reg); } +static void ring_setup_phys_status_page(struct intel_ring_buffer *ring) +{ + struct drm_i915_private *dev_priv = ring->dev->dev_private; + u32 addr; + + addr = dev_priv->status_page_dmah->busaddr; + if (INTEL_INFO(ring->dev)->gen >= 4) + addr |= (dev_priv->status_page_dmah->busaddr >> 28) & 0xf0; + I915_WRITE(HWS_PGA, addr); +} + static int init_ring_common(struct intel_ring_buffer *ring) { struct drm_device *dev = ring->dev; @@ -390,6 +401,11 @@ static int init_ring_common(struct intel_ring_buffer *ring) if (HAS_FORCE_WAKE(dev)) gen6_gt_force_wake_get(dev_priv); + if (I915_NEED_GFX_HWS(dev)) + intel_ring_setup_status_page(ring); + else + ring_setup_phys_status_page(ring); + /* Stop the ring if it's running. */ I915_WRITE_CTL(ring, 0); I915_WRITE_HEAD(ring, 0); @@ -1223,7 +1239,6 @@ static int init_status_page(struct intel_ring_buffer *ring) ring->status_page.obj = obj; memset(ring->status_page.page_addr, 0, PAGE_SIZE); - intel_ring_setup_status_page(ring); DRM_DEBUG_DRIVER("%s hws offset: 0x%08x\n", ring->name, ring->status_page.gfx_addr); @@ -1237,10 +1252,9 @@ err: return ret; } -static int init_phys_hws_pga(struct intel_ring_buffer *ring) +static int init_phys_status_page(struct intel_ring_buffer *ring) { struct drm_i915_private *dev_priv = ring->dev->dev_private; - u32 addr; if (!dev_priv->status_page_dmah) { dev_priv->status_page_dmah = @@ -1249,11 +1263,6 @@ static int init_phys_hws_pga(struct intel_ring_buffer *ring) return -ENOMEM; } - addr = dev_priv->status_page_dmah->busaddr; - if (INTEL_INFO(ring->dev)->gen >= 4) - addr |= (dev_priv->status_page_dmah->busaddr >> 28) & 0xf0; - I915_WRITE(HWS_PGA, addr); - ring->status_page.page_addr = dev_priv->status_page_dmah->vaddr; memset(ring->status_page.page_addr, 0, PAGE_SIZE); @@ -1281,7 +1290,7 @@ static int intel_init_ring_buffer(struct drm_device *dev, return ret; } else { BUG_ON(ring->id != RCS); - ret = init_phys_hws_pga(ring); + ret = init_phys_status_page(ring); if (ret) return ret; } @@ -1893,7 +1902,7 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size) } if (!I915_NEED_GFX_HWS(dev)) { - ret = init_phys_hws_pga(ring); + ret = init_phys_status_page(ring); if (ret) return ret; }
This fixes gpu reset on my gm45 - without this patch the bsd thing is forever stuck since the seqno updates never reach the status page. Tbh I have no idea how this ever worked without rewriting the hws registers after a gpu reset. To satisfy my OCD also give the functions a bit more consistent names: - Use status_page everywhere, also for the physical addressed one. - Use init for the allocation part and setup for the register setup part consistently. Long term I'd really like to share the hw init parts completely between gpu reset, resume and driver load, i.e. to call i915_gem_init_hw instead of the individual pieces we might need. v2: Add the missing paragraph to the commit message about what bug exactly this patch here fixes. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65495 Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/intel_ringbuffer.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)