| From f48a01651b1758550c4d3ee65ec726dfa0658780 Mon Sep 17 00:00:00 2001 |
| From: David Woodhouse <dwmw2@infradead.org> |
| Date: Tue, 20 Jan 2015 17:21:42 +0000 |
| Subject: drm/i915: Init PPGTT before context enable |
| |
| From: David Woodhouse <dwmw2@infradead.org> |
| |
| commit f48a01651b1758550c4d3ee65ec726dfa0658780 upstream. |
| |
| Commit 82460d972 ("drm/i915: Rework ppgtt init to no require an aliasing |
| ppgtt") introduced a regression on Broadwell, triggering the following |
| IOMMU fault at startup: |
| |
| vgaarb: device changed decodes: PCI:0000:00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem |
| dmar: DRHD: handling fault status reg 2 |
| dmar: DMAR:[DMA Write] Request device [00:02.0] fault addr 880000 |
| DMAR:[fault reason 23] Unknown |
| fbcon: inteldrmfb (fb0) is primary device |
| |
| Further commentary from Daniel: |
| |
| I sugggested this change to David after staring at the offending patch |
| for a while. I have no idea and theory whatsoever why this would upset |
| the gpu less than the other way round. But it seems to work. David |
| promised to chase hw people a bit more to get a more meaningful answer. |
| |
| Wrt the comment that this deletes: I've done some digging and afaict |
| loading context before ppgtt enable was once required before our recent |
| restructuring of the context/ppgtt init code: Before that context sw |
| setup (i.e. allocating the default context) and hw setup was smashed |
| together. Also the setup of the default context was the bit that |
| actually allocated the aliasing ppgtt structures. Which is the reason |
| for the context before ppgtt depency. |
| |
| Or was, since with all the untangling there's no no real depency any |
| more (functional, who knows what the hw is doing), so the comment is |
| just stale. |
| |
| Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> |
| Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> |
| Signed-off-by: Jani Nikula <jani.nikula@intel.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/gpu/drm/i915/i915_gem.c | 19 ++++++------------- |
| 1 file changed, 6 insertions(+), 13 deletions(-) |
| |
| --- a/drivers/gpu/drm/i915/i915_gem.c |
| +++ b/drivers/gpu/drm/i915/i915_gem.c |
| @@ -4818,25 +4818,18 @@ i915_gem_init_hw(struct drm_device *dev) |
| for (i = 0; i < NUM_L3_SLICES(dev); i++) |
| i915_gem_l3_remap(&dev_priv->ring[RCS], i); |
| |
| - /* |
| - * XXX: Contexts should only be initialized once. Doing a switch to the |
| - * default context switch however is something we'd like to do after |
| - * reset or thaw (the latter may not actually be necessary for HW, but |
| - * goes with our code better). Context switching requires rings (for |
| - * the do_switch), but before enabling PPGTT. So don't move this. |
| - */ |
| - ret = i915_gem_context_enable(dev_priv); |
| + ret = i915_ppgtt_init_hw(dev); |
| if (ret && ret != -EIO) { |
| - DRM_ERROR("Context enable failed %d\n", ret); |
| + DRM_ERROR("PPGTT enable failed %d\n", ret); |
| i915_gem_cleanup_ringbuffer(dev); |
| - |
| - return ret; |
| } |
| |
| - ret = i915_ppgtt_init_hw(dev); |
| + ret = i915_gem_context_enable(dev_priv); |
| if (ret && ret != -EIO) { |
| - DRM_ERROR("PPGTT enable failed %d\n", ret); |
| + DRM_ERROR("Context enable failed %d\n", ret); |
| i915_gem_cleanup_ringbuffer(dev); |
| + |
| + return ret; |
| } |
| |
| return ret; |