[Intel-gfx] [CI 4/5] drm/i915/guc: kill the GuC client
Daniele Ceraolo Spurio
daniele.ceraolospurio at intel.com
Wed Nov 20 00:13:27 UTC 2019
<snip>
> @@ -220,7 +200,7 @@ static void guc_wq_item_append(struct intel_guc_client *client,
> GEM_BUG_ON(wq_off & (wqi_size - 1));
>
> /* WQ starts from the page after process_desc */
Just realized that I stupidly forgot to actually commit the fix...
I'll send the fixed (for real) version after the bugfix for bug 112205
is merged, so I can get a clean CI run.
Daniele
> - wqi = client->vaddr + wq_off + GUC_PD_SIZE;
> + wqi = guc->workqueue_vaddr + wq_off;
>
> /* Now fill in the 4-word work queue item */
> wqi->header = WQ_TYPE_INORDER |
> @@ -238,12 +218,11 @@ static void guc_wq_item_append(struct intel_guc_client *client,
>
> static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
> {
> - struct intel_guc_client *client = guc->execbuf_client;
> struct intel_engine_cs *engine = rq->engine;
> u32 ctx_desc = lower_32_bits(rq->hw_context->lrc_desc);
> u32 ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
>
> - guc_wq_item_append(client, engine->guc_id, ctx_desc,
> + guc_wq_item_append(guc, engine->guc_id, ctx_desc,
> ring_tail, rq->fence.seqno);
> }
>
> @@ -267,9 +246,8 @@ static void guc_submit(struct intel_engine_cs *engine,
> struct i915_request **end)
> {
> struct intel_guc *guc = &engine->gt->uc.guc;
> - struct intel_guc_client *client = guc->execbuf_client;
>
> - spin_lock(&client->wq_lock);
> + spin_lock(&guc->wq_lock);
>
> do {
> struct i915_request *rq = *out++;
> @@ -278,7 +256,7 @@ static void guc_submit(struct intel_engine_cs *engine,
> guc_add_request(guc, rq);
> } while (out != end);
>
> - spin_unlock(&client->wq_lock);
> + spin_unlock(&guc->wq_lock);
> }
>
> static inline int rq_prio(const struct i915_request *rq)
> @@ -529,126 +507,6 @@ static void guc_reset_finish(struct intel_engine_cs *engine)
> * path of guc_submit() above.
> */
>
> -/**
> - * guc_client_alloc() - Allocate an intel_guc_client
> - * @guc: the intel_guc structure
> - * @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.
> - *
> - * Return: An intel_guc_client object if success, else NULL.
> - */
> -static struct intel_guc_client *
> -guc_client_alloc(struct intel_guc *guc, u32 priority)
> -{
> - struct intel_guc_client *client;
> - struct i915_vma *vma;
> - void *vaddr;
> - int ret;
> -
> - client = kzalloc(sizeof(*client), GFP_KERNEL);
> - if (!client)
> - return ERR_PTR(-ENOMEM);
> -
> - client->guc = guc;
> - client->priority = priority;
> - spin_lock_init(&client->wq_lock);
> -
> - ret = ida_simple_get(&guc->stage_ids, 0, GUC_MAX_STAGE_DESCRIPTORS,
> - GFP_KERNEL);
> - if (ret < 0)
> - goto err_client;
> -
> - client->stage_id = ret;
> -
> - /* The first page is proc_desc. Two following pages are wq. */
> - vma = intel_guc_allocate_vma(guc, GUC_PD_SIZE + GUC_WQ_SIZE);
> - if (IS_ERR(vma)) {
> - ret = PTR_ERR(vma);
> - goto err_id;
> - }
> -
> - /* We'll keep just the first (doorbell/proc) page permanently kmap'd. */
> - client->vma = vma;
> -
> - vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
> - if (IS_ERR(vaddr)) {
> - ret = PTR_ERR(vaddr);
> - goto err_vma;
> - }
> - client->vaddr = vaddr;
> -
> - DRM_DEBUG_DRIVER("new priority %u client %p: stage_id %u\n",
> - priority, client, client->stage_id);
> -
> - return client;
> -
> -err_vma:
> - i915_vma_unpin_and_release(&client->vma, 0);
> -err_id:
> - ida_simple_remove(&guc->stage_ids, client->stage_id);
> -err_client:
> - kfree(client);
> - return ERR_PTR(ret);
> -}
> -
> -static void guc_client_free(struct intel_guc_client *client)
> -{
> - i915_vma_unpin_and_release(&client->vma, I915_VMA_RELEASE_MAP);
> - ida_simple_remove(&client->guc->stage_ids, client->stage_id);
> - kfree(client);
> -}
> -
> -static int guc_clients_create(struct intel_guc *guc)
> -{
> - struct intel_guc_client *client;
> -
> - GEM_BUG_ON(guc->execbuf_client);
> -
> - client = guc_client_alloc(guc, GUC_CLIENT_PRIORITY_KMD_NORMAL);
> - if (IS_ERR(client)) {
> - DRM_ERROR("Failed to create GuC client for submission!\n");
> - return PTR_ERR(client);
> - }
> - guc->execbuf_client = client;
> -
> - return 0;
> -}
> -
> -static void guc_clients_destroy(struct intel_guc *guc)
> -{
> - struct intel_guc_client *client;
> -
> - client = fetch_and_zero(&guc->execbuf_client);
> - if (client)
> - guc_client_free(client);
> -}
> -
> -static void __guc_client_enable(struct intel_guc_client *client)
> -{
> - guc_proc_desc_init(client);
> - guc_stage_desc_init(client);
> -}
> -
> -static void __guc_client_disable(struct intel_guc_client *client)
> -{
> - /* Note: By the time we're here, GuC may have already been reset */
> - guc_stage_desc_fini(client);
> - guc_proc_desc_fini(client);
> -}
> -
> -static void guc_clients_enable(struct intel_guc *guc)
> -{
> - __guc_client_enable(guc->execbuf_client);
> -}
> -
> -static void guc_clients_disable(struct intel_guc *guc)
> -{
> - if (guc->execbuf_client)
> - __guc_client_disable(guc->execbuf_client);
> -}
> -
> /*
> * Set up the memory resources to be shared with the GuC (via the GGTT)
> * at firmware loading time.
> @@ -669,12 +527,20 @@ int intel_guc_submission_init(struct intel_guc *guc)
> */
> GEM_BUG_ON(!guc->stage_desc_pool);
>
> - ret = guc_clients_create(guc);
> + ret = guc_workqueue_create(guc);
> if (ret)
> goto err_pool;
>
> + ret = guc_proc_desc_create(guc);
> + if (ret)
> + goto err_workqueue;
> +
> + spin_lock_init(&guc->wq_lock);
> +
> return 0;
>
> +err_workqueue:
> + guc_workqueue_destroy(guc);
> err_pool:
> guc_stage_desc_pool_destroy(guc);
> return ret;
> @@ -682,10 +548,11 @@ int intel_guc_submission_init(struct intel_guc *guc)
>
> void intel_guc_submission_fini(struct intel_guc *guc)
> {
> - guc_clients_destroy(guc);
> -
> - if (guc->stage_desc_pool)
> + if (guc->stage_desc_pool) {
> + guc_proc_desc_destroy(guc);
> + guc_workqueue_destroy(guc);
> guc_stage_desc_pool_destroy(guc);
> + }
> }
>
> static void guc_interrupts_capture(struct intel_gt *gt)
> @@ -771,9 +638,8 @@ void intel_guc_submission_enable(struct intel_guc *guc)
> sizeof(struct guc_wq_item) *
> I915_NUM_ENGINES > GUC_WQ_SIZE);
>
> - GEM_BUG_ON(!guc->execbuf_client);
> -
> - guc_clients_enable(guc);
> + guc_proc_desc_init(guc);
> + guc_stage_desc_init(guc);
>
> /* Take over from manual control of ELSP (execlists) */
> guc_interrupts_capture(gt);
> @@ -790,8 +656,12 @@ void intel_guc_submission_disable(struct intel_guc *guc)
>
> GEM_BUG_ON(gt->awake); /* GT should be parked first */
>
> + /* Note: By the time we're here, GuC may have already been reset */
> +
> guc_interrupts_release(gt);
> - guc_clients_disable(guc);
> +
> + guc_stage_desc_fini(guc);
> + guc_proc_desc_fini(guc);
> }
>
> static bool __guc_submission_support(struct intel_guc *guc)
> @@ -809,3 +679,8 @@ void intel_guc_submission_init_early(struct intel_guc *guc)
> {
> guc->submission_supported = __guc_submission_support(guc);
> }
> +
> +bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine)
> +{
> + return engine->set_default_submission == guc_set_default_submission;
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> index e2deb4e6f42a..e402a2932592 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> @@ -6,48 +6,10 @@
> #ifndef _INTEL_GUC_SUBMISSION_H_
> #define _INTEL_GUC_SUBMISSION_H_
>
> -#include <linux/spinlock.h>
> +#include <linux/types.h>
>
> -#include "gt/intel_engine_types.h"
> -
> -#include "i915_gem.h"
> -#include "i915_selftest.h"
> -
> -struct drm_i915_private;
> -
> -/*
> - * This structure primarily describes the GEM object shared with the GuC.
> - * The specs sometimes refer to this object as a "GuC context", but we use
> - * the term "client" to avoid confusion with hardware contexts. This
> - * GEM object is held for the entire lifetime of our interaction with
> - * the GuC, being allocated before the GuC is loaded with its firmware.
> - * Because there's no way to update the address used by the GuC after
> - * initialisation, the shared object must stay pinned into the GGTT as
> - * long as the GuC is in use. We also keep the first page (only) mapped
> - * into kernel address space, as it includes shared data that must be
> - * updated on every request submission.
> - *
> - * The single GEM object described here is actually made up of several
> - * separate areas, as far as the GuC is concerned. The first page (kept
> - * kmap'd) includes the "process descriptor" which holds sequence data for
> - * the doorbell, and one cacheline which actually *is* the doorbell; a
> - * write to this will "ring the doorbell" (i.e. send an interrupt to the
> - * GuC). The subsequent pages of the client object constitute the work
> - * queue (a circular array of work items), again described in the process
> - * descriptor. Work queue pages are mapped momentarily as required.
> - */
> -struct intel_guc_client {
> - struct i915_vma *vma;
> - void *vaddr;
> - struct intel_guc *guc;
> -
> - /* bitmap of (host) engine ids */
> - u32 priority;
> - u32 stage_id;
> -
> - /* Protects GuC client's WQ access */
> - spinlock_t wq_lock;
> -};
> +struct intel_guc;
> +struct intel_engine_cs;
>
> void intel_guc_submission_init_early(struct intel_guc *guc);
> int intel_guc_submission_init(struct intel_guc *guc);
> @@ -56,5 +18,6 @@ void intel_guc_submission_disable(struct intel_guc *guc);
> void intel_guc_submission_fini(struct intel_guc *guc);
> int intel_guc_preempt_work_create(struct intel_guc *guc);
> void intel_guc_preempt_work_destroy(struct intel_guc *guc);
> +bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine);
>
> #endif
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5d5974e7f3ed..f32e7b016197 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1802,23 +1802,12 @@ static void i915_guc_log_info(struct seq_file *m,
> static int i915_guc_info(struct seq_file *m, void *data)
> {
> struct drm_i915_private *dev_priv = node_to_i915(m->private);
> - const struct intel_guc *guc = &dev_priv->gt.uc.guc;
> - struct intel_guc_client *client = guc->execbuf_client;
>
> if (!USES_GUC(dev_priv))
> return -ENODEV;
>
> i915_guc_log_info(m, dev_priv);
>
> - if (!USES_GUC_SUBMISSION(dev_priv))
> - return 0;
> -
> - GEM_BUG_ON(!guc->execbuf_client);
> -
> - seq_printf(m, "\nGuC execbuf client @ %p:\n", client);
> - seq_printf(m, "\tPriority %d, GuC stage index: %u\n",
> - client->priority,
> - client->stage_id);
> /* Add more as required ... */
>
> return 0;
>
More information about the Intel-gfx
mailing list