Re: [PATCH v6] drm/i915/guc: Add a second client, to be used for preemption

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Michel Thierry (2017-10-26 19:49:06)
> On 26/10/17 07:17, Michał Winiarski wrote:
> > @@ -763,14 +770,14 @@ static int guc_init_doorbell_hw(struct intel_guc *guc)
> > 
> >          /* Now for every client (and not only execbuf_client) make sure their
> >           * doorbells are known by the GuC */
> > -       //for (client = client_list; client != NULL; client = client->next)
> > -       {
> > -               ret = __create_doorbell(client);
> > -               if (ret) {
> > -                       DRM_ERROR("Couldn't recreate client %u doorbell: %d\n",
> > -                               client->stage_id, ret);
> > -                       return ret;
> > -               }
> > +       ret = __create_doorbell(guc->execbuf_client);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = __create_doorbell(guc->preempt_client);
> > +       if (ret) {
> > +               __destroy_doorbell(guc->execbuf_client);
> > +               return ret;
> >          }
> 
> I'm pretty sure there's an old client left behind after this, e.g.:
> 
>   static int guc_init_doorbell_hw(struct intel_guc *guc)
>   {
> -       struct i915_guc_client *client = guc->execbuf_client;

That doesn't look like it's a problem, or that confusing. This client
local var is just a placeholder used to clear the unwanted doorbells. We
then walk over the known kernel clients to reset them.

So, I think the suggestion is to split this function again, but I don't
see a blocker for merging.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx