- Jan 21, 2021
-
-
Chris Wilson authored
Now that we are careful to always force-restore contexts upon rewinding (where necessary), we can restore our optimisation to skip over completed active execlists when dequeuing. Referenecs: 35f3fd81 ("drm/i915/execlists: Workaround switching back to a completed context") References: 8ab3a381 ("drm/i915/gt: Incrementally check for rewinding") 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/20210120121718.26435-2-chris@chris-wilson.co.uk
-
Chris Wilson authored
Treat the dependency between bonded requests as weak and leave the remainder of the pair on the GPU if one hangs. Signed-off-by:
-
Chris Wilson authored
Simplify the frontbuffer unpin by removing the lock requirement. The LRU bumping was primarily to protect the GTT from being evicted and from frontbuffers being eagerly shrunk. Now we protect frontbuffers from the shrinker, and we avoid accidentally evicting from the GTT, so the benefit from bumping LRU is no more, and we can save more time by not. Signed-off-by:
Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210119214336.1463-6-chris@chris-wilson.co.uk
-
Chris Wilson authored
In the shrinker, we protect framebuffers from light reclaim as we typically expect framebuffers to be reused in the near future (and with low latency requirements). We can apply the same logic to the GGTT eviction and defer framebuffers to the second pass only used if the caller is desperate enough to wait for space to become available. In most cases, the caller will use a smaller partial vma instead of trying to force the object into the GGTT if doing so will cause other users to be evicted. Signed-off-by:
-
Chris Wilson authored
Give obj->mm.quirked a name much more reflective of its purpose (i915_gem_object_has_tiling_quirk) and move it from the obj->mm field as it doesn't denote a quirk of the backing store, but a quirk in the object in its treatment of the backing pages, similar to tiling modes. Then instead of abusing the pinned status of the buffer to protect it from the shrinker, we can instead hide the buffer from the shrinker so it is never considered for being swapped. Signed-off-by:
-
Chris Wilson authored
The obj->stolen is currently used to identify an object allocated from stolen memory. This dates back to when there were just 1.5 types of objects, an object backed by shmemfs and an object backed by shmemfs with a contiguous physical address. Now that we have several different types of objects, we no longer want to treat stolen objects as a special case. Signed-off-by:
-
Chris Wilson authored
When flushing objects larger than the CPU cache it is preferrable to use a single wbinvd() rather than overlapping clflush(). At runtime, we avoid wbinvd() due to its system-wide latencies, but during singlethreaded suspend, no one will observe the imposed latency and we can opt for the faster wbinvd to clear all objects in a single hit. Signed-off-by:
-
Chris Wilson authored
When flushing objects larger than the CPU cache it is preferrable to use a single wbinvd() rather than overlapping clflush(). At runtime, we avoid wbinvd() due to its system-wide latencies, but during singlethreaded suspend, no one will observe the imposed latency and we can opt for the faster wbinvd to clear all objects in a single hit. Signed-off-by:
-
- Jan 20, 2021
-
-
Matthew Auld authored
Sanity check the object size before allocating a new gem object. Fixes: 97d55396 ("drm/i915/region: convert object_create into object_init") Testcase: igt/gem_create/create-massive Signed-off-by:
-
John Harrison authored
There is a module parameter for controlling what GuC/HuC features are enabled. Setting to -1 means 'use the default'. However, the default was not well defined, out of date and needs to be different across platforms. The default is now to disable both GuC and HuC on legacy platforms where legacy means TGL/RKL and anything prior to Gen12. For new platforms, the default is to load HuC but not enable GuC submission as that has not landed yet. Signed-off-by:
John Harrison <John.C.Harrison@Intel.com> Reviewed-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Signed-off-by:
-
Chris Wilson authored
If we enable_breadcrumbs for a request while that request is being removed from HW; we may see that the request is active as we take the ce->signal_lock and proceed to attach the request to ce->signals. However, during unsubmission after marking the request as inactive, we see that the request has not yet been added to ce->signals and so skip the removal. Pull the check during cancel_breadcrumbs under the same spinlock as enabling so that we the two tests are consistent in enable/cancel. Otherwise, we may insert a request onto ce->signals that we expect should not be there: intel_context_remove_breadcrumbs:488 GEM_BUG_ON(!__i915_request_is_complete(rq)) While updating, we can note that we are always called with irqs-disabled, due to the engine->active.lock being held at the single caller, and so remove the irqsave/restore making it symmetric to enable_breadcrumbs. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2931 Fixes: c18636f7 ("drm/i915: Remove requirement for holding i915_request.lock for breadcrumbs") Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210119162057.31097-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
Currently, if a modeset/pageflip needs to wait for render completion to an object, we boost the priority of that rendering above all other work. We can apply the same interactive priority boosting to explicit fences that we can unwrap into a native i915_request (i.e. sync_file). Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210119204454.10343-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
flush_write_domain() is only used within the GEM domain management code, so move it to i915_gem_domain.c and drop the export. Signed-off-by:
-
Matthew Auld authored
In a few places we always end up mapping the pool object with the FORCE constraint(to prevent hitting -EBUSY) which will destroy the cached mapping if it has a different type. As a simple first step, make the mapping type part of the pool interface, where the behaviour is to only give out pool objects which match the requested mapping type. Suggested-by:
-
Matthew Auld authored
Looks like it belongs there anyway, otherwise we have to include the entirety of i915_gem_object.h just to get at the enum. Signed-off-by:
-
CQ Tang authored
io_mapping_map_wc() expects the offset to be relative to the iomapping base address. Currently we just pass in the physical address for the page which only works if the region.start starts at zero. Signed-off-by:
CQ Tang <cq.tang@intel.com> Signed-off-by:
-
Kui Wen authored
For the device local-memory case, sgt.pfn will always be equal to zero, since we instead use sgt.dma. Also, for device local-memory it is perfectly valid for it to start from zero anyway, so no need to add a new check for that either. Signed-off-by:
Kui Wen <kui.wen@intel.com> Signed-off-by:
-
- Jan 19, 2021
-
-
Chris Wilson authored
Rather than trying to avoid the use-after-free possible with the current context chasing, simply remove presentation of the per-client stats from debugfs. While we know from bug reports that this debugfs/i915_gem_objects has been used by chromeos (and chrome itself) for debug purposes, google suggests that it is unparsed, so we are free to invoke debugfs is not ABI and remove details from it. Suggested-by:
-
Chris Wilson authored
The benefit of only resetting a single engine is that we leave other streams of userspace work intact across a hang; vital for process isolation. We had wired up individual engine resets for gen6, but only enabled it from gen8; now let's turn it on for the forgotten gen7. gen6 is still a mystery as how to unravel some global state that appears to be reset along with an engine (in particular the ppgtt enabling in GFX_MODE). Signed-off-by:
-
Chris Wilson authored
Take advantage of calling xcs_resume under a forcewake by using direct mmio access. In particular, we can avoid the sleeping variants to allow resume to be called from softirq context, required for engine resets. v2: Keep the posting read at the start of resume as a guardian memory barrier. Signed-off-by:
-
Chris Wilson authored
During the reset of ring submission, we first stop the engine by clearing the HEAD/TAIL and marking the ring as disabled. However, it would be safer to disable the ring (after emptying) before resetting the HEAD/TAIL. Suggested-by:
-
Chris Wilson authored
Push the sleeping stop_ring() out of the reset resume function to reset prepare; we are not allowed to sleep in the former. Signed-off-by:
-
Chris Wilson authored
The engine resets selftests kick the tasklets, safe up until now as only execlists supported engine resets. Signed-off-by:
-
Chris Wilson authored
CI reports that Baytail requires one more invalidate after CACHE_MODE for it to be happy. Fixes: ace44e13 ("drm/i915/gt: Clear CACHE_MODE prior to clearing residuals") Signed-off-by:
Akeem G Abodunrin <akeem.g.abodunrin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210119110802.22228-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
While immensely convenient for developing to only tackle the first error, and not be flooded by repeated or secondiary issues, many more casual testers are not setup to remotely capture debug traces. For those testers, it is more beneficial to keep the system running in the remote chance that they are able to extract the original debug logs. Signed-off-by:
-
Chris Wilson authored
Now that i915 compiles cleanly with Werror, we can enforce enabling DEBUG_GEM when selecting the default debug config. Signed-off-by:
-
- Jan 18, 2021
-
-
Chris Wilson authored
Similar to commit 49b20dbf ("drm/i915/gt: Perform an arbitration check before busywaiting"), also add a check prior to the busywait on gen8+, as we have now seen (because we added a selftest to add fault injection into the engine resets) the same engine reset failure leading to an indefinite wait on the ring-stop semaphore. So not a Tigerlake specific bug after all, though it still seems odd behaviour for the busywait as we do get the arbitration point elsewhere on a miss. Testcase: igt_reset_fail_engine Signed-off-by:
-
Chris Wilson authored
Since we do a bare context switch with no restore, the clear residual kernel runs on dirty state, and we must be careful to avoid executing with bad state from context registers inherited from a malicious client. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2955 Fixes: 008ead6e ("drm/i915/gt: Restore clear-residual mitigations for Ivybridge, Baytrail") Fixes: 09aa9e45 ("drm/i915/gt: Restore clear-residual mitigations for Ivybridge, Baytrail") Testcase: igt/gem_ctx_isolation # ivb,vlv Signed-off-by:
-
- Jan 16, 2021
-
-
Chris Wilson authored
As context-in/out is now always serialised, we do not have to worry about concurrent enabling/disable of the busy-stats and can reduce the atomic_t active to a plain unsigned int, and the seqlock to a seqcount. Signed-off-by:
Andi Shyti <andi.shyti@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210115142331.24458-3-chris@chris-wilson.co.uk
-
Chris Wilson authored
Lift the busy-stats context-in/out implementation out of intel_lrc, so that we can reuse it for other scheduler implementations. Signed-off-by:
-
Chris Wilson authored
Since schedule-in/out is now entirely serialised by the tasklet bitlock, we do not need to worry about concurrent in/out operations and so reduce the atomic operations to plain instructions. Signed-off-by:
-
- Jan 15, 2021
-
-
Matthew Auld authored
warning: symbol '__i915_gem_object_create_stolen' was not declared. Should it be static? warning: symbol '_i915_gem_object_stolen_init' was not declared. Should it be static? Signed-off-by:
-
Matthew Auld authored
Give more flexibility to the caller, if they already have an allocated object, in case they wish to apply some transformation to the object prior to handing it over to the region specific initialisation step, like in gem_create_ext where we would like to first apply the extensions to the object. Signed-off-by:
-
Matthew Auld authored
Depending on the regions min_page_size we might need to adjust the object size, ensure this matches our expectations. Suggested-by:
-
Matthew Auld authored
In preparation for gem_create_ext break out the gem_create uAPI, so that we don't clutter i915_gem.c once we start adding various extensions Signed-off-by:
-
Chris Wilson authored
Avoid the full blown memory barrier of test_and_set_bit() by noting the completed request and removing it from the lists. Signed-off-by:
-
Chris Wilson authored
Take a snapshot of the ctx->engines, so we can avoid taking the ctx->engines_mutex for a mere read in get_engines(). Signed-off-by:
-
Chris Wilson authored
When cloning the engines from the source context, we need to ensure that the engines are not freed as we copy them, and that the flags we clone from the source correspond with the engines we copy across. To do this we need only take a reference to the src->engines, rather than hold the src->engine_mutex, so long as we verify that nothing changed under the read. Signed-off-by:
-
Chris Wilson authored
Originally, we used the signal->lock as a means of following the previous link in its timeline and peeking at the previous fence. However, we have replaced the explicit serialisation with a series of very careful probes that anticipate the links being deleted and the fences recycled before we are able to acquire a strong reference to it. We do not need the signal->lock crutch anymore, nor want the contention. Signed-off-by:
-
Chris Wilson authored
When we know that we are inside the timeline mutex, or inside the submission flow (under active.lock or the holder's rcu lock), we know that the rq->hwsp is stable and we can use the simpler direct version. Signed-off-by:
-
