summaryrefslogtreecommitdiff
path: root/drivers/gpu/drm/i915/gem
AgeCommit message (Collapse)Author
2021-07-26drm/i915/gem: Migrate to system at dma-buf attach time (v7)Thomas Hellström
Until we support p2p dma or as a complement to that, migrate data to system memory at dma-buf attach time if possible. v2: - Rebase on dynamic exporter. Update the igt_dmabuf_import_same_driver selftest to migrate if we are LMEM capable. v3: - Migrate also in the pin() callback. v4: - Migrate in attach v5: (jason) - Lock around the migration v6: (jason) - Move the can_migrate check outside the lock - Rework the selftests to test more migration conditions. In particular, SMEM, LMEM, and LMEM+SMEM are all checked. v7: (mauld) - Misc style nits Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com> Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210723172142.3273510-9-jason@jlekstrand.net
2021-07-26drm/i915/gem: Correct the locking and pin pattern for dma-buf (v8)Thomas Hellström
If our exported dma-bufs are imported by another instance of our driver, that instance will typically have the imported dma-bufs locked during dma_buf_map_attachment(). But the exporter also locks the same reservation object in the map_dma_buf() callback, which leads to recursive locking. So taking the lock inside _pin_pages_unlocked() is incorrect. Additionally, the current pinning code path is contrary to the defined way that pinning should occur. Remove the explicit pin/unpin from the map/umap functions and move them to the attach/detach allowing correct locking to occur, and to match the static dma-buf drm_prime pattern. Add a live selftest to exercise both dynamic and non-dynamic exports. v2: - Extend the selftest with a fake dynamic importer. - Provide real pin and unpin callbacks to not abuse the interface. v3: (ruhl) - Remove the dynamic export support and move the pinning into the attach/detach path. v4: (ruhl) - Put pages does not need to assert on the dma-resv v5: (jason) - Lock around dma_buf_unmap_attachment() when emulating a dynamic importer in the subtests. - Use pin_pages_unlocked v6: (jason) - Use dma_buf_attach instead of dma_buf_attach_dynamic in the selftests v7: (mauld) - Use __i915_gem_object_get_pages (2 __underscores) instead of the 4 ____underscore version in the selftests v8: (mauld) - Drop the kernel doc from the static i915_gem_dmabuf_attach function - Add missing "err = PTR_ERR()" to a bunch of selftest error cases Reported-by: Michael J. Ruhl <michael.j.ruhl@intel.com> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com> Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210723172142.3273510-8-jason@jlekstrand.net
2021-07-26drm/i915/gem: Always call obj->ops->migrate unless can_migrate failsJason Ekstrand
Without TTM, we have no such hook so we exit early but this is fine because we use TTM on all LMEM platforms and, on integrated platforms, there is no real migration. If we do have the hook, it's better to just let TTM handle the migration because it knows where things are actually placed. This fixes a bug where i915_gem_object_migrate fails to migrate newly created LMEM objects. In that scenario, the object has obj->mm.region set to LMEM but TTM has it in SMEM because that's where all new objects are placed there prior to getting actual pages. When we invoke i915_gem_object_migrate, it exits early because, from the point of view of the GEM object, it's already in LMEM and no migration is needed. Then, when we try to pin the pages, __i915_ttm_get_pages is called which, unaware of our failed attempt at a migration, places the object in SMEM. This only happens on newly created objects because they have this weird state where TTM thinks they're in SMEM, GEM thinks they're in LMEM, and the reality is that they don't exist at all. It's better if GEM just always calls into TTM and let's TTM handle things. That way the lies stay better contained. Once the migration is complete, the object will have pages, obj->mm.region will be correct, and we're done lying. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210723172142.3273510-7-jason@jlekstrand.net
2021-07-26drm/i915/gem/ttm: Only call __i915_gem_object_set_pages if neededJason Ekstrand
__i915_ttm_get_pages does two things. First, it calls ttm_bo_validate() to check the given placement and migrate the BO if needed. Then, it updates the GEM object to match, in case the object was migrated. If no migration occured, however, we might still have pages on the GEM object in which case we don't need to fetch them from TTM and call __i915_gem_object_set_pages. This hasn't been a problem before because the primary user of __i915_ttm_get_pages is __i915_gem_object_get_pages which only calls it if the GEM object doesn't have pages. However, i915_ttm_migrate also uses __i915_ttm_get_pages to do the migration so this meant it was unsafe to call on an already populated object. This patch checks i915_gem_object_has_pages() before trying to __i915_gem_object_set_pages so i915_ttm_migrate is safe to call, even on populated objects. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210723172142.3273510-6-jason@jlekstrand.net
2021-07-26drm/i915/gem: Unify user object creation (v3)Jason Ekstrand
Instead of hand-rolling the same three calls in each function, pull them into an i915_gem_object_create_user helper. Apart from re-ordering of the placements array ENOMEM check, there should be no functional change. v2 (Matthew Auld): - Add the call to i915_gem_flush_free_objects() from i915_gem_dumb_create() in a separate patch - Move i915_gem_object_alloc() below the simple error checks v3 (Matthew Auld): - Add __ to i915_gem_object_create_user and kerneldoc which warns the caller that it's not validating anything. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210723172142.3273510-5-jason@jlekstrand.net
2021-07-26drm/i915/gem: Call i915_gem_flush_free_objects() in i915_gem_dumb_create()Jason Ekstrand
This doesn't really fix anything serious since the chances of a client creating and destroying a mass of dumb BOs is pretty low. However, it is called by the other two create IOCTLs to garbage collect old objects. Call it here too for consistency. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210723172142.3273510-4-jason@jlekstrand.net
2021-07-26drm/i915/gem: Refactor placement setup for i915_gem_object_create* (v2)Jason Ekstrand
Since we don't allow changing the set of regions after creation, we can make ext_set_placements() build up the region set directly in the create_ext and assign it to the object later. This is similar to what we did for contexts with the proto-context only simpler because there's no funny object shuffling. This will be used in the next patch to allow us to de-duplicate a bunch of code. Also, since we know the maximum number of regions up-front, we can use a fixed-size temporary array for the regions. This simplifies memory management a bit for this new delayed approach. v2 (Matthew Auld): - Get rid of MAX_N_PLACEMENTS - Drop kfree(placements) from set_placements() v3 (Matthew Auld): - Properly set ext_data->n_placements Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210723172142.3273510-3-jason@jlekstrand.net
2021-07-26drm/i915/gem: Check object_can_migrate from object_migrateJason Ekstrand
We don't roll them together entirely because there are still a couple cases where we want a separate can_migrate check. For instance, the display code checks that you can migrate a buffer to LMEM before it accepts it in fb_create. The dma-buf import code also uses it to do an early check and return a different error code if someone tries to attach a LMEM-only dma-buf to another driver. However, no one actually wants to call object_migrate when can_migrate has failed. The stated intention is for self-tests but none of those actually take advantage of this unsafe migration. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Cc: Daniel Vetter <daniel@ffwll.ch> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210723172142.3273510-2-jason@jlekstrand.net
2021-07-26Backmerge tag 'v5.14-rc3' into drm-nextDave Airlie
Linux 5.14-rc3 Daniel said we should pull the nouveau fix from fixes in here, probably a good plan. Signed-off-by: Dave Airlie <airlied@redhat.com>
2021-07-23drm/i915/uapi: reject set_domain for discreteMatthew Auld
The CPU domain should be static for discrete, and on DG1 we don't need any flushing since everything is already coherent, so really all this does is an object wait, for which we have an ioctl. Longer term the desired caching should be an immutable creation time property for the BO, which can be set with something like gem_create_ext. One other user is iris + userptr, which uses the set_domain to probe all the pages to check if the GUP succeeds, however we now have a PROBE flag for this purpose. v2: add some more kernel doc, also add the implicit rules with caching Suggested-by: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Kenneth Graunke <kenneth@whitecape.org> Cc: Jason Ekstrand <jason@jlekstrand.net> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Ramalingam C <ramalingam.c@intel.com> Reviewed-by: Ramalingam C <ramalingam.c@intel.com> Acked-by: Kenneth Graunke <kenneth@whitecape.org> Link: https://patchwork.freedesktop.org/patch/msgid/20210715101536.2606307-5-matthew.auld@intel.com
2021-07-22drm/i915/guc: Update intel_gt_wait_for_idle to work with GuCMatthew Brost
When running the GuC the GPU can't be considered idle if the GuC still has contexts pinned. As such, a call has been added in intel_gt_wait_for_idle to idle the UC and in turn the GuC by waiting for the number of unpinned contexts to go to zero. v2: rtimeout -> remaining_timeout v3: Drop unnecessary includes, guc_submission_busy_loop -> guc_submission_send_busy_loop, drop negatie timeout trick, move a refactor of guc_context_unpin to earlier path (John H) v4: Add stddef.h back into intel_gt_requests.h, sort circuit idle function if not in GuC submission mode Cc: John Harrison <john.c.harrison@intel.com> Signed-off-by: Matthew Brost <matthew.brost@intel.com> Reviewed-by: John Harrison <John.C.Harrison@Intel.com> Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210721215101.139794-16-matthew.brost@intel.com
2021-07-22drm/i915/guc: Disable semaphores when using GuC schedulingMatthew Brost
Semaphores are an optimization and not required for basic GuC submission to work properly. Disable until we have time to do the implementation to enable semaphores and tune them for performance. Also long direction is just to delete semaphores from the i915 so another reason to not enable these for GuC submission. This patch fixes an existing bugs where I915_ENGINE_HAS_SEMAPHORES was not honored correctly. v2: Reword commit message v3: (John H) - Add text to commit indicating this also fixing an existing bug v4: (John H) - s/bug/bugs Cc: John Harrison <john.c.harrison@intel.com> Signed-off-by: Matthew Brost <matthew.brost@intel.com> Reviewed-by: John Harrison <John.C.Harrison@Intel.com> Signed-off-by: John Harrison <John.C.Harrison@Intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210721215101.139794-14-matthew.brost@intel.com
2021-07-22drm/i915: Ditch i915 globals shrink infrastructureDaniel Vetter
This essentially reverts commit 84a1074920523430f9dc30ff907f4801b4820072 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Wed Jan 24 11:36:08 2018 +0000 drm/i915: Shrink the GEM kmem_caches upon idling mm/vmscan.c:do_shrink_slab() is a thing, if there's an issue with it then we need to fix that there, not hand-roll our own slab shrinking code in i915. Also when this was added there was only one other caller of kmem_cache_shrink (added 2005 to the acpi code). Now there's a 2nd one outside of i915 code in a kunit test, which seems legit since that wants to very carefully control what's in the kmem_cache. This out of a total of over 500 calls to kmem_cache_create. This alone should have been warning sign enough that we're doing something silly. Noticed while reviewing a patch set from Jason to fix up some issues in our i915_init() and i915_exit() module load/cleanup code. Now that i915_globals.c isn't any different than normal init/exit functions, we should convert them over to one unified table and remove i915_globals.[hc] entirely. v2: Improve commit message (Jason) Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> Cc: David Airlie <airlied@linux.ie> Cc: Jason Ekstrand <jason@jlekstrand.net> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210721183229.4136488-1-daniel.vetter@ffwll.ch
2021-07-21drm/i915: Add TTM offset argument to mmap.Maarten Lankhorst
The FIXED mapping is only used for ttm, and tells userspace that the mapping type is pre-defined. This disables the other type of mmap offsets when discrete memory is used, so fix the selftests as well. Document the struct as well, so it shows up in docbook. Cc: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> [mauld: Included minor fixes from the review comments] Signed-off-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210714122833.766586-1-maarten.lankhorst@linux.intel.com
2021-07-21Merge tag 'drm-misc-next-2021-07-16' of ↵Dave Airlie
git://anongit.freedesktop.org/drm/drm-misc into drm-next drm-misc-next for v5.15: UAPI Changes: Cross-subsystem Changes: - udmabuf: Add support for mapping hugepages - Add dma-buf stats to sysfs. - Assorted fixes to fbdev/omap2. - dma-buf: Document DMA_BUF_IOCTL_SYNC - Improve dma-buf non-dynamic exporter expectations better. - Add module parameters for dma-buf size and list limit. - Add HDMI codec support to vc4, to replace vc4's own codec. - Document dma-buf implicit fencing rules. - dma_resv_test_signaled test_all handling. Core Changes: - Extract i915's eDP backlight code into DRM helpers. - Assorted docbook updates. - Rework drm_dp_aux documentation. - Add support for the DP aux bus. - Shrink dma-fence-chain slightly. - Add alloc/free helpers for dma-fence-chain. - Assorted fixes to TTM., drm/of, bridge - drm_gem_plane_helper_prepare/cleanup_fb is now the default for gem drivers. - Small fix for scheduler completion. - Remove use of drm_device.irq_enabled. - Print the driver name to dmesg when registering framebuffer. - Export drm/gem's shadow plane handling, and use it in vkms. - Assorted small fixes. Driver Changes: - Add eDP backlight to nouveau. - Assorted fixes and cleanups to nouveau, panfrost, vmwgfx, anx7625, amdgpu, gma500, radeon, mgag200, vgem, vc4, vkms, omapdrm. - Add support for Samsung DB7430, Samsung ATNA33XC20, EDT ETMV570G2DHU, EDT ETM0350G0DH6, Innolux EJ030NA panels. - Fix some simple pannels missing bus_format and connector types. - Add mks-guest-stats instrumentation support to vmwgfx. - Merge i915-ttm topic branch. - Make s6e63m0 panel use Mipi-DBI helpers. - Add detect() supoprt for AST. - Use interrupts for hotplug on vc4. - vmwgfx is now moved to drm-misc-next, as sroland is no longer a maintainer for now. - vmwgfx now uses copies of vmware's internal device headers. - Slowly convert ti-sn65dsi83 over to atomic. - Rework amdgpu dma-resv handling. - Fix virtio fencing for planes. - Ensure amdgpu can always evict to SYSTEM. - Many drivers fixed for implicit fencing rules. - Set default prepare/cleanup fb for tiny, vram and simple helpers too. - Rework panfrost gpu reset and related serialization. - Update VKMS todo list. - Make bochs a tiny gpu driver, and use vram helper. - Use linux irq interfaces instead of drm_irq in some drivers. - Add support for Raspberry Pi Pico to GUD. Signed-off-by: Dave Airlie <airlied@redhat.com> # gpg: Signature made Fri 16 Jul 2021 21:06:04 AEST # gpg: using RSA key B97BD6A80CAC4981091AE547FE558C72A67013C3 # gpg: Good signature from "Maarten Lankhorst <maarten.lankhorst@linux.intel.com>" [expired] # gpg: aka "Maarten Lankhorst <maarten@debian.org>" [expired] # gpg: aka "Maarten Lankhorst <maarten.lankhorst@canonical.com>" [expired] # gpg: Note: This key has expired! # Primary key fingerprint: B97B D6A8 0CAC 4981 091A E547 FE55 8C72 A670 13C3 From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/444811c3-cbec-e9d5-9a6b-9632eda7962a@linux.intel.com
2021-07-20drm/i915/uapi: reject caching ioctls for discreteMatthew Auld
It's a noop on DG1, and in the future when need to support other devices which let us control the coherency, then it should be an immutable creation time property for the BO. This will likely be controlled through a new gem_create_ext extension. v2: add some kernel doc for the discrete changes, and document the implicit rules Suggested-by: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Kenneth Graunke <kenneth@whitecape.org> Cc: Jason Ekstrand <jason@jlekstrand.net> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Ramalingam C <ramalingam.c@intel.com> Reviewed-by: Ramalingam C <ramalingam.c@intel.com> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org> Link: https://patchwork.freedesktop.org/patch/msgid/20210715101536.2606307-2-matthew.auld@intel.com
2021-07-19drm/i915: Revert "drm/i915/gem: Asynchronous cmdparser"Jason Ekstrand
This reverts 686c7c35abc2 ("drm/i915/gem: Asynchronous cmdparser"). The justification for this commit in the git history was a vague comment about getting it out from under the struct_mutex. While this may improve perf for some workloads on Gen7 platforms where we rely on the command parser for features such as indirect rendering, no numbers were provided to prove such an improvement. It claims to closed two gitlab/bugzilla issues but with no explanation whatsoever as to why or what bug it's fixing. Meanwhile, by moving command parsing off to an async callback, it leaves us with a problem of what to do on error. When things were synchronous, EXECBUFFER2 would fail with an error code if parsing failed. When moving it to async, we needed another way to handle that error and the solution employed was to set an error on the dma_fence and then trust that said error gets propagated to the client eventually. Moving back to synchronous will help us untangle the fence error propagation mess. This also reverts most of 0edbb9ba1bfe ("drm/i915: Move cmd parser pinning to execbuffer") which is a refactor of some of our allocation paths for asynchronous parsing. Now that everything is synchronous, we don't need it. v2 (Daniel Vetter): - Add stabel Cc and Fixes tag Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Cc: <stable@vger.kernel.org> # v5.6+ Fixes: 9e31c1fe45d5 ("drm/i915: Propagate errors on awaiting already signaled fences") Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210714193419.1459723-2-jason@jlekstrand.net (cherry picked from commit 93b713304188844b8514074dc13ffd56d12235d3) Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
2021-07-16drm/i915: Drop error handling from dma_fence_workJason Ekstrand
Asynchronous command parsing was the only thing which ever returned a non-zero error. With that gone, we can drop the error handling from dma_fence_work. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210714193419.1459723-5-jason@jlekstrand.net
2021-07-16drm/i915: Remove allow_alloc from i915_gem_object_get_sg*Jason Ekstrand
This reverts the rest of 0edbb9ba1bfe ("drm/i915: Move cmd parser pinning to execbuffer"). Now that the only user of i915_gem_object_get_sg without allow_alloc has been removed, we can drop the parameter. This portion of the revert was broken into its own patch to aid review. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210714193419.1459723-4-jason@jlekstrand.net
2021-07-16drm/i915: Revert "drm/i915/gem: Asynchronous cmdparser"Jason Ekstrand
This reverts 686c7c35abc2 ("drm/i915/gem: Asynchronous cmdparser"). The justification for this commit in the git history was a vague comment about getting it out from under the struct_mutex. While this may improve perf for some workloads on Gen7 platforms where we rely on the command parser for features such as indirect rendering, no numbers were provided to prove such an improvement. It claims to closed two gitlab/bugzilla issues but with no explanation whatsoever as to why or what bug it's fixing. Meanwhile, by moving command parsing off to an async callback, it leaves us with a problem of what to do on error. When things were synchronous, EXECBUFFER2 would fail with an error code if parsing failed. When moving it to async, we needed another way to handle that error and the solution employed was to set an error on the dma_fence and then trust that said error gets propagated to the client eventually. Moving back to synchronous will help us untangle the fence error propagation mess. This also reverts most of 0edbb9ba1bfe ("drm/i915: Move cmd parser pinning to execbuffer") which is a refactor of some of our allocation paths for asynchronous parsing. Now that everything is synchronous, we don't need it. v2 (Daniel Vetter): - Add stabel Cc and Fixes tag Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Cc: <stable@vger.kernel.org> # v5.6+ Fixes: 9e31c1fe45d5 ("drm/i915: Propagate errors on awaiting already signaled fences") Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210714193419.1459723-2-jason@jlekstrand.net
2021-07-13swiotlb: Update is_swiotlb_active to add a struct device argumentClaire Chang
Update is_swiotlb_active to add a struct device argument. This will be useful later to allow for different pools. Signed-off-by: Claire Chang <tientzu@chromium.org> Reviewed-by: Christoph Hellwig <hch@lst.de> Tested-by: Stefano Stabellini <sstabellini@kernel.org> Tested-by: Will Deacon <will@kernel.org> Acked-by: Stefano Stabellini <sstabellini@kernel.org> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
2021-07-13Merge drm/drm-next into drm-intel-gt-nextRodrigo Vivi
Catching up with 5.14-rc1 and also preparing for a needed common topic branch for the "Minor revid/stepping and workaround cleanup" Reference: https://patchwork.freedesktop.org/series/92299/ Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
2021-07-12drm/i915: Fix fall-through warning for ClangGustavo A. R. Silva
In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning by explicitly adding a return; statement: drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:65:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough] Link: https://github.com/KSPP/linux/issues/115 Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
2021-07-09drm/i915: use consistent CPU mappings for pin_map usersMatthew Auld
For discrete, users of pin_map() needs to obey the same rules at the TTM backend, where we map system only objects as WB, and everything else as WC. The simplest for now is to just force the correct mapping type as per the new rules for discrete. Suggested-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Ramalingam C <ramalingam.c@intel.com> Reviewed-by: Ramalingam C <ramalingam.c@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210705135310.1502437-1-matthew.auld@intel.com
2021-07-08drm/i915: Finalize contexts in GEM_CONTEXT_CREATE on version 13+Jason Ekstrand
All the proto-context stuff for context creation exists to allow older userspace drivers to set VMs and engine sets via SET_CONTEXT_PARAM. Drivers need to update to use CONTEXT_CREATE_EXT_* for this going forward. Force the issue by blocking the old mechanism on any future hardware generations. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Cc: Jon Bloomfield <jon.bloomfield@intel.com> Cc: Carl Zhang <carl.zhang@intel.com> Cc: Michal Mrozek <michal.mrozek@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-31-jason@jlekstrand.net
2021-07-08drm/i915/gem: Roll all of context creation togetherJason Ekstrand
Now that we have the whole engine set and VM at context creation time, we can just assign those fields instead of creating first and handling the VM and engines later. This lets us avoid creating useless VMs and engine sets and lets us get rid of the complex VM setting code. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-30-jason@jlekstrand.net
2021-07-08i915/gem/selftests: Assign the VM at context creation in igt_shared_ctx_execJason Ekstrand
We want to delete __assign_ppgtt and, generally, stop setting the VM after context creation. This is the one place I could find in the selftests where we set a VM after the fact. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-29-jason@jlekstrand.net
2021-07-08drm/i915/selftests: Take a VM in kernel_context()Jason Ekstrand
This better models where we want to go with contexts in general where things like the VM and engine set are create parameters instead of being set after the fact. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-28-jason@jlekstrand.net
2021-07-08drm/i915/gem: Don't allow changing the engine set on running contexts (v3)Jason Ekstrand
When the APIs were added to manage the engine set on a GEM context directly from userspace, the questionable choice was made to allow changing the engine set on a context at any time. This is horribly racy and there's absolutely no reason why any userspace would want to do this outside of trying to exercise interesting race conditions. By removing support for CONTEXT_PARAM_ENGINES from ctx_setparam, we make it impossible to change the engine set after the context has been fully created. This doesn't yet let us delete all the deferred engine clean-up code as that's still used for handling the case where the client dies or calls GEM_CONTEXT_DESTROY while work is in flight. However, moving to an API where the engine set is effectively immutable gives us more options to potentially clean that code up a bit going forward. It also removes a whole class of ways in which a client can hurt itself or try to get around kernel context banning. v2 (Jason Ekstrand): - Expand the commit mesage v3 (Jason Ekstrand): - Make it more obvious that I915_CONTEXT_PARAM_ENGINES returns -EINVAL Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-27-jason@jlekstrand.net
2021-07-08drm/i915/gem: Don't allow changing the VM on running contexts (v4)Jason Ekstrand
When the APIs were added to manage VMs more directly from userspace, the questionable choice was made to allow changing out the VM on a context at any time. This is horribly racy and there's absolutely no reason why any userspace would want to do this outside of testing that exact race. By removing support for CONTEXT_PARAM_VM from ctx_setparam, we make it impossible to change out the VM after the context has been fully created. This lets us delete a bunch of deferred task code as well as a duplicated (and slightly different) copy of the code which programs the PPGTT registers. v2 (Jason Ekstrand): - Expand the commit message v3 (Daniel Vetter): - Don't drop the __rcu on the vm pointer v4 (Jason Ekstrand): - Make it more obvious that I915_CONTEXT_PARAM_VM returns -EINVAL Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-26-jason@jlekstrand.net
2021-07-08drm/i915/gem: Delay context creation (v3)Jason Ekstrand
The current context uAPI allows for two methods of setting context parameters: SET_CONTEXT_PARAM and CONTEXT_CREATE_EXT_SETPARAM. The former is allowed to be called at any time while the later happens as part of GEM_CONTEXT_CREATE. Currently, everything settable via one is settable via the other. While some params are fairly simple and setting them on a live context is harmless such as the context priority, others are far trickier such as the VM or the set of engines. In order to swap out the VM, for instance, we have to delay until all current in-flight work is complete, swap in the new VM, and then continue. This leads to a plethora of potential race conditions we'd really rather avoid. In previous patches, we added a i915_gem_proto_context struct which is capable of storing and tracking all such create parameters. This commit delays the creation of the actual context until after the client is done configuring it with SET_CONTEXT_PARAM. From the perspective of the client, it has the same u32 context ID the whole time. From the perspective of i915, however, it's an i915_gem_proto_context right up until the point where we attempt to do something which the proto-context can't handle. Then the real context gets created. This is accomplished via a little xarray dance. When GEM_CONTEXT_CREATE is called, we create a proto-context, reserve a slot in context_xa but leave it NULL, the proto-context in the corresponding slot in proto_context_xa. Then, whenever we go to look up a context, we first check context_xa. If it's there, we return the i915_gem_context and we're done. If it's not, we look in proto_context_xa and, if we find it there, we create the actual context and kill the proto-context. In order for this dance to work properly, everything which ever touches a proto-context is guarded by drm_i915_file_private::proto_context_lock, including context creation. Yes, this means context creation now takes a giant global lock but it can't really be helped and that should never be on any driver's fast-path anyway. v2 (Daniel Vetter): - Commit message grammatical fixes. - Use WARN_ON instead of GEM_BUG_ON - Rename lazy_create_context_locked to finalize_create_context_locked - Rework the control-flow logic in the setparam ioctl - Better documentation all around v3 (kernel test robot): - Make finalize_create_context_locked static Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-25-jason@jlekstrand.net
2021-07-08drm/i915/gt: Drop i915_address_space::file (v2)Jason Ekstrand
There's a big comment saying how useful it is but no one is using this for anything anymore. It was added in 2bfa996e031b ("drm/i915: Store owning file on the i915_address_space") and used for debugfs at the time as well as telling the difference between the global GTT and a PPGTT. In f6e8aa387171 ("drm/i915: Report the number of closed vma held by each context in debugfs") we removed one use of it by switching to a context walk and comparing with the VM in the context. Finally, VM stats for debugfs were entirely nuked in db80a1294c23 ("drm/i915/gem: Remove per-client stats from debugfs/i915_gem_objects") v2 (Daniel Vetter): - Delete a struct drm_i915_file_private pre-declaration - Add a comment to the commit message about history Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-24-jason@jlekstrand.net
2021-07-08drm/i915/gem: Return an error ptr from context_lookupJason Ekstrand
We're about to start doing lazy context creation which means contexts get created in i915_gem_context_lookup and we may start having more errors than -ENOENT. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-23-jason@jlekstrand.net
2021-07-08drm/i915/gem: Use the proto-context to handle create parameters (v5)Jason Ekstrand
This means that the proto-context needs to grow support for engine configuration information as well as setparam logic. Fortunately, we'll be deleting a lot of setparam logic on the primary context shortly so it will hopefully balance out. There's an extra bit of fun here when it comes to setting SSEU and the way it interacts with PARAM_ENGINES. Unfortunately, thanks to SET_CONTEXT_PARAM and not being allowed to pick the order in which we handle certain parameters, we have think about those interactions. v2 (Daniel Vetter): - Add a proto_context_free_user_engines helper - Comment on SSEU in the commit message - Use proto_context_set_persistence in set_proto_ctx_param v3 (Daniel Vetter): - Fix a doc comment - Do an explicit HAS_FULL_PPGTT check in set_proto_ctx_vm instead of relying on pc->vm != NULL. - Handle errors for CONTEXT_PARAM_PERSISTENCE - Don't allow more resetting user engines - Rework initialization of UCONTEXT_PERSISTENCE v4 (Jason Ekstrand): - Move hand-rolled initialization of UCONTEXT_PERSISTENCE to an earlier patch v5 (Jason Ekstrand): - Move proto_context_set_persistence to this patch Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-22-jason@jlekstrand.net
2021-07-08drm/i915/gem: Make an alignment check more sensibleJason Ekstrand
What we really want to check is that size of the engines array, i.e. args->size - sizeof(*user) is divisible by the element size, i.e. sizeof(*user->engines) because that's what's required for computing the array length right below the check. However, we're currently not doing this and instead doing a compile-time check that sizeof(*user) is divisible by sizeof(*user->engines) and avoiding the subtraction. As far as I can tell, the only reason for the more confusing pair of checks is to avoid a single subtraction of a constant. The other thing the BUILD_BUG_ON might be trying to implicitly check is that offsetof(user->engines) == sizeof(*user) and we don't have any weird padding throwing us off. However, that's not the check it's doing and it's not even a reliable way to do that check. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-21-jason@jlekstrand.net
2021-07-08drm/i915: Add an i915_gem_vm_lookup helperJason Ekstrand
This is the VM equivalent of i915_gem_context_lookup. It's only used once in this patch but future patches will need to duplicate this lookup code so it's better to have it in a helper. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-20-jason@jlekstrand.net
2021-07-08drm/i915/gem: Optionally set SSEU in intel_context_set_gemJason Ekstrand
For now this is a no-op because everyone passes in a null SSEU but it lets us get some of the error handling and selftest refactoring plumbed through. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-19-jason@jlekstrand.net
2021-07-08drm/i915/gem: Rework error handling in default_enginesJason Ekstrand
Since free_engines works for partially constructed engine sets, we can use the usual goto pattern. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-18-jason@jlekstrand.net
2021-07-08drm/i915/gem: Add an intermediate proto_context struct (v5)Jason Ekstrand
The current context uAPI allows for two methods of setting context parameters: SET_CONTEXT_PARAM and CONTEXT_CREATE_EXT_SETPARAM. The former is allowed to be called at any time while the later happens as part of GEM_CONTEXT_CREATE. Currently, everything settable via one is settable via the other. While some params are fairly simple and setting them on a live context is harmless such the context priority, others are far trickier such as the VM or the set of engines. In order to swap out the VM, for instance, we have to delay until all current in-flight work is complete, swap in the new VM, and then continue. This leads to a plethora of potential race conditions we'd really rather avoid. Unfortunately, both methods of setting the VM and the engine set are in active use today so we can't simply disallow setting the VM or engine set vial SET_CONTEXT_PARAM. In order to work around this wart, this commit adds a proto-context struct which contains all the context create parameters. v2 (Daniel Vetter): - Better commit message - Use __set/clear_bit instead of set/clear_bit because there's no race and we don't need the atomics v3 (Daniel Vetter): - Use manual bitops and BIT() instead of __set_bit v4 (Daniel Vetter): - Add a changelog to the commit message - Better hyperlinking in docs - Create the default PPGTT in i915_gem_create_context v5 (Daniel Vetter): - Hand-roll the initialization of UCONTEXT_PERSISTENCE Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-17-jason@jlekstrand.net
2021-07-08drm/i915: Add gem/i915_gem_context.h to the docsJason Ekstrand
In order to prevent kernel doc warnings, also fill out docs for any missing fields and fix those that forgot the "@". Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-16-jason@jlekstrand.net
2021-07-08drm/i915/gem: Add a separate validate_priority helperJason Ekstrand
With the proto-context stuff added later in this series, we end up having to duplicate set_priority. This lets us avoid duplicating the validation logic. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-15-jason@jlekstrand.net
2021-07-08drm/i915: Stop manually RCU banging in reset_stats_ioctl (v2)Jason Ekstrand
As far as I can tell, the only real reason for this is to avoid taking a reference to the i915_gem_context. The cost of those two atomics probably pales in comparison to the cost of the ioctl itself so we're really not buying ourselves anything here. We're about to make context lookup a tiny bit more complicated, so let's get rid of the one hand- rolled case. Some usermode drivers such as our Vulkan driver call GET_RESET_STATS on every execbuf so the perf here could theoretically be an issue. If this ever does become a performance issue for any such userspace drivers, they can use set CONTEXT_PARAM_RECOVERABLE to false and look for -EIO coming from execbuf to check for hangs instead. v2 (Daniel Vetter): - Add a comment in the commit message about recoverable contexts Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-14-jason@jlekstrand.net
2021-07-08drm/i915/gem: Disallow creating contexts with too many enginesJason Ekstrand
There's no sense in allowing userspace to create more engines than it can possibly access via execbuf. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-13-jason@jlekstrand.net
2021-07-08drm/i915/request: Remove the hook from await_executionJason Ekstrand
This was only ever used for FENCE_SUBMIT automatic engine selection which was removed in the previous commit. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-12-jason@jlekstrand.net
2021-07-08drm/i915/gem: Remove engine auto-magic with FENCE_SUBMIT (v2)Jason Ekstrand
Even though FENCE_SUBMIT is only documented to wait until the request in the in-fence starts instead of waiting until it completes, it has a bit more magic than that. If FENCE_SUBMIT is used to submit something to a balanced engine, we would wait to assign engines until the primary request was ready to start and then attempt to assign it to a different engine than the primary. There is an IGT test (the bonded-slice subtest of gem_exec_balancer) which exercises this by submitting a primary batch to a specific VCS and then using FENCE_SUBMIT to submit a secondary which can run on any VCS and have i915 figure out which VCS to run it on such that they can run in parallel. However, this functionality has never been used in the real world. The media driver (the only user of FENCE_SUBMIT) always picks exactly two physical engines to bond and never asks us to pick which to use. v2 (Daniel Vetter): - Mention the exact IGT test this breaks Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-11-jason@jlekstrand.net
2021-07-08drm/i915/gem: Disallow bonding of virtual engines (v3)Jason Ekstrand
This adds a bunch of complexity which the media driver has never actually used. The media driver does technically bond a balanced engine to another engine but the balanced engine only has one engine in the sibling set. This doesn't actually result in a virtual engine. This functionality was originally added to handle cases where we may have more than two video engines and media might want to load-balance their bonded submits by, for instance, submitting to a balanced vcs0-1 as the primary and then vcs2-3 as the secondary. However, no such hardware has shipped thus far and, if we ever want to enable such use-cases in the future, we'll use the up-and-coming parallel submit API which targets GuC submission. This makes I915_CONTEXT_ENGINES_EXT_BOND a total no-op. We leave the validation code in place in case we ever decide we want to do something interesting with the bonding information. v2 (Jason Ekstrand): - Don't delete quite as much code. v3 (Tvrtko Ursulin): - Add some history to the commit message Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-10-jason@jlekstrand.net
2021-07-08drm/i915: Drop getparam support for I915_CONTEXT_PARAM_ENGINESJason Ekstrand
This has never been used by any userspace except IGT and provides no real functionality beyond parroting back parameters userspace passed in as part of context creation or via setparam. If the context is in legacy mode (where you use I915_EXEC_RENDER and friends), it returns success with zero data so it's not useful for discovering what engines are in the context. It's also not a replacement for the recently removed I915_CONTEXT_CLONE_ENGINES because it doesn't return any of the balancing or bonding information. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-9-jason@jlekstrand.net
2021-07-08drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4)Jason Ekstrand
This API is entirely unnecessary and I'd love to get rid of it. If userspace wants a single timeline across multiple contexts, they can either use implicit synchronization or a syncobj, both of which existed at the time this feature landed. The justification given at the time was that it would help GL drivers which are inherently single-timeline. However, neither of our GL drivers actually wanted the feature. i965 was already in maintenance mode at the time and iris uses syncobj for everything. Unfortunately, as much as I'd love to get rid of it, it is used by the media driver so we can't do that. We can, however, do the next-best thing which is to embed a syncobj in the context and do exactly what we'd expect from userspace internally. This isn't an entirely identical implementation because it's no longer atomic if userspace races with itself by calling execbuffer2 twice simultaneously from different threads. It won't crash in that case; it just doesn't guarantee any ordering between those two submits. It also means that sync files exported from different engines on a SINGLE_TIMELINE context will have different fence contexts. This is visible to userspace if it looks at the obj_name field of sync_fence_info. Moving SINGLE_TIMELINE to a syncobj emulation has a couple of technical advantages beyond mere annoyance. One is that intel_timeline is no longer an api-visible object and can remain entirely an implementation detail. This may be advantageous as we make scheduler changes going forward. Second is that, together with deleting the CLONE_CONTEXT API, we should now have a 1:1 mapping between intel_context and intel_timeline which may help us reduce locking. v2 (Tvrtko Ursulin): - Update the comment on i915_gem_context::syncobj to mention that it's an emulation and the possible race if userspace calls execbuffer2 twice on the same context concurrently. v2 (Jason Ekstrand): - Wrap the checks for eb.gem_context->syncobj in unlikely() - Drop the dma_fence reference - Improved commit message v3 (Jason Ekstrand): - Move the dma_fence_put() to before the error exit v4 (Tvrtko Ursulin): - Add a comment about fence contexts to the commit message Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-8-jason@jlekstrand.net
2021-07-08drm/i915: Drop the CONTEXT_CLONE API (v2)Jason Ekstrand
This API allows one context to grab bits out of another context upon creation. It can be used as a short-cut for setparam(getparam()) for things like I915_CONTEXT_PARAM_VM. However, it's never been used by any real userspace. It's used by a few IGT tests and that's it. Since it doesn't add any real value (most of the stuff you can CLONE you can copy in other ways), drop it. There is one thing that this API allows you to clone which you cannot clone via getparam/setparam: timelines. However, timelines are an implementation detail of i915 and not really something that needs to be exposed to userspace. Also, sharing timelines between contexts isn't obviously useful and supporting it has the potential to complicate i915 internally. It also doesn't add any functionality that the client can't get in other ways. If a client really wants a shared timeline, they can use a syncobj and set it as an in and out fence on every submit. v2 (Jason Ekstrand): - More detailed commit message Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-7-jason@jlekstrand.net
2021-07-08drm/i915/gem: Return void from context_apply_allJason Ekstrand
None of the callbacks we use with it return an error code anymore; they all return 0 unconditionally. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20210708154835.528166-6-jason@jlekstrand.net