drm/i915/guc: local optimisations and updating comments

Tidying up guc_init_proc_desc() and adding commentary to the client
structure after the recent change in GuC page mapping strategy.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1461078516-28678-1-git-send-email-david.s.gordon@intel.com
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 4718b7a51f5d578a6dc6b1470c199bb7b87528c5..d40c13fb6643773c41774b7f8519a804bf33b39b 100644 (file)
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -358,12 +358,14 @@ static void guc_init_proc_desc(struct intel_guc *guc,
 static void guc_init_ctx_desc(struct intel_guc *guc,
                              struct i915_guc_client *client)
 {
+       struct drm_i915_gem_object *client_obj = client->client_obj;
        struct drm_i915_private *dev_priv = guc_to_i915(guc);
        struct intel_engine_cs *engine;
        struct intel_context *ctx = client->owner;
        struct guc_context_desc desc;
        struct sg_table *sg;
        enum intel_engine_id id;
+       u32 gfx_addr;
 
        memset(&desc, 0, sizeof(desc));
 
@@ -392,16 +394,17 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
                lrc->context_desc = (u32)ctx_desc;
 
                /* The state page is after PPHWSP */
-               lrc->ring_lcra = i915_gem_obj_ggtt_offset(obj) +
-                               LRC_STATE_PN * PAGE_SIZE;
+               gfx_addr = i915_gem_obj_ggtt_offset(obj);
+               lrc->ring_lcra = gfx_addr + LRC_STATE_PN * PAGE_SIZE;
                lrc->context_id = (client->ctx_index << GUC_ELC_CTXID_OFFSET) |
                                (engine->guc_id << GUC_ELC_ENGINE_OFFSET);
 
                obj = ctx->engine[id].ringbuf->obj;
+               gfx_addr = i915_gem_obj_ggtt_offset(obj);
 
-               lrc->ring_begin = i915_gem_obj_ggtt_offset(obj);
-               lrc->ring_end = lrc->ring_begin + obj->base.size - 1;
-               lrc->ring_next_free_location = lrc->ring_begin;
+               lrc->ring_begin = gfx_addr;
+               lrc->ring_end = gfx_addr + obj->base.size - 1;
+               lrc->ring_next_free_location = gfx_addr;
                lrc->ring_current_tail_pointer_value = 0;
 
                desc.engines_used |= (1 << engine->guc_id);
@@ -410,22 +413,17 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
        WARN_ON(desc.engines_used == 0);
 
        /*
-        * The CPU address is only needed at certain points, so kmap_atomic on
-        * demand instead of storing it in the ctx descriptor.
-        * XXX: May make debug easier to have it mapped
+        * The doorbell, process descriptor, and workqueue are all parts
+        * of the client object, which the GuC will reference via the GGTT
         */
-       desc.db_trigger_cpu = 0;
-       desc.db_trigger_uk = client->doorbell_offset +
-               i915_gem_obj_ggtt_offset(client->client_obj);
-       desc.db_trigger_phy = client->doorbell_offset +
-               sg_dma_address(client->client_obj->pages->sgl);
-
-       desc.process_desc = client->proc_desc_offset +
-               i915_gem_obj_ggtt_offset(client->client_obj);
-
-       desc.wq_addr = client->wq_offset +
-               i915_gem_obj_ggtt_offset(client->client_obj);
-
+       gfx_addr = i915_gem_obj_ggtt_offset(client_obj);
+       desc.db_trigger_phy = sg_dma_address(client_obj->pages->sgl) +
+                               client->doorbell_offset;
+       desc.db_trigger_cpu = (uintptr_t)client->client_base +
+                               client->doorbell_offset;
+       desc.db_trigger_uk = gfx_addr + client->doorbell_offset;
+       desc.process_desc = gfx_addr + client->proc_desc_offset;
+       desc.wq_addr = gfx_addr + client->wq_offset;
        desc.wq_size = client->wq_size;
 
        /*

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 19ca5933a0908328bc4f8e3eb9b75d89ad834a11..9d79c4c3e256d21fbecb579bd73ef86cf3f332d8 100644 (file)
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -29,6 +29,29 @@
 
 struct drm_i915_gem_request;
 
+/*
+ * This structure primarily describes the GEM object shared with the GuC.
+ * The 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 decriptor" 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.
+ *
+ * Finally, we also keep a few statistics here, including the number of
+ * submissions to each engine, and a record of the last submission failure
+ * (if any).
+ */
 struct i915_guc_client {
        struct drm_i915_gem_object *client_obj;
        void *client_base;              /* first page (only) of above   */