- 13 May, 2020 7 commits
-
-
Chris Wilson authored
Since there can only be one of in_fence/exec_fence, just use the single in_fence local. v2: Consolidate lookup Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200513180937.28992-1-chris@chris-wilson.co.uk
-
Nathan Chancellor authored
When building with clang: drivers/gpu/drm/i915/gt/gen8_ppgtt.c:392:24: warning: duplicate 'inline' declaration specifier [-Wduplicate-decl-specifier] declaration specifier [-Wduplicate-decl-specifier] static __always_inline inline void ^ include/linux/compiler_types.h:138:16: note: expanded from macro 'inline' #define inline inline __gnu_inline __inline_maybe_unused notrace ^ 1 warning generated. __always_inline is defined as 'inline __attribute__((__always_inline))' so we do not need to specify it twice. Fixes: 84eac0c6 ("drm/i915/gt: Force pte cacheline to main memory") Link: https://github.com/ClangBuiltLinux/linux/issues/1024Reported-by:kbuild test robot <lkp@intel.com> Signed-off-by:
Nathan Chancellor <natechancellor@gmail.com> Reviewed-by:
-
Chris Wilson authored
It is possible for a residual tasklet to be pending execution as we resume (whether that's some prior test kicking off the tasklet, or if we are in a suspend/resume stress test). As such, we do not want that tasklet to execute in the middle of our sanitization, such that it sees the poisoned state. For example, <4>[ 449.386553] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI <4>[ 449.386555] CPU: 1 PID: 5115 Comm: i915_selftest Tainted: G U W 5.7.0-rc4-CI-CI_DRM_8472+ #1 <4>[ 449.386556] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP TLC, BIOS ICLSFWR1.R00.3183.A00.1905020411 05/02/2019 <4>[ 449.386585] RIP: 0010:process_csb+0x6bf/0x830 [i915] <4>[ 449.386588] Code: 00 48 c7 c2 10 bc 4c a0 48 c7 c7 d4 75 34 a0 e8 87 0e e6 e0 bf 01 00 00 00 e8 9d e0 e5 e0 31 f6 bf 09 00 00 00 e8 e1 ba d6 e0 <0f> 0b 8b 87 10 05 00 00 85 c0 0f 85 5f f9 ff ff 48 c7 c1 70 a5 4f <4>[ 449.386591] RSP: 0018:ffffc90000170ea0 EFLAGS: 00010297 <4>[ 449.386594] RAX: 0000000080000101 RBX: 0000000000000000 RCX: 0000000000000000 <4>[ 449.386596] RDX: ffff88849d5bc040 RSI: 0000000000000000 RDI: 0000000000000009 <4>[ 449.386598] RBP: ffffc90000170f00 R08: 0000000000000000 R09: 0000000000000000 <4>[ 449.386600] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88843ccea018 <4>[ 449.386602] R13: ffff88843ccea658 R14: ffff88843ccea640 R15: ffff88843ccea000 <4>[ 449.386605] FS: 00007f826a813300(0000) GS:ffff88849fe80000(0000) knlGS:0000000000000000 <4>[ 449.386607] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 <4>[ 449.386609] CR2: 0000560366b94280 CR3: 000000048ba02002 CR4: 0000000000760ee0 <4>[ 449.386611] PKRU: 55555554 <4>[ 449.386613] Call Trace: <4>[ 449.386616] <IRQ> <4>[ 449.386646] ? execlists_submission_tasklet+0xcf/0x140 [i915] <4>[ 449.386674] execlists_submission_tasklet+0x2f/0x140 [i915] <4>[ 449.386679] tasklet_action_common.isra.16+0x6c/0x1c0 <4>[ 449.386684] __do_softirq+0xdf/0x49e <4>[ 449.386687] irq_exit+0xba/0xc0 <4>[ 449.386690] smp_apic_timer_interrupt+0xb7/0x280 <4>[ 449.386693] apic_timer_interrupt+0xf/0x20 <4>[ 449.386695] </IRQ> <4>[ 449.386698] RIP: 0010:_raw_spin_unlock_irqrestore+0x49/0x60 <4>[ 449.386701] Code: c7 02 75 1f 53 9d e8 26 ab 75 ff bf 01 00 00 00 e8 7c a3 69 ff 65 8b 05 7d 9b 5c 7e 85 c0 74 0c 5b 5d c3 e8 09 aa 75 ff 53 9d <eb> df e8 ca 39 5b ff 5b 5d c3 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 <4>[ 449.386703] RSP: 0018:ffffc90000a6b950 EFLAGS: 00000202 ORIG_RAX: ffffffffffffff13 <4>[ 449.386706] RAX: 0000000080000001 RBX: 0000000000000202 RCX: 0000000000000000 <4>[ 449.386708] RDX: ffff88849d5bc040 RSI: ffff88849d5bc900 RDI: ffffffff82386f12 <4>[ 449.386710] RBP: ffff88847d400f00 R08: ffff88849d5bc900 R09: 0000000000000000 <4>[ 449.386712] R10: 0000000000000000 R11: 0000000000000000 R12: 00000000ffff0b0b <4>[ 449.386714] R13: 000000000000000c R14: ffff88847d40bf70 R15: ffff88847d40cef8 <4>[ 449.386742] reset_csb_pointers+0x59/0x140 [i915] <4>[ 449.386769] execlists_sanitize+0x3e/0x60 [i915] <4>[ 449.386797] gt_sanitize+0xd6/0x260 [i915] As part of the reset preparation, engine->reset.prepare() prevents the tasklet from running, so pull the sanitization inside the critical section for reset. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1812 Fixes: 23122a4d ("drm/i915/gt: Scrub execlists state on resume") Signed-off-by:
-
Stanislav Lisovskiy authored
Seems that only skl needs to have SAGV turned off for multipipe scenarios, so lets do it this way. If anything blows up - we can always revert this patch. v2: Changed if condition to look better (Ville). Signed-off-by:
Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> [vsyrjala: wrapped long line to appease checkpatch] Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200513093816.11466-4-stanislav.lisovskiy@intel.com
-
Stanislav Lisovskiy authored
Introduce platform dependent SAGV checking in combination with bandwidth state pipe SAGV mask. This is preparation to adding TGL support, which requires different way of SAGV checking. v2, v3, v4, v5, v6: Fix rebase conflict v7: - Nuke icl specific function, use skl for icl as well, gen specific active_pipes check to be added in the next patch(Ville) v8: - Use more generic intel_crtc_can_enable_sagv for checking(Ville) Signed-off-by: -
Stanislav Lisovskiy authored
For future Gen12 SAGV implementation we need to seemlessly alter wm levels calculated, depending on whether we are allowed to enable SAGV or not. So this accessor will give additional flexibility to do that. Currently this accessor is still simply working as "pass-through" function. This will be changed in next coming patches from this series. v2: - plane_id -> plane->id(Ville Syrjälä) - Moved wm_level var to have more local scope (Ville Syrjälä) - Renamed yuv to color_plane(Ville Syrjälä) in skl_plane_wm_level v3: - plane->id -> plane_id(this time for real, Ville Syrjälä) - Changed colorplane id type from boolean to int as index (Ville Syrjälä) - Moved crtc_state param so that it is first now (Ville Syrjälä) - Moved wm_level declaration to tigher scope in skl_write_plane_wm(Ville Syrjälä) v4: - Started to use enum values for color plane - Do sizeof for a type what we are memset'ing - Zero out wm_uv as well(Ville Syrjälä) v5: - Fixed rebase conflict caused by COLOR_PLANE_* enum removal v6: - Do not use skl_plane_wm_level accessor in skl_allocate_pipe_ddb v7: - Get rid of wm_uv, which is not used in skl_plane_write_wm(Ville) Signed-off-by: -
Chris Wilson authored
Upon gt resume, we first poison then sanitize the engine. However, our testing shows that gen9 will very rarely retain the poisoned value from the HWSP mappings of the execlists status registers. This suggests that it is reading back from the HWSP, so rejig the register reset. v2: Maybe RING_CONTEXT_STATUS_PTR is write masked. It is. References: https://gitlab.freedesktop.org/drm/intel/-/issues/1812Signed-off-by:
-
- 12 May, 2020 2 commits
-
-
Swathi Dhanavanthri authored
This w/a is fixed in B0 stepping and needs to be restricted for A0 stepping only. Bspec: 33451 Signed-off-by:
Swathi Dhanavanthri <swathi.dhanavanthri@intel.com> Reviewed-by:
José Roberto de Souza <jose.souza@intel.com> Signed-off-by:
-
Chris Wilson authored
i915_gem_evict_something() is charged with finding a slot within the GTT that we may reuse. Since our goal is not to stall, we first look for a slot that only overlaps idle vma. To this end, on the first pass we move any active vma to the end of the search list. However, we only stopped moving active vma after we see the first active vma twice. If during the search, that first active vma completed, we would not notice and keep on extending the search list. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1746 Fixes: 2850748e ("drm/i915: Pull i915_vma_pin under the vm->mutex") Fixes: b1e3177b ("drm/i915: Coordinate i915_active with its own mutex") Signed-off-by:
-
- 11 May, 2020 13 commits
-
-
Chris Wilson authored
This reverts commit 0b718ba1. There are still some residual issues with asynchronous binding and execution, but since commit 92581f9f ("drm/i915: Immediately execute the fenced work") we prefer not to use asynchronous binds, and the remaining issues do not seem restricted to Cherryview [at least the ones seen over a few dozen CI runs, less frequent issues are sure to be discovered!] These issues seem to be mitigated, if not eliminated entirely, by the previous commit 84eac0c6 ("drm/i915/gt: Force pte cacheline to main memory"). Signed-off-by:
-
Mika Kuoppala authored
We have problems of tgl not seeing a valid pte entry when iommu is enabled. Add heavy handed flushing of entry modification by flushing the cpu, cacheline and then wcb. This forces the pte out to main memory past this point regarless of promises of coherency. This is an evolution of an experimental patch from Chris Wilson of adding wmb for coherent partners, by adding a clflush to force the cache->memory step. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1840 Testcase: igt/gem_exec_fence/parallel Signed-off-by:
-
Pascal Terjan authored
It has been unused since commit ccb2acea ("drm/i915: use vfuncs for reg_read/write_fw_domains"). Signed-off-by:
Pascal Terjan <pterjan@google.com> Reviewed-by:
-
Chris Wilson authored
Be consistent, and even when we know we had used a WC, flush the mapped object after writing into it. The flush understands the mapping type and will only clflush if !I915_MAP_WC, but will always insert a wmb [sfence] so that we can be sure that all writes are visible. v2: Add the unconditional wmb so we are know that we always flush the writes to memory/HW at that point. Signed-off-by:
-
Chris Wilson authored
Be consistent and ensure that we always emit the asynchronous waits prior to issuing instructions that use the address. This ensures that if we do emit GPU commands to do the await, they are before our use! Reviewed-by:
-
Ville Syrjälä authored
Get rid of several platform specific variants of intel_digital_port_connected() and just use the ISR bits we've stashed away. v2: Duplicate stuff to avoid exposing platform specific functions across files (Jani) Signed-off-by:Imre Deak <imre.deak@intel.com>
-
Ville Syrjälä authored
Instead of constnantly having to figure out which hpd status bit array to use let's store them under dev_priv. Should perhaps take this further and stash even more stuff to make the hpd handling more abstract yet. v2: Remeber cnp (Imre) Add MISSING_CASE() for unknown PCHs (Imre) Cc: Imre Deak <imre.deak@intel.com> Signed-off-by: -
Ville Syrjälä authored
Let's get rid of the platform if ladders in intel_digital_port_connected() and make it a vfunc. Now the if ladders are at the encoder initialization which makes them a bit less convoluted. v2: Add forward decl for intel_encoder in intel_tc.h v3: Duplicate stuff to avoid exposing platform specific functions across files (Jani) Signed-off-by: -
Ville Syrjälä authored
GLK wants the +1 adjustement for the "blocks per line" value for x-tile/y-tile, just like cnl+. Also the x-tile and linear cases are almost identical. The only difference is this +1 which is always done for glk+, and only done for linear on skl/bxt. Let's unify it to a single branch with a special case for the +1, just like we do for y-tile. Signed-off-by:
-
Ville Syrjälä authored
The correct sequence according to bspec is to wait for the ACT sent status before we turn on the pipe. Make it so. Signed-off-by:
-
Chris Wilson authored
Just tidy up the return handling for completed dma-fences. While it may return errors for invalid fence, we already know that we have a good fence and the only error will be an already signaled fence. Signed-off-by:
-
Mika Kuoppala authored
Commit fb5970da ("drm/i915/gt: Use the kernel_context to measure the breadcrumb size") removed the last external user for intel_timeline_init. Mark it static. Signed-off-by:
-
Chris Wilson authored
Since we are using bitops on context_tag to allow us to reserve and release inflight tags concurrently, the scan for the next bit is intentionally racy. [ 516.446854] BUG: KCSAN: data-race in execlists_schedule_in.isra.0 [i915] / execlists_schedule_out [i915] [ 516.446874] [ 516.446886] write (marked) to 0xffff8881f7644048 of 8 bytes by interrupt on cpu 2: [ 516.447076] execlists_schedule_out+0x538/0x6a0 [i915] [ 516.447263] process_csb+0x10b/0x3d0 [i915] [ 516.447449] execlists_submission_tasklet+0x30/0x170 [i915] [ 516.447468] tasklet_action_common.isra.0+0x42/0x90 [ 516.447484] __do_softirq+0xc8/0x206 [ 516.447498] irq_exit+0xcd/0xe0 [ 516.447516] do_IRQ+0x44/0xc0 [ 516.447535] ret_from_intr+0x0/0x1c [ 516.447550] cpuidle_enter_state+0x199/0x400 [ 516.447572] cpuidle_enter+0x50/0x90 [ 516.447587] do_idle+0x197/0x1e0 [ 516.447600] cpu_startup_entry+0x14/0x20 [ 516.447619] start_secondary+0xf9/0x130 [ 516.447643] secondary_startup_64+0xa4/0xb0 [ 516.447655] [ 516.447671] read to 0xffff8881f7644048 of 8 bytes by task 460 on cpu 1: [ 516.447863] execlists_schedule_in.isra.0+0x3cf/0x5a0 [i915] [ 516.448064] execlists_dequeue+0xf8f/0x1690 [i915] [ 516.448252] __execlists_submission_tasklet+0x48/0x60 [i915] [ 516.448440] execlists_submit_request+0x2e2/0x310 [i915] [ 516.448634] submit_notify+0x8f/0xc8 [i915] [ 516.448820] __i915_sw_fence_complete+0x61/0x420 [i915] [ 516.449005] i915_sw_fence_complete+0x58/0x80 [i915] [ 516.449208] i915_sw_fence_commit+0x16/0x20 [i915] [ 516.449399] __i915_request_queue+0x60/0x70 [i915] [ 516.449590] i915_gem_do_execbuffer+0x33f1/0x4a00 [i915] [ 516.449782] i915_gem_execbuffer2_ioctl+0x2a2/0x550 [i915] [ 516.449800] drm_ioctl_kernel+0xe9/0x130 [ 516.449814] drm_ioctl+0x27d/0x45e [ 516.449827] ksys_ioctl+0x89/0xb0 [ 516.449842] __x64_sys_ioctl+0x42/0x60 [ 516.449864] do_syscall_64+0x6e/0x2c0 [ 516.449878] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Signed-off-by:
-
- 09 May, 2020 2 commits
-
-
Gustavo A. R. Silva authored
The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99: struct foo { int stuff; struct boo array[]; }; By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being inadvertently introduced[3] to the codebase from now on. Also, notice that, dynamic memory allocations won't be affected by this change: "Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1] sizeof(flexible-array-member) triggers a warning because flexible array members have incomplete type[1]. There are some instances of code in which the sizeof operator is being incorrectly/erroneously applied to zero-length arrays and the result is zero. Such instances may be hiding some bugs. So, this work (flexible-array member conversions) will also help to get completely rid of those sorts of issues. This issue was found with the help of Coccinelle. [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html [2] https://github.com/KSPP/linux/issues/21 [3] commit 76497732 ("cxgb3/l2t: Fix undefined behaviour") Signed-off-by:Gustavo A. R. Silva <gustavoars@kernel.org> Reviewed-by:
-
Chris Wilson authored
Expose the hardcoded timeout for unsignaled foreign fences as a Kconfig option, primarily to allow brave systems to disable the timeout and solely rely on correct signaling. Signed-off-by:
Michael J. Ruhl <michael.j.ruhl@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200509105021.12542-1-chris@chris-wilson.co.uk
-
- 08 May, 2020 5 commits
-
-
Chris Wilson authored
The downside of using semaphores is that we lose metadata passing along the signaling chain. This is particularly nasty when we need to pass along a fatal error such as EFAULT or EDEADLK. For fatal errors we want to scrub the request before it is executed, which means that we cannot preload the request onto HW and have it wait upon a semaphore. Signed-off-by:
-
Lionel Landwerlin authored
To allow faster engine to engine synchronization, peel the layer of dma-fence-chain to expose potential i915 fences so that the i915_request code can emit HW semaphore wait/signal operations in the ring which is faster than waking up the host to submit unblocked workloads after interrupt notification. This is similar to the peeling we do for e.g. dma_fence_array. Signed-off-by:
Lionel Landwerlin <lionel.g.landwerlin@intel.com> Reviewed-by:
-
Chris Wilson authored
The kernel_context does not use initial-breadcrumbs, so when we ask if its requests have started we do so by comparing against the completion seqno of the previous request. This is very imprecise, not precise enough for the defer_request assertion. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1847Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200508104220.9872-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
As a means for a small code consolidation, but primarily to start thinking more carefully about internal-vs-external linkage, pull the pair of i915_sw_fence_await_dma_fence() calls into a common routine. Signed-off-by:
-
Chris Wilson authored
While we ordinarily do not skip submit-fences due to the accompanying hook that we want to callback on execution, a submit-fence on the same timeline is meaningless. Signed-off-by:
-
- 07 May, 2020 7 commits
-
-
Mika Kuoppala authored
All engines, exception being blitter as it does not care about the form, can access compressed surfaces. So we need to add forced aux table invalidates for those engines. v2: virtual instance masking (Chris) v3: bug on if not found (Chris) References: d248b371 ("drm/i915/gen12: Invalidate aux table entries forcibly") References bspec#43904, hsdes#1809175790 Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Chuansheng Liu <chuansheng.liu@intel.com> Cc: Rafael Antognolli <rafael.antognolli@intel.com> Signed-off-by:
-
Chris Wilson authored
Upon waiting a request (when asked), we gave that request a small priority boost, not enough for it to cause preemption, but enough for it to be scheduled next before all equals. We also used that bit to give new clients a small priority boost, similar to FQ_CODEL, such that we favoured short interactive tasks ahead of long running streams. However, this is causing lots of complications with timeslicing where we both want to honour the boost and yet ignore it. Those complications cause unexpected user behaviour (tasks not being timesliced and run concurrently as epxected), and the easiest way to resolve that is to remove the boost. Hopefully, we can find a compromise again if we need to, but in theory timeslicing itself and future more advanced schedulers should give us the interactivity boost we seek. Testcase: igt/gem_exec_schedule/lateslice Signed-off-by:
-
Chris Wilson authored
We recorded the dependencies for WAIT_FOR_SUBMIT in order that we could correctly perform priority inheritance from the parallel branches to the common trunk. However, for the purpose of timeslicing and reset handling, the dependency is weak -- as we the pair of requests are allowed to run in parallel and not in strict succession. The real significance though is that this allows us to rearrange groups of WAIT_FOR_SUBMIT linked requests along the single engine, and so can resolve user level inter-batch scheduling dependencies from user semaphores. Fixes: c81471f5 ("drm/i915: Copy across scheduler behaviour flags across submit fences") Testcase: igt/gem_exec_fence/submit Signed-off-by:
-
Mika Kuoppala authored
Aux table invalidation can fail on update. So next access may cause memory access to be into stale entry. Proposed workaround is to invalidate entries between all batchbuffers. v2: correct register address (Yang) v3: respect the order (Chris) References bspec#43904, hsdes#1809175790 Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Chuansheng Liu <chuansheng.liu@intel.com> Cc: Rafael Antognolli <rafael.antognolli@intel.com> Cc: Yang A Shi <yang.a.shi@intel.com> Signed-off-by:
-
Mika Kuoppala authored
Flush TDL,L3 and EUs Signed-off-by:
-
Mika Kuoppala authored
HDC pipeline flush is bit on the first dword of the PIPE_CONTROL, not the second. Make it so. v2: function naming (Chris) Signed-off-by:
-
Mika Kuoppala authored
This reverts commit 62037fff. L3 ro cache invalidation is part of the dword0 of pipe control. Also it is not relevant to this gen. Signed-off-by:
-
- 06 May, 2020 1 commit
-
-
Chris Wilson authored
We need to preserve fatal errors from fences that are being terminated as we hook them up. Fixes: ef468849 ("drm/i915: Propagate fence errors") Signed-off-by:
Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200506162136.3325-1-chris@chris-wilson.co.uk
-
- 05 May, 2020 3 commits
-
-
Matt Roper authored
We need to toggle a SDE chicken bit on and then off as the final step when disabling interrupts in preparation for runtime suspend. Bspec: 33450 Bspec: 8402 Cc: Bob Paauwe <bob.j.paauwe@intel.com> Signed-off-by:
Matt Roper <matthew.d.roper@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200501213701.371443-1-matthew.d.roper@intel.comReviewed-by:
Bob Paauwe <bob.j.paauwe@intel.com>
-
Chris Wilson authored
As we only restore the default context state upon banning a context, we only need enough of the state to run the ring and nothing more. That is we only need our bare protocontext. Signed-off-by:
Andi Shyti <andi.shyti@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200504180745.15645-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
If we cannot trust the reset will flush out the CS event queue such that process_csb() reports an accurate view of HW, we will need to search the active and pending contexts to determine which was actually running at the time we issued the reset. Signed-off-by:
-
