Starting from opregion version 2.1 (roughly corresponding to ICL+) the
RVDA field is relative from the beginning of opregion, not absolute
address.
Fix the error path while at it.
v2: Make relative vs. absolute conditional on the opregion version,
bumped for the purpose. Turned out there are machines relying on
absolute RVDA in the wild.
v3: Fix the version checks
Fixes: 04ebaadb9f ("drm/i915/opregion: handle VBT sizes bigger than 6 KB")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190208184254.24123-2-jani.nikula@intel.com
The u32 version field encodes major, minor, revision and reserved. We've
basically been checking for any non-zero version.
Add opregion version logging while at it.
v2: Fix the fix of the version check
Fixes: 04ebaadb9f ("drm/i915/opregion: handle VBT sizes bigger than 6 KB")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190208184254.24123-1-jani.nikula@intel.com
If we allocate while iterating the rbtree of active nodes, we may hit
the shrinker and so retire the i915_active, reaping the rbtree. Modifying
the rbtree as we iterate is not good behaviour, so acquire the
i915_active first to keep the tree intact whenever we allocate.
Fixes: a42375af0a ("drm/i915: Release the active tracker tree upon idling")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190208134704.23039-1-chris@chris-wilson.co.uk
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
In the next patch, we add another user that wants to check whether
requests can be merge into a single HW execution, and in the future we
want to add more conditions under which requests from the same context
cannot be merge. In preparation, extract out can_merge_rq().
v2: Reorder tests to decide if we can continue filling ELSP and bonus
comments.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190208235108.23127-1-chris@chris-wilson.co.uk
If we haven't even begun executing the payload of the stalled request,
then we should not claim that its userspace context was guilty of
submitting a hanging batch.
v2: Check for context corruption before trying to restart.
v3: Preserve semaphores on skipping requests (need to keep the timelines
intact).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190208153708.20023-7-chris@chris-wilson.co.uk
Since we use the debugfs to recover the device after modifying the
i915.reset parameter, we need to be sure that we apply the reset and not
piggy-back onto a concurrent one in order for the parameter to take
effect.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190208153708.20023-5-chris@chris-wilson.co.uk
On wedging, we mark all executing requests as complete and all pending
requests completed as soon as they are ready. Before unwedging though we
wish to flush those pending requests prior to restoring default
execution, and so we must wait. Do so uninterruptibly as we do not provide
the EINTR gracefully back to userspace in this case but persists in
keeping the permanently wedged state without restarting the syscall.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190208153708.20023-4-chris@chris-wilson.co.uk
When declaring the GPU wedged, we do need to hit the GPU with the reset
hammer so that its state matches our presumed state during cleanup. If
the reset fails, it fails, and we may be unhappy but wedged. However, if
we are testing our wedge/unwedged handling, the desync carries over into
the next test and promptly explodes.
References: https://bugs.freedesktop.org/show_bug.cgi?id=106702
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190208153708.20023-3-chris@chris-wilson.co.uk
Previously, we were able to rely on the recursive properties of
struct_mutex to allow us to serialise revoking mmaps and reacquiring the
FENCE registers with them being clobbered over a global device reset.
I then proceeded to throw out the baby with the bath water in order to
pursue a struct_mutex-less reset.
Perusing LWN for alternative strategies, the dilemma on how to serialise
access to a global resource on one side was answered by
https://lwn.net/Articles/202847/ -- Sleepable RCU:
1 int readside(void) {
2 int idx;
3 rcu_read_lock();
4 if (nomoresrcu) {
5 rcu_read_unlock();
6 return -EINVAL;
7 }
8 idx = srcu_read_lock(&ss);
9 rcu_read_unlock();
10 /* SRCU read-side critical section. */
11 srcu_read_unlock(&ss, idx);
12 return 0;
13 }
14
15 void cleanup(void)
16 {
17 nomoresrcu = 1;
18 synchronize_rcu();
19 synchronize_srcu(&ss);
20 cleanup_srcu_struct(&ss);
21 }
No more worrying about stop_machine, just an uber-complex mutex,
optimised for reads, with the overhead pushed to the rare reset path.
However, we do run the risk of a deadlock as we allocate underneath the
SRCU read lock, and the allocation may require a GPU reset, causing a
dependency cycle via the in-flight requests. We resolve that by declaring
the driver wedged and cancelling all in-flight rendering.
v2: Use expedited rcu barriers to match our earlier timing
characteristics.
v3: Try to annotate locking contexts for sparse
v4: Reduce selftest lock duration to avoid a reset deadlock with fences
v5: s/srcu/reset_backoff_srcu/
v6: Remove more stale comments
Testcase: igt/gem_mmap_gtt/hang
Fixes: eb8d0f5af4 ("drm/i915: Remove GPU reset dependence on struct_mutex")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190208153708.20023-2-chris@chris-wilson.co.uk
Currently, we may simultaneously release the fence register from both
fence_update() and i915_gem_restore_fences(). This is dangerous, so
defer the bookkeeping entirely to i915_gem_restore_fences() when the
device is asleep.
Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190208153708.20023-1-chris@chris-wilson.co.uk
On g4x+ we depend on the primary plane DSPCNTR gamma/csc enable
bits for the pipe bottom color. To guarantee that those are
correct already when enabling the crtc let's do an explicit
->disable_plane() call before enabling the pipe.
On skl+ this will be handled by the explicit PIPE_BOTTOM_COLOR
register which is already part of the normal color commit we
do durign crtc enable.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190207202146.26423-8-ville.syrjala@linux.intel.com
Planes scanning out C8 will want to use the legacy lut as
their palette. That means the LUT content are unlikely to
be useful for gamma correction on other planes. Thus we
should disable pipe gamma for all the other planes. And
we should reject any non legacy LUT configurations when
C8 planes are present.
Fixes the appearance of the hw cursor when running
X -depth 8.
Note that CHV with it's independent CGM degamma/gamma LUTs
could probably use the CGM for gamma correction even when
the legacy LUT is used for C8. But that would require a
new uapi for configuring the legacy LUT and CGM LUTs at
the same time. Totally not worth it.
v2: Fix typo (Uma)
Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190207202146.26423-7-ville.syrjala@linux.intel.com
As with pipe gamma we can avoid the potential precision loss from
the pipe csc unit when there is no need to use it. And again
we need the same logic for updating the planes.
v2: Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190207202146.26423-6-ville.syrjala@linux.intel.com
The pipe internal precision is higher than what we currently program to
the degamma/gamma LUTs. We can get a higher quality image by bypassing
the LUTs when they're not needed. Let's do that.
Each plane has its own control bit for this, so we have to update
all active planes. The way we've done this we don't actually have
to run through the whole .check_plane() thing. And we actually
do the .color_check() after .check_plane() so we couldn't even do
that without shuffling the code around.
Additionally on pre-skl we have to update the primary plane regardless
of whether it's active or not on account of the primary plane gamma
enable bit also affecting the pipe bottom color.
v2: Drop the '.' from patch title (Uma)
Fix 'primayr' typo (Uma,Matt)
Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190207202146.26423-5-ville.syrjala@linux.intel.com
Just like we did for pipe gamma, let's also track the pipe csc
state. The hardware only exists on ILK+, and currently we always
enable it on hsw+ and never on any other platforms. Just like
with pipe gamma, the primary plane control register is used
for the readout on pre-SKL, and the pipe bottom color register
on SKL+.
v2: Rebase
v3: Allow fastboot with csc_enable changes (Maarten)
Deal with HAS_GMCH
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190207202146.26423-4-ville.syrjala@linux.intel.com
Track whether pipe gamma is enabled or disabled. For now we
stick to the current behaviour of always enabling gamma. But
we do get working state readout for this now. On SKL+ we use
the pipe bottom color as our hardware state. On pre-SKL we
read the state back from the primary plane control register.
That only really correct for g4x+, as older platforms never
gamma correct pipe bottom color. But doing the readout the
same way on all platforms is fine, and there is no other way
to do it really.
v2: Initialize val at declaration (Uma)
Drop the bogus skl scaler comment change (Uma)
Rebase
v3: Allow fastboot with gamma_enable changes (Maarten)
v4: Drop the PIPE_BOTTOM_COLOR write from
intel_update_pipe_config() again. It snuck back in
during the rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190207203913.5529-1-ville.syrjala@linux.intel.com
On pre-HSW gamma mode is configured via PIPECONF. The bits are
the same except shifted up, so we can reuse just store them in
crtc_state->gamma_mode in the HSW+ way, allowing us to share
some code later.
v2: Allow fastboot with gamma_mode changes (Maarten)
Add space around the '<<' in the reg macro
Deal with HAS_GMCH
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190207202146.26423-2-ville.syrjala@linux.intel.com
Changing the i915_edp_psr_debug was enabling, disabling or switching
PSR version by directly calling intel_psr_disable_locked() and
intel_psr_enable_locked(), what is not the default PSR path that will
be executed by real users.
So lets force a fastset in the PSR CRTC to trigger a pipe update and
stress the default code path.
Recently a bug was found when switching from PSR2 to PSR1 while
enable_psr kernel parameter was set to the default parameter, this
changes fix it and also fixes the bug linked bellow were DRRS was
left enabled together with PSR when enabling PSR from debugfs.
v2: Handling missing case: disabled to PSR1
v3: Not duplicating the whole atomic state(Maarten)
v4: Adding back the missing call to intel_psr_irq_control(Dhinakaran)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108341
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190206211845.5322-1-jose.souza@intel.com
The downgrade of the fullmodeset into fastset
intel_encoder->update_pipe, in possible scenario, skips the En/Dis-able
DDI. Hence breaks the HDCP state change handling.
We also don't have any hdcp tests in CI, because the shard runs don't
have hdcp capable outputs :-/
So this change fixs it by handling the HDCP state change request at
intel_encoder->update_pipe too along with enable and disable of the DDI.
Fixes: d19f958db2 ("drm/i915: Enable fastset for non-boot modesets.")
v2:
Added commit id that broke the HDCP [Daniel]
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
cc: Hans de Goede <hdegoede@redhat.com>
cc: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/1549295080-18353-1-git-send-email-ramalingam.c@intel.com
The LUTs are single buffered so we should program them after
the double buffered pipe updates have been latched by the
hardware.
We'll also fix up the IPS vs. split gamma w/a to do the IPS
disable like everyone else. Note that this is currently dead
code as we don't use the split gamma mode on HSW, but that
will be fixed up shortly.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190205160848.24662-7-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Split the color management hooks along the single vs. double
buffered registers line. Of the currently programmed registers
GAMMA_MODE and the ilk+ pipe CSC are double buffered, the
LUTS and CHV CGM block are single buffered.
The double buffered register will be programmed during the
normal pipe update with evasion, and also during pipe enable
so that the settings will already be correct when the pipe
starts up before the planes are enabled.
The single buffered registers are currently programmed before
the vblank evade. Which is totally wrong, but we'll correct
that later.
v2: Add some docs to explain the two vfuncs (Matt,Uma)
Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190205160848.24662-6-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
For bdw+ let's move the GAMMA_MODE write for the legacy LUT
mode into the .load_luts() funciton directly, rather than
relying on haswell_load_luts(). We'll be getting rid of
haswell_load_luts() entirely soon, and it's anyway cleaner
to have the GAMMA_MODE write in a single place.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190205160848.24662-5-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Pass the crtc state etc. as const to the color management commit
functions. And while at it polish some of the local variables.
v2: Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190205160848.24662-4-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
We shouldn't be computing gamma mode during the commit phase.
Move it to the check phase.
v2: Reword comments a bit (Matt)
Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190205160848.24662-3-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
On g4x+ the pipe gamma enable bit for the primary plane affects
the pipe bottom color as well. The same for the pipe csc enable
bit on ilk+. Thus we must configure those bits correctly even
when the primary plane is disabled.
To make the feasible let's split those settings from the
plane_ctl() function into a seprate funciton that we can
call from the ->disable_plane() hook as well.
For consistency we'll do that on all the plane types. While
that has no real benefits at this time, it'll become useful
when we start to control the pipe gamma/csc enable bits
dynamically when we overhaul the color management code.
On pre-g4x there doesn't appear to be any way to gamma
correct the pipe bottom color, but sticking to the same
pattern doesn't hurt. And it'll still help us to do
crtc state readout correctly for the pipe gamma enable
bit for the color management overhaul.
An alternative apporach would be to still precompute these
bits into plane_state->ctl, but that would require that we
run through the plane check even when the plane isn't logically
enabled on any crtc. Currently that condition causes us to
short circuit the entire thing and not call ->check_plane().
There would also be some chicken and egg problems with
->check_plane() vs. crtc color state check that would
requite splitting certain things into multiple steps.
So all in all this seems like the easier route.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190205160848.24662-2-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
update_wm_post is meant for pre-g4x only. Don't ever set
it on g4x+.
The only effect of a bogus update_wm_post on g4x+ could
be that we clear the legacy_cursor_update flag in
intel_atomic_commit(). Since legacy_cursor_update is
only set for legacy cursor updates (as the name suggests)
and we only set update_wm_post for a modeset the two
cases should never occur at the same time. But let's
be consistent in setting update_wm_post so we don't
end up confusing so many people.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190206185433.8116-1-ville.syrjala@linux.intel.com
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Apply backpressure to hogs that emit requests faster than the GPU can
process them by waiting for their ring to be less than half-full before
proceeding with taking the struct_mutex.
This is a gross hack to apply throttling backpressure, the long term
goal is to remove the struct_mutex contention so that each client
naturally waits, preferably in an asynchronous, nonblocking fashion
(pipelined operations for the win), for their own resources and never
blocks another client within the driver at least. (Realtime priority
goals would extend to ensuring that resource contention favours high
priority clients as well.)
This patch only limits excessive request production and does not attempt
to throttle clients that block waiting for eviction (either global GTT or
system memory) or any other global resources, see above for the long term
goal.
No microbenchmarks are harmed (to the best of my knowledge).
Testcase: igt/gem_exec_schedule/pi-ringfull-*
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190207071829.5574-1-chris@chris-wilson.co.uk
Add err goto label and use it when VMA can't be established or changes
underneath.
v2:
- Dropping Fixes: as it's indeed impossible to race an object to the
error address. (Chris)
v3:
- Use IS_ERR_VALUE (Chris)
Reported-by: Adam Zabrocki <adamza@microsoft.com>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Adam Zabrocki <adamza@microsoft.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> #v2
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190207085454.10598-2-joonas.lahtinen@linux.intel.com
Make sure the underlying VMA in the process address space is the
same as it was during vm_mmap to avoid applying WC to wrong VMA.
A more long-term solution would be to have vm_mmap_locked variant
in linux/mmap.h for when caller wants to hold mmap_sem for an
extended duration.
v2:
- Refactor the compare function
Fixes: 1816f92363 ("drm/i915: Support creation of unbound wc user mappings for objects")
Reported-by: Adam Zabrocki <adamza@microsoft.com>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: <stable@vger.kernel.org> # v4.0+
Cc: Akash Goel <akash.goel@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Adam Zabrocki <adamza@microsoft.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> #v1
Link: https://patchwork.freedesktop.org/patch/msgid/20190207085454.10598-1-joonas.lahtinen@linux.intel.com
This hotplug also isn't needed: drm_dp_mst_topology_mgr_set_mst()
already sends a hotplug on its own from drm_dp_destroy_connector_work()
after destroying connectors in the MST topology.
Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190129191001.442-4-lyude@redhat.com
We have a bad habit of calling drm_fb_helper_hotplug_event() far more
then we actually need to. MST appears to be one of these cases, where we
call drm_fb_helper_hotplug_event() if we fail to resume a connected MST
topology in intel_dp_mst_resume(). We don't actually need to do this at
all though since hotplug events are already sent from
drm_dp_connector_destroy_work() every time connectors are unregistered
from userspace's PoV. Additionally, extra calls to
drm_fb_helper_hotplug_event() also just mean more of a chance of doing a
connector probe somewhere we shouldn't.
So, don't send any hotplug events during resume if the MST topology
fails to come up. Just rely on the DP MST helpers to send them for us.
Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190129191001.442-3-lyude@redhat.com
When resuming, we check whether or not any previously connected
MST topologies are still present and if so, attempt to resume them. If
this fails, we disable said MST topologies and fire off a hotplug event
so that userspace knows to reprobe.
However, sending a hotplug event involves calling
drm_fb_helper_hotplug_event(), which in turn results in fbcon doing a
connector reprobe in the caller's thread - something we can't do at the
point in which i915 calls drm_dp_mst_topology_mgr_resume() since
hotplugging hasn't been fully initialized yet.
This currently causes some rather subtle but fatal issues. For example,
on my T480s the laptop dock connected to it usually disappears during a
suspend cycle, and comes back up a short while after the system has been
resumed. This guarantees pretty much every suspend and resume cycle,
drm_dp_mst_topology_mgr_set_mst(mgr, false); will be caused and in turn,
a connector hotplug will occur. Now it's Rute Goldberg time: when the
connector hotplug occurs, i915 reprobes /all/ of the connectors,
including eDP. However, eDP probing requires that we power on the panel
VDD which in turn, grabs a wakeref to the appropriate power domain on
the GPU (on my T480s, this is the PORT_DDI_A_IO domain). This is where
things start breaking, since this all happens before
intel_power_domains_enable() is called we end up leaking the wakeref
that was acquired and never releasing it later. Come next suspend/resume
cycle, this causes us to fail to shut down the GPU properly, which
causes it not to resume properly and die a horrible complicated death.
(as a note: this only happens when there's both an eDP panel and MST
topology connected which is removed mid-suspend. One or the other seems
to always be OK).
We could try to fix the VDD wakeref leak, but this doesn't seem like
it's worth it at all since we aren't able to handle hotplug detection
while resuming anyway. So, let's go with a more robust solution inspired
by nouveau: block fbdev from handling hotplug events until we resume
fbdev. This allows us to still send sysfs hotplug events to be handled
later by user space while we're resuming, while also preventing us from
actually processing any hotplug events we receive until it's safe.
This fixes the wakeref leak observed on the T480s and as such, also
fixes suspend/resume with MST topologies connected on this machine.
Changes since v2:
* Don't call drm_fb_helper_hotplug_event() under lock, do it after lock
(Chris Wilson)
* Don't call drm_fb_helper_hotplug_event() in
intel_fbdev_output_poll_changed() under lock (Chris Wilson)
* Always set ifbdev->hpd_waiting (Chris Wilson)
Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 0e32b39cee ("drm/i915: add DP 1.2 MST support (v0.7)")
Cc: Todd Previte <tprevite@gmail.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v3.17+
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190129191001.442-2-lyude@redhat.com
On icl the plane watermark blocks field is 11 bits. Bump our define to
match so that readout won't ignore the extra bit. We can safely do this
for older platforms too since the unused bits are hardwired to zero.
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190205205056.30081-1-ville.syrjala@linux.intel.com
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Enable count array is supposed to have one counter for each possible
engine sampler. As such, array sizing and bounds checking is not correct
and would blow up the asserts if more samplers were added.
No ill-effect in the current code base but lets fix it for correctness.
At the same time tidy the assert for readability and robustness.
v2:
* One check per assert. (Chris Wilson)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: b46a33e271 ("drm/i915/pmu: Expose a PMU interface for perf queries")
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190205130353.21105-1-tvrtko.ursulin@linux.intel.com
Disabling WM1+ on ICL causes tons of underruns with
linear/X-tiled framebuffers. We can avoid this by flipping
on a chicken bit affecting the way the hw fill the FIFO.
This may not be the final solution but should hopefully
avoid some underruns in the meantime.
v2: Apparently PIPE_CHICKEN is icl+ only
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190204202232.27153-1-ville.syrjala@linux.intel.com
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
When adding the early latency==0 check back I neglected to
realize that we no longer have a way to return a failure
from the wm computation like we had in the past (since we
now calculate wms before ddb allocations). Also plane_en
being false doesn't actually indicate that the level is
invalid as it wil also happen when the plane is not
enabled.
skl_allocate_pipe_ddb() starts scanning from the maximum
watermark level and it stops as soon as it finds a level
that is deemed viable. The assumption being that if level
n+1 is valid then level n is valid as well. Thus if we
now disable any watermark level by zeroing its latency
the code will think that level to be actually valid
and won't confirm whether the actually enabled lower
watermark level(s) actually fit into the allotted ddb
space. This results in hilarious watermark values that
exceed the ddb allocation of the plane.
The way we must now indicate a failure is to assign an
unreasoanbly big value to min_ddb_alloc which will then
make skl_allocate_pipe_ddb() reject the entire level.
v2: Also do the same for the lines>31 case (Matt)
v3: Make 'blocks' u32 (Matt)
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190205155053.10081-1-ville.syrjala@linux.intel.com
clear_intel_crtc_state() uses the stack for saving a temporary copy of
certain bits of the inherited crtc_state before clearing the unwanted
bits. This pushes it over the stack limit for my little 32b Pineview,
so move the temporary allocation to the heap instead. As we now use a
zeroed struct, we can copy the whole extended state back to both
preserve what bits need to be preserved and zero the rest.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190205092759.16018-1-chris@chris-wilson.co.uk
We generally omit register polling from the i915_reg_rw tracepoint.
Understandable since polling could generate a lot of noise in the
trace. The downside is that the trace is incomplete. As a compromise
let's trace the final register value observed while polling. That
should be generally sufficient to observe what the code should be
doing next.
I suppose in some cases it might make sense to also trace the initial
register value, and maybe the number of times we polled. But that
would require a separate tracepoint so let's leave it for the future.
The other users of _NOTRACE() are i915_pmu and i2c bitbanging,
which I decided to leave alone.
Next we should do something to claw back the tracepoints for
planes and whatnot which were switched to _FW() a while back.
I guess just new macros for raw_rw+trace. The question is
what to call it?
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190204211644.21967-1-ville.syrjala@linux.intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
When calling debugfs functions, they can now return error values if
something went wrong. If that happens, return a NULL as a *dentry to
the relay core instead of passing it an illegal pointer.
The relay core should be able to handle an illegal pointer, but add this
check to be safe.
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190131131507.GA19807@kroah.com
First of all GMCH can be considered a feature by itself
since it is a chip present in some platforms that connects
the IA processor to memory and other components in PC.
Also with the introduction of display block at device info,
we got a redundant definition:
.display.has_gmch_display = 1,
So, let's clean up things a bit and use the standardized
way of has_feature on displays side.
No functional change and no manual interaction to generate
this patch.
It is only:
sed -si -e 's/has_gmch_display/has_gmch/g' \
-e 's/HAS_GMCH_DISPLAY/HAS_GMCH/g' drivers/gpu/drm/i915/*{c,h}
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190204222538.15842-1-rodrigo.vivi@intel.com
Looking forward, we need to break the struct_mutex dependency on
i915_gem_active. In the meantime, external use of i915_gem_active is
quite beguiling, little do new users suspect that it implies a barrier
as each request it tracks must be ordered wrt the previous one. As one
of many, it can be used to track activity across multiple timelines, a
shared fence, which fits our unordered request submission much better. We
need to steer external users away from the singular, exclusive fence
imposed by i915_gem_active to i915_active instead. As part of that
process, we move i915_gem_active out of i915_request.c into
i915_active.c to start separating the two concepts, and rename it to
i915_active_request (both to tie it to the concept of tracking just one
request, and to give it a longer, less appealing name).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190205130005.2807-5-chris@chris-wilson.co.uk
Wrap the active tracking for a GPU references in a slabcache for faster
allocations, and hopefully better fragmentation reduction.
v3: Nothing device specific left, it's just a slabcache that we can
make global.
v4: Include i915_active.h and don't put the initfunc under DEBUG_GEM
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190205130005.2807-4-chris@chris-wilson.co.uk
As soon as we detect that the active tracker is idle and we prepare to
call the retire callback, release the storage for our tree of
per-timeline nodes. We expect these to be infrequently used and quick
to allocate, so there is little benefit in keeping the tree cached and
we would prefer to return the pages back to the system in a timely
fashion.
This also means that when we finalize the struct as a whole, we know as
the activity tracker must be idle, the tree has already been released.
Indeed we can reduce i915_active_fini() just to the assertions that there
is nothing to do.
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: https://patchwork.freedesktop.org/patch/msgid/20190205130005.2807-3-chris@chris-wilson.co.uk
We currently track GPU memory usage inside VMA, such that we never
release memory used by the GPU until after it has finished accessing it.
However, we may want to track other resources aside from VMA, or we may
want to split a VMA into multiple independent regions and track each
separately. For this purpose, generalise our request tracking (akin to
struct reservation_object) so that we can embed it into other objects.
v2: Tweak error handling during selftest setup.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190205130005.2807-2-chris@chris-wilson.co.uk
Jani Nikula