summaryrefslogtreecommitdiffstats
path: root/drivers/gpu/drm/i915/i915_gem.c
Commit message (Collapse)AuthorAgeFilesLines
...
* drm/i915: Wait for engines to idle before retiringChris Wilson2018-06-271-4/+7
| | | | | | | | | | | | In the next^W forthcoming patch, we will start to defer retiring the request from the engine list if it is still active on the submission backend. To preserve the semantics that after wait-for-idle completes the system is idle and fully retired, we need to therefore wait for the backends to idle before calling i915_retire_requests(). 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/20180627115334.16282-1-chris@chris-wilson.co.uk
* drm/i915: Only show debug for state changes when banningChris Wilson2018-06-181-5/+5
| | | | | | | | | | | | Since we trigger 10,000s of hangs and resets during selftesting, we emit many, many thousands of lines of useless debug messages. Reduce the frequency by only logging a change in state of a guilty context. Fixes: 14921f3cef85 ("drm/i915: Fix context ban and hang accounting for client") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180618073135.10849-1-chris@chris-wilson.co.uk
* drm/i915: Fix fallout of fake reset along resumeChris Wilson2018-06-181-9/+5
| | | | | | | | | | | | | | | | | | | | | | | | commit b2209e62a450 ("drm/i915/execlists: Reset the CSB head tracking on reset/sanitization") and commit 1288786b18f7 ("drm/i915: Move GEM sanitize from resume_early to resume") show the conflicting requirements on the code. We must reset the GPU before trashing live state on a fast resume (hibernation debug, or error paths), but we must only reset our state tracking iff the GPU is reset (or power cycled). This is tricky if we are disabling GPU reset to simulate broken hardware; we reset our state tracking but the GPU is left intact and recovers from its stale state. v2: Again without the assertion for forcewake, no longer required since commit b3ee09a4de33 ("drm/i915/ringbuffer: Fix context restore upon reset") as the contexts are reset from the CS ensuring everything is powered up. Fixes: b2209e62a450 ("drm/i915/execlists: Reset the CSB head tracking on reset/sanitization") Fixes: 1288786b18f7 ("drm/i915: Move GEM sanitize from resume_early to resume") 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: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180616202534.18767-1-chris@chris-wilson.co.uk
* drm/i915: Be irqsafe inside resetChris Wilson2018-06-151-2/+4
| | | | | | | | | | As we want to be able to call i915_reset_engine and co from a softirq or timer context, we need to be irqsafe at all times. So we have to forgo the simple spin_lock_irq for the full spin_lock_irqsave. 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/20180615093137.14270-1-chris@chris-wilson.co.uk
* drm/i915: Fix context ban and hang accounting for clientMika Kuoppala2018-06-151-17/+40
| | | | | | | | | | | | | | | | | | | | | | | If client is smart or lucky enough to create a new context after each hang, our context banning mechanism will never catch up, and as a result of that it will be saved from client banning. This can result in a never ending streak of gpu hangs caused by bad or malicious client, preventing access from other legit gpu clients. Fix this by always incrementing per client ban score if it hangs in short successions regardless of context ban scoring. The exception are non bannable contexts. They remain detached from client ban scoring mechanism. v2: xchg timestamp, tidyup (Chris) v3: comment, bannable & banned together (Chris) Fixes: b083a0870c79 ("drm/i915: Add per client max context ban limit") Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20180615104429.31477-1-mika.kuoppala@linux.intel.com
* drm/i915: Make closing request flush mandatoryChris Wilson2018-06-141-2/+2
| | | | | | | | | | | | | | For symmetry, simplicity and ensuring the request is always truly idle upon its completion, always emit the closing flush prior to emitting the request breadcrumb. Previously, we would only emit the flush if we had started a user batch, but this just leaves all the other paths open to speculation (do they affect the GPU caches or not?) With mm switching, a key requirement is that the GPU is flushed and invalidated before hand, so for absolute safety, we want that closing flush be mandatory. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180612105135.4459-1-chris@chris-wilson.co.uk
* drm/i915: Refactor unsettting obj->mm.pagesChris Wilson2018-06-111-31/+37
| | | | | | | | | | | | | As i915_gem_object_phys_attach() wants to play dirty and mess around with obj->mm.pages itself (replacing the shmemfs with a DMA allocation), refactor the gubbins so into i915_gem_object_unset_pages() that we don't have to duplicate all the secrets. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180611075532.26534-1-chris@chris-wilson.co.uk Link: https://patchwork.freedesktop.org/patch/msgid/152871104647.1718.8796913290418060204@jlahtine-desk.ger.corp.intel.com
* drm/i915: Squash GEM load failure message (again)Chris Wilson2018-06-111-3/+2
| | | | | | | | | | | | | | | | | Due to a silent conflict (silent because we are trying to fix the CI test that is meant to exercising these failures!) between commit 51e645b6652c ("drm/i915: Mark the GPU as wedged without error on fault injection") and commit 8571a05a9dd0 ("drm/i915: Use GEM suspend when aborting initialisation"), we failed to actually squash the error message after injecting the load failure. Rearrange the code to export i915_load_failure() for better logging of real errors (and quiet logging of injected errors). Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michał Winiarski <michal.winiarski@intel.com> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180609111058.2660-1-chris@chris-wilson.co.uk
* drm/i915: Mark the GPU as wedged without error on fault injectionChris Wilson2018-06-081-0/+2
| | | | | | | | | | | If we have been instructed (by CI) to inject a fault to load the module with a wedged GPU, do so quietly less we upset CI. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michał Winiarski <michal.winiarski@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180607134558.31150-1-chris@chris-wilson.co.uk
* drm/i915: Change i915_gem_fault() to return vm_fault_tChris Wilson2018-06-071-14/+7
| | | | | | | | | | | | | | | In preparation for vm_fault_t becoming a distinct type, convert the fault handler (i915_gem_fault()) over to the new interface. Based on a patch by Souptick Joarder References: 1c8f422059ae ("mm: change return type to vm_fault_t") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Souptick Joarder <jrdr.linux@gmail.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Matthew Auld <matthew.william.auld@gmail.com> Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180606214520.20220-1-chris@chris-wilson.co.uk
* drm/i915: Use GEM suspend when aborting initialisationChris Wilson2018-06-071-2/+6
| | | | | | | | | | | | | | | | | As part of our GEM initialisation now, we send a request to the hardware in order to record the initial GPU state. This coupled with deferred idle workers, makes aborting on error tricky. We already have the mechanism in place to wait on the GPU and cancel all the deferred workers for suspend, so let's reuse it during the error teardown. It is already used in places for later init error handling, but doing so at this point is slightly ugly due to the mutex dance (it's ok, the module load is still single threaded). Testcase: igt/drv_module_reload/basic-reload-inject Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michał Winiarski <michal.winiarski@intel.com> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180606145441.4460-1-chris@chris-wilson.co.uk
* drm/i915/gtt: Rename i915_hw_ppgtt base memberChris Wilson2018-06-051-16/+14
| | | | | | | | | | | | | | | In the near future, I want to subclass gen6_hw_ppgtt as it contains a few specialised members and I wish to add more. To avoid the ugliness of using ppgtt->base.base, rename the i915_hw_ppgtt base member (i915_address_space) as vm, which is our common shorthand for an i915_address_space local. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Matthew Auld <matthew.william.auld@gmail.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180605153758.18422-1-chris@chris-wilson.co.uk
* drm/i915: Swap magics and use SZ_1MChris Wilson2018-06-051-1/+1
| | | | | | | | Since the kernel provides SZ_1M, use it in preference of 1 << 20. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180605135746.8020-1-chris@chris-wilson.co.uk
* drm/i915: Correctly handle error path in i915_gem_init_hwMichal Wajdeczko2018-06-051-0/+6
| | | | | | | | | | | | | | In function gem_init_hw() we are calling uc_init_hw() but in case of error later in function, we missed to call matching uc_fini_hw() v2: pulled out from the series Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20180605122443.23776-1-michal.wajdeczko@intel.com
* drm/i915: Add new vGPU cap info bit VGT_CAPS_HUGE_GTTChangbin Du2018-06-051-6/+2
| | | | | | | | | | | This adds a new vGPU cap info bit VGT_CAPS_HUGE_GTT, which is to detect whether the host supports shadowing of huge gtt pages. If host does support it, remove the page sizes restriction for vGPU. Signed-off-by: Changbin Du <changbin.du@intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1525770425-5373-1-git-send-email-changbin.du@intel.com
* drm/i915: Move i915_gem_fini to i915_gem.cMichal Wajdeczko2018-06-041-0/+22
| | | | | | | | | | | | | | We should keep i915_gem_init/fini functions together for easier tracking of their symmetry. v2: rebased, pulled out from the series Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20180604090032.20840-1-michal.wajdeczko@intel.com
* drm/i915: Apply the full CPU domain markup before freezingChris Wilson2018-06-011-12/+13
| | | | | | | | | | | | | | | | Let's not take any chances by using a shortcut to mark the objects as in the CPU domain upon freezing (all pages will be written to disk and so on restore all objects will start from the CPU domain). Currently, we simply mark the objects as being in the CPU domain, bypassing the flushes. Let's call the full domain transfer function so that we have less special case code (and symmetry with the suspend path) even though it will be mostly redundant. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180601144125.18026-2-chris@chris-wilson.co.uk
* drm/i915: Flush all writes before suspendChris Wilson2018-06-011-0/+14
| | | | | | | | | | | | | | | | | As we have already suspended the device, this should be a no-op except for marking that all writes are indeed complete. The downside is that we then have to walk all the lists of objects for what should be a no-op (in some cases they will be mmio read to ensure the GGTT writes are indeed flushed, and clflushes to ensure that cpu writes are in memory). It seems prudent and the safer course for us to ensure all writes are flushed to memory before suspend. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180601144125.18026-1-chris@chris-wilson.co.uk
* drm/i915: Assert we idle in the kernel contextChris Wilson2018-06-011-13/+18
| | | | | | | | | | | Now that we always switch to the kernel context upon idling, we can make that assertion. References: 4dfacb0bcbee ("drm/i915: Switch to kernel context before idling at runtime") 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/20180531224057.6036-1-chris@chris-wilson.co.uk
* drm/i915: Only sanitize GEM from late suspendChris Wilson2018-05-311-9/+13
| | | | | | | | | | | | | During testing we encounter a conflict between SUSPEND_TEST_DEVICES and disabling reset (gem_eio/suspend). This results in the device continuing on without being reset, but since it has gone through HW sanitization to account for the suspend/resume cycle, we have to assume the device has been reset to its defaults. A simple way around this is to skip the sanitize phase for SUSPEND_TEST_DEVICES by moving it to suspend-late. 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/20180531082246.9763-4-chris@chris-wilson.co.uk
* drm/i915: After reset on sanitization, reset the engine backendsChris Wilson2018-05-311-0/+24
| | | | | | | | | | | | As we reset the GPU on suspend/resume, we also do need to reset the engine state tracking so call into the engine backends. This is especially important so that we can also sanitize the state tracking across resume. References: https://bugs.freedesktop.org/show_bug.cgi?id=106702 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180531082246.9763-3-chris@chris-wilson.co.uk
* drm/i915: "Race-to-idle" after switching to the kernel contextChris Wilson2018-05-311-2/+25
| | | | | | | | | | | | | | | | | | | | | | During suspend we want to flush out all active contexts and their rendering. To do so we queue a request from the kernel's context, once we know that request is done, we know the GPU is completely idle. To speed up that switch bump the GPU clocks. Switching to the kernel context prior to idling is also used to enforce a barrier before changing OA properties, and when evicting active rendering from the global GTT. All cases where we do want to race-to-idle. v2: Limit the boosting to only the switch before suspend. v3: Limit it to the wait-for-idle on suspend. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: David Weinehall <david.weinehall@linux.intel.com> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Tested-by: David Weinehall <david.weinehall@linux.intel.com> #v1 Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180531082246.9763-2-chris@chris-wilson.co.uk
* drm/i915: Switch to kernel context before idling at runtimeChris Wilson2018-05-311-5/+31
| | | | | | | | | | | | We can reduce our exposure to random neutrinos by resting on the kernel context having flushed out the user contexts to system memory and beyond. The corollary is that we then we require two passes through the idle handler to go to sleep, which on a truly idle system involves an extra pass through the slow and irregular retire work handler. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180531082246.9763-1-chris@chris-wilson.co.uk
* drm/i915: Remove stale asserts from i915_gem_find_active_request()Chris Wilson2018-05-301-9/+8
| | | | | | | | | | | | | | | | | | | Since we use i915_gem_find_active_request() from inside intel_engine_dump() and may call that at any time, we do not guarantee that the engine is paused nor that the signal kthreads and irq handler are suspended, so we cannot assert that the breadcrumb doesn't advance and that the irq hasn't happened on another CPU signaling the request we believe to be idle. The second assert removed (that request->engine == engine) remains valid, but is now more rigorously checked during retirement. Fixes: f636edb214a5 ("drm/i915: Make i915_engine_info pretty printer to standalone") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180529132922.6831-1-chris@chris-wilson.co.uk Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
* drm/i915: Look for an active kernel context before switchingChris Wilson2018-05-241-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We were not very carefully checking to see if an older request on the engine was an earlier switch-to-kernel-context before deciding to emit a new switch. The end result would be that we could get into a permanent loop of trying to emit a new request to perform the switch simply to flush the existing switch. What we need is a means of tracking the completion of each timeline versus the kernel context, that is to detect if a more recent request has been submitted that would result in a switch away from the kernel context. To realise this, we need only to look in our syncmap on the kernel context and check that we have synchronized against all active rings. v2: Since all ringbuffer clients currently share the same timeline, we do have to use the gem_context to distinguish clients. As a bonus, include all the tracing used to debug the death inside suspend. v3: Test, test, test. Construct a selftest to exercise and assert the expected behaviour that multiple switch-to-contexts do not emit redundant requests. Reported-by: Mika Kuoppala <mika.kuoppala@intel.com> Fixes: a89d1f921c15 ("drm/i915: Split i915_gem_timeline into individual timelines") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180524081135.15278-1-chris@chris-wilson.co.uk
* drm/i915: Store a pointer to intel_context in i915_requestChris Wilson2018-05-181-6/+6
| | | | | | | | | | | | | | | | | To ease the frequent and ugly pointer dance of &request->gem_context->engine[request->engine->id] during request submission, store that pointer as request->hw_context. One major advantage that we will exploit later is that this decouples the logical context state from the engine itself. v2: Set mock_context->ops so we don't crash and burn in selftests. Cleanups from Tvrtko. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Acked-by: Zhenyu Wang <zhenyuw@linux.intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180517212633.24934-3-chris@chris-wilson.co.uk
* drm/i915: Move request->ctx asideChris Wilson2018-05-181-5/+5
| | | | | | | | | | | | | | In the next patch, we want to store the intel_context pointer inside i915_request, as it is frequently access via a convoluted dance when submitting the request to hw. Having two context pointers inside i915_request leads to confusion so first rename the existing i915_gem_context pointer to i915_request.gem_context. 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> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180517212633.24934-1-chris@chris-wilson.co.uk
* drm/i915: Stop parking the signaler around resetChris Wilson2018-05-161-14/+0
| | | | | | | | | | | | | | | | | | | | We cannot call kthread_park() from softirq context, so let's avoid it entirely during the reset. We wanted to suspend the signaler so that it would not mark a request as complete at the same time as we marked it as being in error. Instead of parking the signaling, stop the engine from advancing so that the GPU doesn't emit the breadcrumb for our chosen "guilty" request. v2: Refactor setting STOP_RING so that we don't have the same code thrice Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Michałt Winiarski <michal.winiarski@intel.com> CC: Michel Thierry <michel.thierry@intel.com> Cc: Jeff McGee <jeff.mcgee@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180516183355.10553-8-chris@chris-wilson.co.uk
* drm/i915: Move engine reset prepare/finish to backendsChris Wilson2018-05-161-33/+5
| | | | | | | | | | | | | In preparation to more carefully handling incomplete preemption during reset by execlists, we move the existing code wholesale to the backends under a couple of new reset vfuncs. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michał Winiarski <michal.winiarski@intel.com> CC: Michel Thierry <michel.thierry@intel.com> Cc: Jeff McGee <jeff.mcgee@intel.com> Reviewed-by: Jeff McGee <jeff.mcgee@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180516183355.10553-4-chris@chris-wilson.co.uk
* drm/i915: Only sync tasklets once for recursive reset preparationChris Wilson2018-05-161-1/+1
| | | | | | | | | | | When setting up reset, we may need to recursively prepare an engine. In which case we should only synchronously flush the tasklets on the outer most call, the inner calls will then be inside an atomic section where the tasklet will never be run (and so the sync will never complete). 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/20180516183355.10553-2-chris@chris-wilson.co.uk
* drm/i915: Remove tasklet flush before disableChris Wilson2018-05-161-9/+0
| | | | | | | | | | | | | | | | The idea was to try and let the existing tasklet run to completion before we began the reset, but it involves a racy check against anything else that tries to run the tasklet. Rather than acknowledge and ignore the race, let it be and don't try and be too clever. The tasklet will resume execution after reset (after spinning a bit during reset), but before we allow it to resume we will have cleared all the pending state. 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/20180516183355.10553-1-chris@chris-wilson.co.uk
* drm/i915: Mark up nested spinlocksChris Wilson2018-05-141-1/+1
| | | | | | | | | | | | | | | | | When we process the outstanding requests upon banning a context, we need to acquire both the engine and the client's timeline, nesting the locks. This requires explicit markup as the two timelines are now of the same class, since commit a89d1f921c15 ("drm/i915: Split i915_gem_timeline into individual timelines"). Testcase: igt/gem_eio/banned Fixes: a89d1f921c15 ("drm/i915: Split i915_gem_timeline into individual timelines") 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: Michel Thierry <michel.thierry@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180512084957.9829-1-chris@chris-wilson.co.uk
* drm/i915: Flush submission tasklet after bumping priorityChris Wilson2018-05-081-1/+3
| | | | | | | | | | | | | | | | | When called from process context tasklet_schedule() defers itself to ksoftirqd. From experience this may cause unacceptable latencies of over 200ms in executing the submission tasklet, our goal is to reprioritise the HW execution queue and trigger HW preemption immediately, so disable bh over the call to schedule and force the tasklet to run afterwards if scheduled. v2: Keep rcu_read_lock() around for PREEMPT_RCU 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> Link: https://patchwork.freedesktop.org/patch/msgid/20180507135731.10587-1-chris@chris-wilson.co.uk Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
* drm/i915: Lazily unbind vma on closeChris Wilson2018-05-041-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When userspace is passing around swapbuffers using DRI, we frequently have to open and close the same object in the foreign address space. This shows itself as the same object being rebound at roughly 30fps (with a second object also being rebound at 30fps), which involves us having to rewrite the page tables and maintain the drm_mm range manager every time. However, since the object still exists and it is only the local handle that disappears, if we are lazy and do not unbind the VMA immediately when the local user closes the object but defer it until the GPU is idle, then we can reuse the same VMA binding. We still have to be careful to mark the handle and lookup tables as closed to maintain the uABI, just allowing the underlying VMA to be resurrected if the user is able to access the same object from the same context again. If the object itself is destroyed (neither userspace keeping a handle to it), the VMA will be reaped immediately as usual. In the future, this will be even more useful as instantiating a new VMA for use on the GPU will become heavier. A nuisance indeed, so nip it in the bud. v2: s/__i915_vma_final_close/i915_vma_destroy/ etc. v3: Leave a hint as to why we deferred the unbind on close. 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/20180503195115.22309-1-chris@chris-wilson.co.uk
* drm/i915: Split i915_gem_timeline into individual timelinesChris Wilson2018-05-021-73/+56
| | | | | | | | | | | | | | | | | | | | | | We need to move to a more flexible timeline that doesn't assume one fence context per engine, and so allow for a single timeline to be used across a combination of engines. This means that preallocating a fence context per engine is now a hindrance, and so we want to introduce the singular timeline. From the code perspective, this has the notable advantage of clearing up a lot of mirky semantics and some clumsy pointer chasing. By splitting the timeline up into a single entity rather than an array of per-engine timelines, we can realise the goal of the previous patch of tracking the timeline alongside the ring. v2: Tweak wait_for_idle to stop the compiling thinking that ret may be uninitialised. 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/20180502163839.3248-2-chris@chris-wilson.co.uk
* drm/i915: Move timeline from GTT to ringChris Wilson2018-05-021-4/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | In the future, we want to move a request between engines. To achieve this, we first realise that we have two timelines in effect here. The first runs through the GTT is required for ordering vma access, which is tracked currently by engine. The second is implied by sequential execution of commands inside the ringbuffer. This timeline is one that maps to userspace's expectations when submitting requests (i.e. given the same context, batch A is executed before batch B). As the rings's timelines map to userspace and the GTT timeline an implementation detail, move the timeline from the GTT into the ring itself (per-context in logical-ring-contexts/execlists, or a global per-engine timeline for the shared ringbuffers in legacy submission. The two timelines are still assumed to be equivalent at the moment (no migrating requests between engines yet) and so we can simply move from one to the other without adding extra ordering. v2: Reinforce that one isn't allowed to mix the engine execution timeline with the client timeline from userspace (on the ring). 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/20180502163839.3248-1-chris@chris-wilson.co.uk
* drm/i915: Only track live rings for retiringChris Wilson2018-04-301-2/+4
| | | | | | | | | | | | | | | We don't need to track every ring for its lifetime as they are managed by the contexts/engines. What we do want to track are the live rings so that we can sporadically clean up requests if userspace falls behind. We can simply restrict the gt->rings list to being only gt->live_rings. v2: s/live/active/ for consistency with gt.active_requests Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180430131503.5375-4-chris@chris-wilson.co.uk
* drm/i915: Retire requests along ringsChris Wilson2018-04-301-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the next patch, rings are the central timeline as requests may jump between engines. Therefore in the future as we retire in order along the engine timeline, we may retire out-of-order within a ring (as the ring now occurs along multiple engines), leading to much hilarity in miscomputing the position of ring->head. As an added bonus, retiring along the ring reduces the penalty of having one execlists client do cleanup for another (old legacy submission shares a ring between all clients). The downside is that slow and irregular (off the critical path) process of cleaning up stale requests after userspace becomes a modicum less efficient. In the long run, it will become apparent that the ordered ring->request_list matches the ring->timeline, a fun challenge for the future will be unifying the two lists to avoid duplication! v2: We need both engine-order and ring-order processing to maintain our knowledge of where individual rings have completed upto as well as knowing what was last executing on any engine. And finally by decoupling retiring the contexts on the engine and the timelines along the rings, we do have to keep a reference to the context on each request (previously it was guaranteed by the context being pinned). v3: Not just a reference to the context, but we need to keep it pinned as we manipulate the rings; i.e. we need a pin for both the manipulation of the engine state during its retirements, and a separate pin for the manipulation of the ring state. 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/20180430131503.5375-3-chris@chris-wilson.co.uk
* drm/i915: Wrap engine->context_pin() and engine->context_unpin()Chris Wilson2018-04-301-2/+2
| | | | | | | | | | | Make life easier in upcoming patches by moving the context_pin and context_unpin vfuncs into inline helpers. v2: Fixup mock_engine to mark the context as pinned on use. 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/20180430131503.5375-2-chris@chris-wilson.co.uk
* drm/i915: Compile out engine debug for releaseChris Wilson2018-04-261-1/+1
| | | | | | | | | | | | The majority of the engine state dumping is too voluminous to be useful outside of a controlled setup, though a few do accompany severe errors. Keep the debug dumps next to the errors, but hide the others behind a CI compile flag. This becomes more useful when adding more dumps to latency sensitive paths. 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/20180426103219.22181-1-chris@chris-wilson.co.uk
* drm/i915: Pack params to engine->schedule() into a structChris Wilson2018-04-181-8/+10
| | | | | | | | | | | | | | | Today we only want to pass along the priority to engine->schedule(), but in the future we want to have much more control over the various aspects of the GPU during a context's execution, for example controlling the frequency allowed. As we need an ever growing number of parameters for scheduling, move those into a struct for convenience. v2: Move the anonymous struct into its own function for legibility and ye olde gcc. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180418184052.7129-3-chris@chris-wilson.co.uk
* drm/i915: Split out functions for different kinds of workaroundsOscar Mateo2018-04-111-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There are different kind of workarounds (those that modify registers that live in the context image, those that modify global registers, those that whitelist registers, etc...) and they have different requirements in terms of where they are applied and how. Also, by splitting them apart, it should be easier to decide where a new workaround should go. v2: - Add multiple MISSING_CASE - Rebased v3: - Rename mmio_workarounds to gt_workarounds (Chris, Mika) - Create empty placeholders for BDW and CHV GT WAs - Rebased v4: Rebased v5: - Rebased - FORCE_TO_NONPRIV register exists since BDW, so make a path for it to achieve universality, even if empty (Chris) Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> [ickle: appease checkpatch] Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/1523376767-18480-2-git-send-email-oscar.mateo@intel.com
* drm/i915: Don't fiddle with rps/rc6 across GPU resetChris Wilson2018-04-101-7/+0
| | | | | | | | | | | | | | | | | | Resetting the GPU doesn't affect the RPS/RC6 state, so we can stop forcibly reloading the registers. Ville suggested this many moons ago, I said at that time that sanitizing was no harm and meant that our bookkeeping was kept consistent with the HW. However, in a forthcoming series, we want to split rps/rc6 GT powermanagement and one of the key simplifications is the control of when we enable it. Performing a crude sanitize in the middle of i915_gem_reset() is then a huge wart. Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180410133354.13425-1-chris@chris-wilson.co.uk
* drm/i915: Pass the set of guilty engines to i915_reset()Chris Wilson2018-04-061-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, we rely on inspecting the hangcheck state from within the i915_reset() routines to determine which engines were guilty of the hang. This is problematic for cases where we want to run i915_handle_error() and call i915_reset() independently of hangcheck. Instead of relying on the indirect parameter passing, turn it into an explicit parameter providing the set of stalled engines which then are treated as guilty until proven innocent. While we are removing the implicit stalled parameter, also make the reason into an explicit parameter to i915_reset(). We still need a back-channel for i915_handle_error() to hand over the task to the locked waiter, but let's keep that its own channel rather than incriminate another. This leaves stalled/seqno as being private to hangcheck, with no more nefarious snooping by reset, be it whole-device or per-engine. \o/ The only real issue now is that this makes it crystal clear that we don't actually do any testing of hangcheck per se in drv_selftest/live_hangcheck, merely of resets! Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michel Thierry <michel.thierry@intel.com> Cc: Jeff McGee <jeff.mcgee@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by: Michel Thierry <michel.thierry@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180406220354.18911-2-chris@chris-wilson.co.uk
* drm/i915: Treat i915_reset_engine() as guilty until proven innocentChris Wilson2018-04-061-19/+17
| | | | | | | | | | | | | | | | | If we are resetting just one engine, we know it has stalled. So we can pass the stalled parameter directly to i915_gem_reset_engine(), which alleviates the necessity to poke at the generic engine->hangcheck.stalled magic variable, leaving that under control of hangcheck as its name implies. Other than simplifying by removing the indirect parameter along this path, this allows us to introduce new reset mechanisms that run independently of hangcheck. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michel Thierry <michel.thierry@intel.com> Cc: Jeff McGee <jeff.mcgee@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by: Michel Thierry <michel.thierry@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180406220354.18911-1-chris@chris-wilson.co.uk
* drm/i915: Split out parking from the idle worker for reuseChris Wilson2018-04-061-28/+95
| | | | | | | | | | | | | | | | | | | | | | We will want to park GEM before disengaging the drive^W^W^W unwedging. Since we already do the work for idling, expose the guts as a new function that we can then reuse. v2: Just skip if already parked; makes it more forgiving to use by future callers. v3: Extract mark_busy, rename it to i915_gem_unpark and place it next to i915_gem_park so that we can evaluate it for symmetry more easily. Calling GEM from inside i915_request looks to be a bit of a layering violation, for the moment I am imaging them as being notify_cb. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> #v1 Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180406155144.27791-1-chris@chris-wilson.co.uk
* drm/i915: Reorder early initializationMichal Wajdeczko2018-03-231-3/+2
| | | | | | | | | | | | | | | | | In upcoming patch, we want to perform more actions in early initialization of the uC. This reordering will help resolve new dependencies that will be introduced by future patch. v2: s/i915_gem_load_init/i915_gem_init_early (Chris) v3: s/i915_gem_load_cleanup/i915_gem_cleanup_early (Michal) Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.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/20180323123451.59244-1-michal.wajdeczko@intel.com
* drm/i915: Stop engines when declaring the machine wedgedChris Wilson2018-03-161-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we fail to reset the GPU, we declare the machine wedged. However, the GPU may well still be running in the background with an in-flight request. So despite our efforts in cleaning up the request queue and faking the breadcrumb in the HWSP, the GPU may eventually write the in-flght seqno there breaking all of our assumptions and throwing the driver into a deep turmoil, wedging beyond wedged. To avoid this we ideally want to reset the GPU. Since that has already failed, make sure the rings have the stop bit set instead. This is part of the normal GPU reset sequence, but that is actually disabled by igt/gem_eio to force the wedged state. If we assume the worst, we must poke at the bit again before we give up. v2: Move the intel_gpu_reset() from set-wedged in the reset error path into i915_gem_set_wedged() itself. Even if the reset fails (e.g. if it is disabled by gem_eio), it still tries to make sure the engines are stopped. For i915_gem_set_wedged() callers from outside of i915_reset(), this should make sure the GPU is disabled while the driver is marked as being wedged. Testcase: igt/gem_eio Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Michał Winiarski <michal.winiarski@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Michel Thierry <michel.thierry@intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180315151015.22741-1-chris@chris-wilson.co.uk
* drm/i915: Trace GEM steps between submit and wedgingChris Wilson2018-03-161-0/+14
| | | | | | | | | | | | We still have an odd race with wedging/unwedging as shown by igt/gem_eio that defies expectations. Add some more trace_printks to try and visualize the flow over the precipice. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180315131451.4060-1-chris@chris-wilson.co.uk
* drm/i915/guc: Check the locking status of GuC WOPCM registersJackie Li2018-03-141-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | GuC WOPCM registers are write-once registers. Current driver code accesses these registers without checking the accessibility to these registers which will lead to unpredictable driver behaviors if these registers were touch by other components (such as faulty BIOS code). This patch moves the GuC WOPCM registers updating code into intel_wopcm.c and adds check before and after the update to GuC WOPCM registers so that we can make sure the driver is in a known state after writing to these write-once registers. v6: - Made sure module reloading won't bug the kernel while doing locking status checking v7: - Fixed patch format issues v8: - Fixed coding style issue on register lock bit macro definition (Sagar) v9: - Avoided to use redundant !! to cast uint to bool (Chris) - Return error code instead of GEM_BUG_ON for locked with invalid register values case (Sagar) - Updated guc_wopcm_hw_init to use guc_wopcm as first parameter (Michal) - Added code to set and validate the HuC_LOADING_AGENT_GUC bit in GuC WOPCM offset register based on the presence of HuC firmware (Michal) - Use bit fields instead of macros for GuC WOPCM flags (Michal) v10: - Refined variable names, removed redundant comments (Joonas) - Introduced lockable_reg to handle the write once register write and propagate the write error to caller (Joonas) - Used lockable_reg abstraction to avoid locking bit check on generic i915_reg_t (Michal) - Added log message for error paths (Michal) - Removed hw_updated flag and only relies on real hardware status v11: - Replaced lockable_reg with simplified function (Michal) - Used new macros for locking bits of WOPCM size/offset registers instead of using BIT(0) directly (Michal) - use intel_wopcm_init_hw() called from intel_gem_init_hw() to do GuC WOPCM register setup instead of calling from intel_uc_init_hw() (Michal) v12: - Updated function kernel-doc to align with code changes (Michal) - Updated code to use wopcm pointer directly (Michal) v13: - Updated the ordering of s-o-b/cc/r-b tags (Sagar) BSpec: 10875, 10833 Signed-off-by: Jackie Li <yaodong.li@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> (v11) Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v12) Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1520987574-19351-5-git-send-email-yaodong.li@intel.com
OpenPOWER on IntegriCloud