Tvrtko uncovered a fun issue with recovering from a wedge device. In his
tests, he wedged the driver by injecting an unrecoverable hang whilst a
batch was spinning. As we reset the gpu in the middle of the spinner,
when resumed it would continue on from the next instruction in the ring
and write it's breadcrumb. However, on wedging we updated our
bookkeeping to indicate that the GPU had completed executing and would
restart from after the breadcrumb; so the emission of the stale
breadcrumb from before the reset came as a bit of a surprise.
A simple fix is to when rebinding the context into the GPU, we update
the ring register state in the context image to match our bookkeeping.
We already have to update the RING_START and RING_TAIL, so updating
RING_HEAD as well is trivial. This works because whenever we unbind the
context, we keep the bookkeeping in check; and on wedging we unbind all
contexts.
Testcase: igt/gem_eio
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180327210136.16750-1-chris@chris-wilson.co.uk
Tested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
GVT-g dispatches request to host i915 and depends on i915 notify
ring interrupt mechanism to check completion of request.
For now MI_USER_INTERRUPT in guest requests is passed through
in GVT-g cmd parser and i915 does not use it, which causes
unnecessary interrupt handling in i915.
On the other hand, if several requests from guest are combined into
one request in and contain MI_USER_INTERRUPT in the middle of
combined request. GVT-g still has to wait on the whole request to
complete to inject user interrupts to guest.
This patch makes all the MI_USER_INTERRUPT nop to save some interrupt
handling.
Here is test result to run glmark2 on guest for 10 seconds:
host master interrupts number is reduced from 16021 to 11162
host user interrupts number is reduced from 7936 to 3536
v2:
- revise commit message. (Kevin)
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Zhipeng Gong <zhipeng.gong@intel.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Addresses-Coverity-ID: 1466154 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
- Display fixes for booting with MST hub lid closed and display
freezing after hibernation (fd.o bugs 105470 & 105196)
- Fix for a very rare interrupt handling race resulting in GPU hang
* tag 'drm-intel-next-fixes-2018-03-27' of git://anongit.freedesktop.org/drm/drm-intel:
drm/i915: Fix hibernation with ACPI S0 target state
drm/i915/execlists: Use a locked clear_bit() for synchronisation with interrupt
drm/i915: Specify which engines to reset following semaphore/event lockups
drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.
In some places we end up converting switch statements to a series of
if/else, particularly when introducing helper functions to handle a
group of cases. It's tempting to either leave a wrong warning (since now
we don't have a switch case anymore) or to convert to WARN(1, ...),
but we can just provide a better message and avoid the doubt when such
conversions arrise.
Introducing a warning inside i915_driver_load() just for tests we get:
[ 4535.233717] Missing case (ret == 0)
[ 4535.233868] WARNING: CPU: 1 PID: 795 at drivers/gpu/drm/i915/i915_drv.c:1341 i915_driver_load+0x42/0x10e0 [i915]
which is clear enough.
v2: remove __func__ since this is already on the warning.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180319173720.6974-1-lucas.demarchi@intel.com
When cancelling the requests and clearing out the ports following a
successful preemption completion, also clear the active flag. I had
assumed that all preemptions would be followed by an immediate dequeue
(preserving the active user flag), but under rare circumstances we may
be triggering a preemption for the second port only for it to have
completed before the preemotion kicks in; leaving execlists->active set
even though the system is now idle.
We can clear the flag inside the common execlists_cancel_port_requests()
as the other users also expect the semantics of active being cleared.
Fixes: f6322eddaf ("drm/i915/preemption: Allow preemption between submission ports")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180324125829.27026-1-chris@chris-wilson.co.uk
After
commit dd9f31c7a3
Author: Imre Deak <imre.deak@intel.com>
Date: Wed Aug 16 17:46:07 2017 +0300
drm/i915/gen9+: Set same power state before hibernation image
save/restore
during hibernation/suspend the power domain functionality got disabled,
after which resume could leave it incorrectly disabled if the ACPI
target state was S0 during suspend and i915 was not loaded by the loader
kernel.
This was caused by not considering if we resumed from hibernation as the
condition for power domains reiniting.
Fix this by simply tracking if we suspended power domains during system
suspend and reinit power domains accordingly during resume. This will
result in reiniting power domains always when resuming from hibernation,
regardless of the platform and whether or not i915 is loaded by the
loader kernel.
The reason we didn't catch this earlier is that the enabled/disabled
state of power domains during PMSG_FREEZE/PMSG_QUIESCE is platform
and kernel config dependent: on my SKL the target state is S4
during PMSG_FREEZE and (with the driver loaded in the loader kernel)
S0 during PMSG_QUIESCE. On the reporter's machine it's S0 during
PMSG_FREEZE but (contrary to this) power domains are not initialized
during PMSG_QUIESCE since i915 is not loaded in the loader kernel, or
it's loaded but without the DMC firmware being available.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105196
Reported-and-tested-by: amn-bas@hotmail.com
Fixes: dd9f31c7a3 ("drm/i915/gen9+: Set same power state before hibernation image save/restore")
Cc: amn-bas@hotmail.com
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180322143642.26883-1-imre.deak@intel.com
(cherry picked from commit 0f90603c33)
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
These tables are used on voltage vswing sequence initialization on
Icelake.
The swing_sel on the spec's table is defined in a 4 bits binary like
1010. However the register bits are split in upper 1 bit swing_sel
and lower 3 bits swing sel.
In this table here we store this value as a single value in hex like
it is mentioned in the Bspec and split it to the upper and lower bit
values only while programming the registers.
For instance: b1010 is written as 0xA and then while writing to the
register, the upper 1 bit is obtained by (0xA & 0x8) and shifting by
appropriate bits while lower 3 bits are obtained by (0xA & 0x7) and
shifting by appropriate bits.
Some of the columns need to be updated after the spec is updated.
v5 (from Paulo):
* Checkpatch fixes.
v4 (from Paulo):
* Fix minor typo
* Coding style conformance
v3:
* Get rid of HDMI/DVI tables, same as DP (Paulo)
* Use combo_phy in ddi buf trans table defs (Paulo)
v2:
* Added DW4_scaling_hex column to the translation tables (Rodrigo)
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180323172419.24911-3-paulo.r.zanoni@intel.com
This patch defines register definitions required for ICL voltage
vswing programming for Combo PHY DDI Ports. It uses the same bit
definitions and macros as the CNL voltage swing sequences.
v8 (from Paulo):
* Rebase.
v7:
* Kill _MMIIO_PORT2_LN (Paulo)
v6:
* Replace some spaces with TAB (Paulo)
v5:
* Use _PORT instead of _PICK (Paulo)
* Remove DW7 defs for ICL, not used (Paulo)
v4:
* Rebase after _PICK was used instead of _PORT3
* Use _PICK for _MMIO_PORT2 since address of B is less
than address of A so cant use the math (Paulo)
v3:
* Make changes to the existing macro in a diff patch (Paulo)
v2:
* Add new defs fro ICL regs (Paulo)
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180323172419.24911-2-paulo.r.zanoni@intel.com
After
commit dd9f31c7a3
Author: Imre Deak <imre.deak@intel.com>
Date: Wed Aug 16 17:46:07 2017 +0300
drm/i915/gen9+: Set same power state before hibernation image
save/restore
during hibernation/suspend the power domain functionality got disabled,
after which resume could leave it incorrectly disabled if the ACPI
target state was S0 during suspend and i915 was not loaded by the loader
kernel.
This was caused by not considering if we resumed from hibernation as the
condition for power domains reiniting.
Fix this by simply tracking if we suspended power domains during system
suspend and reinit power domains accordingly during resume. This will
result in reiniting power domains always when resuming from hibernation,
regardless of the platform and whether or not i915 is loaded by the
loader kernel.
The reason we didn't catch this earlier is that the enabled/disabled
state of power domains during PMSG_FREEZE/PMSG_QUIESCE is platform
and kernel config dependent: on my SKL the target state is S4
during PMSG_FREEZE and (with the driver loaded in the loader kernel)
S0 during PMSG_QUIESCE. On the reporter's machine it's S0 during
PMSG_FREEZE but (contrary to this) power domains are not initialized
during PMSG_QUIESCE since i915 is not loaded in the loader kernel, or
it's loaded but without the DMC firmware being available.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105196
Reported-and-tested-by: amn-bas@hotmail.com
Fixes: dd9f31c7a3 ("drm/i915/gen9+: Set same power state before hibernation image save/restore")
Cc: amn-bas@hotmail.com
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180322143642.26883-1-imre.deak@intel.com
After resetting the GPU (or subset of engines), call synchronize_irq()
to flush any pending irq before proceeding with the cleanup. For a
device level reset, we disable the interupts around the reset, but when
resetting just one engine, we have to avoid such global disabling. This
leaves us open to an interrupt arriving for the engine as we try to
reset it. We already do try to flush the IIR following the reset, but we
have to ensure that the in-flight interrupt does not land after we start
cleaning up after the reset; enter synchronize_irq().
As it current stands, we very rarely, but fatally, see sequences such as:
2.... 57964564us : execlists_reset_prepare: rcs0
2.... 57964613us : execlists_reset: rcs0 seqno=424
0d.h1 57964615us : gen8_cs_irq_handler: rcs0 CS active=1
2d..1 57964617us : __i915_request_unsubmit: rcs0 fence 29:1056 <- global_seqno 1060
2.... 57964703us : execlists_reset_finish: rcs0
0..s. 57964705us : execlists_submission_tasklet: rcs0 awake?=1, active=0, irq-posted?=1
v2: Move the sync into the execlists reset handler so that we coordinate
the flush with disabling the interrupt handling and canceling the
pending interrupt.
v3: Just use synchronize_hardirq() to avoid the might_sleep(), we do not
yet have threaded-irq to worry about.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Jeff McGee <jeff.mcgee@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180322073533.5313-4-chris@chris-wilson.co.uk
Reviewed-by: Jeff McGee <jeff.mcgee@intel.com>
GVT regression fix that caused guest VM GPU hang.
Fix for race conditions in declaring GPU wedged (hit in CI).
* tag 'drm-intel-next-fixes-2018-03-22' of git://anongit.freedesktop.org/drm/drm-intel:
drm/i915/gvt: force to set all context control bits from guest
drm/i915/gvt: Update PDPs after a vGPU mm object is pinned.
drm/i915/gvt: Invalidate vGPU PPGTT mm objects during a vGPU reset.
drm/i915/kvmgt: Handle kzalloc failure
drm/i915/gvt: fix spelling mistake: "destoried" -> "destroyed"
drm/i915/gvt: Remove reduntant printing of untracked mmio
drm/i915/pmu: Work around compiler warnings on some kernel configs
drm/i915: Only call tasklet_kill() on the first prepare_reset
drm/i915: Wrap engine->schedule in RCU locks for set-wedge protection
drm/i915/icl: do not save DDI A/E sharing bit for ICL
Both request_submit and request_unsubmit deal with transferring the
request from the client's timeline onto the execution timeline and back
again. As both functions deal with a pair of timeline's, using a
shorthand for just one of them is slightly confusing, especially as the
different functions use the shorthand for the alternate timeline.
Instead, use the full version of each timeline so it should be easier to
keep track of the transfer between the request/client and the engine.
v2: Refactor the common lock+list_move
v3: Be clear we require the other timeline list to be locked as well.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180322131034.6036-1-chris@chris-wilson.co.uk
