Chris Wilson [Tue, 29 Nov 2016 12:10:24 +0000 (12:10 +0000)]
drm/i915/guc: Split hw submission for replay after GPU reset
Something I missed before sending off the partial series was that the
non-scheduler guc reset path was broken (in the full series, this is
pushed to the execlists reset handler). The issue is that after a reset,
we have to refill the GuC workqueues, which we do by resubmitting the
requests. However, if we already have submitted them, the fences within
them have already been used and triggering them again is an error.
Instead, just repopulate the guc workqueue.
Chris Wilson [Tue, 29 Nov 2016 12:10:23 +0000 (12:10 +0000)]
drm/i915/guc: Keep the execbuf client allocated across reset
In order to avoid some complexity in trying to reconstruct the
workqueues across reset, remember them instead. The issue comes when we
have to handle a reset between request allocation and submission, the
request has reserved space in the wq, but is not in any list so we fail
to restore the reserved space. By keeping the execbuf client intact
across the reset, we also keep the reservations.
Chris Wilson [Tue, 29 Nov 2016 12:10:21 +0000 (12:10 +0000)]
drm/i915/guc: Rename client->cookie to match use
The client->cookie is a shadow of the doorbell->cookie value, so rename
it to indicate its association with the doorbell, like the doorbell id
and offset.
Chris Wilson [Tue, 29 Nov 2016 12:10:20 +0000 (12:10 +0000)]
drm/i915: Trim i915_guc_info() stack usage
i915_guc_info() (part of debugfs output) tries to avoid holding
struct_mutex for a long period by copying onto the stack. This causes a
warning that the stack frame is massive, so stop doing that. We can even
forgo holding the struct_mutex here as that doesn't serialise the values
being read (and the lists used exist for the device lifetime).
v2: Skip printing anything if guc->execbuf_client is disabled (avoids
potential NULL dereference).
Libin Yang [Mon, 28 Nov 2016 12:07:07 +0000 (20:07 +0800)]
drm/i915: enable dp mst audio
This patch adds support for DP MST audio in i915.
Enable audio codec when DP MST is enabled if has_audio flag is set.
Disable audio codec when DP MST is disabled if has_audio flag is set.
Another separated patches to support DP MST audio will be implemented
in audio driver.
This patch is ported from
commit 3708d5e082c3 ("drm/i915: start adding dp mst audio")
And because commit 3708d5e082c3 ("drm/i915: start adding dp mst audio")
breaks MST multi-monitor setups on some platforms, the orignal patch is
reverted by
commit be754b101f70 ("Revert "drm/i915: start adding dp mst audio"")
As the multi-monitor setups issue is fixed, let's port the patch and
enable the dp mst audio.
Signed-off-by: Libin Yang <libin.yang@intel.com> Cc: Lyude <cpaul@redhat.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Reviewed-by: Jani Nikula <jani.nikula@intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1480334827-112273-3-git-send-email-libin.yang@intel.com
Chris Wilson [Tue, 29 Nov 2016 12:42:05 +0000 (12:42 +0000)]
drm/i915: Fix tracepoint compilation
drivers/gpu/drm/i915/./i915_trace.h: In function ‘trace_event_raw_event_i915_gem_evict’:
drivers/gpu/drm/i915/./i915_trace.h:409:24: error: ‘struct i915_address_space’ has no member named ‘dev’
__entry->dev = vm->dev->primary->index;
A couple of macros missed in the s/vm->dev/vm->i915/ conversion.
Fixes: 49d73912cbfc ("drm/i915: Convert vm->dev backpointer to vm->i915") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20161129124205.19351-1-chris@chris-wilson.co.uk Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
v2: do_div() is only u64/u32, we need a u32/u64!
v3: div_u64() == u64/u32, div64_u64() == u64/u64
Reported-by: kbuild test robot <fengguang.wu@intel.com> Fixes: d79651522e89 ("drm/i915: Enable i915 perf stream for Haswell OA unit") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Robert Bragg <robert@sixbynine.org> Link: http://patchwork.freedesktop.org/patch/msgid/20161123150714.24449-1-chris@chris-wilson.co.uk Reviewed-by: Robert Bragg <robert@sixbynine.org>
Chris Wilson [Tue, 29 Nov 2016 09:50:08 +0000 (09:50 +0000)]
drm/i915: Convert vm->dev backpointer to vm->i915
99% of the time we access i915_address_space->dev we want the i915
device and not the drm device, so let's store the drm_i915_private
backpointer instead. The only real complication here are the inlines
in i915_vma.h where drm_i915_private is not yet defined and so we have
to choose an alternate path for our asserts.
Jérémy Lefaure [Mon, 28 Nov 2016 23:43:19 +0000 (18:43 -0500)]
drm/i915: fix compilation warnings on maybe uninitialized pointers
Two warnings are produced by gcc (tested with gcc 6.2.1):
drivers/gpu/drm/i915/intel_csr.c: In function ‘csr_load_work_fn’:
drivers/gpu/drm/i915/intel_csr.c:400:5: error: ‘fw’ is used
uninitialized in this function [-Werror=uninitialized]
if (fw)
^
and
In file included from drivers/gpu/drm/i915/i915_drv.h:47:0,
from drivers/gpu/drm/i915/intel_guc_loader.c:30:
drivers/gpu/drm/i915/intel_guc_loader.c: In function ‘intel_guc_init’:
./include/drm/drmP.h:228:2: error: ‘fw’ may be used uninitialized in this
function -Werror=maybe-uninitialized]
drm_printk(KERN_DEBUG, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
^~~~~~~~~~
drivers/gpu/drm/i915/intel_guc_loader.c:595:25: note: ‘fw’ was declared here
const struct firmware *fw;
^~
When CONFIG_DRM_I915_WERROR is set, those warnings break the build.
Initializing fw pointer to NULL in both cases removes the warnings.
Zhi Wang [Tue, 29 Nov 2016 06:55:16 +0000 (14:55 +0800)]
drm/i915: Move the release of PT page to the upper caller
a PT page will be released if it doesn't contain any meaningful mappings
during PPGTT page table shrinking. The PT entry in the upper level will
be set to a scratch entry.
Normally this works nicely, but in virtualization world, the PPGTT page
table is tracked by hypervisor. Releasing the PT page before modifying
the upper level PT entry would cause extra efforts.
As the tracked page has been returned to OS before losing track from
hypervisor, it could be written in any pattern. Hypervisor has to recognize
if a page is still being used as a PT page by validating these writing
patterns. It's complicated. Better let the guest modify the PT entry in
upper level PT first, then release the PT page.
Matthew Auld [Mon, 28 Nov 2016 10:36:48 +0000 (10:36 +0000)]
drm/i915: drop the struct_mutex when wedged or trying to reset
We grab the struct_mutex in intel_crtc_page_flip, but if we are wedged
or a reset is in progress we bail early but never seem to actually
release the lock.
Fixes: 7f1847ebf48b ("drm/i915: Simplify checking of GPU reset_counter in display pageflips") Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20161128103648.9235-1-matthew.auld@intel.com Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: <stable@vger.kernel.org> # v4.7+
Chris Wilson [Mon, 28 Nov 2016 14:36:49 +0000 (14:36 +0000)]
Revert "drm/i915/execlists: Use a local lock for dfs_link access"
This reverts commit 27745e829a5c ("drm/i915/execlists: Use a local lock
for dfs_link access") as the struct_mutex was required to prevent
concurrent retiring and freeing, now restored in the previous patch.
Chris Wilson [Mon, 28 Nov 2016 14:36:48 +0000 (14:36 +0000)]
drm/i915: Move priority bumping for flips earlier
David found another issue with priority bumping from mmioflips, where we
are accessing the requests concurrently to them being retired and freed.
Whilst we are skipping the dependency if has been submitted, that is not
sufficient to stop the dependency from disappearing if another thread
retires that request. To prevent we can either employ the struct_mutex (or a
request mutex in the future) to serialise retiring before it is freed.
Alternatively, we need to keep the dependencies alive using RCU whilst
they are being accessed via the DFS.
Fixes: 27745e829a5c ("drm/i915/execlists: Use a local lock for dfs_link access") Fixes: 9a151987d709 ("drm/i915: Add execution priority boosting for mmioflips") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: David Weinehall <david.weinehall@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20161128143649.4289-1-chris@chris-wilson.co.uk Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Arkadiusz Hiler [Fri, 25 Nov 2016 17:59:34 +0000 (18:59 +0100)]
drm/i915/guc: Drop guc2host/host2guc from names
To facilitate code reorganization we are renaming everything that
contains guc2host or host2guc.
host2guc_action() and host2guc_action_response() become guc_send()
and guc_recv() respectively.
Other host2guc_*() functions become simply guc_*().
Other entities are renamed basing on context they appear in:
- HOST2GUC_ACTIONS_& become INTEL_GUC_ACTION_*
- HOST2GUC_{INTERRUPT,TRIGGER} become GUC_SEND_{INTERRUPT,TRIGGER}
- GUC2HOST_STATUS_* become INTEL_GUC_STATUS_*
- GUC2HOST_MSG_* become INTEL_GUC_RECV_MSG_*
- action_lock becomes send_mutex
v2: drop unnecessary backslashes and use BIT() instead of '<<'
v3: shortened enum names and INTEL_GUC_STATUS_*
drm/i915: Don't sanitize has_decoupled_mmio if platform is not broxton
The check in __intel_uncore_early_sanitize() to disable decoupled mmio
would disable it for every platform that is not broxton. While that's
not a problem now since only broxton supports that, simply setting
.has_decoupled_mmio in a new platform's device info wouldn't suffice. So
avoid future confusion and change the workaround to only change the
value of has_decoupled_mmio for broxton.
Pass dev_priv to intel_setup_outputs() and functions called by it, since
those are all intel i915 specific functions. Also, in the majority of
the functions dev_priv is used more often than dev. In the rare cases
where there are a few calls back into drm core, a local dev variable was
added.
Chris Wilson [Fri, 25 Nov 2016 13:17:18 +0000 (13:17 +0000)]
drm/i915: Integrate i915_sw_fence with debugobjects
Add the tracking required to enable debugobjects for fences to improve
error detection in BAT. The debugobject interface lets us track the
lifetime and phases of the fences even while being embedded into larger
structs, i.e. to check they are not used after they have been released.
v2: Don't populate the stubs, debugobjects checks for a NULL pointer and
treats it equivalently.
Chris Wilson [Fri, 25 Nov 2016 13:17:17 +0000 (13:17 +0000)]
drm/i915: Hold a reference on the request for its fence chain
Currently, we have an active reference for the request until it is
retired. Though it cannot be retired before it has been executed by
hardware, the request may be completed before we have finished
processing the execute fence, i.e. we may continue to process that fence
as we free the request.
Fixes: 5590af3e115a ("drm/i915: Drive request submission through fence callbacks") Fixes: 23902e49c999 ("drm/i915: Split request submit/execute phase into two") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20161125131718.20978-3-chris@chris-wilson.co.uk
Chris Wilson [Fri, 25 Nov 2016 13:17:16 +0000 (13:17 +0000)]
drm/i915: Assert no external observers when unwind a failed request alloc
Before we return the request back to the kmem_cache after a failed
i915_gem_request_alloc(), we should assert that it has not been added to
any global state tracking.
Chris Wilson [Fri, 25 Nov 2016 13:17:15 +0000 (13:17 +0000)]
drm/i915: Add is-completed assert to request retire entrypoint
While we will check that the request is completed prior to being
retired, by placing an assert that the request is complete at the
entrypoint of the function we can more clearly document the function's
preconditions.
Chris Wilson [Thu, 24 Nov 2016 14:47:50 +0000 (14:47 +0000)]
drm/i915/debugfs: Update pageflip information
Show the last submitted seqno to the engine, not the overall next seqno,
as this is more pertinent information when inspecting the pageflip and
whether the CS or display engine stalled.
Joonas Lahtinen [Thu, 24 Nov 2016 14:47:49 +0000 (14:47 +0000)]
drm/i915: Rename i915_gem_timeline.next_seqno to .seqno
Rename i915_gem_timeline member 'next_seqno' into 'seqno' as
the variable is pre-increment. We've already had two bugs due
to the confusing name, second is fixed as follow-up patch.
Libin Yang [Fri, 11 Nov 2016 08:46:28 +0000 (16:46 +0800)]
drm/i915/audio: fix hdmi audio noise issue
Some monitors will have noise or even no sound after
applying the patch 6014ac12.
In patch 6014ac12, it will reset the cts value to 0 for HDMI.
However, we need to disable Enable CTS or M Prog bit. This is
the initial setting after HW reset.
Chris Wilson [Thu, 24 Nov 2016 12:58:51 +0000 (12:58 +0000)]
drm/i915: Use the precomputed value for whether to enable command parsing
As i915.enable_cmd_parser is an unsafe option, make it read-only at
runtime. Now that it is constant, we can use the value determined during
initialisation as to whether we need the cmdparser at execbuffer time.
v2: Remove the inline for its single user, it is clear enough (and
shorter) without!
Chris Wilson [Thu, 24 Nov 2016 09:47:52 +0000 (09:47 +0000)]
drm/i915/debugfs: Increment return value of gt.next_seqno
The i915_next_seqno read value is to be the next seqno used by the
kernel. However, in the conversion to atomics ops for gt.next_seqno, in
commit 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during
request allocation"), this was changed from a post-increment to a
pre-increment. This increment was missed from the value reported by
debugfs, so in effect it was reporting the current seqno (last
assigned), not the next seqno.
Fixes: 28176ef4cfa5 ("drm/i915: Reserve space in the global seqno during request allocation")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81209 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20161124094752.19129-1-chris@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Matthew Auld [Mon, 14 Nov 2016 22:39:34 +0000 (22:39 +0000)]
drm/i915: cleanup use of INSTR_CLIENT_MASK
Doing cmd_header >> 29 to extract our 3-bit client value where we know
cmd_header is a u32 shouldn't then also require the use of a mask. So
remove the redundant operation and get rid of INSTR_CLIENT_MASK now that
there are no longer any users.
Chris Wilson [Thu, 24 Nov 2016 09:34:01 +0000 (09:34 +0000)]
drm/i915/debugfs: Drop i915_hws_info
i915_hws_info() has not been kept upto date (missing new engines) and so
I consider it to be unused. HWS is included in the error state, which
would be an avenue to retrieving it if required in future (possibly via
i915_engine_info). As it is currently oopsing with an rpm testcase, just
remove it.
Fixes: 3b3f1650b1ca ("drm/i915: Allocate intel_engine_cs structure only for the enabled engines")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98838 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20161124093401.18852-1-chris@chris-wilson.co.uk Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
drm/i915: Remove all ->config dereferences from intel_hdmi, v2.
In all cases we can now obtain the relevant crtc_state/conn_state
from the relevant callbacks, which means all the ->config accesses
can be removed and the code cleaned up.
Ville Syrjälä [Mon, 14 Nov 2016 16:35:10 +0000 (18:35 +0200)]
drm/i915: Protect dev_priv->atomic_cdclk_freq with all the crtc locks
A modeset on one pipe can update dev_priv->atomic_cdclk_freq without
actually touching the hardware, in which case we won't force a modeset
on all the pipes, and thus won't lock any of the other pipes either.
That means a parallel plane update on another pipe could be looking at
a stale dev_priv->atomic_cdcdlk_freq and thus fail to notice when the
plane configuration is invalid, or potentially reject a valid update.
To overcome this we must protect writes to atomic_cdclk_freq with
all the crtc locks, and thus for reads any single crtc lock will
be sufficient protection.
Ville Syrjälä [Mon, 14 Nov 2016 16:35:09 +0000 (18:35 +0200)]
drm/i915: Fix cdclk vs. dev_cdclk mess when not recomputing things
When we end up not recomputing the cdclk, we need to populate
intel_state->cdclk with the "atomic_cdclk_freq" instead of the
current cdclk_freq. When no pipes are active, the actual cdclk_freq
may be lower than what the configuration of the planes and
pipes would require from the point of view of the software state.
This fixes bogus WARNS from skl_max_scale() which is trying to check
the plane software state against the cdclk frequency. So any time
it got called during DPMS off for instance, we might have tripped
the warn if the current mode would have required a higher than
minimum cdclk.
v2: Drop the dev_cdclk stuff (Maarten)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Mika Kahola <mika.kahola@intel.com> Cc: bruno.pagani@ens-lyon.org Cc: Daniel J Blueman <daniel.blueman@gmail.com> Cc: Paul Bolle <pebolle@tiscali.nl> Cc: Joseph Yasi <joe.yasi@gmail.com> Tested-by: Paul Bolle <pebolle@tiscali.nl> Tested-by: Joseph Yasi <joe.yasi@gmail.com> (v1) Cc: stable@vger.kernel.org Fixes: 1a617b77658e ("drm/i915: Keep track of the cdclk as if all crtc's were active.")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98214 Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1479141311-11904-2-git-send-email-ville.syrjala@linux.intel.com
Ville Syrjälä [Tue, 22 Nov 2016 16:02:01 +0000 (18:02 +0200)]
drm/i915: Use enum plane_id in VLV/CHV wm code
Let's try not to abuse plane->plane for sprites on VLV/CHV and instead
use plane->id. Since out watermark structures aren't entirely plane type
agnostic (for now) and start indexing sprites from 0 we'll add a small
helper to convert between the two bases.
Ville Syrjälä [Tue, 22 Nov 2016 16:01:58 +0000 (18:01 +0200)]
drm/i915: Use enum plane_id in SKL wm code
Nuke skl_wm_plane_id() and just use the new intel_plane->id.
v2: Convert skl_write_plane_wm() as well
v3: Convert skl_pipe_wm_get_hw_state() correctly
v4: Rebase due to changes in the wm code
Drop the cursor FIXME from the total data rate calc (Paulo)
Use the "[PLANE:%d:%s]" format in debug print (Paulo)
Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Lyude <cpaul@redhat.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1479830524-7882-4-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Lyude <lyude@redhat.com>
Ville Syrjälä [Tue, 22 Nov 2016 16:01:57 +0000 (18:01 +0200)]
drm/i915: Add crtc->plane_ids_mask
Add a mask of which planes are available for each pipe. This doesn't
quite work for old platforms with dynamic plane<->pipe assignment, but
as we don't support that sort of stuff (yet) we can get away with it.
The main use I have for this is the for_each_plane_id_on_crtc() macro
for iterating over all possible planes on the crtc. I suppose we could
not add the mask, and instead iterate by comparing intel_plane->pipe
but then we'd need a local intel_plane variable which is just
unnecessary clutter in some cases. But I'm not hung up on this, so if
people prefer the other option I could be convinced to use it.
Ville Syrjälä [Tue, 22 Nov 2016 16:01:56 +0000 (18:01 +0200)]
drm/i915: Add per-pipe plane identifier
As I told people in [1] we really should not be confusing enum plane
as a per-pipe plane identifier. Looks like that happened nonetheless, so
let's fix it up by splitting the two into two enums.
We'll also want something we just directly pass to various register
offset macros and whatnot on SKL+. So let's make this new thing work for that.
Currently we pass intel_plane->plane for the "sprites" and just a
hardcoded zero for the "primary" planes. We want to get rid of that
hardocoding so that we can share the same code for all planes (apart
from the legacy cursor of course).
Imre Deak [Mon, 21 Nov 2016 19:15:07 +0000 (21:15 +0200)]
drm/i915/lspcon: Remove unused force change mode parameter
All callers asked for a forced change but the function ignored this
parameter. It doesn't seem to be necessary to force the change in any
case so let's just remove the parameter.
Imre Deak [Mon, 21 Nov 2016 19:15:06 +0000 (21:15 +0200)]
drm/i915/lspcon: Wait for expected LSPCON mode to settle
Some LSPCON adaptors may return an incorrect LSPCON mode right after
waking from DP Sleep state. This is the case at least for the ParadTech
PS175 adaptor, both when waking because of exiting the DP Sleep to
active state, or due to any other AUX CH transfer. We can determine the
current expected mode based on whether the DPCD area is accessible,
since according to the LSPCON spec this area is only accesible
in PCON mode.
This wait will avoid us trying to change the mode, while the current
expected mode hasn't settled yet and start link training before the
adaptor thinks it's in PCON mode after waking from DP Sleep state.
Imre Deak [Mon, 21 Nov 2016 19:15:04 +0000 (21:15 +0200)]
drm/i915/lspcon: Ensure AUX CH is awake while in DP Sleep state
Some LSPCON adaptors won't properly wake up in response to an AUX
request after the adaptor was placed to a DP Sink Sleep state (via
writing 0x2 to DP_SET_POWER). Based on the DP 1.4 specification 5.2.5,
the sink may place the AUX CH into a low-power state while in Sleep
state, but should wake it up in response to an AUX request within 1-20ms
(answering with AUX defers while waking it up). As opposed to this at
least the ParadTech PS175 adaptor won't fully wake in response to the
first I2C-over-AUX access and will occasionally ignore the offset in I2C
messages. This can result in accessing the DDC register at offset 0
regardless of the specified offset and the LSPCON detection failing.
To fix this do an initial dummy read from the DPCD area. The PS175 will
defer this access until it's fully woken (taking ~150ms) making sure the
following I2C-over-AUX accesses will work correctly.
Cc: Shashank Sharma <shashank.sharma@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@intel.com>
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=98353 Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1479755707-29596-2-git-send-email-imre.deak@intel.com
Chris Wilson [Tue, 22 Nov 2016 14:41:21 +0000 (14:41 +0000)]
drm/i915: Stop the machine as we install the wedged submit_request handler
In order to prevent a race between the old callback submitting an
incomplete request and i915_gem_set_wedged() installing its nop handler,
we must ensure that the swap occurs when the machine is idle
(stop_machine).
Chris Wilson [Tue, 22 Nov 2016 14:41:20 +0000 (14:41 +0000)]
drm/i915: Complete requests in nop_submit_request
Since the submit/execute split in commit d55ac5bf97c6 ("drm/i915: Defer
transfer onto execution timeline to actual hw submission") the
global seqno advance was deferred until the submit_request callback.
After wedging the GPU, we were installing a nop_submit_request handler
(to avoid waking up the dead hw) but I had missed converting this over
to the new scheme. Under the new scheme, we have to explicitly call
i915_gem_submit_request() from the submit_request handler to mark the
request as on the hardware. If we don't the request is always pending,
and any waiter will continue to wait indefinitely and hangcheck will not
be able to resolve the lockup.
References: https://bugs.freedesktop.org/show_bug.cgi?id=98748
Testcase: igt/gem_eio/in-flight Fixes: d55ac5bf97c6 ("drm/i915: Defer transfer onto execution timeline to actual hw submission") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/20161122144121.7379-3-chris@chris-wilson.co.uk
Chris Wilson [Tue, 22 Nov 2016 14:41:19 +0000 (14:41 +0000)]
drm/i915: Disable hangcheck when wedged
If the gpu reset fails and the machine is terminally wedged, further
hangchecks achieve nothing but noise. Disable them, with a corollary
that we re-enable hangchecking after a successful GPU reset in case the
user is artificially bringing the machine back to life through the debug
interface.
Chris Wilson [Tue, 22 Nov 2016 14:41:18 +0000 (14:41 +0000)]
drm/i915: Don't deref context->file_priv ERR_PTR upon reset
When a user context is closed, it's file_priv backpointer is replaced by
ERR_PTR(-EBADF); be careful not to chase this invalid pointer after a
hang and a GPU reset.
Robert Bragg [Mon, 7 Nov 2016 19:49:57 +0000 (19:49 +0000)]
drm/i915: Add a kerneldoc summary for i915_perf.c
In particular this tries to capture for posterity some of the early
challenges we had with using the core perf infrastructure in case we
ever want to revisit adapting perf for device metrics.
Consistent with the kernel.perf_event_paranoid sysctl option that can
allow non-root users to access system wide cpu metrics, this can
optionally allow non-root users to access system wide OA counter metrics
from Gen graphics hardware.
Robert Bragg [Mon, 7 Nov 2016 19:49:53 +0000 (19:49 +0000)]
drm/i915: advertise available metrics via sysfs
Each metric set is given a sysfs entry like:
/sys/class/drm/card0/metrics/<guid>/id
This allows userspace to enumerate the specific sets that are available
for the current system. The 'id' file contains an unsigned integer that
can be used to open the associated metric set via
DRM_IOCTL_I915_PERF_OPEN. The <guid> is a globally unique ID for a
specific OA unit register configuration that can be reliably used by
userspace as a key to lookup corresponding counter meta data and
normalization equations.
The guid registry is currently maintained as part of gputop along with
the XML metric set descriptions and code generation scripts, ref:
Robert Bragg [Mon, 7 Nov 2016 19:49:52 +0000 (19:49 +0000)]
drm/i915: Enable i915 perf stream for Haswell OA unit
Gen graphics hardware can be set up to periodically write snapshots of
performance counters into a circular buffer via its Observation
Architecture and this patch exposes that capability to userspace via the
i915 perf interface.
v2:
Make sure to initialize ->specific_ctx_id when opening, without
relying on _pin_notify hook, in case ctx already pinned.
v3:
Revert back to pinning ctx upfront when opening stream, removing
need to hook in to pinning and to update OACONTROL on the fly.
Signed-off-by: Robert Bragg <robert@sixbynine.org> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Sourab Gupta <sourab.gupta@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/20161107194957.3385-7-robert@sixbynine.org
Robert Bragg [Mon, 7 Nov 2016 19:49:51 +0000 (19:49 +0000)]
drm/i915: Add 'render basic' Haswell OA unit config
Adds a static OA unit, MUX + B Counter configuration for basic render
metrics on Haswell. This is auto generated from an XML
description of metric sets, currently maintained in gputop, ref:
Robert Bragg [Tue, 8 Nov 2016 12:51:48 +0000 (12:51 +0000)]
drm/i915: don't whitelist oacontrol in cmd parser
Being able to program OACONTROL from a non-privileged batch buffer is
not sufficient to be able to configure the OA unit. This was originally
allowed to help enable Mesa to expose OA counters via the
INTEL_performance_query extension, but the current implementation based
on programming OACONTROL via a batch buffer isn't able to report useable
data without a more complete OA unit configuration. Mesa handles the
possibility that writes to OACONTROL may not be allowed and so only
advertises the extension after explicitly testing that a write to
OACONTROL succeeds. Based on this; removing OACONTROL from the whitelist
should be ok for userspace.
Removing this simplifies adding a new kernel api for configuring the OA
unit without needing to consider the possibility that userspace might
trample on OACONTROL state which we'd like to start managing within
the kernel instead. In particular running any Mesa based GL application
currently results in clearing OACONTROL when initializing which would
disable the capturing of metrics.
v2:
This bumps the command parser version from 8 to 9, as the change is
visible to userspace.
Robert Bragg [Mon, 7 Nov 2016 19:49:49 +0000 (19:49 +0000)]
drm/i915: return EACCES for check_cmd() failures
check_cmd() is checking whether a command adheres to certain
restrictions that ensure it's safe to execute within a privileged batch
buffer. Returning false implies a privilege problem, not that the
command is invalid.
The distinction makes the difference between allowing the buffer to be
executed as an unprivileged batch buffer or returning an EINVAL error to
userspace without executing anything.
In a case where userspace may want to test whether it can successfully
write to a register that needs privileges the distinction may be
important and an EINVAL error may be considered fatal.
In particular this is currently true for Mesa, which includes a test for
whether OACONTROL can be written too, but Mesa treats any error when
flushing a batch buffer as fatal, calling exit(1).
As it is currently Mesa can gracefully handle a failure to write to
OACONTROL if the command parser is disabled, but if we were to remove
OACONTROL from the parser's whitelist then the returned EINVAL would
break Mesa applications as they attempt an OACONTROL write.
This bumps the command parser version from 7 to 8, as the change is
visible to userspace.
Robert Bragg [Mon, 7 Nov 2016 19:49:48 +0000 (19:49 +0000)]
drm/i915: rename OACONTROL GEN7_OACONTROL
OACONTROL changes quite a bit for gen8, with some bits split out into a
per-context OACTXCONTROL register. Rename now before adding more gen7 OA
registers
Robert Bragg [Mon, 7 Nov 2016 19:49:47 +0000 (19:49 +0000)]
drm/i915: Add i915 perf infrastructure
Adds base i915 perf infrastructure for Gen performance metrics.
This adds a DRM_IOCTL_I915_PERF_OPEN ioctl that takes an array of uint64
properties to configure a stream of metrics and returns a new fd usable
with standard VFS system calls including read() to read typed and sized
records; ioctl() to enable or disable capture and poll() to wait for
data.
Records read all start with a common { type, size } header with
DRM_I915_PERF_RECORD_SAMPLE being of most interest. Sample records
contain an extensible number of fields and it's the
DRM_I915_PERF_PROP_SAMPLE_xyz properties given when opening that
determine what's included in every sample.
No specific streams are supported yet so any attempt to open a stream
will return an error.
v2:
use i915_gem_context_get() - Chris Wilson
v3:
update read() interface to avoid passing state struct - Chris Wilson
fix some rebase fallout, with i915-perf init/deinit
v4:
s/DRM_IORW/DRM_IOW/ - Emil Velikov
Mika Kuoppala [Fri, 18 Nov 2016 13:10:47 +0000 (15:10 +0200)]
drm/i915: Add per client max context ban limit
If we have a bad client submitting unfavourably across different
contexts, creating new ones, the per context scoring of badness
doesn't remove the root cause, the offending client.
To counter, keep track of per client context bans. Deny access if
client is responsible for more than 3 context bans in
it's lifetime.
v2: move ban check to context create ioctl (Chris)
v3: add commentary about hangs needed to reach client ban (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Mika Kuoppala [Wed, 16 Nov 2016 15:20:32 +0000 (17:20 +0200)]
drm/i915: Add bannable context parameter
Now when driver has per context scoring of 'hanging badness'
and also subsequent hangs during short windows are allowed,
if there is progress made in between, it does not make sense
to expose a ban timing window as a context parameter anymore.
Let the scoring be the sole indicator for ban policy and substitute
ban period context parameter as a boolean to get/set context
bannable property.
v2: allow non root to opt into being banned (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Mika Kuoppala [Wed, 16 Nov 2016 15:20:31 +0000 (17:20 +0200)]
drm/i915: Use request retirement as context progress
As hangcheck score was removed, the active decay of score
was removed also. This removed feature for hangcheck to detect
if the gpu client was accidentally or maliciously causing intermittent
hangs. Reinstate the scoring as a per context property, so that if
one context starts to act unfavourably, ban it.
v2: ban_period_secs as a gate to score check (Chris)
v3: decay in proper spot. scores as tunables (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Mika Kuoppala [Fri, 18 Nov 2016 13:09:04 +0000 (15:09 +0200)]
drm/i915: Decouple hang detection from hangcheck period
Hangcheck state accumulation has gained more steps
along the years, like head movement and more recently the
subunit inactivity check. As the subunit sampling is only
done if the previous state check showed inactivity, we
have added more stages (and time) to reach a hang verdict.
Asymmetric engine states led to different actual weight of
'one hangcheck unit' and it was demonstrated in some
hangs that due to difference in stages, simpler engines
were accused falsely of a hang as their scoring was much
more quicker to accumulate above the hang treshold.
To completely decouple the hangcheck guilty score
from the hangcheck period, convert hangcheck score to a
rough period of inactivity measurement. As these are
tracked as jiffies, they are meaningful also across
reset boundaries. This makes finding a guilty engine
more accurate across multi engine activity scenarios,
especially across asymmetric engines.
We lose the ability to detect cross batch malicious attempts
to hinder the progress. Plan is to move this functionality
to be part of context banning which is more natural fit,
later in the series.
v2: use time_before macros (Chris)
reinstate the pardoning of moving engine after hc (Chris)
v3: avoid global state for per engine stall detection (Chris)
v4: take timeline last retirement into account (Chris)
v5: do debug print on pardoning, split out retirement timestamp (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Mika Kuoppala [Wed, 16 Nov 2016 15:20:29 +0000 (17:20 +0200)]
drm/i915: Split up hangcheck phases
In order to simplify hangcheck state keeping, split hangcheck
per engine loop in three phases: state load, action, state save.
Add few more hangcheck actions to separate between seqno, head
and subunit movements. This helps to gather all the hangcheck
actions under a single switch umbrella.
Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
A.Sunil Kamath [Sun, 20 Nov 2016 17:50:26 +0000 (23:20 +0530)]
drm/i915: Use num_scalers instead of SKL_NUM_SCALERS in debugfs
Better to use num_scaler itself while printing scaler_info.
This fixes a bug of printing information for the missing
second scaler on pipe C for SKL platform.
Chris Wilson [Mon, 21 Nov 2016 11:07:59 +0000 (11:07 +0000)]
drm/i915: Add a warning on shutdown if signal threads still active
When unloading the module, it is expected that we have finished
executing all requests and so the signal threads should be idle. Add a
warning in case there are any residual requests in the signaler rbtrees
at that point.
Chris Wilson [Fri, 18 Nov 2016 21:17:47 +0000 (21:17 +0000)]
drm/i915: Skip final clflush if LLC is coherent
If the LLC is coherent with the object, we do not need to worry about
whether main memory and cache mismatch when we hand the object back to
the system.
Chris Wilson [Fri, 18 Nov 2016 21:17:46 +0000 (21:17 +0000)]
drm/i915: Always flush the dirty CPU cache when pinning the scanout
Currently we only clflush the scanout if it is in the CPU domain. Also
flush if we have a pending CPU clflush. We also want to treat the
dirtyfb path similar, and flush any pending writes there as well.
v2: Only send the fb flush message if flushing the dirt on flip
v3: Make flush-for-flip and dirtyfb look more alike since they serve
similar roles as end-of-frame marker.
Let's add a couple more BUG_ONs before this to ascertain that the request
did make it to hardware. The impossible part of this stacktrace is that
request must have been considered completed by the i915_request_wait()
before we tried to retire it.
Matthew Auld [Fri, 18 Nov 2016 17:02:16 +0000 (17:02 +0000)]
drm/i915: i915_pages_create_for_stolen should return err ptr
When gathering the pages from our backing storage we expect get_pages()
to either give us our sg_table or an err ptr. However when gathering our
fake pages for stolen memory we may return NULL in the event of a
failure. To prevent any funny business we should therefore return the
proper err ptr value.
Chris Wilson [Tue, 15 Nov 2016 16:46:20 +0000 (16:46 +0000)]
drm/i915: Be more careful to drop the GT wakeref
Since we can retire requests from multiple paths, we cannot assume that
i915_gem_retire_requests() is the sole path on which we can transition
to gt.active_requests == 0. A consequence of this is that we would skip
the function if we had already retired all the requests and not
scheduled the idle worker.
This is fallout from changing the routine from considering active_engines
(for which it was the only consumer) to active_requests.
v2: Move kicking the idle working to i915_gem_request_retire() otherwise
we could postpone the idle callback everytime we called retire_requests
even though we did no work.
v3: We only need to move the idle work kicking!
v4: Drop the BUG_ON(!awake) as we may be called from the shrinker in the
middle of constructing a request before we have marked the device awake.
v5: Add a BUG_ON() for active_requests underflow upon retirement (Joonas)
Chris Wilson [Wed, 16 Nov 2016 19:07:04 +0000 (19:07 +0000)]
drm/i915: Move frontbuffer CS write tracking from ggtt vma to object
I tried to avoid having to track the write for every VMA by only
tracking writes to the ggtt. However, for the purposes of frontbuffer
tracking this is insufficient as we need to invalidate around writes not
just to the the ggtt but all aliased ppgtt views of the framebuffer. By
moving the critical section to the object and only doing so for
framebuffer writes we can reduce the tracking even further by only
watching framebuffers and not vma.
Matthew Auld [Thu, 17 Nov 2016 21:04:10 +0000 (21:04 +0000)]
drm/i915: add i915_address_space_fini
We already have an i915_address_space_init, so for symmetry we should
also have a _fini, plus we already open code it twice. This then also
fixes a bug where we leak the timeline for the ggtt vm.
v2: don't forget about the struct_mutex for the ggtt path.
Chris Wilson [Thu, 17 Nov 2016 15:58:46 +0000 (15:58 +0000)]
drm/i915: Add a few more sanity checks for stolen handling
We should never be called via obj->ops->release() on anything other than
a fully formed stolen object, so raise that to an assert. In the process
tidy up a comment and variable no longer used outside of a conditional
BUG.
Chris Wilson [Wed, 16 Nov 2016 12:26:18 +0000 (12:26 +0000)]
drm/i915: Remove stolen object spam
We don't spam the debug when we create a normal object, nor when we
allocate their pages. Yet we do for stolen objects, and since these are
quite frequently used (at least once per context), the resulting spam
floods the dmesg in CI.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
projects / linux-beck.git / log