With gtt remapping in place we can use arbitrarily large
framebuffers. Let's bump the limits to 16kx16k on gen7+.
The limit was chosen to match the maximum 2D surface size
of the 3D engine.
With the remapping we could easily go higher than that for the
display engine. However the modesetting ddx will blindly assume
it can handle whatever is reported via kms. The oversized
buffer dimensions are not caught by glamor nor Mesa until
finally an assert will trip when genxml attempts to pack the
SURFACE_STATE. So we pick a safe limit to avoid the X server
from crashing (or potentially misbehaving if the genxml asserts
are compiled out).
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110187
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509122159.24376-9-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
With gtt remapping plugged in we can simply raise the stride
limit on gen4+. Let's just pick the limit to match the render
engine max stride (256KiB on gen7+, 128KiB on gen4+).
No remapping CCS because the virtual address of each page actually
matters due to the new hash mode
(WaCompressedResourceDisplayNewHashMode:skl,kbl etc.), and no
remapping on gen2/3 due extra complications from fence alignment
and gen2 2KiB GTT tile size. Also no real benefit since the
display engine limits already match the other limits.
v2: Rebase due to is_ccs_modifier()
v3: Tweak the comment and commit msg
v4: Fix gen4+ stride limit to be 128KiB
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> #v3
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509122159.24376-8-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Align dumb buffer stride to 4k if the fb will be big enough to
require gtt remapping.
v2: Leave the stride alone for buffers that look to be for the cursor
v3: Make it not a hack (Daniel)
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509122159.24376-7-ville.syrjala@linux.intel.com
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
The display engine stride limits are getting in our way. On SKL+
we are limited to 8k pixels, which is easily exceeded with three
4k displays. To overcome this limitation we can remap the pages
in the GTT to provide the display engine with a view of memory
with a smaller stride.
The code is mostly already there as We already play tricks with
the plane surface address and x/y offsets.
A few caveats apply:
* linear buffers need the fb stride to be page aligned, as
otherwise the remapped lines wouldn't start at the same
spot
* compressed buffers can't be remapped due to the new
ccs hash mode causing the virtual address of the pages
to affect the interpretation of the compressed data. IIRC
the old hash was limited to the low 12 bits so if we were
using that mode we could remap. As it stands we just refuse
to remapp with compressed fbs.
* no remapping gen2/3 as we'd need a fence for the remapped
vma, which we currently don't have. Need to deal with the
fence POT requirements, and do something about the gen2
gtt page size vs tile size difference
v2: Rebase due to is_ccs_modifier()
Fix up the skl+ stride_mult mess
memset() the gtt_view because otherwise we could leave
junk in plane[1] when going from 2 plane to 1 plane format
v3: intel_check_plane_stride() was split out
v4: Drop the aligned viewport stuff, it was meant for ccs which
can't be remapped anyway
v5: Introduce intel_plane_can_remap()
Reorder the code so that plane_state->view gets filled
even for invisible planes, otherwise we'd keep using
stale values and could explode during remapping. The new
logic never remaps invisible planes since we don't have
a viewport, and instead pins the full fb instead
v6: Fix plane src coord checks after remapping by moving
plane_state->base.src to the final plane x/y offsets.
Allow intel_plane_check_stride() to fail even with
remapping (can happen at least with a linear 64bpp
fb with a 4k plane and a suitably inconvenient src
coordinates).
Improve aux plane FIXME (Daniel)
Move some code shuffling into a separate patch (Daniel)
Testcase: igt/kms_big_fb
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509122159.24376-6-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reorganize some fb stride checking code a bit to prepare for
gtt remapping. And do a bit of s/pitch/stride/ renaming in the
process for a bit more uniformity (apart from the whole
fb->pitches[] thing).
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509122159.24376-5-ville.syrjala@linux.intel.com
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Add a live selftest to excercise rotated/remapped vmas. We simply
write through the rotated/remapped vma, and confirm that the data
appears in the right page when read through the normal vma.
Not sure what the fallout of making all rotated/remapped vmas
mappable/fenceable would be, hence I just hacked it in the test.
v2: Grab rpm reference (Chris)
GEM_BUG_ON(view.type not as expected) (Chris)
Allow CAN_FENCE for rotated/remapped vmas (Chris)
Update intel_plane_uses_fence() to ask for a fence
only for normal vmas on gen4+
v3: Deal with intel_wakeref_t
v4: Rebase
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509122159.24376-4-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Extend the rotated vma mock selftest to cover remapped vmas as
well.
TODO: reindent the loops I guess? Left like this for now to
ease review
v2: Include the vma type in the error message (Chris)
v3: Deal with trimmed sg
v4: Drop leftover debugs
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509122159.24376-3-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To overcome display engine stride limits we'll want to remap the
pages in the GTT. To that end we need a new gtt_view type which
is just like the "rotated" type except not rotated.
v2: Use intel_remapped_plane_info base type
s/unused/unused_mbz/ (Chris)
Separate BUILD_BUG_ON()s (Chris)
Use I915_GTT_PAGE_SIZE (Chris)
v3: Use i915_gem_object_get_dma_address() (Chris)
Trim the sg (Tvrtko)
v4: Actually trim this time. Limit the max length
to one row of pages to keep things simple
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509122159.24376-2-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
With the disappearance of NEWCLIENT, we no longer need to provide the
priority boost on preemption in order to prevent repeated gazumping,
and we can remove the dead code.
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/20190515130052.4475-5-chris@chris-wilson.co.uk
Commit 1413b2bc07 ("drm/i915: Trim NEWCLIENT boosting") had the
intended consequence of not allowing a sequence of work that merely
crossed into a new engine the privilege to be promoted to NEWCLIENT
status. It also had the unintended consequence of actually making
NEWCLIENT effective on heavily oversubscribed transcode machines and
impacting upon their throughput.
If we consider a client packet composed of (rcsA, rcsB, vcs) and 30 of
those clients, using the NEWCLIENT boost that will be scheduled as
rcsA x 30, (rcsB, vcs) x 30
where as before it would have been
(rcsA, rcsB, vcs) x 30
That is with NEWCLIENT only boosting the first request of each client,
we would execute all rcsA requests prior to running on the vcs engines;
acruing a lot of dead time as compared to the previous case where the
vcs engine would be started in parallel to processing the second client.
The previous patch has the effect of delaying submission until it is
required by a third party (either the user with an explicit wait, or by
another client/engine). We reduce the NEWCLIENT bump to a mere WAIT,
which has the effect of removing its preemptive grant and reducing it to
the same level as any other user interaction -- that it will not be
promoted above the interengine dependencies, and so preventing NEWCLIENTS
from starving other engines. This a large nerf to the rrul properties of
the current NEWCLIENT, but it still does give prioritised submission to
new requests from light workloads.
References: b16c765122 ("drm/i915: Priority boost for new clients")
Fixes: 1413b2bc07 ("drm/i915: Trim NEWCLIENT boosting") # customer impact
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
Cc: Dmitry Ermilov <dmitry.ermilov@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190515130052.4475-4-chris@chris-wilson.co.uk
The handling of the no-preemption priority level imposes the restriction
that we need to maintain the implied ordering even though preemption is
disabled. Otherwise we may end up with an AB-BA deadlock across multiple
engine due to a real preemption event reordering the no-preemption
WAITs. To resolve this issue we currently promote all requests to WAIT
on unsubmission, however this interferes with the timeslicing
requirement that we do not apply any implicit promotion that will defeat
the round-robin timeslice list. (If we automatically promote the active
request it will go back to the head of the queue and not the tail!)
So we need implicit promotion to prevent reordering around semaphores
where we are not allowed to preempt, and we must avoid implicit
promotion on unsubmission. So instead of at unsubmit, if we apply that
implicit promotion on adding the dependency, we avoid the semaphore
deadlock and we also reduce the gains made by the promotion for user
space waiting. Furthermore, by keeping the earlier dependencies at a
higher level, we reduce the search space for timeslicing without
altering runtime scheduling too badly (no dependencies at all will be
assigned a higher priority for rrul).
v2: Limit the bump to external edges (as originally intended) i.e.
between contexts and out to the user.
Testcase: igt/gem_concurrent_blit
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/20190515130052.4475-3-chris@chris-wilson.co.uk
Smatch spotted:
drivers/gpu/drm/i915//intel_hdcp.c:1406 hdcp2_authenticate_repeater_topology() warn: should this be a bitwise op?
and indeed looks to be suspect that we do need to use a bitwise or to
combine the two register fields into one counter.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190517102225.3069-3-chris@chris-wilson.co.uk
Just to squelch an smatch warning that doesn't see the with_() being
taken unconditionally:
drivers/gpu/drm/i915//intel_dp.c:230 intel_dp_get_fia_supported_lane_count() error: uninitialized symbol 'lane_info'.
drivers/gpu/drm/i915//intel_dp.c:5338 intel_digital_port_connected() error: uninitialized symbol 'is_connected'.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190517102225.3069-2-chris@chris-wilson.co.uk
In commit b7404c7ecb ("drm/i915: Bump ready tasks ahead of
busywaits"), I tried cutting a corner in order to not install a signal
for each of our dependencies, and only listened to requests on which we
were intending to busywait. The compromise that was made was that
instead of then being able to promote the request with a full
NOSEMAPHORE like its non-busywaiting brethren, as we had not ensured we
had cleared the semaphore chain, we settled for only using the NEWCLIENT
boost. With an over saturated system with multiple NEWCLIENTS in flight
at any time, this was found to be an inadequate promotion and left us
with a much poorer scheduling order than prior to using semaphores.
The outcome of this patch, is that all requests have NOSEMAPHORE
priority when they have no dependencies and are ready to run and not
busywait, restoring the pre-semaphore ordering on saturated systems.
We can demonstrate the effect of poor scheduling order by oversaturating
the system using gem_wsim on a system with multiple vcs engines
(i.e running the same workloads across more clients than required for
peak throughput, e.g. media_load_balance_17i7.wsim -c4 -b context):
x v5.1 (normalized)
+ tip
* fix
+------------------------------------------------------------------------+
| x |
| x |
| x |
| x |
| %x |
| %%x |
| %%x |
| %%x |
| %%x |
| %%x |
| %%x |
| %%x |
| %%x |
| %%x |
| %%x |
| %#x |
| %#x |
| %#x |
| %#x |
| %#x |
| + %#xx |
| + %#xx |
| + %%#xx |
| + %%#xx |
| + %%#xx |
| + %%#xx |
| + %%##x |
| +++ %%##x |
| +++ %%##x |
| +++ %%##x |
| ++++ %%##x |
| ++++ %%##x |
| ++++ %%##xx |
| ++++ %###xx |
| ++++ %###xx |
| ++++ %###xx |
| ++++ %###xx |
| ++++ + %#O#xx |
| ++++ + %#O#xx |
| ++++++ + %#O#xx |
| ++++++++++ %OOOxxx|
| ++++++++++ + %#OOO#xx|
| + ++++++++++++ ++ +++++ + ++ @@OOOO#xx|
| |A_| |
||__________M_______A____________________| |
| |A_| |
+------------------------------------------------------------------------+
N Min Max Median Avg Stddev
x 120 0.99456 1.00628 0.999985 1.0001545 0.0024387139
+ 120 0.873021 1.00037 0.884134 0.90148752 0.039190862
Difference at 99.5% confidence
-0.098667 +/- 0.0110762
-9.86517% +/- 1.10745%
(Student's t, pooled s = 0.0277657)
% 120 0.990207 1.00165 0.9970265 0.99699748 0.0021024
Difference at 99.5% confidence
-0.003157 +/- 0.000908245
-0.315651% +/- 0.0908105%
(Student's t, pooled s = 0.00227678)
Fixes: b7404c7ecb ("drm/i915: Bump ready tasks ahead of busywaits")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
Cc: Dmitry Ermilov <dmitry.ermilov@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190515130052.4475-2-chris@chris-wilson.co.uk
Avoid charging us for the presumed busywait if the request was preempted
after successfully using semaphores to reduce inter-engine latency.
v2: Bump the priority to reflect the lack of semaphores now required.
References: ca6e56f654 ("drm/i915: Disable semaphore busywaits on saturated systems")
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/20190515130052.4475-1-chris@chris-wilson.co.uk
Add an assert that we don't use TypeC ports for eDP. That may in theory
be possible on TypeC legacy ports, but I'm not sure if that's a
practical scenario, so let's deal with that only if there's a use case.
Adding support for that wouldn't be too difficult, since TypeC mode
switching is not possible on TypeC legacy ports.
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-12-imre.deak@intel.com
On ICL we have to make sure that we enable the AUX power domain in a
controlled way (corresponding to the port's actual TypeC mode). Since
the PPS lock - which takes an AUX power ref - is only needed on
eDP on all platforms and eDP/DP on VLV/CHV avoid taking it in all
other cases.
v2:
- Clarify commit log about the condition for taking the PPS lock.
(Ville)
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-11-imre.deak@intel.com
There isn't a separate power domain specific to PLLs. When programming
them we require the same power domain to be enabled which is needed when
accessing other display core parts (not specific to any
pipe/port/transcoder). This corresponds to the DISPLAY_CORE domain added
previously in this patchset, so use that instead to save bits in the
power domain mask.
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-10-imre.deak@intel.com
The power get/put was added in
commit 1c767b339b ("drm/i915: take display port power domain in DP HPD handler")
Author: Imre Deak <imre.deak@intel.com>
Date: Mon Aug 18 14:42:42 2014 +0300
to account for the HW access in ibx_digital_port_connected(). This
latter call was in turn removed in
commit 7d23e3c37b ("drm/i915: Cleaning up intel_dp_hpd_pulse")
Author: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com>
Date: Wed Mar 30 18:05:23 2016 +0530
after which we didn't actually need the power reference.
One way we are accessing the HW during HPD pulse handling is via DP AUX
transfers, but the transfer function takes its own reference, so doesn't
need the reference in intel_dp_hpd_pulse().
The other spot is in
intel_psr_short_pulse()->intel_psr_disable_locked()
but that can only happen when the panel is enabled with the
corresponding modeset already holding the required power reference.
v2:
- Remove the unneeded power get/put from intel_psr_disable_locked().
(Ville)
- Checkpatch commit quoting format fix in the commit log.
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-9-imre.deak@intel.com
We don't need the AUX power for the whole duration of the detect, only
when we're doing AUX transfers. The AUX transfer function takes its own
reference on the AUX power domain already. The two places during detect
which access display core registers (not specific to a
pipe/port/transcoder) only need the power domain that is required for
that access. That power domain is equivalent to the device global power
domain on most platforms (enabled whenever we hold a runtime PM
reference) except on CHV/VLV where it's equivalent to the display power
well.
Add a new power domain that reflects the above, and use this at the two
spots accessing registers. With that we can avoid taking the AUX
reference for the whole duration of the detect function.
Put the domains asynchronously to avoid the unneeded on-off-on toggling.
Also adapt the idea from with_intel_runtime_pm et al. for making it easy
to write short sequences where a display power ref is needed.
v2: (Ville)
- Add with_intel_display_power() helper to simplify things.
- s/bool res/bool is_connected/
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-8-imre.deak@intel.com
In a follow-up patch we will restrict holding the reference on the AUX
power domain to the AUX transfer function. To avoid the unnecessary
on-off-on power togglings drop the reference asynchronously.
There is no reason we couldn't do this in general and also put the
reference asynchronously in pps_unlock(); but that's a separate change
that can be done as a follow-up.
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-6-imre.deak@intel.com
By disabling a power domain asynchronously we can restrict holding a
reference on that power domain to the actual code sequence that
requires the power to be on for the HW access it's doing, by also
avoiding unneeded on-off-on togglings of the power domain (since the
disabling happens with a delay).
One benefit is potential power saving due to the following two reasons:
1. The fact that we will now be holding the reference only for the
necessary duration by the end of the patchset. While simply not
delaying the disabling has the same benefit, it has the problem that
frequent on-off-on power switching has its own power cost (see the 2.
point below) and the debug trace for power well on/off events will
cause a lot of dmesg spam (see details about this further below).
2. Avoiding the power cost of freuqent on-off-on power switching. This
requires us to find the optimal disabling delay based on the measured
power cost of on->off and off->on switching of each power well vs.
the power of keeping the given power well on.
In this patchset I'm not providing this optimal delay for two
reasons:
a) I don't have the means yet to perform the measurement (with high
enough signal-to-noise ratio, or with the help of an energy
counter that takes switching into account). I'm currently looking
for a way to measure this.
b) Before reducing the disabling delay we need an alternative way for
debug tracing powerwell on/off events. Simply avoiding/throttling
the debug messages is not a solution, see further below.
Note that even in the case where we can't measure any considerable
power cost of frequent on-off switching of powerwells, it still would
make sense to do the disabling asynchronously (with 0 delay) to avoid
blocking on the disabling. On VLV I measured this disabling time
overhead to be 1ms on average with a worst case of 4ms.
In the case of the AUX power domains on ICL we would also need to keep
the sequence where we hold the power reference short, the way it would
be by the end of this patchset where we hold it only for the actual AUX
transfer. Anything else would make the locking we need for ICL TypeC
ports (whenever we hold a reference on any AUX power domain) rather
problematic, adding for instance unnecessary lockdep dependencies to
the required TypeC port lock.
I chose the disabling delay to be 100msec for now to avoid the unneeded
toggling (and so not to introduce dmesg spamming) in the DP MST sideband
signaling code. We could optimize this delay later, once we have the
means to measure the switching power cost (see above).
Note that simply removing/throttling the debug tracing for power well
on/off events is not a solution. We need to know the exact spots of
these events and cannot rely only on incorrect register accesses caught
(due to not holding a wakeref at the time of access). Incorrect
powerwell enabling/disabling could lead to other problems, for instance
we need to keep certain powerwells enabled for the duration of modesets
and AUX transfers.
v2:
- Clarify the commit log parts about power cost measurement and the
problem of simply removing/throttling debug tracing. (Chris)
- Optimize out local wakeref vars at intel_runtime_pm_put_raw() and
intel_display_power_put_async() call sites if
CONFIG_DRM_I915_DEBUG_RUNTIME_PM=n. (Chris)
- Rebased on v2 of the wakeref w/o power-on guarantee patch.
- Add missing docbook headers.
v3:
- Checkpatch spelling/missing-empty-line fix.
v4:
- Fix unintended local wakeref var optimization when using
call-arguments with side-effects, by using inline funcs instead of
macros. In this patch in particular this will fix the
intel_display_power_grab_async_put_ref()->intel_runtime_pm_put_raw()
call).
No size change in practice (would be the same disregarding the
corresponding change in intel_display_power_grab_async_put_ref()):
$ size i915-macro.ko
text data bss dec hex filename
2455190 105890 10272 2571352 273c58 i915-macro.ko
$ size i915-inline.ko
text data bss dec hex filename
2455195 105890 10272 2571357 273c5d i915-inline.ko
Kudos to Stan for reporting the raw-wakeref WARNs this issue caused. His
config has CONFIG_DRM_I915_DEBUG_RUNTIME_PM=n, which I didn't retest
after v1, and we are also not testing this config in CI.
Now tested both with CONFIG_DRM_I915_DEBUG_RUNTIME_PM=y/n on ICL,
connecting both Chamelium and regular DP, HDMI sinks.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190513192533.12586-1-imre.deak@intel.com
There is no reason why we couldn't verify the power domains state during
suspend in all cases, so do that. I overlooked this when originally
adding the check.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-4-imre.deak@intel.com
Make sure we print and drop the wakeref tracking info during pm_cleanup
even if there are wakeref holders (either raw-wakeref or wakelock
holders). Dropping the wakeref tracking means that a late put on the ref
will WARN since the wakeref will be unknown, but that is rightly so,
since the put is late and we want to catch that case.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-3-imre.deak@intel.com
It's useful to track runtime PM refs that don't guarantee a device
power-on state to the rest of the driver. One such case is holding a
reference that will be put asynchronously, during which normal users
without their own reference shouldn't access the HW. A follow-up patch
will add support for disabling display power domains asynchronously
which needs this.
For this we can split wakeref_count into a low half-word tracking
all references (raw-wakerefs) and a high half-word tracking
references guaranteeing a power-on state (wakelocks).
Follow-up patches will make use of the API added here.
While at it add the missing docbook header for the unchecked
display-power and runtime_pm put functions.
No functional changes, except for printing leaked raw-wakerefs
and wakelocks separately in intel_runtime_pm_cleanup().
v2:
- Track raw wakerefs/wakelocks in the low/high half-word of
wakeref_count, instead of adding a new counter. (Chris)
v3:
- Add a struct_member(T, m) helper instead of open-coding it. (Chris)
- Checkpatch indentation formatting fix.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-2-imre.deak@intel.com
Add another ICL-Y PCIID that proved to have only 5 ports to the
corresponding PCIID list.
Meanwhile I'm trying to get a complete list of all PCIIDs with less than
6 ports and/or get a VBT fix to mark these ports non-existent, but until
then the only way is to go one-by-one.
This fixes the following error on machines with less than 6 port:
[drm:intel_power_well_enable [i915]] enabling AUX F
------------[ cut here ]------------
WARN_ON(intel_wait_for_register(&dev_priv->uncore, regs->driver, (0x1 << ((pw_idx) * 2)), (0x1 << ((pw_idx) * 2)), 1))
(Internal reference: BSpec/Index/20584/Issues, HSD/1306084116)
Cc: Mika Kahola <mika.kahola@intel.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=108915
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Mika Kahola <mika.kahola@intel.com>
Tested-by: Mika Kahola <mika.kahola@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190510140255.25215-1-imre.deak@intel.com
Adjust the get transcoder timings for mipi dsi as per the
set timing calculations.
v2: Use the existing intel_get_pipe_timings and do the dsi
specific adjustments in the encoder get_config hook.(Ville, Jani)
v3: Exclude VBLANK and HBLANK registers for dsi transcoder.
v4: Fix the incomplete conditional logic.
Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1556809862-31203-1-git-send-email-vandita.kulkarni@intel.com
Below Sparsh warnings are fixed.
Commit: drm: revocation check at drm subsystem
+drivers/gpu/drm/drm_hdcp.c:235:6: warning: symbol
'drm_hdcp_request_srm' was not declared. Should it be static?
+drivers/gpu/drm/drm_hdcp.c:27:3: warning: symbol 'srm_data' was not
declared. Should it be static?
+drivers/gpu/drm/drm_hdcp.c:317:5: warning: symbol 'drm_setup_hdcp_srm'
was not declared. Should it be static?
+drivers/gpu/drm/drm_hdcp.c:327:6: warning: symbol
'drm_teardown_hdcp_srm' was not declared. Should it be static?
cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20190513133504.18612-1-ramalingam.c@intel.com
In all likelihood, the priority and node are already in the CPU cache
and by checking them first, we can avoid having to chase the
*request->hwsp for the current breadcrumb.
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/20190513120102.29660-3-chris@chris-wilson.co.uk
To simplify the next patch, update bump_priority and schedule to accept
the internal i915_sched_ndoe directly and not expect a request pointer.
add/remove: 0/0 grow/shrink: 2/1 up/down: 8/-15 (-7)
Function old new delta
i915_schedule_bump_priority 109 113 +4
i915_schedule 50 54 +4
__i915_schedule 922 907 -15
v2: Adopt node for the old rq local, since it no longer is a request but
the origin node.
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/20190513120102.29660-2-chris@chris-wilson.co.uk
Convert the HSW pch_pfit.force_thru to a proper state variable
with readout and accompanying pipe conf check. Makes the logic
a bit more straightforward, and hopefully prevents some
breakage in the future.
'force_thru' is probably not the best name for this, but I
didn't manage to come up with anything better either, so I
left it alone.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190425162906.5242-2-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
On HSW the pipe A panel fitter lives inside the display power well,
and the input MUX for the EDP transcoder needs to be configured
appropriately to route the data through the power well as needed.
Changing the MUX setting is not allowed while the pipe is active,
so we need to force a full modeset whenever we need to change it.
Currently we may end up doing a fastset which won't change the
MUX settings, but it will drop the power well reference, and that
kills the pipe.
Cc: stable@vger.kernel.org
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: d19f958db2 ("drm/i915: Enable fastset for non-boot modesets.")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190425162906.5242-1-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
On many (all?) the Gemini Lake systems we work with, there is frequent
momentary graphical corruption at the top of the screen, and it seems
that disabling framebuffer compression can avoid this.
The ticket was reported 6 months ago and has already affected a
multitude of users, without any real progress being made. So, lets
disable framebuffer compression on GeminiLake until a solution is found.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108085
Fixes: fd7d6c5c8f ("drm/i915: enable FBC on gen9+ too")
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: <stable@vger.kernel.org> # v4.11+
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Drake <drake@endlessm.com>
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190423092810.28359-1-jian-hong@endlessm.com
Considering the significant size of hdcp related code in drm, all
hdcp related codes are moved into separate file called drm_hdcp.c.
v2:
Rebased.
v2:
Rebased.
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20190507162745.25600-7-ramalingam.c@intel.com
DRM HDCP SRM revocation check services are used from I915 for HDCP1.4
and 2.2 revocation check during the respective authentication flow.
v2:
Rebased.
v3:
%s/*_ksvs_revocated/*_check_ksvs_revoked [Daniel]
unwanted noise is removed.
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20190507162745.25600-6-ramalingam.c@intel.com
On every hdcp revocation check request SRM is read from fw file
/lib/firmware/display_hdcp_srm.bin
SRM table is parsed and stored at drm_hdcp.c, with functions exported
for the services for revocation check from drivers (which
implements the HDCP authentication)
This patch handles the HDCP1.4 and 2.2 versions of SRM table.
v2:
moved the uAPI to request_firmware_direct() [Daniel]
v3:
kdoc added. [Daniel]
srm_header unified and bit field definitions are removed. [Daniel]
locking improved. [Daniel]
vrl length violation is fixed. [Daniel]
v4:
s/__swab16/be16_to_cpu [Daniel]
be24_to_cpu is done through a global func [Daniel]
Unused variables are removed. [Daniel]
unchecked return values are dropped from static funcs [Daniel]
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Acked-by: Satyeshwar Singh <satyeshwar.singh@intel.com>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20190507162745.25600-5-ramalingam.c@intel.com
Existing functions for converting a 3bytes(be24) of big endian value
into u32 of little endian and vice versa are renamed as
s/drm_hdcp2_seq_num_to_u32/drm_hdcp_be24_to_cpu
s/drm_hdcp2_u32_to_seq_num/drm_hdcp_cpu_to_be24
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Suggested-by: Daniel Vetter <daniel@ffwll.ch>
cc: Tomas Winkler <tomas.winkler@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20190507162745.25600-4-ramalingam.c@intel.com
Adding the HDCP2.2 capability of HDCP src and sink info into debugfs
entry "i915_hdcp_sink_capability"
This helps the userspace tests to skip the HDCP2.2 test on non HDCP2.2
sinks.
v2:
Rebased.
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20190507162745.25600-3-ramalingam.c@intel.com
Content protection property is created once and stored in
drm_mode_config. And attached to all HDCP capable connectors.
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20190507162745.25600-2-ramalingam.c@intel.com
Currently there is an underlying assumption that i915_request_unsubmit()
is synchronous wrt the GPU -- that is the request is no longer in flight
as we remove it. In the near future that may change, and this may upset
our signaling as we can process an interrupt for that request while it
is no longer in flight.
CPU0 CPU1
intel_engine_breadcrumbs_irq
(queue request completion)
i915_request_cancel_signaling
... ...
i915_request_enable_signaling
dma_fence_signal
Hence in the time it took us to drop the lock to signal the request, a
preemption event may have occurred and re-queued the request. In the
process, that request would have seen I915_FENCE_FLAG_SIGNAL clear and
so reused the rq->signal_link that was in use on CPU0, leading to bad
pointer chasing in intel_engine_breadcrumbs_irq.
A related issue was that if someone started listening for a signal on a
completed but no longer in-flight request, we missed the opportunity to
immediately signal that request.
Furthermore, as intel_contexts may be immediately released during
request retirement, in order to be entirely sure that
intel_engine_breadcrumbs_irq may no longer dereference the intel_context
(ce->signals and ce->signal_link), we must wait for irq spinlock.
In order to prevent the race, we use a bit in the fence.flags to signal
the transfer onto the signal list inside intel_engine_breadcrumbs_irq.
For simplicity, we use the DMA_FENCE_FLAG_SIGNALED_BIT as it then
quickly signals to any outside observer that the fence is indeed signaled.
v2: Sketch out potential dma-fence API for manual signaling
v3: And the test_and_set_bit()
Fixes: 52c0fdb25c ("drm/i915: Replace global breadcrumbs with per-context interrupt tracking")
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/20190508112452.18942-1-chris@chris-wilson.co.uk
After realising we need to sample RING_START to detect context switches
from preemption events that do not allow for the seqno to advance, we
can also realise that the seqno itself is just a distance along the ring
and so can be replaced by sampling RING_HEAD.
v2: Bonus comment for the mystery separate CS_STALL before MI_USER_INTERRUPT
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/20190508080704.24223-1-chris@chris-wilson.co.uk
If the HW fails to ack a change in forcewake status, the machine is as
good as dead -- it may recover, but in reality it missed the mmio
updates and is now in a very inconsistent state. If it happens, we can't
trust the CI results (or at least the fails may be genuine but due to
the HW being dead and not the actual test!) so reboot the machine (CI
checks for a kernel taint in between each test and reboots if the
machine is tainted).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190508115245.27790-1-chris@chris-wilson.co.uk
There is a bug in hdmi_deep_color_possible() - we compare pipe_bpp
<= 8*3 which returns true every time for hdmi_deep_color_possible 12 bit
deep color mode test in intel_hdmi_compute_config().(Even when the
requested color mode is 10 bit through max bpc property)
Comparing pipe_bpp with bpc * 3 takes care of this condition where
requested max bpc is 10 bit, so hdmi_deep_color_possible with 12 bit
returns false when requested max bpc is 10.(Ville)
v2:Add suggested by Ville Syrjälä
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Clinton Taylor <Clinton.A.Taylor@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190507181856.16091-1-aditya.swarup@intel.com
For us KBP is 100% identical to SPT. Kill the redundant enum
value. Also bspec doesn't talk about KBP either, so this might
avoid some confusion when cross checking the code against the
spec.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190506152627.20283-1-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
