We still have reports of missed irqs even on Sandybridge with the
HWSTAM workaround in place. Testing by the bug reporter gets rid of
them with the forcewake voodoo and no HWSTAM writes.
Because I've slightly botched the rebasing I've left out the ACTHD
readback which is also required to get IVB working. Seems to still
work on the tester's machine, so I think we should go with the more
minmal approach on SNB. Especially since I've only found weak evidence
for holding forcewake while waiting for an interrupt to arrive, but
none for the ACTHD readback.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=45181
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=45332
Tested-by: Nicolas Kalkhof nkalkhof()at()web.de
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Back-merge from drm-fixes into drm-intel-next to sort out two things:
- interlaced support: -fixes contains a bugfix to correctly clear
interlaced configuration bits in case the bios sets up an interlaced
mode and we want to set up the progressive mode (current kernels
don't support interlaced). The actual feature work to support
interlaced depends upon (and conflicts with) this bugfix.
- forcewake voodoo to workaround missed IRQ issues: -fixes only enabled
this for ivybridge, but some recent bug reports indicate that we
need this on Sandybridge, too. But in a slightly different flavour
and with other fixes and reworks on top. Additionally there are some
forcewake cleanup patches heading to -next that would conflict with
currrent -fixes.
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Chris Wilson and me have again stared at funny error states and it's
been pretty clear from the start that something was seriously amiss.
The seqnos last seen by the cpu were a few hundred behind those that
the gpu could have possibly emitted last before it died ...
Chris now tracked it down (hopefully, definit verdict's still out),
but in hindsight we'd have found the bug by simply dumping the cpu
side tracking of the ring head and tail registers.
Fix this and prevent an identical time-waster in the future.
Because the hangs always involved semaphores in one way or another,
we've tried to dump the mbox registers, but couldn't find any
inconsistencies. Still, dump them too.
Reviewed-and-wanted-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
As the buffer is not necessarily accessible through the GTT at the time
of a GPU hang, and capturing some of its contents is far more valuable
than skipping it, provide a clflushed fallback read path. We still
prefer to read through the GTT as that is more consistent with the GPU
access of the same buffer. So example it will demonstrate any errorneous
tiling or swizzling of the command buffer as seen by the GPU.
This becomes necessary with use of CPU relocations and lazy GTT binding,
but could potentially happen anyway as a result of a pathological error.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This was pretty handy when figuring out what exactly went wrong with
ppgtt and it might also be useful when we stop filling the entire gart
with scratch page entries.
Also add the gen6+ DONE reg while at it.
v2: Chris Wilson suggested to allocate the error_state with kzalloc
for better paranoia. Also kill existing spurious clears of the
error_state while at it.
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Since quite a while we also the basic output configuration in the
error_state, so it should contain enough information to diagnose
these MI_WAIT hangs.
Reviewed-and-tested-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Based on a patch by Ben Widawsky, but with different colors
for the bikeshed.
In contrast to Ben's patch this one doesn't add the fault regs.
Afaics they're for the optional page fault support which
- we're not enabling
- and which seems to be unsupported by the hw team. Recent bspec
lacks tons of information about this that the public docs released
half a year back still contain.
Also dump ring HEAD/TAIL registers - I've recently seen a few
error_state where just guessing these is not good enough.
v2: Also dump INSTPM for every ring.
v3: Fix a few really silly goof-ups spotted by Chris Wilson.
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The code already got unwieldy and we want to dump more per-ring
registers.
Only functional change is that we now also capture the video
ring registers on ilk.
v2: fixup a refactor fumble spotted by Chris Wilson.
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
... and add a helpr function for the places where we want a flag.
This way we can use ring->id to index into arrays.
v2: Resurrect the missing beautification-space Chris Wilson noted.
I'm moving this space around because I'll reuse ring_str in the next
patch.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This is only relevant when using module unloading, and really only helps
get rid of a probably benign warning.
I can't remember if I sent this out already, but it's not turning up in
any of my searches.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With the new ducttape of much finer quality, this seems to be no
longer necessary.
Tested on my ivb and snb machine with the usual suspects of testcases.
(v2 by keithp -- limited change to IVB only for now)
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Keith Packard <keithp@keithp.com>
If our semaphore logic gets confused and we have a ring stuck waiting
for one, there's a decent chance it'll just execute garbage when being
kicked. Also, kicking the ring obscures the place where the error
first occured, making error_state decoding much harder.
So drop this an let gpu reset handle this mess in a clean fashion.
In contrast, kicking rings stuck on MI_WAIT is rather harmless, at
worst there'll be a bit of screen-flickering. There's also old
broken userspace out there which needs this as a work-around.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@hchris-wilson.co.uk>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Keith Packard <keithp@keithp.com>
* 'drm-intel-next' of git://people.freedesktop.org/~keithp/linux:
drm/i915: check ACTHD of all rings
drm/i915: DisplayPort hot remove notification to audio driver
drm/i915: HDMI hot remove notification to audio driver
drm/i915: dont trigger hotplug events on unchanged ELD
drm/i915: rename audio ELD registers
drm/i915: fix ELD writing for SandyBridge
Merge in the upstream tree to bring in the mainline fixes.
Conflicts:
drivers/gpu/drm/exynos/exynos_drm_fbdev.c
drivers/gpu/drm/nouveau/nouveau_sgdma.c
Otherwise each driver would need to keep the information inside
their own framebuffer object structure. Also add offsets[]. BOs
on the other hand are driver specific, so those can be kept in
driver specific structures.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Otherwise hangcheck spuriously fires when running blitter/bsd-only
workloads.
Contrary to a similar patch by Ben Widawsky this does not check
INSTDONE of the other rings. Chris Wilson implied that in a failure to
detect a hang, most likely because INSTDONE was fluctuating. Thus only
check ACTHD, which as far as I know is rather reliable. Also, blitter
and bsd rings can't launch complex tasks from a single instruction
(like 3D_PRIM on the render with complex or even infinite shaders).
This fixes spurious gpu hang detection when running
tests/gem_hangcheck_forcewake on snb/ivb.
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Keith Packard <keithp@keithp.com>
So don't forget to restore them on resume and dump them into
the error state.
Cc: stable@kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Keith Packard <keithp@keithp.com>
This patch closes the following race:
We get a PM interrupt A, mask it, set dev_priv->iir = PM_A and kick of the
work item. Scheduler isn't grumpy, so the work queue takes rps_lock,
grabs pm_iir = dev_priv->pm_iir and pm_imr = READ(PMIMR). Note that
pm_imr == pm_iir because we've just masked the interrupt we've got.
Now hw sends out PM interrupt B (not masked), we process it and mask
it. Later on the irq handler also clears PMIIR.
Then the work item proceeds and at the end clears PMIMR. Because
(local) pm_imr == pm_iir we have
pm_imr & ~pm_iir == 0
so all interrupts are enabled.
Hardware is still interrupt-happy, and sends out a new PM interrupt B.
PMIMR doesn't mask B (it does not mask anything), PMIIR is cleared, so
we get it and hit the WARN in the interrupt handler (because
dev_priv->pm_iir == PM_B).
That's why I've moved the
WRITE(PMIMR, 0)
up under the protection of the rps_lock. And write an uncoditional 0
to PMIMR, because that's what we'll do anyway.
This races looks much more likely because we can arbitrarily extend
the window by grabing dev->struct mutex right after the irq handler
has processed the first PM_B interrupt.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Keith Packard <keithp@keithp.com>
Quoting Chris Wilson's more concise description:
"Ah I think I see the problem. As you point out we only mask the current
interrupt received, so that if we have a task pending (and so IMR != 0) we
actually unmask the pending interrupt and so could receive it again before the
tasklet is finally kicked off by the grumpy scheduler."
We need the hw to issue PM interrupts A, B, A while the scheduler is hating us
and refuses to run the rps work item. On receiving PM interrupt A we hit the
WARN because
dev_priv->pm_iir == PM_A | PM_B
Also add a posting read as suggested by Chris to ensure proper ordering of the
writes to PMIMR and PMIIR. Just in case somebody weakens write ordering.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Keith Packard <keithp@keithp.com>
This masks out all interrupts and ack's any pending ones at IRQ
uninstall time to make sure we don't receive any unexpected interrupts
later on.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We were relying on the BIOS to set these bits, which doesn't always
happen.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Various issues involved with the space character were generating
warnings in the checkpatch.pl file. This patch removes most of those
warnings.
Signed-off-by: Akshay Joshi <me@akshayjoshi.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Disable this feature when KMS is not running by setting the
driver->get_vblank_timestamp function pointer to NULL.
Signed-off-by: Keith Packard <keithp@keithp.com>
Tested-by: Justin P. Mattock <justinmattock@gmail.com>
drm_helper_hpd_irq_event queues another work proc to go and deliver
the user-space event, and that function also wants to hold the config
mutex, so we shouldn't hold the mutex across the
drm_helper_hpd_irq_event call.
Signed-off-by: Keith Packard <keithp@keithp.com>
Hotplug detection is a mode setting operation and must hold the
struct_mutex or risk colliding with other mode setting operations.
In particular, the display port hotplug function attempts to re-train
the link if the monitor is supposed to be running when plugged back
in. If that happens while mode setting is underway, the link will get
scrambled, leaving it in an inconsistent state.
This is a special case -- usually the driver mode setting entry points
are covered by the upper level DRM code, but in this case the function
is invoked as a work function not under the control of DRM.
Signed-off-by: Keith Packard <keithp@keithp.com>
Cc: stable@kernel.org
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
In an attempt to fix 38862 and 38863.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Tested-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Keith Packard <keithp@keithp.com>
This lets us make the various IRQ functions static and helps avoid
problems like the one fixed in "drm/i915: Use chipset-specific irq
installers" where one of the exported functions was called rather than
the chipset specific version.
This also fixes a UMS-mode bug -- the correct irq functions for IRL
and later chips were only getting loaded in the KMS path.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Keith Packard <keithp@keithp.com>
Provide a parameter to disable hanghcheck. This is useful mostly for
developers trying to debug known problems, and probably should not be
touched by normal users.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Keith Packard <keithp@keithp.com>
...we need to apply exactly the same workaround for missing interrupts
from BSD as for the BLT ring, apparently.
See also commit 498e720b96
(drm/i915: Fix gen6 (SNB) missed BLT ring interrupts).
Reported-and-tested-by: nkalkhof@web.de
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38529
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Keith Packard <keithp@keithp.com>
The failure appeared in dmesg as:
[drm:i915_hangcheck_ring_idle] *ERROR* Hangcheck timer elapsed... blt
ring idle [waiting on 35064155, at 35064155], missed IRQ?
This works around that problem on by making the blitter command
streamer write interrupt state to the Hardware Status Page when a
MI_USER_INTERRUPT command is decoded, which appears to force the seqno
out to memory before the interrupt happens.
v1->v2: Moved to prior interrupt handler installation and RMW flags as
per feedback.
v2->v3: Removed RMW of flags (by anholt)
Cc: stable@kernel.org
Signed-off-by: Daniel J Blueman <daniel.blueman@gmail.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
Tested-by: Chris Wilson <chris@chris-wilson.co.uk> [v1]
Tested-by: Eric Anholt <eric@anholt.net> [v1,v3]
(incidence of the bug with a testcase went from avg 2/1000 to
0/12651 in the latest test run (plus more for v1))
Tested-by: Kenneth Graunke <kenneth@whitecape.org> [v1]
Tested-by: Robert Hooker <robert.hooker@canonical.com> [v1]
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=33394
Signed-off-by: Dave Airlie <airlied@redhat.com>
The extra semicolon was not intended.
Signed-off-by: Dan Carpenter <error27@gmail.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
It's not used on Ironlake, but is used on later generations, so make
sure it exists before we try to use it in the interrupt handlers.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Add new interrupt handling functions for Ivy Bridge.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
This makes the Ironlake+ code trivial and generally simplifies things.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Set the IRQ handling functions in driver load so they'll just be used
directly, rather than branching over most of the code in the chipset
functions.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
The render P-state handling code requires reading from a GT register.
This means that FORCEWAKE must be written to, a resource which is shared
and should be protected by struct_mutex. Hence we can not manipulate
that register from within the interrupt handling and so must delegate
the task to a workqueue.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Keith Packard <keithp@keithp.com>
Provide a reference count to track the forcewake state of the GPU and
give a safe mechanism for userspace to wake the GT. This also potentially
saves a UC read if the GT is known to be awake already.
The reference count is atomic, but the register access and hardware wake
sequence is protected by struct_mutex.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
... to clarify just how we use it inside the driver and remove the
confusion of the poorly matching agp_type names. We still need to
translate through agp_type for interface into the fake AGP driver.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Keith Packard <keithp@keithp.com>
Required so that we don't obliterate the queue if initialising the
rings after the global IRQ handler is installed.
[Jesse, you recently looked at refactoring the IRQ installation
routines, does moving the initialisation of ring buffer data structures away
from that routine make sense in your grand scheme?]
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Keith Packard <keithp@keithp.com>
I stumbled over this magic bit in the gen3 INSTPM:
Bit11 Interrupt-Based AGPBUSY# Enable:
‘0’ = Pending GMCH interrupts will not cause AGPBUSY# assertion.
‘1’ = Pending GMCH interrupts will cause AGPBUSY# assertion and hence
can cause the CPU to exit C3. There is no suppression of cacheable
writes.
Note that in either case in C3 the interrupts are not lost. They will be
forwarded to the ICH when the GMCH is out of C3.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@kernel.org
Using PM latency request turns out to be very fragile and only works for
some systems, depending upon the ACPI implementation. However, I've
stumbled across a promising bit in INSTPM: "Interrupt-Based AGPBUSY#".
This reverts commit b0b544cd37.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
drivers/gpu/drm/i915/i915_irq.c: In function ‘ironlake_irq_postinstall’:
drivers/gpu/drm/i915/i915_irq.c:1618: warning: unused variable ‘pipe’
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Grab the latest stabilisation bits from -fixes and some suspend and
resume fixes from linus.
Conflicts:
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/i915_irq.c
We can enable some safely, but FDI and transcoder interrupts can occur
and block other interrupts from being detected (like port hotplug
events). So keep them disabled by default (they can be re-enabled for
debugging display bringup, but should generally be off).
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
We had some conversions over to the _PIPE macros, but didn't get
everything. So hide the per-pipe regs with an _ (still used in a few
places for legacy) and add a few _PIPE based macros, then make sure
everyone uses them.
[update: remove usage of non-existent no-op macro]
[update 2: keep modesetting suspend/resume code, update to new reg names]
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
[ickle: stylistic cleanups for checkpatch and taste]
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
