From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 08/43] drm/i915/bdw: Add a context and an engine pointers to the ringbuffer Date: Thu, 14 Aug 2014 17:37:11 +0200 Message-ID: <20140814153711.GX10500@phenom.ffwll.local> References: <1406217891-8912-1-git-send-email-thomas.daniel@intel.com> <1406217891-8912-9-git-send-email-thomas.daniel@intel.com> <20140811141413.GE10500@phenom.ffwll.local> <20140811142032.GF10500@phenom.ffwll.local> <20140813151616.GD10500@phenom.ffwll.local> <20140814153228.GW10500@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-we0-f169.google.com (mail-we0-f169.google.com [74.125.82.169]) by gabe.freedesktop.org (Postfix) with ESMTP id EF1066E6D3 for ; Thu, 14 Aug 2014 08:37:00 -0700 (PDT) Received: by mail-we0-f169.google.com with SMTP id u56so1281926wes.0 for ; Thu, 14 Aug 2014 08:37:00 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140814153228.GW10500@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "Daniel, Thomas" Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Thu, Aug 14, 2014 at 05:32:28PM +0200, Daniel Vetter wrote: > On Thu, Aug 14, 2014 at 03:09:45PM +0000, Daniel, Thomas wrote: > > When it comes to the execlist submission (actually as early as the execlist > > request queueing), the engine and context are indeed used and required. > > intel_logical_ring_advance_and_submit() is the lrc function analogous to > > __intel_ring_advance() and I believe the initial creation of intel_lrc.c > > was actually done by copying intel_ringbuffer.c. This explains why some of > > the lrc code is perhaps not as it would have been if this had been designed > > from scratch, and there is room for future improvement. > > advance_and_submit therefore only gets the ringbuffer struct and uses > > the context pointer in that struct to get the logical ring context itself. At > > that point the engine, context and new tail pointer are handed over to the > > execlist queue backend. > > I guess I need to clarify: Does it make sense to move the ELSP > respectively the submission to the execlist scheduler queue out of there > up a few levels into the execlist cmd submission function? Is it possible > or is there some technical reason that I'm overlooking? > > I want to know what exactly I'm dealing with here before I sign up for it > by merging the patches as-is and asking for a cleanup. I doesn't look bad > really, but there's always a good chance that I've overlooked a bigger > dragon. > > Since you have the patches and worked with them I'm asking you such > explorative questions. Ofc I can do this checking myself, but that takes > time ... This doesn't mean that you have to implement the changes, just be > reasonable confident that it will work out as a cleanup on top. To clarify more the context: Currently you're replies sound like "This is what it looks like and I don't really know why nor whether we can change that". That's not confidence instilling and that makes maintainers reluctant to merge patches for fear of needing to fix things themselves ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch