From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism Date: Mon, 23 Jun 2014 14:41:40 +0100 Message-ID: <20140623134140.GC14360@nuc-i3427.alporthouse.com> References: <1402673891-14618-1-git-send-email-oscar.mateo@intel.com> <1402673891-14618-27-git-send-email-oscar.mateo@intel.com> <20140620210035.GC32083@bdvolkin-ubuntu-desktop> <92648605EABDA246B775AAB04C95A7A3137BEC32@IRSMSX103.ger.corp.intel.com> <20140623131355.GA14360@nuc-i3427.alporthouse.com> <92648605EABDA246B775AAB04C95A7A3137BEC64@IRSMSX103.ger.corp.intel.com> <20140623132719.GB14360@nuc-i3427.alporthouse.com> <92648605EABDA246B775AAB04C95A7A3137BEDF8@IRSMSX103.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from fireflyinternet.com (mail.fireflyinternet.com [87.106.93.118]) by gabe.freedesktop.org (Postfix) with ESMTP id AE2FD6E3E7 for ; Mon, 23 Jun 2014 06:41:44 -0700 (PDT) Content-Disposition: inline In-Reply-To: <92648605EABDA246B775AAB04C95A7A3137BEDF8@IRSMSX103.ger.corp.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "Mateo Lozano, Oscar" Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Mon, Jun 23, 2014 at 01:36:07PM +0000, Mateo Lozano, Oscar wrote: > > -----Original Message----- > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > > Sent: Monday, June 23, 2014 2:27 PM > > To: Mateo Lozano, Oscar > > Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring > > submission mechanism > > = > > On Mon, Jun 23, 2014 at 01:18:35PM +0000, Mateo Lozano, Oscar wrote: > > > > -----Original Message----- > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > > > > Sent: Monday, June 23, 2014 2:14 PM > > > > To: Mateo Lozano, Oscar > > > > Cc: Volkin, Bradley D; intel-gfx@lists.freedesktop.org > > > > Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical > > > > ring submission mechanism > > > > > > > > On Mon, Jun 23, 2014 at 01:09:37PM +0000, Mateo Lozano, Oscar > > wrote: > > > > > So far, yes, but that=B4s only because I artificially made > > > > > intel_lrc.c self- > > > > contained, as Daniel requested. What if we need to execute commands > > > > from somewhere else, like in intel_gen7_queue_flip()? > > > > > > > > > > And this takes me to another discussion: this logical ring vs > > > > > legacy ring split > > > > is probably a good idea (time will tell), but we should provide a > > > > way of sending commands for execution without knowing if Execlists > > > > are enabled or not. In the early series that was easy because we > > > > reused the ring_begin, ring_emit & ring_advance functions, but this > > > > is not the case anymore. And without this, sooner or later somebody > > > > will break legacy or execlists (this already happened last week, > > > > when somebody here was implementing native sync without knowing > > about Execlists). > > > > > > > > > > So, the questions is: how do you feel about a dev_priv.gt vfunc > > > > > that takes a > > > > context, a ring, an array of DWORDS and a BB length and does the > > > > intel_(logical)_ring_begin/emit/advance based on i915.enable_execli= sts? > > > > > > > > I'm still baffled by the design. intel_ring_begin() and friends > > > > should be able to find their context (logical or legacy) from the r= ing and > > dtrt. > > > > -Chris > > > > > > Sorry, Chris, I obviously don=B4t have the same experience with 915 y= ou have: > > how do you propose to extract the right context from the ring? > > = > > The rings are a set of buffers and vfuncs that are associated with a co= ntext. > > Before you can call intel_ring_begin() you must know what context you w= ant > > to operate on and therefore can pick the right logical/legacy ring and > > interface for RCS/BCS/VCS/etc -Chris > = > Ok, but then you need to pass some extra stuff down together with the int= el_engine_cs, either intel_context or intel_ringbuffer, right? Because that= =B4s exactly what I did in previous versions, plumbing intel_context every= where where it was needed (I could have plumbed intel_ringbuffer instead, i= t really doesn=B4t matter). This was rejected for being too intrusive and n= ot allowing easy maintenance in the future. Nope. You didn't redesign the ringbuffers to function as we expected but tacked on extra information and layering violations. -Chris -- = Chris Wilson, Intel Open Source Technology Centre