From 0b063bd3ea9c13df78c82aa742e581c39f9d6156 Mon Sep 17 00:00:00 2001 From: Zhenyu Wang Date: Sat, 1 Apr 2017 10:53:02 +0800 Subject: drm/i915/gvt: cleanup some too chatty scheduler message It's too chatty to have three places to tell us which one is next vgpu for schedule. My log file was bloated to eat all disk space.. Cc: Ping Gao Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/sched_policy.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/sched_policy.c b/drivers/gpu/drm/i915/gvt/sched_policy.c index f84959170674..f459ec8b06a1 100644 --- a/drivers/gpu/drm/i915/gvt/sched_policy.c +++ b/drivers/gpu/drm/i915/gvt/sched_policy.c @@ -150,9 +150,6 @@ static void try_to_schedule_next_vgpu(struct intel_gvt *gvt) } } - gvt_dbg_sched("switch to next vgpu %d\n", - scheduler->next_vgpu->id); - cur_time = ktime_get(); if (scheduler->current_vgpu) { vgpu_data = scheduler->current_vgpu->sched_data; @@ -224,17 +221,12 @@ static void tbs_sched_func(struct gvt_sched_data *sched_data) list_del_init(&vgpu_data->lru_list); list_add_tail(&vgpu_data->lru_list, &sched_data->lru_runq_head); - - gvt_dbg_sched("pick next vgpu %d\n", vgpu->id); } else { scheduler->next_vgpu = gvt->idle_vgpu; } out: - if (scheduler->next_vgpu) { - gvt_dbg_sched("try to schedule next vgpu %d\n", - scheduler->next_vgpu->id); + if (scheduler->next_vgpu) try_to_schedule_next_vgpu(gvt); - } } void intel_gvt_schedule(struct intel_gvt *gvt) -- cgit v1.2.3 From e1236bc06c534a97f73e09aed5e1094108553e9f Mon Sep 17 00:00:00 2001 From: Changbin Du Date: Thu, 6 Apr 2017 10:55:02 +0800 Subject: drm/i915/gvt: Align render mmio list to cacheline Make the global mmio list be cacheline aligned to improve performance. Signed-off-by: Changbin Du Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/render.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c index e24e57afc45e..679411fe653f 100644 --- a/drivers/gpu/drm/i915/gvt/render.c +++ b/drivers/gpu/drm/i915/gvt/render.c @@ -44,7 +44,7 @@ struct render_mmio { u32 value; }; -static struct render_mmio gen8_render_mmio_list[] = { +static struct render_mmio gen8_render_mmio_list[] __cacheline_aligned = { {RCS, _MMIO(0x229c), 0xffff, false}, {RCS, _MMIO(0x2248), 0x0, false}, {RCS, _MMIO(0x2098), 0x0, false}, @@ -75,7 +75,7 @@ static struct render_mmio gen8_render_mmio_list[] = { {BCS, _MMIO(0x22028), 0x0, false}, }; -static struct render_mmio gen9_render_mmio_list[] = { +static struct render_mmio gen9_render_mmio_list[] __cacheline_aligned = { {RCS, _MMIO(0x229c), 0xffff, false}, {RCS, _MMIO(0x2248), 0x0, false}, {RCS, _MMIO(0x2098), 0x0, false}, -- cgit v1.2.3 From 80901ca879083ecb5fd08a8d3413220bec9612ac Mon Sep 17 00:00:00 2001 From: Changbin Du Date: Thu, 6 Apr 2017 10:55:43 +0800 Subject: drm/i915/gvt: remove redundant platform check for mocs load/restore The platform check is done outside, no need check again. Platform doesn't include mocs should not invoke this two functions. Signed-off-by: Changbin Du Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/render.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c index 679411fe653f..a7dae5e2c7cc 100644 --- a/drivers/gpu/drm/i915/gvt/render.c +++ b/drivers/gpu/drm/i915/gvt/render.c @@ -204,9 +204,6 @@ static void load_mocs(struct intel_vgpu *vgpu, int ring_id) if (WARN_ON(ring_id >= ARRAY_SIZE(regs))) return; - if (!(IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))) - return; - offset.reg = regs[ring_id]; for (i = 0; i < 64; i++) { gen9_render_mocs[ring_id][i] = I915_READ(offset); @@ -242,9 +239,6 @@ static void restore_mocs(struct intel_vgpu *vgpu, int ring_id) if (WARN_ON(ring_id >= ARRAY_SIZE(regs))) return; - if (!(IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))) - return; - offset.reg = regs[ring_id]; for (i = 0; i < 64; i++) { vgpu_vreg(vgpu, offset) = I915_READ(offset); -- cgit v1.2.3 From 43c29e1f449d596ed92f12cc19e41d9731ec3312 Mon Sep 17 00:00:00 2001 From: Changbin Du Date: Thu, 6 Apr 2017 10:55:55 +0800 Subject: drm/i915/gvt: remove redundant ring id check which cause significant CPU misprediction From perf data, found a significant overhead at ring id check in the function get_opcode. This inline function is frequently used. Since Intel static predictor will predict the branch to fall through so the prediction most fail. This is wasting CPU pipeline resource. We do not need check the engine id everywhere, it should be reliable. Signed-off-by: Changbin Du Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/cmd_parser.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c index 94f2e701e4d4..1abd153bec28 100644 --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c @@ -616,9 +616,6 @@ static inline u32 get_opcode(u32 cmd, int ring_id) { struct decode_info *d_info; - if (ring_id >= I915_NUM_ENGINES) - return INVALID_OP; - d_info = ring_decode_info[ring_id][CMD_TYPE(cmd)]; if (d_info == NULL) return INVALID_OP; @@ -661,9 +658,6 @@ static inline void print_opcode(u32 cmd, int ring_id) struct decode_info *d_info; int i; - if (ring_id >= I915_NUM_ENGINES) - return; - d_info = ring_decode_info[ring_id][CMD_TYPE(cmd)]; if (d_info == NULL) return; -- cgit v1.2.3 From fd3bd0a99cffffe476d54edd2eb13b52b1e9a27d Mon Sep 17 00:00:00 2001 From: Changbin Du Date: Thu, 6 Apr 2017 10:56:03 +0800 Subject: drm/i915/gvt: use directly assignment for structure copying Let c compiler handle the structure copying. The compiler will use builtin function to handle that. Signed-off-by: Changbin Du Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/cmd_parser.c | 2 +- drivers/gpu/drm/i915/gvt/execlist.c | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c index 1abd153bec28..41b2c3aaa04a 100644 --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c @@ -2477,7 +2477,7 @@ static int cmd_parser_exec(struct parser_exec_state *s) t1 = get_cycles(); - memcpy(&s_before_advance_custom, s, sizeof(struct parser_exec_state)); + s_before_advance_custom = *s; if (info->handler) { ret = info->handler(s); diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c index ce4276a7cf9c..d077ed97970f 100644 --- a/drivers/gpu/drm/i915/gvt/execlist.c +++ b/drivers/gpu/drm/i915/gvt/execlist.c @@ -687,9 +687,7 @@ static int submit_context(struct intel_vgpu *vgpu, int ring_id, } if (emulate_schedule_in) - memcpy(&workload->elsp_dwords, - &vgpu->execlist[ring_id].elsp_dwords, - sizeof(workload->elsp_dwords)); + workload->elsp_dwords = vgpu->execlist[ring_id].elsp_dwords; gvt_dbg_el("workload %p ring id %d head %x tail %x start %x ctl %x\n", workload, ring_id, head, tail, start, ctl); -- cgit v1.2.3 From efa69d734adbf8a562d58d9fdc3429f2717764e7 Mon Sep 17 00:00:00 2001 From: Pei Zhang Date: Fri, 7 Apr 2017 16:50:16 +0800 Subject: drm/i915/gvt: add mmio init for virtual display GVT implements a purely virtual monitor for virtual GPU independent of the host. Some DDI related MMIO are not initialized in current code which cause the display initialization failure in guest. This patch fills the gap. Signed-off-by: Pei Zhang Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/display.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c index 4cf2b29fbaa1..e0261fcc5b50 100644 --- a/drivers/gpu/drm/i915/gvt/display.c +++ b/drivers/gpu/drm/i915/gvt/display.c @@ -189,17 +189,44 @@ static void emulate_monitor_status_change(struct intel_vgpu *vgpu) } if (intel_vgpu_has_monitor_on_port(vgpu, PORT_B)) { - vgpu_vreg(vgpu, SDEISR) |= SDE_PORTB_HOTPLUG_CPT; vgpu_vreg(vgpu, SFUSE_STRAP) |= SFUSE_STRAP_DDIB_DETECTED; + vgpu_vreg(vgpu, TRANS_DDI_FUNC_CTL(TRANSCODER_A)) &= + ~(TRANS_DDI_BPC_MASK | TRANS_DDI_MODE_SELECT_MASK | + TRANS_DDI_PORT_MASK); + vgpu_vreg(vgpu, TRANS_DDI_FUNC_CTL(TRANSCODER_A)) |= + (TRANS_DDI_BPC_8 | TRANS_DDI_MODE_SELECT_DP_SST | + (PORT_B << TRANS_DDI_PORT_SHIFT) | + TRANS_DDI_FUNC_ENABLE); + vgpu_vreg(vgpu, DDI_BUF_CTL(PORT_B)) |= DDI_BUF_CTL_ENABLE; + vgpu_vreg(vgpu, DDI_BUF_CTL(PORT_B)) &= ~DDI_BUF_IS_IDLE; + vgpu_vreg(vgpu, SDEISR) |= SDE_PORTB_HOTPLUG_CPT; } if (intel_vgpu_has_monitor_on_port(vgpu, PORT_C)) { vgpu_vreg(vgpu, SDEISR) |= SDE_PORTC_HOTPLUG_CPT; + vgpu_vreg(vgpu, TRANS_DDI_FUNC_CTL(TRANSCODER_A)) &= + ~(TRANS_DDI_BPC_MASK | TRANS_DDI_MODE_SELECT_MASK | + TRANS_DDI_PORT_MASK); + vgpu_vreg(vgpu, TRANS_DDI_FUNC_CTL(TRANSCODER_A)) |= + (TRANS_DDI_BPC_8 | TRANS_DDI_MODE_SELECT_DP_SST | + (PORT_C << TRANS_DDI_PORT_SHIFT) | + TRANS_DDI_FUNC_ENABLE); + vgpu_vreg(vgpu, DDI_BUF_CTL(PORT_C)) |= DDI_BUF_CTL_ENABLE; + vgpu_vreg(vgpu, DDI_BUF_CTL(PORT_C)) &= ~DDI_BUF_IS_IDLE; vgpu_vreg(vgpu, SFUSE_STRAP) |= SFUSE_STRAP_DDIC_DETECTED; } if (intel_vgpu_has_monitor_on_port(vgpu, PORT_D)) { vgpu_vreg(vgpu, SDEISR) |= SDE_PORTD_HOTPLUG_CPT; + vgpu_vreg(vgpu, TRANS_DDI_FUNC_CTL(TRANSCODER_A)) &= + ~(TRANS_DDI_BPC_MASK | TRANS_DDI_MODE_SELECT_MASK | + TRANS_DDI_PORT_MASK); + vgpu_vreg(vgpu, TRANS_DDI_FUNC_CTL(TRANSCODER_A)) |= + (TRANS_DDI_BPC_8 | TRANS_DDI_MODE_SELECT_DP_SST | + (PORT_D << TRANS_DDI_PORT_SHIFT) | + TRANS_DDI_FUNC_ENABLE); + vgpu_vreg(vgpu, DDI_BUF_CTL(PORT_D)) |= DDI_BUF_CTL_ENABLE; + vgpu_vreg(vgpu, DDI_BUF_CTL(PORT_D)) &= ~DDI_BUF_IS_IDLE; vgpu_vreg(vgpu, SFUSE_STRAP) |= SFUSE_STRAP_DDID_DETECTED; } -- cgit v1.2.3 From 954180aa69325feade02fa79f056fe1561f31fbb Mon Sep 17 00:00:00 2001 From: Zhenyu Wang Date: Wed, 12 Apr 2017 14:22:50 +0800 Subject: drm/i915/gvt: remove some debug messages in scheduler timer handler As those debug messages might appear in every timer call for scheduler, it's too noisy, eat too much log and aren't meaningful. So remove them. Cc: Ping Gao Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/sched_policy.c | 7 +------ drivers/gpu/drm/i915/gvt/scheduler.c | 5 +---- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/sched_policy.c b/drivers/gpu/drm/i915/gvt/sched_policy.c index f459ec8b06a1..79ba4b3440aa 100644 --- a/drivers/gpu/drm/i915/gvt/sched_policy.c +++ b/drivers/gpu/drm/i915/gvt/sched_policy.c @@ -133,9 +133,6 @@ static void try_to_schedule_next_vgpu(struct intel_gvt *gvt) if (!scheduler->next_vgpu) return; - gvt_dbg_sched("try to schedule next vgpu %d\n", - scheduler->next_vgpu->id); - /* * after the flag is set, workload dispatch thread will * stop dispatching workload for current vgpu @@ -144,10 +141,8 @@ static void try_to_schedule_next_vgpu(struct intel_gvt *gvt) /* still have uncompleted workload? */ for_each_engine(engine, gvt->dev_priv, i) { - if (scheduler->current_workload[i]) { - gvt_dbg_sched("still have running workload\n"); + if (scheduler->current_workload[i]) return; - } } cur_time = ktime_get(); diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 375038252761..0b685dd26cb3 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -274,11 +274,8 @@ static struct intel_vgpu_workload *pick_next_workload( goto out; } - if (list_empty(workload_q_head(scheduler->current_vgpu, ring_id))) { - gvt_dbg_sched("ring id %d stop - no available workload\n", - ring_id); + if (list_empty(workload_q_head(scheduler->current_vgpu, ring_id))) goto out; - } /* * still have current workload, maybe the workload disptacher -- cgit v1.2.3 From 5ad59bf0960b807f01cb6ef8c54f68b3476a1546 Mon Sep 17 00:00:00 2001 From: Zhenyu Wang Date: Wed, 12 Apr 2017 16:24:57 +0800 Subject: drm/i915/gvt: Fix PTE write flush for taking runtime pm properly Make sure to take runtime pm when write PTE flush which ensure to write to hw properly. This fixes warning during mdev/vgpu creation which will do ggtt reset. ------------[ cut here ]------------ WARNING: CPU: 1 PID: 9375 at drivers/gpu/drm/i915/intel_drv.h:1748 fwtable_write32+0x1c2/0x1e0 [i915] RPM wakelock ref not held during HW access Call Trace: ? dump_stack+0x5c/0x81 ? __warn+0xbe/0xe0 ? warn_slowpath_fmt+0x5a/0x80 ? wake_up_klogd+0x37/0x40 ? vprintk_emit+0x2ef/0x370 ? fwtable_write32+0x1c2/0x1e0 [i915] ? gtt_set_entry64+0xbb/0xd0 [i915] ? intel_vgpu_reset_ggtt+0x88/0xf0 [i915] ? intel_vgpu_init_gtt+0xa5/0x4f0 [i915] ? intel_gvt_create_vgpu+0x1b5/0x250 [i915] ? kobject_put+0x1b/0x50 ? intel_vgpu_create+0x4e/0x130 [kvmgt] ? mdev_device_create+0x186/0x2a0 [mdev] ? create_store+0xba/0xe0 [mdev] ? create_store+0xba/0xe0 [mdev] ? kernfs_fop_write+0x109/0x1a0 ? kernfs_fop_write+0x109/0x1a0 ? __vfs_write+0x33/0x160 ? __fput+0x161/0x1d0 ? vfs_write+0xb0/0x190 ? SyS_write+0x52/0xc0 ? exit_to_usermode_loop+0x7a/0xa0 ? entry_SYSCALL_64_fastpath+0x1e/0xad v2: remove unrelated oops info v3: change to take runtime pm for ggtt reset instead of get/put for each pte write flush Fixes: d650ac060237 ("drm/i915/gvt: reset the GGTT entry when vGPU created") Cc: Ping Gao Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/gtt.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index 69d3d8ddecc2..acbe3f2ad6fc 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.c +++ b/drivers/gpu/drm/i915/gvt/gtt.c @@ -2290,12 +2290,15 @@ void intel_gvt_clean_gtt(struct intel_gvt *gvt) void intel_vgpu_reset_ggtt(struct intel_vgpu *vgpu) { struct intel_gvt *gvt = vgpu->gvt; + struct drm_i915_private *dev_priv = gvt->dev_priv; struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops; u32 index; u32 offset; u32 num_entries; struct intel_gvt_gtt_entry e; + intel_runtime_pm_get(dev_priv); + memset(&e, 0, sizeof(struct intel_gvt_gtt_entry)); e.type = GTT_TYPE_GGTT_PTE; ops->set_pfn(&e, gvt->gtt.scratch_ggtt_mfn); @@ -2310,6 +2313,8 @@ void intel_vgpu_reset_ggtt(struct intel_vgpu *vgpu) num_entries = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT; for (offset = 0; offset < num_entries; offset++) ops->set_entry(NULL, &e, index + offset, false, 0, vgpu); + + intel_runtime_pm_put(dev_priv); } /** -- cgit v1.2.3 From c821ee6d2bb4cfc9991bf285f53103cde9d3593a Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Thu, 13 Apr 2017 22:48:28 +0300 Subject: drm/i915/gvt: fix a bounds check in ring_id_to_context_switch_event() There are two bugs here. The && should be || and the > is off by one so it should be >= ARRAY_SIZE(). Fixes: 8453d674ae7e ("drm/i915/gvt: vGPU execlist virtualization") Signed-off-by: Dan Carpenter Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/execlist.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c index d077ed97970f..dc9aef3e92d4 100644 --- a/drivers/gpu/drm/i915/gvt/execlist.c +++ b/drivers/gpu/drm/i915/gvt/execlist.c @@ -56,8 +56,8 @@ static int context_switch_events[] = { static int ring_id_to_context_switch_event(int ring_id) { - if (WARN_ON(ring_id < RCS && ring_id > - ARRAY_SIZE(context_switch_events))) + if (WARN_ON(ring_id < RCS || + ring_id >= ARRAY_SIZE(context_switch_events))) return -EINVAL; return context_switch_events[ring_id]; -- cgit v1.2.3 From 1676a2b35cd5a548da17d1106fb0d4d238c0d191 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 3 Apr 2017 11:51:24 +0100 Subject: drm/i915: Park the signaler before sleeping If the signal to park arrives before we sleep, then we need to check kthread_should_park() before sleeping to avoid missing the signal. Otherwise, if the signal arrives whilst we are processing completed requests, we will reset the current->state back to TASK_INTERRUPTIBLE and so miss the wakeup. Fixes: fe3288b5da2c ("drm/i915: Park the breadcrumbs signaler across a GPU reset") Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Tvrtko Ursulin Link: http://patchwork.freedesktop.org/patch/msgid/20170403105124.8969-1-chris@chris-wilson.co.uk Reviewed-by: Tvrtko Ursulin (cherry picked from commit b1becb88268beb72df6495e35d3d76c138d215bb) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_breadcrumbs.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index b6ea192ad550..308c56a021ab 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -629,6 +629,9 @@ static int intel_breadcrumbs_signaler(void *arg) } else { DEFINE_WAIT(exec); + if (kthread_should_park()) + kthread_parkme(); + if (kthread_should_stop()) { GEM_BUG_ON(request); break; @@ -641,9 +644,6 @@ static int intel_breadcrumbs_signaler(void *arg) if (request) remove_wait_queue(&request->execute, &exec); - - if (kthread_should_park()) - kthread_parkme(); } i915_gem_request_put(request); } while (1); -- cgit v1.2.3 From d445aaaac0879a4c4400bf59f20465ba3e8445f1 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 4 Apr 2017 13:05:31 +0100 Subject: drm/i915: Apply a cond_resched() to the saturated signaler If the engine is continually completing nops, we can saturate the signaler and keep it working indefinitely. This angers the NMI watchdog! A good example is to disable semaphores on snb and run igt/gem_exec_nop - the parallel, multi-engine workloads are more than sufficient to hog the CPU, preventing the system from even processing ICMP echo replies. v2: Tvrtko dug into cond_resched() on x86 and found that it only depended upon preempt_count and not tif_need_resched() - which means that we would always call schedule() at that point. Fixes: c81d46138da6 ("drm/i915: Convert trace-irq to the breadcrumb waiter") Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Tvrtko Ursulin Link: http://patchwork.freedesktop.org/patch/msgid/20170404120531.10737-1-chris@chris-wilson.co.uk Reviewed-by: Tvrtko Ursulin (cherry picked from commit a7980a640cbd339aa80f406d1786a275a2c320bc) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_breadcrumbs.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index 308c56a021ab..9ccbf26124c6 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -580,6 +580,8 @@ static int intel_breadcrumbs_signaler(void *arg) signaler_set_rtpriority(); do { + bool do_schedule = true; + set_current_state(TASK_INTERRUPTIBLE); /* We are either woken up by the interrupt bottom-half, @@ -626,7 +628,18 @@ static int intel_breadcrumbs_signaler(void *arg) spin_unlock_irq(&b->rb_lock); i915_gem_request_put(request); - } else { + + /* If the engine is saturated we may be continually + * processing completed requests. This angers the + * NMI watchdog if we never let anything else + * have access to the CPU. Let's pretend to be nice + * and relinquish the CPU if we burn through the + * entire RT timeslice! + */ + do_schedule = need_resched(); + } + + if (unlikely(do_schedule)) { DEFINE_WAIT(exec); if (kthread_should_park()) -- cgit v1.2.3 From 440df938b482801c62722cc9460ee55b9a4bd847 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 29 Mar 2017 17:21:23 +0300 Subject: drm/i915: Make legacy cursor updates more unsynced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We're clearing the legacy_cursor_update flag before calling drm_atomic_helper_setup_commit() which means the helper will wait for the flip to complete before cleaning up the framebuffers. That's not what we want for the legacy cursor, so let's clear the flag after setting up the commit. Also toss in a FIXME about solving these problems in a nicer way using the fabled vblank workers. v2: Also unsync with legacy page flips Cc: Maarten Lankhorst Cc: Daniel Vetter Cc: Uwe Kleine-König Cc: Rafael Ristovski Fixes: a5509abda48e ("drm/i915: Fix legacy cursor vs. watermarks for ILK-BDW") Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/20170329142123.5923-1-ville.syrjala@linux.intel.com Reviewed-by: Maarten Lankhorst (cherry picked from commit 895203044067af64400cedbc055898bcec98d102) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 881dec88df6e..3617927af269 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13007,17 +13007,6 @@ static int intel_atomic_commit(struct drm_device *dev, struct drm_i915_private *dev_priv = to_i915(dev); int ret = 0; - /* - * The intel_legacy_cursor_update() fast path takes care - * of avoiding the vblank waits for simple cursor - * movement and flips. For cursor on/off and size changes, - * we want to perform the vblank waits so that watermark - * updates happen during the correct frames. Gen9+ have - * double buffered watermarks and so shouldn't need this. - */ - if (INTEL_GEN(dev_priv) < 9) - state->legacy_cursor_update = false; - ret = drm_atomic_helper_setup_commit(state, nonblock); if (ret) return ret; @@ -13033,6 +13022,26 @@ static int intel_atomic_commit(struct drm_device *dev, return ret; } + /* + * The intel_legacy_cursor_update() fast path takes care + * of avoiding the vblank waits for simple cursor + * movement and flips. For cursor on/off and size changes, + * we want to perform the vblank waits so that watermark + * updates happen during the correct frames. Gen9+ have + * double buffered watermarks and so shouldn't need this. + * + * Do this after drm_atomic_helper_setup_commit() and + * intel_atomic_prepare_commit() because we still want + * to skip the flip and fb cleanup waits. Although that + * does risk yanking the mapping from under the display + * engine. + * + * FIXME doing watermarks and fb cleanup from a vblank worker + * (assuming we had any) would solve these problems. + */ + if (INTEL_GEN(dev_priv) < 9) + state->legacy_cursor_update = false; + drm_atomic_helper_swap_state(state, true); dev_priv->wm.distrust_bios_wm = false; intel_shared_dpll_swap_state(state); -- cgit v1.2.3 From bdb57b8dca8e77f85c4cd9aecabd0591e6b26b2d Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 5 Apr 2017 23:15:14 +0100 Subject: drm/i915: Use the right mapping_gfp_mask for final shmem allocation Many sightings report the greater prevalence of allocation failures. This is all due to the incorrect use of mapping_gfp_constraint(), so remove it in favour of just querying the mapping_gfp_mask() which are the exact gfp_t we wanted in the first place. We still do expect a higher chance of reporting ENOMEM, as that is the intention of using __GFP_NORETRY -- to fail rather than oom after having reclaimed from our bo caches, and having done a direct|kswapd reclaim pass. Reported-by: Jason Ekstrand Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100594 Fixes: 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/20170405221514.23251-1-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen (cherry picked from commit b268d9fe0f10544f5f7a1b7015e2b97075e6215d) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index bbc6f1c9f175..28b92017b1ea 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2340,7 +2340,7 @@ rebuild_st: * defer the oom here by reporting the ENOMEM back * to userspace. */ - reclaim = mapping_gfp_constraint(mapping, 0); + reclaim = mapping_gfp_mask(mapping); reclaim |= __GFP_NORETRY; /* reclaim, but no oom */ page = shmem_read_mapping_page_gfp(mapping, i, reclaim); -- cgit v1.2.3 From dde7b00e4c85db74a4ae02ec31752cc8be3c75f1 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 12 Apr 2017 09:02:51 +0100 Subject: drm/i915: Fix use after free in lpe_audio_platdev_destroy() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [31908.547136] BUG: KASAN: use-after-free in intel_lpe_audio_teardown+0x78/0xb0 [i915] at addr ffff8801f7788358 [31908.547297] Read of size 8 by task drv_selftest/3781 [31908.547405] CPU: 0 PID: 3781 Comm: drv_selftest Tainted: G BU W 4.10.0+ #451 [31908.547553] Hardware name: / , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015 [31908.547682] Call Trace: [31908.547772] dump_stack+0x68/0x9f [31908.547857] kasan_object_err+0x1c/0x70 [31908.547947] kasan_report_error+0x1f1/0x4f0 [31908.548038] ? kfree+0xaa/0x170 [31908.548121] kasan_report+0x34/0x40 [31908.548211] ? klist_children_get+0x20/0x30 [31908.548472] ? intel_lpe_audio_teardown+0x78/0xb0 [i915] [31908.548567] __asan_load8+0x5e/0x70 [31908.548824] intel_lpe_audio_teardown+0x78/0xb0 [i915] [31908.549080] intel_audio_deinit+0x28/0x80 [i915] [31908.549315] i915_driver_unload+0xe4/0x360 [i915] [31908.549551] ? i915_driver_load+0x1d70/0x1d70 [i915] [31908.549651] ? trace_hardirqs_on+0xd/0x10 [31908.549885] i915_pci_remove+0x23/0x30 [i915] [31908.549978] pci_device_remove+0x5c/0x100 [31908.550069] device_release_driver_internal+0x1db/0x2e0 [31908.550165] driver_detach+0x68/0xc0 [31908.550256] bus_remove_driver+0x8b/0x150 [31908.550346] driver_unregister+0x3e/0x60 [31908.550439] pci_unregister_driver+0x1d/0x110 [31908.550531] ? find_module_all+0x7a/0xa0 [31908.550791] i915_exit+0x1a/0x87 [i915] [31908.550881] SyS_delete_module+0x264/0x2c0 [31908.550971] ? free_module+0x430/0x430 [31908.551064] ? trace_hardirqs_off_caller+0x16/0x110 [31908.551159] ? trace_hardirqs_on_caller+0x16/0x280 [31908.551256] ? trace_hardirqs_on_thunk+0x1a/0x1c [31908.551350] entry_SYSCALL_64_fastpath+0x1c/0xb1 [31908.551440] RIP: 0033:0x7f1d67312ec7 [31908.551520] RSP: 002b:00007ffebe34e888 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0 [31908.551650] RAX: ffffffffffffffda RBX: ffffffff811123f6 RCX: 00007f1d67312ec7 [31908.551743] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000560d0af476b8 [31908.551837] RBP: ffff880233d87f98 R08: 0000000000000000 R09: 00007ffebe34e8b8 [31908.551930] R10: 00007f1d68adf8c0 R11: 0000000000000206 R12: 0000000000000000 [31908.552023] R13: 0000560d0af46440 R14: 0000000000000034 R15: 00007ffebe34d860 [31908.552121] ? trace_hardirqs_off_caller+0x16/0x110 [31908.552217] Object at ffff8801f7788000, in cache kmalloc-2048 size: 2048 [31908.552306] Allocated: [31908.552377] PID = 3781 [31908.552456] save_stack_trace+0x16/0x20 [31908.552539] kasan_kmalloc+0xee/0x190 [31908.552627] __kmalloc+0xdb/0x1b0 [31908.552713] platform_device_alloc+0x27/0x90 [31908.552804] platform_device_register_full+0x36/0x220 [31908.553066] intel_lpe_audio_init+0x41e/0x570 [i915] [31908.553320] intel_audio_init+0xd/0x40 [i915] [31908.553552] i915_driver_load+0x13f5/0x1d70 [i915] [31908.553788] i915_pci_probe+0x65/0xe0 [i915] [31908.553881] pci_device_probe+0xda/0x140 [31908.553969] driver_probe_device+0x400/0x660 [31908.554058] __driver_attach+0x11c/0x120 [31908.554147] bus_for_each_dev+0xe6/0x150 [31908.554237] driver_attach+0x26/0x30 [31908.554325] bus_add_driver+0x26b/0x3b0 [31908.554412] driver_register+0xce/0x190 [31908.554502] __pci_register_driver+0xaf/0xc0 [31908.554589] 0xffffffffa0550063 [31908.554675] do_one_initcall+0x8b/0x1e0 [31908.554764] do_init_module+0x102/0x325 [31908.554852] load_module+0x3aad/0x45e0 [31908.554944] SyS_finit_module+0x169/0x1a0 [31908.555033] entry_SYSCALL_64_fastpath+0x1c/0xb1 [31908.555119] Freed: [31908.555188] PID = 3781 [31908.555266] save_stack_trace+0x16/0x20 [31908.555349] kasan_slab_free+0xb0/0x180 [31908.555436] kfree+0xaa/0x170 [31908.555520] platform_device_release+0x76/0x80 [31908.555610] device_release+0x45/0xe0 [31908.555698] kobject_put+0x11f/0x260 [31908.555785] put_device+0x12/0x20 [31908.555871] platform_device_unregister+0x1b/0x20 [31908.556135] intel_lpe_audio_teardown+0x5c/0xb0 [i915] [31908.556390] intel_audio_deinit+0x28/0x80 [i915] [31908.556622] i915_driver_unload+0xe4/0x360 [i915] [31908.556858] i915_pci_remove+0x23/0x30 [i915] [31908.556948] pci_device_remove+0x5c/0x100 [31908.557037] device_release_driver_internal+0x1db/0x2e0 [31908.557129] driver_detach+0x68/0xc0 [31908.557217] bus_remove_driver+0x8b/0x150 [31908.557304] driver_unregister+0x3e/0x60 [31908.557394] pci_unregister_driver+0x1d/0x110 [31908.557653] i915_exit+0x1a/0x87 [i915] [31908.557741] SyS_delete_module+0x264/0x2c0 [31908.557834] entry_SYSCALL_64_fastpath+0x1c/0xb1 [31908.557919] Memory state around the buggy address: [31908.558005] ffff8801f7788200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [31908.558127] ffff8801f7788280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [31908.558255] >ffff8801f7788300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [31908.558374] ^ [31908.558467] ffff8801f7788380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [31908.558595] ffff8801f7788400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb v2: Just leak the memory (8 bytes) as freeing it ourselves is not safe, and we need to coordinate a proper fix in platform_device itself. Fixes: eef57324d926 ("drm/i915: setup bridge for HDMI LPE audio driver") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99952 Signed-off-by: Chris Wilson Cc: Pierre-Louis Bossart Cc: Jerome Anand Cc: Jani Nikula Cc: Takashi Iwai Link: http://patchwork.freedesktop.org/patch/msgid/20170412080251.30648-1-chris@chris-wilson.co.uk Reviewed-by: Takashi Iwai Reviewed-by: Ville Syrjälä (cherry picked from commit 48ae80741da4b8a26b6df0f765713912bc7cc480) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_lpe_audio.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index d8ca187ae001..25d8e76489e4 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -131,8 +131,15 @@ err: static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv) { + /* XXX Note that platform_device_register_full() allocates a dma_mask + * and never frees it. We can't free it here as we cannot guarantee + * this is the last reference (i.e. that the dma_mask will not be + * used after our unregister). So ee choose to leak the sizeof(u64) + * allocation here - it should be fixed in the platform_device rather + * than us fiddle with its internals. + */ + platform_device_unregister(dev_priv->lpe_audio.platdev); - kfree(dev_priv->lpe_audio.platdev->dev.dma_mask); } static void lpe_audio_irq_unmask(struct irq_data *d) -- cgit v1.2.3 From dea655939837034f04992cef480162949b7cc19f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 12 Apr 2017 22:30:17 +0300 Subject: drm/i915: Perform link quality check unconditionally during long pulse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apparently some DP sinks are a little nuts and cause HPD to drop intermittently during modesets. This happens eg. on an ASUS PB287Q. In oder to recover from this we can't really use the previous connector status to determine if the link needs retraining, so let's just ignore that piece of information and do the retrain unconditionally. We do of course still check whether the link is supposed to be running or not. To actually get read out the EDID and update things properly we also need to nuke the goto out added by commit 7d23e3c37bb3 ("drm/i915: Cleaning up intel_dp_hpd_pulse"). I'm actually not sure why that was there. Perhaps to avoid an EDID read if the connector status didn't appear to change, but that sort of thing is quite racy and would have failed anyway if we failed to keep up with the hotplugs (if we missed the HPD down in between two HPD ups). And now that we take this codepath unconditionally we definitely need to drop the goto as otherwise we would never do the EDID read. v2: Drop the goto that made us skip EDID reads entirely. Doh! v3: Rebase due to locking changes s/apparely/apparently/ in the comment (Chris) Cc: stable@vger.kernel.org Cc: Manasi Navare Cc: Palmer Dabbelt Reported-by: Palmer Dabbelt Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99766 References: https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson Link: http://patchwork.freedesktop.org/patch/msgid/20170412193017.21029-1-ville.syrjala@linux.intel.com (cherry picked from commit 1a36147bb93921651f7fbd7a6e522da6c349081b) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_dp.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 6e04cb54e3ff..ee77b519835c 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4636,9 +4636,20 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) */ status = connector_status_disconnected; goto out; - } else if (connector->status == connector_status_connected) { + } else { + /* + * If display is now connected check links status, + * there has been known issues of link loss triggerring + * long pulse. + * + * Some sinks (eg. ASUS PB287Q) seem to perform some + * weird HPD ping pong during modesets. So we can apparently + * end up with HPD going low during a modeset, and then + * going back up soon after. And once that happens we must + * retrain the link to get a picture. That's in case no + * userspace component reacted to intermittent HPD dip. + */ intel_dp_check_link_status(intel_dp); - goto out; } /* -- cgit v1.2.3 From 5af9e672b84f006695b820f791f2eceecca59bbc Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Thu, 13 Apr 2017 22:52:17 +0300 Subject: drm/i915: checking for NULL instead of IS_ERR() in mock selftests i915_gem_request_alloc() uses error pointers. It never returns NULLs. Fixes: 0daf0113cff6 ("drm/i915: Mock infrastructure for request emission") Signed-off-by: Dan Carpenter Link: http://patchwork.freedesktop.org/patch/msgid/20170413195217.GA26108@mwanda Reviewed-by: Chris Wilson Signed-off-by: Chris Wilson (cherry picked from commit be02f7556447a0dee672acb5e462f03377b98ae8) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/selftests/mock_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/selftests/mock_request.c b/drivers/gpu/drm/i915/selftests/mock_request.c index 0e8d2e7f8c70..8097e3693ec4 100644 --- a/drivers/gpu/drm/i915/selftests/mock_request.c +++ b/drivers/gpu/drm/i915/selftests/mock_request.c @@ -35,7 +35,7 @@ mock_request(struct intel_engine_cs *engine, /* NB the i915->requests slab cache is enlarged to fit mock_request */ request = i915_gem_request_alloc(engine, context); - if (!request) + if (IS_ERR(request)) return NULL; mock = container_of(request, typeof(*mock), base); -- cgit v1.2.3 From acf2dc2266b99be9103b5c35acbb8f9fe923bf3d Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Thu, 13 Apr 2017 14:15:27 +0300 Subject: drm/i915: Fix system hang with EI UP masked on Haswell Previously with commit a9c1f90c8e17 ("drm/i915: Don't mask EI UP interrupt on IVB|SNB") certain, seemingly unrelated bit (GEN6_PM_RP_UP_EI_EXPIRED) was needed to be unmasked for IVB and SNB in order to prevent system hang with chained batchbuffers. Our CI was seeing incomplete results with tests that used chained batches and it was found out that HSW needs to have this same bit unmasked to reliably survive chained batches. Always unmask GEN6_PM_RP_UP_EI_EXPIRED on Haswell to prevent system hang with batch chaining. Testcase: igt/gem_exec_fence/nb-await-default Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100672 Cc: Chris Wilson Cc: stable@vger.kernel.org Signed-off-by: Mika Kuoppala Acked-by: Chris Wilson Link: http://patchwork.freedesktop.org/patch/msgid/1492082127-29007-1-git-send-email-mika.kuoppala@intel.com (cherry picked from commit 3396a273851c14634b98bb27be37508b06df94f4) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d9d196977f4a..fd97fe00cd0d 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -4252,12 +4252,12 @@ void intel_irq_init(struct drm_i915_private *dev_priv) dev_priv->rps.pm_intrmsk_mbz = 0; /* - * SNB,IVB can while VLV,CHV may hard hang on looping batchbuffer + * SNB,IVB,HSW can while VLV,CHV may hard hang on looping batchbuffer * if GEN6_PM_UP_EI_EXPIRED is masked. * * TODO: verify if this can be reproduced on VLV,CHV. */ - if (INTEL_INFO(dev_priv)->gen <= 7 && !IS_HASWELL(dev_priv)) + if (INTEL_INFO(dev_priv)->gen <= 7) dev_priv->rps.pm_intrmsk_mbz |= GEN6_PM_RP_UP_EI_EXPIRED; if (INTEL_INFO(dev_priv)->gen >= 8) -- cgit v1.2.3 From b162d47e142581bfae356d9474856c972c40adbe Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 19 Apr 2017 10:41:17 +0100 Subject: drm/i915/selftests: Allocate inode/file dynamically Avoid having too large a stack by creating the fake struct inode/file on the heap instead. drivers/gpu/drm/i915/selftests/mock_drm.c: In function 'mock_file': drivers/gpu/drm/i915/selftests/mock_drm.c:46:1: error: the frame size of 1328 bytes is larger than 1280 bytes [-Werror=frame-larger-than=] drivers/gpu/drm/i915/selftests/mock_drm.c: In function 'mock_file_free': drivers/gpu/drm/i915/selftests/mock_drm.c:54:1: error: the frame size of 1312 bytes is larger than 1280 bytes [-Werror=frame-larger-than=] Reported-by: Arnd Bergmann Fixes: 66d9cb5d805a ("drm/i915: Mock the GEM device for self-testing") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Tvrtko Ursulin Cc: Matthew Auld Cc: Arnd Bergmann Acked-by: Arnd Bergmann Link: http://patchwork.freedesktop.org/patch/msgid/20170419094143.16922-2-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen (cherry picked from commit 2310b3c952c5dc56c2e08f71b907b8e23ab3270d) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/selftests/mock_drm.c | 45 ++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.c b/drivers/gpu/drm/i915/selftests/mock_drm.c index 113dec05c7dc..09c704153456 100644 --- a/drivers/gpu/drm/i915/selftests/mock_drm.c +++ b/drivers/gpu/drm/i915/selftests/mock_drm.c @@ -24,31 +24,50 @@ #include "mock_drm.h" -static inline struct inode fake_inode(struct drm_i915_private *i915) -{ - return (struct inode){ .i_rdev = i915->drm.primary->index }; -} - struct drm_file *mock_file(struct drm_i915_private *i915) { - struct inode inode = fake_inode(i915); - struct file filp = {}; + struct file *filp; + struct inode *inode; struct drm_file *file; int err; - err = drm_open(&inode, &filp); - if (unlikely(err)) - return ERR_PTR(err); + inode = kzalloc(sizeof(*inode), GFP_KERNEL); + if (!inode) { + err = -ENOMEM; + goto err; + } + + inode->i_rdev = i915->drm.primary->index; - file = filp.private_data; + filp = kzalloc(sizeof(*filp), GFP_KERNEL); + if (!filp) { + err = -ENOMEM; + goto err_inode; + } + + err = drm_open(inode, filp); + if (err) + goto err_filp; + + file = filp->private_data; + memset(&file->filp, POISON_INUSE, sizeof(file->filp)); file->authenticated = true; + + kfree(filp); + kfree(inode); return file; + +err_filp: + kfree(filp); +err_inode: + kfree(inode); +err: + return ERR_PTR(err); } void mock_file_free(struct drm_i915_private *i915, struct drm_file *file) { - struct inode inode = fake_inode(i915); struct file filp = { .private_data = file }; - drm_release(&inode, &filp); + drm_release(NULL, &filp); } -- cgit v1.2.3 From 96dabe99cae8a1b5fc645f213eeac928541d3899 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 21 Apr 2017 14:58:15 +0100 Subject: drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The busy-spin, as the first stage of intel_wait_for_register(), is currently under suspicion for causing: [ 62.034926] NMI watchdog: Watchdog detected hard LOCKUP on cpu 1 [ 62.034928] Modules linked in: i2c_dev i915 intel_gtt drm_kms_helper prime_numbers [ 62.034932] CPU: 1 PID: 183 Comm: kworker/1:2 Not tainted 4.11.0-rc7+ #471 [ 62.034933] Hardware name: / , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015 [ 62.034934] Workqueue: pm pm_runtime_work [ 62.034936] task: ffff880275a04ec0 task.stack: ffffc900002d8000 [ 62.034936] RIP: 0010:__intel_wait_for_register_fw+0x77/0x1a0 [i915] [ 62.034937] RSP: 0018:ffffc900002dbc38 EFLAGS: 00000082 [ 62.034939] RAX: ffffc90003530094 RBX: 0000000000130094 RCX: 0000000000000001 [ 62.034940] RDX: 00000000000000a1 RSI: ffff88027fd15e58 RDI: 0000000000000000 [ 62.034941] RBP: ffffc900002dbc78 R08: 0000000000000002 R09: 0000000000000000 [ 62.034942] R10: ffffc900002dbc18 R11: ffff880276429dd0 R12: ffff8802707c0000 [ 62.034943] R13: 00000000000000a0 R14: 0000000000000000 R15: 00000000fffefc10 [ 62.034945] FS: 0000000000000000(0000) GS:ffff88027fd00000(0000) knlGS:0000000000000000 [ 62.034945] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 62.034947] CR2: 00007ffd3cd98ff8 CR3: 0000000274c19000 CR4: 00000000001006e0 [ 62.034947] Call Trace: [ 62.034948] intel_wait_for_register+0x77/0x140 [i915] [ 62.034949] vlv_suspend_complete+0x23/0x5b0 [i915] [ 62.034950] intel_runtime_suspend+0x16c/0x2a0 [i915] [ 62.034950] pci_pm_runtime_suspend+0x50/0x180 [ 62.034951] ? pci_pm_runtime_resume+0xa0/0xa0 [ 62.034952] __rpm_callback+0xc5/0x210 [ 62.034953] rpm_callback+0x1f/0x80 [ 62.034953] ? pci_pm_runtime_resume+0xa0/0xa0 [ 62.034954] rpm_suspend+0x118/0x580 [ 62.034955] pm_runtime_work+0x64/0x90 [ 62.034956] process_one_work+0x1bb/0x3e0 [ 62.034956] worker_thread+0x46/0x4f0 [ 62.034957] ? __schedule+0x18b/0x610 [ 62.034958] kthread+0xff/0x140 [ 62.034958] ? process_one_work+0x3e0/0x3e0 [ 62.034959] ? kthread_create_on_node+ and related hard lockups in CI for byt and bsw. Note this effectively reverts commits 41ce405e6894 and b27366958869 ("drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()") v2: Convert bool allow into a u32 mask for clarity and repeat the comment on vlv rc6 timing to justify the 3ms timeout used for the wait (Ville) Fixes: 41ce405e6894 ("drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()") Fixes: b27366958869 ("drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100718 Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Ville Syrjälä Cc: Tomi Sarvela Reviewed-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/20170421135815.11897-1-chris@chris-wilson.co.uk Tested-by: Tomi Sarvela (cherry picked from commit 3dd14c04d77d7d702de5aa7157df4cc9417329f3) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_drv.c | 46 +++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index c616b4e755bc..1aa26d5f1779 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2177,6 +2177,20 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv) I915_WRITE(VLV_GUNIT_CLOCK_GATE2, s->clock_gate_dis2); } +static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv, + u32 mask, u32 val) +{ + /* The HW does not like us polling for PW_STATUS frequently, so + * use the sleeping loop rather than risk the busy spin within + * intel_wait_for_register(). + * + * Transitioning between RC6 states should be at most 2ms (see + * valleyview_enable_rps) so use a 3ms timeout. + */ + return wait_for((I915_READ_NOTRACE(VLV_GTLC_PW_STATUS) & mask) == val, + 3); +} + int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on) { u32 val; @@ -2205,8 +2219,9 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on) static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow) { + u32 mask; u32 val; - int err = 0; + int err; val = I915_READ(VLV_GTLC_WAKE_CTRL); val &= ~VLV_GTLC_ALLOWWAKEREQ; @@ -2215,45 +2230,32 @@ static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow) I915_WRITE(VLV_GTLC_WAKE_CTRL, val); POSTING_READ(VLV_GTLC_WAKE_CTRL); - err = intel_wait_for_register(dev_priv, - VLV_GTLC_PW_STATUS, - VLV_GTLC_ALLOWWAKEACK, - allow, - 1); + mask = VLV_GTLC_ALLOWWAKEACK; + val = allow ? mask : 0; + + err = vlv_wait_for_pw_status(dev_priv, mask, val); if (err) DRM_ERROR("timeout disabling GT waking\n"); return err; } -static int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv, - bool wait_for_on) +static void vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv, + bool wait_for_on) { u32 mask; u32 val; - int err; mask = VLV_GTLC_PW_MEDIA_STATUS_MASK | VLV_GTLC_PW_RENDER_STATUS_MASK; val = wait_for_on ? mask : 0; - if ((I915_READ(VLV_GTLC_PW_STATUS) & mask) == val) - return 0; - - DRM_DEBUG_KMS("waiting for GT wells to go %s (%08x)\n", - onoff(wait_for_on), - I915_READ(VLV_GTLC_PW_STATUS)); /* * RC6 transitioning can be delayed up to 2 msec (see * valleyview_enable_rps), use 3 msec for safety. */ - err = intel_wait_for_register(dev_priv, - VLV_GTLC_PW_STATUS, mask, val, - 3); - if (err) + if (vlv_wait_for_pw_status(dev_priv, mask, val)) DRM_ERROR("timeout waiting for GT wells to go %s\n", onoff(wait_for_on)); - - return err; } static void vlv_check_no_gt_access(struct drm_i915_private *dev_priv) @@ -2274,7 +2276,7 @@ static int vlv_suspend_complete(struct drm_i915_private *dev_priv) * Bspec defines the following GT well on flags as debug only, so * don't treat them as hard failures. */ - (void)vlv_wait_for_gt_wells(dev_priv, false); + vlv_wait_for_gt_wells(dev_priv, false); mask = VLV_GTLC_RENDER_CTX_EXISTS | VLV_GTLC_MEDIA_CTX_EXISTS; WARN_ON((I915_READ(VLV_GTLC_WAKE_CTRL) & mask) != mask); -- cgit v1.2.3 From 88326ef05b262f681d837ecf65db10a7edb609f1 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Sat, 22 Apr 2017 09:15:37 +0100 Subject: drm/i915: Confirm the request is still active before adding it to the await MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Although we do check the completion-status of the request before actually adding a wait on it (either to its submit fence or its completion dma-fence), we currently do not check before adding it to the dependency lists. In fact, without checking for a completed request we may try to use the signaler after it has been retired and its dependency tree freed: [ 60.044057] BUG: KASAN: use-after-free in __list_add_valid+0x1d/0xd0 at addr ffff880348c9e6a0 [ 60.044118] Read of size 8 by task gem_exec_fence/530 [ 60.044164] CPU: 1 PID: 530 Comm: gem_exec_fence Tainted: G E 4.11.0-rc7+ #46 [ 60.044226] Hardware name: ��������������������������������� ���������������������������������/���������������������������������, BIOS RYBDWi35.86A.0246.2 [ 60.044290] Call Trace: [ 60.044337] dump_stack+0x4d/0x6a [ 60.044383] kasan_object_err+0x21/0x70 [ 60.044435] kasan_report+0x225/0x4e0 [ 60.044488] ? __list_add_valid+0x1d/0xd0 [ 60.044534] ? kasan_kmalloc+0xad/0xe0 [ 60.044587] __asan_load8+0x5e/0x70 [ 60.044639] __list_add_valid+0x1d/0xd0 [ 60.044788] __i915_priotree_add_dependency+0x67/0x130 [i915] [ 60.044895] i915_gem_request_await_request+0xa8/0x370 [i915] [ 60.044974] i915_gem_request_await_dma_fence+0x129/0x140 [i915] [ 60.045049] i915_gem_do_execbuffer.isra.37+0xb0a/0x26b0 [i915] [ 60.045077] ? save_stack+0xb1/0xd0 [ 60.045105] ? save_stack_trace+0x1b/0x20 [ 60.045132] ? save_stack+0x46/0xd0 [ 60.045158] ? kasan_kmalloc+0xad/0xe0 [ 60.045184] ? __kmalloc+0xd8/0x670 [ 60.045229] ? drm_ioctl+0x359/0x640 [drm] [ 60.045256] ? SyS_ioctl+0x41/0x70 [ 60.045330] ? i915_vma_move_to_active+0x540/0x540 [i915] [ 60.045360] ? tty_insert_flip_string_flags+0xa1/0xf0 [ 60.045387] ? tty_flip_buffer_push+0x63/0x70 [ 60.045414] ? remove_wait_queue+0xa9/0xc0 [ 60.045441] ? kasan_unpoison_shadow+0x35/0x50 [ 60.045467] ? kasan_kmalloc+0xad/0xe0 [ 60.045494] ? kasan_check_write+0x14/0x20 [ 60.045568] i915_gem_execbuffer2+0xdb/0x2a0 [i915] [ 60.045616] drm_ioctl+0x359/0x640 [drm] [ 60.045705] ? i915_gem_execbuffer+0x5a0/0x5a0 [i915] [ 60.045751] ? drm_version+0x150/0x150 [drm] [ 60.045778] ? compat_start_thread+0x60/0x60 [ 60.045805] ? plist_del+0xda/0x1a0 [ 60.045833] do_vfs_ioctl+0x12e/0x910 [ 60.045860] ? ioctl_preallocate+0x130/0x130 [ 60.045886] ? pci_mmcfg_check_reserved+0xc0/0xc0 [ 60.045913] ? vfs_write+0x196/0x240 [ 60.045939] ? __fget_light+0xa7/0xc0 [ 60.045965] SyS_ioctl+0x41/0x70 [ 60.045991] entry_SYSCALL_64_fastpath+0x17/0x98 [ 60.046017] RIP: 0033:0x7feb2baefc47 [ 60.046042] RSP: 002b:00007fff56d28e58 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [ 60.046075] RAX: ffffffffffffffda RBX: 00007fff56d290a8 RCX: 00007feb2baefc47 [ 60.046102] RDX: 00007fff56d29050 RSI: 00000000c0406469 RDI: 0000000000000003 [ 60.046129] RBP: 00007fff56d29050 R08: 000055ecc4cd27d0 R09: 00007feb2bda8600 [ 60.046154] R10: 0000000000000073 R11: 0000000000000246 R12: 00000000c0406469 [ 60.046177] R13: 0000000000000003 R14: 000000000000000f R15: 0000000000000099 [ 60.046203] Object at ffff880348c9e680, in cache i915_dependency size: 64 [ 60.046225] Allocated: [ 60.046246] PID = 530 [ 60.046269] save_stack_trace+0x1b/0x20 [ 60.046292] save_stack+0x46/0xd0 [ 60.046318] kasan_kmalloc+0xad/0xe0 [ 60.046343] kasan_slab_alloc+0x12/0x20 [ 60.046368] kmem_cache_alloc+0xab/0x650 [ 60.046445] i915_gem_request_await_request+0x88/0x370 [i915] [ 60.046559] i915_gem_request_await_dma_fence+0x129/0x140 [i915] [ 60.046705] i915_gem_do_execbuffer.isra.37+0xb0a/0x26b0 [i915] [ 60.046849] i915_gem_execbuffer2+0xdb/0x2a0 [i915] [ 60.046936] drm_ioctl+0x359/0x640 [drm] [ 60.046987] do_vfs_ioctl+0x12e/0x910 [ 60.047038] SyS_ioctl+0x41/0x70 [ 60.047090] entry_SYSCALL_64_fastpath+0x17/0x98 [ 60.047139] Freed: [ 60.047179] PID = 530 [ 60.047223] save_stack_trace+0x1b/0x20 [ 60.047269] save_stack+0x46/0xd0 [ 60.047317] kasan_slab_free+0x72/0xc0 [ 60.047366] kmem_cache_free+0x39/0x160 [ 60.047512] i915_gem_request_retire+0x83f/0x930 [i915] [ 60.047657] i915_gem_request_alloc+0x166/0x600 [i915] [ 60.047799] i915_gem_do_execbuffer.isra.37+0xad8/0x26b0 [i915] [ 60.047897] i915_gem_execbuffer2+0xdb/0x2a0 [i915] [ 60.047942] drm_ioctl+0x359/0x640 [drm] [ 60.047968] do_vfs_ioctl+0x12e/0x910 [ 60.047993] SyS_ioctl+0x41/0x70 [ 60.048019] entry_SYSCALL_64_fastpath+0x17/0x98 [ 60.048044] Memory state around the buggy address: [ 60.048066] ffff880348c9e580: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc [ 60.048105] ffff880348c9e600: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc [ 60.048138] >ffff880348c9e680: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc [ 60.048170] ^ [ 60.048191] ffff880348c9e700: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc [ 60.048225] ffff880348c9e780: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc Note to hit the use-after-free requires us to be passed back a request via a fence-array, that is from explicit fencing accumulated into a sync-file fence-array. Fixes: 52e542090701 ("drm/i915/scheduler: Record all dependencies upon request construction") Testcase: igt/gem_exec_fence/expired-history Signed-off-by: Chris Wilson Reviewed-by: Michał Winiarski Reviewed-by: Joonas Lahtinen Cc: Tvrtko Ursulin Link: http://patchwork.freedesktop.org/patch/msgid/20170422081537.6468-1-chris@chris-wilson.co.uk (cherry picked from commit ade0b0c965f59176daddbef9c4717354034f9bce) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_gem_request.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 6348353b91ec..5ddbc9499775 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -652,6 +652,9 @@ i915_gem_request_await_request(struct drm_i915_gem_request *to, GEM_BUG_ON(to == from); + if (i915_gem_request_completed(from)) + return 0; + if (to->engine->schedule) { ret = i915_priotree_add_dependency(to->i915, &to->priotree, -- cgit v1.2.3