Re: [Intel-gfx] [PATCH 02/15] drm/i915: Don't emit mbox updates without semaphores
On Mon, Dec 16, 2013 at 08:50:38PM -0800, Ben Widawsky wrote: Aside from the fact that it leaves confusing dumps on error capture, it is entirely unnecessary, and potentially harmful in cases like BDW, where the instruction has changed. In reality (seemingly), this will have no behavioral impact. Signed-off-by: Ben Widawsky b...@bwidawsk.net The reason why we currently do is because i915.semaphores can change at runtime. So we emit the instructions whilst i915.semaphores=0 just in case, it is enabled later. This restriction can be lifted with a little more work in handling the missed semaphores, I think, or it may just require a proof that everything is safe as is. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/15] drm/i915: Don't emit mbox updates without semaphores
On Tue, Dec 17, 2013 at 07:24:41PM +, Chris Wilson wrote: On Mon, Dec 16, 2013 at 08:50:38PM -0800, Ben Widawsky wrote: Aside from the fact that it leaves confusing dumps on error capture, it is entirely unnecessary, and potentially harmful in cases like BDW, where the instruction has changed. In reality (seemingly), this will have no behavioral impact. Signed-off-by: Ben Widawsky b...@bwidawsk.net The reason why we currently do is because i915.semaphores can change at runtime. So we emit the instructions whilst i915.semaphores=0 just in case, it is enabled later. This restriction can be lifted with a little more work in handling the missed semaphores, I think, or it may just require a proof that everything is safe as is. -Chris It should still check the module parameter - I guess it would be nice to guard changing the module parameter with struct_mutex (generally, not just here), as that also breaks the emit path. So in short, I think it's broken for two reasons. My (and Daniel's) vote is to just make the module param static. -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/15] drm/i915: Don't emit mbox updates without semaphores
On Tue, Dec 17, 2013 at 02:02:23PM -0800, Ben Widawsky wrote: On Tue, Dec 17, 2013 at 07:24:41PM +, Chris Wilson wrote: On Mon, Dec 16, 2013 at 08:50:38PM -0800, Ben Widawsky wrote: Aside from the fact that it leaves confusing dumps on error capture, it is entirely unnecessary, and potentially harmful in cases like BDW, where the instruction has changed. In reality (seemingly), this will have no behavioral impact. Signed-off-by: Ben Widawsky b...@bwidawsk.net The reason why we currently do is because i915.semaphores can change at runtime. So we emit the instructions whilst i915.semaphores=0 just in case, it is enabled later. This restriction can be lifted with a little more work in handling the missed semaphores, I think, or it may just require a proof that everything is safe as is. -Chris It should still check the module parameter - I guess it would be nice to guard changing the module parameter with struct_mutex (generally, not just here), as that also breaks the emit path. So in short, I think it's broken for two reasons. My (and Daniel's) vote is to just make the module param static. Dynamic i915.semaphores is something I can live happily without. If we ever do need such a thing, it needs to be internal to the kernel. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/15] drm/i915: Don't emit mbox updates without semaphores
Aside from the fact that it leaves confusing dumps on error capture, it
is entirely unnecessary, and potentially harmful in cases like BDW,
where the instruction has changed.
In reality (seemingly), this will have no behavioral impact.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index e05a021..b106984 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -663,14 +663,15 @@ gen6_add_request(struct intel_ring_buffer *ring)
struct drm_device *dev = ring-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_ring_buffer *useless;
- int i, ret;
+ int i, ret, num_dwords = 4;
- ret = intel_ring_begin(ring, ((I915_NUM_RINGS-1) *
- MBOX_UPDATE_DWORDS) +
- 4);
+ if (i915_semaphore_is_enabled(dev))
+ num_dwords += ((I915_NUM_RINGS-1) * MBOX_UPDATE_DWORDS);
+#undef MBOX_UPDATE_DWORDS
+
+ ret = intel_ring_begin(ring, num_dwords);
if (ret)
return ret;
-#undef MBOX_UPDATE_DWORDS
for_each_ring(useless, dev_priv, i) {
u32 mbox_reg = ring-signal_mbox[i];
--
1.8.5.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
