On Sat, Dec 14, 2013 at 09:38:57PM +0000, Chris Wilson wrote: > On Sat, Dec 14, 2013 at 11:47:24AM -0800, Ben Widawsky wrote: > > On Fri, Dec 13, 2013 at 08:16:03PM -0800, Ben Widawsky wrote: > > > Since the semaphore information is in an object, just dump it, and let > > > the user parse it later. > > > > > > NOTE: The page being used for the semaphores are incoherent with the > > > CPU. No matter what I do, I cannot figure out a way to read anything but > > > 0s. Note that the semaphore waits are indeed working. > > > > > > Signed-off-by: Ben Widawsky <ben@xxxxxxxxxxxx> > > > > > > > [snip] > > > > > +static void gen8_record_semaphore_state(struct drm_i915_private *dev_priv, > > > + struct drm_i915_error_state *error, > > > + struct intel_ring_buffer *ring) > > > +{ > > > + struct intel_ring_buffer *useless; > > > + int i; > > > + > > > + if (!i915_semaphore_is_enabled(dev_priv->dev)) > > > + return; > > > + > > > + if (!error->semaphore_obj) > > > + error->semaphore_obj = > > > + i915_error_object_create(dev_priv, > > > + dev_priv->semaphore_obj); > > > + > > > + for_each_ring(useless, dev_priv, i) { > > > + u16 signal_offset = GEN8_SIGNAL_OFFSET(ring, i) / 4; > > > + u16 wait_offset = GEN8_WAIT_OFFSET(ring, i) / 4; > > > + u32 *tmp = error->semaphore_obj->pages[0]; > > > + > > > + error->semaphore_mboxes[ring->id][i] = tmp[signal_offset]; > > > + error->semaphore_seqno[ring->id][i] = tmp[wait_offset]; > > > + } > > > +} > > > + > > > > Chris, I preemptively assume this rubs you the wrong way. I had been > > trying to do the analogous of the I915_READ from previous gens. I felt > > it was more consistent with reading of hardware state. If you want me to > > simply use the ring->semaphore.*, I can. I just wanted to make my > > intention clear. > > Capturing the semaphore_obj itself looks a bit silly since we never do > dump it into i915_error_state, but I actually like how you've reading back of > the semaphore page for debugging hangs. I don't understand this comment. The semaphore page is dumped, though I snipped that, so maybe that's why you made the comment. > And don't we also include our own semaphore seqno values in the > i915_error_state as well? Oh, I see now. You've overwritten those > values. Whoops. We do need to keep ring->sync_seqno[] in the debug > output for a certain class of (driver) bug. So it looks like we could > do with all three values (current signal, last wait, last known sync) > in the error state. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Ben Widawsky, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx