On 14/02/17 05:53, Joonas Lahtinen wrote:
Started adding proper teardown to guc_client_alloc, ended up removing
quite a few dead ends where errors communicating with the GuC were
silently ignored. There also seemed to be quite a few erronous
teardown actions performed in case of an error (ordering wrong).
v2:
- Increase function symmetry/proximity (Michal/Daniele)
- Fix __reserve_doorbell accounting for high priority (Daniele)
- Call __update_doorbell_desc! (Daniele)
- Isolate __guc_{,de}allocate_doorbell (Michal/Daniele)
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx>
Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_debugfs.c | 4 +-
drivers/gpu/drm/i915/i915_guc_submission.c | 371 ++++++++++++++++-------------
drivers/gpu/drm/i915/intel_guc_fwif.h | 4 +-
drivers/gpu/drm/i915/intel_uc.h | 11 +-
4 files changed, 215 insertions(+), 175 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index cd023fd..f3972cf 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2492,7 +2492,7 @@ static void i915_guc_client_info(struct seq_file *m,
seq_printf(m, "\tPriority %d, GuC ctx index: %u, PD offset 0x%x\n",
client->priority, client->ctx_index, client->proc_desc_offset);
- seq_printf(m, "\tDoorbell id %d, offset: 0x%x, cookie 0x%x\n",
+ seq_printf(m, "\tDoorbell id %d, offset: 0x%lx, cookie 0x%x\n",
client->doorbell_id, client->doorbell_offset, client->doorbell_cookie);
seq_printf(m, "\tWQ size %d, offset: 0x%x, tail %d\n",
client->wq_size, client->wq_offset, client->wq_tail);
@@ -2527,7 +2527,7 @@ static int i915_guc_info(struct seq_file *m, void *data)
}
seq_printf(m, "Doorbell map:\n");
- seq_printf(m, "\t%*pb\n", GUC_MAX_DOORBELLS, guc->doorbell_bitmap);
+ seq_printf(m, "\t%*pb\n", GUC_NUM_DOORBELLS, guc->doorbell_bitmap);
seq_printf(m, "Doorbell next cacheline: 0x%x\n\n", guc->db_cacheline);
seq_printf(m, "GuC total action count: %llu\n", guc->action_count);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 8ced9e2..258fca3 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -62,27 +62,73 @@
*
*/
+static inline bool is_high_priority(struct i915_guc_client* client)
+{
+ return client->priority <= GUC_CTX_PRIORITY_HIGH;
+}
+
+static int __reserve_doorbell(struct i915_guc_client *client)
+{
+ unsigned long offset;
+ unsigned long end;
+ u16 id;
+
+ GEM_BUG_ON(client->doorbell_id != GUC_DOORBELL_INVALID);
+
+ /*
+ * The bitmap tracks which doorbell registers are currently in use.
+ * It is split into two halves; the first half is used for normal
+ * priority contexts, the second half for high-priority ones.
+ * Note that logically higher priorities are numerically less than
+ * normal ones, so the test below means "is it high-priority?"
+ */
+
+ offset = 0;
+ end = GUC_NUM_DOORBELLS/2;
+ if (is_high_priority(client)) {
+ offset = end;
+ end += offset;
+ }
+
+ id = find_next_zero_bit(client->guc->doorbell_bitmap, offset, end);
+ if (id == end)
+ return -ENOSPC;
+
+ __set_bit(id, client->guc->doorbell_bitmap);
+ client->doorbell_id = id;
+ DRM_DEBUG_DRIVER("client %u (high prio=%s) reserved doorbell %d: \n",
+ client->ctx_index, yesno(is_high_priority(client)),
+ id);
+ return 0;
+}
+
+static void __unreserve_doorbell(struct i915_guc_client *client)
+{
+ GEM_BUG_ON(client->doorbell_id == GUC_DOORBELL_INVALID);
+
+ __clear_bit(client->doorbell_id, client->guc->doorbell_bitmap);
+ client->doorbell_id = GUC_DOORBELL_INVALID;
+}
+
/*
* Tell the GuC to allocate or deallocate a specific doorbell
*/
-static int guc_allocate_doorbell(struct intel_guc *guc,
- struct i915_guc_client *client)
+static int __guc_allocate_doorbell(struct intel_guc *guc, u32 ctx_index)
{
u32 action[] = {
INTEL_GUC_ACTION_ALLOCATE_DOORBELL,
- client->ctx_index
+ ctx_index
};
return intel_guc_send(guc, action, ARRAY_SIZE(action));
}
-static int guc_release_doorbell(struct intel_guc *guc,
- struct i915_guc_client *client)
+static int __guc_deallocate_doorbell(struct intel_guc *guc, u32 ctx_index)
{
u32 action[] = {
INTEL_GUC_ACTION_DEALLOCATE_DOORBELL,
- client->ctx_index
+ ctx_index
};
return intel_guc_send(guc, action, ARRAY_SIZE(action));
@@ -95,104 +141,98 @@ static int guc_release_doorbell(struct intel_guc *guc,
* client object which contains the page being used for the doorbell
*/
-static int guc_update_doorbell_id(struct intel_guc *guc,
- struct i915_guc_client *client,
- u16 new_id)
+static int __update_doorbell_desc(struct i915_guc_client *client, u16 new_id)
{
- struct sg_table *sg = guc->ctx_pool_vma->pages;
- void *doorbell_bitmap = guc->doorbell_bitmap;
- struct guc_doorbell_info *doorbell;
+ struct sg_table *sg = client->guc->ctx_pool_vma->pages;
struct guc_context_desc desc;
size_t len;
- doorbell = client->vaddr + client->doorbell_offset;
-
- 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)guc_release_doorbell(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);
+ 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);
+ sizeof(desc) * client->ctx_index);
if (len != sizeof(desc))
return -EFAULT;
- client->doorbell_id = new_id;
- if (new_id == GUC_INVALID_DOORBELL_ID)
- return 0;
+ return 0;
+}
+
+static struct guc_doorbell_info *__get_doorbell(struct i915_guc_client *client)
+{
+ return client->vaddr + client->doorbell_offset;
+}
+
+static bool has_doorbell(struct i915_guc_client *client)
+{
+ if (client->doorbell_id == GUC_DOORBELL_INVALID)
+ return false;
- /* Activate the new doorbell */
- __set_bit(new_id, doorbell_bitmap);
+ return test_bit(client->doorbell_id, client->guc->doorbell_bitmap);
Should we warn/gem_bug if client->doorbell_id != GUC_DOORBELL_INVALID
and the bit in guc->doorbell_bitmap is not set? Not sure if you plan to
decouple them more in the future, but with the current code it should
always be impossible to have a valid doorbell without the bit in the
bitmask being set. Maybe we can just return client->doorbell_id !=
GUC_DOORBELL_INVALID here if we have no plan for a case where they can
be out of sync.
+}
+
+static int __create_doorbell(struct i915_guc_client *client)
+{
+ struct guc_doorbell_info *doorbell;
+ int err;
+
+ doorbell = __get_doorbell(client);
doorbell->db_status = GUC_DOORBELL_ENABLED;
doorbell->cookie = client->doorbell_cookie;
- return guc_allocate_doorbell(guc, client);
+
+ err = __guc_allocate_doorbell(client->guc, client->ctx_index);
+ if (err) {
+ doorbell->db_status = GUC_DOORBELL_DISABLED;
+ doorbell->cookie = 0;
+ }
+ return err;
}
-static void guc_disable_doorbell(struct intel_guc *guc,
- struct i915_guc_client *client)
+static int __destroy_doorbell(struct i915_guc_client *client)
{
- (void)guc_update_doorbell_id(guc, client, GUC_INVALID_DOORBELL_ID);
+ struct guc_doorbell_info *doorbell;
- /* XXX: wait for any interrupts */
- /* XXX: wait for workqueue to drain */
+ doorbell = __get_doorbell(client);
+ doorbell->db_status = GUC_DOORBELL_DISABLED;
+ doorbell->cookie = 0;
+
Not from this patch (but since we're at it...), I did a bit of digging
and I've found that doorbell release flow requires SW to poll the
GEN8_DRBREGL(db_id) register after updating doorbell->db_status to wait
for the GEN8_DRB_VALID bit to go to zero. This ensures that any pending
events are processed before we call into GuC to release the doorbell.
Note that GuC will fail the DEALLOCATE_DOORBELL action if the bit has
not gone to zero yet. This is currently not an issue, probably because
we use a single doorbell and we don't ring it near release time, but the
situation could change in the future so I believe it is worth to fix it
now. I can follow up with an additional patch if you want to keep this
one as refactoring only.
+ return __guc_deallocate_doorbell(client->guc, client->ctx_index);
}
-static uint16_t
-select_doorbell_register(struct intel_guc *guc, uint32_t priority)
+static int destroy_doorbell(struct i915_guc_client *client)
{
- /*
- * The bitmap tracks which doorbell registers are currently in use.
- * It is split into two halves; the first half is used for normal
- * priority contexts, the second half for high-priority ones.
- * Note that logically higher priorities are numerically less than
- * normal ones, so the test below means "is it high-priority?"
- */
- const bool hi_pri = (priority <= GUC_CTX_PRIORITY_HIGH);
- const uint16_t half = GUC_MAX_DOORBELLS / 2;
- const uint16_t start = hi_pri ? half : 0;
- const uint16_t end = start + half;
- uint16_t id;
+ int err;
- id = find_next_zero_bit(guc->doorbell_bitmap, end, start);
- if (id == end)
- id = GUC_INVALID_DOORBELL_ID;
+ GEM_BUG_ON(!has_doorbell(client));
- DRM_DEBUG_DRIVER("assigned %s priority doorbell id 0x%x\n",
- hi_pri ? "high" : "normal", id);
+ /* XXX: wait for any interrupts */
+ /* XXX: wait for workqueue to drain */
- return id;
-}
+ err = __destroy_doorbell(client);
+ if (err)
+ return err;
-/*
- * Select, assign and relase doorbell cachelines
- *
- * These functions track which doorbell cachelines are in use.
- * The data they manipulate is protected by the intel_guc_send lock.
- */
+ __unreserve_doorbell(client);
-static uint32_t select_doorbell_cacheline(struct intel_guc *guc)
+ return 0;
+}
+
+static unsigned long __reserve_cacheline(struct intel_guc* guc)
"reserve_cacheline" doesn't really reflect what happens, because more
doorbells can use the same cacheline (while they are on different
physical pages) and there is no concept of unreserving the cacheline.
The idea is to try to avoid having lots of doorbells on the same
cacheline if possible to make the monitoring more efficient on the HW,
so I'd keep the "select_cacheline" naming for this function. We should
probably also look at modifying the function to use something more
elaborated than a simple round robin scheme to ensure doorbells are
equally distribuited on cachelines, but that can probably wait until we
use more doorbells.
{
- const uint32_t cacheline_size = cache_line_size();
- uint32_t offset;
+ unsigned long offset;
/* Doorbell uses a single cache line within a page */
offset = offset_in_page(guc->db_cacheline);
/* Moving to next cache line to reduce contention */
- guc->db_cacheline += cacheline_size;
-
- DRM_DEBUG_DRIVER("selected doorbell cacheline 0x%x, next 0x%x, linesize %u\n",
- offset, guc->db_cacheline, cacheline_size);
+ guc->db_cacheline += cache_line_size();
+ DRM_DEBUG_DRIVER("reserved cacheline 0x%lx, next 0x%x, linesize %u\n",
+ offset, guc->db_cacheline, cache_line_size());
return offset;
}
@@ -584,93 +624,84 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
return vma;
}
-static void
-guc_client_free(struct drm_i915_private *dev_priv,
- struct i915_guc_client *client)
+static void guc_client_free(struct i915_guc_client *client)
{
- struct intel_guc *guc = &dev_priv->guc;
-
- if (!client)
- return;
-
/*
* XXX: wait for any outstanding submissions before freeing memory.
* Be sure to drop any locks
*/
-
- if (client->vaddr) {
- /*
- * If we got as far as setting up a doorbell, make sure we
- * shut it down before unmapping & deallocating the memory.
- */
- guc_disable_doorbell(guc, client);
-
- i915_gem_object_unpin_map(client->vma->obj);
- }
-
+ WARN_ON(destroy_doorbell(client));
+ guc_ctx_desc_fini(client->guc, client);
+ i915_gem_object_unpin_map(client->vma->obj);
i915_vma_unpin_and_release(&client->vma);
-
- if (client->ctx_index != GUC_INVALID_CTX_ID) {
- guc_ctx_desc_fini(guc, client);
- ida_simple_remove(&guc->ctx_ids, client->ctx_index);
- }
-
+ ida_simple_remove(&client->guc->ctx_ids, client->ctx_index);
kfree(client);
}
/* Check that a doorbell register is in the expected state */
-static bool guc_doorbell_check(struct intel_guc *guc, uint16_t db_id)
+static bool doorbell_ok(struct intel_guc *guc, u16 db_id)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
- i915_reg_t drbreg = GEN8_DRBREGL(db_id);
- uint32_t value = I915_READ(drbreg);
- bool enabled = (value & GUC_DOORBELL_ENABLED) != 0;
- bool expected = test_bit(db_id, guc->doorbell_bitmap);
+ u32 drbregl;
+ bool valid;
+
+ GEM_BUG_ON(db_id >= GUC_DOORBELL_INVALID);
- if (enabled == expected)
+ drbregl = I915_READ(GEN8_DRBREGL(db_id));
+ valid = drbregl & GEN8_DRB_VALID;
+
+ if (test_bit(db_id, guc->doorbell_bitmap) == valid)
return true;
- DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) 0x%x, should be %s\n",
- db_id, drbreg.reg, value,
- expected ? "active" : "inactive");
+ DRM_DEBUG_DRIVER("Doorbell %d has unexpected state (0x%x): valid=%s\n",
+ db_id, drbregl, yesno(valid));
return false;
}
+static int __reset_doorbell(struct i915_guc_client* client, u16 db_id)
+{
+ int err;
+
+ err = __update_doorbell_desc(client, db_id);
+ if (!err)
+ err = __create_doorbell(client);
+ if (!err)
+ err = __destroy_doorbell(client);
+
+ return err;
+}
+
/*
* Borrow the first client to set up & tear down each unused doorbell
* in turn, to ensure that all doorbell h/w is (re)initialised.
*/
-static void guc_init_doorbell_hw(struct intel_guc *guc)
+static int guc_init_doorbell_hw(struct intel_guc *guc)
{
struct i915_guc_client *client = guc->execbuf_client;
- uint16_t db_id;
- int i, err;
+ int err;
+ int i;
- guc_disable_doorbell(guc, client);
+ if (has_doorbell(client))
+ destroy_doorbell(client);
- for (i = 0; i < GUC_MAX_DOORBELLS; ++i) {
- /* Skip if doorbell is OK */
- if (guc_doorbell_check(guc, i))
+ for (i = 0; i < GUC_NUM_DOORBELLS; ++i) {
+ if (doorbell_ok(guc, i))
continue;
As mentioned in the previous patch version, here we assume that all the
doorbells should be disabled an we want to reset HW that has been left
enabled from previous users, so the doorbell_bitmap should be clear.
Maybe adding a simple
GEM_BUG_ON(test_bit(i, guc->doorbell_bitmap));
will help making sure that we're not leaving bits set when
releasing/resetting.
- err = guc_update_doorbell_id(guc, client, i);
- if (err)
- DRM_DEBUG_DRIVER("Doorbell %d update failed, err %d\n",
- i, err);
+ err = __reset_doorbell(client, i);
If the reset fails the doorbell is in a bad state, so it might be worth
to ensure that the bit is set in the bitmask to make sure we don't
assign that doorbell to any client, but we'd have to make sure to clear
the bitmask on GuC reset (see comment above).
+ WARN(err, "Doorbell %d reset failed, err %d\n", i, err);
}
- db_id = select_doorbell_register(guc, client->priority);
- WARN_ON(db_id == GUC_INVALID_DOORBELL_ID);
-
- err = guc_update_doorbell_id(guc, client, db_id);
+ err = __reserve_doorbell(client);
if (err)
- DRM_WARN("Failed to restore doorbell to %d, err %d\n",
- db_id, err);
+ return err;
Continuing the discussion from the previous patch revision:
<snip>
> Shouldn't this be create_doorbell() instead of reserve?
That would end up calling the __update_doorbell, which we can't as the
desc is for some strange reason not mapped yet.
</snip>
As far as I can see, everything should already be allocated and mapped
here. Aren't we anyway already calling __update_doorbell both in the
client_alloc and in __reset_doorbell above?
Also, where are we re-creating the doorbell that we destroyed at the
beginning of the function?
/* Read back & verify all doorbell registers */
- for (i = 0; i < GUC_MAX_DOORBELLS; ++i)
- (void)guc_doorbell_check(guc, i);
+ for (i = 0; i < GUC_NUM_DOORBELLS; ++i)
+ WARN_ON(!doorbell_ok(guc, i));
+
+ return 0;
}
/**
@@ -696,49 +727,46 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
struct intel_guc *guc = &dev_priv->guc;
struct i915_vma *vma;
void *vaddr;
- uint16_t db_id;
+ int ret;
client = kzalloc(sizeof(*client), GFP_KERNEL);
if (!client)
- return NULL;
+ return ERR_PTR(-ENOMEM);
- client->owner = ctx;
client->guc = guc;
+ client->owner = ctx;
client->engines = engines;
client->priority = priority;
- client->doorbell_id = GUC_INVALID_DOORBELL_ID;
+ client->doorbell_id = GUC_DOORBELL_INVALID;
+ client->wq_offset = GUC_DB_SIZE;
+ client->wq_size = GUC_WQ_SIZE;
+ spin_lock_init(&client->wq_lock);
- client->ctx_index = (uint32_t)ida_simple_get(&guc->ctx_ids, 0,
- GUC_MAX_GPU_CONTEXTS, GFP_KERNEL);
- if (client->ctx_index >= GUC_MAX_GPU_CONTEXTS) {
- client->ctx_index = GUC_INVALID_CTX_ID;
- goto err;
- }
+ ret = ida_simple_get(&guc->ctx_ids, 0, GUC_MAX_GPU_CONTEXTS,
+ GFP_KERNEL);
+ if (ret < 0)
+ goto err_client;
+
+ client->ctx_index = ret;
/* The first page is doorbell/proc_desc. Two followed pages are wq. */
vma = intel_guc_allocate_vma(guc, GUC_DB_SIZE + GUC_WQ_SIZE);
- if (IS_ERR(vma))
- goto err;
+ 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))
- goto err;
-
+ if (IS_ERR(vaddr)) {
+ ret = PTR_ERR(vaddr);
+ goto err_vma;
+ }
client->vaddr = vaddr;
- spin_lock_init(&client->wq_lock);
- client->wq_offset = GUC_DB_SIZE;
- client->wq_size = GUC_WQ_SIZE;
-
- db_id = select_doorbell_register(guc, client->priority);
- if (db_id == GUC_INVALID_DOORBELL_ID)
- /* XXX: evict a doorbell instead? */
- goto err;
-
- client->doorbell_offset = select_doorbell_cacheline(guc);
+ client->doorbell_offset = __reserve_cacheline(guc);
/*
* Since the doorbell only requires a single cacheline, we can save
@@ -753,27 +781,35 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
guc_proc_desc_init(guc, client);
guc_ctx_desc_init(guc, client);
- /* For runtime client allocation we need to enable the doorbell. Not
- * required yet for the static execbuf_client as this special kernel
- * client is enabled from i915_guc_submission_enable().
- *
- * guc_update_doorbell_id(guc, client, db_id);
- */
+ /* For runtime client allocation we need to enable the doorbell. */
this comment is a bit unclear now because you're not deferring the
initialization of execbuf_client to guc_init_doorbell_hw anymore, so
there is no difference between execbuf_client and "runtime" clients.
Maybe we can just remove the comment.
Note that creating the doorbell here will cause it to be destroyed and
re-allocated when i915_guc_submission_enable is called. A worth
compromise IMO to avoid special cases, but worth pointing out :)
Thanks,
Daniele
+ ret = __update_doorbell_desc(client, client->doorbell_id);
+ if (ret)
+ goto err_vaddr;
+
+ ret = __create_doorbell(client);
+ if (ret)
+ goto err_db;
DRM_DEBUG_DRIVER("new priority %u client %p for engine(s) 0x%x: ctx_index %u\n",
- priority, client, client->engines, client->ctx_index);
- DRM_DEBUG_DRIVER("doorbell id %u, cacheline offset 0x%x\n",
- client->doorbell_id, client->doorbell_offset);
+ priority, client, client->engines, client->ctx_index);
+ DRM_DEBUG_DRIVER("doorbell id %u, cacheline offset 0x%lx\n",
+ client->doorbell_id, client->doorbell_offset);
return client;
+err_db:
+ __update_doorbell_desc(client, GUC_DOORBELL_INVALID);
+err_vaddr:
+ i915_gem_object_unpin_map(client->vma->obj);
+err_vma:
+ i915_vma_unpin_and_release(&client->vma);
+err_id:
+ ida_simple_remove(&guc->ctx_ids, client->ctx_index);
+err_client:
+ kfree(client);
-err:
- guc_client_free(dev_priv, client);
- return NULL;
+ return ERR_PTR(ret);
}
-
-
static void guc_policies_init(struct guc_policies *policies)
{
struct guc_policy *policy;
@@ -881,7 +917,7 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
return 0;
/* Wipe bitmap & delete client in case of reinitialisation */
- bitmap_clear(guc->doorbell_bitmap, 0, GUC_MAX_DOORBELLS);
+ bitmap_clear(guc->doorbell_bitmap, 0, GUC_NUM_DOORBELLS);
i915_guc_submission_disable(dev_priv);
if (!i915.enable_guc_submission)
@@ -932,14 +968,19 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
struct i915_guc_client *client = guc->execbuf_client;
struct intel_engine_cs *engine;
enum intel_engine_id id;
+ int err;
if (!client)
return -ENODEV;
- intel_guc_sample_forcewake(guc);
+ err = intel_guc_sample_forcewake(guc);
+ if (err)
+ return err;
guc_reset_wq(client);
- guc_init_doorbell_hw(guc);
+ err = guc_init_doorbell_hw(guc);
+ if (err)
+ return err;
/* Take over from manual control of ELSP (execlists) */
for_each_engine(engine, dev_priv, id) {
@@ -978,7 +1019,7 @@ void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
if (!client)
return;
- guc_client_free(dev_priv, client);
+ guc_client_free(client);
i915_vma_unpin_and_release(&guc->ads_vma);
i915_vma_unpin_and_release(&guc->log.vma);
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 25691f0..3ae8cef 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -241,8 +241,8 @@ union guc_doorbell_qw {
u64 value_qw;
} __packed;
-#define GUC_MAX_DOORBELLS 256
-#define GUC_INVALID_DOORBELL_ID (GUC_MAX_DOORBELLS)
+#define GUC_NUM_DOORBELLS 256
+#define GUC_DOORBELL_INVALID (GUC_NUM_DOORBELLS)
#define GUC_DB_SIZE (PAGE_SIZE)
#define GUC_WQ_SIZE (PAGE_SIZE * 2)
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index d74f4d3..604e71e 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -72,13 +72,12 @@ struct i915_guc_client {
uint32_t engines; /* bitmap of (host) engine ids */
uint32_t priority;
- uint32_t ctx_index;
+ u32 ctx_index;
uint32_t proc_desc_offset;
- uint32_t doorbell_offset;
- uint32_t doorbell_cookie;
- uint16_t doorbell_id;
- uint16_t padding[3]; /* Maintain alignment */
+ u16 doorbell_id;
+ unsigned long doorbell_offset;
+ u32 doorbell_cookie;
spinlock_t wq_lock;
uint32_t wq_offset;
@@ -159,7 +158,7 @@ struct intel_guc {
struct i915_guc_client *execbuf_client;
- DECLARE_BITMAP(doorbell_bitmap, GUC_MAX_DOORBELLS);
+ DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
uint32_t db_cacheline; /* Cyclic counter mod pagesize */
/* Action status & statistics */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
