- 24 Aug, 2018 2 commits
-
-
Paulo Zanoni authored
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
-
Paulo Zanoni authored
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:
-
- 23 Aug, 2018 2 commits
-
-
Rodrigo Vivi authored
We use kzalloc to allocate the write_buf that we use for i2c transfer on hdcp write. But it seems that we are forgetting to free the memory that is not needed after i2c transfer is completed. Reported-by:
Brian J Wood <brian.j.wood@intel.com> Fixes: 2320175f ("drm/i915: Implement HDCP for HDMI") Cc: Ramalingam C <ramalingam.c@intel.com> Cc: Sean Paul <seanpaul@chromium.org> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: <stable@vger.kernel.org> # v4.17+ Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20180823205136.31310-1-rodrigo.vivi@intel.com
-
Imre Deak authored
For S0ix we want to deinit power domains (and so deactivate the DMC firmware) exactly when the platform supports the DC9 state. To reach S0ix we need DC9 on these platforms (for which the DMC FW needs to be deactivated) while to reach S0ix on the rest of the DMC platforms we need DC6 (which needs the DMC FW to stay active). Simplify the condition accordingly so it will be automatically correct for upcoming DC9 platforms like ICL. Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Michel Thierry <michel.thierry@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: 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/20180822112602.27543-1-imre.deak@intel.com
-
- 22 Aug, 2018 7 commits
-
-
Dhinakaran Pandiyan authored
Rename PLANE_CTL_DECOMPRESSION_ENABLE to resemble the bpsec name - PLANE_CTL_RENDER_DECOMPRESSION_ENABLE Suggested-by:
Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Reviewed-by:
-
Dhinakaran Pandiyan authored
Code looks cleaner with modifiers hidden inside this wrapper. v2: Remove const qualifier (Ville) Signed-off-by:
-
Azhar Shaikh authored
Log the PSR mode/revision (PSR1 or PSR2) in the debugfs file i915_edp_psr_status. Suggested-by:
Azhar Shaikh <azhar.shaikh@intel.com> Reviewed-by:
-
Ville Syrjälä authored
The workaround was supposed to look at the plane destination coordinates. Currently it's looking at some mixture of src and dst coordinates that doesn't make sense. Fix it up. Signed-off-by:
-
Dhinakaran Pandiyan authored
gen8_de_irq_postinstall() wasn't masking the IRQ bit before passing the debug flag to psr_irq_control(). This check was missed when new debug bits were defined in 'commit c44301fc ("drm/i915: Allow control of PSR at runtime through debugfs, v6")'. Instead of ANDing the irq bit in all the callers, move it to the callee. v2: Rebased. Fixes: c44301fc ("drm/i915: Allow control of PSR at runtime through debugfs, v6") Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180821221156.2442-3-dhinakaran.pandiyan@intel.com
-
Dhinakaran Pandiyan authored
We print the last attempted entry and last exit timestamps only when IRQ debug is requested. This check was missed when new debug flags were added in 'commit c44301fc ("drm/i915: Allow control of PSR at runtime through debugfs, v6") Fixes: c44301fc ("drm/i915: Allow control of PSR at runtime through debugfs, v6") Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by:
-
Dhinakaran Pandiyan authored
Knowing the status of the PSR HW state machine is useful for debug, especially since we are seeing errors with PSR2 in CI. Cc: José Roberto de Souza <jose.souza@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by:
-
- 21 Aug, 2018 1 commit
-
-
Chris Wilson authored
Since we no longer maintain our read position in the CSB pointers register, it always returns 0 and not where we last read up to. As a result the CSB probing in the state dumper starts from 0, either missing entries or showing stale one. Signed-off-by:
Mika Kuoppala <mika.kuoppala@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180821101138.15822-1-chris@chris-wilson.co.uk
-
- 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:
-
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:
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:
-
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 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:
-
