- 12 Feb, 2018 1 commit
-
-
Chris Wilson authored
If we fail to reset the GPU (i915_reset()), we do one final intel_gpu_reset() attempt as we mark the device wedged. The idea here is even though the GPU has proven unreliable (and so we want to stop using it for the time being), we don't want it spinning away in the background whilst the driver idles so we try to reset it one more time. However, we want to dump the i915_gem_set_wedged() debugging info before we do, so that we can see the accurate state of the GPU when it failed. Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180209114056.9957-1-chris@chris-wilson.co.uk
-
- 09 Feb, 2018 5 commits
-
-
Tvrtko Ursulin authored
Instead of INTEL_GEN != x use !IS_GENx for more optimisation opportunities. Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180208130606.15556-16-tvrtko.ursulin@linux.intel.comReviewed-by:
-
Tvrtko Ursulin authored
Coccinelle patch: @@ identifier p; @@ -INTEL_INFO(p)->gen +INTEL_GEN(p) Signed-off-by:
-
Ville Syrjälä authored
Most of our ioctl functions have an _ioctl suffix in the name. I like that idea since it makes it easy to figure out how the function is going to get called. Rename the handful of exceptions to follow the same pattern. Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180207164841.19431-1-ville.syrjala@linux.intel.comReviewed-by:
-
Ville Syrjälä authored
Check that userspace isn't passing in garbage in the colorkey ioctl flags. Signed-off-by:
-
Imre Deak authored
FORCEWAKE_ACK is depricated by BSpec at least starting from BDW, referring to the multi-threaded version of it instead. Accessing FORCEWAKE_ACK triggers an unclaimed register access error - at least on GLK - see the Reference: below. The correct registers to use would be FORCEWAKE_MT_ACK on IVB+ and FORCEWAKE_ACK_RENDER_GEN9 on SKL+ like it's done elsewhere in the driver. The forcewake check itself is inconsistent and redundant, since there could be other forcewake requesters besides the kernel (being the multithreaded version of the register) and the kernel's per-domain forcewake counters are shown anyway at the end of the file. So let's just remove the check. Suggested-by:
Imre Deak <imre.deak@intel.com> Tested-by:
Lionel Landwerlin <lionel.g.landwerlin@intel.com> Reviewed-by:
-
- 08 Feb, 2018 24 commits
-
-
Chris Wilson authored
Since commit 4a118ecb ("drm/i915: Filter out spurious execlists context-switch interrupts") we probe execlists->active, and no longer have to peek at the execlist interrupt to determine if the tasklet still needs to be run to drain the ELSP. References: 4a118ecb ("drm/i915: Filter out spurious execlists context-switch interrupts") Signed-off-by:
-
Chris Wilson authored
clang is confused by our if-else-chain that abruptly exits before a final else: drivers/gpu/drm/i915/intel_crt.c:821:11: warning: variable 'status' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] else if (ret < 0) ^~~~~~~ drivers/gpu/drm/i915/intel_crt.c:826:9: note: uninitialized use occurs here return status; ^~~~~~ drivers/gpu/drm/i915/intel_crt.c:821:7: note: remove the 'if' if its condition is always true else if (ret < 0) ^~~~~~~~~~~~ drivers/gpu/drm/i915/intel_crt.c:761:12: note: initialize the variable 'status' to silence this warning int status, ret; In this case, we can reduce the final else-if clause to an unconditional else. Signed-off-by: -
Chris Wilson authored
The struct platform_device memdups the provided data pointer requiring us to free the template we construct during lpe_audio_platdev_create(): unreferenced object 0xffff88026eafe400 (size 512): comm "insmod", pid 6850, jiffies 4295060179 (age 22.300s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [<000000008e4a834c>] intel_audio_init+0x9/0x30 [i915] [<000000001360e195>] i915_driver_load+0x802/0x14e0 [i915] [<00000000ab3f0e99>] i915_pci_probe+0x29/0x70 [i915] [<0000000016330ee5>] pci_device_probe+0x9c/0x120 [<000000000257d054>] driver_probe_device+0x307/0x470 [<000000009f0a6cb6>] __driver_attach+0x98/0xe0 [<0000000031b46e58>] bus_for_each_dev+0x57/0x80 [<000000000e28239d>] bus_add_driver+0x1bd/0x260 [<00000000abbe5161>] driver_register+0x52/0xc0 [<000000005c6e23d4>] do_one_initcall+0x36/0x150 [<00000000a55002f4>] do_init_module+0x56/0x1d7 [<00000000e48f2217>] load_module+0x23c8/0x2910 [<000000002b60bf61>] SyS_finit_module+0xb8/0xd0 [<0000000041cbad96>] entry_SYSCALL_64_fastpath+0x17/0x70 [<000000009f1d37ab>] 0xffffffffffffffff Signed-off-by:Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171209222133.31880-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
The unused-but-set warning enabled by W=1 catches out a lot of the atomic helper iterator macros and drown us in their noise (or trip over Werror and die). Path of least resistance is to ignore the warning. Signed-off-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
Chris Wilson authored
drivers/gpu/drm/i915/i915_gem_internal.c:183: warning: No description found for parameter 'i915' drivers/gpu/drm/i915/i915_gem_internal.c:183: warning: No description found for parameter 'size' Signed-off-by:
-
Chris Wilson authored
drivers/gpu/drm/i915/i915_gem_execbuffer.c:1983: warning: No description found for parameter 'dev_priv' drivers/gpu/drm/i915/i915_gem_execbuffer.c:1983: warning: No description found for parameter 'file' Signed-off-by:
-
Chris Wilson authored
drivers/gpu/drm/i915/i915_syncmap.c:92: warning: No description found for parameter 'root' drivers/gpu/drm/i915/i915_syncmap.c:155: warning: No description found for parameter 'root' drivers/gpu/drm/i915/i915_syncmap.c:155: warning: No description found for parameter 'id' drivers/gpu/drm/i915/i915_syncmap.c:155: warning: No description found for parameter 'seqno' drivers/gpu/drm/i915/i915_syncmap.c:354: warning: No description found for parameter 'root' drivers/gpu/drm/i915/i915_syncmap.c:354: warning: No description found for parameter 'id' drivers/gpu/drm/i915/i915_syncmap.c:354: warning: No description found for parameter 'seqno' drivers/gpu/drm/i915/i915_syncmap.c:396: warning: No description found for parameter 'root' Signed-off-by:
-
Chris Wilson authored
drivers/gpu/drm/i915/i915_drv.c:891: warning: No description found for parameter 'ent' Signed-off-by:
-
Chris Wilson authored
The comment is very old and quite misleading now. drivers/gpu/drm/i915/i915_gem_context.c:349: warning: No description found for parameter 'dev_priv' drivers/gpu/drm/i915/i915_gem_context.c:349: warning: No description found for parameter 'file_priv' Signed-off-by:
-
Chris Wilson authored
drivers/gpu/drm/i915/i915_gem_request.c:941: warning: No description found for parameter 'write' Signed-off-by:
-
Chris Wilson authored
drivers/gpu/drm/i915/i915_gem_userptr.c:761: warning: No description found for parameter 'dev' drivers/gpu/drm/i915/i915_gem_userptr.c:761: warning: No description found for parameter 'data' drivers/gpu/drm/i915/i915_gem_userptr.c:761: warning: No description found for parameter 'file' Signed-off-by:
-
Chris Wilson authored
drivers/gpu/drm/i915/intel_ringbuffer.c:179: warning: No description found for parameter 'req' drivers/gpu/drm/i915/intel_ringbuffer.c:741: warning: No description found for parameter 'req' drivers/gpu/drm/i915/intel_ringbuffer.c:741: warning: No description found for parameter 'cs' Signed-off-by:
-
Chris Wilson authored
drivers/gpu/drm/i915/i915_gpu_error.c:1815: warning: No description found for parameter 'dev_priv' drivers/gpu/drm/i915/i915_gpu_error.c:1815: warning: No description found for parameter 'engine_mask' drivers/gpu/drm/i915/i915_gpu_error.c:1815: warning: No description found for parameter 'error_msg' drivers/gpu/drm/i915/i915_gpu_error.c:1815: warning: Excess function parameter 'dev' description in 'i915_capture_error_state' Signed-off-by:
-
Chris Wilson authored
After we assert the reset request (and wait for 20us), when the device has been fully reset it asserts the reset-status bit. Before we stop requesting the reset and allow the device to return to normal, we should wait for the reset to be completed. (Similar to how we wait for the device to return to normal after deasserting the reset request.) v2: Rename i915_reset_completed() probe to not cause as much confusion. Signed-off-by:
-
Chris Wilson authored
Although the mmio are uncached and so should be flushed on every write, be paranoid and do a mmio read after setting the ring head/tail to be sure they have taken effect before moving on. v2: post tail to be pleasing to the eye Signed-off-by:
-
Chris Wilson authored
Reduce the window of opportunity for set-wedged being called concurrently with reset (after i915_reset() has performed the i915_gem_unset_wedged()) by moving the set_bit(I915_WEDGED) to before we complete the inflight requests. When i915_reset() is being blocked on a request, such completion may allow it to start and beginning resetting the GPU before i915_gem_set_wedged() has finished (and so before set-wedge will have marked the device as wedged). As such, i915_gem_init_hw() may see a wedged device even from inside i915_reset(). References: 36703e79 ("drm/i915: Break modeset deadlocks on reset") Signed-off-by:
-
Chris Wilson authored
Userspace provides a 64b value for the priority, we need to be careful to preserve the full range before validation to prevent truncation (and letting an illegal value pass). Reported-by:
Antonio Argenziano <antonio.argenziano@intel.com> Fixes: ac14fbd4 ("drm/i915/scheduler: Support user-defined priorities") Signed-off-by:
-
Chris Wilson authored
We only need to wake up the RPS worker once when initially enabling the client boost, it remains in effect then until the last client no longer requires the boost. References: https://bugs.freedesktop.org/show_bug.cgi?id=102250 References: 7b92c1bd ("drm/i915: Avoid keeping waitboost active for signaling threads") Signed-off-by:
Michał Winiarski <michal.winiarski@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180206143137.15509-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
drivers/gpu/drm/i915/i915_oa_cnl.c: In function ‘i915_perf_load_test_config_cnl’: drivers/gpu/drm/i915/i915_oa_cnl.c:99:2: error: ‘strncpy’ output truncated before terminating nul copying 36 bytes from a string of the same length [-Werror=stringop-truncation] v2: strlcpy Fixes: 95690a02 ("drm/i915/perf: enable perf support on CNL") Signed-off-by:
-
Chris Wilson authored
drivers/gpu/drm/i915/i915_oa_cflgt3.c: In function ‘i915_perf_load_test_config_cflgt3’: drivers/gpu/drm/i915/i915_oa_cflgt3.c:87:2: error: ‘strncpy’ output truncated before terminating nul copying 36 bytes from a string of the same length [-Werror=stringop-truncation] v2: strlcpy Fixes: 4407eaa9 ("drm/i915/perf: add support for Coffeelake GT3") Signed-off-by:
-
Daniele Ceraolo Spurio authored
Since commit 5896a5c8 (drm/i915: Always stop the rings before a missing GPU reset) we attempt to stop the engines during gem_sanitize even if reset=0 and nothing bad happened on the gpu. The specs says that the STOP_RINGS bit needs to be cleared to resume normal operation, but for some reason the value of the bit seems to be changing without us writing to it (maybe rc6 entry/exit?), so normal operation resumes correctly. However, it still feels incorrect to stop the engines if there hasn't been any issue so skip the whole reset call in gem_sanitize if i915.reset=0 Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Signed-off-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180207212440.13438-1-daniele.ceraolospurio@intel.comReviewed-by:
-
Chris Wilson authored
If we remove some hardcoded assumptions about the preempt context having a fixed id, reserved from use by normal user contexts, we may only allocate the i915_gem_context when required. Then the subsequent decisions on using preemption reduce to having the preempt context available. v2: Include an assert that we don't allocate the preempt context twice. Signed-off-by:
Michel Thierry <michel.thierry@intel.com>
-
Chris Wilson authored
Rather than having the high level ioctl interface guess the underlying implementation details, having the implementation declare what capabilities it exports. We define an intel_driver_caps, similar to the intel_device_info, which instead of trying to describe the HW gives details on what the driver itself supports. This is then populated by the engine backend for the new scheduler capability field for use elsewhere. v2: Use caps.scheduler for validating CONTEXT_PARAM_SET_PRIORITY (Mika) One less assumption of engine[RCS] \o/ Signed-off-by:Tomasz Lis <tomasz.lis@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180207210544.26351-2-chris@chris-wilson.co.ukReviewed-by:
-
Chris Wilson authored
In the next patch, we may only conditionally allocate the preempt-client if there is a global preempt context and so we need to be prepared in case the preempt-client itself is NULL. v2: Grep for more preempt_client. Signed-off-by:
-
- 07 Feb, 2018 8 commits
-
-
Chris Wilson authored
As we peek inside struct device to query members guarded by CONFIG_PM, so must be the code. Reported-by:
kbuild test robot <fengguang.wu@intel.com> Fixes: 1fe699e3 ("drm/i915/pmu: Fix sleep under atomic in RC6 readout") Signed-off-by:
-
Tvrtko Ursulin authored
We are not allowed to call intel_runtime_pm_get from the PMU counter read callback since the former can sleep, and the latter is running under IRQ context. To workaround this, we record the last known RC6 and while runtime suspended estimate its increase by querying the runtime PM core timestamps. Downside of this approach is that we can temporarily lose a chunk of RC6 time, from the last PMU read-out to runtime suspend entry, but that will eventually catch up, once device comes back online and in the presence of PMU queries. Also, we have to be careful not to overshoot the RC6 estimate, so once resumed after a period of approximation, we only update the counter once it catches up. With the observation that RC6 is increasing while the device is suspended, this should not pose a problem and can only cause slight inaccuracies due clock base differences. v2: Simplify by estimating on top of PM core counters. (Imre) Signed-off-by:
-
Chris Wilson authored
On blb and pnv, we are seeing sporadic i915 0000:00:02.0: Resetting chip after gpu hang [drm:intel_gpu_reset [i915]] rcs0: timed out on STOP_RING [drm:i915_reset [i915]] *ERROR* Failed hw init on reset -5 which notably lack the actual root cause of the error. Ostensibly it should be the init_ring_common() that failed, but it's error paths are covered by DRM_ERROR. Signed-off-by:
-
Chris Wilson authored
If we submit a request and see that the previous request on this timeline was already signaled, we first do not need to add the dependency tracker for that completed request and secondly we know that we there is then a large backlog in retiring requests affecting this timeline. Given that we just submitted more work to the HW, now would be a good time to catch up on those retirements. v2: Try to sum up the compromises involved in flushing the retirement queue after submission. Signed-off-by:
-
Chris Wilson authored
If the last request on the timeline is already complete, we do not need to emit the serialisation barriers. Signed-off-by:
-
Maarten Lankhorst authored
According to bspec, result_lines > 31 is only a maximum for latency level 1 through 7. For level 0 the number of lines is ignored, so always write 0 there to prevent overflowing the 5 bits value. This is required to make NV12 work. Changes since v1: - Rebase on top of GEN11 wm changes. It seems to use res_lines for level 0 limit calculations, but still doesn't appear to program it. Signed-off-by:
-
Chris Wilson authored
When a request is preempted, it is unsubmitted from the HW queue and removed from the active list of breadcrumbs. In the process, this however triggers the signaler and it may see the clear rbtree with the old, and still valid, seqno, or it may match the cleared seqno with the now zero rq->global_seqno. This confuses the signaler into action and signaling the fence. Fixes: d6a2289d ("drm/i915: Remove the preempted request from the execution queue") Signed-off-by:
-
Joonas Lahtinen authored
Signed-off-by:
-
- 06 Feb, 2018 2 commits
-
-
Tvrtko Ursulin authored
Commit 99e48bf9 ("drm/i915: Lock out execlist tasklet while peeking inside for busy-stats") added a tasklet_disable call in busy stats enabling, but we failed to understand that the PMU enable callback runs as an hard IRQ (IPI). Consequence of this is that the PMU enable callback can interrupt the execlists tasklet, and will then deadlock when it calls intel_engine_stats_enable->tasklet_disable. To fix this, I realized it is possible to move the engine stats enablement and disablement to PMU event init and destroy hooks. This allows for much simpler implementation since those hooks run in normal context (can sleep). v2: Extract engine_event_destroy. (Chris Wilson) Signed-off-by:
-
Rafael Antognolli authored
This workaround should prevent a bug that can be hit on a context restore. To avoid the issue, we must emit a PIPE_CONTROL with CS stall (0x7a000004 0x00100000 0x00000000 0x00000000) followed by 12DW's of NOOP(0x0) in the indirect context batch buffer, to ensure the engine is idle prior to programming 3DSTATE_SAMPLE_PATTERN. It's also not clear whether we should add those extra dwords because of the workaround itself, or if that's just padding for the WA BB (and next commands could come right after the PIPE_CONTROL). We keep them for now. References: HSD#1939868 v2: More descriptive changelog and comments. v3: Explain that PIPE_CONTROL is actually 6 dwords, and that we advance 10 more dwords because of that. Signed-off-by:Rafael Antognolli <rafael.antognolli@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Acked-by:
-
