TDR/watchdog support for gen8
Subsystems (4)
cover TDR/watchdog support for gen8
This patch series introduces the following features:
* Feature 1: TDR (Timeout Detection and Recovery) for gen8 execlist mode.
TDR is an umbrella term for anything that goes into detecting and recovering from GPU hangs and is a term more widely used outside of the i915 driver.
This feature introduces an extensible framework that currently supports gen8 but that can be easily extended to support gen7 as well (which is already the case in Android but unfortunately in a not quite upstreamable form). The code contained in this submission represents the essentials of what is currently in Android merged with what is currently in upstream (as of the time when this work commenced a few months back).
A new hang recovery path has been added alongside the legacy GPU reset path, which takes care of engine recovery only. Aside from adding support for per-engine recovery this feature also introduces rules for how to promote a potential per-engine reset to a legacy, full GPU reset.
The hang checker now integrates with the error handler in a slightly different way in that it allows hang recovery on multiple engines at the same time by passing an engine flag mask to the error handler where flags representing all of the hung engines are set. This allows us to schedule hang recovery once for all currently hung engines instead of one hang recovery per detected engine hang. Previously, when only full GPU reset was supported this was all the same since it wouldn't matter if one or four engines were hung at any given point since it would all amount to the same thing - the GPU getting reset. As it stands now the behaviour is different depending on which engine is hung since each engine is reset independently of all the other engines, therefore we have to think about this in terms of scheduling cost and recovery latency. (see the open questions section below)
* Feature 2: Watchdog Timeout (a.k.a "media engine reset") for gen8.
This feature allows userland applications to control whether or not individual batch buffers should have a first-level, fine-grained, hardware-based hang detection mechanism on top of the ordinary, software-based periodic hang checker that is already in the driver. The advantage over relying solely on the current software-based hang checker is that the watchdog timeout mechanism is about 1000x quicker and more precise. Since it's not a full driver-level hang detection mechanism but only targetting one individual batch buffer at a time it can afford to be that quick without risking an increase in false positive hang detections.
This feature includes the following changes:
a) IRQ service routine for handling watchdog timeout interrupts and connecting these directly to the per-engine hang recovery path via the driver error handler.
b) Injection of watchdog timer enablement/cancellation instructions before/after the batch buffer start instruction in the ring buffer so that watchdog timeout is connected to the submission of an individual batch buffer.
c) Extension of the DRM batch buffer interface, exposing the watchdog timeout feature to userland.
There is currently full watchdog timeout support for gen7 in Android and it is quite similar to the gen8 implementation so there is nothing obvious that prevents us from upstreaming that code along with the gen8 code. However, watchdog timeout is fully dependent on the per-engine hang recovery path and that is not part of this code submission for gen7. Therefore watchdog timeout support for gen7 has been excluded until per-engine hang recovery support for gen7 has landed upstream.
As part of this submission we've had to reinstate the work queue that was previously in place between the error handler and the hang recovery path. The reason for this is that the per-engine recovery path is called directly from the interrupt handler in the case of watchdog timeout. In that situation there's no way of grabbing the struct_mutex, which is a requirement for the hang recovery path. Therefore, by reinstating the work queue we provide a unified execution context for the hang recovery code that allows the hang recovery code to grab whatever locks it needs without sacrificing interrupt latency too much or sleeping indefinitely in hard interrupt context.
* Feature 3. Context Submission Status Consistency (CSSC) checking
Something that becomes apparent when you run long-duration operations tests with concurrent rendering processes with intermittently injected hangs is that it seems like the GPU fails to send context event interrupts to the driver under some circumstances (in particular context completion and element switch events that would normally lead to the context event IRQ handler removing completed contexts from the execlist queue and submit new contexts to the hardware, thus driving work submission forward). What this means is that the driver sometimes gets stuck on a context that never seems to finish, all the while the hardware has completed and is waiting for more work.
Normally such hangs would be dealt with by the hang detection / hang recovery functionality in the driver. The problem with this in a per-engine hang recovery situation is that the per-engine hang recovery path relies on context resubmission in order to kick off the hardware again following an engine reset. This can only be done safely if the hardware and driver share the same opinion about the current state. Therefore, unless steps are taken the per-engine hang recovery path is ineffective in this type of situation.
One step that can be taken to deal with this kind of state inconsistency is to, after having detected such an inconsistency at the start of the per-engine hang recovery path, fake the presumed lost context event interrupt by manually calling the context event IRQ handler from the per-engine hang recovery path. The important thing to note here is that even if the hardware would fail to send an IRQ to the driver it doesn't mean that it would also fail to store the proper context state transition events in the CSB buffer. These events would be there, waiting to be processed, in the event of a lost context event IRQ. By faking the IRQ these events would be found and acted upon and hopefully TDR would then find the unprocessed context completion event that would cause the blocking context to be removed from the execlist queue, thus allowing work submission to resume.
In order to facilitate validation of this feature test infrastructure embedded in the context event IRQ handler has been included for inducing state inconsistencies by faking the loss of IRQs. By actively losing IRQs the hardware goes into one state while the driver is held back waiting for the state transition to happen. This then causes the context submission state consistency checker to kick in, detect the inconsistency and attempt to rectify it. Failing to do so would promote the hang to legacy full GPU reset like any other per-engine hang recovery failures.
OPEN QUESTIONS:
1. For the future, do we want to investigate the possibility of concurrent per-engine hang detection? In the current upstream driver there is only one work queue that handles the hang checker and everything from initial hang detection to final hang recovery runs in this thread. This makes sense if you're only supporting one form of hang recovery - using full GPU reset and nothing tied to any particular engine. However, as part of this patch series we're changing that by introducing per-engine hang recovery. It could make sense to introduce multiple work queues - one per engine - to run multiple hang checking threads in parallel.
This would potentially allow savings in terms of recovery latency since we don't have to scan all engines every time the hang checker is scheduled and the error handler does not have to scan all engines every time it is scheduled. Instead, we could implement one work queue per engine that would invoke the hang checker that only checks _that_ particular engine and then the error handler is invoked for _that_ particular engine. If one engine has hung the latency for getting to the hang recovery path for that particular engine would be (Time For Hang Checking One Engine) + (Time For Error Handling One Engine) rather than the time it takes to do hang checking for all engines + the time it takes to do error handling for all engines that have been detected as hung (which in the worst case would be all engines). There would potentially be as many hang checker and error handling threads going on concurrently as there are engines in the hardware but they would all be running in parallel without any significant locking. The first time where any thread needs exclusive access to the driver is at the point of the actual hang recovery but the time it takes to get there would theoretically be lower and the time it actually takes to do per-engine hang recovery is quite a lot lower than the time it takes to actually detect a hang reliably.
How much we would save by such a change still needs to be analysed and compared against the current single-thread model but it makes sense from a theoretical design point of view.
2. In Android it has become apparent that if hang recovery takes too long there is a distinct risk that userland will fail in such a way that the display might freeze until reboot (SurfaceFlinger/HWC fails to flip the display for a long enough time while the hang is being handled, which leads to it giving up any further attempts). In such time-constrained scenarios it might be a problem to wait until the start of the engine recovery path to rectify any context submission state inconsistencies since it would typically take around 3 seconds to get there assuming a default hang check sample period of 1 second. In Android it turned out that we needed rectification to happen after about 2 seconds, which is considerably less time than what it normally takes to recover from hangs. This might become a problem if this TDR implementation were ever to be ported to Android or other similar time-constrained environments. Issues like this has yet to show up in X or elsewhere and it is an open question how this TDR implementation would behave were it ever to be ported back to Android.
Worth keeping in mind for the future!
GITHUB REPOSITORY:
https://github.com/telf/TDR_patch_series_1.git
Tim Gore (1):
drm/i915: drm/i915 changes to simulated hangs
Tomas Elf (19):
drm/i915: Make i915_gem_reset_ring_status() public drm/i915: Generalise common GPU engine reset request/unrequest code drm/i915: TDR / per-engine hang recovery support for gen8. drm/i915: TDR / per-engine hang detection
drm/i915: Extending i915_gem_check_wedge to check engine reset in
progress
drm/i915: Reinstate hang recovery work queue.
drm/i915: Watchdog timeout: Hang detection integration into error
handler
drm/i915: Watchdog timeout: IRQ handler for gen8 drm/i915: Watchdog timeout: Ringbuffer command emission for gen8 drm/i915: Watchdog timeout: DRM kernel interface enablement drm/i915: Fake lost context event interrupts through forced CSB
checking.
drm/i915: Debugfs interface for per-engine hang recovery. drm/i915: Test infrastructure for context state inconsistency
simulation
drm/i915: TDR/watchdog trace points.
drm/i915: Port of Added scheduler support to __wait_request() calls drm/i915: Fix __i915_wait_request() behaviour during hang detection. drm/i915: Extended error state with TDR count, watchdog count and
engine reset count
drm/i915: TDR / per-engine hang recovery kernel docs drm/i915: Enable TDR / per-engine hang recovery
Documentation/DocBook/gpu.tmpl | 476 ++++++++++++++++++ drivers/gpu/drm/i915/i915_debugfs.c | 163 +++++- drivers/gpu/drm/i915/i915_dma.c | 80 +++ drivers/gpu/drm/i915/i915_drv.c | 328 ++++++++++++ drivers/gpu/drm/i915/i915_drv.h | 92 +++- drivers/gpu/drm/i915/i915_gem.c | 152 +++++- drivers/gpu/drm/i915/i915_gpu_error.c | 8 +- drivers/gpu/drm/i915/i915_irq.c | 286 +++++++++-- drivers/gpu/drm/i915/i915_params.c | 19 + drivers/gpu/drm/i915/i915_reg.h | 9 + drivers/gpu/drm/i915/i915_trace.h | 354 ++++++++++++- drivers/gpu/drm/i915/intel_display.c | 3 +- drivers/gpu/drm/i915/intel_lrc.c | 857 +++++++++++++++++++++++++++++++- drivers/gpu/drm/i915/intel_lrc.h | 16 +- drivers/gpu/drm/i915/intel_lrc_tdr.h | 39 ++ drivers/gpu/drm/i915/intel_ringbuffer.c | 90 +++- drivers/gpu/drm/i915/intel_ringbuffer.h | 95 ++++ drivers/gpu/drm/i915/intel_uncore.c | 197 +++++++- include/uapi/drm/i915_drm.h | 5 +- 19 files changed, 3144 insertions(+), 125 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_lrc_tdr.h
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 1/20 drm/i915: Make i915_gem_reset_ring_status() public
Makes i915_gem_reset_ring_status() public for use from engine reset path in order to replicate the same behavior as in full GPU reset but for a single engine.
--- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_gem.c | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +2 -0
@@ -2957,6 +2957,8 @@ static inline bool i915_stop_ring_allow_warn(struct drm_i915_private *dev_priv) }void i915_gem_reset (struct drm_device *dev); +void i915_gem_reset_ring_status (struct drm_i915_private *dev_priv, +struct intel_engine_cs *ring);bool i915_gem_clflush_object (struct drm_i915_gem_object *obj,bool force);int __must_check i915_gem_init (struct drm_device *dev);int i915_gem_init_rings (struct drm_device *dev);
drivers/gpu/drm/i915/i915_gem.c +2 -2
@@ -2716,8 +2716,8 @@ i915_gem_find_active_request(struct intel_engine_cs *ring)return NULL ; } -static void i915_gem_reset_ring_status (struct drm_i915_private *dev_priv, -struct intel_engine_cs *ring) +void i915_gem_reset_ring_status (struct drm_i915_private *dev_priv, +struct intel_engine_cs *ring) {struct drm_i915_gem_request *request;bool ring_hung;
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 2/20 drm/i915: Generalise common GPU engine reset request/unrequest code
In reply to Tomas ElfGPU engine reset handshaking is something that is applicable to both full GPU reset and engine reset, which is something that is part of the upcoming TDR per-engine hang recovery patches. Break out the common engine reset request/unrequest code (originally written by Mika Kuoppala) for reuse later in the TDR enablement patch series.
--- drivers/gpu/drm/i915/intel_uncore.c | 46 ++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 14 deletions(-)
drivers/gpu/drm/i915/intel_uncore.c +32 -14
@@ -1478,32 +1478,50 @@ static int wait_for_register(struct drm_i915_private *dev_priv,return wait_for ((I915_READ (reg) & mask) == value, timeout_ms); } +static inline int gen8_request_engine_reset (struct intel_engine_cs *engine) +{ +struct drm_i915_private *dev_priv = engine->dev->dev_private; +int ret =0 ; + +I915_WRITE (RING_RESET_CTL (engine->mmio_base), +_MASKED_BIT_ENABLE (RESET_CTL_REQUEST_RESET )); + + ret =wait_for_register (dev_priv, +RING_RESET_CTL (engine->mmio_base), +RESET_CTL_READY_TO_RESET , +RESET_CTL_READY_TO_RESET , +700 ); +if (ret) +DRM_ERROR ("%s: reset request timeout\n" , engine->name); + +return ret; +} + +static inline int gen8_unrequest_engine_reset (struct intel_engine_cs *engine) +{ +struct drm_i915_private *dev_priv = engine->dev->dev_private; + +I915_WRITE (RING_RESET_CTL (engine->mmio_base), +_MASKED_BIT_DISABLE (RESET_CTL_REQUEST_RESET )); + +return 0 ; +} +static int gen8_do_reset (struct drm_device *dev) {struct drm_i915_private *dev_priv = dev->dev_private;struct intel_engine_cs *engine;int i; -for_each_ring (engine, dev_priv, i) { -I915_WRITE (RING_RESET_CTL (engine->mmio_base), -_MASKED_BIT_ENABLE (RESET_CTL_REQUEST_RESET )); - -if (wait_for_register (dev_priv, -RING_RESET_CTL (engine->mmio_base), -RESET_CTL_READY_TO_RESET , -RESET_CTL_READY_TO_RESET , -700 )) { -DRM_ERROR ("%s: reset request timeout\n" , engine->name); +for_each_ring (engine, dev_priv, i) +if (gen8_request_engine_reset (engine)) goto not_ready; - } - }return gen6_do_reset (dev); not_ready:for_each_ring (engine, dev_priv, i) -I915_WRITE (RING_RESET_CTL (engine->mmio_base), -_MASKED_BIT_DISABLE (RESET_CTL_REQUEST_RESET )); +gen8_unrequest_engine_reset (engine);return -EIO ; }
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 3/20 drm/i915: TDR / per-engine hang recovery support for gen8.
In reply to Tomas ElfTDR = Timeout Detection and Recovery.
This change introduces support for TDR-style per-engine reset as an initial, less intrusive hang recovery option to be attempted before falling back to the legacy full GPU reset recovery mode if necessary. Initially we're only supporting gen8 but adding support for gen7 is straight-forward since we've already established an extensible framework where gen7 support can be plugged in (add corresponding versions of intel_ring_enable, intel_ring_disable, intel_ring_save, intel_ring_restore, etc.).
1. Per-engine recovery vs. Full GPU recovery
To capture the state of a single engine being detected as hung there is now a new flag for every engine that can be set once the decision has been made to schedule hang recovery for that particular engine. This patch only provides the hang recovery path but not the hang detection integration so for now there is no way of detecting individual engines as hung and targetting that individual engine for per-engine hang recovery.
The following algorithm is used to determine when to use which recovery mode given that hang detection has somehow detected a hang on an individual engine and given that per-engine hang recovery has been enabled (which it by default is not):
1. The error handler checks all engines that have been marked as hung by the hang checker and checks how long ago it was since it last attempted to do per-engine hang recovery for each respective, currently hung engine. If the measured time period is within a certain time window, i.e. the last per-engine hang recovery was done too recently, it is determined that the previously attempted per-engine hang recovery was ineffective and the step is taken to promote the current hang to a full GPU reset. The default value for this time window is 10 seconds, meaning any hang happening within 10 seconds of a previous hang on the same engine will be promoted to full GPU reset. (of course, as long as the per-engine hang recovery option is disabled this won't matter and the error handler will always go for legacy full GPU reset)
2. If the error handler determines that no currently hung engine has recently had hang recovery a per-engine hang recovery is scheduled.
3. If the decision to go with per-engine hang recovery is not taken, or if per-engine hang recovery is attempted but failed for whatever reason, TDR falls back to legacy full GPU recovery.
NOTE: Gen7 and earlier will always promote to full GPU reset since there is currently no per-engine reset support for these gens.
2. Context Submission Status Consistency.
Per-engine hang recovery on gen8 (or execlist submission mode in general) relies on the basic concept of context submission status consistency. What this means is that we make sure that the status of the hardware and the driver when it comes to the submission of the currently running context on any engine is consistent. For example, when submitting a context to the corresponding ELSP port of an engine we expect the owning request of that context to be at the head of the corresponding execution list queue. Likewise, as long as the context is executing on the GPU we expect the EXECLIST_STATUS register and the context status buffer (CSB) to reflect this. Thus, if the context submission status is consistent the ID of the currently executing context should be in EXECLIST_STATUS and it should be consistent with the context of the head request element in the execution list queue corresponding to that engine.
The reason why this is important for per-engine hang recovery in execlist mode is because this recovery mode relies on context resubmission in order to resume execution following the recovery. If a context has been determined to be hung and the per-engine hang recovery mode is engaged leading to the resubmission of that context it's important that the hardware is in fact not busy doing something else or is being idle since a resubmission during this state could cause unforseen side-effects such as unexpected preemptions.
There are rare, although consistently reproducable, situations that have shown up in practice where the driver and hardware are no longer consistent with each other, e.g. due to lost context completion interrupts after which the hardware would be idle but the driver would still think that a context would still be active.
3. There is a new reset path for engine reset alongside the legacy full GPU reset path. This path does the following:
1) Check for context submission consistency to make sure that the context that the hardware is currently stuck on is actually what the driver is working on. If not then clearly we're not in a consistently hung state and we bail out early.
2) Disable/idle the engine. This is done through reset handshaking on gen8+ unlike earlier gens where this was done by clearing the ring valid bits in MI_MODE and ring control registers, which are no longer supported on gen8+. Reset handshaking translates to setting the reset request bit in the reset control register.
3) Save the current engine state. What this translates to on gen8 is simply to read the current value of the head register and nudge it so that it points to the next valid instruction in the ring buffer. Since we assume that the execution is currently stuck in a batch buffer the effect of this is that the batchbuffer start instruction of the hung batch buffer is skipped so that when execution resumes, following the hang recovery completion, it resumes immediately following the batch buffer.
This effectively means that we're forcefully terminating the currently active, hung batch buffer. Obviously, the outcome of this intervention is potentially undefined but there are not many good options in this scenario. It's better than resetting the entire GPU in the vast majority of cases.
Save the nudged head value to be applied later.
4) Reset the engine.
5) Apply the nudged head value to the head register.
6) Reenable the engine. For gen8 this means resubmitting the fixed-up context, allowing execution to resume. In order to resubmit a context without relying on the currently hung execlist queue we use a new, privileged API that is dedicated to TDR use only. This submission API bypasses any currently queued work and gets exclusive access to the ELSP ports.
7) If the engine hang recovery procedure fails at any point in between disablement and reenablement of the engine there is a back-off
procedure: For gen8 it's possible to back out of the reset handshake by clearing the reset request bit in the reset control register.
NOTE:
It's possible that some of Ben Widawsky's original per-engine reset patches from 3 years ago are in this commit but since this work has gone through the hands of at least 3 people already any kind of ownership tracking has been lost a long time ago. If you think that you should be on the sob list just let me know.
* RFCv2: (Chris Wilson / Daniel Vetter)
- Simply use the previously private function i915_gem_reset_ring_status() from the engine hang recovery path to set active/pending context status. This replicates the same behaviour as in full GPU reset but for a single, targetted engine.
- Remove all additional uevents for both full GPU reset and per-engine reset. Adapted uevent behaviour to the new per-engine hang recovery mode in that it will only send one uevent regardless of which form of recovery is employed. If a per-engine reset is attempted first then one uevent will be dispatched. If that recovery mode fails and the hang is promoted to a full GPU reset no further uevents will be dispatched at that point.
- Tidied up the TDR context resubmission path in intel_lrc.c . Reduced the amount of duplication by relying entirely on the normal unqueue function. Added a new parameter to the unqueue function that takes into consideration if the unqueue call is for a first-time context submission or a resubmission and adapts the handling of elsp_submitted accordingly. The reason for this is that for context resubmission we don't expect any further interrupts for the submission or the following context completion. A more elegant way of handling this would be to phase out elsp_submitted altogether, however that's part of a LRC/execlist cleanup effort that is happening independently of this patch series. For now we make this change as simple as possible with as few non-TDR-related side-effects as possible.
--- drivers/gpu/drm/i915/i915_dma.c | 18 ++ drivers/gpu/drm/i915/i915_drv.c | 206 ++++++++++++ drivers/gpu/drm/i915/i915_drv.h | 60 ++++ drivers/gpu/drm/i915/i915_irq.c | 169 +++++++++- drivers/gpu/drm/i915/i915_params.c | 19 ++ drivers/gpu/drm/i915/i915_reg.h | 2 + drivers/gpu/drm/i915/intel_lrc.c | 557 +++++++++++++++++++++++++++++++- drivers/gpu/drm/i915/intel_lrc.h | 14 + drivers/gpu/drm/i915/intel_lrc_tdr.h | 36 +++ drivers/gpu/drm/i915/intel_ringbuffer.c | 84 ++++- drivers/gpu/drm/i915/intel_ringbuffer.h | 64 ++++ drivers/gpu/drm/i915/intel_uncore.c | 147 +++++++++ 12 files changed, 1350 insertions(+), 26 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_lrc_tdr.h
drivers/gpu/drm/i915/i915_dma.c +18 -0
@@ -837,6 +837,22 @@ static void intel_device_info_runtime_init(struct drm_device *dev) info->has_eu_pg ? "y" : "n"); } +static void +i915_hangcheck_init (struct drm_device *dev) +{ +int i; +struct drm_i915_private *dev_priv = dev->dev_private; + +for (i =0 ; i <I915_NUM_RINGS ; i++) { +struct intel_engine_cs *engine = &dev_priv->ring[i]; +struct intel_ring_hangcheck *hc = &engine->hangcheck; + +i915_hangcheck_reinit (engine); + hc->reset_count =0 ; + hc->tdr_count =0 ; + } +} +static void intel_init_dpio (struct drm_i915_private *dev_priv) {if (!IS_VALLEYVIEW (dev_priv))
@@ -1038,6 +1054,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)i915_gem_load (dev); +i915_hangcheck_init (dev); +/* On the 945G/GM, the chipset reports the MSI capability on the * integrated graphics even though the support isn't actually there * according to the published specs. It doesn't appear to function
drivers/gpu/drm/i915/i915_drv.c +206 -0
230 lines not on this page · raw message
drivers/gpu/drm/i915/i915_drv.h +60 -0
88 lines not on this page · raw message
drivers/gpu/drm/i915/i915_irq.c +156 -13
211 lines not on this page · raw message
drivers/gpu/drm/i915/i915_params.c +19 -0
@@ -35,6 +35,8 @@ struct i915_params i915 __read_mostly = { .enable_fbc = -1, .enable_execlists = -1, .enable_hangcheck = true, + .enable_engine_reset = false, + .gpu_reset_promotion_time = 10, .enable_ppgtt = -1, .enable_psr = 0, .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT),
@@ -108,6 +110,23 @@ MODULE_PARM_DESC(enable_hangcheck,"WARNING: Disabling this can cause system wide hangs. " "(default: true)"); +module_param_named_unsafe (enable_engine_reset, i915.enable_engine_reset, bool,0644 ); +MODULE_PARM_DESC (enable_engine_reset, +"Enable GPU engine hang recovery mode. Used as a soft, low-impact form " +"of hang recovery that targets individual GPU engines rather than the " +"entire GPU" +"(default: false)" ); + +module_param_named (gpu_reset_promotion_time, + i915.gpu_reset_promotion_time, int,0644 ); +MODULE_PARM_DESC (gpu_reset_promotion_time, +"Catch excessive engine resets. Each engine maintains a " +"timestamp of the last time it was reset. If it hangs again " +"within this period then fall back to full GPU reset to try and" +" recover from the hang. Only applicable if enable_engine_reset " +"is enabled." +"default=10 seconds" ); +module_param_named_unsafe (enable_ppgtt, i915.enable_ppgtt, int,0400 );MODULE_PARM_DESC (enable_ppgtt, "Override PPGTT usage. "
drivers/gpu/drm/i915/i915_reg.h +2 -0
@@ -135,6 +135,8 @@#define GEN6_GRDOM_RENDER (1 << 1)#define GEN6_GRDOM_MEDIA (1 << 2)#define GEN6_GRDOM_BLT (1 << 3) +#define GEN6_GRDOM_VECS (1 << 4) +#define GEN8_GRDOM_MEDIA2 (1 << 7)#define RING_PP_DIR_BASE (ring) ((ring)->mmio_base+0x228)#define RING_PP_DIR_BASE_READ (ring) ((ring)->mmio_base+0x518)
drivers/gpu/drm/i915/intel_lrc.c +545 -12
671 lines not on this page · raw message
drivers/gpu/drm/i915/intel_lrc.h +14 -0
@@ -31,7 +31,9 @@/* Execlists regs */ #define RING_ELSP (ring) ((ring)->mmio_base+0x230)#define RING_EXECLIST_STATUS_LO (ring) ((ring)->mmio_base+0x234) +#define EXECLIST_STATUS_CURRENT_ACTIVE_ELEMENT_STATUS (0x3 << 14)#define RING_EXECLIST_STATUS_HI (ring) ((ring)->mmio_base+0x234 + 4) +#define RING_EXECLIST_STATUS_CTX_ID (ring) RING_EXECLIST_STATUS_HI(ring)#define RING_CONTEXT_CONTROL (ring) ((ring)->mmio_base+0x244)#define CTX_CTRL_INHIBIT_SYN_CTX_SWITCH (1 << 3)#define CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT (1 << 0)
@@ -98,4 +100,16 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj);void intel_lrc_irq_handler (struct intel_engine_cs *ring);void intel_execlists_retire_requests (struct intel_engine_cs *ring); +int intel_execlists_read_tail (struct intel_engine_cs *ring, +struct intel_context *ctx, +u32 *tail); + +int intel_execlists_write_head (struct intel_engine_cs *ring, +struct intel_context *ctx, +u32 head); + +int intel_execlists_read_head (struct intel_engine_cs *ring, +struct intel_context *ctx, +u32 *head); +#endif /* _INTEL_LRC_H_ */
drivers/gpu/drm/i915/intel_lrc_tdr.h new file +36 -0
@@ -0,0 +1,36 @@ +/* +* Copyright © 2015 Intel Corporation +* +* Permission is hereby granted, free of charge, to any person obtaining a +* copy of this software and associated documentation files (the "Software"), +* to deal in the Software without restriction, including without limitation +* the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following conditions: +* +* The above copyright notice and this permission notice (including the next +* paragraph) shall be included in all copies or substantial portions of the +* Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +* DEALINGS IN THE SOFTWARE. +*/ + +#ifndef _INTEL_LRC_TDR_H_ +#define _INTEL_LRC_TDR_H_ + +/* Privileged execlist API used exclusively by TDR */ + +void intel_execlists_TDR_context_resubmission (struct intel_engine_cs *ring); + +enum context_submission_status +intel_execlists_TDR_get_current_request (struct intel_engine_cs *ring, +struct drm_i915_gem_request **req); + +#endif /* _INTEL_LRC_TDR_H_ */ +
drivers/gpu/drm/i915/intel_ringbuffer.c +83 -1
96 lines not on this page · raw message
drivers/gpu/drm/i915/intel_ringbuffer.h +64 -0
94 lines not on this page · raw message
drivers/gpu/drm/i915/intel_uncore.c +147 -0
153 lines not on this page · raw message
Signature · 1 line
1.9.1
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 4/20 drm/i915: TDR / per-engine hang detection
In reply to Tomas ElfWith the per-engine hang recovery path already in place this patch adds per-engine hang detection by letting the periodic hang checker detect hangs on individual engines and communicate this to the error handler. During hang checking every engine is checked and the hang detection status for each engine is aggregated into a single 32-bit engine flag mask that contains all the engine flags (1 << ring->id) of all the hung engines or'ed together. The per-engine path in the error handler then sets up the hangcheck state for each invidual, hung engine based on the engine flag mask before potentially calling the per-engine hang recovery path.
This allows the hang detection to happen in lock-step for all engines in parallel and lets the driver process all hung engines in turn in the error handler.
--- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 4 ++-- drivers/gpu/drm/i915/i915_irq.c | 41 +++++++++++++++++++++++-------------- 3 files changed, 29 insertions(+), 18 deletions(-)
drivers/gpu/drm/i915/i915_debugfs.c +1 -1
@@ -4587,7 +4587,7 @@ i915_wedged_set(void *data, u64 val)intel_runtime_pm_get (dev_priv); -i915_handle_error (dev, val, +i915_handle_error (dev,0x0 , val,"Manually setting wedged to %llu" , val);intel_runtime_pm_put (dev_priv);
drivers/gpu/drm/i915/i915_drv.h +2 -2
@@ -2755,8 +2755,8 @@ static inline void i915_hangcheck_reinit(struct intel_engine_cs *engine)/* i915_irq.c */ void i915_queue_hangcheck (struct drm_device *dev); -__printf(3, 4) -void i915_handle_error (struct drm_device *dev,bool wedged, +__printf(4, 5) +void i915_handle_error (struct drm_device *dev,u32 engine_mask,bool wedged,const char *fmt, ...);extern void intel_irq_init (struct drm_i915_private *dev_priv);
drivers/gpu/drm/i915/i915_irq.c +26 -15
86 lines not on this page · raw message
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 5/20 drm/i915: Extending i915_gem_check_wedge to check engine reset in progress
In reply to Tomas Elfi915_gem_wedge now returns a non-zero result in three different cases:
1. Legacy: A hang has been detected and full GPU reset is in progress.
2. Per-engine recovery:
a. A single engine reference can be passed to the function, in which case only that engine will be checked. If that particular engine is detected to be hung and is to be reset this will yield a non-zero result but not if reset is in progress for any other engine.
b. No engine reference is passed to the function, in which case all engines are checked for ongoing per-engine hang recovery.
Also, i915_wait_request was updated to take advantage of this new functionality. This is important since the TDR hang recovery mechanism needs a way to force waiting threads that hold the struct_mutex to give up the struct_mutex and try again after the hang recovery has completed. If i915_wait_request does not take per-engine hang recovery into account there is no way for a waiting thread to know that a per-engine recovery is about to happen and that it needs to back off.
--- drivers/gpu/drm/i915/i915_drv.h | 3 +- drivers/gpu/drm/i915/i915_gem.c | 60 +++++++++++++++++++++++++++------ drivers/gpu/drm/i915/intel_lrc.c | 4 +-- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++- 4 files changed, 56 insertions(+), 15 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +2 -1
@@ -2985,7 +2985,8 @@ i915_gem_find_active_request(struct intel_engine_cs *ring);bool i915_gem_retire_requests (struct drm_device *dev);void i915_gem_retire_requests_ring (struct intel_engine_cs *ring); -int __must_check i915_gem_check_wedge (struct i915_gpu_error *error, +int __must_check i915_gem_check_wedge (struct drm_i915_private *dev_priv, +struct intel_engine_cs *engine,bool interruptible);static inline bool i915_reset_in_progress (struct i915_gpu_error *error)
drivers/gpu/drm/i915/i915_gem.c +49 -11
120 lines not on this page · raw message
drivers/gpu/drm/i915/intel_lrc.c +2 -2
@@ -1027,8 +1027,8 @@ int intel_logical_ring_begin(struct drm_i915_gem_request *req, int num_dwords)WARN_ON (req ==NULL ); dev_priv = req->ring->dev->dev_private; - - ret =i915_gem_check_wedge (&dev_priv->gpu_error, + ret =i915_gem_check_wedge (dev_priv, + req->ring, dev_priv->mm.interruptible);if (ret)return ret;
drivers/gpu/drm/i915/intel_ringbuffer.c +3 -1
@@ -2497,8 +2497,10 @@ int intel_ring_begin(struct drm_i915_gem_request *req, ring = req->ring; dev_priv = ring->dev->dev_private; - ret =i915_gem_check_wedge (&dev_priv->gpu_error, + ret =i915_gem_check_wedge (dev_priv, + ring, dev_priv->mm.interruptible); +if (ret)return ret;
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 6/20 drm/i915: Reinstate hang recovery work queue.
In reply to Tomas ElfThere used to be a work queue separating the error handler from the hang recovery path, which was removed a while back in this commit:
commit b8d24a06568368076ebd5a858a011699a97bfa42 Author: Mika Kuoppala <[email protected]> Date: Wed Jan 28 17:03:14 2015 +0200 drm/i915: Remove nested work in gpu error handling
Now we need to revert most of that commit since the work queue separating hang detection from hang recovery is needed in preparation for the upcoming watchdog timeout feature. The watchdog interrupt service routine will be a second callsite of the error handler alongside the periodic hang checker, which runs in a work queue context. Seeing as the error handler will be serving a caller in a hard interrupt execution context that means that the error handler must never end up in a situation where it needs to grab the struct_mutex. Unfortunately, that is exactly what we need to do first at the start of the hang recovery path, which might potentially sleep if the struct_mutex is already held by another thread. Not good when you're in a hard interrupt context.
--- drivers/gpu/drm/i915/i915_dma.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 31 ++++++++++++++++++++++++------- 3 files changed, 26 insertions(+), 7 deletions(-)
drivers/gpu/drm/i915/i915_dma.c +1 -0
@@ -1202,6 +1202,7 @@ int i915_driver_unload(struct drm_device *dev)/* Free error state after interrupts are fully disabled. */ cancel_delayed_work_sync (&dev_priv->gpu_error.hangcheck_work);i915_destroy_error_state (dev); +cancel_work_sync (&dev_priv->gpu_error.work);if (dev->pdev->msi_enabled)pci_disable_msi (dev->pdev);
drivers/gpu/drm/i915/i915_drv.h +1 -0
@@ -1357,6 +1357,7 @@ struct i915_gpu_error {spinlock_t lock;/* Protected by the above dev->gpu_error.lock. */ struct drm_i915_error_state *first_error; +struct work_struct work;unsigned long missed_irq_rings;
drivers/gpu/drm/i915/i915_irq.c +24 -7
@@ -2394,16 +2394,19 @@ static void i915_error_wake_up(struct drm_i915_private *dev_priv, }/** -* i915_reset_and_wakeup - do process context error handling work -* @dev: drm device +* i915_error_work_func - do process context error handling work +* @work: work item containing error struct, passed by the error handler * * Fire an error uevent so userspace can see that a hang or error * was detected. */ -static void i915_reset_and_wakeup (struct drm_device *dev) +static void i915_error_work_func (struct work_struct *work) { -struct drm_i915_private *dev_priv =to_i915 (dev); -struct i915_gpu_error *error = &dev_priv->gpu_error; + struct i915_gpu_error *error = container_of(work, struct i915_gpu_error, + work); +struct drm_i915_private *dev_priv = + container_of(error, struct drm_i915_private, gpu_error); +struct drm_device *dev = dev_priv->dev;char *error_event[] = {I915_ERROR_UEVENT "=1" ,NULL };char *reset_event[] = {I915_RESET_UEVENT "=1" ,NULL };char *reset_done_event[] = {I915_ERROR_UEVENT "=0" ,NULL };
@@ -2764,7 +2767,21 @@ void i915_handle_error(struct drm_device *dev, u32 engine_mask, bool wedged,i915_error_wake_up (dev_priv, false); } -i915_reset_and_wakeup (dev); +/* +* Gen 7: +* +* Our reset work can grab modeset locks (since it needs to reset the +* state of outstanding pageflips). Hence it must not be run on our own +* dev-priv->wq work queue for otherwise the flush_work in the pageflip +* code will deadlock. +* If error_work is already in the work queue then it will not be added +* again. It hasn't yet executed so it will see the reset flags when +* it is scheduled. If it isn't in the queue or it is currently +* executing then this call will add it to the queue again so that +* even if it misses the reset flags during the current call it is +* guaranteed to see them on the next call. +*/ +schedule_work (&dev_priv->gpu_error.work); }/* Called from drm generic code, passed 'crtc' which
@@ -4532,7 +4549,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)struct drm_device *dev = dev_priv->dev;intel_hpd_init_work (dev_priv); - +INIT_WORK (&dev_priv->gpu_error.work, i915_error_work_func);INIT_WORK (&dev_priv->rps.work, gen6_pm_rps_work);INIT_WORK (&dev_priv->l3_parity.error_work, ivybridge_parity_work);
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 7/20 drm/i915: Watchdog timeout: Hang detection integration into error handler
In reply to Tomas ElfThis patch enables watchdog timeout hang detection as an entrypoint into the driver error handler. This form of hang detection overrides the promotion logic normally used by the periodic hang checker and instead allows for direct access to the per-engine hang recovery path.
NOTE: I don't know if Ben Widawsky had any part in this code from 3 years ago. There have been so many people involved in this already that I am in no position to know. If I've missed anyone's sob line please let me know.
--- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 6 +++--- drivers/gpu/drm/i915/i915_irq.c | 43 ++++++++++++++++++++++--------------- 3 files changed, 30 insertions(+), 21 deletions(-)
drivers/gpu/drm/i915/i915_debugfs.c +1 -1
@@ -4587,7 +4587,7 @@ i915_wedged_set(void *data, u64 val)intel_runtime_pm_get (dev_priv); -i915_handle_error (dev,0x0 , val, +i915_handle_error (dev,0x0 , false, val,"Manually setting wedged to %llu" , val);intel_runtime_pm_put (dev_priv);
drivers/gpu/drm/i915/i915_drv.h +3 -3
@@ -2756,9 +2756,9 @@ static inline void i915_hangcheck_reinit(struct intel_engine_cs *engine)/* i915_irq.c */ void i915_queue_hangcheck (struct drm_device *dev); -__printf(4, 5) -void i915_handle_error (struct drm_device *dev,u32 engine_mask,bool wedged, -const char *fmt, ...); +__printf(5, 6) +void i915_handle_error (struct drm_device *dev,u32 engine_mask, +bool watchdog,bool wedged,const char *fmt, ...);extern void intel_irq_init (struct drm_i915_private *dev_priv);int intel_irq_install (struct drm_i915_private *dev_priv);
drivers/gpu/drm/i915/i915_irq.c +26 -17
91 lines not on this page · raw message
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 8/20 drm/i915: Watchdog timeout: IRQ handler for gen8
In reply to Tomas Elf*** General ***
Watchdog timeout (or "media engine reset") is a feature that allows userland applications to enable hang detection on individual batch buffers. The detection mechanism itself is mostly bound to the hardware and the only thing that the driver needs to do to support this form of hang detection is to implement the interrupt handling support as well as watchdog command emission before and after the emitted batch buffer start instruction in the ring buffer.
The principle of the hang detection mechanism is as follows:
1. Once the decision has been made to enable watchdog timeout for a particular batch buffer and the driver is in the process of emitting the batch buffer start instruction into the ring buffer it also emits a watchdog timer start instruction before and a watchdog timer cancellation instruction after the batch buffer start instruction in the ring buffer.
2. Once the GPU execution reaches the watchdog timer start instruction the hardware watchdog counter is started by the hardware. The counter keeps counting until either reaching a previously configured threshold value or the timer cancellation instruction is executed.
2a. If the counter reaches the threshold value the hardware fires a watchdog interrupt that is picked up by the watchdog interrupt handler. This means that a hang has been detected and the driver needs to deal with it the same way it would deal with a engine hang detected by the periodic hang checker. The only difference between the two is that we never promote full GPU reset following a watchdog timeout in case a per-engine reset was attempted too recently. Thusly, the watchdog interrupt handler calls the error handler directly passing the engine mask of the hung engine in question, which immediately results in a per-engine hang recovery being scheduled.
2b. If the batch buffer completes and the execution reaches the watchdog cancellation instruction before the watchdog counter reaches its threshold value the watchdog is cancelled and nothing more comes of it. No hang is detected.
*** This patch introduces: ***
1. IRQ handler code for watchdog timeout allowing direct hang recovery based on hardware-driven hang detection, which then integrates directly with the per-engine hang recovery path.
2. Watchdog timeout init code patch for setup of watchdog timeout threshold values and gen-specific register information.
The current default watchdog threshold value is 60 ms, since this has been empirically determined to be a good compromise for low-latency requirements and low rate of false positives.
Currently the render engine and all available media engines support watchdog timeout. The specifications elude to the VECS engine being supported but that is currently not supported by this commit.
NOTE: I don't know if Ben Widawsky had any part in this code from 3 years ago. There have been so many people involved in this already that I am in no position to know. If I've missed anyone's sob line please let me know.
--- drivers/gpu/drm/i915/i915_dma.c | 59 +++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 39 ++++++++++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 7 ++++ drivers/gpu/drm/i915/intel_lrc.c | 7 ++++ drivers/gpu/drm/i915/intel_ringbuffer.h | 9 +++++ 6 files changed, 122 insertions(+)
drivers/gpu/drm/i915/i915_dma.c +59 -0
@@ -871,6 +871,64 @@ static void intel_init_dpio(struct drm_i915_private *dev_priv) } } +void i915_watchdog_init (struct drm_device *dev) +{ +struct drm_i915_private *dev_priv = dev->dev_private; +int freq; +int i; + +/* +* Based on pre-defined time out value (60ms or 30ms) calculate +* timer count thresholds needed based on core frequency. +* +* For RCS. +* The timestamp resolution changed in Gen7 and beyond to 80ns +* for all pipes. Before that it was 640ns. +*/ + +#define KM_RCS_ENGINE_TIMEOUT_VALUE_IN_MS 60 +#define KM_BSD_ENGINE_TIMEOUT_VALUE_IN_MS 60 +#define KM_TIMER_MILLISECOND 1000 + +/* +* Timestamp timer resolution = 0.080 uSec, +* or 12500000 counts per second +*/ +#define KM_TIMESTAMP_CNTS_PER_SEC_80NS 12500000 + +/* +* Timestamp timer resolution = 0.640 uSec, +* or 1562500 counts per second +*/ +#define KM_TIMESTAMP_CNTS_PER_SEC_640NS 1562500 + +if (INTEL_INFO (dev)->gen >=7 ) + freq =KM_TIMESTAMP_CNTS_PER_SEC_80NS ; +else + freq =KM_TIMESTAMP_CNTS_PER_SEC_640NS ; + + dev_priv->ring[RCS ].watchdog_threshold = + ((KM_RCS_ENGINE_TIMEOUT_VALUE_IN_MS ) * + (freq /KM_TIMER_MILLISECOND )); + + dev_priv->ring[VCS ].watchdog_threshold = + ((KM_BSD_ENGINE_TIMEOUT_VALUE_IN_MS ) * + (freq /KM_TIMER_MILLISECOND )); + + dev_priv->ring[VCS2 ].watchdog_threshold = + ((KM_BSD_ENGINE_TIMEOUT_VALUE_IN_MS ) * + (freq /KM_TIMER_MILLISECOND )); + +for (i =0 ; i <I915_NUM_RINGS ; i++) + dev_priv->ring[i].hangcheck.watchdog_count =0 ; + +DRM_INFO ("Watchdog Timeout [ms], " \ +"RCS: 0x%08X, VCS: 0x%08X, VCS2: 0x%08X\n" , \ +KM_RCS_ENGINE_TIMEOUT_VALUE_IN_MS , +KM_BSD_ENGINE_TIMEOUT_VALUE_IN_MS , +KM_BSD_ENGINE_TIMEOUT_VALUE_IN_MS ); +} +/** * i915_driver_load - setup chip and create an initial config * @dev: DRM device
@@ -1055,6 +1113,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)i915_gem_load (dev);i915_hangcheck_init (dev); +i915_watchdog_init (dev);/* On the 945G/GM, the chipset reports the MSI capability on the * integrated graphics even though the support isn't actually there
drivers/gpu/drm/i915/i915_drv.h +1 -0
@@ -2741,6 +2741,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv);void intel_hpd_init_work (struct drm_i915_private *dev_priv);void intel_hpd_cancel_work (struct drm_i915_private *dev_priv);bool intel_hpd_pin_to_port (enum hpd_pin pin,enum port *port); +void i915_watchdog_init (struct drm_device *dev);static inline void i915_hangcheck_reinit (struct intel_engine_cs *engine) {struct intel_ring_hangcheck *hc = &engine->hangcheck;
drivers/gpu/drm/i915/i915_irq.c +39 -0
@@ -1305,6 +1305,18 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv,intel_lrc_irq_handler (&dev_priv->ring[RCS ]);if (tmp & (GT_RENDER_USER_INTERRUPT <<GEN8_RCS_IRQ_SHIFT ))notify_ring (&dev_priv->ring[RCS ]); +if (tmp & (GT_GEN8_RCS_WATCHDOG_INTERRUPT <<GEN8_RCS_IRQ_SHIFT )) { +struct intel_engine_cs *ring; + +/* Stop the counter to prevent further interrupts */ + ring = &dev_priv->ring[RCS ]; +I915_WRITE (RING_CNTR (ring->mmio_base), +GEN6_RCS_WATCHDOG_DISABLE ); + + ring->hangcheck.watchdog_count++; +i915_handle_error (ring->dev,intel_ring_flag (ring), true, true, +"Render engine watchdog timed out" ); + }if (tmp & (GT_CONTEXT_SWITCH_INTERRUPT <<GEN8_BCS_IRQ_SHIFT ))intel_lrc_irq_handler (&dev_priv->ring[BCS ]);
@@ -1324,11 +1336,35 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv,intel_lrc_irq_handler (&dev_priv->ring[VCS ]);if (tmp & (GT_RENDER_USER_INTERRUPT <<GEN8_VCS1_IRQ_SHIFT ))notify_ring (&dev_priv->ring[VCS ]); +if (tmp & (GT_GEN8_VCS_WATCHDOG_INTERRUPT <<GEN8_VCS1_IRQ_SHIFT )) { +struct intel_engine_cs *ring; + +/* Stop the counter to prevent further interrupts */ + ring = &dev_priv->ring[VCS ]; +I915_WRITE (RING_CNTR (ring->mmio_base), +GEN8_VCS_WATCHDOG_DISABLE ); + + ring->hangcheck.watchdog_count++; +i915_handle_error (ring->dev,intel_ring_flag (ring), true, true, +"Media engine watchdog timed out" ); + }if (tmp & (GT_CONTEXT_SWITCH_INTERRUPT <<GEN8_VCS2_IRQ_SHIFT ))intel_lrc_irq_handler (&dev_priv->ring[VCS2 ]);if (tmp & (GT_RENDER_USER_INTERRUPT <<GEN8_VCS2_IRQ_SHIFT ))notify_ring (&dev_priv->ring[VCS2 ]); +if (tmp & (GT_GEN8_VCS_WATCHDOG_INTERRUPT <<GEN8_VCS2_IRQ_SHIFT )) { +struct intel_engine_cs *ring; + +/* Stop the counter to prevent further interrupts */ + ring = &dev_priv->ring[VCS2 ]; +I915_WRITE (RING_CNTR (ring->mmio_base), +GEN8_VCS_WATCHDOG_DISABLE ); + + ring->hangcheck.watchdog_count++; +i915_handle_error (ring->dev,intel_ring_flag (ring), true, true, +"Media engine 2 watchdog timed out" ); + } }else DRM_ERROR ("The master control interrupt lied (GT1)!\n" ); }
@@ -3776,11 +3812,14 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv) { /* These are interrupts we'll toggle with the ring mask register */ uint32_t gt_interrupts[] = { + GT_GEN8_RCS_WATCHDOG_INTERRUPT << GEN8_RCS_IRQ_SHIFT | GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT | GT_CONTEXT_SWITCH_INTERRUPT << GEN8_RCS_IRQ_SHIFT | GT_RENDER_L3_PARITY_ERROR_INTERRUPT | GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT | GT_CONTEXT_SWITCH_INTERRUPT << GEN8_BCS_IRQ_SHIFT, + GT_GEN8_VCS_WATCHDOG_INTERRUPT << GEN8_VCS1_IRQ_SHIFT | + GT_GEN8_VCS_WATCHDOG_INTERRUPT << GEN8_VCS2_IRQ_SHIFT | GT_RENDER_USER_INTERRUPT << GEN8_VCS1_IRQ_SHIFT | GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VCS1_IRQ_SHIFT | GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT |
drivers/gpu/drm/i915/i915_reg.h +7 -0
@@ -1509,6 +1509,8 @@ enum skl_disp_power_wells {#define RING_HEAD (base) ((base)+0x34)#define RING_START (base) ((base)+0x38)#define RING_CTL (base) ((base)+0x3c) +#define RING_CNTR (base) ((base)+0x178) +#define RING_THRESH (base) ((base)+0x17C)#define RING_SYNC_0 (base) ((base)+0x40)#define RING_SYNC_1 (base) ((base)+0x44)#define RING_SYNC_2 (base) ((base)+0x48)
@@ -1943,6 +1945,11 @@ enum skl_disp_power_wells {#define GT_BSD_USER_INTERRUPT (1 << 12)#define GT_RENDER_L3_PARITY_ERROR_INTERRUPT_S1 (1 << 11)/* hsw+; rsvd on snb, ivb, vlv */ #define GT_CONTEXT_SWITCH_INTERRUPT (1 << 8) +#define GT_GEN6_RENDER_WATCHDOG_INTERRUPT (1 << 6) +#define GT_GEN8_RCS_WATCHDOG_INTERRUPT (1 << 6) +#define GEN6_RCS_WATCHDOG_DISABLE 1 +#define GT_GEN8_VCS_WATCHDOG_INTERRUPT (1 << 6) +#define GEN8_VCS_WATCHDOG_DISABLE 0xFFFFFFFF#define GT_RENDER_L3_PARITY_ERROR_INTERRUPT (1 << 5)/* !snb */ #define GT_RENDER_PIPECTL_NOTIFY_INTERRUPT (1 << 4)#define GT_RENDER_CS_MASTER_ERROR_INTERRUPT (1 << 3)
drivers/gpu/drm/i915/intel_lrc.c +7 -0
@@ -2355,6 +2355,9 @@ static int logical_render_ring_init(struct drm_device *dev)if (HAS_L3_DPF (dev)) ring->irq_keep_mask |=GT_RENDER_L3_PARITY_ERROR_INTERRUPT ; + ring->irq_keep_mask |= + (GT_GEN8_RCS_WATCHDOG_INTERRUPT <<GEN8_RCS_IRQ_SHIFT ); +if (INTEL_INFO (dev)->gen >=9 ) ring->init_hw = gen9_init_render_ring;else
@@ -2415,6 +2418,8 @@ static int logical_bsd_ring_init(struct drm_device *dev)GT_RENDER_USER_INTERRUPT <<GEN8_VCS1_IRQ_SHIFT ; ring->irq_keep_mask =GT_CONTEXT_SWITCH_INTERRUPT <<GEN8_VCS1_IRQ_SHIFT ; + ring->irq_keep_mask |= + (GT_GEN8_VCS_WATCHDOG_INTERRUPT <<GEN8_VCS1_IRQ_SHIFT ); ring->init_hw = gen8_init_common_ring;if (IS_BROXTON (dev) &&INTEL_REVID (dev) <BXT_REVID_B0 ) {
@@ -2449,6 +2454,8 @@ static int logical_bsd2_ring_init(struct drm_device *dev)GT_RENDER_USER_INTERRUPT <<GEN8_VCS2_IRQ_SHIFT ; ring->irq_keep_mask =GT_CONTEXT_SWITCH_INTERRUPT <<GEN8_VCS2_IRQ_SHIFT ; + ring->irq_keep_mask |= + (GT_GEN8_VCS_WATCHDOG_INTERRUPT <<GEN8_VCS2_IRQ_SHIFT ); ring->init_hw = gen8_init_common_ring; ring->get_seqno = gen8_get_seqno;
drivers/gpu/drm/i915/intel_ringbuffer.h +9 -0
@@ -137,6 +137,9 @@ struct intel_ring_hangcheck {/* Number of TDR hang detections */ u32 tdr_count; + +/* Number of watchdog hang detections for this ring */ +u32 watchdog_count; };struct intel_ringbuffer {
@@ -363,6 +366,12 @@ struct intel_engine_cs {/* Saved head value to be restored after reset */ u32 saved_head; +/* +* Watchdog timer threshold values +* only RCS, VCS, VCS2 rings have watchdog timeout support +*/ +uint32_t watchdog_threshold; +struct {struct drm_i915_gem_object *obj;u32 gtt_offset;
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 9/20 drm/i915: Watchdog timeout: Ringbuffer command emission for gen8
In reply to Tomas Elf*** General ***
Watchdog timeout (or "media engine reset") is a feature that allows userland applications to enable hang detection on individual batch buffers. The detection mechanism itself is mostly bound to the hardware and the only thing that the driver needs to do to support this form of hang detection is to implement the interrupt handling support as well as watchdog command emission before and after the emitted batch buffer start instruction in the ring buffer.
The principle of the hang detection mechanism is as follows:
1. Once the decision has been made to enable watchdog timeout for a particular batch buffer and the driver is in the process of emitting the batch buffer start instruction into the ring buffer it also emits a watchdog timer start instruction before and a watchdog timer cancellation instruction after the batch buffer start instruction in the ring buffer.
2. Once the GPU execution reaches the watchdog timer start instruction the hardware watchdog counter is started by the hardware. The counter keeps counting until either reaching a previously configured threshold value or the timer cancellation instruction is executed.
2a. If the counter reaches the threshold value the hardware fires a watchdog interrupt that is picked up by the watchdog interrupt handler. This means that a hang has been detected and the driver needs to deal with it the same way it would deal with a engine hang detected by the periodic hang checker. The only difference between the two is that we never promote full GPU reset following a watchdog timeout in case a per-engine reset was attempted too recently. Thusly, the watchdog interrupt handler calls the error handler directly passing the engine mask of the hung engine in question, which immediately results in a per-engine hang recovery being scheduled.
2b. If the batch buffer completes and the execution reaches the watchdog cancellation instruction before the watchdog counter reaches its threshold value the watchdog is cancelled and nothing more comes of it. No hang is detected.
*** This patch introduces: ***
1. Command emission into the ring buffer for starting and stopping the watchdog timer before/after batch buffer start during batch buffer submission.
2. Feature support query functions for verifying that the requested engine actually supports watchdog timeout and fails the batch buffer submission otherwise.
NOTE: I don't know if Ben Widawsky had any part in this code from 3 years ago. There have been so many people involved in this already that I am in no position to know. If I've missed anyone's sob line please let me know.
--- drivers/gpu/drm/i915/intel_lrc.c | 99 +++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_ringbuffer.h | 22 ++++++++ 2 files changed, 121 insertions(+)
drivers/gpu/drm/i915/intel_lrc.c +99 -0
@@ -1056,6 +1056,80 @@ int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request)return intel_logical_ring_begin (request,0 ); } +static int +gen8_ring_start_watchdog (struct drm_i915_gem_request *req) +{ +int ret; +struct intel_ringbuffer *ringbuf = req->ringbuf; +struct intel_engine_cs *ring = ringbuf->ring; + + ret =intel_logical_ring_begin (req,10 ); +if (ret) +return ret; + +/* +* i915_reg.h includes a warning to place a MI_NOOP +* before a MI_LOAD_REGISTER_IMM +*/ +intel_logical_ring_emit (ringbuf,MI_NOOP ); +intel_logical_ring_emit (ringbuf,MI_NOOP ); + +/* Set counter period */ +intel_logical_ring_emit (ringbuf,MI_LOAD_REGISTER_IMM (1 )); +intel_logical_ring_emit (ringbuf,RING_THRESH (ring->mmio_base)); +intel_logical_ring_emit (ringbuf, ring->watchdog_threshold); +intel_logical_ring_emit (ringbuf,MI_NOOP ); + +/* Start counter */ +intel_logical_ring_emit (ringbuf,MI_LOAD_REGISTER_IMM (1 )); +intel_logical_ring_emit (ringbuf,RING_CNTR (ring->mmio_base)); +intel_logical_ring_emit (ringbuf,I915_WATCHDOG_ENABLE ); +intel_logical_ring_emit (ringbuf,MI_NOOP ); +intel_logical_ring_advance (ringbuf); + +return 0 ; +} + +static int +gen8_ring_stop_watchdog (struct drm_i915_gem_request *req) +{ +int ret; +struct intel_ringbuffer *ringbuf = req->ringbuf; +struct intel_engine_cs *ring = ringbuf->ring; + + ret =intel_logical_ring_begin (req,6 ); +if (ret) +return ret; + +/* +* i915_reg.h includes a warning to place a MI_NOOP +* before a MI_LOAD_REGISTER_IMM +*/ +intel_logical_ring_emit (ringbuf,MI_NOOP ); +intel_logical_ring_emit (ringbuf,MI_NOOP ); + +intel_logical_ring_emit (ringbuf,MI_LOAD_REGISTER_IMM (1 )); +intel_logical_ring_emit (ringbuf,RING_CNTR (ring->mmio_base)); + +switch (ring->id) { +default : +WARN (1 ,"%s does not support watchdog timeout! " \ +"Defaulting to render engine.\n" , ring->name); +case RCS : +intel_logical_ring_emit (ringbuf,GEN6_RCS_WATCHDOG_DISABLE ); +break ; +case VCS : +case VCS2 : +intel_logical_ring_emit (ringbuf,GEN8_VCS_WATCHDOG_DISABLE ); +break ; + } + +intel_logical_ring_emit (ringbuf,MI_NOOP ); +intel_logical_ring_advance (ringbuf); + +return 0 ; +} +/** * execlists_submission() - submit a batchbuffer for execution, Execlists style * @dev: DRM device.
@@ -1085,6 +1159,12 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,int instp_mode;u32 instp_mask;int ret; +bool watchdog_running = false; +/* +* NB: Place-holder until watchdog timeout is enabled through DRM +* execbuf interface +*/ +bool enable_watchdog = false; instp_mode = args->flags &I915_EXEC_CONSTANTS_MASK ; instp_mask =I915_EXEC_CONSTANTS_MASK ;
@@ -1121,6 +1201,18 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,if (ret)return ret; +/* Start watchdog timer */ +if (enable_watchdog) { +if (!intel_ring_supports_watchdog (ring)) +return -EINVAL ; + + ret =gen8_ring_start_watchdog (params->request); +if (ret) +return ret; + + watchdog_running = true; + } +if (ring == &dev_priv->ring[RCS ] && instp_mode != dev_priv->relative_constants_mode) { ret =intel_logical_ring_begin (params->request,4 );
@@ -1145,6 +1237,13 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,trace_i915_gem_ring_dispatch (params->request, params->dispatch_flags); +/* Cancel watchdog timer */ +if (watchdog_running) { + ret =gen8_ring_stop_watchdog (params->request); +if (ret) +return ret; + } +i915_gem_execbuffer_move_to_active (vmas, params->request);i915_gem_execbuffer_retire_commands (params);
drivers/gpu/drm/i915/intel_ringbuffer.h +22 -0
@@ -31,6 +31,8 @@ struct intel_hw_status_page {struct drm_i915_gem_object *obj; }; +#define I915_WATCHDOG_ENABLE 0 +#define I915_READ_TAIL (ring) I915_READ(RING_TAIL((ring)->mmio_base))#define I915_WRITE_TAIL (ring, val) I915_WRITE(RING_TAIL((ring)->mmio_base), val)
@@ -524,6 +526,26 @@ int intel_ring_save(struct intel_engine_cs *ring,int intel_ring_restore (struct intel_engine_cs *ring,struct drm_i915_gem_request *req); +static inline bool intel_ring_supports_watchdog (struct intel_engine_cs *ring) +{ +bool ret = false; + +if (WARN_ON (!ring)) + goto exit; + + ret = ( ring->id ==RCS || + ring->id ==VCS || + ring->id ==VCS2 ); + +if (!ret) +DRM_ERROR ("%s does not support watchdog timeout!\n" , ring->name); + +exit: +return ret; +} +int intel_ring_start_watchdog (struct intel_engine_cs *ring); +int intel_ring_stop_watchdog (struct intel_engine_cs *ring); +int __must_check intel_ring_idle (struct intel_engine_cs *ring);void intel_ring_init_seqno (struct intel_engine_cs *ring,u32 seqno);int intel_ring_flush_all_caches (struct drm_i915_gem_request *req);
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 10/20 drm/i915: Watchdog timeout: DRM kernel interface enablement
In reply to Tomas ElfFinal enablement patch for GPU hang recovery using watchdog timeout. Added execbuf flag for watchdog timeout in DRM kernel interface.
--- drivers/gpu/drm/i915/intel_lrc.c | 6 ++---- include/uapi/drm/i915_drm.h | 5 ++++- 2 files changed, 6 insertions(+), 5 deletions(-)
drivers/gpu/drm/i915/intel_lrc.c +2 -4
@@ -1160,10 +1160,6 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,u32 instp_mask;int ret;bool watchdog_running = false; -/* -* NB: Place-holder until watchdog timeout is enabled through DRM -* execbuf interface -*/ bool enable_watchdog = false; instp_mode = args->flags &I915_EXEC_CONSTANTS_MASK ;
@@ -1201,6 +1197,8 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,if (ret)return ret; + enable_watchdog = args->flags &I915_EXEC_ENABLE_WATCHDOG ; +/* Start watchdog timer */ if (enable_watchdog) {if (!intel_ring_supports_watchdog (ring))
include/uapi/drm/i915_drm.h +4 -1
@@ -776,7 +776,10 @@ struct drm_i915_gem_execbuffer2 {*/ #define I915_EXEC_RESOURCE_STREAMER (1<<15) -#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_RESOURCE_STREAMER<<1) +/* Enable watchdog timer for this batch buffer */ +#define I915_EXEC_ENABLE_WATCHDOG (1<<16) + +#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_ENABLE_WATCHDOG<<1)#define I915_EXEC_CONTEXT_ID_MASK (0xffffffff)#define i915_execbuffer2_set_context_id (eb2, context) \
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 10/20] drm/i915: Watchdog timeout: DRM kernel interface enablement
Quoted 2 lines · Tomas Elf
Final enablement patch for GPU hang recovery using watchdog timeout. Added execbuf flag for watchdog timeout in DRM kernel interface.
Usual argument: per-batch or per-context flag? What's the use case being consider that would favour one approach or another. That usecase should be explained in the changelog so that we can be reminded of it again later.
-Chris
Signature · 5 lines
Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 10/20] drm/i915: Watchdog timeout: DRM kernel interface enablement
Quoted 8 lines · Chris Wilson
Quoted 2 lines · Tomas Elf
Final enablement patch for GPU hang recovery using watchdog timeout. Added execbuf flag for watchdog timeout in DRM kernel interface.
Usual argument: per-batch or per-context flag? What's the use case being consider that would favour one approach or another. That usecase should be explained in the changelog so that we can be reminded of it again later.
It is per-batch, user expects certain workloads to be completed before watchdog times out hence it is per-batch.
This is mainly used for media workloads, don't know the exact usecase but I guess encode/decode work of frame to complete within a short time etc. I will add these details to commit msg.
regards
Arun
Quoted 2 lines
-Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 10/20] drm/i915: Watchdog timeout: DRM kernel interface enablement
Quoted 10 lines · Arun Siluvery
Quoted 8 lines · Chris Wilson
Quoted 2 lines · Tomas Elf
Final enablement patch for GPU hang recovery using watchdog timeout. Added execbuf flag for watchdog timeout in DRM kernel interface.
Usual argument: per-batch or per-context flag? What's the use case being consider that would favour one approach or another. That usecase should be explained in the changelog so that we can be reminded of it again later.
Quoted 5 lines
It is per-batch, user expects certain workloads to be completed before watchdog times out hence it is per-batch.
This is mainly used for media workloads, don't know the exact usecase but I guess encode/decode work of frame to complete within a short time etc. I will add these details to commit msg.
Usual reminder: We need an open-source user to get these new interfaces in. Looking at that should make it pretty clear whether the watchdog should be per-ctx or per-batch.
-Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 11/20 drm/i915: Fake lost context event interrupts through forced CSB checking.
In reply to Tomas Elf*** General ***
A recurring issue during long-duration operations testing of concurrent rendering tasks with intermittent hangs is that context completion interrupts following engine resets are sometimes lost. This becomes a real problem since the hardware might have completed a previously hung context following a per-engine hang recovery and then gone idle somehow without sending an interrupt telling the driver about this. At this point the driver would be stuck waiting for context completion, thinking that the context is still active, even though the hardware would be idle and waiting for more work. The periodic hang checker would detect hangs caused by stuck workloads in the GPU as well as these inconsistencies causing software hangs. The difference lies in how we handle these two types of hangs.
*** Rectification ***
The way hangs caused by inconsistent context submission states are resolved is by checking the context submission state consistency as a pre-stage to the engine recovery path. If the state is not consistent at that point then the normal form of engine recovery is not attempted. Instead, an attempt to rectify the inconsistency is made by faking the presumed lost context event interrupt - or more specifically by calling the context event interrupt handler manually from the hang recovery path outside of the normal interrupt execution context. The reason this works is because regardless of whether or not an IRQ goes missing the hardware always updates the CSB buffer during context state transitions, which means that in the case of a missing IRQ there would be outstanding CSB events waiting to be processed, out of which one might be the context completion event belonging to the context currently blocking the work submission flow in one of the execlist queues. The faked context event interrupt would then end up in the interrupt handler, which would process the outstanding events and purge the stuck context.
If this rectification attempt fails (because there are no outstanding CSB events, at least none that could account for the inconsistency) then the engine recovery is failed and the error handler falls back to legacy full GPU reset mode. Assuming the full GPU reset is successful this form of recovery will always cause the system to become consistent since the GPU is reset and forced into an idle state and all pending driver work is discarded, which would consistently reflect the idle GPU hardware state.
If the rectification attempt succeeds, meaning that unprocessed CSB events were found and acted upon which lead to old contexts being purged from the execlist queue and new work being submitted to hardware, then the inconsistency rectification is considered to have successfully resolved the detected hang that brought on the hang recovery. Therefore the engine recovery is ended early at that point and no further attempts at resolving the hang are made and the hang detection is cleared, allowing the driver to resume executing.
*** Detection ***
In principle a context submission status inconsistency is detected by comparing the ID of the context in the head request of an execlist queue with the context ID currently in the EXECLIST_STATUS register of the same engine (the latter denoting the ID of the context currently running on the hardware). If the two do not match it is assumed that an interrupt was missed and that the driver is now stuck in an inconsistent state. Of course, the driver and hardware can go in and out of consistency momentarily many times per second as contexts start and complete in the driver independently from the actual GPU hardware. The only way an inconsistency detection can be trusted is by first making sure that the detected state is stable, either by observing sustained, initial signs of a hang in the periodic hang checker or at the onset of the hang recovery path, at which point it has been decided that the execution is hung and that the driver is stable in that state.
*** WARNING ***
In time-constrained scenarios waiting until the onset of hang recovery before detecting and potentially rectifying context submission state inconsistencies might cause problematic side-effects. For example, in Android the SurfaceFlinger/HWC compositor has a hard time limit of 3 seconds after which any unresolved hangs might cause display freezes (due to dropped display flip requests), which can only be resolved by a reboot. If hang detection and hang recovery takes upwards of 3 seconds then there is a distinct risk that handling inconsistencies this late might cause issues. Whether or not this will become a problem remains to be shown in practice. So far no issues have been spotted in other environments such as X but it is worth being aware of.
--- drivers/gpu/drm/i915/i915_drv.c | 182 +++++++++++++++++++++++++++-------- drivers/gpu/drm/i915/i915_irq.c | 32 ++---- drivers/gpu/drm/i915/intel_lrc.c | 83 +++++++++++++++- drivers/gpu/drm/i915/intel_lrc.h | 2 +- drivers/gpu/drm/i915/intel_lrc_tdr.h | 3 + 5 files changed, 232 insertions(+), 70 deletions(-)
drivers/gpu/drm/i915/i915_drv.c +144 -38
@@ -969,6 +969,120 @@ int i915_reset(struct drm_device *dev) }/** +* i915_gem_reset_engine_CSSC_precheck - check/rectify context inconsistency +* @dev_priv: ... +* @engine: The engine whose state is to be checked. +* @req: Output parameter containing the request most recently submitted +* to hardware, if any. May be NULL. +* @ret: Output parameter containing the error code to be returned to +* i915_handle_error(). +* +* Before an engine reset can be attempted it is important that the submission +* state of the currently running (i.e. hung) context is verified as +* consistent. If the context submission state is inconsistent that means that +* the context that the driver thinks is running on hardware is in fact not +* running at all. It might be that the hardware is idle or is running another +* context altogether. The reason why this is important in the case of engine +* reset in particular is because at the end of the engine recovery path the +* fixed-up context needs to be resubmitted to hardware in order for the +* context changes (HEAD register nudged past the hung batch buffer) to take +* effect. Context resubmission requires the same context as is resubmitted to +* be running on hardware - otherwise we might cause unexpected preemptions or +* submit a context to a GPU engine that is idle, which would not make much +* sense. (if the engine is idle why does the driver think that the context in +* question is hung etc.) +* If an inconsistent state like this is detected then a rectification attempt +* is made by faking the presumed lost context event interrupt. The outcome of +* this attempt is returned back to the per-engine recovery path: If it was +* succesful the hang recovery can be aborted early since we now have resolved +* the hang this way. If it was not successful then fail the hang recovery and +* let the error handler promote to the next level of hang recovery. +* +* Returns: +* True: Work currently in progress, consistent state. +* Proceed with engine reset. +* False: No work in progress or work in progress but state irrecoverably +* inconsistent (context event IRQ faking attempted but failed). +* Do not proceed with engine reset. +*/ +static bool i915_gem_reset_engine_CSSC_precheck ( +struct drm_i915_private *dev_priv, +struct intel_engine_cs *engine, +struct drm_i915_gem_request **req, +int *ret) +{ +bool precheck_ok = true; +enum context_submission_status status; + +WARN_ON (!ret); + + *ret =0 ; + + status =intel_execlists_TDR_get_current_request (engine, req); + +if (status ==CONTEXT_SUBMISSION_STATUS_NONE_SUBMITTED ) { +/* +* No work in flight, no way to carry out a per-engine hang +* recovery in this state. Just do early exit and forget it +* happened. If this state persists then the error handler will +* be called by the periodic hang checker soon after this and +* at that point the hang will hopefully be promoted to full +* GPU reset, which will take care of it. +*/ +WARN (1 ,"No work in flight! Aborting recovery on %s\n" , + engine->name); + + precheck_ok = false; + *ret =0 ; + + }else if (status ==CONTEXT_SUBMISSION_STATUS_INCONSISTENT ) { +if (!intel_execlists_TDR_force_CSB_check (dev_priv, engine)) { +DRM_ERROR ("Inconsistency rectification on %s unsuccessful!\n" , + engine->name); + +/* +* Context submission state is inconsistent and +* faking a context event IRQ did not help. +* Fail and promote to higher level of +* recovery! +*/ + precheck_ok = false; + *ret = -EINVAL ; + }else { +DRM_INFO ("Inconsistency rectification on %s successful!\n" , + engine->name); + +/* +* Rectifying the inconsistent context +* submission status helped! No reset required, +* just exit and move on! +*/ + precheck_ok = false; + *ret =0 ; + +/* +* Reset the hangcheck state otherwise the hang checker +* will detect another hang immediately. Since the +* forced CSB checker resulted in more work being +* submitted to hardware we know that we are not hung +* anymore so it should be safe to clear any hang +* detections for this engine prior to this point. +*/ +i915_hangcheck_reinit (engine); + } + + }else if (status !=CONTEXT_SUBMISSION_STATUS_OK ) { +WARN (1 ,"Unexpected context submission status (%u) on %s\n" , + status, engine->name); + + precheck_ok = false; + *ret = -EINVAL ; + } + +return precheck_ok; +} + +/** * i915_reset_engine - reset GPU engine after a hang * @engine: engine to reset *
@@ -1009,28 +1123,22 @@ int i915_reset_engine(struct intel_engine_cs *engine)i915_gem_reset_ring_status (dev_priv, engine);if (i915.enable_execlists) { -enum context_submission_status status = -intel_execlists_TDR_get_current_request (engine,NULL ); -/* -* If the context submission state in hardware is not -* consistent with the the corresponding state in the driver or -* if there for some reason is no current context in the -* process of being submitted then bail out and try again. Do -* not proceed unless we have reliable current context state -* information. The reason why this is important is because -* per-engine hang recovery relies on context resubmission in -* order to force the execution to resume following the hung -* batch buffer. If the hardware is not currently running the -* same context as the driver thinks is hung then anything can -* happen at the point of context resubmission, e.g. unexpected -* preemptions or the previously hung context could be -* submitted when the hardware is idle which makes no sense. +* Check context submission status consistency (CSSC) before +* moving on. If the driver and hardware have different +* opinions about what is going on and this inconsistency +* cannot be rectified then just fail and let TDR escalate to a +* higher form of hang recovery. */ -if (status !=CONTEXT_SUBMISSION_STATUS_OK ) { - ret = -EAGAIN ; +if (!i915_gem_reset_engine_CSSC_precheck (dev_priv, + engine, +NULL , + &ret)) { +DRM_INFO ("Aborting hang recovery on %s (%d)\n" , + engine->name, ret); + goto reset_engine_error; - } + } } ret =intel_ring_disable (engine);
@@ -1040,27 +1148,25 @@ int i915_reset_engine(struct intel_engine_cs *engine) }if (i915.enable_execlists) { -enum context_submission_status status; -bool inconsistent; - - status =intel_execlists_TDR_get_current_request (engine, - ¤t_request); - - inconsistent = (status !=CONTEXT_SUBMISSION_STATUS_OK ); -if (inconsistent) { -/* -* If we somehow have reached this point with -* an inconsistent context submission status then -* back out of the previously requested reset and -* retry later. -*/ -WARN (inconsistent, -"Inconsistent context status on %s: %u\n" , - engine->name, status); +/* +* Get a hold of the currently executing context. +* +* Context submission status consistency is done implicitly so +* we might as well check it post-engine disablement since we +* get that option for free. Also, it's conceivable that the +* context submission state might have changed as part of the +* reset request on gen8+ so it's not completely devoid of +* value to do this. +*/ +if (!i915_gem_reset_engine_CSSC_precheck (dev_priv, + engine, + ¤t_request, + &ret)) { +DRM_INFO ("Aborting hang recovery on %s (%d)\n" , + engine->name, ret); - ret = -EAGAIN ; goto reenable_reset_engine_error; - } + } }/* Sample the current ring head position */
drivers/gpu/drm/i915/i915_irq.c +6 -26
@@ -36,6 +36,7 @@#include "i915_drv.h" #include "i915_trace.h" #include "intel_drv.h" +#include "intel_lrc_tdr.h" /** * DOC: interrupt handling
@@ -1302,7 +1303,7 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv, ret =IRQ_HANDLED ;if (tmp & (GT_CONTEXT_SWITCH_INTERRUPT <<GEN8_RCS_IRQ_SHIFT )) -intel_lrc_irq_handler (&dev_priv->ring[RCS ]); +intel_lrc_irq_handler (&dev_priv->ring[RCS ], true);if (tmp & (GT_RENDER_USER_INTERRUPT <<GEN8_RCS_IRQ_SHIFT ))notify_ring (&dev_priv->ring[RCS ]);if (tmp & (GT_GEN8_RCS_WATCHDOG_INTERRUPT <<GEN8_RCS_IRQ_SHIFT )) {
@@ -1319,7 +1320,7 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv, }if (tmp & (GT_CONTEXT_SWITCH_INTERRUPT <<GEN8_BCS_IRQ_SHIFT )) -intel_lrc_irq_handler (&dev_priv->ring[BCS ]); +intel_lrc_irq_handler (&dev_priv->ring[BCS ], true);if (tmp & (GT_RENDER_USER_INTERRUPT <<GEN8_BCS_IRQ_SHIFT ))notify_ring (&dev_priv->ring[BCS ]); }else
@@ -1333,7 +1334,7 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv, ret =IRQ_HANDLED ;if (tmp & (GT_CONTEXT_SWITCH_INTERRUPT <<GEN8_VCS1_IRQ_SHIFT )) -intel_lrc_irq_handler (&dev_priv->ring[VCS ]); +intel_lrc_irq_handler (&dev_priv->ring[VCS ], true);if (tmp & (GT_RENDER_USER_INTERRUPT <<GEN8_VCS1_IRQ_SHIFT ))notify_ring (&dev_priv->ring[VCS ]);if (tmp & (GT_GEN8_VCS_WATCHDOG_INTERRUPT <<GEN8_VCS1_IRQ_SHIFT )) {
@@ -1350,7 +1351,7 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv, }if (tmp & (GT_CONTEXT_SWITCH_INTERRUPT <<GEN8_VCS2_IRQ_SHIFT )) -intel_lrc_irq_handler (&dev_priv->ring[VCS2 ]); +intel_lrc_irq_handler (&dev_priv->ring[VCS2 ], true);if (tmp & (GT_RENDER_USER_INTERRUPT <<GEN8_VCS2_IRQ_SHIFT ))notify_ring (&dev_priv->ring[VCS2 ]);if (tmp & (GT_GEN8_VCS_WATCHDOG_INTERRUPT <<GEN8_VCS2_IRQ_SHIFT )) {
@@ -1376,7 +1377,7 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_i915_private *dev_priv, ret =IRQ_HANDLED ;if (tmp & (GT_CONTEXT_SWITCH_INTERRUPT <<GEN8_VECS_IRQ_SHIFT )) -intel_lrc_irq_handler (&dev_priv->ring[VECS ]); +intel_lrc_irq_handler (&dev_priv->ring[VECS ], true);if (tmp & (GT_RENDER_USER_INTERRUPT <<GEN8_VECS_IRQ_SHIFT ))notify_ring (&dev_priv->ring[VECS ]); }else
@@ -2476,27 +2477,6 @@ static void i915_error_work_func(struct work_struct *work) ret =i915_reset_engine (ring); -/* -* Execlist mode only: -* -* -EAGAIN means that between detecting a hang (and -* also determining that the currently submitted -* context is stable and valid) and trying to recover -* from the hang the current context changed state. -* This means that we are probably not completely hung -* after all. Just fail and retry by exiting all the -* way back and wait for the next hang detection. If we -* have a true hang on our hands then we will detect it -* again, otherwise we will continue like nothing -* happened. -*/ -if (ret == -EAGAIN ) { -DRM_ERROR ("Reset of %s aborted due to " \ -"change in context submission " \ -"state - retrying!" , ring->name); - ret =0 ; - } -if (ret) {DRM_ERROR ("Reset of %s failed! (%d)" , ring->name, ret);
drivers/gpu/drm/i915/intel_lrc.c +78 -5
138 lines not on this page · raw message
drivers/gpu/drm/i915/intel_lrc.h +1 -1
@@ -97,7 +97,7 @@ int intel_execlists_submission(struct i915_execbuffer_params *params, struct list_head *vmas);u32 intel_execlists_ctx_id (struct drm_i915_gem_object *ctx_obj); -void intel_lrc_irq_handler (struct intel_engine_cs *ring); +int intel_lrc_irq_handler (struct intel_engine_cs *ring,bool do_lock);void intel_execlists_retire_requests (struct intel_engine_cs *ring); int intel_execlists_read_tail(struct intel_engine_cs *ring,
drivers/gpu/drm/i915/intel_lrc_tdr.h +3 -0
@@ -32,5 +32,8 @@ enum context_submission_status intel_execlists_TDR_get_current_request(struct intel_engine_cs *ring, struct drm_i915_gem_request **req); +bool intel_execlists_TDR_force_CSB_check (struct drm_i915_private *dev_priv, +struct intel_engine_cs *engine); +#endif /* _INTEL_LRC_TDR_H_ */
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 12/20 drm/i915: Debugfs interface for per-engine hang recovery.
In reply to Tomas Elf1. The i915_wedged_set() function now allows for both legacy full GPU reset and per-engine reset of one or more engines at a time:
a) Legacy hang recovery by passing 0.
b) Multiple engine hang recovery by passing in an engine flag mask where bit 0 corresponds to engine 0 = RCS, bit 1 corresponds to engine 1 = VCS etc. This allows for any combination of engine hang recoveries to be tested. For example, by passing in the value 0x3 hang recovery for engines 0 and 1 (RCS and VCS) are scheduled at the same time.
2. The i915_hangcheck_info() function is complemented with statistics related to:
a) Number of engine hangs detected by periodic hang checker.
b) Number of watchdog timeout hangs detected.
c) Number of full GPU resets carried out.
d) Number of engine resets carried out.
--- drivers/gpu/drm/i915/i915_debugfs.c | 75 +++++++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 4 deletions(-)
drivers/gpu/drm/i915/i915_debugfs.c +71 -4
@@ -1352,6 +1352,8 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused) }else seq_printf (m,"Hangcheck inactive\n" ); +seq_printf (m,"Full GPU resets = %u\n" ,i915_reset_count (&dev_priv->gpu_error)); +for_each_ring (ring, dev_priv, i) {seq_printf (m,"%s:\n" , ring->name); seq_printf(m, "\tseqno = %x [current %x]\n",
@@ -1363,6 +1365,12 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused) (long long)ring->hangcheck.max_acthd);seq_printf (m,"\tscore = %d\n" , ring->hangcheck.score);seq_printf (m,"\taction = %d\n" , ring->hangcheck.action); +seq_printf (m,"\tengine resets = %u\n" , + ring->hangcheck.reset_count); +seq_printf (m,"\tengine hang detections = %u\n" , + ring->hangcheck.tdr_count); +seq_printf (m,"\tengine watchdog timeout detections = %u\n" , + ring->hangcheck.watchdog_count); }return 0 ;
@@ -4568,11 +4576,48 @@ i915_wedged_get(void *data, u64 *val)return 0 ; } +static const char *ringid_to_str (enum intel_ring_id ring_id) +{ +switch (ring_id) { +case RCS : +return "RCS" ; +case VCS : +return "VCS" ; +case BCS : +return "BCS" ; +case VECS : +return "VECS" ; +case VCS2 : +return "VCS2" ; + } + +return "unknown" ; +} +static int i915_wedged_set (void *data,u64 val) {struct drm_device *dev = data;struct drm_i915_private *dev_priv = dev->dev_private; +struct intel_engine_cs *engine; +u32 i; +#define ENGINE_MSGLEN 64 +char msg[ENGINE_MSGLEN ]; + +/* +* Val contains the engine flag mask of engines to be reset. +* +* * Full GPU reset is caused by passing val == 0x0 +* +* * Any combination of engine hangs is caused by setting up val as a +* mask with the following bits set for each engine to be hung: +* +* Bit 0: RCS engine +* Bit 1: VCS engine +* Bit 2: BCS engine +* Bit 3: VECS engine +* Bit 4: VCS2 engine (if available) +*/ /* * There is no safeguard against this debugfs entry colliding
@@ -4581,14 +4626,36 @@ i915_wedged_set(void *data, u64 val)* test harness is responsible enough not to inject gpu hangs * while it is writing to 'i915_wedged' */ - -if (i915_reset_in_progress (&dev_priv->gpu_error)) +if (i915_gem_check_wedge (dev_priv,NULL , true))return -EAGAIN ;intel_runtime_pm_get (dev_priv); -i915_handle_error (dev,0x0 , false, val, -"Manually setting wedged to %llu" , val); +memset (msg,0 ,sizeof (msg)); + +if (val) { +scnprintf (msg,sizeof (msg),"Manual reset:" ); + +/* Assemble message string */ +for_each_ring (engine, dev_priv, i) +if (intel_ring_flag (engine) & val) { +DRM_INFO ("Manual reset: %s\n" , engine->name); + +scnprintf (msg,sizeof (msg), +"%s [%s]" , + msg, +ringid_to_str (i)); + } + + }else { +scnprintf (msg,sizeof (msg),"Manual global reset" ); + } + +i915_handle_error (dev, + val, + false, + true, + msg);intel_runtime_pm_put (dev_priv);
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 13/20 drm/i915: Test infrastructure for context state inconsistency simulation
In reply to Tomas ElfAdded debugfs functions and embedded test infrastructure in the context event interrupt handler for simulating the loss of context event interrupts so that a context submission state inconsistency can be induced. This is useful for testing the consistency checker pre-stage to the engine hang recovery path since in order to test that the inconsistency detection works we first need to induce a state inconsistency that the inconsistency checker can detect and act upon.
--- drivers/gpu/drm/i915/i915_debugfs.c | 88 +++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_dma.c | 2 + drivers/gpu/drm/i915/i915_drv.c | 3 ++ drivers/gpu/drm/i915/i915_drv.h | 12 +++++ drivers/gpu/drm/i915/intel_lrc.c | 68 ++++++++++++++++++++++++++++ 5 files changed, 173 insertions(+)
drivers/gpu/drm/i915/i915_debugfs.c +88 -0
@@ -4667,6 +4667,93 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_wedged_fops, "%llu\n");static int +i915_fake_ctx_submission_inconsistency_get (void *data,u64 *val) +{ +struct drm_device *dev = data; +struct drm_i915_private *dev_priv = dev->dev_private; +struct intel_engine_cs *ring; +unsigned i; + +DRM_INFO ("Faked inconsistent context submission state: %x\n" , + dev_priv->gpu_error.faked_lost_ctx_event_irq); + +for_each_ring (ring, dev_priv, i) { +u32 fake_cnt = + (dev_priv->gpu_error.faked_lost_ctx_event_irq >> (i<<2 )) &0xf ; + +DRM_INFO ("%s: Faking %s [%u IRQs left to drop]\n" , + ring->name, + fake_cnt?"enabled" :"disabled" , + fake_cnt); + } + + *val = (u64 ) dev_priv->gpu_error.faked_lost_ctx_event_irq; + +return 0 ; +} + +static int +i915_fake_ctx_submission_inconsistency_set (void *data,u64 val) +{ +struct drm_device *dev = data; +struct drm_i915_private *dev_priv = dev->dev_private; +u32 fake_status; + +/* +* Set up a simulated/faked lost context event interrupt. This is used +* to induce inconsistent HW/driver states that the context submission +* status consistency checker (involved as a pre-stage to GPU engine +* hang recovery), which is required for validation purposes. +* +* val contains the new faked_lost_ctx_event_irq word that is to be +* merged with the already set faked_lost_ctx_event_irq word. +* +* val == 0 means clear all previously set fake bits. +* +* Each nibble contains a number between 0-15 denoting the number of +* interrupts left to lose on the engine that nibble corresponds to. +* +* RCS: faked_lost_ctx_event_irq[3:0] +* VCS: faked_lost_ctx_event_irq[7:4] +* BCS: faked_lost_ctx_event_irq[11:8] +* VECS: faked_lost_ctx_event_irq[15:12] +* etc +* +* The number in each nibble is decremented by the context event +* interrupt handler in intel_lrc.c once the faked interrupt loss is +* executed. If a targetted interrupt is received when bit +* corresponding to that engine is set that interrupt will be dropped +* without side-effects, thus inducing an inconsistency since the +* hardware has entered a state where removal of a context from the +* context queue is required but the driver is not informed of this and +* is therefore stuck in that state until inconsistency rectification +* (forced CSB checking) or reboot. +*/ + + fake_status = + dev_priv->gpu_error.faked_lost_ctx_event_irq; + +DRM_INFO ("Faking lost context event IRQ (new status: %x, old status: %x)\n" , + (u32 ) val, fake_status); + +if (val) { + dev_priv->gpu_error.faked_lost_ctx_event_irq |= ((u32 ) val); + }else { +DRM_INFO ("Clearing lost context event IRQ mask\n" ); + + dev_priv->gpu_error.faked_lost_ctx_event_irq =0 ; + } + + +return 0 ; +} + +DEFINE_SIMPLE_ATTRIBUTE (i915_fake_ctx_submission_inconsistency_fops, + i915_fake_ctx_submission_inconsistency_get, + i915_fake_ctx_submission_inconsistency_set, +"%llu\n" ); + +static int i915_ring_stop_get (void *data,u64 *val) {struct drm_device *dev = data;
@@ -5320,6 +5407,7 @@ static const struct i915_debugfs_files {const struct file_operations *fops; } i915_debugfs_files[] = { {"i915_wedged", &i915_wedged_fops}, + {"i915_fake_ctx_inconsistency", &i915_fake_ctx_submission_inconsistency_fops}, {"i915_max_freq", &i915_max_freq_fops}, {"i915_min_freq", &i915_min_freq_fops}, {"i915_cache_sharing", &i915_cache_sharing_fops},
drivers/gpu/drm/i915/i915_dma.c +2 -0
@@ -843,6 +843,8 @@ i915_hangcheck_init(struct drm_device *dev)int i;struct drm_i915_private *dev_priv = dev->dev_private; + dev_priv->gpu_error.faked_lost_ctx_event_irq =0 ; +for (i =0 ; i <I915_NUM_RINGS ; i++) {struct intel_engine_cs *engine = &dev_priv->ring[i];struct intel_ring_hangcheck *hc = &engine->hangcheck;
drivers/gpu/drm/i915/i915_drv.c +3 -0
@@ -917,6 +917,9 @@ int i915_reset(struct drm_device *dev) } } +/* Clear simulated lost context event interrupts */ + dev_priv->gpu_error.faked_lost_ctx_event_irq =0 ; +if (i915_stop_ring_allow_warn (dev_priv))pr_notice ("drm/i915: Resetting chip after gpu hang\n" );
drivers/gpu/drm/i915/i915_drv.h +12 -0
@@ -1400,6 +1400,18 @@ struct i915_gpu_error {#define I915_STOP_RING_ALLOW_BAN (1 << 31)#define I915_STOP_RING_ALLOW_WARN (1 << 30) +/* +* Bit mask for simulation of lost context event IRQs on each +* respective engine. +* +* Bits 0:3: Number of lost IRQs to be faked on RCS +* Bits 4:7: Number of lost IRQs to be faked on VCS +* Bits 8:11: Number of lost IRQs to be faked on BCS +* Bits 12:15: Number of lost IRQs to be faked on VECS +* Bits 16:19: Number of lost IRQs to be faked on VCS2 +*/ +u32 faked_lost_ctx_event_irq; +/* For missed irq/seqno simulation. */ unsigned int test_irq_rings;
drivers/gpu/drm/i915/intel_lrc.c +68 -0
92 lines not on this page · raw message
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 14/20 drm/i915: TDR/watchdog trace points.
In reply to Tomas ElfDefined trace points and sprinkled the usage of these throughout the TDR/watchdog implementation.
The following trace points are supported:
1. trace_i915_tdr_gpu_recovery:
Called at the onset of the full GPU reset recovery path.
2. trace_i915_tdr_engine_recovery:
Called at the onset of the per-engine recovery path.
3. i915_tdr_recovery_start:
Called at the onset of hang recovery before recovery mode has been decided.
4. i915_tdr_recovery_complete:
Called at the point of hang recovery completion.
5. i915_tdr_recovery_queued:
Called once the error handler decides to schedule the actual hang recovery, which marks the end of the hang detection path.
6. i915_tdr_engine_save:
Called at the point of saving the engine state during per-engine hang recovery.
7. i915_tdr_gpu_reset_complete:
Called at the point of full GPU reset recovery completion.
8. i915_tdr_engine_reset_complete:
Called at the point of per-engine recovery completion.
9. i915_tdr_forced_csb_check:
Called at the completion of a forced CSB check.
10. i915_tdr_hang_check:
Called for every engine in the periodic hang checker loop before moving on to the next engine. Provides an overview of all hang check stats in real-time. The collected stats are:
a. Engine name.
b. Current engine seqno.
c. Seqno of previous hang check iteration for that engine.
d. ACTHD register value of given engine.
e. Current hang check score of given engine (and whether or not the engine has been detected as hung).
f. Current action for given engine.
g. Busyness of given engine.
h. Submission status of currently running context on given engine.
11. i915_tdr_inconsistency:
Called when an inconsistency is detected to provide more information in the log about the nature of the inconsistency. The collected information is:
a. Engine name.
b. ID of the currently executing context on hardware.
c. Is the given engine idle or not?
d. The ID of the context that was most recently submitted to the ELSP port from the execlist queue for the given engine.
e. The submission/IRQ balance of the request most recently submitted to hardware (elsp_submitted).
--- drivers/gpu/drm/i915/i915_drv.c | 3 + drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- drivers/gpu/drm/i915/i915_irq.c | 11 +- drivers/gpu/drm/i915/i915_trace.h | 339 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_lrc.c | 21 ++- drivers/gpu/drm/i915/intel_uncore.c | 4 + 7 files changed, 377 insertions(+), 4 deletions(-)
drivers/gpu/drm/i915/i915_drv.c +3 -0
@@ -896,6 +896,7 @@ int i915_reset(struct drm_device *dev)bool simulated;int ret; +trace_i915_tdr_gpu_recovery (dev);intel_reset_gt_powersave (dev);mutex_lock (&dev->struct_mutex);
@@ -1120,6 +1121,8 @@ int i915_reset_engine(struct intel_engine_cs *engine)WARN_ON (!mutex_is_locked (&dev->struct_mutex)); +trace_i915_tdr_engine_recovery (engine); +/* Take wake lock to prevent power saving mode */ intel_uncore_forcewake_get (dev_priv,FORCEWAKE_ALL );
drivers/gpu/drm/i915/i915_drv.h +1 -0
@@ -3335,6 +3335,7 @@ void i915_destroy_error_state(struct drm_device *dev);void i915_get_extra_instdone (struct drm_device *dev,uint32_t *instdone);const char *i915_cache_level_str (struct drm_i915_private *i915,int type); +const char *hangcheck_action_to_str (enum intel_ring_hangcheck_action a);/* i915_cmd_parser.c */ int i915_cmd_parser_get_version (void );
drivers/gpu/drm/i915/i915_gpu_error.c +1 -1
@@ -221,7 +221,7 @@ static void print_error_buffers(struct drm_i915_error_state_buf *m, } } -static const char *hangcheck_action_to_str (enum intel_ring_hangcheck_action a) +const char *hangcheck_action_to_str (enum intel_ring_hangcheck_action a) {switch (a) {case HANGCHECK_IDLE :
drivers/gpu/drm/i915/i915_irq.c +10 -1
@@ -2454,6 +2454,7 @@ static void i915_error_work_func(struct work_struct *work)mutex_lock (&dev->struct_mutex); +trace_i915_tdr_recovery_start (dev);kobject_uevent_env (&dev->primary->kdev->kobj,KOBJ_CHANGE , error_event);for_each_ring (ring, dev_priv, i) {
@@ -2572,6 +2573,7 @@ static void i915_error_work_func(struct work_struct *work)kobject_uevent_env (&dev->primary->kdev->kobj,KOBJ_CHANGE , reset_done_event); } +trace_i915_tdr_recovery_complete (dev); }static void i915_report_and_clear_eir (struct drm_device *dev)
@@ -2697,6 +2699,7 @@ void i915_handle_error(struct drm_device *dev, u32 engine_mask,struct drm_i915_private *dev_priv = dev->dev_private;va_list args;char error_msg[80 ]; +bool full_reset = true;struct intel_engine_cs *engine;
@@ -2715,7 +2718,6 @@ void i915_handle_error(struct drm_device *dev, u32 engine_mask,* 2. The hardware does not support it (pre-gen7). * 3. We already tried per-engine reset recently. */ -bool full_reset = true;if (!i915.enable_engine_reset) {DRM_INFO ("Engine reset disabled: Using full GPU reset.\n" );
@@ -2744,6 +2746,7 @@ void i915_handle_error(struct drm_device *dev, u32 engine_mask, i915.gpu_reset_promotion_time; engine->hangcheck.last_engine_reset_time = now; + }else {/* * Watchdog timeout always results
@@ -2792,6 +2795,8 @@ void i915_handle_error(struct drm_device *dev, u32 engine_mask,i915_error_wake_up (dev_priv, false); } +trace_i915_tdr_recovery_queued (dev, engine_mask, watchdog, full_reset); +/* * Gen 7: *
@@ -3158,6 +3163,7 @@ static void i915_hangcheck_elapsed(struct work_struct *work)for_each_ring (ring, dev_priv, i) {u64 acthd;u32 seqno; +u32 head;bool busy = true;semaphore_clear_deadlocks (dev_priv);
@@ -3234,7 +3240,10 @@ static void i915_hangcheck_elapsed(struct work_struct *work) ring->hangcheck.seqno = seqno; ring->hangcheck.acthd = acthd; + head =I915_READ_HEAD (ring); busy_count += busy; + +trace_i915_tdr_hang_check (ring, seqno, acthd, head, busy); }for_each_ring (ring, dev_priv, i) {
drivers/gpu/drm/i915/i915_trace.h +339 -0
345 lines not on this page · raw message
drivers/gpu/drm/i915/intel_lrc.c +19 -2
@@ -2285,7 +2285,7 @@ gen8_ring_save(struct intel_engine_cs *ring, struct drm_i915_gem_request *req,struct intel_context *ctx;int ret =0 ;int clamp_to_tail =0 ; -uint32_t head; +uint32_t head, old_head;uint32_t tail;uint32_t head_addr;uint32_t tail_addr;
@@ -2300,7 +2300,7 @@ gen8_ring_save(struct intel_engine_cs *ring, struct drm_i915_gem_request *req,* Read head from MMIO register since it contains the * most up to date value of head at this point. */ - head =I915_READ_HEAD (ring); + old_head = head =I915_READ_HEAD (ring);/* * Read tail from the context because the execlist queue
@@ -2357,6 +2357,9 @@ gen8_ring_save(struct intel_engine_cs *ring, struct drm_i915_gem_request *req, head |= (head_addr &HEAD_ADDR ); ring->saved_head = head; +trace_i915_tdr_engine_save (ring, old_head, + head, force_advance); +return 0 ; }
@@ -3264,6 +3267,19 @@ intel_execlists_TDR_get_current_request(struct intel_engine_cs *ring,CONTEXT_SUBMISSION_STATUS_NONE_SUBMITTED ; } +/* +* This may or may not be a sustained inconsistency. Most of the time +* it's only a matter of a transitory inconsistency during context +* submission/completion but if we happen to detect a sustained +* inconsistency then it helps to have more information. +*/ +if (status ==CONTEXT_SUBMISSION_STATUS_INCONSISTENT ) +trace_i915_tdr_inconsistency (ring, + hw_context, + hw_active, + sw_context, + tmpreq); +if (req) *req = tmpreq;
@@ -3328,6 +3344,7 @@ bool intel_execlists_TDR_force_CSB_check(struct drm_i915_private *dev_priv,spin_unlock_irqrestore (&engine->execlist_lock, flags); +trace_i915_tdr_forced_csb_check (engine, !!was_effective);wake_up_all (&engine->irq_queue);return !!was_effective;
drivers/gpu/drm/i915/intel_uncore.c +4 -0
@@ -1464,6 +1464,8 @@ static int gen6_do_reset(struct drm_device *dev)/* Spin waiting for the device to ack the reset request */ ret =wait_for ((__raw_i915_read32 (dev_priv,GEN6_GDRST ) &GEN6_GRDOM_FULL ) ==0 ,500 ); +trace_i915_tdr_gpu_reset_complete (dev); +intel_uncore_forcewake_reset (dev, true);return ret;
@@ -1620,6 +1622,8 @@ static int do_engine_reset_nolock(struct intel_engine_cs *engine)break ; } +trace_i915_tdr_engine_reset_complete (engine); +return ret; }
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 15/20 drm/i915: Port of Added scheduler support to __wait_request() calls
In reply to Tomas ElfThis is a partial port of the following patch from John Harrison's GPU scheduler patch series: (patch sent to Intel-GFX with the subject line "[Intel-gfx] [RFC 19/39] drm/i915: Added scheduler support to __wait_request() calls" on Fri 17 July 2015)
Author: John Harrison <[email protected]>
Date: Thu Apr 10 10:48:55 2014 +0100
Subject: drm/i915: Added scheduler support to __wait_request() calls
Removed all scheduler references and backported it to this baseline. The reason we need this is because Chris Wilson has pointed out that threads that don't hold the struct_mutex should not be thrown out of __i915_wait_request during TDR hang recovery. Therefore we need a way to determine which threads are holding the mutex and which are not.
--- drivers/gpu/drm/i915/i915_drv.h | 3 ++- drivers/gpu/drm/i915/i915_gem.c | 16 ++++++++++------ drivers/gpu/drm/i915/intel_display.c | 3 ++- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 4 files changed, 15 insertions(+), 9 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +2 -1
@@ -3054,7 +3054,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,unsigned reset_counter, bool interruptible, s64 *timeout, - struct intel_rps_client *rps); + struct intel_rps_client *rps, + bool is_locked);int __must_check i915_wait_request (struct drm_i915_gem_request *req);int i915_gem_fault (struct vm_area_struct *vma,struct vm_fault *vmf);int __must_check
drivers/gpu/drm/i915/i915_gem.c +10 -6
@@ -1206,6 +1206,8 @@ static int __i915_spin_request(struct drm_i915_gem_request *req)* @reset_counter: reset sequence associated with the given request * @interruptible: do an interruptible wait (normally yes) * @timeout: in - how long to wait (NULL forever); out - how much time remaining +* @rps: ... +* @is_locked: true = Caller is holding struct_mutex * * Note: It is of utmost importance that the passed in seqno and reset_counter * values have been read by the caller in an smp safe manner. Where read-side
@@ -1221,7 +1223,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,unsigned reset_counter, bool interruptible, s64 *timeout, - struct intel_rps_client *rps) + struct intel_rps_client *rps, + bool is_locked) { struct intel_engine_cs *ring = i915_gem_request_get_ring(req);struct drm_device *dev = ring->dev;
@@ -1455,7 +1458,7 @@ i915_wait_request(struct drm_i915_gem_request *req) ret =__i915_wait_request (req,atomic_read (&dev_priv->gpu_error.reset_counter), - interruptible,NULL ,NULL ); + interruptible,NULL ,NULL , true);if (ret)return ret;
@@ -1568,7 +1571,7 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,mutex_unlock (&dev->struct_mutex);for (i =0 ; ret ==0 && i < n; i++) ret =__i915_wait_request (requests[i], reset_counter, true, -NULL , rps); +NULL , rps, false);mutex_lock (&dev->struct_mutex);for (i =0 ; i < n; i++) {
@@ -3085,7 +3088,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)if (ret ==0 ) ret =__i915_wait_request (req[i], reset_counter, true, args->timeout_ns >0 ? &args->timeout_ns :NULL , - file->driver_priv); + file->driver_priv, false);i915_gem_request_unreference__unlocked (req[i]); }return ret;
@@ -3118,7 +3121,8 @@ __i915_gem_object_sync(struct drm_i915_gem_object *obj,atomic_read (&i915->gpu_error.reset_counter), i915->mm.interruptible,NULL , - &i915->rps.semaphores); + &i915->rps.semaphores, + true); /* Is the mutex always held by this thread at this point? */if (ret)return ret;
@@ -4077,7 +4081,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)if (target ==NULL )return 0 ; - ret =__i915_wait_request (target, reset_counter, true,NULL ,NULL ); + ret =__i915_wait_request (target, reset_counter, true,NULL ,NULL , false);if (ret ==0 )queue_delayed_work (dev_priv->wq, &dev_priv->mm.retire_work,0 );
drivers/gpu/drm/i915/intel_display.c +2 -1
@@ -11198,7 +11198,8 @@ static void intel_mmio_flip_work_func(struct work_struct *work)WARN_ON (__i915_wait_request (mmio_flip->req, mmio_flip->crtc->reset_counter, false,NULL , - &mmio_flip->i915->rps.mmioflips)); + &mmio_flip->i915->rps.mmioflips, + false));i915_gem_request_unreference__unlocked (mmio_flip->req); }
drivers/gpu/drm/i915/intel_ringbuffer.c +1 -1
@@ -2368,7 +2368,7 @@ int intel_ring_idle(struct intel_engine_cs *ring)return __i915_wait_request (req,atomic_read (&to_i915 (ring->dev)->gpu_error.reset_counter),to_i915 (ring->dev)->mm.interruptible, -NULL ,NULL ); +NULL ,NULL , true);/* Is the mutex always held by this thread at this point? */ }int intel_ring_alloc_request_extras (struct drm_i915_gem_request *request)
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 16/20 drm/i915: Fix __i915_wait_request() behaviour during hang detection.
In reply to Tomas ElfUse is_locked parameter in __i915_wait_request() to determine if a thread should be forced to back off and retry or if it can continue sleeping. Don't return -EIO from __i915_wait_request since that is bad for the upper layers, only -EAGAIN to signify reset in progress. (unless the driver is terminally wedged, in which case there's no mode of recovery left to attempt)
Also, use is_locked in trace_i915_gem_request_wait_begin() trace point for more accurate reflection of current thread's lock state.
--- drivers/gpu/drm/i915/i915_gem.c | 88 ++++++++++++++++++++++++++++++++------- drivers/gpu/drm/i915/i915_trace.h | 15 ++----- 2 files changed, 78 insertions(+), 25 deletions(-)
drivers/gpu/drm/i915/i915_gem.c +74 -14
@@ -1235,7 +1235,6 @@ int __i915_wait_request(struct drm_i915_gem_request *req,unsigned long timeout_expire;s64 before, now;int ret; -int reset_in_progress =0 ;WARN (!intel_irqs_enabled (dev_priv),"IRQs disabled" );
@@ -1252,7 +1251,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req,gen6_rps_boost (dev_priv, rps, req->emitted_jiffies);/* Record current time in case interrupted by signal, or wedged */ -trace_i915_gem_request_wait_begin (req); +trace_i915_gem_request_wait_begin (req, is_locked); before =ktime_get_raw_ns ();/* Optimistic spin for the next jiffie before touching IRQs */
@@ -1267,24 +1266,85 @@ int __i915_wait_request(struct drm_i915_gem_request *req,for (;;) {struct timer_list timer; +bool full_gpu_reset_completed_unlocked = false; +bool reset_in_progress_locked = false;prepare_to_wait (&ring->irq_queue, &wait, interruptible ?TASK_INTERRUPTIBLE :TASK_UNINTERRUPTIBLE ); -/* We need to check whether any gpu reset happened in between -* the caller grabbing the seqno and now ... */ - reset_in_progress = -i915_gem_check_wedge (ring->dev->dev_private,NULL , interruptible); +/* +* Rules for waiting with/without struct_mutex held during +* asynchronous TDR hang detection/recovery: +* +* ("reset in progress" = TDR has detected a hang, hang +* recovery may or may not have commenced) +* +* 1. Is the driver terminally wedged? If so, return -EIO since +* there is no point in having the caller retry - the driver +* is irrecoverably stuck. +* +* 2. Is this thread holding the struct_mutex and is any of the +* following true? +* +* a) Is any kind of reset in progress? +* b) Has a full GPU reset happened while this thread were +* sleeping? +* +* If so: +* Return -EAGAIN. The caller should interpret this as: +* Release struct_mutex, try to acquire struct_mutex +* (through i915_mutex_lock_interruptible(), which will +* fail as long as any reset is in progress) and retry the +* call to __i915_wait_request(), which hopefully will go +* better as soon as the hang has been resolved. +* +* 3. Is this thread not holding the struct_mutex and has a +* full GPU reset completed? (that is, the reset count has +* changed but there is currenly no full GPU reset in +* progress?) +* +* If so: +* Return 0. Since the request has been purged there is no +* requests left to wait for. Just go home. +* +* 4. Is this thread not holding the struct_mutex and is any +* kind of reset in progress? +* +* If so: +* This thread may keep on waiting. +*/ + +if (i915_terminally_wedged (&dev_priv->gpu_error)) { + ret = -EIO ; +break ; + } -if ((reset_counter !=atomic_read (&dev_priv->gpu_error.reset_counter)) || - reset_in_progress) { + reset_in_progress_locked = + (((i915_gem_check_wedge (ring->dev->dev_private,NULL , interruptible)) || + (reset_counter !=atomic_read (&dev_priv->gpu_error.reset_counter))) && + is_locked); -/* ... but upgrade the -EAGAIN to an -EIO if the gpu -* is truely gone. */ -if (reset_in_progress) - ret = reset_in_progress; -else - ret = -EAGAIN ; +if (reset_in_progress_locked) { +/* +* If the caller is holding the struct_mutex throw them +* out since TDR needs access to it. +*/ + ret = -EAGAIN ; +break ; + } + + full_gpu_reset_completed_unlocked = + (((reset_counter !=atomic_read (&dev_priv->gpu_error.reset_counter)) && + (!i915_reset_in_progress (&dev_priv->gpu_error)))); + +if (full_gpu_reset_completed_unlocked) { +/* +* Full GPU reset without holding the struct_mutex has +* completed - just return. If recovery is still in +* progress the thread will keep on sleeping until +* recovery is complete. +*/ + ret =0 ;break ; }
drivers/gpu/drm/i915/i915_trace.h +4 -11
@@ -591,8 +591,8 @@ DEFINE_EVENT(i915_gem_request, i915_gem_request_complete, ); TRACE_EVENT(i915_gem_request_wait_begin, - TP_PROTO(struct drm_i915_gem_request *req), - TP_ARGS(req), + TP_PROTO(struct drm_i915_gem_request *req, bool blocking), + TP_ARGS(req, blocking), TP_STRUCT__entry( __field(u32, dev)
@@ -601,25 +601,18 @@ TRACE_EVENT(i915_gem_request_wait_begin, __field(bool, blocking) ), -/* NB: the blocking information is racy since mutex_is_locked -* doesn't check that the current thread holds the lock. The only -* other option would be to pass the boolean information of whether -* or not the class was blocking down through the stack which is -* less desirable. -*/ TP_fast_assign ( struct intel_engine_cs *ring =i915_gem_request_get_ring (req); __entry->dev = ring->dev->primary->index; __entry->ring = ring->id; __entry->seqno =i915_gem_request_get_seqno (req); - __entry->blocking = -mutex_is_locked (&ring->dev->struct_mutex); + __entry->blocking = blocking; ),TP_printk ("dev=%u, ring=%u, seqno=%u, blocking=%s" , __entry->dev, __entry->ring, - __entry->seqno, __entry->blocking ?"yes (NB)" :"no" ) + __entry->seqno, __entry->blocking ?"yes" :"no" ) ); DEFINE_EVENT(i915_gem_request, i915_gem_request_wait_end,
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 17/20 drm/i915: Extended error state with TDR count, watchdog count and engine reset count
In reply to Tomas ElfThese new TDR-specific metrics have previously been added to i915_hangcheck_info() in debugfs. During design review Chris Wilson asked for these metrics to be added to the error state as well.
--- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_gpu_error.c | 6 ++++++ 2 files changed, 9 insertions(+)
drivers/gpu/drm/i915/i915_drv.h +3 -0
@@ -526,6 +526,9 @@ struct drm_i915_error_state {int hangcheck_score;enum intel_ring_hangcheck_action hangcheck_action;int num_requests; +int hangcheck_tdr_count; +int hangcheck_watchdog_count; +int hangcheck_reset_count;/* our own tracking of ring head and tail */ u32 cpu_ring_head;
drivers/gpu/drm/i915/i915_gpu_error.c +6 -0
@@ -304,6 +304,9 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,err_printf (m," hangcheck: %s [%d]\n" ,hangcheck_action_to_str (ring->hangcheck_action), ring->hangcheck_score); +err_printf (m," TDR count: %d\n" , ring->hangcheck_tdr_count); +err_printf (m," Watchdog count: %d\n" , ring->hangcheck_watchdog_count); +err_printf (m," Engine reset count: %d\n" , ring->hangcheck_reset_count); }void i915_error_printf (struct drm_i915_error_state_buf *e,const char *f, ...)
@@ -929,6 +932,9 @@ static void i915_record_ring_state(struct drm_device *dev, ering->hangcheck_score = ring->hangcheck.score; ering->hangcheck_action = ring->hangcheck.action; + ering->hangcheck_tdr_count = ring->hangcheck.tdr_count; + ering->hangcheck_watchdog_count = ring->hangcheck.watchdog_count; + ering->hangcheck_reset_count = ring->hangcheck.reset_count;if (USES_PPGTT (dev)) {int i;
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 18/20 drm/i915: TDR / per-engine hang recovery kernel docs
In reply to Tomas Elf--- Documentation/DocBook/gpu.tmpl | 476 ++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_irq.c | 8 +- 2 files changed, 483 insertions(+), 1 deletion(-)
Documentation/DocBook/gpu.tmpl +476 -0
@@ -4128,6 +4128,482 @@ int num_ioctls;</synopsis> </sect2> </sect1> + <!-- + TODO: + Create sections with no subsection listing. Deal with subsection + references as they appear in the text. + + How do we create inline references to functions with DocBook tags? + If you want to refer to a function it would be nice if you could tag + the function name so that the reader can click the tag while reading + and get straight to the DocBook page on that function. + --> + + <sect1> + <title>GPU hang management</title> + <para> + There are two sides to handling GPU hangs: <link linkend='detection' + endterm="detection.title"/> and <link linkend='recovery' + endterm="recovery.title"/>. In this section we will discuss how the driver + detect hangs and what it can do to recover from them. + </para> + + <sect2 id='detection'> + <title id='detection.title'>Detection</title> + <para> + There is no theoretically sound definition of what a GPU hang actually is, + only assumptions based on empirical observations. One such observation is + that if a batch buffer takes more than a certain amount of time to finish + then we would assume that it's hung. However, one problem with that + assumption is that the execution might be ongoing inside the batch buffer. + In fact, it's easy to determine whether or not execution is progressing + within a batch buffer. If taking that into account we could create a more + refined hang detection algorithm. Unfortunately, there is then the + complication that the execution might be stuck in a never-ending loop which + keeps execution busy for an unbounded amount of time. These are all + practical problems that we need to deal with when detecting a hang and + whatever hang detection algorithm we come up with will have a certain + probability of false positives. + </para> + <para> + The i915 driver currently supports two forms of hang + detection: + <orderedlist> + <listitem> + <link linkend='periodic_hang_checking' endterm="periodic_hang_checking.title"/> + </listitem> + <listitem> + <link linkend='watchdog' endterm="watchdog.title"/> + </listitem> + </orderedlist> + </para> + + <sect3 id='periodic_hang_checking'> + <title id='periodic_hang_checking.title'>Periodic Hang Checking</title> + <para> + The periodic hang checker is a work queue that keeps running in the + background as long as there is work outstanding that is pending execution. + i915_hangcheck_elapsed() implements the work function of the queue and is + executed at every hang checker invocation. + </para> + + <para> + While being scheduled the hang checker keeps track of a hang score for each + individual engine. The hang score is an indication of what degree of + severity a hang has reached for a certain engine. The higher the score gets + the more radical forms of intervention are employed to force execution to + resume. + </para> + + <para> + The hang checker is scheduled from two places: + </para> + + <orderedlist> + <listitem> + __i915_add_request(), after a new request has been added and is pending submission. + </listitem> + + <listitem> + i915_hangcheck_elapsed() itself, if work is still pending for any GPU + engine the hang checker is rescheduled. + </listitem> + + </orderedlist> + <para> + The periodic hang checker keeps track of the sequence number + progression of the currently executing requests on every GPU engine. If + they keep progressing in between every hang checker invocation this is + interpreted as the engine being active, the hang score is cleared and and + no intervention is made. If the sequence number has stalled for one or more + engines in between two hang checks that is an indication of one of two + things: + </para> + + <orderedlist> + <listitem> + There is no more work pending on the given engine. If there are no + threads waiting for request completion this is an indication that no more + hang checking is necessary and the hang checker is not rescheduled. If + there is someone waiting for request completion the hang checker is + rescheduled and the hang score is continually incremented. + </listitem> + + <listitem> + <para>The given engine is truly hung. In this case a number of hardware + state checks are made to determine what the most suitable course of action + is and a corresponding hang score incrementation is made to reflect the + current hang severity.</para> + </listitem> + </orderedlist> + + <para> + If the hang score of any engine reaches the hung threshold hang recovery is + scheduled by calling i915_handle_error() with a engine flag mask containing + the bits representing all currently hung engines. + </para> + + + <sect4> + <title>Context Submission State Consistency Checking</title> + + <para> + On top of this there is the context submission status consistency pre-check + in the hang checker that keeps track of driver/HW consistency. The + underlying problem that this pre-check is trying to solve is the fact that + on some occasions the driver does not receive the proper context event + interrupt upon context state changes. Specifically, this has been observed + following the completion of engine reset and the subsequent resubmission of + the fixed-up context. At this point the engine hang is unblocked and the + context completes and the hardware marks the context as complete in the + context status buffer (CSB) for the given engine. However, the interrupt + that would normally signal this to the driver is lost. What this means to + the driver is that it gets stuck waiting for context completion on the + given engine until reboot, stalling all further submissions to the engine + ELSP. + </para> + + <para> + The way to detect this is to check for inconsistencies between the context + submission state in the hardware as well as in the driver. What this means + is that the EXECLIST_STATUS register has to be checked for every engine. + From this register the ID of the currently running context can be extracted + as well as information about whether or not the engine is idle or not. This + information can then be compared against the current state of the execlist + queue for the given engine. If the hardware is idle but the driver has + pending contexts in the execlist queue for a prolonged period of time then + it's safe to assume that the driver/HW state is inconsistent. + </para> + + <para> + The way driver/HW state inconsistencies are rectified is by faking the + presumably lost context event interrupts simply by calling the execlist + interrupt handler manually. + </para> + + <para> + What this means to the periodic hang checker is the following: + </para> + + <orderedlist> + <listitem> + <para> + State consistency checking happens at the start of the hang check + procedure. If an inconsistency has been detected enough times (more + detections than the threshold level of I915_FAKED_CONTEXT_IRQ_THRESHOLD) + the hang checker will fake a context event interrupt. If there are + outstanding, unprocessed context events in the CSB buffer these will be + acted upon. + </para> + </listitem> + + <listitem> + <para> + As long as the driver/HW state has been determined to be inconsistent the + error handler will not be called. The reason for this is that the engine + recovery mode, which is the hang recovery mode that the driver prefers, is + not effective if context submissions does not work. If the driver/HW state + is inconsistent it might mean that the hardware is currently executing (and + might be hung in) a completely different context than the driver expects, which would lead to + unexpected pre-emptions, which might mean that trying to resubmit the + context that the driver has identified as hung might make the situation + worse. Therefore, before any recovery is scheduled the driver/HW state must + be confirmed as consistent and stable. + </para> + </listitem> + + <listitem> + <para> + If any inconsistent driver/HW states persist regardless of any attempts to + rectify the situation there is a final fall-back: In case the hang score on + any engine reaches twice that of the normal hang threshold the error + handler is called with no engine mask populated, meaning that a full GPU + reset is forced. Going for a full GPU reset in this case makes sense since + there are two problems that need fixing: 1) <emphasis role="bold">The GPU + is hung</emphasis> and 2) <emphasis role="bold">The driver/HW state is + inconsistent</emphasis>. The full GPU reset solves both of these problems + and does not require the driver/HW state to be consistent to begin with so + its a sensible choice in this situation. + </para> + </listitem> + + </orderedlist> + + </sect4> + </sect3> + + <sect3 id='watchdog'> + <title id='watchdog.title'>Watchdog Timeout</title> + <para> + Unlike the <link linkend='periodic_hang_checker'>periodic hang + checker</link> Watchdog Timeout is a mode of hang detection that relies on + the GPU hardware to notify the driver in the event of a hang. Another + dissimilarity is that this mode does not target every engine at all times + but rather targets individual batch buffers that have been selected by the + submitting application. The way this works is that a submitter can opt-in + to use Watchdog Timeout for a particular batch buffer is by setting the + Watchdog Timeout enablement flag for that batch buffer. By doing so the + driver will emit instructions in the ring buffer before the batch buffer + start instruction to enable the Watchdog HW timer and afterwards to cancel + the same timer. The purpose of this is to keep track of how long the + execution stays inside the batch buffer once the execution reaches that + point. If the execution takes to long to clear the batch buffer and the + preset Watchdog Timer Threshold elapses the GPU hardware will fire a + Watchdog Timeout interrupt to the driver, which is interpreted as current + batch buffer for the given engine being hung. Thus, hang detection in this + case is purely interrupt-driven and the driver is free to do other things. + </para> + + <para> + Once the GT interrupt handler receives the Watchdog Timeout interrupt it + then proceeds by making a direct call to i915_handle_error() with + information about which engine is hung and by setting the dedicated + watchdog priority flag that allows the error handler to circumvent the + normal hang promotion logic that applies to hang detections originating + from the periodic hang checker. + </para> + + <para> + In order to enable this Watchdog Timeout for a particular batch buffer + userland libDRM has to enable the corresponding bit contained in + I915_EXEC_ENABLE_WATCHDOG in the batch buffer flag bitmask. This feature is + disabled by default and therefore it operates purely on an opt-in basis + from userland's point of view. + </para> + + </sect3> + + </sect2> + + <sect2 id='recovery'> + <title id='recovery.title'>Recovery</title> + <para> + Once a hang has been detected, either through periodic hang checking or + Watchdog Timeout, the error handler (i915_handle_error) takes over and + decices what to do from there on. Generally speaking there are two modes of + hang recovery that the error handler can choose from: + + <orderedlist> + <listitem> + <link linkend='engine_reset' endterm="engine_reset.title"/> + </listitem> + <listitem> + <link linkend='GPU_reset' endterm="GPU_reset.title"/> + </listitem> + </orderedlist> + + Exactly what recovery mode the hang is promoted to depends on a number of factors: + </para> + + <literallayout></literallayout> + <itemizedlist> + <listitem> + <para> + <emphasis role="bold"> + Did the caller say that a hang had been detected but did not specifically ask for engine reset? + </emphasis> + If the wedged parameter is set in the call to i915_handle_error() but the + engine_mask parameter is set to 0 it means that we need to do some kind of + hang recovery but no engine is specified. In that case the outcome will + always be an attempt to do a GPU reset. + </para> + </listitem> + + <listitem> + <literallayout></literallayout> + <para> + <emphasis role="bold"> + Did the caller say that a hang had been detected and specify at least one hung engine? + </emphasis> + If one or more engines have been specified as hung the first attempt will + always be to do an engine reset of those hung engines. There are two + reasons why an GPU reset would be carried out instead of a simple engine + reset: + </para> + <orderedlist> + + <listitem> + <para> + An engine reset was carried out on the same engine too recently. What + constitutes "too recent" is determined by the i915 module parameter + gpu_reset_promotion_time. If two engine resets were attempted within the + time window defined by this module parameter it is decided that the + previous engine reset was ineffective and therefore there is no point in + trying another one. Thus, a full GPU reset will be done instead. + </para> + </listitem> + + <listitem> + <para> + An engine reset was carried out but failed. In this case the hang recovery + path (i915_error_work_func) would go straight from the failed engine reset + attempt (i915_reset_engine call) to a full GPU reset without delay. + </para> + </listitem> + + </orderedlist> + </listitem> + + <listitem> + <literallayout></literallayout> + <literallayout></literallayout> + <para> + <emphasis role="bold"> + Did the Watchdog Timeout detect the hang? + </emphasis> + In case of the Watchdog Timeout calling the error handler the dedicated + watchdog parameter will be set and this forces the error handler to only + consider engine reset and not full GPU reset. We will only promote to full + GPU reset if the driver itself, based on its own hang detection mechanism, + has detected a persisting hang that will not be resolved by an engine hang. + Watchdog Timeout is user-controlled and is therefore not trusted the same + way. + </para> + <literallayout></literallayout> + </listitem> + </itemizedlist> + + <para> + When the error handler reaches a decision of what hang recovery mode to use + it sets up the corresponding reset in progress flag. There is one main + reset in progress flag for GPU resets as well as one dedicated reset in + progress flag in each hangcheck struct for each engine. After that the + error handler schedules the actual hang recovery work queue, which ends up + in i915_error_work_func, which is the function that grabs all necessary + locks and actually calls the main hang recovery functions. For all engines + that have their respective error in progress flags the <link + linkend='engine_reset' endterm="engine_reset.title">engine reset + path</link> is taken for each engine in sequence. If the GPU reset in + progress flag is set no attempts at carrying out engine resets are made and + instead the legacy <link linkend='GPU_reset' endterm="GPU_reset.title">full + GPU reset path</link> is taken. + </para> + + <sect3 id='engine_reset'> + <title id='engine_reset.title'>Engine Reset</title> + <para> + The engine reset path is implemented in i915_reset_engine and the following + is a summary of how that function operates: + + <orderedlist> + <listitem> + <para> + Get currently running context and check context submission status + consistency. If the currently running (hung) context is in an inconsistent + state there is really no reason why the execution should be at this point + since the hang checker does a consistency check before scheduling hang + recovery unless the state has changed since hang recovery was scheduled, in + which case the engine is not truly hung. If so, do early exit. + </para> + </listitem> + + <listitem> + <para> + Force engine to idle and save the current context image. On gen8+ this is + done by setting the reset request bit in the reset control register. On + gen7 and earlier gens the MI_MODE register in combination with the ring + control register has to be used to disable the engine. + </para> + </listitem> + + <listitem> + <para> + Save the head MMIO register value and nudge it to the following valid + instruction in the ring buffer following the batch buffer start instruction + of the currently hung batch buffer. + </para> + </listitem> + + <listitem> + <para> + Reset engine. + </para> + </listitem> + + <listitem> + <para> + Call the init() function for the previously hung engine, which should + reapply HW workarounds and carry out other essential state + reinitialization. + </para> + </listitem> + + <listitem> + <para> + Write the previously nudged head register value to both MMIO and context registers. + </para> + </listitem> + + <listitem> + <para> + Submit updated context to ELSP in order to force execution to resume (gen8 only). + </para> + </listitem> + + <listitem> + <para> + Clear reset in progress engine flag and wake up all threads waiting for requests to complete. + </para> + </listitem> + </orderedlist> + + <literallayout></literallayout> + + <para> + The intended outcome of an engine reset is that the hung batch buffer is + dropped by forcing the execution to resume following the batch buffer start + instruction in the ring buffer. This should only affect the hung engine and + none other. No reinitialization aside from a subset of the state for the + hung engine should happen and pending work should be retained requiring no + further resubmissions. + </para> + + </para> + </sect3> + + <sect3 id='GPU_reset'> + <title id='GPU_reset.title'>GPU reset</title> + <para> + Basically the GPU reset function, i915_reset, does 3 things: + <literallayout></literallayout> + + <orderedlist> + <listitem> + <para> + Reset GEM. + </para> + </listitem> + + <listitem> + <para> + Do the actual GPU reset. + </para> + </listitem> + + <listitem> + <para> + Reinitialize the GEM part of the driver, including purging all pending work, reinitialize the engines and ring setup and more. + </para> + </listitem> + </orderedlist> + + <literallayout></literallayout> + + The intended outcome of a GPU reset is that all work, including the hung + batch buffer as well as all batch buffers following it, is dropped and the + GEM part of the driver is reinitialized following the GPU reset. This means + that the driver goes to an idle state together with the hardware and should + start over from a state in which it is ready to accept more work and move + forwards from there. All pending work will have to be resubmitted by the + submitting application. + + </para> + </sect3> + + </sect2> + + </sect1> + <sect1> <title> Tracing </title> <para>
drivers/gpu/drm/i915/i915_irq.c +7 -1
@@ -2682,7 +2682,9 @@ static void i915_report_and_clear_eir(struct drm_device *dev)* or if one of the current engine resets fails we fall * back to legacy full GPU reset. * @watchdog: true = Engine hang detected by hardware watchdog. +* * @wedged: true = Hang detected, invoke hang recovery. +* * @fmt, ...: Error message describing reason for error. * * Do some basic checking of register state at error time and
@@ -3134,7 +3136,11 @@ ring_stuck(struct intel_engine_cs *ring, u64 acthd)return HANGCHECK_HUNG ; } -/* +/** +* i915_hangcheck_elapsed - hang checker work function +* +* @work: Work item containing reference to private DRM struct. +* * This is called when the chip hasn't reported back with completed * batchbuffers in a long time. We keep track per ring seqno progress and * if there are no progress, hangcheck score for that ring is increased.
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 19/20 drm/i915: drm/i915 changes to simulated hangs
In reply to Tomas ElfFrom: Tim Gore <[email protected]>
Simulated hangs, as used by drv_hangman and some other IGT tests, are not handled correctly with the new per-engine hang recovery mode. This patch fixes several issues needed to get them working in the execlist case.
1) The "simulated" hang is effected by not submitting a particular batch buffer to the hardware. In this way it is not handled by the hardware are hence remains in the software queue, leading the TDR mechanism to declare a hang. The place where the submission of the batch was being blocked was in intel_logical_ring_advance_and_submit. Because this means the request never enters the execlist_queue, the TDR mechanism does not detect a hang and the situation is never cleared. Also, blocking the batch buffer here is before the intel_ctx_submit_request object gets allocated. During TDR we need to actually complete the submission process to unhang the ring, but we are not in user context so cannot allocate the request object. To overcome both these issues I moved the place where submission is blocked to execlists_context_unqueue. This means that the request enters the ring->execlist_queue, so the TDR mechanism detects the hang and can resubmit the request after the stop_rings bit is cleared.
2) A further problem arises from a workaround in i915_hangcheck_sample to deal with a context submission status of "...INCONSISTENT" being reported by intel_execlists_TDR_get_current_request() when the hardware is idle. A simulated hang, because it causes the sw and hw context id's to be out of sync, results in a context submission status of "...INCONSISTENT" being reported, triggering this workaround which resubmits the batch and clears the hang, avoiding a ring reset. But we want the ring reset to occur, since this is part of what we are testing. So, I have made intel_execlists_TDR_get_current_request() aware of simulated hangs, so that it returns a status of OK in this case. This avoids the workaround being triggered, leading to the TDR mechanism declaring a ring hang and doing a ring reset.
Issue: VIZ-5488
--- drivers/gpu/drm/i915/i915_drv.c | 10 ++++++++++ drivers/gpu/drm/i915/intel_lrc.c | 26 +++++++++++++++++++++----- 2 files changed, 31 insertions(+), 5 deletions(-)
drivers/gpu/drm/i915/i915_drv.c +10 -0
@@ -1175,6 +1175,16 @@ int i915_reset_engine(struct intel_engine_cs *engine) } } +/* Clear any simulated hang flags */ +if (dev_priv->gpu_error.stop_rings) { +DRM_INFO ("Simulated gpu hang, reset stop_rings bits %08x\n" , + (0x1 << engine->id)); + dev_priv->gpu_error.stop_rings &= ~(0x1 << engine->id); +/* if all hangs are cleared, then clear the ALLOW_BAN/ERROR bits */ +if ((dev_priv->gpu_error.stop_rings & ((1 <<I915_NUM_RINGS ) -1 )) ==0 ) + dev_priv->gpu_error.stop_rings =0 ; + } +/* Sample the current ring head position */ head =I915_READ_HEAD (engine) &HEAD_ADDR ;
drivers/gpu/drm/i915/intel_lrc.c +21 -5
@@ -641,6 +641,16 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring, bool tdr_res } } +/* Check for a simulated hang request */ +if (intel_ring_stopped (ring)) { +/* +* Mark the request at the head of the queue as submitted but +* dont actually submit it. +*/ + req0->elsp_submitted++; +return ; + } +WARN_ON (req1 && req1->elsp_submitted && !tdr_resubmission);execlists_submit_requests (req0, req1, tdr_resubmission);
@@ -1006,16 +1016,12 @@ static int logical_ring_wait_for_space(struct drm_i915_gem_request *req,static void intel_logical_ring_advance_and_submit (struct drm_i915_gem_request *request) { -struct intel_engine_cs *ring = request->ring;struct drm_i915_private *dev_priv = request->i915;intel_logical_ring_advance (request->ringbuf); request->tail = request->ringbuf->tail; -if (intel_ring_stopped (ring)) -return ; -if (dev_priv->guc.execbuf_client)i915_guc_submit (dev_priv->guc.execbuf_client, request);else
@@ -3250,7 +3256,17 @@ intel_execlists_TDR_get_current_request(struct intel_engine_cs *ring, }if (tmpctx) { - status = ((hw_context == sw_context) && hw_active) ? +/* +* Check for simuated hang. In this case the head entry in the +* sw execlist queue will not have been submitted to the ELSP, so +* the hw and sw context id's may well disagree, but we still want +* to proceed with hang recovery. So we return OK which allows +* the TDR recovery mechanism to proceed with a ring reset. +*/ +if (intel_ring_stopped (ring)) + status =CONTEXT_SUBMISSION_STATUS_OK ; +else + status = ((hw_context == sw_context) && hw_active) ?CONTEXT_SUBMISSION_STATUS_OK :CONTEXT_SUBMISSION_STATUS_INCONSISTENT ; }else {
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 19/20] drm/i915: drm/i915 changes to simulated hangs
Quoted 5 lines · Tomas Elf
From: Tim Gore <[email protected]>
Simulated hangs, as used by
drv_hangmanand some other IGT tests, are not handled correctly with the new per-engine hang recovery mode. This patch fixes several issues needed to get them working in the execlist case.
I'm working on eliminating this from the kernel and igt since we can use real hangs on all platforms but gen2.
-Chris
Signature · 5 lines
Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 19/20] drm/i915: drm/i915 changes to simulated hangs
Quoted 9 lines · Chris Wilson
Quoted 5 lines · Tomas Elf
From: Tim Gore <[email protected]>
Simulated hangs, as used by
drv_hangmanand some other IGT tests, are not handled correctly with the new per-engine hang recovery mode. This patch fixes several issues needed to get them working in the execlist case.I'm working on eliminating this from the kernel and igt since we can use real hangs on all platforms but gen2.
great, I can probably ignore this patch.
regards
Arun
Quoted 2 lines
-Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 20/20 drm/i915: Enable TDR / per-engine hang recovery
In reply to Tomas ElfThis is the final enablement patch for per-engine hang recovery. It sets up per-engine hang recovery to be used per default in favour of full GPU reset. Legacy full GPU reset will no longer be the preferred mode of hang recovery and will only be used as a fall-back in case of frequent hangs on individual engines or in the case of engine hang recovery failures.
--- drivers/gpu/drm/i915/i915_params.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/i915_params.c +2 -2
@@ -35,7 +35,7 @@ struct i915_params i915 __read_mostly = { .enable_fbc = -1, .enable_execlists = -1, .enable_hangcheck = true, - .enable_engine_reset = false, + .enable_engine_reset = true, .gpu_reset_promotion_time = 10, .enable_ppgtt = -1, .enable_psr = 0,
@@ -115,7 +115,7 @@ MODULE_PARM_DESC(enable_engine_reset,"Enable GPU engine hang recovery mode. Used as a soft, low-impact form " "of hang recovery that targets individual GPU engines rather than the " "entire GPU" - "(default: false)"); + "(default: true)");module_param_named (gpu_reset_promotion_time, i915.gpu_reset_promotion_time, int,0644 );
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx