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

Daniele Ceraolo Spurio
2017-02-24T01:39:46Z

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