drm/i915/guc: rework guc_add_workqueue_item()

Mostly little optimisations and future-proofing against code breakage.
For instance, if the driver is correctly following the submission
protocol, the "out of space" condition is impossible, so the previous
runtime WARN_ON() is promoted to a GEM_BUG_ON() for a more dramatic
effect in development and less impact in end-user systems.

Similarly we can make alignment checking more stringent and replace
other WARN_ON() conditions that don't relate to the runtime hardware
state with either BUILD_BUG_ON() for compile-time-detectable issues, or
GEM_BUG_ON() for logical "can't happen" errors.

With those changes, we can convert it to void, as suggested by Chris
Wilson, and update the calling code appropriately.

v2:
    Note that we're now putting the request seqno in the "fence_id"
    field of each GuC-work-item, in case it turns up somewhere useful
    (e.g. in a GuC log) [Tvrtko Ursulin].

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
This commit is contained in:
Dave Gordon 2016-05-13 15:36:34 +01:00 committed by Tvrtko Ursulin
parent 551aaecd88
commit 0a31afbc0f
3 changed files with 38 additions and 38 deletions

View File

@ -486,23 +486,28 @@ int i915_guc_wq_check_space(struct drm_i915_gem_request *request)
return -EAGAIN;
}
static int guc_add_workqueue_item(struct i915_guc_client *gc,
struct drm_i915_gem_request *rq)
static void guc_add_workqueue_item(struct i915_guc_client *gc,
struct drm_i915_gem_request *rq)
{
/* wqi_len is in DWords, and does not include the one-word header */
const size_t wqi_size = sizeof(struct guc_wq_item);
const u32 wqi_len = wqi_size/sizeof(u32) - 1;
struct guc_process_desc *desc;
struct guc_wq_item *wqi;
void *base;
u32 tail, wq_len, wq_off, space;
u32 freespace, tail, wq_off, wq_page;
desc = gc->client_base + gc->proc_desc_offset;
space = CIRC_SPACE(gc->wq_tail, desc->head, gc->wq_size);
if (WARN_ON(space < sizeof(struct guc_wq_item)))
return -ENOSPC; /* shouldn't happen */
/* postincrement WQ tail for next time */
wq_off = gc->wq_tail;
gc->wq_tail += sizeof(struct guc_wq_item);
gc->wq_tail &= gc->wq_size - 1;
/* Free space is guaranteed, see i915_guc_wq_check_space() above */
freespace = CIRC_SPACE(gc->wq_tail, desc->head, gc->wq_size);
GEM_BUG_ON(freespace < wqi_size);
/* The GuC firmware wants the tail index in QWords, not bytes */
tail = rq->tail;
GEM_BUG_ON(tail & 7);
tail >>= 3;
GEM_BUG_ON(tail > WQ_RING_TAIL_MAX);
/* For now workqueue item is 4 DWs; workqueue buffer is 2 pages. So we
* should not have the case where structure wqi is across page, neither
@ -511,19 +516,23 @@ static int guc_add_workqueue_item(struct i915_guc_client *gc,
* XXX: if not the case, we need save data to a temp wqi and copy it to
* workqueue buffer dw by dw.
*/
WARN_ON(sizeof(struct guc_wq_item) != 16);
WARN_ON(wq_off & 3);
BUILD_BUG_ON(wqi_size != 16);
/* wq starts from the page after doorbell / process_desc */
base = kmap_atomic(i915_gem_object_get_page(gc->client_obj,
(wq_off + GUC_DB_SIZE) >> PAGE_SHIFT));
/* postincrement WQ tail for next time */
wq_off = gc->wq_tail;
gc->wq_tail += wqi_size;
gc->wq_tail &= gc->wq_size - 1;
GEM_BUG_ON(wq_off & (wqi_size - 1));
/* WQ starts from the page after doorbell / process_desc */
wq_page = (wq_off + GUC_DB_SIZE) >> PAGE_SHIFT;
wq_off &= PAGE_SIZE - 1;
base = kmap_atomic(i915_gem_object_get_page(gc->client_obj, wq_page));
wqi = (struct guc_wq_item *)((char *)base + wq_off);
/* len does not include the header */
wq_len = sizeof(struct guc_wq_item) / sizeof(u32) - 1;
/* Now fill in the 4-word work queue item */
wqi->header = WQ_TYPE_INORDER |
(wq_len << WQ_LEN_SHIFT) |
(wqi_len << WQ_LEN_SHIFT) |
(rq->engine->guc_id << WQ_TARGET_SHIFT) |
WQ_NO_WCFLUSH_WAIT;
@ -531,14 +540,10 @@ static int guc_add_workqueue_item(struct i915_guc_client *gc,
wqi->context_desc = (u32)intel_lr_context_descriptor(rq->ctx,
rq->engine);
/* The GuC firmware wants the tail index in QWords, not bytes */
tail = rq->ringbuf->tail >> 3;
wqi->ring_tail = tail << WQ_RING_TAIL_SHIFT;
wqi->fence_id = 0; /*XXX: what fence to be here */
wqi->fence_id = rq->seqno;
kunmap_atomic(base);
return 0;
}
/**
@ -565,26 +570,20 @@ int i915_guc_submit(struct drm_i915_gem_request *rq)
unsigned int engine_id = rq->engine->guc_id;
struct intel_guc *guc = &rq->i915->guc;
struct i915_guc_client *client = guc->execbuf_client;
int q_ret, b_ret;
int b_ret;
q_ret = guc_add_workqueue_item(client, rq);
if (q_ret == 0)
b_ret = guc_ring_doorbell(client);
guc_add_workqueue_item(client, rq);
b_ret = guc_ring_doorbell(client);
client->submissions[engine_id] += 1;
if (q_ret) {
client->q_fail += 1;
client->retcode = q_ret;
} else if (b_ret) {
client->retcode = b_ret;
if (b_ret)
client->b_fail += 1;
client->retcode = q_ret = b_ret;
} else {
client->retcode = 0;
}
guc->submissions[engine_id] += 1;
guc->last_seqno[engine_id] = rq->seqno;
return q_ret;
return b_ret;
}
/*

View File

@ -81,7 +81,7 @@ struct i915_guc_client {
uint32_t unused; /* Was 'wq_head' */
uint32_t no_wq_space;
uint32_t q_fail;
uint32_t q_fail; /* No longer used */
uint32_t b_fail;
int retcode;

View File

@ -71,7 +71,8 @@
#define WQ_WORKLOAD_TOUCH (2 << WQ_WORKLOAD_SHIFT)
#define WQ_RING_TAIL_SHIFT 20
#define WQ_RING_TAIL_MASK (0x7FF << WQ_RING_TAIL_SHIFT)
#define WQ_RING_TAIL_MAX 0x7FF /* 2^11 QWords */
#define WQ_RING_TAIL_MASK (WQ_RING_TAIL_MAX << WQ_RING_TAIL_SHIFT)
#define GUC_DOORBELL_ENABLED 1
#define GUC_DOORBELL_DISABLED 0