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

Michel Thierry michel.thierry at intel.com
Thu Oct 26 20:15:27 UTC 2017


On 10/26/2017 1:02 PM, Chris Wilson wrote:
> 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.

Ok, I'll need to split the function (or pass some flags to execute just 
parts of it) for the selftest anyway.


More information about the Intel-gfx mailing list