Imre Deak [Mon, 4 Apr 2016 14:27:10 +0000 (17:27 +0300)]
drm/i915/bxt: Add HW state verification for DDI PHY and CDCLK
I caught a few errors in our current PHY/CDCLK programming by sanity
checking the actual programmed state, so I thought it would be also
useful for the future. In addition to verifying the state after
programming it also verify it after exiting DC5, to make sure DMC
restored/kept intact everything related.
v2:
- Inlining __phy_reg_verify_state() doesn't make sense and also
incorrect, so don't do it (PW/CI gcc)
v3:
- Rebase on latest -nightly
Imre Deak [Fri, 1 Apr 2016 13:02:44 +0000 (16:02 +0300)]
drm/i915/bxt: Don't reprogram an already enabled DDI PHY
If BIOS has already programmed and enabled a PHY, don't reprogram it as
that may interfere with the currently active outputs. A follow-up patch
will add state verification, so we can catch any misconfiguration on
BIOS's behalf.
Imre Deak [Fri, 1 Apr 2016 13:02:43 +0000 (16:02 +0300)]
drm/i915/bxt: Sanitize the DBUF HW state together with CDCLK
When determining whether CDCLK is enabled by BIOS and so we should skip
reprogramming it, we didn't check the related DBUF power request and
state. In theory BIOS could enable one without the other so check for
this case and reprogram things if something is amiss.
Imre Deak [Fri, 1 Apr 2016 13:02:42 +0000 (16:02 +0300)]
drm/i915/bxt: Don't toggle power well 1 on-demand
Power well 1 is managed by the DMC firmware so don't toggle it on-demand
from the driver. This means we need to follow the BSpec display
initialization sequence during driver loading and resuming (both system
and runtime) and enable power well 1 only once there. Afterwards DMC
will toggle power well 1 whenever entering/exiting DC5.
For this to work we also need to do away getting the PLL power domain,
since that just kept runtime PM disabled for good.
Imre Deak [Fri, 1 Apr 2016 13:02:41 +0000 (16:02 +0300)]
drm/i915/bxt: Power down DDI PHYs separately during the per PHY uninit
The power-down step logically belongs to the individual PHY uninit
sequence so move it there. The only functional change is that we will
power down now PHY 1 separately before PHY 0 and preserve the other bits
in the register which are defined as reserved.
Imre Deak [Mon, 4 Apr 2016 12:42:57 +0000 (15:42 +0300)]
drm/i915/skl: Unexport skl_pw1_misc_io_init
On Broxton we need to enable/disable power well 1 during the init/unit
display sequence similarly to Skylake/Kabylake. The code for this will
be added in a follow-up patch, but to prepare for that unexport
skl_pw1_misc_io_init(). It's a simple function called only from a single
place and having it inlined in the Skylake display core init/unit
functions will make it easier to compare it with its Broxton
counterpart.
This also flips the order of Misc IO and power well 1 disabling which
matches the enabling order. The specification doesn't prescribe the
disabling order, so this should be fine.
v2:
- Fix incorrect enable vs. disable power well call in
skl_display_core_uninit() (Patrik)
- Add commit comment about chaning the order of PW1 and Misc IO power
well disabling (Patrik)
Imre Deak [Fri, 1 Apr 2016 13:02:38 +0000 (16:02 +0300)]
drm/i915/bxt: Suspend power domains during suspend-to-idle
On SKL/KBL suspend-to-idle (aka freeze/s0ix) is performed with DMC
firmware assistance where the target display power state is DC6. On
Broxton on the other hand we don't use the firmware for this, but rely
instead on a manual DC9 flow. For this we have to uninitialize the
display following the BSpec display uninit sequence, just as during
S3/S4, so make sure we follow this sequence.
Imre Deak [Fri, 1 Apr 2016 13:02:37 +0000 (16:02 +0300)]
drm/i915/gen9: Fix DMC/DC state asserts
The display power well support and DC state management doesn't depend on
runtime PM support, so remove the incorrect asserts about this.
Also Broxton does support DC5, so the related assert in
assert_can_enable_dc5() is incorrect. There is a more generic and
correct assert for this already in gen9_set_dc_state(), so we can remove
all the other ones.
At the same time convert WARNs to WARN_ONCE for consistency with the
other DC state asserts.
Imre Deak [Fri, 1 Apr 2016 13:02:36 +0000 (16:02 +0300)]
drm/i915/gen9: Make power well disabling synchronous
So far we only power well enabling was synchronous not disabling. Since
we don't exactly know how the firmware (both DMC and PCU) synchronizes
against the actual power well state during DC transitions, make the
disabling also synchronous.
Imre Deak [Tue, 5 Apr 2016 10:26:05 +0000 (13:26 +0300)]
drm/i915/gen9: Reset secondary power well requests left on by DMC/KVMR
DMC forces on power well 1 and the misc IO power well by setting the
corresponding request bits both in the BIOS and the DEBUG power well
request registers. This is somewhat unexpected since the firmware should
really just save and restore state but not alter it. We also depend on
being able to disable power well 1, and the misc IO power well before
entering S3/S4 on BXT and SKL or entering DC9 on BXT. To fix this make
sure these request bits are cleared whenever we want to disable the
given power wells.
On SKL there is another twist where the firmware also clears the power
well 1 request bit in HSW_POWER_WELL_DRIVER (but not that of the misc IO
power well). This happens to not cause a problem due to the forced-on
request bits in the other request registers.
I've filed a bug about all this, but fixing that may take a while and
having this sanity check in place makes sense even for future firmware
versions.
At the same time also check the KVMR request bits. I haven't seen this
being altered, but we don't expect any request bits in here either, so
sanitize this register as well.
v2:
- Apply the workaround on SKL as well. I noticed the related failure
from the CI report, later Patrik also reported seeing it on his
machine.
Imre Deak [Fri, 1 Apr 2016 13:02:34 +0000 (16:02 +0300)]
drm/i915/bxt: Add a note about BXT_PORT_CL1CM_DW30 being read-only
This register is read-only, so we have never actually set
OCL2_LDOFUSE_PWR_DIS in it as specified by the specification. Add a code
comment about this. I filed a specification update request to clarify
this there.
Imre Deak [Fri, 1 Apr 2016 13:02:33 +0000 (16:02 +0300)]
drm/i915/bxt: Fix GRC code register field definitions
This has been corrected in BSpec quite some time ago, but we missed it
somehow. The wrong field definitions resulted in configuring PHY0 with
an incorrect GRC value.
v2:
- Remove the FIXME comment, we left in the code exactly about this
issue. (Ville)
Imre Deak [Fri, 1 Apr 2016 13:02:32 +0000 (16:02 +0300)]
drm/i915/bxt: Reject DMC firmware versions with known bugs
DMC version 1.06 has a known bug, where the firmware polls forever for a
port PLL to lock, if the PLL was disabled when entering DC5, which locks
up the machine. Version 1.07 fixes this, so make that the minimum
required version on BXT.
Ville Syrjälä [Wed, 13 Apr 2016 18:19:56 +0000 (21:19 +0300)]
drm/i915: Move gt/pm irq handling out from irq disabled section on VLV
No need to actually handle the GT/PM interrupt while we have interrupt
sources disabled. Move the actual processing to happen after we've
restored VLV_IER and master interrupt enable.
Ville Syrjälä [Wed, 13 Apr 2016 18:19:54 +0000 (21:19 +0300)]
drm/i915: Split PORT_HOTPLUG_STAT ack out from i9xx_hpd_irq_handler()
Split the VLV/CHV hoplug irq handling to ack and handler phases. This
way we can move the actual irq handling outside the section where
we have disabled the interrupt sources.
For now, we leave things as is for pre-VLV GMCH platforms, but
eventually they could get the same treatment.
Ville Syrjälä [Wed, 13 Apr 2016 18:19:52 +0000 (21:19 +0300)]
drm/i915: Eliminate loop from VLV irq handler
Now that we've dealt with the races in clearing IIR bits via VLV_IER
and the master interrupt enable, we can go ahead aliminate the loop
from the VLV interrupt handler.
Ville Syrjälä [Wed, 13 Apr 2016 18:19:51 +0000 (21:19 +0300)]
drm/i915: Clear VLV_IER around irq processing
On VLV/CHV the master interrupt enable bit only affects GT/PM
interrupts. Display interrupts are not affected by the master
irq control.
Also it seems that the CPU interrupt will only be generated when
the combined result of all GT/PM/display interrupts has a 0->1
edge. We already use the master interrupt enable bit to make sure
GT/PM interrupt can generate such an edge if we don't end up clearing
all IIR bits. We must do the same for display interrupts, and for
that we can simply clear out VLV_IER, and restore after we've acked
all the interrupts we are about to process.
So with both master interrupt enable and VLV_IER cleared out, we will
guarantee that there will be a 0->1 edge if any IIR bits are still set
at the end, and thus another CPU interrupt will be generated.
Ville Syrjälä [Wed, 13 Apr 2016 18:19:50 +0000 (21:19 +0300)]
drm/i915: Clear VLV_MASTER_IER around irq processing
Like on CHV, let's clear out the master irq enable bit when we ack
GT/PM interrupts. This will allow GT/PM interrupts to re-raise the
CPU interrupt if we fail to clear all the bits from the IIR(s).
Ville Syrjälä [Wed, 13 Apr 2016 18:19:49 +0000 (21:19 +0300)]
drm/i915: Clear VLV_IIR after PIPESTAT
On VLV/CHV VLV_IIR is not double double buffered, and it doesn't detect
edges from PIPESTAT & co. like it does on gen4. Instead it just
directly latches the level from PIPESTAT & co. That means we must clear
VLV_IIR after PIPESTAT & co. or else we'll get a spurious bit in VLV_IIR
every single time.
Ville Syrjälä [Wed, 13 Apr 2016 18:19:48 +0000 (21:19 +0300)]
drm/i915: Set up VLV_MASTER_IER consistently
We're lacking VLV_MASTER_IER setup from valleyview_irq_preinstall(), so
add it there. Also cargo cult in some POSTING_READ()s to match the other
platforms.
Ville Syrjälä [Wed, 13 Apr 2016 18:19:47 +0000 (21:19 +0300)]
drm/i915: Use GEN8_MASTER_IRQ_CONTROL consistently
Use GEN8_MASTER_IRQ_CONTROL instead of DE_MASTER_IRQ_CONTROL or
MASTER_INTERRUPT_ENABLE with the GEN8_MASTER_IRQ register. They're
all bit 31 so there's no actual bug here, but let's be consistent
which name we use for the bit.
Ville Syrjälä [Wed, 13 Apr 2016 18:09:30 +0000 (21:09 +0300)]
drm/i915: Ignore GTFIFODBG FIFO free entry fields on CHV
On CHV GTFIFODBG has some read-only bits to indicate the number
of free FIFO entries. Ignore these when checking to see if any
of the sticky error bits are set.
This gets rid of these during device resume:
[drm:cherryview_enable_rps] GT fifo had a previous error 1080000
While at it, move the assignments out of the if().
Peter Antoine [Wed, 13 Apr 2016 14:03:25 +0000 (15:03 +0100)]
drm/i915/mocs: Program MOCS for all engines on init
Allow for the MOCS to be programmed for all engines.
Currently we program the MOCS when the first render batch
goes through. This works on most platforms but fails on
platforms that do not run a render batch early,
i.e. headless servers. The patch now programs all initialised engines
on init and the RCS is programmed again within the initial batch. This
is done for predictable consistency with regards to the hardware
context.
Hardware context loading sets the values of the MOCS for RCS
and L3CC. Programming them from within the batch makes sure that
the render context is valid, no matter what the previous state of
the saved-context was.
v2: posted correct version to the mailing list.
v3: moved programming to within engine->init_hw() (Chris Wilson)
v4: code formatting and white-space changes. (Chris Wilson)
Chris Wilson [Wed, 13 Apr 2016 16:35:15 +0000 (17:35 +0100)]
drm/i915: Late request cancellations are harmful
Conceptually, each request is a record of a hardware transaction - we
build up a list of pending commands and then either commit them to
hardware, or cancel them. However, whilst building up the list of
pending commands, we may modify state outside of the request and make
references to the pending request. If we do so and then cancel that
request, external objects then point to the deleted request leading to
both graphical and memory corruption.
The easiest example is to consider object/VMA tracking. When we mark an
object as active in a request, we store a pointer to this, the most
recent request, in the object. Then we want to free that object, we wait
for the most recent request to be idle before proceeding (otherwise the
hardware will write to pages now owned by the system, or we will attempt
to read from those pages before the hardware is finished writing). If
the request was cancelled instead, that wait completes immediately. As a
result, all requests must be committed and not cancelled if the external
state is unknown.
All that remains of i915_gem_request_cancel() users are just a couple of
extremely unlikely allocation failures, so remove the API entirely.
A consequence of committing all incomplete requests is that we generate
excess breadcrumbs and fill the ring much more often with dummy work. We
have completely undone the outstanding_last_seqno optimisation.
Chris Wilson [Wed, 13 Apr 2016 16:35:14 +0000 (17:35 +0100)]
drm/i915: Reorganise legacy context switch to cope with late failure
After mi_set_context() succeeds, we need to update the state of the
engine's last_context. This ensures that we hold a pin on the context
whilst the hardware may write to it. However, since we didn't complete
the post-switch setup of the context, we need to force the subsequent
use of the same context to complete the setup (which means updating
should_skip_switch()).
Chris Wilson [Wed, 13 Apr 2016 16:35:13 +0000 (17:35 +0100)]
drm/i915: Split out !RCS legacy context switching
Having the !RCS legacy context switch threaded through the RCS switching
code makes it much harder to follow and understand. In the next patch, I
want to fix a bug handling the incomplete switch, this is made much
simpler if we segregate the two paths now.
Chris Wilson [Wed, 13 Apr 2016 16:35:12 +0000 (17:35 +0100)]
drm/i915: Move the mb() following release-mmap into release-mmap
As paranoia, we want to ensure that the CPU's PTEs have been revoked for
the object before we return from i915_gem_release_mmap(). This allows us
to rely on there being no outstanding memory accesses from userspace
and guarantees serialisation of the code against concurrent access just
by calling i915_gem_release_mmap().
v2: Reduce the mb() into a wmb() following the revoke.
Chris Wilson [Wed, 13 Apr 2016 16:35:11 +0000 (17:35 +0100)]
drm/i915: Force ringbuffers to not be at offset 0
For reasons unknown Sandybridge GT1 (at least) will eventually hang when
it encounters a ring wraparound at offset 0. The test case that
reproduces the bug reliably forces a large number of interrupted context
switches, thereby causing very frequent ring wraparounds, but there are
similar bug reports in the wild with the same symptoms, seqno writes
stop just before the wrap and the ringbuffer at address 0. It is also
timing crucial, but adding various delays hasn't helped pinpoint where
the window lies.
Whether the fault is restricted to the ringbuffer itself or the GTT
addressing is unclear, but moving the ringbuffer fixes all the hangs I
have been able to reproduce.
Chris Wilson [Wed, 13 Apr 2016 16:35:10 +0000 (17:35 +0100)]
drm/i915: Prevent machine death on Ivybridge context switching
Two concurrent writes into the same register cacheline has the chance of
killing the machine on Ivybridge and other gen7. This includes LRI
emitted from the command parser. The MI_SET_CONTEXT itself serves as
serialising barrier and prevents the pair of register writes in the first
packet from triggering the fault. However, if a second switch-context
immediately occurs then we may have two adjacent blocks of LRI to the
same registers which may then trigger the hang. To counteract this we
need to insert a delay after the second register write using SRM.
This is easiest to reproduce with something like
igt/gem_ctx_switch/interruptible that triggers back-to-back context
switches (with no operations in between them in the command stream,
which requires the execbuf operation to be interrupted after the
MI_SET_CONTEXT) but can be observed sporadically elsewhere when running
interruptible igt. No reports from the wild though, so it must be of low
enough frequency that no one has correlated the random machine freezes
with i915.ko
The issue was introduced with
commit 2c550183476dfa25641309ae9a28d30feed14379 [v3.19]
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Tue Dec 16 10:02:27 2014 +0000
drm/i915: Disable PSMI sleep messages on all rings around context switches
Chris Wilson [Wed, 13 Apr 2016 16:35:09 +0000 (17:35 +0100)]
drm/i915: Suppress error message when GPU resets are disabled
If we do not have lowlevel support for reseting the GPU, or if the user
has explicitly disabled reseting the device, the failure is expected.
Since it is an expected failure, we should be using a lower priority
message than *ERROR*, perhaps NOTICE. In the absence of DRM_NOTICE, just
emit the expected failure as a DEBUG message.
Chris Wilson [Wed, 13 Apr 2016 16:35:08 +0000 (17:35 +0100)]
drm/i915: Prevent leaking of -EIO from i915_wait_request()
Reporting -EIO from i915_wait_request() has proven very troublematic
over the years, with numerous hard-to-reproduce bugs cropping up in the
corner case of where a reset occurs and the code wasn't expecting such
an error.
If the we reset the GPU or have detected a hang and wish to reset the
GPU, the request is forcibly complete and the wait broken. Currently, we
report either -EAGAIN or -EIO in order for the caller to retreat and
restart the wait (if appropriate) after dropping and then reacquiring
the struct_mutex (essential to allow the GPU reset to proceed). However,
if we take the view that the request is complete (no further work will
be done on it by the GPU because it is dead and soon to be reset), then
we can proceed with the task at hand and then drop the struct_mutex
allowing the reset to occur. This transfers the burden of checking
whether it is safe to proceed to the caller, which in all but one
instance it is safe - completely eliminating the source of all spurious
-EIO.
Of note, we only have two API entry points where we expect that
userspace can observe an EIO. First is when submitting an execbuf, if
the GPU is terminally wedged, then the operation cannot succeed and an
-EIO is reported. Secondly, existing userspace uses the throttle ioctl
to detect an already wedged GPU before starting using HW acceleration
(or to confirm that the GPU is wedged after an error condition). So if
the GPU is wedged when the user calls throttle, also report -EIO.
v2: Split more carefully the change to i915_wait_request() and assorted
ABI from the reset handling.
v3: Add a couple of WARN_ON(EIO) to the interruptible modesetting code
so that we don't start to leak EIO there in future (and break our hang
resistant modesetting).
Chris Wilson [Wed, 13 Apr 2016 16:35:07 +0000 (17:35 +0100)]
drm/i915: Simplify reset_counter handling during atomic modesetting
Now that the reset_counter is stored on the request, we can rearrange
the code to handle reading the counter versus waiting during the atomic
modesetting for readibility (by deleting the hairiest of codes).
Chris Wilson [Wed, 13 Apr 2016 16:35:06 +0000 (17:35 +0100)]
drm/i915: Store the reset counter when constructing a request
As the request is only valid during the same global reset epoch, we can
record the current reset_counter when constructing the request and reuse
it when waiting upon that request in future. This removes a very hairy
atomic check serialised by the struct_mutex at the time of waiting and
allows us to transfer those waits to a central dispatcher for all
waiters and all requests.
PS: With per-engine resets, we obviously cannot assume a global reset
epoch for the requests - a per-engine epoch makes the most sense. The
challenge then is how to handle checking in the waiter for when to break
the wait, as the fine-grained reset may also want to requeue the
request (i.e. the assumption that just because the epoch changes the
request is completed may be broken - or we just avoid breaking that
assumption with the fine-grained resets).
Chris Wilson [Wed, 13 Apr 2016 16:35:05 +0000 (17:35 +0100)]
drm/i915: Tighten reset_counter for reset status
In the reset_counter, we use two bits to track a GPU hang and reset. The
low bit is a "reset-in-progress" flag that we set to signal when we need
to break waiters in order for the recovery task to grab the mutex. As
soon as the recovery task has the mutex, we can clear that flag (which
we do by incrementing the reset_counter thereby incrementing the gobal
reset epoch). By clearing that flag when the recovery task holds the
struct_mutex, we can forgo a second flag that simply tells GEM to ignore
the "reset-in-progress" flag.
The second flag we store in the reset_counter is whether the
reset failed and we consider the GPU terminally wedged. Whilst this flag
is set, all access to the GPU (at least through GEM rather than direct mmio
access) is verboten.
PS: Fun is in store, as in the future we want to move from a global
reset epoch to a per-engine reset engine with request recovery.
Chris Wilson [Wed, 13 Apr 2016 16:35:04 +0000 (17:35 +0100)]
drm/i915: Simplify checking of GPU reset_counter in display pageflips
If we, when we store the reset_counter for the operation, we ensure that
it is not in a wedged or in the middle of a reset, we can then assert that
if any reset occurs the reset_counter must change. Later we can just
compare the operation's reset epoch against the current counter to see
if we need to abort the operation (to handle the hang).
Chris Wilson [Wed, 13 Apr 2016 16:35:03 +0000 (17:35 +0100)]
drm/i915: Hide the atomic_read(reset_counter) behind a helper
This is principally a little bit of syntatic sugar to hide the
atomic_read()s throughout the code to retrieve the current reset_counter.
It also provides the other utility functions to check the reset state on the
already read reset_counter, so that (in later patches) we can read it once
and do multiple tests rather than risk the value changing between tests.
v2: Be more strict on converting existing i915_reset_in_progress() over to
the more verbose i915_reset_in_progress_or_wedged().
Chris Wilson [Wed, 13 Apr 2016 16:35:02 +0000 (17:35 +0100)]
drm/i915: Add GEM debugging Kconfig option
Currently there is a #define to enable extra BUG_ON for debugging
requests and associated activities. I want to expand its use to cover
all of GEM internals (so that we can saturate the code with asserts).
We can add a Kconfig option to make it easier to enable - with the usual
caveats of not enabling unless explicitly requested.
Chris Wilson [Wed, 13 Apr 2016 16:35:01 +0000 (17:35 +0100)]
drm/i915: Disentangle i915_drv.h includes
Separate out the layers of includes (linux, drm, intel, i915) so that it
is a little easier to order our definitions between our multiple
reentrant headers. A couple of headers needed fixes to make them more
standalone (forgotten includes, forward declarations etc).
Chris Wilson [Wed, 13 Apr 2016 16:35:00 +0000 (17:35 +0100)]
drm/i915: Force clean compilation with -Werror
Our driver compiles clean (nowadays thanks to 0day) but for me, at least,
it would be beneficial if the compiler threw an error rather than a
warning when it found a piece of suspect code. (I use this to
compile-check patch series and want to break on the first compiler error
in order to fix the patch.)
v2: Kick off a new "Debugging" submenu for i915.ko
At this point, we applied it to the kernel and promptly kicked it out
again as it broke buildbots (due to a compiler warning on 32bits):
Revert "drm/i915: Force clean compilation with -Werror"
v3: Avoid enabling -Werror for allyesconfig/allmodconfig builds, using
COMPILE_TEST as a suitable proxy suggested by Andrew Morton. (Damien)
Only make the option available for EXPERT to reinforce that the option
should not be casually enabled.
Mika Kuoppala [Wed, 13 Apr 2016 14:26:44 +0000 (17:26 +0300)]
drm/i915: Calculate edram size
With gen9+ the edram capabilities are defined so
that we can calculate the edram (ellc) size accordingly.
Note that there are undefined combinations for some subset of
edram capability bits. Return the closest size for undefined indexes.
Even if we get it wrong with beginning of future gen enabling, the size
information is currently only used for boot message and in debugfs entry.
v2: Use function instead of hard to read macro (Daniel)
v3: s/INTEL_INFO/INTEL_GEN (Matthew)
Mika Kuoppala [Wed, 13 Apr 2016 14:26:43 +0000 (17:26 +0300)]
drm/i915: Store and use edram capabilities
Store the edram capabilities instead of only the size of
edram. This is preparatory patch to allow edram size calculation
based on edram capability bits for gen9+. With gen9 the
edram is behind llc and is a separate entity. With hsw/bdw
it was more of a victim cache for LLC so the name 'eLLC' might
be warranted. Regardless, rename all mentions of eLLC to EDRAM to
clear the confusion.
v2: return bytes for edram size (Chris)
s/eLLC/eDRAM in output if we are gen > 8
v3: rebase, INTEL_GEN (Chris)
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Michał Winiarski [Tue, 12 Apr 2016 13:51:55 +0000 (15:51 +0200)]
drm/i915: Adjust size of PIPE_CONTROL used for gen8 render seqno write
We started to use PIPE_CONTROL to write render ring seqno in order to
combat seqno write vs interrupt generation problems. This was introduced
by commit 7c17d377374d ("drm/i915: Use ordered seqno write interrupt
generation on gen8+ execlists").
On gen8+ size of PIPE_CONTROL with Post Sync Operation should be
6 dwords. When we're using older 5-dword variant it's possible to
observe inconsistent values written by PIPE_CONTROL with Post
Sync Operation from user batches, resulting in rendering corruptions.
v2: Fix BAT failures
v3: Comments on alignment and thrashing high dword of seqno (Chris)
v4: Updated commit msg (Mika)
Testcase: igt/gem_pipe_control_store_loop/*-qword-write
Issue: VIZ-7393 Cc: stable@vger.kernel.org Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Tested-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1460469115-26002-1-git-send-email-michal.winiarski@intel.com
Mika Kuoppala [Tue, 5 Apr 2016 12:56:17 +0000 (15:56 +0300)]
drm/i915/skl: Fix spurious gpu hang with gt3/gt4 revs
Experiments with heaven 4.0 benchmark and skylake gt3e (rev 0xa)
suggest that WaForceContextSaveRestoreNonCoherent is needed for all
revs. Extending this to all revs cures a gpu hang with rev 0xa when
running heaven4.0 gpu benchmark.
We have been here before, with problems enabling gt4e and extending
up to revision F0 instead of false claims of bspec of E0 only. See
commit <e238659ddd88> ("drm/i915/skl: Default to noncoherent access
up to F0"). In retrospect we should have covered this with this big
blanket back then already, as E0 vs F0 discrepancy was suspicious
enough.
Previously the WaForceEnableNonCoherent has been tied to
context non-coherence, atleast in relevant hsds. So keep this tie
and extended this alongside.
Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Cc: Ben Widawsky <benjamin.widawsky@intel.com> Cc: Timo Aaltonen <tjaalton@ubuntu.com> Cc: stable@vger.kernel.org Reported-by: Mike Lothian <mike@fireburn.co.uk>
References: https://bugs.freedesktop.org/show_bug.cgi?id=93491 Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com> Tested-by: Timo Aaltonen <tjaalton@ubuntu.com> Link: http://patchwork.freedesktop.org/patch/msgid/1459860977-27751-2-git-send-email-mika.kuoppala@intel.com
Mika Kuoppala [Tue, 5 Apr 2016 12:56:16 +0000 (15:56 +0300)]
drm/i915/skl: Fix rc6 based gpu/system hang
For all gt3 and gt4 skylake variants, extend the usage of
WaRsDisableCoarsePowerGating for all revisions. Without this
gt3 and gt4 skylakes up to atleast rev 0xa can gpu hang or
system hang.
Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Cc: Ben Widawsky <benjamin.widawsky@intel.com> Cc: Timo Aaltonen <tjaalton@ubuntu.com> Cc: <stable@vger.kernel.org> Reported-by: Mikael Djurfeldt <mikael@djurfeldt.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=94161 Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com> Tested-by: Timo Aaltonen <tjaalton@ubuntu.com> Link: http://patchwork.freedesktop.org/patch/msgid/1459860977-27751-1-git-send-email-mika.kuoppala@intel.com
We can use the new pin/lazy unpin API for simplicity
and more performance in the execlist submission paths.
v2:
* Fix error handling and convert more users.
* Compact some names for readability.
v3:
* intel_lr_context_free was not unpinning.
* Special case for GPU reset which otherwise unbalances
the HWS object pages pin count by running the engine
initialization only (not destructors).
drm/i915: Split execlists hardware status page initialisation from setup
Split the hardware status page into setup and initialisation,
where setup means setting up the driver state to support the
engine, and initialization means programming the hardware
with the before set up state.
This way the design matches the design of the engine setup/init
code which is split in the same fashion and it enables the
stages to be used in a balanced fashion (engine setup - hws
setup, engine init - hws init).
This will enable the upcoming improvements to slot in without
any kludges on the GPU reset path.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Ville Syrjälä [Tue, 15 Mar 2016 14:40:03 +0000 (16:40 +0200)]
drm/i915: Power down the DSI PLL before reconfiguring it
On VLV at least, the BIOS may leave the DSI PLL enabled in some wonky
state where it just refuses to lock. Simply disabling the PLL before
reconfiguring it is not enough to fix it, but power gating the PLL
prior to reconfiguring does work.
This happens on BYT FFRD8 when booting with HDMI connected so the DSI
display will not be lit up by the BIOS.
Also we can remove the code for BXT that disables the PLL before
enabling it again.
v2: s/vlv/intel/ since BXT made thing generic
v3: Remove the BXT disable PLL before enable trick
Ville Syrjälä [Mon, 11 Apr 2016 13:56:31 +0000 (16:56 +0300)]
drm/i915: Move DPINVGTT setup to vlv_display_irq_reset()
DPINVGTT lives inside the disp2d power well so we can't frob it unless
we know the power well is active. Let's this stuff into
vlv_display_irq_reset() which is only called at the right times so that
we don't get unclaimed register access errors.
Ville Syrjälä [Mon, 11 Apr 2016 13:56:30 +0000 (16:56 +0300)]
drm/i915: Move vlv_init_display_clock_gating() to the display power well
The registers frobbed by vlv_init_display_clock_gating() libve inside
the disp2d power well, so frobbing them while the power well is down
results in unclaimed register access warning (and of course the values
won't stick). Let's do this setup after we know the power well is
enabled.
It's also worth noting that DSPCLK_GATE_D and CBR1_VLV lose their state
when the power well goes down, but fortunately the values we've been
writing are actually the reset defaults.
MI_ARB_VLV actually retains its value even if the power well was turned
off, we just can't access it while the power well is down.
Ville Syrjälä [Mon, 11 Apr 2016 13:56:28 +0000 (16:56 +0300)]
drm/i915: Use GEN5_IRQ_INIT() in vlv_display_irq_postinstall()
Replace the hand rolled IMR/IER setup in vlv_display_irq_postinstall()
with GEN5_IRQ_INIT(). Also rename the iir_mask to enable_mask to avoid
consusion since we no longer deal with IIR here.
Ville Syrjälä [Mon, 11 Apr 2016 13:56:27 +0000 (16:56 +0300)]
drm/i915: Clear display interrupt before enabling when turning on the power well
For a bit of extra paranoia make sure the display irqs are all cleared
before we enabled them when turning on the power well. This should
really be the case already since the power well was off which resets
everything.
Ville Syrjälä [Mon, 11 Apr 2016 13:56:25 +0000 (16:56 +0300)]
drm/i915: Skip display irq setup if display irqs aren't flagged as enabled
During runtime PM we'll be reinitializing interrupt support from the
ground up. However since the display power well will be off at that
time, well end up with a ton of unclaimed register accesses from the
display irq setup. Since we turned off the power well already before
runtime suspend, we've flagged display irqs as disabled during runtime
PM transitions. So we can just check that flag to see if we should do
skip display irqs during irq setup.
During driver load display irqs will be flagged as enabled since we've
turned on the power well already, however the power well code will have
skipped the display irq setup since irq support as a whole wasn't yet
enabled when the power well was enabled. So we'll want to do the display
irq setup in that case.
Ville Syrjälä [Tue, 12 Apr 2016 15:56:14 +0000 (18:56 +0300)]
drm/i915: Fix up vlv/chv display irq setup
The vlv/chv display irq setup was a bit of mess after I ran out of steam
when working on it last. Fix it up so that we just have a _reset() and
_postinstall() hooks for the display irqs, and use those consistently.
v2: Clear out pipestat_irq_mask[] and PIPE_FIFO_UNDERRUN_STATUS in
vlv_display_irq_reset() (Imre)
Ville Syrjälä [Mon, 11 Apr 2016 13:56:23 +0000 (16:56 +0300)]
drm/i915: Remove "VLV magic" from irq setup
No clue what this is supposed to achieve. I think it's been there since
the very beginning, so presumably some kind of kludge for very early
silicon. Let's just throw it out.
Ville Syrjälä [Fri, 1 Apr 2016 18:53:19 +0000 (21:53 +0300)]
drm/i915: Replace ILK eDP underrun suppression with something better
The underruns we were seeing when enabling eDP port A on ILK seem to
have been caused by prematurely clearing the LP1+ watermark values when
disabling LP1+ watermarks. Now that the watermarks are handled
properly, we can rip out the underrun suppression around the port A
enable.
We still need to worry about the underruns on FDI when enabling
the eDP PLL. But as Bspec tells us, we can avoid that by a vblank
wait on the pipe driving FDI just prior to enabling the eDP PLL.
Ville Syrjälä [Fri, 1 Apr 2016 18:53:18 +0000 (21:53 +0300)]
drm/i915: Make sure LP1+ watermarks levels are preserved when going from 1 to 2 pipes
Once again ILK is unhappy if we clear out the LP1+ watermark levels
outright, and instead we must disable the levels we don't want while
still leaving the actual programmed watermark levels intact.
Fixes underruns on the already enabled pipe when programming watermarks
while enabling the second pipe.
Ville Syrjälä [Fri, 1 Apr 2016 18:53:17 +0000 (21:53 +0300)]
drm/i915: Try to shut up more ILK underruns
Take a bigger hammer to the underrun suppression on ILK. Instead of
trying to suppress them at specific points in the modeset sequence just
silence them across the entire sequence. This gets rid of some underruns
at least on my ILK. Note that this changes SNB and IVB to follow the
same approach just to keep the code less convoluted. The difference is
that on those platforms we won't suppress CPU underruns for port A since
it doesn't seem to be necessary.
My ILK has port A eDP and two PCH HDMI ports, so I can't be sure this is
as effective on other PCH port types. Perhaps we still need some of
Daniel's extra vblank waits [2]?
I've still been able to trigger an underrun on the other pipe, but
fixing that perhaps needs the LP1+ disable trick I implemented here [1]
which never got merged.
A few details which hamper stress testing on my ILK are that sometimes
the PCH transcoder gets messed up and refuses to shut down, and sometimes
even the panel power sequencer apparently gets stuck on the always on
position.
drm/i915: Only grab correct forcewake for the engine with execlists
Rather than blindly waking up all forcewake domains on command
submission, we can teach each engine what is (or are) the correct
one to take.
On platforms with multiple forcewake domains like VLV, CHV, SKL
and BXT, this has the potential of lowering the GPU and CPU
power use and submission latency.
To implement it we add a function named
intel_uncore_forcewake_for_reg whose purpose is to query which
forcewake domains need to be taken to read or write a specific
register with raw mmio accessors.
These enables the execlists engine setup to query which
forcewake domains are relevant per engine on the currently
running platform.
v2:
* Kerneldoc.
* Split from intel_uncore.c macro extraction, WARN_ON,
no warns on old platforms. (Chris Wilson)
v3:
* Single domain per engine, mention all registers,
bi-directional function and a new name, fix handling
of gen6 and gen7 writes. (Chris Wilson)
drm/i915: Extract knowledge of register forcewake domains
Knowledge of which register per platform belonds in which
forcewake domain was embedded in the MMIO accessors themselves.
Extract it into standalone macros so they can be used from
new code in the following patches.
This causes GCC to compile some of the MMIO accessors slightly
differently and grows the code a tiny amount. But none of the
growth is on the fast-path so it does not matter hugely.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Acked-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
drm/i915: Do not serialize forcewake acquire across domains
On platforms with multiple forcewake domains it seems more efficient
to request all desired ones and then to wait for acks to avoid
needlessly serializing on each domain.
As the vast majority of users do not use the domain id variable,
we can eliminate it from the iterator and also change the latter
using the same principle as was recently done for for_each_engine.
For a couple of callers which do need the domain mask, store it
in the domain array (which already has the domain id), then both
can be retrieved thence.
Result is clearer code and smaller generated binary, especially
in the tight fw get/put loops. Also, relationship between domain
id and mask is no longer assumed in the macro.
v2: Improve grammar in the commit message and rename the
iterator to for_each_fw_domain_masked for consistency.
(Dave Gordon)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
drm/i915: Use consistent forcewake auto-release timeout across kernel configs
Because it is based on jiffies, current implementation releases the
forcewake at any time between straight away and between 1ms and 10ms,
depending on the kernel configuration (CONFIG_HZ).
This is probably not what has been desired, since the dynamics of keeping
parts of the GPU awake should not be correlated with this kernel
configuration parameter.
Change the auto-release mechanism to use hrtimers and set the timeout to
1ms with a 1ms of slack. This should make the GPU power consistent
across kernel configs, and timer slack should enable some timer coalescing
where multiple force-wake domains exist, or with unrelated timers.
For GlBench/T-Rex this decreases the number of forcewake releases from
~480 to ~300 per second, and for a heavy combined OGL/OCL test from
~670 to ~360 (HZ=1000 kernel).
Even though this reduction can be attributed to the average release period
extending from 0-1ms to 1-2ms, as discussed above, it will make the
forcewake timeout consistent for different CONFIG_HZ values.
Real life measurements with the above workload has shown that, with this
patch, both manage to auto-release the forcewake between 2-4 times per
10ms, even though the number of forcewake gets is dramatically different.
T-Rex requests between 5-10 explicit gets and 5-10 implict gets in each
10ms period, while the OGL/OCL test requests 250 and 380 times in the same
period.
The two data points together suggest that the nature of the forwake
accesses is bursty and that further changes and potential timeout
extensions, or moving the start of timeout from the first to the last
automatic forcewake grab, should be carefully measured for power and
performance effects.
v2:
* Commit spelling. (Dave Gordon)
* More discussion on numbers in the commit. (Chris Wilson)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Dave Gordon <david.s.gordon@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Ville Syrjälä [Mon, 11 Apr 2016 07:23:51 +0000 (10:23 +0300)]
drm/i915: Get panel_type from OpRegion panel details
We've had problems on several occasions with using the panel type
from the VBT block 40. Usually it seems to be 2, which often
doesn't give us the correct timings for the panel. After some
more digging I found a way to get a panel type via the OpRegion
SWSCI GBDA "Get Panel Details" method. Let's try to use it.
The spec has this to say about the output:
"Bits [15:8] - Panel Type
Bits contain the panel type user setting from CMOS
00h = Not Valid, use default Panel Type & Timings from VBT
01h - 0Fh = Panel Number"
Another version of the spec lists the valid range as 1-16, which makes
more sense since VBT supports 16 panels. Based on actual results
from Rob's G45, 1-16 is what we need to accept.
The other bits in the output don't look relevant for the problem at
hand.
The input is specified as:
"Bits [31:4] - Reserved
Reserved (must be zero)
Bits [3:0] - Panel Number
These bits contain the sequential index of Panel, starting at 0 and
counting upwards from the first integrated Internal Flat-Panel Display
Encoder present, and then from the first external Display Encoder
(e.g., S/DVO-B then S/DVO-C) which supports Internal Flat-Panels.
0h - 0Fh = Panel number"
For now I've just hardcoded the input panel number as 0. That would seem
like a decent choise for LVDS. Not so sure about eDP when port != A.
v2: Accept values 1-16
Filter out bogus results in opregion code (Jani)
Add debug logging for all the different branches (Jani)
Ville Syrjälä [Mon, 11 Apr 2016 07:22:09 +0000 (10:22 +0300)]
drm/i915: Reject panel_type > 0xf from VBT
VBT can only contain 16 panel entries, indexed with the panel_type.
To play it safe we should reject panel_type > 0xf, so that we don't
read past the valid data.
Ville Syrjälä [Mon, 7 Mar 2016 15:56:59 +0000 (17:56 +0200)]
drm/i915: Restore GMBUS operation after a failed bit-banging fallback
When the GMBUS based i2c transfer times out, we try to fall back to
bit-banging and retry the operation that way. However if the bit-banging
attempt also fails, we should probably go back to the GMBUS method for
the next attempt. Maybe there simply wasn't anyone one the bus at this
time.
There's also a bit of a mess going on with the force_bit handling.
It's supposed to be a ref count actually, and it is as far as
intel_gmbus_force_bit() is concerned. But it's treated as just a
flag by the timeout based bit-banging fallback. I suppose that's
fine since we should never end up in the timeout fallback case
if force_bit was already non-zero. However now that we want to restore
things back to where they were after the bit-banging attempt failed,
we're going to have to do things a bit differently to avoid clobbering
the force_bit count as set up by intel_gmbus_force_bit(). So let's
dedicate the high bit as a flag for the low level timeout based fallback
and treat the rest of the bits as a ref count just as before.
Ville Syrjälä [Mon, 7 Mar 2016 15:56:58 +0000 (17:56 +0200)]
drm/i915: Protect force_bit with gmbus_mutex
Extend the protection of gmbus_mutex around the force_bit
RMW in intel_gmbus_force_bit(), in case someone gets the
idea of calling it from a separate thread while there's
other stuff happening on the same bus.
Chris Wilson [Tue, 5 Apr 2016 14:00:00 +0000 (15:00 +0100)]
drm/i915/userptr: Hold mmref whilst calling get-user-pages
Holding a reference to the containing task_struct is not sufficient to
prevent the mm_struct from being reaped under memory pressure. If this
happens whilst we are calling get_user_pages(), explosions erupt -
sometimes an immediate GPF, sometimes page flag corruption. To prevent
the target mm from being reaped as we are reading from it, acquire a
reference before we begin.
Chris Wilson [Tue, 5 Apr 2016 13:59:59 +0000 (14:59 +0100)]
drm/i915/userptr: Flush cancellations before mmu-notifier invalidate returns
In order to ensure that all invalidations are completed before the
operation returns to userspace (i.e. before the munmap() syscall returns)
we need to wait upon the outstanding operations.
We are allowed to block inside the invalidate_range_start callback, and
as struct_mutex is the inner lock with mmap_sem we can wait upon the
struct_mutex without provoking lockdep into warning about a deadlock.
However, we don't actually want to wait upon outstanding rendering
whilst holding the struct_mutex if we can help it otherwise we also
block other processes from submitting work to the GPU. So first we do a
wait without the lock and then when we reacquire the lock, we double
check that everything is ready for removing the invalidated pages.
Finally to wait upon the outstanding unpinning tasks, we create a
private workqueue as a means to conveniently wait upon all at once. The
drawback is that this workqueue is per-mm, so any threads concurrently
invalidating objects will wait upon each other. The advantage of using
the workqueue is that we can wait in parallel for completion of
rendering and unpinning of several objects (of particular importance if
the process terminates with a whole mm full of objects).
v2: Apply a cup of tea to the changelog.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94699
Testcase: igt/gem_userptr_blits/sync-unmap-cycles Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Michał Winiarski <michal.winiarski@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1459864801-28606-1-git-send-email-chris@chris-wilson.co.uk Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Daniel Vetter [Mon, 11 Apr 2016 17:21:06 +0000 (19:21 +0200)]
Merge tag 'v4.6-rc3' into drm-intel-next-queued
Linux 4.6-rc3
Backmerge requested by Chris Wilson to make his patches apply cleanly.
Tiny conflict in vmalloc.c with the (properly acked and all) patch in
drm-intel-next:
Chris Wilson [Fri, 8 Apr 2016 11:11:14 +0000 (12:11 +0100)]
drm/i915: Avoid allocating a vmap arena for a single page
If we want a contiguous mapping of a single page sized object, we can
forgo using vmap() and just use a regular kmap(). Note that this is only
suitable if the desired pgprot_t is compatible.
Chris Wilson [Fri, 8 Apr 2016 11:11:13 +0000 (12:11 +0100)]
drm,i915: Introduce drm_malloc_gfp()
I have instances where I want to use drm_malloc_ab() but with a custom
gfp mask. And with those, where I want a temporary allocation, I want to
try a high-order kmalloc() before using a vmalloc().
We now have two implementations for vmapping a whole object, one for
dma-buf and one for the ringbuffer. If we couple the mapping into the
obj->pages lifetime, then we can reuse an obj->mapping for both and at
the same time couple it into the shrinker. There is a third vmapping
routine in the cmdparser that maps only a range within the object, for
the time being that is left alone, but will eventually use these routines
in order to cache the mapping between invocations.
v2: Mark the failable kmalloc() as __GFP_NOWARN (vsyrjala)
v3: Call unpin_vmap from the right dmabuf unmapper
v4: Rename vmap to map as we don't wish to imply the type of mapping
involved, just that it contiguously maps the object into kernel space.
Add kerneldoc and lockdep annotations
Chris Wilson [Fri, 8 Apr 2016 11:11:10 +0000 (12:11 +0100)]
drm/i915: Consolidate common error handling in intel_pin_and_map_ringbuffer_obj
After we pin the ringbuffer into the GGTT, all error paths need to unpin
it again. Move this common step into one block, and make the unable to
iomap error code consistent (i.e. treat it as out of memory to avoid
confusing it with a invalid argument).
Dongwon Kim [Thu, 17 Mar 2016 01:06:13 +0000 (18:06 -0700)]
drm/i915/bxt: Reversed polarity of PORT_PLL_REF_SEL bit
For BXT, description of polarities of PORT_PLL_REF_SEL
has been reversed for newer Gen9LP steppings according to the
recent update in Bspec. This bit now should be set for
"Non-SSC" mode for all Gen9LP starting from B0 stepping.
v2: Only B0 and newer stepping should be affected by this
change.
projects / linux-beck.git / log