drm/i915: Complain if we take too long under vblank evasion.
Instead of only complaining when we actually miss a vblank, always
complain if we take longer than 100 us. This will make it easier to
find cases where we potentially miss vblanks.
drm/i915/glk: Remove MODULE_FIRMWARE() tag from Geminilake's DMC
Geminilake's DMC is not yet available in the linux-firmware repository.
To prevent userspace tools such as mkinitramfs to complain about
missing firmware, remove the MODULE_FIRMWARE() tag for now.
Fixes: dbb28b5c3d3c ("drm/i915/DMC/GLK: Load DMC on GLK") Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: intel-gfx@lists.freedesktop.org Cc: <drm-intel-fixes@lists.freedesktop.org> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Acked-by: Jani Nikula <jani.nikula@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20170306085651.14008-1-ander.conselvan.de.oliveira@intel.com
Chris Wilson [Fri, 3 Mar 2017 19:08:24 +0000 (19:08 +0000)]
drm/i915: Split breadcrumbs spinlock into two
As we now take the breadcrumbs spinlock within the interrupt handler, we
wish to minimise its hold time. During the interrupt we do not care
about the state of the full rbtree, only that of the first element, so
we can guard that with a separate lock.
v2: Rename first_wait to irq_wait to make it clearer that it is guarded
by irq_lock.
Chris Wilson [Fri, 3 Mar 2017 17:14:22 +0000 (17:14 +0000)]
drm/i915: Refactor wakeup of the next breadcrumb waiter
Refactor the common task of updating the first_waiter, serialised with
the interrupt handler. When we update the first_waiter, we also need to
wakeup the new bottom-half in order to complete the actions that we may
have delegated to it (such as checking the irq-seqno coherency or waking
up other lower priority concurrent waiters).
Chris Wilson [Fri, 3 Mar 2017 14:45:57 +0000 (14:45 +0000)]
drm/i915: Take reference for signaling the request from hardirq
Being inside a spinlock signaling that the hardware just completed a
request doesn't prevent a second thread already spotting that the
request is complete, freeing it and reallocating it! The code currently
tries to prevent this using RCU -- but that only prevents the request
from being freed, it doesn't prevent us from reallocating it - that
requires us to take a reference.
Fixes: 56299fb7d904 ("drm/i915: Signal first fence from irq handler if complete")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100051 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: http://patchwork.freedesktop.org/patch/msgid/20170303144557.4815-1-chris@chris-wilson.co.uk Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Ville Syrjälä [Thu, 2 Mar 2017 17:15:07 +0000 (19:15 +0200)]
drm/i915: Add cxsr toggle tracepoint
Add a tracepoint for observing changes in the cxsr state. The tracepoint
will dump out the frame and scanline counters for each pipe so that the
information can be compared with eg. plane update tracepoints.
Add tracepoints for observing the WM/FIFO programming on VLV/CHV. When
compared with the plane and pipe update tracepoints this can be used
to verify that everything is performed in the right sequence.
Ville Syrjälä [Thu, 2 Mar 2017 17:15:05 +0000 (19:15 +0200)]
drm/i915: Add plane update/disable tracepoints
Add tracepoints for plane programming. The tracepoints will dump
the frame and scanline counters, so this can be used to verify eg. that
the plane gets reprogrammed at the right time with respect to watermark
programming (if we have appropriate tracepoints for that as well).
On VLV/CHV enabling sprite0 when sprite1 has already been enabled may
lead to an underrun. This only happens when sprite0 FIFO size is zero
prior to enabling it. Hence an effective workaround is to always
allocate at least one cacheline for sprite0 when sprite1 is active.
I've not observed this sort of failure during any other type of plane
enable/disable sequence.
v2: s/noninverted/raw/ for consistency with other platforms
Ville Syrjälä [Thu, 2 Mar 2017 17:15:02 +0000 (19:15 +0200)]
drm/i915: Sanitize VLV/CHV watermarks properly
Clear out the watermark for all disabled planes to 0. This is required
to avoid falsely thinking that the inherited watermarks are bogus in
case the watermark is actually higher than the FIFO size.
v2: s/noninverted/raw/ for consistency with other platforms
Ville Syrjälä [Thu, 2 Mar 2017 17:15:01 +0000 (19:15 +0200)]
drm/i915: Only use update_wm_{pre,post} for pre-ilk platforms
Now that vlv/chv have more proper wm programming support, let's reduce
the the update_wm_{pre,post} flags to only cover the pre-ilk platforms.
When we finally convert those as well we can drop these flags entirely.
Ville Syrjälä [Thu, 2 Mar 2017 17:15:00 +0000 (19:15 +0200)]
drm/i915: Nuke crtc->wm.cxsr_allowed
Remove crtc->wm.cxsr_allowed and just rely on crtc_state->disable_cxsr
instead. This was used only by vlv/chv to indicate whether to enable
cxsr in the wm computation. That doesn't really work anymore, and as far
as the optimal watermarks go we'll just consider the number of planes
and the current pipe, and for the intermediate watermarks we'll also
start to consider disable_cxsr which is set appropriately when planes
are being enabled/disabled.
We'll also flip over the crtc_state->wm.need_postvbl_update setup so
that it's the wm code that will set it. Previously the generic code set
it up, and then the wm code cleared it again if it thought it's not
needed after all.
Ville Syrjälä [Thu, 2 Mar 2017 17:14:59 +0000 (19:14 +0200)]
drm/i915: Compute proper intermediate wms for vlv/cvh
Since the watermark registers arent double buffered on VLV/CHV, we'll
need to play around with intermediate watermarks same was as we do on
ILK-BDW.
The watermark registers on VLV/CHV contain inverted values, so to find
the intermediate watermark value we just take the minimum of the
active and optimal values. This also means that, unlike ILK-BDW,
there's no chance that we'd fail to find a working intermediate
watermarks. As long as both the active and optimal watermarks are valid
the intermediate watermarks will come out valid as well.
Ville Syrjälä [Thu, 2 Mar 2017 17:14:58 +0000 (19:14 +0200)]
drm/i915: Skip useless watermark/FIFO related work on VLV/CHV when not needed
Check whether anything relevant has actually change when we compute new
watermarks for each plane in the state. If the watermarks for no
primary/sprite planes changed we don't have to recompute the FIFO split
or reprogram the DSBARB registers. And even the cursor watermarks didn't
change we can skip the merge+invert step between all the planes on
the pipe as well.
v2: s/noninverted/raw/ for consistency with other platforms
v3: Drop duplicated vlv_get_fifo_size() call during init
Ville Syrjälä [Thu, 2 Mar 2017 17:14:57 +0000 (19:14 +0200)]
drm/i915: Compute vlv/chv wms the atomic way
Start computing the vlv/chv watermarks the atomic way, from the
.compute_pipe_wm() hook. We'll recompute the actual watermarks
for only planes that are part of the state, the other planes will
keep their watermark from the last time it was computed.
And the actual watermark programming will happen from the
.initial_watermarks() hook. For now we'll just compute the
optimal watermarks, and we'll hook up the intermediate
watermarks properly later.
The DSPARB registers responsible for the FIFO paritioning are
double buffered, so they will be programming from
intel_begin_crtc_commit().
v2: s/noninverted/raw/ for consistency with other platforms
s/vlv_plane_wm_set/vlv_raw_plane_wm_set/ for clarity
Ville Syrjälä [Thu, 2 Mar 2017 17:14:56 +0000 (19:14 +0200)]
drm/i915: Compute VLV/CHV FIFO sizes based on the PM2 watermarks
Let's compute the watermarks first and the FIFO size second. This way we
can make sure the FIFO split is the most accommodating to the watermarks.
Previously we could have potentially computed a FIFO split that couldn't
accommodate the PM2 watermarks simply due to a bad split even if the
total FIFO size would have been sufficient.
It'll also allow us to avoid recomputing the wms for all planes whenever
the FIFO split would change. Thus we don't have to add any extra planes
to the state when the FIFO needs to be repartitioned.
To help with this we'll keep around copies of the non-inverted
watermarks in the crtc state. For now that doesn't help too much, but
once we start to do the watermark computation only for the planes
that change we'll need the non-inverted values around for the other
planes.
v2: s/noninverted/raw/ for consistency with other platforms
Fix the memset() of the "raw" watermarks
Ville Syrjälä [Thu, 2 Mar 2017 17:14:54 +0000 (19:14 +0200)]
drm/i915: Plop vlv wm state into crtc_state
Relocate the vlv/chv wm state to live under intel_crtc_state. Note
that for now this just behaves as a temporary storage. But it'll be
easier to conver the thing over to properly pre-computing the state
when it's already in the right place.
Ville Syrjälä [Thu, 2 Mar 2017 17:14:52 +0000 (19:14 +0200)]
drm/i915: Track plane fifo sizes under intel_crtc
Track the plane fifo sizes under intel_crtc instead of under each
intel_plane. Avoids looping over the planes in a bunch of places,
and later we'll move this tracking into the crtc state properly.
Ville Syrjälä [Thu, 2 Mar 2017 17:14:51 +0000 (19:14 +0200)]
drm/i915: Track visible planes in a bitmask
In a lot of place we wish to know which planes on the crtc are actually
visible, or how many of them there are. Let's start tracking that in a
bitmask in the crtc state.
We already track enabled planes (ie. ones with an fb and crtc specified by
the user) but that's not quite the same thing as enabled planes may
still end up being invisible due to clipping and whatnot.
Mika Kuoppala [Tue, 28 Feb 2017 15:28:11 +0000 (17:28 +0200)]
drm/i915/gtt: Setup vm callbacks late
If we manage to tangle errorpaths and get call to callbacks,
it is better to defensively keep them as null until object init is
finished so that we get clean null deref on callsite,
instead of more cryptic wreckage with partly initialized vm objects.
Mika Kuoppala [Tue, 28 Feb 2017 15:28:09 +0000 (17:28 +0200)]
drm/i915/gtt: Prefer i915_vm_is_48bit() over macro
If we setup the vm size early, we can use the newly introduced
i915_vm_is_48bit() in majority of callsites wanting to know the vm size.
As we operate either with 3lvl or 4lvl page table structure,
wrap the vm size query inside a function which tells us if
4lvl setup is needed for particular vm, as the following
code uses the function names where level is noted.
Chris Wilson [Fri, 3 Mar 2017 12:19:46 +0000 (12:19 +0000)]
drm/i915: Ensure the engine is idle before manually changing HWS
During reset_all_global_seqno() on seqno rollover, we have to update the
HWS. This causes all in flight requests to be completed, so first we
wait. However, we were only waiting for the requests themselves to be
completed and clearing out the waiter rbtrees - what I had missed was
the extra reference in execlists->port[]. Since commit fe9ae7a3bfdb
("drm/i915/execlists: Detect an out-of-order context switch") we can
detect when the request is retired before the context switch interrupt
is completed. The impact should be neglible outside of debugging.
drm/i915: Remove duplicate DDI enabling logic from MST path
The logic to enable a DDI in intel_mst_pre_enable_dp() is essentially
the same as in intel_ddi_pre_enable_dp(). So reuse the latter function
by calling the post_disable hook on the intel_dig_port instead of
duplicating that code.
v2: Don't oops because of a NULL encoder->crtc. (Ville)
v3: Warn for MST + PORT_E too. (Ville) Cc: Imre Deak <imre.deak@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20170302125857.14665-8-ander.conselvan.de.oliveira@intel.com
drm/i915: Pass intel_crtc to DDI functions called from crtc en/disable
Pass intel_crtc to functions intel_ddi_enable_transcoder_func(),
intel_ddi_set_pipe_settings() and intel_ddi_set_vc_payload_alloc(),
instead of the generic crtc type. By changing the functions
intel_ddi_get_crtc_encoder() so that it receives an intel_crtc
parameter, there is no need for the drm_crtc in the callers.
drm/i915: Pass pipe_config to fdi_link_train() functions
It is preferred to pass pipe_config to functions instead of accessing
crtc->config directly. Follow suit and pass pipe_config to the fdi link
train functions.
Chris Wilson [Fri, 3 Mar 2017 09:00:56 +0000 (09:00 +0000)]
drm/i915: Differentiate between hangcheck waiting for timer or scheduler
Check timer_pending() as well as work_pending() to see if the timer for
the hangcheck has already expired and the work is pending execution on
some list somewhere.
Chris Wilson [Thu, 2 Mar 2017 12:25:25 +0000 (12:25 +0000)]
drm/i915: Drop spinlocks around adding to the client request list
Adding to the tail of the client request list as the only other user is
in the throttle ioctl that iterates forwards over the list. It only
needs protection against deletion of a request as it reads it, it simply
won't see a new request added to the end of the list, or it would be too
early and rejected. We can further reduce the number of spinlocks
required when throttling by removing stale requests from the client_list
as we throttle.
Ville Syrjälä [Mon, 20 Feb 2017 14:04:43 +0000 (16:04 +0200)]
drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks
Currently ILK-BDW explicitly disable LP1+ watermarks from their
.init_clock_gating() hooks. Unfortunately that hook gets called way too
late since by that time we've already initialized all the watermark
state tracking which then gets out of sync with the hardware state.
We may eventually want to consider killing off the explicit LP1+
disable from .init_clock_gating(). In the meantime however, we can
avoid the problem by reordering the init sequence such that
intel_modeset_init_hw()->intel_init_clock_gating() gets called
prior to the hardware state takeover.
I suppose prior to the two stage watermark programming we were
magically saved by something that forced the watermarks to be
reprogrammed fully after .init_clock_gating() got called. But
now that no longer happens.
Note that the diff might look a bit odd as it kills off one
call of intel_update_cdclk(), but that's fine because
intel_modeset_init_hw() does the exact same thing. Previously
we just did it twice.
Actually even this new init sequence is pretty bogus as
.init_clock_gating() really should be called before any gem
hardware init since it can configure various clock gating
workarounds and whatnot that affect the GT side as well. Also
intel_modeset_init() really should get split up into better
defined init stages. Another "fun" detail is that
intel_modeset_gem_init() is where RPS/RC6 gets configured.
Why that is done from the display code is beyond me. I've
decided to leave all this be for now, and just try to fix
the init sequence enough for watermarks to work.
Chris Wilson [Thu, 2 Mar 2017 15:03:56 +0000 (15:03 +0000)]
drm/i915: Include GT/seqno activity in engine/hangcheck debugfs
Whilst investigating some mysterious failures with hangcheck not running
during gem_busy/basic-hang-default, the question is why did we decide to
cancel the retire_work (which queues the hangcheck)? That decision is
based around GT activity, so include that information in the debug
report.
v2: Include the GT awake status in the error state
Chris Wilson [Thu, 2 Mar 2017 14:53:23 +0000 (14:53 +0000)]
drm/i915/guc: Disable irq for __i915_guc_submit wq_lock
__i915_guc_submit may be, despite my assertion, called from outside of
an irq-safe spinlock so we need to use a full spin_lock_irqsave and not
cheat using a spin_lock. (The initial notify callback from the completed
fence is called before the spinlock is taken to wake up all waiters and
call their callbacks.)
assert_spin_locked() becomes an unconditionally compiled BUG_ON(),
adding debug code right into the heart of critical routines like
interrupt handlers.
text data bss dec hex 1296480 19944 2272 1318696 141f28 before (lockdep disabled) 1295984 19944 2272 1318200 141d38 after
Chris Wilson [Thu, 2 Mar 2017 11:51:30 +0000 (11:51 +0000)]
drm/i915: Assert that fence->lock is held in an irq-safe manner
Everytime we take the fence->lock (aka request->lock), we must do so
with irqs disabled since it may be used from within an hardirq context.
As sometimes we are taking the lock in a nested manner, assert that the
caller did disable the irqs for us.
Ville Syrjälä [Fri, 17 Feb 2017 15:01:59 +0000 (17:01 +0200)]
drm/i915: Fix legacy cursor vs. watermarks for ILK-BDW
In order to make cursor updates actually safe wrt. watermark programming
we have to clear the legacy_cursor_update flag in the atomic state. That
will cause the regular atomic update path to do the necessary vblank
wait after the plane update if needed, otherwise the vblank wait would
be skipped and we'd feed the optimal watermarks to the hardware before
the plane update has actually happened.
To make the slow vs. fast path determination in
intel_legacy_cursor_update() a little simpler we can ignore the actual
visibility of the plane (which can only get computed once we've already
chosen out path) and instead we simply check whether the fb is being
set or cleared by the user. This means a fully clipped but logically
visible cursor will be considered visible as far as watermark
programming is concerned. We can do that for the cursor since it's a
fixed size plane and the clipped size doesn't play a role in the
watermark computation.
This should fix underruns that can occur when the cursor gets
enable/disabled or the size gets changed. Hopefully it's good enough
that only pure cursor movement and flips go through unthrottled.
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Uwe Kleine-König <uwe@kleine-koenig.org> Reported-by: Uwe Kleine-König <uwe@kleine-koenig.org> Fixes: f79f26921ee1 ("drm/i915: Add a cursor hack to allow converting legacy page flip to atomic, v3.") Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20170217150159.11683-1-ville.syrjala@linux.intel.com Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Tested-by: Rafael Ristovski <rafael.ristovski@gmail.com>
Anusha Srivatsa [Wed, 1 Mar 2017 19:58:55 +0000 (11:58 -0800)]
i915/HuC: Add an extra check for platforms that do not have HUC
Return silently without producing much noise on platforms
that have a HuC but the firmware is absent.
Cc: Ander Conselvan De Oliveira <ander.conselvan.de.oliveira@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@itel.com> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1488398335-13121-1-git-send-email-anusha.srivatsa@intel.com
Chris Wilson [Thu, 2 Mar 2017 07:41:57 +0000 (07:41 +0000)]
drm/i915: Restore the invalid access without RPM warning
A long time ago we turned off the warning as it was too painful, we had
too much broken code. Turn it back on now as we are mostly clean and
need to prevent returning to such orangeness.
Chris Wilson [Thu, 2 Mar 2017 08:30:29 +0000 (08:30 +0000)]
drm/i915: Hold rpm during GEM suspend in driver unload/suspend
i915_gem_suspend() tries to access the device to ensure it is idle and
all writes from the device are flushed to memory. It assumed is already
held the runtime pm wakeref, but we should explicitly acquire it for our
access to be safe.
v2: Keep holding rpm until the end to cover i915_gem_sanitize() as well.
Fixes: 5ab57c702069 ("drm/i915: Flush logical context image out to memory upon suspend") Fixes: 1c777c5d1dc ("drm/i915/hsw: Fix GPU hang during resume from S3-devices state") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/20170302083029.19576-1-chris@chris-wilson.co.uk Reviewed-by: Imre Deak <imre.deak@intel.com> Cc: <stable@vger.kernel.org> # v4.9+
drm/i915: Enable DDI IO power domains in the DP MST path
Commit 62b695662a24 ("drm/i915: Only enable DDI IO power domains after
enabling DPLL") changed how the DDI IO power domains get enabled, but
neglected the need to enable those domains when enabling a DP connector
with MST enabled, leading to
Kernel panic - not syncing: Timeout: Not all CPUs entered broadcast exception handler
Fixes: 62b695662a24 ("drm/i915: Only enable DDI IO power domains after enabling DPLL") Cc: Imre Deak <imre.deak@intel.com> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Cc: David Weinehall <david.weinehall@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: intel-gfx@lists.freedesktop.org Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Reviewed-by: Imre Deak <imre.deak@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20170301141318.3607-2-ander.conselvan.de.oliveira@intel.com
Reintroduce a lock around tiling vs framebuffer creation to prevent
modification of the obj->tiling_and_stride whilst the framebuffer is
being created. Rather than use struct_mutex once again, use the
per-object lock - this will also be required in future to prevent
changing the tiling whilst submitting rendering.
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Fixes: 24dbf51a5517 ("drm/i915: struct_mutex is not required for allocating the framebuffer") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20170301154128.2841-2-chris@chris-wilson.co.uk
Hans de Goede [Wed, 1 Mar 2017 13:15:05 +0000 (15:15 +0200)]
drm/i915/dsi: Call MIPI_SEQ_TEAR_ON and DISPLAY_ON for cmd-mode (untested)
According to the spec we should call MIPI_SEQ_TEAR_ON and DISPLAY_ON
on enable for cmd-mode, just like we already call their counterparts
on disable. Note: untested, my panel is a vid-mode panel.
Hans de Goede [Wed, 1 Mar 2017 13:15:01 +0000 (15:15 +0200)]
drm/i915/dsi: Execute MIPI_SEQ_DEASSERT_RESET before calling device_ready()
Execute MIPI_SEQ_DEASSERT_RESET before putting the device in ready
state (LP-11), this is the sequence in which things should be done
according to the spec.
Hans de Goede [Wed, 1 Mar 2017 13:15:00 +0000 (15:15 +0200)]
drm/i915/dsi: Group DPOunit clock gate workaround with PLL enable
Move the DPOunit clock gate workaround to directly after the PLL enable.
The exact location of the workaround does not matter and there are 2
reasons to group it with the PLL enable:
1) This moves it out of the middle of the init sequence from the spec,
making it easier to follow the init sequence / compare it to the spec
2) It is grouped with the pll disable call in intel_dsi_post_disable,
so for consistency it should be grouped with the pll enable in
intel_dsi_pre_enable
Hans de Goede [Wed, 1 Mar 2017 13:14:58 +0000 (15:14 +0200)]
drm/i915/dsi: Drop bogus MIPI_SEQ_ASSERT_RESET before POWER_ON
intel_dsi_post_disable(), which does the MIPI_SEQ_ASSERT_RESET,
will always be called at some point before intel_dsi_pre_enable()
making the MIPI_SEQ_ASSERT_RESET in intel_dsi_pre_enable() redundant.
In addition, calling MIPI_SEQ_ASSERT_RESET in the enable path goes
against the VBT spec.
drm/i915/gen9: Fix PCODE polling during CDCLK change notification
there is still one report of the CDCLK-change request timing out on a
KBL machine, see the Reference link. On that machine the maximum time
the request took to succeed was 34ms, so increase the timeout to 50ms.
v2:
- Change timeout from 100 to 50 ms to maintain the current 50 ms limit
for atomic waits in the driver. (Chris, Tvrtko)
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=99345 Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: <stable@vger.kernel.org> Signed-off-by: Imre Deak <imre.deak@intel.com> Acked-by: Chris Wilson <chris@chris-wilson.co.uk> Link: http://patchwork.freedesktop.org/patch/msgid/1487946730-17162-1-git-send-email-imre.deak@intel.com
Deepak M [Wed, 1 Mar 2017 07:21:33 +0000 (12:51 +0530)]
drm/i915/glk: Add MIPIIO Enable/disable sequence
v2: Addressed Jani's Review comments(renamed bit field macros)
v3: Jani's Review comment for aligning code to platforms and added
wrapper functions.
v4: Corrected enable/disable seuqence as per BSPEC
v5: Corrected waiting twice for same bit (Review comments: Jani)
v6: Rebased to Han's patches(dsi restructuring code)
Chris Wilson [Tue, 28 Feb 2017 14:55:19 +0000 (14:55 +0000)]
drm/i915: Tighten mmio arrays for MIPI_PORT
drivers/gpu/drm/i915/intel_dsi.c: In function ‘intel_dsi_prepare’:
drivers/gpu/drm/i915/intel_dsi.c:1308:1: error: the frame size of 2488 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
which is caused by the compiling expanding every _MIPI_PORT into an
on-stack array of u32[3] at every callsite. Not sure why only one
machine/compiler appears susceptible, but with a minor tweak to _MIPI_PORT
we can defer the error until later.
This is a partial revert of commit ce64645d86ac ("drm/i915: use variadic
macros and arrays to choose port/pipe based registers") for a particular
bad offender.
Chris Wilson [Tue, 28 Feb 2017 11:28:02 +0000 (11:28 +0000)]
drm/i915/guc: Make wq_lock irq-safe
Following the use of dma_fence_signal() from within our interrupt
handler, we need to make guc->wq_lock also irq-safe. This was done
previously as part of the guc scheduler patch (which also started
mixing our fences with the interrupt handler), but is now required to
fix the current guc submission backend.
v4: Document that __i915_guc_submit is always under an irq disabled
section
v5: Move wq_rsvd adjustment to its own function
Fixes: 67b807a89230 ("drm/i915: Delay disabling the user interrupt for breadcrumbs") 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> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20170228112803.11646-2-chris@chris-wilson.co.uk Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Hans de Goede [Tue, 28 Feb 2017 09:26:21 +0000 (11:26 +0200)]
drm/i915/dsi: VLV/CHT Only wait for LP00 on MIPI PORT A
On some devices only MIPI PORT C is used, in this case checking the
MIPI PORT A CTRL AFE_LATCHOUT bit (there is no such bit for PORT C
on VLV/CHT) will result in false positive "DSI LP not going Low" errors
as this checks the PORT A clk status.
In case both ports are used we have already checked the AFE_LATCHOUT
bit when going through the for_each_dsi_port() loop for PORT A and
checking the same bit again for PORT C is a no-op.
Hans de Goede [Tue, 28 Feb 2017 09:26:20 +0000 (11:26 +0200)]
drm/i915/dsi: Make intel_dsi_enable/disable directly exec VBT sequences
The drm_panel_enable/disable and drm_panel_prepare/unprepare calls are
not fine grained enough to abstract all the different steps we need to
take (and VBT sequences we need to exec) properly. So simply remove the
panel _enable/disable and prepare/unprepare callbacks and instead
export intel_dsi_exec_vbt_sequence() from intel_dsi_panel_vbt.c
and call that from intel_dsi_enable/disable().
Hans de Goede [Tue, 28 Feb 2017 09:26:19 +0000 (11:26 +0200)]
drm/i915/dsi: Move intel_dsi_clear_device_ready()
Move the intel_dsi_clear_device_ready() function to higher up in
intel_dsi.c this pairs it with intel_dsi_device_ready(); and pairs
intel_dsi_*enable* with intel_dsi_*disable without
intel_dsi_clear_device_ready() sitting in the middle of them.
This commit purely moves code around, it does not make any
changes what-so-ever.
Hans de Goede [Tue, 28 Feb 2017 09:26:18 +0000 (11:26 +0200)]
drm/i915/dsi: Add intel_dsi_unprepare() helper
The enable path has an intel_dsi_prepare() helper which prepares various
registers for the mode-set. Move the code undoing this to a new
intel_dsi_unprepare() helper function for better symmetry between the
enable and disable paths. No functional changes.
Hans de Goede [Tue, 28 Feb 2017 09:26:16 +0000 (11:26 +0200)]
drm/i915/dsi: Move calling of wait_for_dsi_fifo_empty to mipi_exec_send_packet
Instead of calling wait_for_dsi_fifo_empty on all dsi ports after calling
a drm_panel_foo helper which calls VBT sequences, move it to the VBT
mipi_exec_send_packet helper, which is the one VBT instruction which
actually puts data in the fifo.
This results in a nice cleanup making it clearer what all the steps on
intel_dsi_enable / disable are and this also makes the VBT code properly
wait till a command has actually been send before executing the next
steps (typically a delay) in the VBT sequence.
Deepak M [Fri, 17 Feb 2017 12:43:30 +0000 (18:13 +0530)]
drm/i915/glk: Program new MIPI DSI PHY registers for GLK
Program the clk lane and tlpx time count registers
to configure DSI PHY.
v2: Addressed Jani's Review comments(renamed bit field macros)
v3: Program clk lane timing reg same as dphy param reg.
v4: Removed "line over 80 character" warning
Chris Wilson [Mon, 27 Feb 2017 20:58:50 +0000 (20:58 +0000)]
drm/i915: Delay disabling the user interrupt for breadcrumbs
A significant cost in setting up a wait is the overhead of enabling the
interrupt. As we disable the interrupt whenever the queue of waiters is
empty, if we are frequently waiting on alternating batches, we end up
re-enabling the interrupt on a frequent basis. We do want to disable the
interrupt during normal operations as under high load it may add several
thousand interrupts/s - we have been known in the past to occupy whole
cores with our interrupt handler after accidentally leaving user
interrupts enabled. As a compromise, leave the interrupt enabled until
the next IRQ, or the system is idle. This gives a small window for a
waiter to keep the interrupt active and not be delayed by having to
re-enable the interrupt.
v2: Restore hangcheck/missed-irq detection for continuations
v3: Be more careful restoring the hangcheck timer after reset
v4: Be more careful restoring the fake irq after reset (if required!)
v5: Redo changes to intel_engine_wakeup()
v6: Factor out __intel_engine_wakeup()
v7: Improve commentary for declaring a missed wakeup
Chris Wilson [Mon, 27 Feb 2017 20:58:49 +0000 (20:58 +0000)]
drm/i915: Defer enabling hangcheck to the first fake breadcrumb interrupt
By deferring hangcheck to the fake breadcrumb interrupt, we can simply
the enabling procedure slightly - as by enabling the fake, we then
enable the hangcheck. By always enabling the hangcheck from each fake
interrupt (it will be a no-op for an already queued hangcheck), it will
make restoring the breadcrumbs after a reset simpler in the next patch.
Chris Wilson [Mon, 27 Feb 2017 20:58:48 +0000 (20:58 +0000)]
drm/i915: Signal first fence from irq handler if complete
As execlists and other non-semaphore multi-engine devices coordinate
between engines using interrupts, we can shave off a few 10s of
microsecond of scheduling latency by doing the fence signaling from the
interrupt as opposed to a RT kthread. (Realistically the delay adds
about 1% to an individual cross-engine workload.) We only signal the
first fence in order to limit the amount of work we move into the
interrupt handler. We also have to remember that our breadcrumbs may be
unordered with respect to the interrupt and so we still require the
waiter process to perform some heavyweight coherency fixups, as well as
traversing the tree of waiters.
v2: No need for early exit in irq handler - it breaks the flow between
patches and prevents the tracepoint
v3: Restore rcu hold across irq signaling of request
Chris Wilson [Mon, 27 Feb 2017 20:58:47 +0000 (20:58 +0000)]
drm/i915: Report both waiters and success from intel_engine_wakeup()
The two users of the return value from intel_engine_wakeup() are
expecting different results. In the breadcrumbs hangcheck, we are using
it to determine whether wake_up_process() detected the waiter was
currently running (and if so we presume that it hasn't yet missed the
interrupt). However, in the fake_irq path, we are using the return value
as a check as to whether there are any waiters, and so we may
incorrectly stop the fake-irq if that waiter was currently running.
To handle the two different needs, return both bits of information! We
uninline it from the irq path in preparation for the next patch which
makes the irq hotpath special and relegates intel_engine_wakeup() to the
slow fixup paths.
Chris Wilson [Thu, 23 Feb 2017 14:10:20 +0000 (14:10 +0000)]
drm/i915: Distinguish between timeout and error in sideband transactions
After initiating a sideband transaction, we only want to wait for the
transaction to become idle. If, as we are, we wait for both the busy
and error flag to clear, if an error is raised we just spin until the
timeout. Once the hw is idle, we can then check to see if the hw flagged
an error, and report it distinctly.
Chris Wilson [Mon, 27 Feb 2017 13:59:13 +0000 (13:59 +0000)]
drm/i915: Reduce context alignment
No hardware was ever shipped that needed more than 4096 byte alignment
and future hardware will not use this legacy path. So reduce the
alignment to make it easier and quicker to launch workloads.
Chris Wilson [Mon, 27 Feb 2017 13:59:12 +0000 (13:59 +0000)]
drm/i915: Remove redundant TLB invalidate on switching ppgtt
We are required to reload the TLBs around ppgtt switches. However, we
already do an unconditional TLB invalidate before every batch and a flush
afterwards, so this condition is already satisfied without extra flushes
around the LRI instructions.
Chris Wilson [Mon, 27 Feb 2017 13:59:11 +0000 (13:59 +0000)]
drm/i915: Remove redundant TLB invalidate on switching contexts
We are required to reload the TLBs around context switches
(MI_SET_CONTEXT specifically) and the recommendation is do that before
the MI_SET_CONTEXT so that it is serialised with the switch and not
forgotten:
[DevSNB] If Flush TLB invalidation Mode is enabled it’s the driver’s
responsibility to invalidate the TLBs at least once after the previous
context switch after any GTT mappings changed (including new GTT entries).
This can be done by a pipeline PIPE_CONTROL with TLB inv bit set
immediately before MI_SET_CONTEXT.
However, we already do an unconditional TLB invalidate before every
batch so this condition is satifisfied.
Mika Kuoppala [Wed, 15 Feb 2017 13:52:59 +0000 (15:52 +0200)]
drm/i915: Avoid tweaking evaluation thresholds on Baytrail v3
Certain Baytrails, namely the 4 cpu core variants, have been
plaqued by spurious system hangs, mostly occurring with light loads.
Multiple bisects by various people point to a commit which changes the
reclocking strategy for Baytrail to follow its bigger brethen:
commit 8fb55197e64d ("drm/i915: Agressive downclocking on Baytrail")
There is also a review comment attached to this commit from Deepak S
on avoiding punit access on Cherryview and thus it was excluded on
common reclocking path. By taking the same approach and omitting
the punit access by not tweaking the thresholds when the hardware
has been asked to move into different frequency, considerable gains
in stability have been observed.
With J1900 box, light render/video load would end up in system hang
in usually less than 12 hours. With this patch applied, the cumulative
uptime has now been 34 days without issues. To provoke system hang,
light loads on both render and bsd engines in parallel have been used:
glxgears >/dev/null 2>/dev/null &
mpv --vo=vaapi --hwdec=vaapi --loop=inf vid.mp4
So far, author has not witnessed system hang with above load
and this patch applied. Reports from the tenacious people at
kernel bugzilla are also promising.
Considering that the punit access frequency with this patch is
considerably less, there is a possibility that this will push
the, still unknown, root cause past the triggering point on most loads.
But as we now can reliably reproduce the hang independently,
we can reduce the pain that users are having and use a
static thresholds until a root cause is found.
v3: don't break debugfs and simplification (Chris Wilson)
References: https://bugzilla.kernel.org/show_bug.cgi?id=109051 Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Len Brown <len.brown@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Jani Nikula <jani.nikula@intel.com> Cc: fritsch@xbmc.org Cc: miku@iki.fi Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> CC: Michal Feix <michal@feix.cz> Cc: Hans de Goede <hdegoede@redhat.com> Cc: Deepak S <deepak.s@linux.intel.com> Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com> Cc: <stable@vger.kernel.org> # v4.2+ Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Acked-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1487166779-26945-1-git-send-email-mika.kuoppala@intel.com
Chris Wilson [Mon, 27 Feb 2017 12:26:54 +0000 (12:26 +0000)]
drm/i915: Remove the vma from the drm_mm if binding fails
As we track whether a vma has been inserted into the drm_mm using the
vma->flags, if we fail to bind the vma into the GTT we do not update
those bits and will attempt to reinsert the vma into the drm_mm on
future passes. To prevent that, we want to unwind i915_vma_insert() if
we fail in our attempt to bind.
Fixes: 59bfa1248e22 ("drm/i915: Start passing around i915_vma from execbuffer")
Testcase: igt/drv_selftest/live_gtt Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.william.auld@gmail.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: <stable@vger.kernel.org> # v4.9+ Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20170227122654.27651-3-chris@chris-wilson.co.uk
Chris Wilson [Mon, 27 Feb 2017 12:26:53 +0000 (12:26 +0000)]
drm/i915: Unwind vma->pages allocation upon failure
If we fail to allocate the ppgtt range after allocating the pages for
the vma, we should unwind the local allocation before reporting back the
failure.
Chris Wilson [Mon, 27 Feb 2017 12:26:52 +0000 (12:26 +0000)]
drm/i915: Only unwind the local pgtable layer if empty
Only if we allocated the layer and the lower level failed should we
remove this layer when unwinding. Otherwise we ignore the overlapping
entries by overwriting the old layer with scratch.
Fixes: c5d092a4293f ("drm/i915: Remove bitmap tracking for used-pml4") Fixes: e2b763caa6eb ("drm/i915: Remove bitmap tracking for used-pdpes") Reported-by: Matthew Auld <matthew.william.auld@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99947
Testcase: igt/drv_selftest/live_gtt Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.william.auld@gmail.com> Tested-by: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20170227122654.27651-1-chris@chris-wilson.co.uk