On Mon, Jan 11, 2016 at 02:47:33PM -0800, Jesse Barnes wrote: > On 01/11/2016 11:03 AM, John Harrison wrote: > > On 08/01/2016 22:05, Chris Wilson wrote: > >> On Fri, Jan 08, 2016 at 06:47:24PM +0000, john.c.harri...@intel.com wrote: > >>> From: John Harrison <john.c.harri...@intel.com> > >>> > >>> The fence object used inside the request structure requires a sequence > >>> number. Although this is not used by the i915 driver itself, it could > >>> potentially be used by non-i915 code if the fence is passed outside of > >>> the driver. This is the intention as it allows external kernel drivers > >>> and user applications to wait on batch buffer completion > >>> asynchronously via the dma-buff fence API. > >> That doesn't make any sense as they are not limited by a single > >> timeline. > > I don't understand what you mean. Who is not limited by a single timeline? > > The point is that the current seqno values cannot be used as there is no > > guarantee that they will increment globally once things like a scheduler > > and pre-emption arrive. Whereas, the fence internal implementation makes > > various assumptions about the linearity of the timeline. External users do > > not want to care about timelines or seqnos at all, they just want the fence > > API to work as documented. > > > >> > >>> To ensure that such external users are not confused by strange things > >>> happening with the seqno, this patch adds in a per context timeline > >>> that can provide a guaranteed in-order seqno value for the fence. This > >>> is safe because the scheduler will not re-order batch buffers within a > >>> context - they are considered to be mutually dependent. > >> You haven't added per-context breadcrumbs. What we need for being able > >> to execute requests from parallel timelines, but with requests within a > >> timeline being ordered, is a per-context page where we can emit the > >> per-context issued breadcrumb. Then instead of looking up the current > >> HW seqno in a global page, the request just looks at the current context > >> HW seqno in the context seq, just > >> i915_seqno_passed(*req->p_context_seqno, req->seqno). > > This patch is not attempting to implement per context seqno values. That > > can be done as future work. This patch is doing the simplest, least > > invasive implementation in order to make external fences work. > > Right. I think we want to move to per-context seqnos, but we don't have to > do it before this work lands. It should be easier to do it after the rest of > these bits land in fact, since seqno handling will be well encapsulated aiui.
This patch is irrelevent then. I think it is actually worse because it is encapsulating a design detail that is fundamentally wrong. -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
