Re: [RFC 06/11] drm/i915/guc: Remove extra arguments from guc_client_alloc

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 23/02/17 08:38, Oscar Mateo wrote:
>
>
> On 02/23/2017 11:14 AM, Michał Winiarski wrote:
>> We're always using all engines and kernel context for guc clients, let's
>> remove those arguments from guc_client_alloc.
> I am quite new to the GuC but, by the look of it, passing the ctx was
> groundwork for direct submission (which means calling guc_client_alloc
> with contexts different than kernel_context). Why not leave it there,
> since someone was careful to lay the groundwork? I don't see an obvious
> use for the engines, though (clients that only have access to some of
> the engines?).
>

If I recall correctly the idea behind having the engine flag was for 
possibly having dedicated clients for single engines and/or engines 
groups if 1 work-queue + doorbell pair turned out to not be enough for 
our needs. There might also be some little performance benefit in that 
scenario because we can update different work-queues in parallel, but 
we're still capped by how fast GuC can process them. Not sure if it 
still makes sense though.

Daniele

>> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_guc_submission.c | 16 ++++------------
>>   1 file changed, 4 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index 6c64ce1..3080735 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -838,21 +838,15 @@ static void guc_init_doorbell_hw(struct
>> intel_guc *guc)
>>   /**
>>    * guc_client_alloc() - Allocate an i915_guc_client
>>    * @dev_priv:    driver private data structure
>> - * @engines:    The set of engines to enable for this client
>>    * @priority:    four levels priority _CRITICAL, _HIGH, _NORMAL and
>> _LOW
>>    *         The kernel client to replace ExecList submission is
>> created with
>>    *         NORMAL priority. Priority of a client for scheduler can
>> be HIGH,
>>    *         while a preemption context can use CRITICAL.
>> - * @ctx:    the context that owns the client (we use the default render
>> - *         context)
>> - *
>>    * Return:    An i915_guc_client object if success, else NULL.
>>    */
>>   static struct i915_guc_client *
>>   guc_client_alloc(struct drm_i915_private *dev_priv,
>> -         uint32_t engines,
>> -         uint32_t priority,
>> -         struct i915_gem_context *ctx)
>> +         uint32_t priority)
>>   {
>>       struct i915_guc_client *client;
>>       struct intel_guc *guc = &dev_priv->guc;
>> @@ -864,9 +858,9 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
>>       if (!client)
>>           return NULL;
>>   -    client->owner = ctx;
>> +    client->owner = dev_priv->kernel_context;
>>       client->guc = guc;
>> -    client->engines = engines;
>> +    client->engines = INTEL_INFO(dev_priv)->ring_mask;
>>       client->priority = priority;
>>       client->doorbell_id = GUC_INVALID_DOORBELL_ID;
>>   @@ -1062,9 +1056,7 @@ int i915_guc_submission_init(struct
>> drm_i915_private *dev_priv)
>>       guc_addon_create(guc);
>>         guc->execbuf_client = guc_client_alloc(dev_priv,
>> -                           INTEL_INFO(dev_priv)->ring_mask,
>> -                           GUC_CTX_PRIORITY_KMD_NORMAL,
>> -                           dev_priv->kernel_context);
>> +                           GUC_CTX_PRIORITY_KMD_NORMAL);
>>       if (!guc->execbuf_client) {
>>           DRM_ERROR("Failed to create GuC client for execbuf!\n");
>>           goto err;
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx