| Message ID | 20170223191421.4502-2-michal.winiarski@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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?). > 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;
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
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;
We're always using all engines and kernel context for guc clients, let's remove those arguments from guc_client_alloc. 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(-)