For reasons that are not apparent to anybody, 990bbdadab (drm/i915:
Group the GT routines together in both code and vtable) breaks the use
of the BitStream Decoder ring on SandyBridge. The active ingredient of
that patch is the conversion from a udelay(10) to a udelay(1) in the
busy-wait loop of waiting for the forcewake acknowledge. If we restore
that udelay(10) or insert another udelay(1) afterwards (or any wait
longer than 250ns) everything works again. An alternative is also to
remove any delay from the busy-wait loop.
Given that in the atomic sections we want to complete the wait as quick
as possible to avoid blocking the CPU for too long, it makes sense to
remove the delay altogether and simply spin on the exit condition until
it completes. So we replace the udelay(1) with cpu_relax().
Papers over regression from
commit 990bbdadab
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Mon Jul 2 11:51:02 2012 -0300
drm/i915: Group the GT routines together in both code and vtable
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51738
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This function isn't used outside of intel_panel.c, so make it static.
Signed-off-by: Stéphane Marchesin <marcheu@chromium.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The exporter should have given us pages in the correct place, avoid
the prepare object mapping phase on dmar systems.
This fixes an oops on a GM45/R600 machine, when running the intel/radeon
tests.
Signed-off-by: Dave Airlie <airlied@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
the following warning was produced,
drivers/gpu/drm/i915/i915_gem_context.c: In function ‘i915_switch_context’:
drivers/gpu/drm/i915/i915_gem_context.c:454:6: warning: unused variable ‘ret’ [-Wunused-variable]
fix up by removing it
Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
If a buffer that was the target of a PIPE_CONTROL from userland was a
reused one that hadn't been evicted which had not previously had this
workaround applied, then the early return for a correct
presumed_offset in this function meant we would not bind it into the
GTT and the write would land somewhere else.
Fixes reproducible failures with GL_EXT_timer_query usage in apitrace,
and I also expect it to fix the intermittent OQ issues on snb that
danvet's been working on.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=48019
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=52932
Signed-off-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Carl Worth <cworth@cworth.org>
Tested-by: Carl Worth <cworth@cworth.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
... by adding seemingly redudant posting reads.
This little dragon lair exploded the first time around when we've
refactored the code a bit to use the common wait_for_atomic_us in
"drm/i915: Group the GT routines together in both code and vtable",
which caused QA to file fdo bug #51738.
Chris Wilson entertained a few approaches to fixing #51738: Replacing
the udelay(1) with the previously-used udelay(10) (or any other
"sufficiently larger" delay), adding a posting read, or ditching the
delay completely and using cpu_relax. We went with the cpu_relax and
"915: Workaround hang with BSD and forcewake on SandyBridge". Which
blew up in fdo bug #52424, but adding the posting read while still
using cpu_relax seems to also fix that, it looks like the
posting read is the important ingriedient to fix these rc6 related
hangs on snb.
Popular theories as to why this is like it is include:
- A herd of pink elephants got royally angered somehow.
- The gpu has internally different functional units and judging by the
register offsets, the forcewake request register and the forcewake
ack registers are _not_ in the same functional unit (or at least
aren't reached through the same routes). Hence the posting read
syncs up with the wrong block and gets the entire gpu confused.
- ...
As a minimal ducttape fix for 3.6, let's just put these posting reads
into place again. We can try fancier approaches (like adding back the
cpu_relax instead of the udelay) in -next.
This (re-)fixes a regression introduced in
commit 990bbdadab
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Mon Jul 2 11:51:02 2012 -0300
drm/i915: Group the GT routines together in both code and vtable
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Du Yan <yanx.du@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=52424
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51738u
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drv_priv->gmbus is an array. Comparing it with NULL is somewhat less useful
than a chocolate teapot.
Possibly we should be testing bus != NULL each iteration of the loop
instead ?
gcc could help by warning too!
Signed-off-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Otherwise our initial behaviour is "randomly save a bogus PLL
choice" as far as I can see.
Signed-off-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We now refuse to load on gen6+ if kms is not enabled:
commit 26394d9251
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Mon Mar 26 21:33:18 2012 +0200
drm/i915: refuse to load on gen6+ without kms
Which results in the drm core calling our lastclose function to clean
up the mess, but that one is neatly broken for such failure cases
since kms has been introduced in
commit 79e539453b
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date: Fri Nov 7 14:24:08 2008 -0800
DRM: i915: add mode setting support
Reported-and-tested-by: Paulo Zanoni <przanoni@gmail.com>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Fixes failures in transform feedback on gen7 because our SOL_RESET
flag was setting the transform feedback offsets in the old context
(occasionally happened to be ours) instead of the new context.
Signed-off-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
LVDS is the first output where dpms on/off and prepare/commit don't
perfectly match. Now the idea behind this special case seems to be
that for simple resolution changes on the LVDS we don't need to stop
the pipe, because (at least on newer chips) we can adjust the panel
fitter on the fly.
There are a few problems with the current code though:
- We still stop and restart the pipe unconditionally, because the crtc
helper code isn't flexible enough.
- We show some ugly flickering, especially when changing crtcs (this
the crtc helper would actually take into account, but we don't
implement the encoder->get_crtc callback required to make this work
properly).
So it doesn't even work as advertised. I agree that it would be nice
to do resolution changes on LVDS (and also eDP) whithout blacking the
screen where the panel fitter allows to do that. But imo we should
implement this as a special case a few layers up in the mode set code,
akin to how we already detect simple framebuffer changes (and only
update the required registers with ->mode_set_base).
Until this is all in place, make our lives easier and just rip it out.
Also note that this seems to fix actual bugs with enabling the lvds
output, see:
http://lists.freedesktop.org/archives/intel-gfx/2012-July/018614.html
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Giacomo Comes <comes@naic.edu>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Takashi Iwai <tiwai@suse.de>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The issue is that we stale data in the CPU caches, when we come to
swap-out the object, the CPU may short-circuit the reads from those
cacheline and so corrupt the context object.
Secondary, leaving the context object as being marked in the CPU write
domain whilst on the GPU active list is a bad idea and will throw
warnings later.
Note: Thanks to calling set_to_gtt_domain with write = false and not
setting any gpu write domain when putting a context object onto the
active list (when we switch away from it) the set_to_gtt_domain call
won't block.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: Added a note to the commit message and a comment in the code
to explain the clever non-blocking trick.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
When we enable/disable the CPU backlight registers we can't forget to
enable/disable the PCH backlight registers. Since we're using the CPU
registers we should also unset the override bit.
Fixes a regression on the following commit:
drm/i915: properly enable the blc controller on the right pipe
The commit just deleted the code that sets the PCH registers, so it
was relying on the values set by the BIOS. I told my BIOS to boot on
the DVI monitor instead of the LVDS panel, so I noticed the bug.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
If we drop the breadcrumb request after a batch due to a signal for
example we aim to fix it up at the next opportunity. In this case we
emit a second batchbuffer with no waits upon the first and so no
opportunity to insert the missing request, so we need to emit the
missing flush for coherency. (Note that that invalidating the render
cache is the same as flushing it, so there should have been no
observable corruption.)
Note that beside simply adding the missing flush, avoiding potential
render corruption, this will also fix at least parts of the problem
introduced by some funny interaction of these two commits:
commit de2b998552
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed Jul 4 22:52:50 2012 +0200
drm/i915: don't return a spurious -EIO from intel_ring_begin
which allowed intel_ring_begin to return -ERESTARTSYS and
commit cc889e0f6c
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed Jun 13 20:45:19 2012 +0200
drm/i915: disable flushing_list/gpu_write_list
which essentially disabled the flushing list.
The issue happens when we submit a batch & emit it, but get
interrupted (thanks to the first patch) while trying to emit the
flush. On the next batch we still assume that the full gpu domain
handling is in effect and hence compute the invalidate&flushing
domains. But thanks to the 2nd patch we totally ignore these and only
invalidate all gpu domains, presuming that any required flushes have
been issued already. Which is wrong and eventually results in us
updating the new write_domain values with the computed
pending_write_domain values, which leaves an object with write_domain
== 0 on the gpu_write_list.
As soon as we try to unbind that object, things blow up.
Fix this by emitting the missing flush according to the new
ring->gpu_caches_dirty flag.
Note that this does _not_ fix all the current cases where we end up
with an object on the flushing_list that can't be flushed.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=52040
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
[danvet: Add bug explanation to commit message.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
While debugging Haswell link train failures I observed that we never
try the maximum voltage configuration more than once consecutively. We
start the training, the monitor keeps telling us to increase the
voltage, then when we reach the maximum we just go back to the start
(because of the "memset" above "voltage_tries = 0"). When we reach
this point, we keep alternating between the maximum and the minimum
voltages until we give up.
The DP spec suggests that we should try the same voltage 5 times
before giving up. This patch makes us try the maximum voltage at
least 5 times before going back to the minimum voltages.
This patch does not fix any particular bug I'm aware of.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Paulo Zanoni
Hunt Xu
Chris Wilson
Stéphane Marchesin
Dave Airlie
Devendra Naga
Eric Anholt
Daniel Vetter
Alan Cox
Dan Carpenter
Ben Widawsky
Ander Conselvan de Oliveira