summaryrefslogtreecommitdiffstats
path: root/drivers/gpu/drm/i915/gem
Commit message (Collapse)AuthorAgeFilesLines
...
| * | | drm/i915: Avoid calling i915_gem_object_unbind holding object lockChris Wilson2019-12-071-30/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the extreme case, we may wish to wait on an rcu-barrier to reap stale vm to purge the last of the object bindings. However, we are not allowed to use rcu_barrier() beneath the dma_resv (i.e. object) lock and do not take lightly the prospect of unlocking a mutex deep in the bowels of the routine. i915_gem_object_unbind() itself does not need the object lock, and it turns out the callers do not need to the unbind as part of a locked sequence around set-cache-level, so rearrange the code to avoid taking the object lock in the callers. <4> [186.816311] ====================================================== <4> [186.816313] WARNING: possible circular locking dependency detected <4> [186.816316] 5.4.0-rc8-CI-CI_DRM_7486+ #1 Tainted: G U <4> [186.816318] ------------------------------------------------------ <4> [186.816320] perf_pmu/1321 is trying to acquire lock: <4> [186.816322] ffff88849487c4d8 (&mm->mmap_sem#2){++++}, at: __might_fault+0x39/0x90 <4> [186.816331] but task is already holding lock: <4> [186.816333] ffffe8ffffa05008 (&cpuctx_mutex){+.+.}, at: perf_event_ctx_lock_nested+0xa9/0x1b0 <4> [186.816339] which lock already depends on the new lock. <4> [186.816341] the existing dependency chain (in reverse order) is: <4> [186.816343] -> #6 (&cpuctx_mutex){+.+.}: <4> [186.816349] __mutex_lock+0x9a/0x9d0 <4> [186.816352] perf_event_init_cpu+0xa4/0x140 <4> [186.816357] perf_event_init+0x19d/0x1cd <4> [186.816362] start_kernel+0x372/0x4f4 <4> [186.816365] secondary_startup_64+0xa4/0xb0 <4> [186.816381] -> #5 (pmus_lock){+.+.}: <4> [186.816385] __mutex_lock+0x9a/0x9d0 <4> [186.816387] perf_event_init_cpu+0x6b/0x140 <4> [186.816404] cpuhp_invoke_callback+0x9b/0x9d0 <4> [186.816406] _cpu_up+0xa2/0x140 <4> [186.816409] do_cpu_up+0x61/0xa0 <4> [186.816411] smp_init+0x57/0x96 <4> [186.816413] kernel_init_freeable+0xac/0x1c7 <4> [186.816416] kernel_init+0x5/0x100 <4> [186.816419] ret_from_fork+0x24/0x50 <4> [186.816421] -> #4 (cpu_hotplug_lock.rw_sem){++++}: <4> [186.816424] cpus_read_lock+0x34/0xd0 <4> [186.816427] rcu_barrier+0xaa/0x190 <4> [186.816429] kernel_init+0x21/0x100 <4> [186.816431] ret_from_fork+0x24/0x50 <4> [186.816433] -> #3 (rcu_state.barrier_mutex){+.+.}: <4> [186.816436] __mutex_lock+0x9a/0x9d0 <4> [186.816438] rcu_barrier+0x23/0x190 <4> [186.816502] i915_gem_object_unbind+0x3a6/0x400 [i915] <4> [186.816537] i915_gem_object_set_cache_level+0x32/0x90 [i915] <4> [186.816571] i915_gem_object_pin_to_display_plane+0x5d/0x160 [i915] <4> [186.816612] intel_pin_and_fence_fb_obj+0x9e/0x200 [i915] <4> [186.816679] intel_plane_pin_fb+0x3f/0xd0 [i915] <4> [186.816717] intel_prepare_plane_fb+0x130/0x520 [i915] <4> [186.816722] drm_atomic_helper_prepare_planes+0x85/0x110 <4> [186.816761] intel_atomic_commit+0xc6/0x350 [i915] <4> [186.816764] drm_atomic_helper_update_plane+0xed/0x110 <4> [186.816768] setplane_internal+0x97/0x190 <4> [186.816770] drm_mode_setplane+0xcd/0x190 <4> [186.816773] drm_ioctl_kernel+0xa7/0xf0 <4> [186.816775] drm_ioctl+0x2e1/0x390 <4> [186.816778] do_vfs_ioctl+0xa0/0x6f0 <4> [186.816780] ksys_ioctl+0x35/0x60 <4> [186.816782] __x64_sys_ioctl+0x11/0x20 <4> [186.816785] do_syscall_64+0x4f/0x210 <4> [186.816787] entry_SYSCALL_64_after_hwframe+0x49/0xbe <4> [186.816789] -> #2 (reservation_ww_class_mutex){+.+.}: <4> [186.816793] __ww_mutex_lock.constprop.15+0xc3/0x1090 <4> [186.816795] ww_mutex_lock+0x39/0x70 <4> [186.816798] dma_resv_lockdep+0x10e/0x1f7 <4> [186.816800] do_one_initcall+0x58/0x2ff <4> [186.816802] kernel_init_freeable+0x137/0x1c7 <4> [186.816804] kernel_init+0x5/0x100 <4> [186.816806] ret_from_fork+0x24/0x50 <4> [186.816808] -> #1 (reservation_ww_class_acquire){+.+.}: <4> [186.816811] dma_resv_lockdep+0xec/0x1f7 <4> [186.816813] do_one_initcall+0x58/0x2ff <4> [186.816815] kernel_init_freeable+0x137/0x1c7 <4> [186.816817] kernel_init+0x5/0x100 <4> [186.816819] ret_from_fork+0x24/0x50 <4> [186.816820] -> #0 (&mm->mmap_sem#2){++++}: <4> [186.816824] __lock_acquire+0x1328/0x15d0 <4> [186.816826] lock_acquire+0xa7/0x1c0 <4> [186.816828] __might_fault+0x63/0x90 <4> [186.816831] _copy_to_user+0x1e/0x80 <4> [186.816834] perf_read+0x200/0x2b0 <4> [186.816836] vfs_read+0x96/0x160 <4> [186.816838] ksys_read+0x9f/0xe0 <4> [186.816839] do_syscall_64+0x4f/0x210 <4> [186.816841] entry_SYSCALL_64_after_hwframe+0x49/0xbe <4> [186.816843] other info that might help us debug this: <4> [186.816846] Chain exists of: &mm->mmap_sem#2 --> pmus_lock --> &cpuctx_mutex <4> [186.816849] Possible unsafe locking scenario: <4> [186.816851] CPU0 CPU1 <4> [186.816853] ---- ---- <4> [186.816854] lock(&cpuctx_mutex); <4> [186.816856] lock(pmus_lock); <4> [186.816858] lock(&cpuctx_mutex); <4> [186.816860] lock(&mm->mmap_sem#2); <4> [186.816861] *** DEADLOCK *** Closes: https://gitlab.freedesktop.org/drm/intel/issues/728 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Andi Shyti <andi.shyti@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191206105527.1130413-5-chris@chris-wilson.co.uk
| * | | drm/i915/gem: Pin gen6_ppgtt prior to constructing the requestChris Wilson2019-12-061-6/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | All pinning must be done prior to i915_request_create, to avoid timeline->mutex inversions. Here we slightly abuse the context_barrier_task stages to utilise the 'skip' decision as an opportunity to acquire the pin on the new ppgtt. Consider it s/skip/prepare/. At the moment, we only have on user of context_barrier_task, so it might be worth breaking it down for the specific task of set-vm and refactor it later if we find a second purpose. <4> [402.377487] WARNING: possible circular locking dependency detected <4> [402.377493] 5.4.0-rc8-CI-CI_DRM_7491+ #1 Tainted: G U <4> [402.377497] ------------------------------------------------------ <4> [402.377502] gem_exec_parall/2506 is trying to acquire lock: <4> [402.377507] ffff888403cdac70 (&kernel#2){+.+.}, at: i915_request_create+0x16/0x1c0 [i915] <4> [402.377593] but task is already holding lock: <4> [402.377597] ffff88835efad550 (&ppgtt->pin_mutex){+.+.}, at: gen6_ppgtt_pin+0x4d/0x110 [i915] <4> [402.377660] which lock already depends on the new lock. <4> [402.377664] the existing dependency chain (in reverse order) is: <4> [402.377668] -> #1 (&ppgtt->pin_mutex){+.+.}: <4> [402.377674] __mutex_lock+0x9a/0x9d0 <4> [402.377713] gen6_ppgtt_pin+0x4d/0x110 [i915] <4> [402.377756] emit_ppgtt_update+0x1dc/0x370 [i915] <4> [402.377801] context_barrier_task+0x176/0x310 [i915] <4> [402.377844] ctx_setparam+0x400/0xb10 [i915] <4> [402.377886] i915_gem_context_setparam_ioctl+0xc8/0x160 [i915] <4> [402.377891] drm_ioctl_kernel+0xa7/0xf0 <4> [402.377895] drm_ioctl+0x2e1/0x390 <4> [402.377899] do_vfs_ioctl+0xa0/0x6f0 <4> [402.377903] ksys_ioctl+0x35/0x60 <4> [402.377906] __x64_sys_ioctl+0x11/0x20 <4> [402.377910] do_syscall_64+0x4f/0x210 <4> [402.377914] entry_SYSCALL_64_after_hwframe+0x49/0xbe <4> [402.377917] -> #0 (&kernel#2){+.+.}: <4> [402.377923] __lock_acquire+0x1328/0x15d0 <4> [402.377926] lock_acquire+0xa7/0x1c0 <4> [402.377930] __mutex_lock+0x9a/0x9d0 <4> [402.377977] i915_request_create+0x16/0x1c0 [i915] <4> [402.378013] intel_engine_flush_barriers+0x4c/0x100 [i915] <4> [402.378062] i915_ggtt_pin+0x7d/0x130 [i915] <4> [402.378108] gen6_ppgtt_pin+0x9c/0x110 [i915] <4> [402.378148] ring_context_pin+0x2e/0xc0 [i915] <4> [402.378183] __intel_context_do_pin+0x6b/0x190 [i915] <4> [402.378226] i915_gem_do_execbuffer+0x180c/0x26b0 [i915] <4> [402.378268] i915_gem_execbuffer2_ioctl+0x11b/0x460 [i915] <4> [402.378272] drm_ioctl_kernel+0xa7/0xf0 <4> [402.378275] drm_ioctl+0x2e1/0x390 <4> [402.378279] do_vfs_ioctl+0xa0/0x6f0 <4> [402.378282] ksys_ioctl+0x35/0x60 <4> [402.378286] __x64_sys_ioctl+0x11/0x20 <4> [402.378289] do_syscall_64+0x4f/0x210 <4> [402.378292] entry_SYSCALL_64_after_hwframe+0x49/0xbe <4> [402.378295] other info that might help us debug this: <4> [402.378299] Possible unsafe locking scenario: <4> [402.378302] CPU0 CPU1 <4> [402.378305] ---- ---- <4> [402.378307] lock(&ppgtt->pin_mutex); <4> [402.378310] lock(&kernel#2); <4> [402.378314] lock(&ppgtt->pin_mutex); <4> [402.378317] lock(&kernel#2); <4> [402.378320] Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Andi Shyti <andi.shyti@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191206105527.1130413-4-chris@chris-wilson.co.uk
| * | | drm/i915: Remove vestigal i915_gem_context locals from cmdparserChris Wilson2019-12-051-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The use GEM context itself was removed in commit cd30a5031704 ("drm/i915/gem: Excise the per-batch whitelist from the context"), but the locals were left in place as an oversight. Remove the parameters and clean up. References: cd30a5031704 ("drm/i915/gem: Excise the per-batch whitelist from the context") 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/20191204232616.94397-1-chris@chris-wilson.co.uk
| * | | drm/i915/gem: Hook user-extensions upto MMAP_OFFSET_IOCTLChris Wilson2019-12-041-2/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Call i915_user_extensions() to validate the arg->extensions pointer, and so return consistent error numbers for the future. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191204162803.3841140-1-chris@chris-wilson.co.uk
| * | | drm/i915/gem: Hold the obj->vma.lock while walking the vma.listChris Wilson2019-12-041-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remember to take the lock before walking the obj->vma.list so that the nodes do not change beneath us! E.g., i915_gem_object_bump_inactive_ggtt:387 GEM_BUG_ON(vma->vm != &i915->ggtt.vm) Closes: https://gitlab.freedesktop.org/drm/intel/issues/691 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191204164527.3872783-1-chris@chris-wilson.co.uk
| * | | drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSETAbdiel Janulgue2019-12-0410-123/+523
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is really just an alias of mmap_gtt. The 'mmap offset' nomenclature comes from the value returned by this ioctl which is the offset into the device fd which userpace uses with mmap(2). mmap_gtt was our initial mmap_offset implementation, this extends our CPU mmap support to allow additional fault handlers that depends on the object's backing pages. Note that we multiplex mmap_gtt and mmap_offset through the same ioctl, and use the zero extending behaviour of drm to differentiate between them, when we inspect the flags. To support multiple mmap types on an object we need to support multiple mmap_offsets for an object (each offset in the global device address space corresponding to a unique instance of the object for a file + mmap type). As we drop the simplified drm core idea of a single mmap_offset, we need to provide replacement hooks for the dumb mmap interface as well. Link: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1675 Testcase: igt/gem_mmap_offset Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20191204120032.3682839-1-chris@chris-wilson.co.uk
| * | | drm/i915/gem: Unbind all current vma on changing cache-levelChris Wilson2019-12-021-119/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Avoid dangerous race handling of destroyed vma by unbinding all vma instead. Unfortunately, this stops us from trying to be clever and only doing the minimal change required, so on first use of scanout we may encounter an annoying stall as it transitions to a new cache level. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112413 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191202174310.2630302-1-chris@chris-wilson.co.uk
| * | | drm/i915/selftests: Keep engine awake during live_coherencyChris Wilson2019-11-301-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Keep the engine awake and so avoid frequent cycling in and out of powersaving mode to eliminate the unnecessary overhead and speed up the testing. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191129222702.1456292-1-chris@chris-wilson.co.uk
| * | | drm/i915/gem: Take timeline->mutex to walk list-of-requestsChris Wilson2019-11-291-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Though the context is closed and so no more requests can be added to the timeline, retirement can still be removing requests. It can even be removing the very request we are inspecting and so cause us to wander into dead links. Serialise with the retirement by taking the timeline->mutex used for guarding the timeline->requests list. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112404 Fixes: 4a3174152147 ("drm/i915/gem: Refine occupancy test in kill_context()") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191129151845.1092933-1-chris@chris-wilson.co.uk
| * | | Revert "drm/i915: use a separate context for gpu relocs"Chris Wilson2019-11-291-29/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since commit c45e788d95b4 ("drm/i915/tgl: Suspend pre-parser across GTT invalidations"), we now disable the advanced preparser on Tigerlake for the invalidation phase at the start of the batch, we no longer need to emit the GPU relocations from a second context as they are now flushed inlined. References: 8a9a982767b7 ("drm/i915: use a separate context for gpu relocs") References: c45e788d95b4 ("drm/i915/tgl: Suspend pre-parser across GTT invalidations") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191129124846.949100-1-chris@chris-wilson.co.uk
| * | | drm/i915/selftests: Use sgt_iter for huge_pages_freeChris Wilson2019-11-291-4/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use the normal sgt_iter to walk the pages scatterlist on free so that we handle the error path correctly. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112225 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191128232946.546831-1-chris@chris-wilson.co.uk
| * | | drm/i915/selftests: Drop local vm reference!Chris Wilson2019-11-281-19/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After obtaining a local reference to the vm from the context, remember to drop it before it goes out of scope! Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191128185402.110678-1-chris@chris-wilson.co.uk
| * | | drm/i915/selftests: Count the number of engines usedChris Wilson2019-11-281-9/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Don't rely on the RUNTIME_INFO() when we loop over a particular context and only run on a filtered set of engines. 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/20191127223252.3777141-1-chris@chris-wilson.co.uk
| * | | drm/i915/gem: Excise the per-batch whitelist from the contextChris Wilson2019-11-282-12/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | One does not lightly add a new hidden struct_mutex dependency deep within the execbuf bowels! The immediate suspicion in seeing the whitelist cached on the context, is that it is intended to be preserved between batches, as the kernel is quite adept at caching small allocations itself. But no, it's sole purpose is to serialise command submission in order to save a kmalloc on a slow, slow path! By removing the whitelist dependency from the context, our freedom to chop the big struct_mutex is greatly augmented. v2: s/set_bit/__set_bit/ as the whitelist shall never be accessed concurrently. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191128113424.3885958-1-chris@chris-wilson.co.uk
| * | | drm/i915: Serialise with engine-pm around requests on the kernel_contextChris Wilson2019-11-256-9/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As the engine->kernel_context is used within the engine-pm barrier, we have to be careful when emitting requests outside of the barrier, as the strict timeline locking rules do not apply. Instead, we must ensure the engine_park() cannot be entered as we build the request, which is simplest by taking an explicit engine-pm wakeref around the request construction. 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/20191125105858.1718307-1-chris@chris-wilson.co.uk
| * | | drm/i915/selftests: Shorten infinite wait for sseuChris Wilson2019-11-221-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use our more regular igt_flush_test() to bind the wait-for-idle and error out instead of waiting around forever on critical failure. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Stuart Summers <stuart.summers@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191121233021.507400-1-chris@chris-wilson.co.uk
| * | | drm/i915/selftests: Take a ref to the request we wait uponChris Wilson2019-11-201-9/+30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | i915_request_add() consumes the passed in reference to the i915_request, so if the selftest caller wishes to wait upon it afterwards, it needs to take a reference for itself. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191120102741.3734346-1-chris@chris-wilson.co.uk
| * | | drm/i915/gem: Protect the obj->vma.list during iterationChris Wilson2019-11-191-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Take the obj->vma.lock to prevent modifications to the list as we iterate, to avoid the dreaded NULL pointer. <1>[ 347.820823] BUG: kernel NULL pointer dereference, address: 0000000000000150 <1>[ 347.820856] #PF: supervisor read access in kernel mode <1>[ 347.820874] #PF: error_code(0x0000) - not-present page <6>[ 347.820892] PGD 0 P4D 0 <4>[ 347.820908] Oops: 0000 [#1] PREEMPT SMP NOPTI <4>[ 347.820926] CPU: 3 PID: 1303 Comm: gem_persistent_ Tainted: G U 5.4.0-rc7-CI-CI_DRM_7352+ #1 <4>[ 347.820956] Hardware name: /NUC6CAYB, BIOS AYAPLCEL.86A.0049.2018.0508.1356 05/08/2018 <4>[ 347.821132] RIP: 0010:i915_gem_object_flush_write_domain+0xd9/0x1d0 [i915] <4>[ 347.821157] Code: 0f 84 e9 00 00 00 48 8b 80 e0 fd ff ff f6 c4 40 75 11 e9 ed 00 00 00 48 8b 80 e0 fd ff ff f6 c4 40 74 26 48 8b 83 b0 00 00 00 <48> 8b b8 50 01 00 00 e8 fb 20 fb ff 48 8b 83 30 03 00 00 49 39 c4 <4>[ 347.821210] RSP: 0018:ffffc90000a1f8f8 EFLAGS: 00010202 <4>[ 347.821229] RAX: 0000000000000000 RBX: ffffc900008479a0 RCX: 0000000000000018 <4>[ 347.821252] RDX: 0000000000000000 RSI: 000000000000000d RDI: ffff888275a090b0 <4>[ 347.821274] RBP: ffff8882673c8040 R08: ffff88825991b8d0 R09: 0000000000000000 <4>[ 347.821297] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8882673c8280 <4>[ 347.821319] R13: ffff8882673c8368 R14: 0000000000000000 R15: ffff888266a54000 <4>[ 347.821343] FS: 00007f75865f4240(0000) GS:ffff888277b80000(0000) knlGS:0000000000000000 <4>[ 347.821368] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 <4>[ 347.821389] CR2: 0000000000000150 CR3: 000000025aee0000 CR4: 00000000003406e0 <4>[ 347.821411] Call Trace: <4>[ 347.821555] i915_gem_object_prepare_read+0xea/0x2a0 [i915] <4>[ 347.821706] intel_engine_cmd_parser+0x5ce/0xe90 [i915] <4>[ 347.821834] ? __i915_sw_fence_complete+0x1a0/0x250 [i915] <4>[ 347.821990] i915_gem_do_execbuffer+0xb4c/0x2550 [i915] Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191119100929.2628356-8-chris@chris-wilson.co.uk
| * | | drm/i915/gem: Merge GGTT vma flush into a single loopChris Wilson2019-11-191-9/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We only need the one loop to find the dirty vma flush them and their chipset. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191119100929.2628356-6-chris@chris-wilson.co.uk
| * | | drm/i915/gem: Track ggtt writes from userspace on the bound vmaChris Wilson2019-11-191-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When userspace writes into the GTT itself, it is supposed to call set-domain to let the kernel keep track and so manage the CPU/GPU caches. As we track writes on the individual i915_vma, we should also be sure to mark them as dirty. 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/20191119112515.2766748-1-chris@chris-wilson.co.uk
| * | | drm/i915/gem: Purge the sudden reappearance of i915_gem_object_pin()Chris Wilson2019-11-151-13/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This died many years ago as we now use i915_vma first and foremost. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191115170835.1367869-1-chris@chris-wilson.co.uk
| * | | drm/i915/gem: Silence sparse for RCU protection inside the constructorChris Wilson2019-11-151-2/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Inside the constructor, while cloning, we need to replace the dst->engines. Having forgotten that dst->engines is marked as RCU protected, we need to add the appropriate annotations to make sparse happy. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191114225736.616885-5-chris@chris-wilson.co.uk
| * | | Merge drm/drm-next into drm-intel-next-queuedJani Nikula2019-11-153-31/+92
| |\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Backmerge to get dfce90259d74 ("Backmerge i915 security patches from commit 'ea0b163b13ff' into drm-next") and thus 100d46bd72ec ("Merge Intel Gen8/Gen9 graphics fixes from Jon Bloomfield."). Signed-off-by: Jani Nikula <jani.nikula@intel.com>
| * | | | drm/i915/selftests: Remove unused local variable 'file'Chris Wilson2019-11-121-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c:453 igt_threaded_blt() error: uninitialized symbol 'file'. Fixes: 34485832cb98 ("drm/i915/selftests: Exercise parallel blit operations on a single ctx") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191112163643.3527-1-chris@chris-wilson.co.uk
| * | | | drm/i915: Flush context free work on cleanupChris Wilson2019-11-121-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Throw in a flush_work() to specifically flush the context cleanup work before the module is unloaded. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112248 Fixes: a4e7ccdac38e ("drm/i915: Move context management under GEM") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191112150051.1603-1-chris@chris-wilson.co.uk
| * | | | drm/i915/gem: Pass mem region to preallocated stolenChris Wilson2019-11-111-12/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As the memory regions are setup early, we can rely on its existence as we takeover the HW settings from BIOS. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191111182143.23479-2-chris@chris-wilson.co.uk
| * | | | drm/i915/gem: Replace implicit dev_priv->uncore for stolen initChris Wilson2019-11-111-98/+109
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Pass around the intended intel_uncore for mmio access during stolen setup, and avoid relying on the implicit magic I915_READ() macros. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191111182143.23479-1-chris@chris-wilson.co.uk
| * | | | drm/i915/selftests: Perform some basic cycle counting of MI opsChris Wilson2019-11-111-2/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some basic information that is useful to know, such as how many cycles is a MI_NOOP. v2: Keep volatile pages pinned at all times! (Matthew) Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Anna Karas <anna.karas@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191111172716.23733-1-chris@chris-wilson.co.uk
| * | | | drm/i915/userptr: Handle unlocked gup retriesChris Wilson2019-11-111-3/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Enable gup to retry and fault the pages outside of the mmap_sem lock in our worker. As we are inside our worker, outside of any critical path, we can allow the mmap_sem lock to be dropped in order to service a page fault; this in turn allows the mm to populate the page using a slow fault handler. References: 5b56d49fc31d ("mm: add locked parameter to get_user_pages_remote()") Testcase: igt/gem_userptr/userfault Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191111133205.11590-2-chris@chris-wilson.co.uk
| * | | | drm/i915/userptr: Try to acquire the page lock around set_page_dirty()Chris Wilson2019-11-111-1/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | set_page_dirty says: For pages with a mapping this should be done under the page lock for the benefit of asynchronous memory errors who prefer a consistent dirty state. This rule can be broken in some special cases, but should be better not to. Under those rules, it is only safe for us to use the plain set_page_dirty calls for shmemfs/anonymous memory. Userptr may be used with real mappings and so needs to use the locked version (set_page_dirty_lock). However, following a try_to_unmap() we may want to remove the userptr and so call put_pages(). However, try_to_unmap() acquires the page lock and so we must avoid recursively locking the pages ourselves -- which means that we cannot safely acquire the lock around set_page_dirty(). Since we can't be sure of the lock, we have to risk skip dirtying the page, or else risk calling set_page_dirty() without a lock and so risk fs corruption. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=203317 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112012 Fixes: 5cc9ed4b9a7a ("drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl") References: cb6d7c7dc7ff ("drm/i915/userptr: Acquire the page lock around set_page_dirty()") References: 505a8ec7e11a ("Revert "drm/i915/userptr: Acquire the page lock around set_page_dirty()"") References: 6dcc693bc57f ("ext4: warn when page is dirtied without buffers") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: stable@vger.kernel.org Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191111133205.11590-1-chris@chris-wilson.co.uk
| * | | | drm/i915/selftests: Fill all the drm_vma_manager holesChris Wilson2019-11-111-12/+39
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To test mmap_offset_exhaustion, we first have to fill the entire vma manager leaving a single page. Don't assume that the vma manager is not already fragment, and fill all the holes. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191111122706.28292-2-chris@chris-wilson.co.uk
| * | | | drm/i915/selftests: Exercise parallel blit operations on a single ctxChris Wilson2019-11-111-36/+57
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Make sure that our code is robust enough to handle multiple threads trying to clear objects for a single client context. This brings the joy of a shared GGTT to all! References: https://bugs.freedesktop.org/show_bug.cgi?id=112176 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191111122706.28292-1-chris@chris-wilson.co.uk
| * | | | drm/i915/gem: Update context name on closingChris Wilson2019-11-111-0/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Update the context.name on closing so that the persistent requests are clear in debug prints. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191111114323.5833-3-chris@chris-wilson.co.uk
| * | | | drm/i915/gem: Embed context/timeline name inside the GEM contextChris Wilson2019-11-113-22/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use a small char buffer inside the i915_gem_context to store the user friendly name so that ctx->name has the same lifetime as the RCU protected GEM context. That is, e.g. when using print_request() that prints the timeline name (ctx->name), the name will not be prematurely freed upon the context being closed and the last reference dropped. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191111114323.5833-2-chris@chris-wilson.co.uk
| * | | | drm/i915/selftests: Mark up sole accessor to ctx->vm as being protectedChris Wilson2019-11-081-10/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the selftests, where we are accessing a private ctx from within the confines of a single test, we know that the ctx->vm pointer is static and bounded by the lifetime of the test. We can use a simple helper to provide the RCU annotations to keep sparse happy. 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/20191107221201.30497-1-chris@chris-wilson.co.uk
| * | | | drm/i915/selftests: Complete transition to a real struct file mockChris Wilson2019-11-085-26/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since drm provided us with a real struct file we can use for our anonymous internal clients (mock_file), complete our transition to using that as the primary interface (and not the mocked up struct drm_file we previous were using). Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191107213929.23286-1-chris@chris-wilson.co.uk
| * | | | drm/i915: make more headers self-containedMasahiro Yamada2019-11-081-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The headers in the gem/selftests/, gt/selftests, gvt/, selftests/ directories have never been compile-tested, but it would be possible to make them self-contained. This commit only addresses missing <linux/types.h> and forward struct declarations. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20191108094142.25942-1-yamada.masahiro@socionext.com
| * | | | drm/i915: change to_mock() to an inline functionMasahiro Yamada2019-11-081-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since this function is defined in a header file, it should be 'static inline' instead of 'static'. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20191108051356.29980-1-yamada.masahiro@socionext.com
| * | | | drm/i915/selftests: Verify mmap_gtt revocation on unbindingChris Wilson2019-11-071-0/+111
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Whenever, we unbind (or change fence registers) on an object, we must revoke any and all mmap_gtt using the previous bindings. Those user PTEs point at the GGTT which know points into a new object, the wrong object. Ergo, those PTEs must be cleared so that any user access provokes a new page fault. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Reviewed-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191107180601.30815-5-chris@chris-wilson.co.uk
| * | | | drm/i915/selftests: Wrap vm_mmap() around GEM objectsChris Wilson2019-11-071-0/+100
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Provide a utility function to create a vma corresponding to an mmap() of our device. And use it to exercise the equivalent of userspace performing a GTT mmap of our objects. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Reviewed-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191107180601.30815-4-chris@chris-wilson.co.uk
| * | | | drm/i915/selftests: Replace mock_file hackery with drm's true fakeChris Wilson2019-11-073-9/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As drm now exports a method to create an anonymous struct file around a drm_device for internal use, make use of it to avoid our horrible hacks. Danial suggested that the mock_file_put() wrapper was suitable for drm-core, along with the mock_drm_getfile() [and that the vestigal mock_drm_file() in this patch should perhaps be the drm interface itself]. However, the eventual goal is to remove the mock_drm_file() and use the struct file and fput() directly, in this patch we take a simple transition in that direction. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20191107180601.30815-3-chris@chris-wilson.co.uk
| * | | | drm/i915/gem: Safely acquire the ctx->vm when copyingChris Wilson2019-11-071-37/+49
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As we read the ctx->vm unlocked before cloning/exporting, we should validate our reference is correct before returning it. We already do for clone_vm() but were not so strict around get_ppgtt(). Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191106091312.12921-1-chris@chris-wilson.co.uk
| * | | | drm/i915: FB backing gem obj should reside in LMEMRamalingam C2019-11-071-1/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If Local memory is supported by hardware, we want framebuffer backing gem objects from local memory. if the backing obj is not from LMEM, pin_to_display is failed. v2: memory regions are correctly assigned to obj->memory_regions [tvrtko] migration failure is reported as debug log [Tvrtko] v3: Migration is dropped. only error is reported [Daniel] mem region check is move to pin_to_display [Chris] v4: s/dev_priv/i915 [chris] v5: i915_gem_object_is_lmem is used for detecting the obj mem type. [Matt] cc: Matthew Auld <matthew.auld@intel.com> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20191105144414.30470-1-ramalingam.c@intel.com
| * | | | drm/i915: use might_lock_nested in get_pages annotationDaniel Vetter2019-11-071-18/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | So strictly speaking the existing annotation is also ok, because we have a chain of obj->mm.lock#I915_MM_GET_PAGES -> fs_reclaim -> obj->mm.lock (the shrinker cannot get at an object while we're in get_pages, hence this is safe). But it's confusing, so try to take the right subclass of the lock. This does a bit reduce our lockdep based checking, but then it's also less fragile, in case we ever change the nesting around. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Will Deacon <will@kernel.org> Cc: linux-kernel@vger.kernel.org Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191104173720.2696-3-daniel.vetter@ffwll.ch
| * | | | drm/i915: Switch obj->mm.lock lockdep annotations on its headDaniel Vetter2019-11-078-23/+38
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The trouble with having a plain nesting flag for locks which do not naturally nest (unlike block devices and their partitions, which is the original motivation for nesting levels) is that lockdep will never spot a true deadlock if you screw up. This patch is an attempt at trying better, by highlighting a bit more of the actual nature of the nesting that's going on. Essentially we have two kinds of objects: - objects without pages allocated, which cannot be on any lru and are hence inaccessible to the shrinker. - objects which have pages allocated, which are on an lru, and which the shrinker can decide to throw out. For the former type of object, memory allocations while holding obj->mm.lock are permissible. For the latter they are not. And get/put_pages transitions between the two types of objects. This is still not entirely fool-proof since the rules might change. But as long as we run such a code ever at runtime lockdep should be able to observe the inconsistency and complain (like with any other lockdep class that we've split up in multiple classes). But there are a few clear benefits: - We can drop the nesting flag parameter from __i915_gem_object_put_pages, because that function by definition is never going allocate memory, and calling it on an object which doesn't have its pages allocated would be a bug. - We strictly catch more bugs, since there's not only one place in the entire tree which is annotated with the special class. All the other places that had explicit lockdep nesting annotations we're now going to leave up to lockdep again. - Specifically this catches stuff like calling get_pages from put_pages (which isn't really a good idea, if we can call get_pages so could the shrinker). I've seen patches do exactly that. Of course I fully expect CI will show me for the fool I am with this one here :-) v2: There can only be one (lockdep only has a cache for the first subclass, not for deeper ones, and we don't want to make these locks even slower). Still separate enums for better documentation. Real fix: don't forget about phys objs and pin_map(), and fix the shrinker to have the right annotations ... silly me. v3: Forgot usertptr too ... v4: Improve comment for pages_pin_count, drop the IMPORTANT comment and instead prime lockdep (Chris). v5: Appease checkpatch, no double empty lines (Chris) v6: More rebasing over selftest changes. Also somehow I forgot to push this patch :-/ Also format comments consistently while at it. v7: Fix typo in commit message (Joonas) Also drop the priming, with the lmem merge we now have allocations while holding the lmem lock, which wreaks the generic priming I've done in earlier patches. Should probably be resurrected when lmem is fixed. See commit 232a6ebae419193f5b8da4fa869ae5089ab105c2 Author: Matthew Auld <matthew.auld@intel.com> Date: Tue Oct 8 17:01:14 2019 +0100 drm/i915: introduce intel_memory_region I'm keeping the priming patch locally so it wont get lost. Cc: Matthew Auld <matthew.auld@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: "Tang, CQ" <cq.tang@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v5) Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v6) Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191105090148.30269-1-daniel.vetter@ffwll.ch [mlankhorst: Fix commit typos pointed out by Michael Ruhl]
| * | | | drm/i915/gem: Fix error path to unlock if the GEM context is closedChris Wilson2019-11-061-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When inside the lock, remember to unlock even if you want to leave early. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Fixes: a4e7ccdac38e ("drm/i915: Move context management under GEM") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191106144155.25727-1-chris@chris-wilson.co.uk
| * | | | drm/i915/gem: Early rejection of no-aperture map_ggttChris Wilson2019-11-061-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the device does not have an aperture through which we can indirectly access and detile the buffers, simply reject the ioctl. Later we can extend the ioctl to support different modes, but as an extension the user must opt in and explicitly control the mmap type (viz MMAP_OFFSET_IOCTL). Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191105145305.14314-1-chris@chris-wilson.co.uk
| * | | | drm/i915: Defer rc6 shutdown to suspend_lateChris Wilson2019-11-011-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently we shutdown rc6 during i915_gem_resume() but this is called during the preparation phase (i915_drm_prepare) for all suspend paths, but we only want to shutdown rc6 for S3+. Move the actual shutdown to i915_gem_suspend_late(). We then need to differentiate between suspend targets, to distinguish S0 (s2idle) where the device is kept awake but needs to be in a low power mode (the same as runtime suspend) from the device suspend levels where we lose control of HW and so must disable any HW access to dangling memory. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111909 Fixes: c113236718e8 ("drm/i915: Extract GT render sleep (rc6) management") Testcase: igt/gem_exec_suspend/power-S0 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Andi Shyti <andi.shyti@intel.com> Acked-by: Andi Shyti <andi.shyti@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191101141009.15581-4-chris@chris-wilson.co.uk
| * | | | drm/i915/gt: Move user_forcewake application to GTChris Wilson2019-11-011-22/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We already track the debugfs user_forcewake on the GT, so it is natural to pull the suspend/resume handling under gt/ as well. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Andi Shyti <andi.shyti@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191101141009.15581-3-chris@chris-wilson.co.uk
| * | | | drm/i915/gem: Leave reloading kernel context on resume to GTChris Wilson2019-11-011-30/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As we already do reload the kernel context in intel_gt_resume, repeating that action inside i915_gem_resume() as well is redundant. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Andi Shyti <andi.shyti@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191101141009.15581-2-chris@chris-wilson.co.uk
OpenPOWER on IntegriCloud