- 20 Aug, 2018 5 commits
-
-
Manasi Navare authored
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
-
Manasi Navare authored
drm/i915/icl: Implement HSDIV_RATIO of MG_CLKTOP2_HSCLKCTL_PORT reg as separate divider value defines 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:
James Ausmus <james.ausmus@intel.com> Signed-off-by:
-
Chris Wilson authored
If the display has been disabled by modparam, we still want to connect together the HW bits and bobs with the associated drivers so that we can continue to manage their runtime power gating. Fixes: 10810944 ("drm/i915: Check num_pipes before initializing audio component") Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Imre Deak <imre.deak@intel.com> Cc: Takashi Iwai <tiwai@suse.de> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Elaine Wang <elaine.wang@intel.com> Reviewed-by:
Imre Deak <imre.deak@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180817100241.4628-1-chris@chris-wilson.co.uk
-
Fredrik Schön authored
100 ms is not enough time for the LSPCON adapter on Intel NUC devices to settle. This causes dropped display modes at boot or screen reconfiguration. Empirical testing can reproduce the error up to a timeout of 190 ms. Basic boot and stress testing at 200 ms has not (yet) failed. Increase timeout to 400 ms to get some margin of error. Changes from v1: The initial suggestion of 1000 ms was lowered due to concerns about delaying valid timeout cases. Update patch metadata. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107503 Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1570392 Fixes: 357c0ae9 ("drm/i915/lspcon: Wait for expected LSPCON mode to settle") Cc: Shashank Sharma <shashank.sharma@intel.com> Cc: Imre Deak <imre.deak@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: <stable@vger.kernel.org> # v4.11+ Reviewed-by:
Shashank Sharma <shashank.sharma@intel.com> Signed-off-by:
Fredrik Schön <fredrik.schon@gmail.com> Signed-off-by:
Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180817200728.8154-1-fredrik.schon@gmail.com
-
Imre Deak authored
After commit 2cd9a689 ("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:
-
- 17 Aug, 2018 1 commit
-
-
Anusha Srivatsa authored
Let us reuse the already defined has_csr check and not redefine it. The main difference is that in effect this will flip .has_csr to 1 (via GEN9_FEATURES which GEN11_FEATURES pulls in). Suggested-by:
Anusha Srivatsa <anusha.srivatsa@intel.com> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=107382Reviewed-by:
-
- 16 Aug, 2018 7 commits
-
-
Chris Wilson authored
Show the reset depth (the tasklet disable count) in the GEM_TRACE to indicate when we might not expect tasklets to be flushed. Signed-off-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180815135827.25869-1-chris@chris-wilson.co.uk
-
Lucas De Marchi authored
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:
-
Lucas De Marchi authored
The definition on i915_reg.h is going to change to depend on dev_priv->gpio_mmio_base being properly initialized. Define our own macros since init_generic_mmio_info() is called before than gpio_mmio_base being set. Cc: intel-gvt-dev@lists.freedesktop.org Cc: Zhenyu Wang <zhenyuw@linux.intel.com> Signed-off-by:
Zhenyu Wang <zhenyuw@linux.intel.com> Signed-off-by:
-
Lucas De Marchi authored
This is the only place that they are being used - the others use the GMBUS* macros that rely on dev_priv being already properly initialized. Cc: intel-gvt-dev@lists.freedesktop.org Cc: Zhenyu Wang <zhenyuw@linux.intel.com> Signed-off-by:
-
Imre Deak authored
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 authored
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 authored
The context owns both the ppgtt and the vma within it, and our activity tracking on the context ensures that we do not release active ppgtt. As the context fulfils our obligations for active memory tracking, we can relinquish the reference from the vma. This fixes a silly transient refleak from closed vma being kept alive until the entire system was idle, keeping all vm alive as well. Reported-by:
-
- 15 Aug, 2018 4 commits
-
-
Chris Wilson authored
As the only error is for a programming error in constructing the static tables describing the register values, replace the error code propagation with an assert. Signed-off-by:
-
Chris Wilson authored
We want to add no connectors, encoders or crtcs if the display is disabled, but we still need to hook up any existing HW so that we can power it down. Signed-off-by:
-
Imre Deak authored
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:
-
Chris Wilson authored
If we pardon a per-engine reset, we may leave the STOP_RING bit asserted in RING_MI_MODE resulting in the engine hanging. Unconditionally clear it on the per-engine exit path as we know that either we skipped the reset and so need the cancellation, or the reset was successful and the cancellation is a no-op, or there was an error and we will follow up with a full-reset or wedging (both of which will stop the engines again as required). Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107188 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106560Signed-off-by:
-
- 14 Aug, 2018 3 commits
-
-
Chris Wilson authored
If we cannot setup rc6, we cannot let the GPU suspend itself as it cannot save its state (to a powercontext). As such, we must disable runtime-pm, but we should do so using the low-level pm-runtime function which leaves our own debugging functions intact (and continue to detect errors in our runtime-pm handling should we ever be able to enable rc6). Signed-off-by:
-
Jani Nikula authored
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 dae84799 ("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 55bc60db ("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:
-
Chris Wilson authored
This reapplies commit 39f3be16 ("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: 39f3be16 ("drm/i915: Kick waiters on resetting legacy rings") References: a69ab52b ("drm/i915: Remove extra waiter kick on legacy resets") Signed-off-by:
-
- 13 Aug, 2018 5 commits
-
-
Chris Wilson authored
Do not call gen6_reset_rps_interrupts() when we know the registers do not exist. Signed-off-by:
-
Chris Wilson authored
Since the gt powerstate is allocated by i915_gem_init, clean it from i915_gem_fini for symmetry and to correct the imbalance on error. Signed-off-by:
-
Mika Kuoppala authored
If engine reports that it is not ready for reset, we give up. Evidence shows that forcing a per engine reset on an engine which is not reporting to be ready for reset, can bring it back into a working order. There is risk that we corrupt the context image currently executing on that engine. But that is a risk worth taking as if we unblock the engine, we prevent a whole device wedging in a case of full gpu reset. Reset individual engine even if it reports that it is not prepared for reset, but only if we aim for full gpu reset and not on first reset attempt. v2: force reset only on later attempts, readability (Chris) v3: simplify with adequate caffeine levels (Chris) v4: comment about risks and migitations (Chris) Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Mika Kuoppala authored
There is a possibility for per gen reset logic to be more nasty if the softer approach on resetting does not bear fruit. Expose retry count to per gen reset logic if it wants to take such tough measures. Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Chris Wilson authored
We require that we keep the list of outstanding work short so that we do not "leak" memory while pageflipping under stress. However that system stress may delay kernel workers virtually indefinitely, which incurs the pageflips stall and eventually hit a timeout waiting for the cleanup. Try to combat CPU starvation of our short-lived cleanup workers by switching to a high priority workqueue. Testcase: igt/kms_cursor_legacy/all-pipes-torture-move References: https://bugs.freedesktop.org/show_bug.cgi?id=107122Signed-off-by:
-
- 10 Aug, 2018 3 commits
-
-
Paulo Zanoni authored
The RS_CTX_ENABLE and CTX_SAVE_INHIBIT bits are not present on ICL anymore, but we still try to set them and then check them with GEM_BUG_ON, resulting in a BUG() call. The bug can be reproduced by igt/drv_selftest/live_hangcheck/others-priority and our CI was able to catch it. It is worth noticing that commit 05f0addd ("drm/i915/icl: Enhanced execution list support") already tried to avoid the save bits on ICL, but only inside populate_lr_context(). Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Testcase: igt/drv_selftest/live_hangcheck/others-priority Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107399 References: 05f0addd ("drm/i915/icl: Enhanced execution list support") Signed-off-by:
-
Maarten Lankhorst authored
This will make it easier to test PSR1 on PSR2 capable eDP machines. Changes since v1: - Remove I915_PSR_DEBUG_FORCE_PSR2, it did nothing, not sure forcing PSR2 would even work. - Handle NULL crtc in intel_psr_set_debugfs_mode. (dhnkrn) Signed-off-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180808141911.7647-2-maarten.lankhorst@linux.intel.comReviewed-by:
Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
-
Maarten Lankhorst authored
Currently tests modify i915.enable_psr and then do a modeset cycle to change PSR. We can write a value to i915_edp_psr_debug to force a certain PSR mode without a modeset. To retain compatibility with older userspace, we also still allow the override through the module parameter, and add some tracking to check whether a debugfs mode is specified. Changes since v1: - Rename dev_priv->psr.enabled to .dp, and .hw_configured to .enabled. - Fix i915_psr_debugfs_mode to match the writes to debugfs. - Rename __i915_edp_psr_write to intel_psr_set_debugfs_mode, simplify it and move it to intel_psr.c. This keeps all internals in intel_psr.c - Perform an interruptible wait for hw completion outside of the psr lock, instead of being forced to trywait and return -EBUSY. Changes since v2: - Rebase on top of intel_psr changes. Changes since v3: - Assign psr.dp during init. (dhnkrn) - Add prepared bool, which should be used instead of relying on psr.dp. (dhnkrn) - Fix -EDEADLK handling in debugfs. (dhnkrn) - Clean up waiting for idle in intel_psr_set_debugfs_mode. - Print PSR mode when trying to enable PSR. (dhnkrn) - Move changing psr debug setting to i915_edp_psr_debug_set. (dhnkrn) Changes since v4: - Return error in _set() function. - Change flag values to make them easier to remember. (dhnkrn) - Only assign psr.dp once. (dhnkrn) - Only set crtc_state->has_psr on the crtc with psr.dp. - Fix typo. (dhnkrn) Changes since v5: - Only wait for PSR idle on the PSR connector correctly. (dhnkrn) - Reinstate WARN_ON(drrs.dp) in intel_psr_enable. (dhnkrn) - Remove stray comment. (dhnkrn) - Be silent in intel_psr_compute_config on wrong connector. (dhnkrn) Signed-off-by:
-
- 09 Aug, 2018 3 commits
-
-
Chris Wilson authored
The call to i915_gem_unpark() checks that we hold a rpm wakeref before taking a long term wakeref for i915->gt.awake. We should therefore make sure we do hold the wakeref when directly calling unpark to disable the retire worker. Fixes: 932cac10 ("drm/i915/selftests: Prevent background reaping of active objects") Signed-off-by:
-
Chris Wilson authored
On suspend, we cancel the automatic forcewake and clear all other sources of forcewake so the machine can sleep before we do suspend. However, we expose the forcewake to userspace (only via debugfs, but nevertheless we do) and want to restore that upon resume or else our accounting will be off and we may not acquire the forcewake before we use it. So record which domains we cleared on suspend and reacquire them early on resume. v2: Hold the spinlock to appease our sanitychecks v3: s/fw_domains_user/fw_domains_saved/ to convey intent more clearly Reported-by:
Imre Deak <imre.deak@linux.intel.com> Fixes: b8473050 ("drm/i915: Fix forcewake active domain tracking") Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180808210842.3555-1-chris@chris-wilson.co.uk
-
Rodrigo Vivi authored
One more CFL ID added to spec. Cc: José Roberto de Souza <jose.souza@intel.com> Signed-off-by:
-
- 08 Aug, 2018 9 commits
-
-
Chris Wilson authored
Now with a more efficacious workaround for the lost interrupts after reset, we can remove the hack of kicking the waiters after reset. The issue was that the kick only worked for the immediate window after the reset (those seqno that would complete in the time it took for the waiter thread to perform its check) but miss any seqno that lacked an interrupt afterwards. References: 39f3be16 ("drm/i915: Kick waiters on resetting legacy rings") Signed-off-by:
-
Chris Wilson authored
An oddity occurs on Sandybridge, Ivybridge and Haswell (and presumably Valleyview) in that for the period following the GPU restart after a reset, there are no GT interrupts received. From Ville's notes, bit 0 in the HWSTAM corresponds to the render interrupt, and if we unmask it we do see immediate resumption of GT interrupt delivery (via the master irq handler) after the reset. v2: Limit the w/a to the render interrupt from rcs Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107500 Fixes: c5498089 ("drm/i915: Mask everything in ring HWSTAM on gen6+ in ringbuffer mode") References: d420a50c ("drm/i915: Clean up the HWSTAM mess") Testcase: igt/gem_eio/reset-stress Signed-off-by:
-
Chris Wilson authored
Hitting the timeout and finding that all engines are actually idle is indicative of an interrupt delivery problem. This problem is an issue that we need to fix, so make sure we log it and provide the GEM trace. Signed-off-by:
-
Imre Deak authored
On ICL there are 5 fused power gates, so add the two missing ones for clarity. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by:
-
Imre Deak authored
There is no need for separate IDs for power wells on a new platform with the same functionality as an other power well on a previous platform, we can just reuse the ID from the previous platform. This is only possible after the previous patches where we removed dependence on the actual enum values. This also fixes a problem on ICL where in assert_can_enable_dc5/9() we would've failed to look up the PW#2 power well. v2: - Keep an ID assigned for the ICL PW#2 power well too. (Paulo) Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by:
-
Imre Deak authored
The format for the ID names is <platform>_DISP_PW_* so rename the IDs not following this accordingly. Leave BXT_DPIO_CMN_BC as-is since we'll change that to use another existing ID in the next patch. v2: - Fix line over 80 chars checkpatch warning. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by:
-
Imre Deak authored
Now that we removed dependence on the power well IDs to determine the control register and request/status flag offsets the only purpose of power well IDs is to look up power wells directly bypassing the power domains framework. However this direct lookup isn't needed for most of the exisiting power wells and hopefully won't be needed for any new power wells in the future. To make maintenance of the power well ID enum easier, don't require a unique ID for each power well, only if it's necessary. Remove the IDs becoming redundant this way and assign to all the corresponding power wells a new DISP_PW_ID_NONE ID. After the previous two patches the IDs don't need to have a fixed value, so remove the explicit initializers and adjust the enum's code comment accordingly. v2: - Keep required ID assignments for HSW_DISP_PW_GLOBAL and ICL_DISP_PW_2. (Paulo) Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by:
-
Imre Deak authored
Similarly to the previous patch use a separate request/status HW flag index defined right after the corresponding control registers instead of depending for this on the power well IDs. Since the set of control/status registers varies among the different power wells (on a single platform), also add a new i915_power_well_registers struct that we populate and assign to each DDI power well as needed. Also clarify a bit the code comment describing the function and layout of the control registers. This also fixes a problem on ICL, where we incorrectly read the KVMR control register in hsw_power_well_requesters() even for DDI and AUX power wells. v2: - Clarify platform range tags in code comments. (Paulo) - Fix line over 80 chars checkpatch warning. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by:
-
Imre Deak authored
Atm, we determine the control/status flag offsets within the PUNIT control/status registers based on the power well's ID. Since the power well ID enum is global across all platforms, the associated macros to get the flag offsets involves some magic. This makes checking the register/bit definitions against the specification more difficult than necessary. Also the values in the power well ID enum must stay fixed, making code maintenance of the enum cumbersome. To solve the above define the control/status flag indices right after the corresponding registers and use these to derive the control/status flag values by storing the indices in the i915_power_well_desc struct. Initializing anonymous union fields require the preceding field in the struct to be explicitly initialized - even when using named initializers - and the initialization to be done right before the union initialization, hence the reordering of the .id fields. v2: - Clarify commit log message about anonymous union initializers. (Paulo) Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by:
-
