Re: [RFC/CI] drm/i915: Sanitize GuC client initialization

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On pe, 2017-02-10 at 12:03 -0800, Daniele Ceraolo Spurio wrote:
> <snip>
> > 
> > > 
> > > +
> > >  /*
> > >   * Tell the GuC to allocate or deallocate a specific doorbell
> > >   */
> > > 
> > > -static int guc_allocate_doorbell(struct intel_guc *guc,
> > > -				 struct i915_guc_client *client)
> > > +static int __create_doorbell_hw(struct i915_guc_client *client)
> > 
> > I would rather prefer to only change signature of this function into
> > 
> > 	static int guc_allocate_doorbell(struct intel_guc *guc, u32 index)
> > 
> > as a clean wrap around GUC_ACTION_ALLOCATE_DOORBELL. This way we also preserve
> > consistency between function name and the guc action name used inside.
> > 
> > Based on the above we can still add
> > 
> > 	static int __create_doorbell_hw(struct i915_guc_client *client)
> > 	{
> > 		return guc_allocate_doorbell(client->guc, client->ctx_index);
> > 	}
> > 
> > Note that location of the ctx_index member may change in the future, and this
> > approach will minimize impact of these future changes.
> > 
> 
> +1.
> The client is a SW abstraction that we use to track our registrations 
> with GuC, but it only works with our current mode of operation. If we 
> plumb it too deep into the low-level functions it'll be more difficult 
> to do any reworks in the future.

Indeed, the ctx_index should move. I'd like to keep it __ prefixed
function, because the client needs the desc updated for the function to
be meaningful.

So I'll make it __guc_{de}allocate_doorbell for now.

Might be good to have i915_guc_doorbell, i915_guc_context,
i915_guc_client in the future to reduce code entangelment.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx