- 04 Oct, 2019 27 commits
-
-
Chris Wilson authored
We no longer need to placate lockdep by holding struct_mutex for our initialisation, so don't. Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191004134015.13204-21-chris@chris-wilson.co.uk
-
Chris Wilson authored
We no longer need struct_mutex to serialise request emission, so remove it from the gt selftests. Signed-off-by:
-
Chris Wilson authored
struct_mutex provides no serialisation of the registers and data structures being saved and restored across suspend/resume. It is completely superfluous here. Signed-off-by:
-
Chris Wilson authored
Having a struct_mutex around the read of a BIOS blob serves no purpose. Signed-off-by:
-
Chris Wilson authored
It protects nothing being accessed for the intel_framebuffer, so it's own locking had better be sufficient. Signed-off-by:
-
Chris Wilson authored
The overlay uses the modeset mutex to control itself and only required the struct_mutex for requests, which is now obsolete. Signed-off-by:
-
Chris Wilson authored
Keep track of the GEM contexts underneath i915->gem.contexts and assign them their own lock for the purposes of list management. v2: Focus on lock tracking; ctx->vm is protected by ctx->mutex v3: Correct split with removal of logical HW ID Signed-off-by:
-
Chris Wilson authored
With the introduction of ctx->engines[] we allow multiple logical contexts to be used on the same engine (e.g. with virtual engines). According to bspec, aach logical context requires a unique tag in order for context-switching to occur correctly between them. [Simple experiments show that it is not so easy to trick the HW into performing a lite-restore with matching logical IDs, though my memory from early Broadwell experiments do suggest that it should be generating lite-restores.] We only need to keep a unique tag for the active lifetime of the context, and for as long as we need to identify that context. The HW uses the tag to determine if it should use a lite-restore (why not the LRCA?) and passes the tag back for various status identifies. The only status we need to track is for OA, so when using perf, we assign the specific context a unique tag. v2: Calculate required number of tags to fill ELSP. Fixes: 976b55f0 ("drm/i915: Allow a context to define its set of engines") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111895Signed-off-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Reviewed-by:
-
Chris Wilson authored
As our global unpark/park keep track of the number of active users, we can simply move the accounting from the GEM layer to the base GT layer. It was placed originally inside GEM to benefit from the 100ms extra delay on idleness, but that has been eliminated and now there is no substantive difference between the layers. In moving it, we move another piece of the puzzle out from underneath struct_mutex. Signed-off-by:
-
Chris Wilson authored
Requests are run from the gt and are tided into the gt runtime power management, so pull the runtime request management under gt/ Signed-off-by:
-
Chris Wilson authored
Now that we can retire without taking struct_mutex, we can do so to handle shrinking the mmap-offset space after an allocation failure. Signed-off-by:
-
Chris Wilson authored
wait_for_timelines is essentially the same loop as retiring requests (with an extra timeout), so merge the two into one routine. v2: i915_retire_requests_timeout and keep VT'd w/a as !interruptible Signed-off-by:
-
Chris Wilson authored
Nothing inside the idle worker now requires struct_mutex, so we can remove the indirection of using our own worker. Signed-off-by:
-
Chris Wilson authored
We don't need to hold struct_mutex now for retiring requests, so drop it from i915_retire_requests() and i915_gem_wait_for_idle(), finally removing I915_WAIT_LOCKED for good. Signed-off-by:
-
Chris Wilson authored
Now that we now longer need to guarantee that the active callback is under the struct_mutex, we can lift it out of the i915_gem_park() and into the engine parking itself. Signed-off-by:
-
Chris Wilson authored
Forgo the struct_mutex serialisation for i915_active, and interpose its own mutex handling for active/retire. This is a multi-layered sleight-of-hand. First, we had to ensure that no active/retire callbacks accidentally inverted the mutex ordering rules, nor assumed that they were themselves serialised by struct_mutex. More challenging though, is the rule over updating elements of the active rbtree. Instead of the whole i915_active now being serialised by struct_mutex, allocations/rotations of the tree are serialised by the i915_active.mutex and individual nodes are serialised by the caller using the i915_timeline.mutex (we need to use nested spinlocks to interact with the dma_fence callback lists). The pain point here is that instead of a single mutex around execbuf, we now have to take a mutex for active tracker (one for each vma, context, etc) and a couple of spinlocks for each fence update. The improvement in fine grained locking allowing for multiple concurrent clients (eventually!) should be worth it in typical loads. v2: Add some comments that barely elucidate anything :( Signed-off-by:
-
Chris Wilson authored
As we need to use a mutex to serialise i915_active activation (because we want to allow the callback to sleep), we need to push the i915_active.retire into a worker callback in case we get need to retire from an atomic context. Signed-off-by:
Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191004134015.13204-5-chris@chris-wilson.co.uk
-
Chris Wilson authored
Replace the struct_mutex requirement for pinning the i915_vma with the local vm->mutex instead. Note that the vm->mutex is tainted by the shrinker (we require unbinding from inside fs-reclaim) and so we cannot allocate while holding that mutex. Instead we have to preallocate workers to do allocate and apply the PTE updates after we have we reserved their slot in the drm_mm (using fences to order the PTE writes with the GPU work and with later unbind). In adding the asynchronous vma binding, one subtle requirement is to avoid coupling the binding fence into the backing object->resv. That is the asynchronous binding only applies to the vma timeline itself and not to the pages as that is a more global timeline (the binding of one vma does not need to be ordered with another vma, nor does the implicit GEM fencing depend on a vma, only on writes to the backing store). Keeping the vma binding distinct from the backing store timelines is verified by a number of async gem_exec_fence and gem_exec_schedule tests. The way we do this is quite simple, we keep the fence for the vma binding separate and only wait on it as required, and never add it to the obj->resv itself. Another consequence in reducing the locking around the vma is the destruction of the vma is no longer globally serialised by struct_mutex. A natural solution would be to add a kref to i915_vma, but that requires decoupling the reference cycles, possibly by introducing a new i915_mm_pages object that is own by both obj->mm and vma->pages. However, we have not taken that route due to the overshadowing lmem/ttm discussions, and instead play a series of complicated games with trylocks to (hopefully) ensure that only one destruction path is called! v2: Add some commentary, and some helpers to reduce patch churn. Signed-off-by:
-
Chris Wilson authored
Since we cannot allocate underneath the vm->mutex (it is used in the direct-reclaim paths), we need to shift the allocations off into a mutexless worker with fence recursion prevention. To know when we need this protection, we mark up the address spaces that do allocate before insertion. In the future, we may wish to extend the async bind scheme to more than just allocations. v2: s/vm->bind_alloc/vm->bind_async_flags/ Signed-off-by:
-
Chris Wilson authored
The premise here is to simply avoiding having to acquire the vm->mutex inside vma create/destroy to update the vm->unbound_lists, to avoid some nasty lock recursions later. Signed-off-by:
-
Chris Wilson authored
A subset of 71724f70 ("drm/mm: Use helpers for drm_mm_node booleans") in order to prepare drm-intel-next-queued for subsequent patches before we can backmerge 71724f70 itself. Signed-off-by:
-
Chris Wilson authored
The L3 cache remapping is stored as u32 elements, and we should ensure that the user only supplies complete slice information(u32). Signed-off-by:
-
Kai Vehmanen authored
The CDCLK>=2*BCLK constraint applies to all generations since gen10. Extend the constraint logic in audio get/put_power(). Signed-off-by:
Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by:
Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191003085531.30990-2-kai.vehmanen@linux.intel.com
-
Kai Vehmanen authored
On platfroms with gen10+ display, driver must set the enable bit of AUDIO_PIN_BUF_CTL register before transactions with the HDA controller can proceed. Add setting this bit to the audio power up sequence. Failing to do this resulted in errors during display audio codec probe, and failures during resume from suspend. Note: We may also need to disable the bit afterwards, but there are still unresolved issues with that. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111214Signed-off-by:
-
Jani Nikula authored
Add aux_busy_last_status to intel_dp. Don't bother with initializing to all ones; the only difference is potentially missing logging for one error case if the readout is all zeros. Reviewed-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by:
-
Chris Wilson authored
If we unwind the active requests, and on resubmission discover that we intend to preempt the active contexts with themselves, simply skip the ELSP submission. Signed-off-by:
-
Imre Deak authored
The Thunderbolt PLL divider values on TGL differ from the ICL ones, update the PLL parameter calculation function accordingly. Bspec: 49204 v2: - Remove unused refclk config. (José) Cc: Jose Souza <jose.souza@intel.com> Cc: Clinton A Taylor <clinton.a.taylor@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Mika Westerberg <mika.westerberg@intel.com> Tested-by:
Mika Westerberg <mika.westerberg@intel.com> Signed-off-by:
Imre Deak <imre.deak@intel.com> Reviewed-by:
Jose Souza <jose.souza@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191002204108.32242-1-imre.deak@intel.com
-
- 03 Oct, 2019 2 commits
-
-
Jani Nikula authored
Unify on current common usage to allow repurposing drm_dbg() later. Fix newlines while at it. Reviewed-by:
-
Jani Nikula authored
Unify on current common usage to allow repurposing drm_err() later. Fix newlines while at it. Reviewed-by:
-
- 02 Oct, 2019 11 commits
-
-
Chris Wilson authored
If execlists's lite-restore is based on the common GEM context tag rather than the per-intel_context LRCA, then a context switch between two intel_contexts on the same engine derived from the same GEM context will perform a lite-restore instead of a full context switch. We can exploit this by poisoning the ringbuffer of the first context and trying to trick a simple RING_TAIL update (i.e. lite-restore) v2: Also check what happens if preempt ce[0] with ce[1] (both instances on the same engine from the same parent context) [Tvrtko] Signed-off-by:
-
José Roberto de Souza authored
All the MG registers is based on the tc_port not port, so MG_PHY_PORT_LN() was subtracting port and PORT_C what is very fragile. So replacing port to tc_port in all MG register macros and users like we have for DKL. Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Imre Deak <imre.deak@intel.com> Reviewed-by:
Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by:
-
Ville Syrjälä authored
Use BIT(pipe) for better legibility when populating the crtc_mask for encoders. Also remove the redundant possible_crtcs setup for the TV encoder. Reviewed-by:
-
Ville Syrjälä authored
Don't advertize non-exisiting crtcs in the encoder possible_crtcs bitmask. Reviewed-by:
Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Signed-off-by:
-
Chris Wilson authored
We repeat obj->ops->flags in our object checks, so pull that into its own little helper for clarity. Signed-off-by:
-
Chris Wilson authored
For selftests, we desire repeatability and so prefer using a prng with known seed over true randomness. Extract random_offset() as a selftest utility that can take the prng state. Suggested-by:
-
Ville Syrjälä authored
I forgot to update the g4x sprite scaling stride check when GTT remapping was introduced. The stride of the original framebuffer is irrelevant when remapping is used and instead we want to check the stride of the remapped view. Also drop the duplicate width_bytes check. We already check that a few lines earlier. Fixes: df79cf44 ("drm/i915: Store the final plane stride in plane_state") Signed-off-by:
-
Ville Syrjälä authored
Drop the tv_mode NULL check since intel_tv_mode_find() never actually returns NULL, and flip the condition around so that the MODE_OK case is at the end, which is customary to all the other .mode_valid() implementations. Signed-off-by:
-
Ville Syrjälä authored
When adding the max plane size checks to the .mode_valid() hooks I naturally forgot about MST. Take care of that one as well. Cc: Manasi Navare <manasi.d.navare@intel.com> Cc: Sean Paul <sean@poorly.run> Cc: José Roberto de Souza <jose.souza@intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Fixes: 2d20411e ("drm/i915: Don't advertise modes that exceed the max plane size") Signed-off-by:
-
Jani Nikula authored
Split out the code related to vga client and vgaarb all over the place into new intel_vga.[ch]. No functional changes. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
-
Chris Wilson authored
Daniel Vetter uncovered a nasty cycle in using the mmu-notifiers to invalidate userptr objects which also happen to be pulled into GGTT mmaps. That is when we unbind the userptr object (on mmu invalidation), we revoke all CPU mmaps, which may then recurse into mmu invalidation. We looked for ways of breaking the cycle, but the revocation on invalidation is required and cannot be avoided. The only solution we could see was to not allow such GGTT bindings of userptr objects in the first place. In practice, no one really wants to use a GGTT mmapping of a CPU pointer... Just before Daniel's explosive lockdep patches land in v5.4-rc1, we got a genuine blip from CI: <4>[ 246.793958] ====================================================== <4>[ 246.793972] WARNING: possible circular locking dependency detected <4>[ 246.793989] 5.3.0-gbd6c56f50d15-drmtip_372+ #1 Tainted: G U <4>[ 246.794003] ------------------------------------------------------ <4>[ 246.794017] kswapd0/145 is trying to acquire lock: <4>[ 246.794030] 000000003f565be6 (&dev->struct_mutex/1){+.+.}, at: userptr_mn_invalidate_range_start+0x18f/0x220 [i915] <4>[ 246.794250] but task is already holding lock: <4>[ 246.794263] 000000001799cef9 (&anon_vma->rwsem){++++}, at: page_lock_anon_vma_read+0xe6/0x2a0 <4>[ 246.794291] which lock already depends on the new lock. <4>[ 246.794307] the existing dependency chain (in reverse order) is: <4>[ 246.794322] -> #3 (&anon_vma->rwsem){++++}: <4>[ 246.794344] down_write+0x33/0x70 <4>[ 246.794357] __vma_adjust+0x3d9/0x7b0 <4>[ 246.794370] __split_vma+0x16a/0x180 <4>[ 246.794385] mprotect_fixup+0x2a5/0x320 <4>[ 246.794399] do_mprotect_pkey+0x208/0x2e0 <4>[ 246.794413] __x64_sys_mprotect+0x16/0x20 <4>[ 246.794429] do_syscall_64+0x55/0x1c0 <4>[ 246.794443] entry_SYSCALL_64_after_hwframe+0x49/0xbe <4>[ 246.794456] -> #2 (&mapping->i_mmap_rwsem){++++}: <4>[ 246.794478] down_write+0x33/0x70 <4>[ 246.794493] unmap_mapping_pages+0x48/0x130 <4>[ 246.794519] i915_vma_revoke_mmap+0x81/0x1b0 [i915] <4>[ 246.794519] i915_vma_unbind+0x11d/0x4a0 [i915] <4>[ 246.794519] i915_vma_destroy+0x31/0x300 [i915] <4>[ 246.794519] __i915_gem_free_objects+0xb8/0x4b0 [i915] <4>[ 246.794519] drm_file_free.part.0+0x1e6/0x290 <4>[ 246.794519] drm_release+0xa6/0xe0 <4>[ 246.794519] __fput+0xc2/0x250 <4>[ 246.794519] task_work_run+0x82/0xb0 <4>[ 246.794519] do_exit+0x35b/0xdb0 <4>[ 246.794519] do_group_exit+0x34/0xb0 <4>[ 246.794519] __x64_sys_exit_group+0xf/0x10 <4>[ 246.794519] do_syscall_64+0x55/0x1c0 <4>[ 246.794519] entry_SYSCALL_64_after_hwframe+0x49/0xbe <4>[ 246.794519] -> #1 (&vm->mutex){+.+.}: <4>[ 246.794519] i915_gem_shrinker_taints_mutex+0x6d/0xe0 [i915] <4>[ 246.794519] i915_address_space_init+0x9f/0x160 [i915] <4>[ 246.794519] i915_ggtt_init_hw+0x55/0x170 [i915] <4>[ 246.794519] i915_driver_probe+0xc9f/0x1620 [i915] <4>[ 246.794519] i915_pci_probe+0x43/0x1b0 [i915] <4>[ 246.794519] pci_device_probe+0x9e/0x120 <4>[ 246.794519] really_probe+0xea/0x3d0 <4>[ 246.794519] driver_probe_device+0x10b/0x120 <4>[ 246.794519] device_driver_attach+0x4a/0x50 <4>[ 246.794519] __driver_attach+0x97/0x130 <4>[ 246.794519] bus_for_each_dev+0x74/0xc0 <4>[ 246.794519] bus_add_driver+0x13f/0x210 <4>[ 246.794519] driver_register+0x56/0xe0 <4>[ 246.794519] do_one_initcall+0x58/0x300 <4>[ 246.794519] do_init_module+0x56/0x1f6 <4>[ 246.794519] load_module+0x25bd/0x2a40 <4>[ 246.794519] __se_sys_finit_module+0xd3/0xf0 <4>[ 246.794519] do_syscall_64+0x55/0x1c0 <4>[ 246.794519] entry_SYSCALL_64_after_hwframe+0x49/0xbe <4>[ 246.794519] -> #0 (&dev->struct_mutex/1){+.+.}: <4>[ 246.794519] __lock_acquire+0x15d8/0x1e90 <4>[ 246.794519] lock_acquire+0xa6/0x1c0 <4>[ 246.794519] __mutex_lock+0x9d/0x9b0 <4>[ 246.794519] userptr_mn_invalidate_range_start+0x18f/0x220 [i915] <4>[ 246.794519] __mmu_notifier_invalidate_range_start+0x85/0x110 <4>[ 246.794519] try_to_unmap_one+0x76b/0x860 <4>[ 246.794519] rmap_walk_anon+0x104/0x280 <4>[ 246.794519] try_to_unmap+0xc0/0xf0 <4>[ 246.794519] shrink_page_list+0x561/0xc10 <4>[ 246.794519] shrink_inactive_list+0x220/0x440 <4>[ 246.794519] shrink_node_memcg+0x36e/0x740 <4>[ 246.794519] shrink_node+0xcb/0x490 <4>[ 246.794519] balance_pgdat+0x241/0x580 <4>[ 246.794519] kswapd+0x16c/0x530 <4>[ 246.794519] kthread+0x119/0x130 <4>[ 246.794519] ret_from_fork+0x24/0x50 <4>[ 246.794519] other info that might help us debug this: <4>[ 246.794519] Chain exists of: &dev->struct_mutex/1 --> &mapping->i_mmap_rwsem --> &anon_vma->rwsem <4>[ 246.794519] Possible unsafe locking scenario: <4>[ 246.794519] CPU0 CPU1 <4>[ 246.794519] ---- ---- <4>[ 246.794519] lock(&anon_vma->rwsem); <4>[ 246.794519] lock(&mapping->i_mmap_rwsem); <4>[ 246.794519] lock(&anon_vma->rwsem); <4>[ 246.794519] lock(&dev->struct_mutex/1); <4>[ 246.794519] *** DEADLOCK *** v2: Say no to mmap_ioctl Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111744 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111870Signed-off-by:
-
