On Sat, Dec 14, 2013 at 09:16:51PM +0000, Chris Wilson wrote: > On Sat, Dec 14, 2013 at 11:39:27AM -0800, Ben Widawsky wrote: > > > > On Sat, Dec 14, 2013 at 09:08:47AM +0000, Chris Wilson wrote: > > > On Fri, Dec 13, 2013 at 08:15:48PM -0800, Ben Widawsky wrote: > > > > Ben Widawsky (17): > > > > drm/i915: Reorder/respace MI instruction definition > > > > drm/i915: Don't emit mbox updates without semaphores > > > > drm/i915: Move semaphore specific ring members to struct > > > > drm/i915: Virtualize the ringbuffer signal func > > > > drm/i915: Move ring_begin to signal() > > > > drm/i915: Make semaphore updates more precise > > > > drm/i915: gen specific semaphore info > > > > drm/i915: Create for_all_rings > > > > drm/i915: init ring->id early > > > > drm/i915/bdw: implement semaphore signal > > > > drm/i915/bdw: implement semaphore wait > > > > drm/i915: FORCE_RESTORE for gen8 semaphores > > > > drm/i915/bdw: poll semaphores > > > > drm/i915: Extract semaphore error collection > > > > drm/i915/bdw: collect semaphore error state > > > > drm/i915: unleash semaphores on gen8 > > > > drm/i915: semaphore debugfs > > > > > > By the end, don't you use a mix of tables and formula for writing the > > > offsets for the wait/signal commands? Looks very inconsistent when there > > > is a very simple routine for generating the appropriate semaphore slot > > > given (waiter, signaller). > > > -Chris > > > > > > > Excluding debugfs, which was left intentionally that way... > > > > The only inconsistency, I think was in the error state capture, > > right? That was a last minute addition probably after I should have quit > > for the day. Do you see something other than error state (and debugfs)? > > I'll discuss the error state stuff with on the patch itself. > > The error state and signaller use the table, but the waiter uses the > offset calculation. And debugfs does its own compact thing (which is > ideal for neatly formating the table.). > > perhaps That's not new to the series. > > #define GEN8_SEMAPHORE(signaller, waiter) \ > (i915_gem_obj_ggtt_offset(dev_priv->semaphore_obj) + \ > (signaller * I915_NUM_RINGS + waiter) * SEQNO_SIZE) > > #define GEN8_WAIT_OFFSET(from) GEN8_SEMAPHORE(from, ring->id) > #define GEN8_SIGNAL_OFFSET(to) GEN8_SEMAPHORE(ring->id, to) > > would make it clearer what I mean. > -Chris Well, the other half is not currently gen specific, and I liked keeping it that way. In either case, the fix on top is quite simple. Patches welcome :-) > > -- > 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
