From d7d4eeddb8f72342f70621c4b3cb718af9361712 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 17 Oct 2012 12:09:54 +0100 Subject: drm/i915: Allow DRM_ROOT_ONLY|DRM_MASTER to submit privileged batchbuffers With the introduction of per-process GTT space, the hardware designers thought it wise to also limit the ability to write to MMIO space to only a "secure" batch buffer. The ability to rewrite registers is the only way to program the hardware to perform certain operations like scanline waits (required for tear-free windowed updates). So we either have a choice of adding an interface to perform those synchronized updates inside the kernel, or we permit certain processes the ability to write to the "safe" registers from within its command stream. This patch exposes the ability to submit a SECURE batch buffer to DRM_ROOT_ONLY|DRM_MASTER processes. v2: Haswell split up bit8 into a ppgtt bit (still bit8) and a security bit (bit 13, accidentally not set). Also add a comment explaining why secure batches need a global gtt binding. Signed-off-by: Chris Wilson (v1) [danvet: added hsw fixup.] Reviewed-by: Jesse Barnes Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 48 ++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 9 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 984a0c5fbf5d..6c6f95a534b1 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -965,7 +965,9 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring) } static int -i965_dispatch_execbuffer(struct intel_ring_buffer *ring, u32 offset, u32 length) +i965_dispatch_execbuffer(struct intel_ring_buffer *ring, + u32 offset, u32 length, + unsigned flags) { int ret; @@ -976,7 +978,7 @@ i965_dispatch_execbuffer(struct intel_ring_buffer *ring, u32 offset, u32 length) intel_ring_emit(ring, MI_BATCH_BUFFER_START | MI_BATCH_GTT | - MI_BATCH_NON_SECURE_I965); + (flags & I915_DISPATCH_SECURE ? 0 : MI_BATCH_NON_SECURE_I965)); intel_ring_emit(ring, offset); intel_ring_advance(ring); @@ -985,7 +987,8 @@ i965_dispatch_execbuffer(struct intel_ring_buffer *ring, u32 offset, u32 length) static int i830_dispatch_execbuffer(struct intel_ring_buffer *ring, - u32 offset, u32 len) + u32 offset, u32 len, + unsigned flags) { int ret; @@ -994,7 +997,7 @@ i830_dispatch_execbuffer(struct intel_ring_buffer *ring, return ret; intel_ring_emit(ring, MI_BATCH_BUFFER); - intel_ring_emit(ring, offset | MI_BATCH_NON_SECURE); + intel_ring_emit(ring, offset | (flags & I915_DISPATCH_SECURE ? 0 : MI_BATCH_NON_SECURE)); intel_ring_emit(ring, offset + len - 8); intel_ring_emit(ring, 0); intel_ring_advance(ring); @@ -1004,7 +1007,8 @@ i830_dispatch_execbuffer(struct intel_ring_buffer *ring, static int i915_dispatch_execbuffer(struct intel_ring_buffer *ring, - u32 offset, u32 len) + u32 offset, u32 len, + unsigned flags) { int ret; @@ -1013,7 +1017,7 @@ i915_dispatch_execbuffer(struct intel_ring_buffer *ring, return ret; intel_ring_emit(ring, MI_BATCH_BUFFER_START | MI_BATCH_GTT); - intel_ring_emit(ring, offset | MI_BATCH_NON_SECURE); + intel_ring_emit(ring, offset | (flags & I915_DISPATCH_SECURE ? 0 : MI_BATCH_NON_SECURE)); intel_ring_advance(ring); return 0; @@ -1402,9 +1406,31 @@ static int gen6_ring_flush(struct intel_ring_buffer *ring, return 0; } +static int +hsw_ring_dispatch_execbuffer(struct intel_ring_buffer *ring, + u32 offset, u32 len, + unsigned flags) +{ + int ret; + + ret = intel_ring_begin(ring, 2); + if (ret) + return ret; + + intel_ring_emit(ring, + MI_BATCH_BUFFER_START | MI_BATCH_PPGTT_HSW | + (flags & I915_DISPATCH_SECURE ? 0 : MI_BATCH_NON_SECURE_HSW)); + /* bit0-7 is the length on GEN6+ */ + intel_ring_emit(ring, offset); + intel_ring_advance(ring); + + return 0; +} + static int gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring, - u32 offset, u32 len) + u32 offset, u32 len, + unsigned flags) { int ret; @@ -1412,7 +1438,9 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring, if (ret) return ret; - intel_ring_emit(ring, MI_BATCH_BUFFER_START | MI_BATCH_NON_SECURE_I965); + intel_ring_emit(ring, + MI_BATCH_BUFFER_START | + (flags & I915_DISPATCH_SECURE ? 0 : MI_BATCH_NON_SECURE_I965)); /* bit0-7 is the length on GEN6+ */ intel_ring_emit(ring, offset); intel_ring_advance(ring); @@ -1491,7 +1519,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev) ring->irq_enable_mask = I915_USER_INTERRUPT; } ring->write_tail = ring_write_tail; - if (INTEL_INFO(dev)->gen >= 6) + if (IS_HASWELL(dev)) + ring->dispatch_execbuffer = hsw_ring_dispatch_execbuffer; + else if (INTEL_INFO(dev)->gen >= 6) ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer; else if (INTEL_INFO(dev)->gen >= 4) ring->dispatch_execbuffer = i965_dispatch_execbuffer; -- cgit v1.2.2 From 17f10fdc010254b8e9c0f1779abdaaee4757cabf Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Mon, 29 Oct 2012 16:59:26 +0200 Subject: drm/i915/ringbuffer: exclude last 2 cachelines on 845g on all callpaths Make intel_render_ring_init_dri and intel_init_ring_buffer symmetrical with regards of workaround introduced by: commit 27c1cbd06a7620b354cbb363834f3bb8df4f410d Author: Chris Wilson Date: Mon Apr 9 13:59:46 2012 +0100 drm/i915/ringbuffer: Exclude last 2 cachlines of ring on 845g Signed-off-by: Mika Kuoppala Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 785df4fbff21..b13393b593b8 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1590,7 +1590,7 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size) ring->size = size; ring->effective_size = ring->size; - if (IS_I830(ring->dev)) + if (IS_I830(ring->dev) || IS_845G(ring->dev)) ring->effective_size -= 128; ring->virtual_start = ioremap_wc(start, size); -- cgit v1.2.2 From 9a28977181724ebbd9bdc45291cf29da55a729ee Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Fri, 26 Oct 2012 09:42:42 -0700 Subject: drm/i915: TLB invalidation with MI_FLUSH_DW requires a post-sync op v3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So store into the scratch space of the HWS to make sure the invalidate occurs. v2: use GTT address space for store, clean up #defines (Chris) v3: use correct #define in blt ring flush (Chris) Signed-off-by: Jesse Barnes Reviewed-by: Antti Koskipää Reviewed-by: Chris Wilson References: https://bugs.launchpad.net/ubuntu/+source/xserver-xorg-video-intel/+bug/1063252 Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index b13393b593b8..1591955044c8 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1395,10 +1395,17 @@ static int gen6_ring_flush(struct intel_ring_buffer *ring, return ret; cmd = MI_FLUSH_DW; + /* + * Bspec vol 1c.5 - video engine command streamer: + * "If ENABLED, all TLBs will be invalidated once the flush + * operation is complete. This bit is only valid when the + * Post-Sync Operation field is a value of 1h or 3h." + */ if (invalidate & I915_GEM_GPU_DOMAINS) - cmd |= MI_INVALIDATE_TLB | MI_INVALIDATE_BSD; + cmd |= MI_INVALIDATE_TLB | MI_INVALIDATE_BSD | + MI_FLUSH_DW_STORE_INDEX | MI_FLUSH_DW_OP_STOREDW; intel_ring_emit(ring, cmd); - intel_ring_emit(ring, 0); + intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT); intel_ring_emit(ring, 0); intel_ring_emit(ring, MI_NOOP); intel_ring_advance(ring); @@ -1460,10 +1467,17 @@ static int blt_ring_flush(struct intel_ring_buffer *ring, return ret; cmd = MI_FLUSH_DW; + /* + * Bspec vol 1c.3 - blitter engine command streamer: + * "If ENABLED, all TLBs will be invalidated once the flush + * operation is complete. This bit is only valid when the + * Post-Sync Operation field is a value of 1h or 3h." + */ if (invalidate & I915_GEM_DOMAIN_RENDER) - cmd |= MI_INVALIDATE_TLB; + cmd |= MI_INVALIDATE_TLB | MI_FLUSH_DW_STORE_INDEX | + MI_FLUSH_DW_OP_STOREDW | MI_FLUSH_DW_OP_STOREDW; intel_ring_emit(ring, cmd); - intel_ring_emit(ring, 0); + intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT); intel_ring_emit(ring, 0); intel_ring_emit(ring, MI_NOOP); intel_ring_advance(ring); -- cgit v1.2.2 From 3ac7831314eba873d60b58718123c503f6961337 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Thu, 25 Oct 2012 12:15:47 -0700 Subject: drm/i915: PIPE_CONTROL TLB invalidate requires CS stall MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "If ENABLED, PIPE_CONTROL command will flush the in flight data written out by render engine to Global Observation point on flush done. Also Requires stall bit ([20] of DW1) set." So set the stall bit to ensure proper invalidation. Signed-off-by: Jesse Barnes Reviewed-by: Antti Koskipää Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 1591955044c8..f7617a4e005f 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -245,7 +245,7 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring, /* * TLB invalidate requires a post-sync write. */ - flags |= PIPE_CONTROL_QW_WRITE; + flags |= PIPE_CONTROL_QW_WRITE | PIPE_CONTROL_CS_STALL; } ret = intel_ring_begin(ring, 4); -- cgit v1.2.2 From b3fcabb15bb83202fb5e4e5b296711b91c4942a3 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sun, 4 Nov 2012 12:24:47 +0100 Subject: drm/i915: drop the double-OP_STOREDW usage in blt_ring_flush This has been introduced in "drm/i915: TLB invalidation with MI_FLUSH_DW requires a post-sync op". Reported-by: Fengguang Wu Reported-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index f7617a4e005f..a035ac223fb0 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1475,7 +1475,7 @@ static int blt_ring_flush(struct intel_ring_buffer *ring, */ if (invalidate & I915_GEM_DOMAIN_RENDER) cmd |= MI_INVALIDATE_TLB | MI_FLUSH_DW_STORE_INDEX | - MI_FLUSH_DW_OP_STOREDW | MI_FLUSH_DW_OP_STOREDW; + MI_FLUSH_DW_OP_STOREDW; intel_ring_emit(ring, cmd); intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT); intel_ring_emit(ring, 0); -- cgit v1.2.2 From 6b8294a4d392c2c9f8867e8505511f3fc9419ba7 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 16 Nov 2012 11:43:20 +0000 Subject: drm/i915: Restore physical HWS_PGA after resume By always setting up the HWS register for both physical and virtual address variations during render ring we can reduce the number of different special cases that get set up at varying different times during module load. Fixes regression from commit c630119f43471a8ece356b01dabf07f944f453b3 Author: Daniel Vetter Date: Wed Oct 17 11:32:57 2012 +0200 drm/i915: don't save/restore HWS_PGA reg for kms Signed-off-by: Chris Wilson Cc: Daniel Vetter Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 45 +++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 10 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index a035ac223fb0..1aa76892a830 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1079,6 +1079,29 @@ err: return ret; } +static int init_phys_hws_pga(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 = + drm_pci_alloc(ring->dev, PAGE_SIZE, PAGE_SIZE); + if (!dev_priv->status_page_dmah) + 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); + + return 0; +} + static int intel_init_ring_buffer(struct drm_device *dev, struct intel_ring_buffer *ring) { @@ -1097,6 +1120,11 @@ static int intel_init_ring_buffer(struct drm_device *dev, ret = init_status_page(ring); if (ret) return ret; + } else { + BUG_ON(ring->id != RCS); + ret = init_phys_hws_pga(ring); + if (ret) + return ret; } obj = i915_gem_alloc_object(dev, ring->size); @@ -1545,12 +1573,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev) ring->init = init_render_ring; ring->cleanup = render_ring_cleanup; - - if (!I915_NEED_GFX_HWS(dev)) { - ring->status_page.page_addr = dev_priv->status_page_dmah->vaddr; - memset(ring->status_page.page_addr, 0, PAGE_SIZE); - } - return intel_init_ring_buffer(dev, ring); } @@ -1558,6 +1580,7 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size) { drm_i915_private_t *dev_priv = dev->dev_private; struct intel_ring_buffer *ring = &dev_priv->ring[RCS]; + int ret; ring->name = "render ring"; ring->id = RCS; @@ -1595,9 +1618,6 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size) ring->init = init_render_ring; ring->cleanup = render_ring_cleanup; - if (!I915_NEED_GFX_HWS(dev)) - ring->status_page.page_addr = dev_priv->status_page_dmah->vaddr; - ring->dev = dev; INIT_LIST_HEAD(&ring->active_list); INIT_LIST_HEAD(&ring->request_list); @@ -1614,6 +1634,12 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size) return -ENOMEM; } + if (!I915_NEED_GFX_HWS(dev)) { + ret = init_phys_hws_pga(ring); + if (ret) + return ret; + } + return 0; } @@ -1662,7 +1688,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev) } ring->init = init_ring_common; - return intel_init_ring_buffer(dev, ring); } -- cgit v1.2.2 From 1c8b46fc8c865189f562c9ab163d63863759712f Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 14 Nov 2012 09:15:14 +0000 Subject: drm/i915: Use LRI to update the semaphore registers The bspec was recently updated to remove the ability to update the semaphore using the MI_SEMAPHORE_BOX command, the ability to wait upon the semaphore value remained. Instead the advice is to update the register using the MI_LOAD_REGISTER_IMM command. In cursory testing, semaphores continue to function - the question is whether this fixes some of the deadlocks where the semaphore registers contained stale values? Signed-off-by: Chris Wilson Cc: Daniel J Blueman Reviewed-by: Ben Widawsky Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 1aa76892a830..987eb5fdaf39 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -558,12 +558,9 @@ update_mboxes(struct intel_ring_buffer *ring, u32 seqno, u32 mmio_offset) { - intel_ring_emit(ring, MI_SEMAPHORE_MBOX | - MI_SEMAPHORE_GLOBAL_GTT | - MI_SEMAPHORE_REGISTER | - MI_SEMAPHORE_UPDATE); - intel_ring_emit(ring, seqno); + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); intel_ring_emit(ring, mmio_offset); + intel_ring_emit(ring, seqno); } /** -- cgit v1.2.2 From 9d7730914f4cd496e356acfab95b41075aa8eae8 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 27 Nov 2012 16:22:52 +0000 Subject: drm/i915: Preallocate next seqno before touching the ring Based on the work by Mika Kuoppala, we realised that we need to handle seqno wraparound prior to committing our changes to the ring. The most obvious point then is to grab the seqno inside intel_ring_begin(), and then to reuse that seqno for all ring operations until the next request. As intel_ring_begin() can fail, the callers must already be prepared to handle such failure and so we can safely add further checks. This patch looks like it should be split up into the interface changes and the tweaks to move seqno wrapping from the execbuffer into the core seqno increment. However, I found no easy way to break it into incremental steps without introducing further broken behaviour. v2: Mika found a silly mistake and a subtle error in the existing code; inside i915_gem_retire_requests() we were resetting the sync_seqno of the target ring based on the seqno from this ring - which are only related by the order of their allocation, not retirement. Hence we were applying the optimisation that the rings were synchronised too early, fortunately the only real casualty there is the handling of seqno wrapping. v3: Do not forget to reset the sync_seqno upon module reinitialisation, ala resume. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=863861 Reviewed-by: Mika Kuoppala [v2] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 49 +++++++++++++++++---------------- 1 file changed, 26 insertions(+), 23 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 987eb5fdaf39..e4682cdc00b0 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -555,12 +555,11 @@ static void render_ring_cleanup(struct intel_ring_buffer *ring) static void update_mboxes(struct intel_ring_buffer *ring, - u32 seqno, - u32 mmio_offset) + u32 mmio_offset) { intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); intel_ring_emit(ring, mmio_offset); - intel_ring_emit(ring, seqno); + intel_ring_emit(ring, ring->outstanding_lazy_request); } /** @@ -573,8 +572,7 @@ update_mboxes(struct intel_ring_buffer *ring, * This acts like a signal in the canonical semaphore. */ static int -gen6_add_request(struct intel_ring_buffer *ring, - u32 *seqno) +gen6_add_request(struct intel_ring_buffer *ring) { u32 mbox1_reg; u32 mbox2_reg; @@ -587,13 +585,11 @@ gen6_add_request(struct intel_ring_buffer *ring, mbox1_reg = ring->signal_mbox[0]; mbox2_reg = ring->signal_mbox[1]; - *seqno = i915_gem_next_request_seqno(ring); - - update_mboxes(ring, *seqno, mbox1_reg); - update_mboxes(ring, *seqno, mbox2_reg); + update_mboxes(ring, mbox1_reg); + update_mboxes(ring, mbox2_reg); intel_ring_emit(ring, MI_STORE_DWORD_INDEX); intel_ring_emit(ring, I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT); - intel_ring_emit(ring, *seqno); + intel_ring_emit(ring, ring->outstanding_lazy_request); intel_ring_emit(ring, MI_USER_INTERRUPT); intel_ring_advance(ring); @@ -650,10 +646,8 @@ do { \ } while (0) static int -pc_render_add_request(struct intel_ring_buffer *ring, - u32 *result) +pc_render_add_request(struct intel_ring_buffer *ring) { - u32 seqno = i915_gem_next_request_seqno(ring); struct pipe_control *pc = ring->private; u32 scratch_addr = pc->gtt_offset + 128; int ret; @@ -674,7 +668,7 @@ pc_render_add_request(struct intel_ring_buffer *ring, PIPE_CONTROL_WRITE_FLUSH | PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE); intel_ring_emit(ring, pc->gtt_offset | PIPE_CONTROL_GLOBAL_GTT); - intel_ring_emit(ring, seqno); + intel_ring_emit(ring, ring->outstanding_lazy_request); intel_ring_emit(ring, 0); PIPE_CONTROL_FLUSH(ring, scratch_addr); scratch_addr += 128; /* write to separate cachelines */ @@ -693,11 +687,10 @@ pc_render_add_request(struct intel_ring_buffer *ring, PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE | PIPE_CONTROL_NOTIFY); intel_ring_emit(ring, pc->gtt_offset | PIPE_CONTROL_GLOBAL_GTT); - intel_ring_emit(ring, seqno); + intel_ring_emit(ring, ring->outstanding_lazy_request); intel_ring_emit(ring, 0); intel_ring_advance(ring); - *result = seqno; return 0; } @@ -885,25 +878,20 @@ bsd_ring_flush(struct intel_ring_buffer *ring, } static int -i9xx_add_request(struct intel_ring_buffer *ring, - u32 *result) +i9xx_add_request(struct intel_ring_buffer *ring) { - u32 seqno; int ret; ret = intel_ring_begin(ring, 4); if (ret) return ret; - seqno = i915_gem_next_request_seqno(ring); - intel_ring_emit(ring, MI_STORE_DWORD_INDEX); intel_ring_emit(ring, I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT); - intel_ring_emit(ring, seqno); + intel_ring_emit(ring, ring->outstanding_lazy_request); intel_ring_emit(ring, MI_USER_INTERRUPT); intel_ring_advance(ring); - *result = seqno; return 0; } @@ -1110,6 +1098,7 @@ static int intel_init_ring_buffer(struct drm_device *dev, INIT_LIST_HEAD(&ring->active_list); INIT_LIST_HEAD(&ring->request_list); ring->size = 32 * PAGE_SIZE; + memset(ring->sync_seqno, 0, sizeof(ring->sync_seqno)); init_waitqueue_head(&ring->irq_queue); @@ -1338,6 +1327,15 @@ int intel_wait_ring_buffer(struct intel_ring_buffer *ring, int n) return -EBUSY; } +static int +intel_ring_alloc_seqno(struct intel_ring_buffer *ring) +{ + if (ring->outstanding_lazy_request) + return 0; + + return i915_gem_get_seqno(ring->dev, &ring->outstanding_lazy_request); +} + int intel_ring_begin(struct intel_ring_buffer *ring, int num_dwords) { @@ -1349,6 +1347,11 @@ int intel_ring_begin(struct intel_ring_buffer *ring, if (ret) return ret; + /* Preallocate the olr before touching the ring */ + ret = intel_ring_alloc_seqno(ring); + if (ret) + return ret; + if (unlikely(ring->tail + n > ring->effective_size)) { ret = intel_wrap_ring_buffer(ring); if (unlikely(ret)) -- cgit v1.2.2 From 3e9605018ab3e333d51cc90fccfde2031886763b Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 27 Nov 2012 16:22:54 +0000 Subject: drm/i915: Rearrange code to only have a single method for waiting upon the ring Replace the wait for the ring to be clear with the more common wait for the ring to be idle. The principle advantage is one less exported intel_ring_wait function, and the removal of a hardcoded value. Signed-off-by: Chris Wilson Reviewed-by: Mika Kuoppala Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 73 ++++++++++++++++++++++----------- 1 file changed, 48 insertions(+), 25 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index e4682cdc00b0..bc7cf7c63108 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1175,7 +1175,7 @@ void intel_cleanup_ring_buffer(struct intel_ring_buffer *ring) /* Disable the ring buffer. The ring must be idle at this point */ dev_priv = ring->dev->dev_private; - ret = intel_wait_ring_idle(ring); + ret = intel_ring_idle(ring); if (ret) DRM_ERROR("failed to quiesce %s whilst cleaning up: %d\n", ring->name, ret); @@ -1194,28 +1194,6 @@ void intel_cleanup_ring_buffer(struct intel_ring_buffer *ring) cleanup_status_page(ring); } -static int intel_wrap_ring_buffer(struct intel_ring_buffer *ring) -{ - uint32_t __iomem *virt; - int rem = ring->size - ring->tail; - - if (ring->space < rem) { - int ret = intel_wait_ring_buffer(ring, rem); - if (ret) - return ret; - } - - virt = ring->virtual_start + ring->tail; - rem /= 4; - while (rem--) - iowrite32(MI_NOOP, virt++); - - ring->tail = 0; - ring->space = ring_space(ring); - - return 0; -} - static int intel_ring_wait_seqno(struct intel_ring_buffer *ring, u32 seqno) { int ret; @@ -1284,7 +1262,7 @@ static int intel_ring_wait_request(struct intel_ring_buffer *ring, int n) return 0; } -int intel_wait_ring_buffer(struct intel_ring_buffer *ring, int n) +static int ring_wait_for_space(struct intel_ring_buffer *ring, int n) { struct drm_device *dev = ring->dev; struct drm_i915_private *dev_priv = dev->dev_private; @@ -1327,6 +1305,51 @@ int intel_wait_ring_buffer(struct intel_ring_buffer *ring, int n) return -EBUSY; } +static int intel_wrap_ring_buffer(struct intel_ring_buffer *ring) +{ + uint32_t __iomem *virt; + int rem = ring->size - ring->tail; + + if (ring->space < rem) { + int ret = ring_wait_for_space(ring, rem); + if (ret) + return ret; + } + + virt = ring->virtual_start + ring->tail; + rem /= 4; + while (rem--) + iowrite32(MI_NOOP, virt++); + + ring->tail = 0; + ring->space = ring_space(ring); + + return 0; +} + +int intel_ring_idle(struct intel_ring_buffer *ring) +{ + u32 seqno; + int ret; + + /* We need to add any requests required to flush the objects and ring */ + if (ring->outstanding_lazy_request) { + ret = i915_add_request(ring, NULL, NULL); + if (ret) + return ret; + } + + /* Wait upon the last request to be completed */ + if (list_empty(&ring->request_list)) + return 0; + + seqno = list_entry(ring->request_list.prev, + struct drm_i915_gem_request, + list)->seqno; + + return i915_wait_seqno(ring, seqno); +} + static int intel_ring_alloc_seqno(struct intel_ring_buffer *ring) { @@ -1359,7 +1382,7 @@ int intel_ring_begin(struct intel_ring_buffer *ring, } if (unlikely(ring->space < n)) { - ret = intel_wait_ring_buffer(ring, n); + ret = ring_wait_for_space(ring, n); if (unlikely(ret)) return ret; } -- cgit v1.2.2 From 633cf8f5056c3e72158e4dbc387b3d65926d2d55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Mon, 3 Dec 2012 18:43:32 +0200 Subject: drm/i915: Don't allow ring tail to reach the same cacheline as head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From BSpec: "If the Ring Buffer Head Pointer and the Tail Pointer are on the same cacheline, the Head Pointer must not be greater than the Tail Pointer." The easiest way to enforce this is to reduce the reported ring space. References: Gen2 BSpec "1. Programming Environment" / 1.4.4.6 "Ring Buffer Use" Gen3 BSpec "vol1c Memory Interface Functions" / 2.3.4.5 "Ring Buffer Use" Gen4+ BSpec "vol1c Memory Interface and Command Stream" / 5.3.4.5 "Ring Buffer Use" v2: Include the exact BSpec references in the description v3: s/64/I915_RING_FREE_SPACE, and add the BSpec information to the code Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index bc7cf7c63108..2346b920bd86 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -45,7 +45,7 @@ struct pipe_control { static inline int ring_space(struct intel_ring_buffer *ring) { - int space = (ring->head & HEAD_ADDR) - (ring->tail + 8); + int space = (ring->head & HEAD_ADDR) - (ring->tail + I915_RING_FREE_SPACE); if (space < 0) space += ring->size; return space; @@ -1227,7 +1227,7 @@ static int intel_ring_wait_request(struct intel_ring_buffer *ring, int n) if (request->tail == -1) continue; - space = request->tail - (ring->tail + 8); + space = request->tail - (ring->tail + I915_RING_FREE_SPACE); if (space < 0) space += ring->size; if (space >= n) { -- cgit v1.2.2 From b45305fce5bb1abec263fcff9d81ebecd6306ede Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 17 Dec 2012 16:21:27 +0100 Subject: drm/i915: Implement workaround for broken CS tlb on i830/845 Now that Chris Wilson demonstrated that the key for stability on early gen 2 is to simple _never_ exchange the physical backing storage of batch buffers I've tried a stab at a kernel solution. Doesn't look too nefarious imho, now that I don't try to be too clever for my own good any more. v2: After discussing the various techniques, we've decided to always blit batches on the suspect devices, but allow userspace to opt out of the kernel workaround assume full responsibility for providing coherent batches. The principal reason is that avoiding the blit does improve performance in a few key microbenchmarks and also in cairo-trace replays. Signed-Off-by: Daniel Vetter Signed-off-by: Chris Wilson [danvet: - Drop the hunk which uses HAS_BROKEN_CS_TLB to implement the ring wrap w/a. Suggested by Chris Wilson. - Also add the ACTHD check from Chris Wilson for the error state dumping, so that we still catch batches when userspace opts out of the w/a.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 76 +++++++++++++++++++++++++++++---- 1 file changed, 68 insertions(+), 8 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 2346b920bd86..ae253e04c391 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -547,9 +547,14 @@ static int init_render_ring(struct intel_ring_buffer *ring) static void render_ring_cleanup(struct intel_ring_buffer *ring) { + struct drm_device *dev = ring->dev; + if (!ring->private) return; + if (HAS_BROKEN_CS_TLB(dev)) + drm_gem_object_unreference(to_gem_object(ring->private)); + cleanup_pipe_control(ring); } @@ -969,6 +974,8 @@ i965_dispatch_execbuffer(struct intel_ring_buffer *ring, return 0; } +/* Just userspace ABI convention to limit the wa batch bo to a resonable size */ +#define I830_BATCH_LIMIT (256*1024) static int i830_dispatch_execbuffer(struct intel_ring_buffer *ring, u32 offset, u32 len, @@ -976,15 +983,47 @@ i830_dispatch_execbuffer(struct intel_ring_buffer *ring, { int ret; - ret = intel_ring_begin(ring, 4); - if (ret) - return ret; + if (flags & I915_DISPATCH_PINNED) { + ret = intel_ring_begin(ring, 4); + if (ret) + return ret; - intel_ring_emit(ring, MI_BATCH_BUFFER); - intel_ring_emit(ring, offset | (flags & I915_DISPATCH_SECURE ? 0 : MI_BATCH_NON_SECURE)); - intel_ring_emit(ring, offset + len - 8); - intel_ring_emit(ring, 0); - intel_ring_advance(ring); + intel_ring_emit(ring, MI_BATCH_BUFFER); + intel_ring_emit(ring, offset | (flags & I915_DISPATCH_SECURE ? 0 : MI_BATCH_NON_SECURE)); + intel_ring_emit(ring, offset + len - 8); + intel_ring_emit(ring, MI_NOOP); + intel_ring_advance(ring); + } else { + struct drm_i915_gem_object *obj = ring->private; + u32 cs_offset = obj->gtt_offset; + + if (len > I830_BATCH_LIMIT) + return -ENOSPC; + + ret = intel_ring_begin(ring, 9+3); + if (ret) + return ret; + /* Blit the batch (which has now all relocs applied) to the stable batch + * scratch bo area (so that the CS never stumbles over its tlb + * invalidation bug) ... */ + intel_ring_emit(ring, XY_SRC_COPY_BLT_CMD | + XY_SRC_COPY_BLT_WRITE_ALPHA | + XY_SRC_COPY_BLT_WRITE_RGB); + intel_ring_emit(ring, BLT_DEPTH_32 | BLT_ROP_GXCOPY | 4096); + intel_ring_emit(ring, 0); + intel_ring_emit(ring, (DIV_ROUND_UP(len, 4096) << 16) | 1024); + intel_ring_emit(ring, cs_offset); + intel_ring_emit(ring, 0); + intel_ring_emit(ring, 4096); + intel_ring_emit(ring, offset); + intel_ring_emit(ring, MI_FLUSH); + + /* ... and execute it. */ + intel_ring_emit(ring, MI_BATCH_BUFFER); + intel_ring_emit(ring, cs_offset | (flags & I915_DISPATCH_SECURE ? 0 : MI_BATCH_NON_SECURE)); + intel_ring_emit(ring, cs_offset + len - 8); + intel_ring_advance(ring); + } return 0; } @@ -1596,6 +1635,27 @@ int intel_init_render_ring_buffer(struct drm_device *dev) ring->init = init_render_ring; ring->cleanup = render_ring_cleanup; + /* Workaround batchbuffer to combat CS tlb bug. */ + if (HAS_BROKEN_CS_TLB(dev)) { + struct drm_i915_gem_object *obj; + int ret; + + obj = i915_gem_alloc_object(dev, I830_BATCH_LIMIT); + if (obj == NULL) { + DRM_ERROR("Failed to allocate batch bo\n"); + return -ENOMEM; + } + + ret = i915_gem_object_pin(obj, 0, true, false); + if (ret != 0) { + drm_gem_object_unreference(&obj->base); + DRM_ERROR("Failed to ping batch bo\n"); + return ret; + } + + ring->private = obj; + } + return intel_init_ring_buffer(dev, ring); } -- cgit v1.2.2