Skip to content

Commit

Permalink
drm/i915/guc: Simplify code by keeping vmap of guc_client object
Browse files Browse the repository at this point in the history 
GuC client object is always pinned during its life cycle. We cache
the vmap of client object, which includes guc_process_desc, doorbell
and work queue. By doing so, we can simplify the code where driver
communicate with GuC.

As a result, this patch removes the kmap_atomic in wq_check_space,
where usleep_range could be called while kmap_atomic is held. This
fixes issue below.

[   34.098798] BUG: scheduling while atomic: gem_close_race/1941/0x00000002
[   34.098822] Modules linked in: hid_generic usbhid i915 asix usbnet libphy mii i2c_algo_bit drm_kms_helper cfbfillrect syscopyarea cfbimgblt sysfillrect sysimgblt fb_sys_fops cfbcopyarea drm coretemp i2c_hid hid video pinctrl_sunrisepoint pinctrl_intel acpi_pad nls_iso8859_1 e1000e ptp psmouse pps_core ahci libahci
[   34.098824] CPU: 0 PID: 1941 Comm: gem_close_race Tainted: G     U          4.4.0-160121+ torvalds#123
[   34.098824] Hardware name: Intel Corporation Skylake Client platform/Skylake AIO DDR3L RVP10, BIOS SKLSE2R1.R00.X100.B01.1509220551 09/22/2015
[   34.098825]  0000000000013e40 ffff880166c27a78 ffffffff81280d02 ffff880172c13e40
[   34.098826]  ffff880166c27a88 ffffffff810c203a ffff880166c27ac8 ffffffff814ec808
[   34.098827]  ffff88016b7c6000 ffff880166c28000 00000000000f4240 0000000000000001
[   34.098827] Call Trace:
[   34.098831]  [<ffffffff81280d02>] dump_stack+0x4b/0x79
[   34.098833]  [<ffffffff810c203a>] __schedule_bug+0x41/0x4f
[   34.098834]  [<ffffffff814ec808>] __schedule+0x5a8/0x690
[   34.098835]  [<ffffffff814ec927>] schedule+0x37/0x80
[   34.098836]  [<ffffffff814ef3fd>] schedule_hrtimeout_range_clock+0xad/0x130
[   34.098837]  [<ffffffff81090be0>] ? hrtimer_init+0x10/0x10
[   34.098838]  [<ffffffff814ef3f1>] ? schedule_hrtimeout_range_clock+0xa1/0x130
[   34.098839]  [<ffffffff814ef48e>] schedule_hrtimeout_range+0xe/0x10
[   34.098840]  [<ffffffff814eef9b>] usleep_range+0x3b/0x40
[   34.098853]  [<ffffffffa01ec109>] i915_guc_wq_check_space+0x119/0x210 [i915]
[   34.098861]  [<ffffffffa01da47c>] intel_logical_ring_alloc_request_extras+0x5c/0x70 [i915]
[   34.098869]  [<ffffffffa01cdbf1>] i915_gem_request_alloc+0x91/0x170 [i915]
[   34.098875]  [<ffffffffa01c1c07>] i915_gem_do_execbuffer.isra.25+0xbc7/0x12a0 [i915]
[   34.098882]  [<ffffffffa01cb785>] ? i915_gem_object_get_pages_gtt+0x225/0x3c0 [i915]
[   34.098889]  [<ffffffffa01d1fb6>] ? i915_gem_pwrite_ioctl+0xd6/0x9f0 [i915]
[   34.098895]  [<ffffffffa01c2e68>] i915_gem_execbuffer2+0xa8/0x250 [i915]
[   34.098900]  [<ffffffffa00f65d8>] drm_ioctl+0x258/0x4f0 [drm]
[   34.098906]  [<ffffffffa01c2dc0>] ? i915_gem_execbuffer+0x340/0x340 [i915]
[   34.098908]  [<ffffffff8111590d>] do_vfs_ioctl+0x2cd/0x4a0
[   34.098909]  [<ffffffff8111eac2>] ? __fget+0x72/0xb0
[   34.098910]  [<ffffffff81115b1c>] SyS_ioctl+0x3c/0x70
[   34.098911]  [<ffffffff814effd7>] entry_SYSCALL_64_fastpath+0x12/0x6a
[   34.100208] ------------[ cut here ]------------

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93847
Cc: Dave Gordon <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Signed-off-by: Alex Dai <[email protected]>
  • Loading branch information
Alex Dai authored and 0day robot committed Feb 17, 2016
1 parent 5c224cf commit cba8cf8
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 38 deletions.
53 changes: 16 additions & 37 deletions drivers/gpu/drm/i915/i915_guc_submission.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,9 @@ static int guc_ring_doorbell(struct i915_guc_client *gc)
struct guc_process_desc *desc;
union guc_doorbell_qw db_cmp, db_exc, db_ret;
union guc_doorbell_qw *db;
void *base;
int attempt = 2, ret = -EAGAIN;

base = kmap_atomic(i915_gem_object_get_page(gc->client_obj, 0));
desc = base + gc->proc_desc_offset;
desc = gc->client_base + gc->proc_desc_offset;

/* Update the tail so it is visible to GuC */
desc->tail = gc->wq_tail;
Expand All @@ -215,7 +213,7 @@ static int guc_ring_doorbell(struct i915_guc_client *gc)
db_exc.cookie = 1;

/* pointer of current doorbell cacheline */
db = base + gc->doorbell_offset;
db = gc->client_base + gc->doorbell_offset;

while (attempt--) {
/* lets ring the doorbell */
Expand Down Expand Up @@ -244,10 +242,6 @@ static int guc_ring_doorbell(struct i915_guc_client *gc)
db_exc.cookie = 1;
}

/* Finally, update the cached copy of the GuC's WQ head */
gc->wq_head = desc->head;

kunmap_atomic(base);
return ret;
}

Expand Down Expand Up @@ -341,10 +335,8 @@ static void guc_init_proc_desc(struct intel_guc *guc,
struct i915_guc_client *client)
{
struct guc_process_desc *desc;
void *base;

base = kmap_atomic(i915_gem_object_get_page(client->client_obj, 0));
desc = base + client->proc_desc_offset;
desc = client->client_base + client->proc_desc_offset;

memset(desc, 0, sizeof(*desc));

Expand All @@ -361,8 +353,6 @@ static void guc_init_proc_desc(struct intel_guc *guc,
desc->wq_size_bytes = client->wq_size;
desc->wq_status = WQ_STATUS_ACTIVE;
desc->priority = client->priority;

kunmap_atomic(base);
}

/*
Expand Down Expand Up @@ -474,25 +464,16 @@ static void guc_fini_ctx_desc(struct intel_guc *guc,
int i915_guc_wq_check_space(struct i915_guc_client *gc)
{
struct guc_process_desc *desc;
void *base;
u32 size = sizeof(struct guc_wq_item);
int ret = -ETIMEDOUT, timeout_counter = 200;

if (!gc)
return 0;

/* Quickly return if wq space is available since last time we cache the
* head position. */
if (CIRC_SPACE(gc->wq_tail, gc->wq_head, gc->wq_size) >= size)
return 0;

base = kmap_atomic(i915_gem_object_get_page(gc->client_obj, 0));
desc = base + gc->proc_desc_offset;
desc = gc->client_base + gc->proc_desc_offset;

while (timeout_counter-- > 0) {
gc->wq_head = desc->head;

if (CIRC_SPACE(gc->wq_tail, gc->wq_head, gc->wq_size) >= size) {
if (CIRC_SPACE(gc->wq_tail, desc->head, gc->wq_size) >= size) {
ret = 0;
break;
}
Expand All @@ -501,24 +482,23 @@ int i915_guc_wq_check_space(struct i915_guc_client *gc)
usleep_range(1000, 2000);
};

kunmap_atomic(base);

return ret;
}

static int guc_add_workqueue_item(struct i915_guc_client *gc,
struct drm_i915_gem_request *rq)
{
struct guc_process_desc *desc;
struct guc_wq_item *wqi;
void *base;
u32 tail, wq_len, wq_off, space;
u32 tail, wq_len, wqi_off, space;

space = CIRC_SPACE(gc->wq_tail, gc->wq_head, gc->wq_size);
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;
wqi_off = gc->wq_tail;
gc->wq_tail += sizeof(struct guc_wq_item);
gc->wq_tail &= gc->wq_size - 1;

Expand All @@ -530,13 +510,10 @@ static int guc_add_workqueue_item(struct i915_guc_client *gc,
* workqueue buffer dw by dw.
*/
WARN_ON(sizeof(struct guc_wq_item) != 16);
WARN_ON(wq_off & 3);
WARN_ON(wqi_off & 3);

/* 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));
wq_off &= PAGE_SIZE - 1;
wqi = (struct guc_wq_item *)((char *)base + wq_off);
wqi = gc->client_base + gc->wq_offset + wqi_off;

/* len does not include the header */
wq_len = sizeof(struct guc_wq_item) / sizeof(u32) - 1;
Expand All @@ -553,8 +530,6 @@ static int guc_add_workqueue_item(struct i915_guc_client *gc,
wqi->ring_tail = tail << WQ_RING_TAIL_SHIFT;
wqi->fence_id = 0; /*XXX: what fence to be here */

kunmap_atomic(base);

return 0;
}

Expand Down Expand Up @@ -675,6 +650,8 @@ static void guc_client_free(struct drm_device *dev,
* Be sure to drop any locks
*/

vunmap(client->client_base);

gem_release_guc_obj(client->client_obj);

if (client->ctx_index != GUC_INVALID_CTX_ID) {
Expand Down Expand Up @@ -727,6 +704,8 @@ static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
if (!obj)
goto err;

client->client_base = i915_gem_object_vmap(obj, 0, obj->pages->nents);

client->client_obj = obj;
client->wq_offset = GUC_DB_SIZE;
client->wq_size = GUC_WQ_SIZE;
Expand Down
3 changes: 2 additions & 1 deletion drivers/gpu/drm/i915/intel_guc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ struct i915_guc_client {
uint32_t priority;
uint32_t ctx_index;

void *client_base;

uint32_t proc_desc_offset;
uint32_t doorbell_offset;
uint32_t cookie;
Expand All @@ -43,7 +45,6 @@ struct i915_guc_client {
uint32_t wq_offset;
uint32_t wq_size;
uint32_t wq_tail;
uint32_t wq_head;

/* GuC submission statistics & status */
uint64_t submissions[GUC_MAX_ENGINES_NUM];
Expand Down

0 comments on commit cba8cf8

Please sign in to comment.