[Intel-gfx] [PATCH v4 3/6] drm/i915/guc: refactor doorbell management code
Yu Dai
yu.dai at intel.com
Thu Apr 7 21:26:43 UTC 2016
On 04/07/2016 10:21 AM, Dave Gordon wrote:
> During a hibernate/resume cycle, the driver, the GuC, and the doorbell
> hardware can end up in inconsistent states. This patch refactors the
> driver's handling and tracking of doorbells, in preparation for a later
> one which will resolve the issue.
>
> Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
> ---
> drivers/gpu/drm/i915/i915_guc_submission.c | 88 ++++++++++++++++++------------
> 1 file changed, 53 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 2171759..2fc69f1 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -175,8 +175,48 @@ static int host2guc_sample_forcewake(struct intel_guc *guc,
> * client object which contains the page being used for the doorbell
> */
>
> +static int guc_update_doorbell_id(struct i915_guc_client *client,
> + struct guc_doorbell_info *doorbell,
> + u16 new_id)
> +{
> + struct sg_table *sg = client->guc->ctx_pool_obj->pages;
> + void *doorbell_bitmap = client->guc->doorbell_bitmap;
> + struct guc_context_desc desc;
> + size_t len;
> +
> + if (client->doorbell_id != GUC_INVALID_DOORBELL_ID &&
> + test_bit(client->doorbell_id, doorbell_bitmap)) {
> + /* Deactivate the old doorbell */
> + doorbell->db_status = GUC_DOORBELL_DISABLED;
> + (void)host2guc_release_doorbell(client->guc, client);
> + clear_bit(client->doorbell_id, doorbell_bitmap);
> + }
> +
> + /* Update the GuC's idea of the doorbell ID */
> + len = sg_pcopy_to_buffer(sg->sgl, sg->nents, &desc, sizeof(desc),
> + sizeof(desc) * client->ctx_index);
> + if (len != sizeof(desc))
> + return -EFAULT;
> + desc.db_id = new_id;
> + len = sg_pcopy_from_buffer(sg->sgl, sg->nents, &desc, sizeof(desc),
> + sizeof(desc) * client->ctx_index);
> + if (len != sizeof(desc))
> + return -EFAULT;
> +
We may cache the vmap of context pool for its life cycle to avoid these
copies. That is why a generic vmap helper function is really nice to have.
Alex
> + client->doorbell_id = new_id;
> + if (new_id == GUC_INVALID_DOORBELL_ID)
> + return 0;
> +
> + /* Activate the new doorbell */
> + set_bit(client->doorbell_id, doorbell_bitmap);
> + doorbell->db_status = GUC_DOORBELL_ENABLED;
> + doorbell->cookie = 0;
> + return host2guc_allocate_doorbell(client->guc, client);
> +}
> +
> static void guc_init_doorbell(struct intel_guc *guc,
> - struct i915_guc_client *client)
> + struct i915_guc_client *client,
> + uint16_t db_id)
> {
> struct guc_doorbell_info *doorbell;
> void *base;
> @@ -184,8 +224,7 @@ static void guc_init_doorbell(struct intel_guc *guc,
> base = kmap_atomic(i915_gem_object_get_page(client->client_obj, 0));
> doorbell = base + client->doorbell_offset;
>
> - doorbell->db_status = 1;
> - doorbell->cookie = 0;
> + guc_update_doorbell_id(client, doorbell, db_id);
>
> kunmap_atomic(base);
> }
> @@ -193,27 +232,16 @@ static void guc_init_doorbell(struct intel_guc *guc,
> static void guc_disable_doorbell(struct intel_guc *guc,
> struct i915_guc_client *client)
> {
> - struct drm_i915_private *dev_priv = guc_to_i915(guc);
> struct guc_doorbell_info *doorbell;
> void *base;
> - i915_reg_t drbreg = GEN8_DRBREGL(client->doorbell_id);
> - int value;
>
> base = kmap_atomic(i915_gem_object_get_page(client->client_obj, 0));
> doorbell = base + client->doorbell_offset;
>
> - doorbell->db_status = 0;
> + guc_update_doorbell_id(client, doorbell, GUC_INVALID_DOORBELL_ID);
>
> kunmap_atomic(base);
>
> - I915_WRITE(drbreg, I915_READ(drbreg) & ~GEN8_DRB_VALID);
> -
> - value = I915_READ(drbreg);
> - WARN_ON((value & GEN8_DRB_VALID) != 0);
> -
> - I915_WRITE(GEN8_DRBREGU(client->doorbell_id), 0);
> - I915_WRITE(drbreg, 0);
> -
> /* XXX: wait for any interrupts */
> /* XXX: wait for workqueue to drain */
> }
> @@ -260,7 +288,7 @@ static uint16_t assign_doorbell(struct intel_guc *guc, uint32_t priority)
> if (id == end)
> id = GUC_INVALID_DOORBELL_ID;
> else
> - bitmap_set(guc->doorbell_bitmap, id, 1);
> + set_bit(id, guc->doorbell_bitmap);
>
> DRM_DEBUG_DRIVER("assigned %s priority doorbell id 0x%x\n",
> hi_pri ? "high" : "normal", id);
> @@ -268,11 +296,6 @@ static uint16_t assign_doorbell(struct intel_guc *guc, uint32_t priority)
> return id;
> }
>
> -static void release_doorbell(struct intel_guc *guc, uint16_t id)
> -{
> - bitmap_clear(guc->doorbell_bitmap, id, 1);
> -}
> -
> /*
> * Initialise the process descriptor shared with the GuC firmware.
> */
> @@ -667,15 +690,7 @@ static void guc_client_free(struct drm_device *dev,
> if (!client)
> return;
>
> - if (client->doorbell_id != GUC_INVALID_DOORBELL_ID) {
> - /*
> - * First disable the doorbell, then tell the GuC we've
> - * finished with it, finally deallocate it in our bitmap
> - */
> - guc_disable_doorbell(guc, client);
> - host2guc_release_doorbell(guc, client);
> - release_doorbell(guc, client->doorbell_id);
> - }
> + guc_disable_doorbell(guc, client);
>
> /*
> * XXX: wait for any outstanding submissions before freeing memory.
> @@ -712,6 +727,7 @@ static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_guc *guc = &dev_priv->guc;
> struct drm_i915_gem_object *obj;
> + uint16_t db_id;
>
> client = kzalloc(sizeof(*client), GFP_KERNEL);
> if (!client)
> @@ -750,22 +766,24 @@ static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
> else
> client->proc_desc_offset = (GUC_DB_SIZE / 2);
>
> - client->doorbell_id = assign_doorbell(guc, client->priority);
> - if (client->doorbell_id == GUC_INVALID_DOORBELL_ID)
> + db_id = assign_doorbell(guc, client->priority);
> + if (db_id == GUC_INVALID_DOORBELL_ID)
> /* XXX: evict a doorbell instead */
> goto err;
>
> guc_init_proc_desc(guc, client);
> guc_init_ctx_desc(guc, client);
> - guc_init_doorbell(guc, client);
> + guc_init_doorbell(guc, client, db_id);
>
> /* XXX: Any cache flushes needed? General domain mgmt calls? */
>
> if (host2guc_allocate_doorbell(guc, client))
> goto err;
>
> - DRM_DEBUG_DRIVER("new priority %u client %p: ctx_index %u db_id %u\n",
> - priority, client, client->ctx_index, client->doorbell_id);
> + DRM_DEBUG_DRIVER("new priority %u client %p: ctx_index %u\n",
> + priority, client, client->ctx_index);
> + DRM_DEBUG_DRIVER("doorbell id %u, cacheline offset 0x%x\n",
> + client->doorbell_id, client->doorbell_offset);
>
> return client;
>
More information about the Intel-gfx
mailing list