[Intel-gfx] [PATCH v4 2/3] drm/i915/guc: Introduce buffer based cmd transport

[Michal Wajdeczko]

On Thu, May 25, 2017 at 10:48:19AM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 25/05/17 04:36, Michal Wajdeczko wrote:
> > Buffer based command transport can replace MMIO based mechanism.
> > It may be used to perform host-2-guc and guc-to-host communication.
> > 
> > Portions of this patch are based on work by:
> >  Michel Thierry <michel.thierry at intel.com>
> >  Robert Beckett <robert.beckett at intel.com>
> >  Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> > 
> > v2: use gem_object_pin_map (Chris)
> >     don't use DEBUG_RATELIMITED (Chris)
> >     don't track action stats (Chris)
> >     simplify next fence (Chris)
> >     use READ_ONCE (Chris)
> >     move blob allocation to new function (Chris)
> > 
> > v3: use static owner id (Daniele)
> > v4: but keep channel initialization generic (Daniele)
> >     and introduce owner_sub_id (Daniele)
> > 
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> > Cc: Oscar Mateo <oscar.mateo at intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> 
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> 
> minor possible bikeshed below.
> 
> > +
> > +/** Holds all command transport channels.
> > + *
> > + * @host_channel: main channel used by the host
> > + */
> > +struct intel_guc_ct {
> > +	struct intel_guc_ct_channel host_channel;
> > +	/* other channels are tbd */
> > +};
> > +
> 
> We could also use an array if we modify the enum:
> 
> 	enum {
> 		CTB_OWNER_HOST = 0,
> 		/* other channels are tbd */
> 		CTB_MAX_CHANNELS
> 	};
> 
> and use:
> 
> 	struct intel_guc_ct_channel ct_channels[CTB_MAX_CHANNELS];
> 
> we can then address the host specific one with:
> 
> 	&guc.ct_channels[CTB_OWNER_HOST];
> 

I would wait with this approach until we actually introduce other
channels and have more than single entry in this array.

Thanks,
Michal