[Intel-gfx] [PATCH] drm/i915/selftests: Add a GuC doorbells selftest

[Michel Thierry]

On 25/10/17 14:19, Michel Thierry wrote:
> On 25/10/17 14:08, Chris Wilson wrote:
>> Quoting Michel Thierry (2017-10-25 21:53:44)
>>> Try to create multiple clients (one of each kind) and exercise the
>>> doorbell alloc/dealloc code.
>>>
>>> Since our usage mode require very few clients/doorbells, this code has
>>> been exercised very lightly and it's good to have a simple test for it.
>>>
>>> As reference, this test already helped identify the bug fixed by
>>> commit 7f1ea2ac3017 ("drm/i915/guc: Fix doorbell id selection").
>>>
>>> Signed-off-by: Michel Thierry <michel.thierry at intel.com>
>>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>> Cc: Michal Winiarski <michal.winiarski at intel.com>
>>> ---
>>> +/* create a client of each type and their doorbells */
>>> +static int igt_guc_doorbells(void *arg)
>>> +{
>>> +       struct drm_i915_private *dev_priv = arg;
>>> +       struct intel_guc *guc;
>>> +       struct i915_guc_client *clients[GUC_CLIENT_PRIORITY_NUM];
>>
>> Don't you actually want to test up to GUC_NUM_DOORBELLS? (-reserved?)
>> And check failure when full?
>>
>> I'm thinking an idr for the clients, that will give you an universal
>> -EEXISTS test as well for a repeated id, as well as an easy structure to
>> cleanup.
>>

cc-ing people that may be interested,

There is already an idr for the clients, guc->stage_ids, but its max 
range is GUC_MAX_STAGE_DESCRIPTORS, not GUC_NUM_DOORBELLS (this is 
because other drivers support the idea of 'leasing doorbells' and you 
could have more clients than doorbells).

But we don't support that, and right now any client after 
GUC_NUM_DOORBELLS would be allocated but then get a -ENOSPC while 
creating/reserving its doorbell.

I could change the limit of guc->stage_ids idr and reject earlier, any 
comments?

-Michel