From: Chris Wilson Date: Tue, 18 Dec 2018 10:27:12 +0000 (+0000) Subject: drm/i915: Apply missed interrupt after reset w/a to all ringbuffer gen X-Git-Tag: v5.4-rc1~1388^2~28^2~69 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=060f23225d8203b8cd9e412d984e5237e63c83dc;p=platform%2Fkernel%2Flinux-rpi.git drm/i915: Apply missed interrupt after reset w/a to all ringbuffer gen Having completed a test run of gem_eio across all machines in CI we also observe the phenomenon (of lost interrupts after resetting the GPU) on gen3 machines as well as the previously sighted gen6/gen7. Let's apply the same HWSTAM workaround that was effective for gen6+ for all, as although we haven't seen the same failure on gen4/5 it seems prudent to keep the code the same. As a consequence we can remove the extra setting of HWSTAM and apply the register from a single site. v2: Delazy and move the HWSTAM into its own function v3: Mask off all HWSP writes on driver unload and engine cleanup. v4: And what about the physical hwsp? v5: No, engine->init_hw() is not called from driver_init_hw(), don't be daft. Really scrub HWSTAM as early as we can in driver_init_mmio() v6: Rename set_hwsp as it was setting the mask not the hwsp register. v7: Ville pointed out that although vcs(bsd) was introduced for g4x/ilk, per-engine HWSTAM was not introduced until gen6! References: https://bugs.freedesktop.org/show_bug.cgi?id=108735 Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Ville Syrjälä Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181218102712.11058-1-chris@chris-wilson.co.uk --- diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index e2dac9b..0c7fc98 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3586,9 +3586,6 @@ static void ironlake_irq_reset(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); - if (IS_GEN(dev_priv, 5)) - I915_WRITE(HWSTAM, 0xffffffff); - GEN3_IRQ_RESET(DE); if (IS_GEN(dev_priv, 7)) I915_WRITE(GEN7_ERR_INT, 0xffffffff); @@ -4368,8 +4365,6 @@ static void i8xx_irq_reset(struct drm_device *dev) i9xx_pipestat_irq_reset(dev_priv); - I915_WRITE16(HWSTAM, 0xffff); - GEN2_IRQ_RESET(); } @@ -4537,8 +4532,6 @@ static void i915_irq_reset(struct drm_device *dev) i9xx_pipestat_irq_reset(dev_priv); - I915_WRITE(HWSTAM, 0xffffffff); - GEN3_IRQ_RESET(); } @@ -4648,8 +4641,6 @@ static void i965_irq_reset(struct drm_device *dev) i9xx_pipestat_irq_reset(dev_priv); - I915_WRITE(HWSTAM, 0xffffffff); - GEN3_IRQ_RESET(); } diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 66d0ad9..561b474 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -261,6 +261,31 @@ static void __sprint_engine_name(char *name, const struct engine_info *info) info->instance) >= INTEL_ENGINE_CS_MAX_NAME); } +void intel_engine_set_hwsp_writemask(struct intel_engine_cs *engine, u32 mask) +{ + struct drm_i915_private *dev_priv = engine->i915; + i915_reg_t hwstam; + + /* + * Though they added more rings on g4x/ilk, they did not add + * per-engine HWSTAM until gen6. + */ + if (INTEL_GEN(dev_priv) < 6 && engine->class != RENDER_CLASS) + return; + + hwstam = RING_HWSTAM(engine->mmio_base); + if (INTEL_GEN(dev_priv) >= 3) + I915_WRITE(hwstam, mask); + else + I915_WRITE16(hwstam, mask); +} + +static void intel_engine_sanitize_mmio(struct intel_engine_cs *engine) +{ + /* Mask off all writes into the unknown HWSP */ + intel_engine_set_hwsp_writemask(engine, ~0u); +} + static int intel_engine_setup(struct drm_i915_private *dev_priv, enum intel_engine_id id) @@ -312,6 +337,9 @@ intel_engine_setup(struct drm_i915_private *dev_priv, ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier); + /* Scrub mmio state on takeover */ + intel_engine_sanitize_mmio(engine); + dev_priv->engine_class[info->class][info->instance] = engine; dev_priv->engine[id] = engine; return 0; @@ -495,6 +523,9 @@ void intel_engine_setup_common(struct intel_engine_cs *engine) static void cleanup_status_page(struct intel_engine_cs *engine) { + /* Prevent writes into HWSP after returning the page to the system */ + intel_engine_set_hwsp_writemask(engine, ~0u); + if (HWS_NEEDS_PHYSICAL(engine->i915)) { void *addr = fetch_and_zero(&engine->status_page.page_addr); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index f16fb30..4762c1e 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1655,7 +1655,7 @@ static void enable_execlists(struct intel_engine_cs *engine) { struct drm_i915_private *dev_priv = engine->i915; - I915_WRITE(RING_HWSTAM(engine->mmio_base), 0xffffffff); + intel_engine_set_hwsp_writemask(engine, ~0u); /* HWSTAM */ /* * Make sure we're not enabling the new 12-deep CSB diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index fdeca2b..65fd92e 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -379,11 +379,25 @@ gen7_render_ring_flush(struct i915_request *rq, u32 mode) return 0; } -static void ring_setup_phys_status_page(struct intel_engine_cs *engine) +static void set_hwstam(struct intel_engine_cs *engine, u32 mask) +{ + /* + * Keep the render interrupt unmasked as this papers over + * lost interrupts following a reset. + */ + if (engine->class == RENDER_CLASS) { + if (INTEL_GEN(engine->i915) >= 6) + mask &= ~BIT(0); + else + mask &= ~I915_USER_INTERRUPT; + } + + intel_engine_set_hwsp_writemask(engine, mask); +} + +static void set_hws_pga(struct intel_engine_cs *engine, phys_addr_t phys) { struct drm_i915_private *dev_priv = engine->i915; - struct page *page = virt_to_page(engine->status_page.page_addr); - phys_addr_t phys = PFN_PHYS(page_to_pfn(page)); u32 addr; addr = lower_32_bits(phys); @@ -393,12 +407,22 @@ static void ring_setup_phys_status_page(struct intel_engine_cs *engine) I915_WRITE(HWS_PGA, addr); } -static void intel_ring_setup_status_page(struct intel_engine_cs *engine) +static void ring_setup_phys_status_page(struct intel_engine_cs *engine) +{ + struct page *page = virt_to_page(engine->status_page.page_addr); + phys_addr_t phys = PFN_PHYS(page_to_pfn(page)); + + set_hws_pga(engine, phys); + set_hwstam(engine, ~0u); +} + +static void set_hwsp(struct intel_engine_cs *engine, u32 offset) { struct drm_i915_private *dev_priv = engine->i915; - i915_reg_t mmio; + i915_reg_t hwsp; - /* The ring status page addresses are no longer next to the rest of + /* + * The ring status page addresses are no longer next to the rest of * the ring registers as of gen7. */ if (IS_GEN(dev_priv, 7)) { @@ -410,56 +434,55 @@ static void intel_ring_setup_status_page(struct intel_engine_cs *engine) default: GEM_BUG_ON(engine->id); case RCS: - mmio = RENDER_HWS_PGA_GEN7; + hwsp = RENDER_HWS_PGA_GEN7; break; case BCS: - mmio = BLT_HWS_PGA_GEN7; + hwsp = BLT_HWS_PGA_GEN7; break; case VCS: - mmio = BSD_HWS_PGA_GEN7; + hwsp = BSD_HWS_PGA_GEN7; break; case VECS: - mmio = VEBOX_HWS_PGA_GEN7; + hwsp = VEBOX_HWS_PGA_GEN7; break; } } else if (IS_GEN(dev_priv, 6)) { - mmio = RING_HWS_PGA_GEN6(engine->mmio_base); + hwsp = RING_HWS_PGA_GEN6(engine->mmio_base); } else { - mmio = RING_HWS_PGA(engine->mmio_base); + hwsp = RING_HWS_PGA(engine->mmio_base); } - if (INTEL_GEN(dev_priv) >= 6) { - u32 mask = ~0u; + I915_WRITE(hwsp, offset); + POSTING_READ(hwsp); +} - /* - * Keep the render interrupt unmasked as this papers over - * lost interrupts following a reset. - */ - if (engine->id == RCS) - mask &= ~BIT(0); +static void flush_cs_tlb(struct intel_engine_cs *engine) +{ + struct drm_i915_private *dev_priv = engine->i915; + i915_reg_t instpm = RING_INSTPM(engine->mmio_base); - I915_WRITE(RING_HWSTAM(engine->mmio_base), mask); - } + if (!IS_GEN_RANGE(dev_priv, 6, 7)) + return; - I915_WRITE(mmio, engine->status_page.ggtt_offset); - POSTING_READ(mmio); + /* ring should be idle before issuing a sync flush*/ + WARN_ON((I915_READ_MODE(engine) & MODE_IDLE) == 0); - /* Flush the TLB for this page */ - if (IS_GEN_RANGE(dev_priv, 6, 7)) { - i915_reg_t reg = RING_INSTPM(engine->mmio_base); + I915_WRITE(instpm, + _MASKED_BIT_ENABLE(INSTPM_TLB_INVALIDATE | + INSTPM_SYNC_FLUSH)); + if (intel_wait_for_register(dev_priv, + instpm, INSTPM_SYNC_FLUSH, 0, + 1000)) + DRM_ERROR("%s: wait for SyncFlush to complete for TLB invalidation timed out\n", + engine->name); +} - /* ring should be idle before issuing a sync flush*/ - WARN_ON((I915_READ_MODE(engine) & MODE_IDLE) == 0); +static void ring_setup_status_page(struct intel_engine_cs *engine) +{ + set_hwsp(engine, engine->status_page.ggtt_offset); + set_hwstam(engine, ~0u); - I915_WRITE(reg, - _MASKED_BIT_ENABLE(INSTPM_TLB_INVALIDATE | - INSTPM_SYNC_FLUSH)); - if (intel_wait_for_register(dev_priv, - reg, INSTPM_SYNC_FLUSH, 0, - 1000)) - DRM_ERROR("%s: wait for SyncFlush to complete for TLB invalidation timed out\n", - engine->name); - } + flush_cs_tlb(engine); } static bool stop_ring(struct intel_engine_cs *engine) @@ -529,7 +552,7 @@ static int init_ring_common(struct intel_engine_cs *engine) if (HWS_NEEDS_PHYSICAL(dev_priv)) ring_setup_phys_status_page(engine); else - intel_ring_setup_status_page(engine); + ring_setup_status_page(engine); intel_engine_reset_breadcrumbs(engine); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 1ae74e5..6b41b9c 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -903,6 +903,8 @@ int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine); int intel_engine_stop_cs(struct intel_engine_cs *engine); void intel_engine_cancel_stop_cs(struct intel_engine_cs *engine); +void intel_engine_set_hwsp_writemask(struct intel_engine_cs *engine, u32 mask); + u64 intel_engine_get_active_head(const struct intel_engine_cs *engine); u64 intel_engine_get_last_batch_head(const struct intel_engine_cs *engine);