From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 51/53] drm/i915/bdw: Document Logical Rings, LR contexts and Execlists Date: Tue, 17 Jun 2014 11:39:35 +0200 Message-ID: <20140617093935.GK5821@phenom.ffwll.local> References: <1402673891-14618-1-git-send-email-oscar.mateo@intel.com> <1402673891-14618-52-git-send-email-oscar.mateo@intel.com> <20140613165116.GN6451@nuc-i3427.alporthouse.com> <92648605EABDA246B775AAB04C95A7A30131B63B@IRSMSX103.ger.corp.intel.com> <20140616175600.GA5821@phenom.ffwll.local> <92648605EABDA246B775AAB04C95A7A30131D96F@IRSMSX103.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f176.google.com (mail-wi0-f176.google.com [209.85.212.176]) by gabe.freedesktop.org (Postfix) with ESMTP id 24E0E6E468 for ; Tue, 17 Jun 2014 02:39:43 -0700 (PDT) Received: by mail-wi0-f176.google.com with SMTP id n3so5464879wiv.9 for ; Tue, 17 Jun 2014 02:39:42 -0700 (PDT) Content-Disposition: inline In-Reply-To: <92648605EABDA246B775AAB04C95A7A30131D96F@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 Tue, Jun 17, 2014 at 08:22:55AM +0000, Mateo Lozano, Oscar wrote: > > > -----Original Message----- > > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel > > Vetter > > Sent: Monday, June 16, 2014 6:56 PM > > To: Mateo Lozano, Oscar > > Cc: Chris Wilson; intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH 51/53] drm/i915/bdw: Document Logical > > Rings, LR contexts and Execlists > > > > On Mon, Jun 16, 2014 at 03:24:26PM +0000, Mateo Lozano, Oscar wrote: > > > > -----Original Message----- > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > > > > Sent: Friday, June 13, 2014 5:51 PM > > > > To: Mateo Lozano, Oscar > > > > Cc: intel-gfx@lists.freedesktop.org > > > > Subject: Re: [Intel-gfx] [PATCH 51/53] drm/i915/bdw: Document > > > > Logical Rings, LR contexts and Execlists > > > > > > > > On Fri, Jun 13, 2014 at 04:38:09PM +0100, oscar.mateo@intel.com > > wrote: > > > > > +/** > > > > > + * intel_execlists_ctx_id() - get the Execlists Context ID > > > > > + * @ctx_obj: Logical Ring Context backing object. > > > > > + * > > > > > + * Do not confuse with ctx->id! Unfortunately we have a name > > > > > +overload > > > > > + * here: the old context ID we pass to userspace as a handler so > > > > > +that > > > > > + * they can refer to a context, and the new context ID we pass to > > > > > +the > > > > > + * ELSP so that the GPU can inform us of the context status via > > > > > + * interrupts. > > > > > + * > > > > > + * Return: 20-bits globally unique context ID. > > > > > + */ > > > > > > > > Use tag for the ctx id we pass around in hw? > > > > -Chris > > > > > > I also tried other names, like "submission id", but it confuses people > > > when they search for in the BSpec. Maybe changing ctx->id to ctx->tag, > > > and leaving id for the hardware? > > > > I think Chris' idea was to reuse the id from the idr for the hw tag. But I guess > > that fails because our idr is global. > > > > Or I'm totally confused. > > > > I'd vote for hw_ctx_id or something. > > -Daniel > > In the first version of the series I tried to reuse the id from the idr, > but that was a bad idea because the id we pass to the hw has to be > globally unique, while our idr is per file_priv. What I did is adding an > id field to the file_priv and then generating the hw ctx id by using > some bits from ctx->id, some from file_priv->id and finally some from > ring->id (since we multiplex several hw contexts inside our struct > intel_context). But the ELSP context descriptor only allows 20 bits for > the id, so I had to limit the maximum number of contexts, files or rings > artificially (ugly). Considerations like this should be somewhere in the commit message. Especially when it's all stuff you've discovered before review started and hence doesn't have a public record anywhere. > Another proposal: s/ctx->id/ctx->handle. After all, our ctx->id software construct is just a userspace handle... Not sure either is clearer really. As long as there's a clear disdinction between the hw id and the userspace handle I'm ok, maybe augmented with some comments to explain the struct fields in the header. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch