On finishing the reset, the intention is to restart the GPU before we
relinquish the forcewake taken to handle the reset - the goal being the
GPU reloads a context before it is allowed to sleep. For this purpose,
we used tasklet_flush() which although it accomplished the goal of
restarting the GPU, carried with it a sting in its tail: it cleared the
TASKLET_STATE_SCHED bit. This meant that if another CPU queued a new
request to this engine, we would clear the flag and later attempt to
requeue the tasklet on the local CPU, breaking the per-cpu softirq
lists.
Remove the dangerous tasklet_kill() and just run the tasklet func
directly as we know it is safe to do so (the tasklets are internally
locked to allow mixed usage from direct submission).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180828152702.27536-1-chris@chris-wilson.co.uk
During power domains initialization we acquire power well references for
power wells in the INIT power domain. The rest of power wells - which
BIOS could have left enabled - we can only acquire references as needed
during display HW readout and so must defer sanitization until then
(also implying that we must always do HW readout to cleanup unused power
wells).
Thus during initialization these latter power wells can have a refcount
of 0 while still being enabled. To avoid the false-positive state
mismatch error this causes remove the check from
intel_power_domains_init_hw() and rely on the state check in
intel_power_domains_enable() which follows the HW readout.
v2:
- Add comment to log and code clarifying how unused power wells get
disabled. (Chris)
Fixes: 6dfc4a8f13 ("drm/i915: Verify power domains after enabling them")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
References: https://bugs.freedesktop.org/show_bug.cgi?id=107411
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/20180828122231.14336-1-imre.deak@intel.com
This re-applies the workaround for "some DP sinks, [which] are a
little nuts" from commit 1a36147bb9 ("drm/i915: Perform link
quality check unconditionally during long pulse").
It makes the secondary AOC E2460P monitor connected via DP to an
acer Veriton N4640G usable again.
This hunk was dropped in commit c85d200e83 ("drm/i915: Move SST
DP link retraining into the ->post_hotplug() hook")
Fixes: c85d200e83 ("drm/i915: Move SST DP link retraining into the ->post_hotplug() hook")
[Cleaned up commit message, added stable cc]
Signed-off-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
Cc: stable@vger.kernel.org
Link: https://patchwork.freedesktop.org/patch/msgid/20180825191035.3945-1-lyude@redhat.com
None of the current lookup_power_well() callers are actually checking
for NULL return values, they all just use the pointer right away. The
first idea was to replace these theoretical segfaults with a BUG()
since this would at least make our code a little more explicit to the
reader. It was suggested that just converting the BUG() to a WARN()
and returning any power well would probably be better since it would
still keep the system running while at the same time exposing the
driver bug.
We can only hit this NULL/BUG()/WARN() condition if we try to lookup a
power well that isn't defined on a given platform. If that ever
happens, we have to fix our code, making it lookup the correct power
well. Because of this, I don't think it's worth trying to implement
error checking in every caller. Improving our CI system will be a
better use of our time once a bug is found in the wild.
v2: Avoid the BUG() with a WARN() return a random PW (Michal).
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180820233139.11936-2-paulo.r.zanoni@intel.com
Unlike the other ports, TC ports are not available to use as soon as
we get a hotplug. The TC PHYs can be shared between multiple
controllers: display, USB, etc. As a result, handshaking through FIA
is required around connect and disconnect to cleanly transfer
ownership with the controller and set the type-C power state.
This patch implements the flow sequences described by our
specification. We opt to grab ownership of the ports as soon as we get
the hotplugs in order to simplify the interactions and avoid surprises
in the user space side. We may consider changing this in the future,
once we improve our testing capabilities on this area.
v2:
* This unifies the DP and HDMI patches so we can discuss everything
at once so people looking at random single patches can actually
understand the direction.
* I found out the spec was updated a while ago. There's a small
difference in the connect flow and the patch was updated for that.
* Our spec also now gives a good explanation on what is really
happening. As a result, comments were added.
* Add some more comments as requested by Rodrigo (Rodrigo).
v3:
* Downgrade a DRM_ERROR that shouldn't ever happen but we can't act
on in case it does (Chris).
BSpec: 21750, 4250.
Cc: Animesh Manna <animesh.manna@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180801173441.9789-1-paulo.r.zanoni@intel.com
PLLs are the source clocks for the DDIs so in order to determine the
ddi clock we need to check the PLL configuration.
For MG PHy Ports (C - F), depending on whether it is a TBT PLL or MG
PLL the link lock can be obtained from the the PLL divisors based on
the specification.
v2 (from Paulo):
* Make the algorithm look more like what's in the spec, also document
where we differ form the spec and why.
* Make the code a little more consistent with our coding style.
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180817215209.29133-2-paulo.r.zanoni@intel.com
The register value of Divider Ratio for high speed divider
(hsdiv_ratio) in MG_CLKTOP2_HSCLKCTL_PORT register is not same as the
actual numerical value of the divider. So this patch implements
separate divider value defines for that field.
icl_mg_pll_find_divisors() can use these defines instead of magic
register values.
The new defines are going to be used in the next patch.
v2 (from Paulo):
* Rebase.
* Make it look a little more like the rest of our code.
v3 (from Paulo):
* Make hsdiv u32 now that it's a bit field (José).
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Suggested-by: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180817215209.29133-1-paulo.r.zanoni@intel.com
After
commit 2cd9a689e9 ("drm/i915: Refactor intel_display_set_init_power() logic")
it makes more sense to check the power domain/well refcounts after
enabling the power domains functionality. Before that it's guaranteed
that most power wells (in the INIT domain) will have a reference held,
so not an interesting state.
While at it also add the check after the init_hw/fini_hw, disable and
suspend/resume steps. Make the test optional on a Kconfig option since
it may add substantial overhead: on VLV/CHV the corresponding PUNIT reg
access for each power well may take up to 20ms.
v2:
- Add the state check to more spots. (Chris)
v3:
- During suspend check the state before deiniting display core.
Afterwards DC states are disabled (and so the dc_off power well is
enabled) even though we don't hold a reference on it.
- Do the test conditionally based on a new Kconfig option. (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
[Add DRM_I915_DEBUG_RUNTIME_PM to welcome messages]
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180817145837.26592-1-imre.deak@intel.com
Instead of defining all registers twice, define just a PCH_GPIO_BASE
that has the same address as PCH_GPIO_A and use that to calculate all
the others. This also brings VLV and !HAS_GMCH_DISPLAY in line, doing
the same thing.
v2: Fix GMBUS registers to be relative to gpio base; create GPIO()
macro to return a particular gpio address and move the enum out of
i915_reg.h (suggested by Jani)
v3: Move base offset inside the GPIO() macro so the GMBUS defines don't
actually need to be changed (suggested by Daniel/Ville)
v4: Move definition of i915_gpio to intel_display.h and remove
GMBUS/GPIO handling from gvt since now they have their own
defines.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180727193647.8639-3-lucas.demarchi@intel.com
The device global init_power_on flag is somewhat arbitrary and makes
debugging power refcounting problems difficult. Instead arrange things
so that all display power domain get has a corresponding put call. After
this change we have the following sequences:
driver loading:
intel_power_domains_init_hw();
<other init steps>
intel_power_domains_enable();
driver unloading:
intel_power_domains_disable();
<other uninit steps>
intel_power_domains_fini_hw();
system suspend:
intel_power_domains_disable();
<other suspend steps>
intel_power_domains_suspend();
system resume:
intel_power_domains_resume();
<other resume steps>
intel_power_domains_enable();
at other times while the driver is loaded:
intel_display_power_get();
...
intel_display_power_put();
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
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/20180816123757.3286-2-imre.deak@intel.com
Currently, we cancel the extra wakeref we have for !runtime-pm devices
inside power_wells_fini_hw. However, this is not strictly paired with
the acquisition of that wakeref in runtime_pm_enable (as the fini_hw may
be called on errors paths before we even call runtime_pm_enable). Make
the symmetry more explicit and include a check that we do release all of
our rpm wakerefs.
v2: Fixup transfer of ownership back to core whilst keeping our wakeref
count balanced.
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>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180816123757.3286-1-imre.deak@intel.com
The case where the firmware isn't specified for a platform (although
runtime PM works only with DMC on this platform) is the same case where
the firmware is specified but can't be loaded for some reason. Hence we
need to get a display init power domain ref in the first case too to
keep the refcount bookkeeping in balance.
Also convert the related log message to be a debug one, since it's a
valid scenario for a new platform, where we need to have
dev_info->has_csr=1 set, but add support for actually loading the
firmware only later.
v2:
- In addition to the debug log, WARN on non-alpha support platforms,
since then the first case isn't valid scenario. (Chris)
References: https://bugs.freedesktop.org/show_bug.cgi?id=107382
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Anusha Srivatsa <anusha.srivatsa@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/20180815131038.24446-1-imre.deak@intel.com
Since Haswell we have no color range indication either in the pipe or
port registers for DP. Instead, there's a separate register for setting
the DP Main Stream Attributes (MSA) directly. The MSA register
definition makes no references to colorimetry, just a vague reference to
the DP spec. The connection to the color range was lost.
Apparently we've failed to set the proper MSA bit for limited, or CEA,
range ever since the first DDI platforms. We've started setting other
MSA parameters since commit dae847991a ("drm/i915: add
intel_ddi_set_pipe_settings").
Without the crucial bit of information, the DP sink has no way of
knowing the source is actually transmitting limited range RGB, leading
to "washed out" colors. With the colorimetry information, compliant
sinks should be able to handle the limited range properly. Native
(i.e. non-LSPCON) HDMI was not affected because we do pass the color
range via AVI infoframes.
Though not the root cause, the problem was made worse for DDI platforms
with commit 55bc60db59 ("drm/i915: Add "Automatic" mode for the
"Broadcast RGB" property"), which selects limited range RGB
automatically based on the mode, as per the DP, HDMI and CEA specs.
After all these years, the fix boils down to flipping one bit.
[Per testing reports, this fixes DP sinks, but not the LSPCON. My
educated guess is that the LSPCON fails to turn the CEA range MSA into
AVI infoframes for HDMI.]
Reported-by: Michał Kopeć <mkopec12@gmail.com>
Reported-by: N. W. <nw9165-3201@yahoo.com>
Reported-by: Nicholas Stommel <nicholas.stommel@gmail.com>
Reported-by: Tom Yan <tom.ty89@gmail.com>
Tested-by: Nicholas Stommel <nicholas.stommel@gmail.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=100023
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107476
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94921
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: <stable@vger.kernel.org> # v3.9+
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180814060001.18224-1-jani.nikula@intel.com
This reapplies commit 39f3be162c ("drm/i915: Kick waiters on resetting
legacy rings") after the improved gem_eio was run across all machines we
found that gen3 and early gen4 still lost the immediate interrupt
following reset, and the HWSTAM w/a applied to gen6+ is inadequate.
Unlike the later gen, on gen3/4 the principle (and only tests to fail so
far) are the wait vs reset test cases, whereas the reset stress case
works fine (which was the predominantly failing case for gen6+). That is
enough to suggest the underlying issue is sufficiently different to
support the difference in HWSTAM efficacy.
Testcase: igt/gem_eio/wait-10ms
References: 39f3be162c ("drm/i915: Kick waiters on resetting legacy rings")
References: a69ab52b03 ("drm/i915: Remove extra waiter kick on legacy resets")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180814104056.27001-1-chris@chris-wilson.co.uk
