Validate the compliance test link parameters when the compliance
test dpcd registers are read. Also validate them in compute_config
before using them since the max values might have been reduced
due to link training fallback.
If either the link rate or lane count is invalid, we still bail
from using the test parameters since the combination would not work
and instead use the fallback values.
v2:
* Added commit message to explain why we still bail when either of
of the params is invalid (Ville Syrjala)
* Add reason for validating in the comment (Jani Nikula)
* Also check if index >= 0 after validating (Jani Nikula)
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Tested-by: Nathan Ciobanu <nathan.d.ciobanu@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1496954463-18038-2-git-send-email-manasi.d.navare@intel.com
There's no reason to entirely wedge the gpu, for the minimal deadlock
bugfix we only need to unbreak/decouple the atomic commit from the gpu
reset. The simplest way to fix that is by replacing the
unconditional fence wait a the top of commit_tail by a wait which
completes either when the fences are done (normal case, or when a
reset doesn't need to touch the display state). Or when the gpu reset
needs to force-unblock all pending modeset states.
The lesser source of deadlocks is when we try to pin a new framebuffer
and run into a stall. There's a bunch of places this can happen, like
eviction, changing the caching mode, acquiring a fence on older
platforms. And we can't just break the depency loop and keep going,
the only way would be to break out and restart. But the problem with
that approach is that we must stall for the reset to complete before
we grab any locks, and with the atomic infrastructure that's a bit
tricky. The only place is the ioctl code, and we don't want to insert
code into e.g. the BUSY ioctl. Hence for that problem just create a
critical section, and if any code is in there, wedge the GPU. For the
steady-state this should never be a problem.
Note that in both cases TDR itself keeps working, so from a userspace
pov this trickery isn't observable. Users themselvs might spot a short
glitch while the rendering is catching up again, but that's still
better than pre-TDR where we've thrown away all the rendering,
including innocent batches. Also, this fixes the regression TDR
introduced of making gpu resets deadlock-prone when we do need to
touch the display.
One thing I noticed is that gpu_error.flags seems to use both our own
wait-queue in gpu_error.wait_queue, and the generic wait_on_bit
facilities. Not entirely sure why this inconsistency exists, I just
picked one style.
A possible future avenue could be to insert the gpu reset in-between
ongoing modeset changes, which would avoid the momentary glitch. But
that's a lot more work to implement in the atomic commit machinery,
and given that we only need this for pre-g4x hw, of questionable
utility just for the sake of polishing gpu reset even more on those
old boxes. It might be useful for other features though.
v2: Rebase onto 4.13 with a s/wait_queue_t/struct wait_queue_entry/.
v3: Really emabarrassing fixup, I checked the wrong bit and broke the
unbreak/wakeup logic.
v4: Also handle deadlocks in pin_to_display.
v5: Review from Michel:
- Fixup the BUILD_BUG_ON
- Don't forget about the overlay
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> (v2)
Cc: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170808080828.23650-3-daniel.vetter@ffwll.ch
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Blocking in a worker is ok, that's what the unbound_wq is for. And it
unifies the paths between the blocking and nonblocking commit, giving
me just one path where I have to implement the deadlock avoidance
trickery in the next patch.
I first tried to implement the following patch without this rework, but
force-completing i915_sw_fence creates some serious challenges around
properly cleaning things up. So wasn't a feasible short-term approach.
Another approach would be to simple keep track of all pending atomic
commit work items and manually queue them from the reset code. With the
caveat that double-queue in case we race with the i915_sw_fence must be
avoided. Given all that, taking the cost of a double schedule in atomic
for the short-term fix is the best approach, but can be changed in the future of course.
v2: Amend commit message (Chris).
v3: Add comment explaining why we do nothing in the sw_fence complete
callback (Michel).
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> (v2)
Cc: Michel Thierry <michel.thierry@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170808080828.23650-2-daniel.vetter@ffwll.ch
... using the biggest hammer we have. This is essentially a weaponized
version of the timeout-based wedging Chris added in
commit 36703e79a9
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Thu Jun 22 11:56:25 2017 +0100
drm/i915: Break modeset deadlocks on reset
Because defense-in-depth is good it's good to still have both. Also
note that with the locking change we can now restrict this a lot (old
gpus and special testing only), so this doesn't kill the TDR benefits
on at least anything remotely modern.
And futuremore with a few tricks it should be possible to make a much
more educated guess about whether an atomic commit is stuck waiting on
the gpu (atomic_t counting the pending i915_sw_fence used by the
atomic modeset code should do it), so we can improve this.
But for now just start with something that is guaranteed to recover
faster, for much better CI througput.
This defacto reverts TDR on these platforms, but there's not really a
single commit to specify as the sole offender.
v2: Add a debug message to explain what's going on. We can't DRM_ERROR
because that spams CI. And the timeout based fallback still prints a
DRM_ERROR, in case something goes wrong.
v3: Fix comment layout (Michel)
Fixes: 4680816be3 ("drm/i915: Wait first for submission, before waiting for request completion")
Fixes: 221fe79945 ("drm/i915: Perform a direct reset of the GPU from the waiter")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> (v2)
Cc: Michel Thierry <michel.thierry@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> (v2)
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170808080828.23650-1-daniel.vetter@ffwll.ch
When switching between contexts using the aliasing_ppgtt, the VM is
shared. We don't need to reload the PD registers unless they are dirty.
Martin Peres reported an issue that looks like corruption between
Haswell context switches, bisecting to commit f9326be5f1 ("drm/i915:
Rearrange switch_context to load the aliasing ppgtt on first use").
Switching between the same mm (the aliasing_ppgtt is used for all
contexts in this case) should be a nop, but appears to trigger some
side-effects in the context switch. However, as we know the switch
is redundant in this case, we can skip it and continue to ignore the
issue until somebody feels strong enough to investigate full-ppgtt on
gen7 again!
Except.. Martin was using full-ppgtt which is not supported as it
doesn't work correctly yet. So whilst the bisect did yield valuable
information about the failures, the fix should not have any user impact
under default settings, with the exception of a slightly lower
throughput on xcs as the VM would always be reloaded.
v2: Also remember to set the legacy_active_context following the switch
on xcs (commit e8a9c58fcd ("drm/i915: Unify active context tracking
between legacy/execlists/guc"))
Fixes: f9326be5f1 ("drm/i915: Rearrange switch_context to load the aliasing ppgtt on first use")
Fixes: e8a9c58fcd ("drm/i915: Unify active context tracking between legacy/execlists/guc")
Reported-by: Martin Peres <martin.peres@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Martin Peres <martin.peres@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170812152724.6883-1-chris@chris-wilson.co.uk
For 0.85V cnl_get_buf_trans_edp() returns the DP table, instead of EDP.
Use the correct table.
The error was pointed out by this clang warning:
drivers/gpu/drm/i915/intel_ddi.c:392:39: warning: variable
'cnl_ddi_translations_edp_0_85V' is not needed and will not be emitted
[-Wunneeded-internal-declaration]
static const struct cnl_ddi_buf_trans cnl_ddi_translations_edp_0_85V[] = {
Fixes: cf54ca8bc5 ("drm/i915/cnl: Implement voltage swing sequence.")
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170717195854.192139-1-mka@chromium.org
In our snb farm in CI we have plenty of underruns, but not enough
stolen memory to enable fbc. Which means every time there's an
underrun the no_fbc_reason swichtes to something that makes
kms_frontbuffer_tracking fail instead of skip, adding massive amounts
of additional noise to igt test runs.
Make sure we don't try to disable fbc when it's off already.
v2: Squash in additional WARN_ON suggestion from Chris.
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170811072327.4335-1-daniel.vetter@ffwll.ch
This was based on a patch originally by Kristian. It has been modified
pretty heavily to use the new callbacks from the previous patch.
v2:
- Add LINEAR and Yf modifiers to list (Ville)
- Combine i8xx and i965 into one list of formats (Ville)
- Allow 1010102 formats for Y/Yf tiled (Ville)
v3:
- Handle cursor formats (Ville)
- Put handling for LINEAR in the mod_support functions (Ville)
v4:
- List each modifier explicitly in supported modifiers (Ville)
- Handle the CURSOR plane (Ville)
v5:
- Split out cursor and sprite handling (Ville)
v6:
- Actually use the sprite funcs (Emil)
- Use unreachable (Emil)
v7:
- Only allow Intel modifiers and LINEAR (Ben)
v8
- Fix spite assert introduced in v6 (Daniel)
v9
- Change vendor check logic to avoid magic 56 (Emil)
- Reorder skl_mod_support (Ville)
- make intel_plane_funcs static, could be done as of v5 (Ville)
- rename local variable intel_format_modifiers to modifiers (Ville)
- actually use sprite modifiers
- split out modifier/formats by platform (Ville)
v10:
- Undo vendor check from v9
v11:
- Squash CCS advertisement into this patch (daniels)
- Don't advertise CCS on higher sprite planes (daniels)
v12:
- Don't advertise Y-tiled or CCS on any sprite planes, since we don't
allocate enough DDB space for it to work. (daniels)
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> (v8)
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Stone <daniels@collabora.com>
SKL+ display engine can scan out certain kinds of compressed surfaces
produced by the render engine. This involved telling the display engine
the location of the color control surfae (CCS) which describes
which parts of the main surface are compressed and which are not. The
location of CCS is provided by userspace as just another plane with its
own offset.
Add the required stuff to validate the user provided AUX plane metadata
and convert the user provided linear offset into something the hardware
can consume.
Due to hardware limitations we require that the main surface and
the AUX surface (CCS) be part of the same bo. The hardware also
makes life hard by not allowing you to provide separate x/y offsets
for the main and AUX surfaces (excpet with NV12), so finding suitable
offsets for both requires a bit of work. Assuming we still want keep
playing tricks with the offsets. I've just gone with a dumb "search
backward for suitable offsets" approach, which is far from optimal,
but it works.
Also not all planes will be capable of scanning out compressed surfaces,
and eg. 90/270 degree rotation is not supported in combination with
decompression either.
This patch may contain work from at least the following people:
* Vandana Kannan <vandana.kannan@intel.com>
* Daniel Vetter <daniel@ffwll.ch>
* Ben Widawsky <ben@bwidawsk.net>
v2: Deal with display workarounds 0390, 0531, 1125 (Paulo)
v3: Pretend CCS tiles are regular 128 byte wide Y tiles (Jason)
Put the AUX register defines to the correct place
Fix up the slightly bogus rotation check
v4: Use I915_WRITE_FW() due to plane update locking changes
s/return -EINVAL/goto err/ in intel_framebuffer_init()
Eliminate a bunch hardcoded numbers in CCS code
v5: (By Ben)
conflict resolution +
- res_blocks += fixed_16_16_to_u32_round_up(y_tile_minimum);
+ res_blocks += fixed16_to_u32_round_up(y_tile_minimum);
v6: (daniels) Fix botched commit message.
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Signed-off-by: Ville Syrjä <ville.syrjala@linux.intel.com>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net> (v1)
Reviewed-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Stone <daniels@collabora.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170801165817.7063-1-ben@bwidawsk.net
SKL+ display engine can scan out certain kinds of compressed surfaces
produced by the render engine. This involved telling the display engine
the location of the color control surfae (CCS) which describes which
parts of the main surface are compressed and which are not. The location
of CCS is provided by userspace as just another plane with its own offset.
By providing our own format information for the CCS formats, we should
be able to make framebuffer_check() do the right thing for the CCS
surface as well.
Note that we'll return the same format info for both Y and Yf tiled
format as that's what happens with the non-CCS Y vs. Yf as well. If
desired, we could potentially return a unique pointer for each
pixel_format+tiling+ccs combination, in which case we immediately be
able to tell if any of that stuff changed by just comparing the
pointers. But that does sound a bit wasteful space wise.
v2: Drop the 'dev' argument from the hook
v3: Include the description of the CCS surface layout
v4: Pretend CCS tiles are regular 128 byte wide Y tiles (Jason)
v5: Re-drop 'dev', fix commit message, add missing drm_fourcc.h
description of CCS layout. (daniels)
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Acked-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net> (v3)
Reviewed-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Ville Syrjä <ville.syrjala@linux.intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Stone <daniels@collabora.com>
Ben Widawsky/Daniel Stone need the extended modifier support from
drm-misc to be able to merge CCS support for i915.ko
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In the original selftest, we didn't care what the engine->id was, just
that it could uniquely identify it. Later though, we started tracking
the mock engines in the fixed size arrays around the drm_i915_private and
so we now require their indices to be correct. This becomes an issue when
using the standalone harness which runs all available tests at module load,
and so we quickly assign an out-of-bounds index to an engine as we
reallocate the mock GEM device between tests. It doesn't show up in
igt/drv_selftest as that runs each subtest individually.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102045
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20170809163930.26470-1-chris@chris-wilson.co.uk
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
UAPI Changes:
- vc4: Add ioctl to allow attaching a label to a bo (Eric)
- Add new format/modifier blob plane property (Ben)
- armada: Use __u32/__u64 instead of uint32_t/uint64_t (Mikko)
- [kinda uapi] fb_helper: Expose display_info size via fb_info (David)
Core Changes:
- Default gem_dumb_[map_offset|destroy] as mmap/destroy implementations (Noralf)
- Simplify atomic properties by removing the helpers and handling in core (Daniel)
Driver Changes:
- stm: Add STM32 DSI controller driver (Phillipe)
- vc4: Add HDMI CEC support (Hans)
- rockchip: Refactor register init & soc version handling (Mark)
- misc: Remove .load_lut, .gamma_set, .gamma_get dead code (Peter)
- dw-hdmi: Add HDMI CEC support (Russell)
Cc: Philippe CORNU <philippe.cornu@st.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Mark yao <mark.yao@rock-chips.com>
Cc: Peter Rosin <peda@axentia.se>
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Cc: Mikko Rapeli <mikko.rapeli@iki.fi>
Cc: David Lechner <david@lechnology.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
* tag 'drm-misc-next-2017-08-08' of git://anongit.freedesktop.org/git/drm-misc: (107 commits)
drm: Nuke drm_atomic_legacy_backoff
drm: Nuke drm_atomic_helper_connector_dpms
drm: Nuke drm_atomic_helper_connector_set_property
drm: Nuke drm_atomic_helper_plane_set_property
drm: Nuke drm_atomic_helper_crtc_set_property
drm: Handle properties in the core for atomic drivers
drm: Don't update property values for atomic drivers
drm/omap: Rework the rotation-on-crtc hack
drm/radeon: Use the drm_driver.dumb_destroy default
drm/i915: Use the drm_driver.dumb_destroy default
drm/sti: Use .dumb_map_offset and .dumb_destroy defaults
drm: bridge: synopsys/dw-hdmi: Provide default configuration function for HDMI 2.0 PHY
drm/fb-helper: pass physical dimensions to fbdev
uapi drm/armada_drm.h: use __u32 and __u64 instead of uint32_t and uint64_t
drm/bridge: dw-hdmi: remove CEC engine register definitions
drm/bridge: dw-hdmi: add cec driver
drm/bridge: dw-hdmi: add missing cec_notifier_put
drm: remove unused and redundant callbacks
staging: vboxvideo: remove dead gamma lut code
drm: dw-hdmi-i2s: add missing company name on Copyright
...
The reason behind the original indirection through the helper
functions was to allow existing drivers to overwrite how they handle
properties. For example when a vendor-specific userspace had
expectations that didn't match atomic. That seemed likely, since
atomic is standardizing a _lot_ more of the behaviour of a kms driver.
But 20 drivers later there's no such need at all. Worse, this forces
all drivers to hook up the default behaviour, breaking userspace if
they forget to do that. And it forces us to export a bunch of core
function just for those helpers.
And finally, these helpers are the last places using
drm_atomic_legacy_backoff() and the implicit acquire_ctx.
This patch here just implements the new behaviour and updates the
docs. Follow-up patches will garbage-collect all the dead code.
v2: Fixup docs even better!
v3: Make it actually work ...
v4: Drop the uses_atomic_modeset() checks from the previous patch
again, since they're now moved up in the callchain.
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Archit Taneja <architt@codeaurora.org> (v3)
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170725120204.2107-1-daniel.vetter@ffwll.ch
Atomic drivers only use the property value store for immutable (i.e.
can't be set by userspace, but the kernel can still adjust it)
properties. The only tricky part is the removal of the update in
drm_atomic_helper_update_legacy_modeset_state().
This was added in
commit 8c10342cb4 (tag: topic/drm-misc-2015-07-28)
Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Date: Mon Jul 27 13:24:29 2015 +0200
drm/atomic: Update legacy DPMS state during modesets, v3.
by copying it from the i915 code, where it was originally added in
commit 68d3472047
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu Sep 6 22:08:35 2012 +0200
drm/i915: update dpms property in set_mode
for the legacy modeset code. The reason we needed this hack was that
i915 didn't yet set DRIVER_ATOMIC, and we checked for that instead of
the newer-ish drm_drv_uses_atomic_modeset(), which avoids such
troubles. With the correct feature checks this isn't needed anymore at
all.
Also make sure that drivers don't accidentally get this wrong by
making the exported version of drm_object_property_get_value() only
work for legacy drivers. Only gma500 uses it anyway.
v2: Fixup the uses_atomic_modeset() checks (Maarten)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170725120137.1903-1-daniel.vetter@ffwll.ch
I want/need to rework the core property handling, and this hack is
getting in the way. But since it's a non-standard propety only used by
legacy userspace we know that this will only every be called from
ioctl code. And never on some other free-standing state struct, where
this old hack wouldn't work either.
v2: don't forget zorder and get_property!
v3: Shadow the legacy state to avoid locking issues in get_property
(Maarten).
v4: Review from Laurent
- Move struct omap_crtc_state into omap_crtc.c
- Clean up comments.
- Don't forget to copy the shadowed state over on duplicate.
v5: Don't forget to update the reset handler (Maarten).
v6: Update omap_crtc_state shadow values in omap_crtc_atomic_check (Maarten).
v7:
- Fix get_property to return 0 and set value in *val (Maarten).
- Update comment in set_property for changes in v6 (Maarten).
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> (v4)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> (v4)
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/a6a10a4f-2ebc-5f81-00bd-5e906967f384@linux.intel.com
