Age | Commit message (Collapse) | Author | Files | Lines |
|
Use the unlocked variants for pin_map and pin_pages, and add lock
around unpinning/putting pages.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Straightforward conversion by using unlocked versions.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
We can no longer call intel_timeline_pin with a null argument,
so add a ww loop that locks the backing object.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Use unlocked versions when the ww lock is not held.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Use pin_map_unlocked when we're not holding locks.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Convert normal functions to unlocked versions where needed.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Convert a few calls to use the unlocked versions.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Only needs to convert a single call to the unlocked version.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
igt_emit_store_dw needs to use the unlocked version, as it's not
holding a lock. This fixes igt_gpu_fill_dw() which is used by
some other selftests.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Use some unlocked versions where we're not holding the ww lock.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Convert a single pin_pages call to use the unlocked version.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Ensure we hold the lock around put_pages, and use the unlocked wrappers
for pinning pages and mappings.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Also quite simple, a single call needs to use the unlocked version.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Use pin_pages_unlocked() where we don't have a lock.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Straightforward conversion, just convert a bunch of calls to
unlocked versions.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Straightforward conversion, just convert a bunch of calls to
unlocked versions.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Straightforward conversion, just convert a bunch of calls to
unlocked versions.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Straightforward conversion, just convert a bunch of calls to
unlocked versions.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
We may create page table objects on the fly, but we may need to
wait with the ww lock held. Instead of waiting on a freed obj
lock, ensure we have the same lock for each object to keep
-EDEADLK working. This ensures that i915_vma_pin_ww can lock
the page tables when required.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Quick fix, just use the unlocked version.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Because of the long lifetime of the mapping, we cannot wrap this in a
simple limited ww lock. Just use the unlocked version of pin_map,
because we'll likely release the mapping a lot later, in a different
thread.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
vmap is using pin_pages, but needs to use ww locking,
add pin_pages_unlocked to correctly lock the mapping.
Also add ww locking to begin/end cpu access.
Changes since v1:
- Fix i915_gem_map_dma_buf by using pin_pages_unlocked().
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
In the ucode functions, the calls are done before userspace runs,
when debugging using debugfs, or when creating semi-permanent mappings;
we can safely use the unlocked versions that does the ww dance for us.
Because there is no pin_pages_unlocked yet, add it as convenience function.
This removes possible lockdep splats about missing resv lock for ucode.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
We need to lock a few more objects, some temporarily,
add ww lock where needed.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
i915_gem_object_pin_map potentially needs a ww context, so ensure we
have one we can revoke.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
By default, we assume that it's called inside igt_create_request
to keep existing selftests working, but allow for manual pinning
when passing a ww context.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Stolen objects need to lock, and we may call put_pages when
refcount drops to 0, ensure all calls are handled correctly.
Changes since v1:
- Rebase on top of upstream changes.
Idea-from: Thomas Hellström <[email protected]>
Signed-off-by: Thomas Hellström <[email protected]>
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Matthew Auld <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
pin_map needs the ww lock, so ensure we pin both before submission.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
[danvet: Again pick older version just to side-step conflicts.]
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
We are removing obj->mm.lock, and need to take the reservation lock
before we can pin pages. Move the pinning pages into the helper, and
merge gtt pwrite/pread preparation and cleanup paths.
The fence lock is also removed; it will conflict with fence annotations,
because of memory allocations done when pagefaulting inside copy_*_user.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
[danvet: Pick the older version to avoid the conflicts]
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
We need to take the obj lock to pin pages, so wait until the callers
have done so, before making the object unshrinkable.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
We need to lock the object to move it to the correct domain,
add the missing lock.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
[danvet: Pick version from an older patch series.]
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Take the ww lock around engine_unpark. Because of the
many many places where rpm is used, I chose the safest option
and used a trylock to opportunistically take this lock for
__engine_unpark.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Make creation separate from pinning, in order to take the lock only
once, and pin the mapping with the lock held.
Changes since v1:
- Rebase on top of upstream changes.
Changes since v2:
- Fully clear wa_ctx on error.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
We previously complained when ww == NULL.
This function is now only used in selftests to pin an object,
and ww locking is now fixed.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
[danvet: Resolve conflict because we don't have a set-domain refactor,
see
https://lore.kernel.org/intel-gfx/[email protected]/
The really worrying thing here is that the above patch had a change in
arguments for i915_gem_object_set_to_gtt_domain(), without any
explanation. I decided to just faithfully apply Maarten's change but
not the argument change which was in Maarten's context diff.]
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
This should be done as part of the ww loop, in order to remove a
i915_vma_pin that needs ww held.
Now only i915_ggtt_pin() callers remaining.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Take a simple lock so we hold ww around (un)pin_pages as needed.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Instead of multiple lockings, lock the object once,
and perform the ww dance around attach_phys and pin_pages.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Pin in the caller, not in the work itself. This should also
work better for dma-fence annotations.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Try to pin to ggtt first, and use a full ww loop to handle
eviction correctly.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
We map the initial context during first pin.
This allows us to remove pin_map from state allocation, which saves
us a few retry loops. We won't need this until first pin anyway.
intel_ring_submission_setup() is also reworked slightly to do all
pinning in a single ww loop.
Changes since v1:
- Handle -EDEADLK backoff in intel_ring_submission_setup() better.
- Handle smatch errors reported by Dan and testbot.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
This allows us to remove pin_map from state allocation, which saves
us a few retry loops. We won't need this until first pin, anyway.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
[danvet: Resolve context conflict because we don't have the
i915_scheduler.c extraction from the below patches set:
https://lore.kernel.org/intel-gfx/[email protected]/]
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
With userptr fixed, there is no need for all separate lockdep classes
now, and we can remove all lockdep tricks used. A trylock in the
shrinker is all we need now to flatten the locking hierarchy.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
[danvet: Resolve conflict because we don't have the patch from Chris
to rebrand i915_gem_shrinker_taints_mutex to fs_reclaim_taints_mutex.
It's not a bad idea, but if we do it, it should be moved to the right
header. See
https://lore.kernel.org/intel-gfx/[email protected]/]
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Instead of doing what we do currently, which will never work with
PROVE_LOCKING, do the same as AMD does, and something similar to
relocation slowpath. When all locks are dropped, we acquire the
pages for pinning. When the locks are taken, we transfer those
pages in .get_pages() to the bo. As a final check before installing
the fences, we ensure that the mmu notifier was not called; if it is,
we return -EAGAIN to userspace to signal it has to start over.
Changes since v1:
- Unbinding is done in submit_init only. submit_begin() removed.
- MMU_NOTFIER -> MMU_NOTIFIER
Changes since v2:
- Make i915->mm.notifier a spinlock.
Changes since v3:
- Add WARN_ON if there are any page references left, should have been 0.
- Return 0 on success in submit_init(), bug from spinlock conversion.
- Release pvec outside of notifier_lock (Thomas).
Changes since v4:
- Mention why we're clearing eb->[i + 1].vma in the code. (Thomas)
- Actually check all invalidations in eb_move_to_gpu. (Thomas)
- Do not wait when process is exiting to fix gem_ctx_persistence.userptr.
Changes since v5:
- Clarify why check on PF_EXITING is (temporarily) required.
Changes since v6:
- Ensure userptr validity is checked in set_domain through a special path.
Signed-off-by: Maarten Lankhorst <[email protected]>
Acked-by: Dave Airlie <[email protected]>
[danvet: s/kfree/kvfree/ in i915_gem_object_userptr_drop_ref in the
previous review round, but which got lost. The other open questions
around page refcount are imo better discussed in a separate series,
with amdgpu folks involved].
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Now that unsynchronized mappings are removed, the only time userptr
works is when the MMU notifier is enabled. Put all of the userptr
code behind a mmu notifier ifdef.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
We should not allow this any more, as it will break with the new userptr
implementation, it could still be made to work, but there's no point in
doing so.
Inspection of the beignet opencl driver shows that it's only used
when normal userptr is not available, which means for new kernels
you will need CONFIG_I915_USERPTR.
Signed-off-by: Maarten Lankhorst <[email protected]>
Acked-by: Dave Airlie <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Acked-by: Jason Ekstrand <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
There are a couple of ioctl's related to tiling and cache placement,
that make no sense for userptr, reject those:
- i915_gem_set_tiling_ioctl()
Tiling should always be linear for userptr. Changing placement will
fail with -ENXIO.
- i915_gem_set_caching_ioctl()
Userptr memory should always be cached. Changing caching mode will
fail with -ENXIO.
- i915_gem_set_domain_ioctl()
Still temporarily allowed to work as intended, it's used to check
userptr validity. With the reworked userptr code, it will keep
working for this usecase.
This plus the previous changes have been tested against beignet
by using its own unit tests, and intel-video-compute by using
piglit's opencl tests.
Changes since v1:
- set_domain was apparently used in iris for checking userptr validity,
keep it working as intended.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Acked-by: Jason Ekstrand <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
It doesn't make sense to export a memory address, we will prevent
allowing access this way to different address spaces when we
rework userptr handling, so best to explicitly disable it.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Acked-by: Jason Ekstrand <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Userptr should not need the kernel for a userspace memcpy, userspace
needs to call memcpy directly.
Specifically, disable i915_gem_pwrite_ioctl() and i915_gem_pread_ioctl().
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Acked-by: Jason Ekstrand <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
As soon as we install fences, we should stop allocating memory
in order to prevent any potential deadlocks.
This is required later on, when we start adding support for
dma-fence annotations.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Simple adding of i915_gem_object_lock, we may start to pass ww to
get_pages() in the future, but that won't be the case here;
We override shmem's get_pages() handling by calling
i915_gem_object_get_pages_phys(), no ww is needed.
Changes since v1:
- Call shmem put pages directly, the callback would
go down the phys free path.
Signed-off-by: Maarten Lankhorst <[email protected]>
Reviewed-by: Thomas Hellström <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|