From 7a7d58dc2997b6ca5bc1eaa4bd8d019cf7c4a148 Mon Sep 17 00:00:00 2001 From: Dhinakaran Pandiyan Date: Tue, 13 Mar 2018 22:48:25 -0700 Subject: drm/i915/dp: Write to SET_POWER dpcd to enable MST hub. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If bios sets up an MST output and hardware state readout code sees this is an SST configuration, when disabling the encoder we end up calling ->post_disable_dp() hook instead of the MST version. Consequently, we write to the DP_SET_POWER dpcd to set it D3 state. Further along when we try enable the encoder in MST mode, POWER_UP_PHY transaction fails to power up the MST hub. This results in continuous link training failures which keep the system busy delaying boot. We could identify bios MST boot discrepancy and handle it accordingly but a simple way to solve this is to write to the DP_SET_POWER dpcd for MST too. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105470 Cc: Ville Syrjälä Cc: Jani Nikula Reviewed-by: Ville Syrjälä Reported-by: Laura Abbott Cc: stable@vger.kernel.org Fixes: 5ea2355a100a ("drm/i915/mst: Use MST sideband message transactions for dpms control") Tested-by: Laura Abbott Signed-off-by: Dhinakaran Pandiyan Signed-off-by: Jani Nikula Link: https://patchwork.freedesktop.org/patch/msgid/20180314054825.1718-1-dhinakaran.pandiyan@intel.com (cherry picked from commit ad260ab32a4d94fa974f58262f8000472d34fd5b) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_ddi.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index dbcf1a0..8c2d778 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -2205,8 +2205,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, intel_prepare_dp_ddi_buffers(encoder, crtc_state); intel_ddi_init_dp_buf_reg(encoder); - if (!is_mst) - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); intel_dp_start_link_train(intel_dp); if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) intel_dp_stop_link_train(intel_dp); @@ -2304,14 +2303,12 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder, struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base); struct intel_dp *intel_dp = &dig_port->dp; - bool is_mst = intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST); /* * Power down sink before disabling the port, otherwise we end * up getting interrupts from the sink on detecting link loss. */ - if (!is_mst) - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); intel_disable_ddi_buf(encoder); -- cgit v1.1 From 8f30c4085b72485c05e85d56e9a84098da2ba5b7 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 20 Mar 2018 10:04:48 +0000 Subject: drm/i915: Specify which engines to reset following semaphore/event lockups If the GPU is stuck waiting for an event or for a semaphore, we need to reset the GPU in order to recover. We have to tell the reset routine which engines we want reset, but we were still using the old interface and declaring it as "not-fatal". Fixes: 14b730fcb8d9 ("drm/i915/tdr: Prepare error handler to accept mask of hung engines") Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Michel Thierry Reviewed-by: Michel Thierry Link: https://patchwork.freedesktop.org/patch/msgid/20180320100449.1360-1-chris@chris-wilson.co.uk (cherry picked from commit ca98317b89428e6ac17be0938b467ed78654dd56) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_hangcheck.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_hangcheck.c b/drivers/gpu/drm/i915/intel_hangcheck.c index 42e45ae..c8ea510 100644 --- a/drivers/gpu/drm/i915/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/intel_hangcheck.c @@ -246,7 +246,7 @@ engine_stuck(struct intel_engine_cs *engine, u64 acthd) */ tmp = I915_READ_CTL(engine); if (tmp & RING_WAIT) { - i915_handle_error(dev_priv, 0, + i915_handle_error(dev_priv, BIT(engine->id), "Kicking stuck wait on %s", engine->name); I915_WRITE_CTL(engine, tmp); @@ -258,7 +258,7 @@ engine_stuck(struct intel_engine_cs *engine, u64 acthd) default: return ENGINE_DEAD; case 1: - i915_handle_error(dev_priv, 0, + i915_handle_error(dev_priv, ALL_ENGINES, "Kicking stuck semaphore on %s", engine->name); I915_WRITE_CTL(engine, tmp); -- cgit v1.1 From 76cb9d314a5385020c3adabf563df8efe71c6325 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 21 Mar 2018 09:10:27 +0000 Subject: drm/i915/execlists: Use a locked clear_bit() for synchronisation with interrupt We were relying on the uncached reads when processing the CSB to provide ourselves with the serialisation with the interrupt handler (so we could detect new interrupts in the middle of processing the old one). However, in commit 767a983ab255 ("drm/i915/execlists: Read the context-status HEAD from the HWSP") those uncached reads were eliminated (on one path at least) and along with them our serialisation. The result is that we would very rarely miss notification of a new interrupt and leave a context-switch unprocessed, hanging the GPU. Fixes: 767a983ab255 ("drm/i915/execlists: Read the context-status HEAD from the HWSP") Signed-off-by: Chris Wilson Cc: Michel Thierry Cc: Tvrtko Ursulin Cc: Mika Kuoppala Reviewed-by: Tvrtko Ursulin Reviewed-by: Mika Kuoppala Reviewed-by: Michel Thierry Link: https://patchwork.freedesktop.org/patch/msgid/20180321091027.21034-1-chris@chris-wilson.co.uk (cherry picked from commit 9153e6b7c85edbc89e874e5c83f86217c53dcfaf) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_lrc.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 3a69b36..697af5a 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -831,7 +831,8 @@ static void execlists_submission_tasklet(unsigned long data) struct drm_i915_private *dev_priv = engine->i915; bool fw = false; - /* We can skip acquiring intel_runtime_pm_get() here as it was taken + /* + * We can skip acquiring intel_runtime_pm_get() here as it was taken * on our behalf by the request (see i915_gem_mark_busy()) and it will * not be relinquished until the device is idle (see * i915_gem_idle_work_handler()). As a precaution, we make sure @@ -840,7 +841,8 @@ static void execlists_submission_tasklet(unsigned long data) */ GEM_BUG_ON(!dev_priv->gt.awake); - /* Prefer doing test_and_clear_bit() as a two stage operation to avoid + /* + * Prefer doing test_and_clear_bit() as a two stage operation to avoid * imposing the cost of a locked atomic transaction when submitting a * new request (outside of the context-switch interrupt). */ @@ -856,17 +858,10 @@ static void execlists_submission_tasklet(unsigned long data) execlists->csb_head = -1; /* force mmio read of CSB ptrs */ } - /* The write will be ordered by the uncached read (itself - * a memory barrier), so we do not need another in the form - * of a locked instruction. The race between the interrupt - * handler and the split test/clear is harmless as we order - * our clear before the CSB read. If the interrupt arrived - * first between the test and the clear, we read the updated - * CSB and clear the bit. If the interrupt arrives as we read - * the CSB or later (i.e. after we had cleared the bit) the bit - * is set and we do a new loop. - */ - __clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); + /* Clear before reading to catch new interrupts */ + clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); + smp_mb__after_atomic(); + if (unlikely(execlists->csb_head == -1)) { /* following a reset */ if (!fw) { intel_uncore_forcewake_get(dev_priv, -- cgit v1.1 From 300efa9eea451bdcf3b5a1eb292222e06e85bb2c Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Thu, 22 Mar 2018 16:36:42 +0200 Subject: drm/i915: Fix hibernation with ACPI S0 target state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After commit dd9f31c7a3887950cbd0d49eb9d43f7a1518a356 Author: Imre Deak Date: Wed Aug 16 17:46:07 2017 +0300 drm/i915/gen9+: Set same power state before hibernation image save/restore during hibernation/suspend the power domain functionality got disabled, after which resume could leave it incorrectly disabled if the ACPI target state was S0 during suspend and i915 was not loaded by the loader kernel. This was caused by not considering if we resumed from hibernation as the condition for power domains reiniting. Fix this by simply tracking if we suspended power domains during system suspend and reinit power domains accordingly during resume. This will result in reiniting power domains always when resuming from hibernation, regardless of the platform and whether or not i915 is loaded by the loader kernel. The reason we didn't catch this earlier is that the enabled/disabled state of power domains during PMSG_FREEZE/PMSG_QUIESCE is platform and kernel config dependent: on my SKL the target state is S4 during PMSG_FREEZE and (with the driver loaded in the loader kernel) S0 during PMSG_QUIESCE. On the reporter's machine it's S0 during PMSG_FREEZE but (contrary to this) power domains are not initialized during PMSG_QUIESCE since i915 is not loaded in the loader kernel, or it's loaded but without the DMC firmware being available. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105196 Reported-and-tested-by: amn-bas@hotmail.com Fixes: dd9f31c7a388 ("drm/i915/gen9+: Set same power state before hibernation image save/restore") Cc: amn-bas@hotmail.com Cc: Ville Syrjälä Cc: Signed-off-by: Imre Deak Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20180322143642.26883-1-imre.deak@intel.com (cherry picked from commit 0f90603c33bdf6575cfdc81edd53f3f13ba166fb) Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_drv.c | 22 ++++++++++------------ drivers/gpu/drm/i915/i915_drv.h | 2 +- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index d7c4de4..07c07d5 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1611,15 +1611,12 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) { struct drm_i915_private *dev_priv = to_i915(dev); struct pci_dev *pdev = dev_priv->drm.pdev; - bool fw_csr; int ret; disable_rpm_wakeref_asserts(dev_priv); intel_display_set_init_power(dev_priv, false); - fw_csr = !IS_GEN9_LP(dev_priv) && !hibernation && - suspend_to_idle(dev_priv) && dev_priv->csr.dmc_payload; /* * In case of firmware assisted context save/restore don't manually * deinit the power domains. This also means the CSR/DMC firmware will @@ -1627,8 +1624,11 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) * also enable deeper system power states that would be blocked if the * firmware was inactive. */ - if (!fw_csr) + if (IS_GEN9_LP(dev_priv) || hibernation || !suspend_to_idle(dev_priv) || + dev_priv->csr.dmc_payload == NULL) { intel_power_domains_suspend(dev_priv); + dev_priv->power_domains_suspended = true; + } ret = 0; if (IS_GEN9_LP(dev_priv)) @@ -1640,8 +1640,10 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) if (ret) { DRM_ERROR("Suspend complete failed: %d\n", ret); - if (!fw_csr) + if (dev_priv->power_domains_suspended) { intel_power_domains_init_hw(dev_priv, true); + dev_priv->power_domains_suspended = false; + } goto out; } @@ -1662,8 +1664,6 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) if (!(hibernation && INTEL_GEN(dev_priv) < 6)) pci_set_power_state(pdev, PCI_D3hot); - dev_priv->suspended_to_idle = suspend_to_idle(dev_priv); - out: enable_rpm_wakeref_asserts(dev_priv); @@ -1830,8 +1830,7 @@ static int i915_drm_resume_early(struct drm_device *dev) intel_uncore_resume_early(dev_priv); if (IS_GEN9_LP(dev_priv)) { - if (!dev_priv->suspended_to_idle) - gen9_sanitize_dc_state(dev_priv); + gen9_sanitize_dc_state(dev_priv); bxt_disable_dc9(dev_priv); } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { hsw_disable_pc8(dev_priv); @@ -1839,8 +1838,7 @@ static int i915_drm_resume_early(struct drm_device *dev) intel_uncore_sanitize(dev_priv); - if (IS_GEN9_LP(dev_priv) || - !(dev_priv->suspended_to_idle && dev_priv->csr.dmc_payload)) + if (dev_priv->power_domains_suspended) intel_power_domains_init_hw(dev_priv, true); else intel_display_set_init_power(dev_priv, true); @@ -1850,7 +1848,7 @@ static int i915_drm_resume_early(struct drm_device *dev) enable_rpm_wakeref_asserts(dev_priv); out: - dev_priv->suspended_to_idle = false; + dev_priv->power_domains_suspended = false; return ret; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6e740f6..ce18b6c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2119,7 +2119,7 @@ struct drm_i915_private { u32 bxt_phy_grc; u32 suspend_count; - bool suspended_to_idle; + bool power_domains_suspended; struct i915_suspend_saved_registers regfile; struct vlv_s0ix_state vlv_s0ix_state; -- cgit v1.1