From bc87229f323e6b5ee9ac57817463fb2ff65e6811 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Wed, 18 Nov 2015 17:32:30 +0200 Subject: [PATCH 01/70] drm/i915/skl: enable PC9/10 power states during suspend-to-idle During suspend-to-idle we need to keep the DMC firmware active and DC6 enabled, since otherwise we won't reach deep system power states like PC9/10. The lead for this came from Nivedita who noticed that the kernel's turbostat tool didn't report any PC9/10 residency change across an 'echo freeze > /sys/power/state'. Reported-by: Nivedita Swaminathan Signed-off-by: Imre Deak Reviewed-by: Patrik Jakobsson Link: http://patchwork.freedesktop.org/patch/msgid/1447860750-18110-1-git-send-email-imre.deak@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 44 +++++++++++++++++++++++++-------- drivers/gpu/drm/i915/i915_drv.h | 1 + 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6344dfb72177..649e20aaa3ff 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -624,6 +624,14 @@ static int vlv_resume_prepare(struct drm_i915_private *dev_priv, bool rpm_resume); static int bxt_resume_prepare(struct drm_i915_private *dev_priv); +static bool suspend_to_idle(struct drm_i915_private *dev_priv) +{ +#if IS_ENABLED(CONFIG_ACPI_SLEEP) + if (acpi_target_system_state() < ACPI_STATE_S3) + return true; +#endif + return false; +} static int i915_drm_suspend(struct drm_device *dev) { @@ -676,11 +684,7 @@ static int i915_drm_suspend(struct drm_device *dev) i915_save_state(dev); - opregion_target_state = PCI_D3cold; -#if IS_ENABLED(CONFIG_ACPI_SLEEP) - if (acpi_target_system_state() < ACPI_STATE_S3) - opregion_target_state = PCI_D1; -#endif + opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold; intel_opregion_notify_adapter(dev, opregion_target_state); intel_uncore_forcewake_reset(dev, false); @@ -701,15 +705,26 @@ static int i915_drm_suspend(struct drm_device *dev) static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation) { struct drm_i915_private *dev_priv = drm_dev->dev_private; + bool fw_csr; int ret; - intel_power_domains_suspend(dev_priv); + fw_csr = 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 + * stay active, it will power down any HW resources as required and + * also enable deeper system power states that would be blocked if the + * firmware was inactive. + */ + if (!fw_csr) + intel_power_domains_suspend(dev_priv); ret = intel_suspend_complete(dev_priv); if (ret) { DRM_ERROR("Suspend complete failed: %d\n", ret); - intel_power_domains_init_hw(dev_priv, true); + if (!fw_csr) + intel_power_domains_init_hw(dev_priv, true); return ret; } @@ -730,6 +745,8 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation) if (!(hibernation && INTEL_INFO(dev_priv)->gen < 6)) pci_set_power_state(drm_dev->pdev, PCI_D3hot); + dev_priv->suspended_to_idle = suspend_to_idle(dev_priv); + return 0; } @@ -842,8 +859,10 @@ static int i915_drm_resume_early(struct drm_device *dev) * FIXME: This should be solved with a special hdmi sink device or * similar so that power domains can be employed. */ - if (pci_enable_device(dev->pdev)) - return -EIO; + if (pci_enable_device(dev->pdev)) { + ret = -EIO; + goto out; + } pci_set_master(dev->pdev); @@ -861,7 +880,12 @@ static int i915_drm_resume_early(struct drm_device *dev) hsw_disable_pc8(dev_priv); intel_uncore_sanitize(dev); - intel_power_domains_init_hw(dev_priv, true); + + if (!(dev_priv->suspended_to_idle && dev_priv->csr.dmc_payload)) + intel_power_domains_init_hw(dev_priv, true); + +out: + dev_priv->suspended_to_idle = false; return ret; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5a5c06b14ab2..3d8741eff7d3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1885,6 +1885,7 @@ struct drm_i915_private { u32 chv_phy_control; u32 suspend_count; + bool suspended_to_idle; struct i915_suspend_saved_registers regfile; struct vlv_s0ix_state vlv_s0ix_state; From a7de5506004415527035d7858b5993c906e2544e Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Wed, 18 Nov 2015 19:53:50 +0200 Subject: [PATCH 02/70] drm/i915/skl: re-enable power well support Now that the known DMC/DC issues are fixed, let's try again and re-enable the power well support. Signed-off-by: Imre Deak Reviewed-by: Patrik Jakobsson Link: http://patchwork.freedesktop.org/patch/msgid/1447869230-21416-1-git-send-email-imre.deak@intel.com --- drivers/gpu/drm/i915/intel_runtime_pm.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index a1dc81518995..10154a773f74 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -1841,11 +1841,6 @@ sanitize_disable_power_well_option(const struct drm_i915_private *dev_priv, if (disable_power_well >= 0) return !!disable_power_well; - if (IS_SKYLAKE(dev_priv)) { - DRM_DEBUG_KMS("Disabling display power well support\n"); - return 0; - } - return 1; } From 9895ad03c871893f6acd66a3b6210db0c1ba5933 Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Fri, 20 Nov 2015 15:55:33 +0000 Subject: [PATCH 03/70] drm/i915/pm: Unstatic power_domain_str MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let us print human-parseable values from the power domain code; upcoming display code also wants to use it. This requires moving it out of i915_debugfs.c, as that is only conditionally compiled. v2: Move it out of the header. Signed-off-by: Daniel Stone Reviewed-by: Ville Syrjälä Signed-off-by: Imre Deak Link: http://patchwork.freedesktop.org/patch/msgid/1448034934-11926-1-git-send-email-daniels@collabora.com --- drivers/gpu/drm/i915/i915_debugfs.c | 67 +------------------------ drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_runtime_pm.c | 66 ++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 66 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 411a9c68b4ee..b28da6fd7540 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2685,71 +2685,6 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused) return 0; } -static const char *power_domain_str(enum intel_display_power_domain domain) -{ - switch (domain) { - case POWER_DOMAIN_PIPE_A: - return "PIPE_A"; - case POWER_DOMAIN_PIPE_B: - return "PIPE_B"; - case POWER_DOMAIN_PIPE_C: - return "PIPE_C"; - case POWER_DOMAIN_PIPE_A_PANEL_FITTER: - return "PIPE_A_PANEL_FITTER"; - case POWER_DOMAIN_PIPE_B_PANEL_FITTER: - return "PIPE_B_PANEL_FITTER"; - case POWER_DOMAIN_PIPE_C_PANEL_FITTER: - return "PIPE_C_PANEL_FITTER"; - case POWER_DOMAIN_TRANSCODER_A: - return "TRANSCODER_A"; - case POWER_DOMAIN_TRANSCODER_B: - return "TRANSCODER_B"; - case POWER_DOMAIN_TRANSCODER_C: - return "TRANSCODER_C"; - case POWER_DOMAIN_TRANSCODER_EDP: - return "TRANSCODER_EDP"; - case POWER_DOMAIN_PORT_DDI_A_LANES: - return "PORT_DDI_A_LANES"; - case POWER_DOMAIN_PORT_DDI_B_LANES: - return "PORT_DDI_B_LANES"; - case POWER_DOMAIN_PORT_DDI_C_LANES: - return "PORT_DDI_C_LANES"; - case POWER_DOMAIN_PORT_DDI_D_LANES: - return "PORT_DDI_D_LANES"; - case POWER_DOMAIN_PORT_DDI_E_LANES: - return "PORT_DDI_E_LANES"; - case POWER_DOMAIN_PORT_DSI: - return "PORT_DSI"; - case POWER_DOMAIN_PORT_CRT: - return "PORT_CRT"; - case POWER_DOMAIN_PORT_OTHER: - return "PORT_OTHER"; - case POWER_DOMAIN_VGA: - return "VGA"; - case POWER_DOMAIN_AUDIO: - return "AUDIO"; - case POWER_DOMAIN_PLLS: - return "PLLS"; - case POWER_DOMAIN_AUX_A: - return "AUX_A"; - case POWER_DOMAIN_AUX_B: - return "AUX_B"; - case POWER_DOMAIN_AUX_C: - return "AUX_C"; - case POWER_DOMAIN_AUX_D: - return "AUX_D"; - case POWER_DOMAIN_GMBUS: - return "GMBUS"; - case POWER_DOMAIN_MODESET: - return "MODESET"; - case POWER_DOMAIN_INIT: - return "INIT"; - default: - MISSING_CASE(domain); - return "?"; - } -} - static int i915_power_domain_info(struct seq_file *m, void *unused) { struct drm_info_node *node = m->private; @@ -2775,7 +2710,7 @@ static int i915_power_domain_info(struct seq_file *m, void *unused) continue; seq_printf(m, " %-23s %d\n", - power_domain_str(power_domain), + intel_display_power_domain_str(power_domain), power_domains->domain_use_count[power_domain]); } } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index e558d4b36f73..7127332ae31f 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1412,6 +1412,8 @@ void intel_power_domains_suspend(struct drm_i915_private *dev_priv); void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv); void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv); void intel_runtime_pm_enable(struct drm_i915_private *dev_priv); +const char * +intel_display_power_domain_str(enum intel_display_power_domain domain); bool intel_display_power_is_enabled(struct drm_i915_private *dev_priv, enum intel_display_power_domain domain); diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 10154a773f74..ec38409242d5 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -65,6 +65,72 @@ bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv, int power_well_id); +const char * +intel_display_power_domain_str(enum intel_display_power_domain domain) +{ + switch (domain) { + case POWER_DOMAIN_PIPE_A: + return "PIPE_A"; + case POWER_DOMAIN_PIPE_B: + return "PIPE_B"; + case POWER_DOMAIN_PIPE_C: + return "PIPE_C"; + case POWER_DOMAIN_PIPE_A_PANEL_FITTER: + return "PIPE_A_PANEL_FITTER"; + case POWER_DOMAIN_PIPE_B_PANEL_FITTER: + return "PIPE_B_PANEL_FITTER"; + case POWER_DOMAIN_PIPE_C_PANEL_FITTER: + return "PIPE_C_PANEL_FITTER"; + case POWER_DOMAIN_TRANSCODER_A: + return "TRANSCODER_A"; + case POWER_DOMAIN_TRANSCODER_B: + return "TRANSCODER_B"; + case POWER_DOMAIN_TRANSCODER_C: + return "TRANSCODER_C"; + case POWER_DOMAIN_TRANSCODER_EDP: + return "TRANSCODER_EDP"; + case POWER_DOMAIN_PORT_DDI_A_LANES: + return "PORT_DDI_A_LANES"; + case POWER_DOMAIN_PORT_DDI_B_LANES: + return "PORT_DDI_B_LANES"; + case POWER_DOMAIN_PORT_DDI_C_LANES: + return "PORT_DDI_C_LANES"; + case POWER_DOMAIN_PORT_DDI_D_LANES: + return "PORT_DDI_D_LANES"; + case POWER_DOMAIN_PORT_DDI_E_LANES: + return "PORT_DDI_E_LANES"; + case POWER_DOMAIN_PORT_DSI: + return "PORT_DSI"; + case POWER_DOMAIN_PORT_CRT: + return "PORT_CRT"; + case POWER_DOMAIN_PORT_OTHER: + return "PORT_OTHER"; + case POWER_DOMAIN_VGA: + return "VGA"; + case POWER_DOMAIN_AUDIO: + return "AUDIO"; + case POWER_DOMAIN_PLLS: + return "PLLS"; + case POWER_DOMAIN_AUX_A: + return "AUX_A"; + case POWER_DOMAIN_AUX_B: + return "AUX_B"; + case POWER_DOMAIN_AUX_C: + return "AUX_C"; + case POWER_DOMAIN_AUX_D: + return "AUX_D"; + case POWER_DOMAIN_GMBUS: + return "GMBUS"; + case POWER_DOMAIN_INIT: + return "INIT"; + case POWER_DOMAIN_MODESET: + return "MODESET"; + default: + MISSING_CASE(domain); + return "?"; + } +} + static void intel_power_well_enable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { From 11c86db8f052fec64804c19519a8d35958ed8aef Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Fri, 20 Nov 2015 15:55:34 +0000 Subject: [PATCH 04/70] drm/i915/pm: Print offending domain in refcount failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we experience a refcounting failure in a power domain/well (unref'ing at least one too many times), log the name of the offending domain or well. Signed-off-by: Daniel Stone Reviewed-by: Ville Syrjälä Acked-by: Jani Nikula Signed-off-by: Imre Deak Link: http://patchwork.freedesktop.org/patch/msgid/1448034934-11926-2-git-send-email-daniels@collabora.com --- drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index ec38409242d5..3fa43af94946 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -1499,11 +1499,15 @@ void intel_display_power_put(struct drm_i915_private *dev_priv, mutex_lock(&power_domains->lock); - WARN_ON(!power_domains->domain_use_count[domain]); + WARN(!power_domains->domain_use_count[domain], + "Use count on domain %s is already zero\n", + intel_display_power_domain_str(domain)); power_domains->domain_use_count[domain]--; for_each_power_well_rev(i, power_well, BIT(domain), power_domains) { - WARN_ON(!power_well->count); + WARN(!power_well->count, + "Use count on power well %s is already zero", + power_well->name); if (!--power_well->count) intel_power_well_disable(dev_priv, power_well); From 408952d43b27a54437244c56c0e0d8efa5607926 Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Fri, 20 Nov 2015 13:23:36 +0000 Subject: [PATCH 05/70] drm/i915: Remove incorrect warning in context cleanup Commit e9f24d5fb7cf3628b195b18ff3ac4e37937ceeae Author: Tvrtko Ursulin Date: Mon Oct 5 13:26:36 2015 +0100 drm/i915: Clean up associated VMAs on context destruction Added a warning based on an incorrect assumption that all VMAs in a VM will be on the inactive list at the point last reference to a context and VM is dropped. This is not true because i915_gem_object_retire__read will not put VMA on the inactive list until all activities on the object in question (in all VMs) have been retired. As a consequence, whether or not a context/VM will be destroyed with its VMAs still on the active list, can depend on completely unrelated activities using the same object from a different context or engine. Signed-off-by: Tvrtko Ursulin Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92638 Testcase: igt/gem_request_retire/retire-vma-not-inactive Cc: Daniel Vetter Cc: Chris Wilson Cc: Michel Thierry Link: http://patchwork.freedesktop.org/patch/msgid/1448025816-25584-1-git-send-email-tvrtko.ursulin@linux.intel.com Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_context.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 4b9400402aa3..43761c5bcaca 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -141,8 +141,6 @@ static void i915_gem_context_clean(struct intel_context *ctx) if (!ppgtt) return; - WARN_ON(!list_empty(&ppgtt->base.active_list)); - list_for_each_entry_safe(vma, next, &ppgtt->base.inactive_list, mm_list) { if (WARN_ON(__i915_vma_unbind_no_wait(vma))) From ca1a95334ddaf624c1b0424113fe9b8100da1614 Mon Sep 17 00:00:00 2001 From: Rodrigo Vivi Date: Thu, 12 Nov 2015 14:46:05 -0800 Subject: [PATCH 06/70] drm/i915: Remove duplicated dpcd write on hsw_psr_enable_sink. Commit (89251b17) intended to remove this line and let only one DP_PSR_EN_CFG set, but it was wrong and this call is now duplicated at the code. Also "& ~DP_PSR_MAIN_LINK_ACTIVE" doesn't do anything at all. It was like that since I introduced this call but probably the idea was to be informative and make clear statement that we were not using the link standby. So it is better to remove this one here and let the code a bit cleaner. v2: Improve commit message as requested by Paulo. Cc: Paulo Zanoni Tested-by: Brian Norris Signed-off-by: Rodrigo Vivi Reviewed-by: Paulo Zanoni Reviewed-by: Damien Lespiau damien.lespiau@intel.com Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_psr.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index bc5ea2a6cf4c..9c64807e88cf 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -191,9 +191,6 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp) aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0); - drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, - DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE); - /* Enable AUX frame sync at sink */ if (dev_priv->psr.aux_frame_sync) drm_dp_dpcd_writeb(&intel_dp->aux, From 921ec285a6589cf3beb7f56a70744f75b09349f8 Mon Sep 17 00:00:00 2001 From: Rodrigo Vivi Date: Wed, 18 Nov 2015 11:21:12 -0800 Subject: [PATCH 07/70] drm/i915: PSR: Let's rely more on frontbuffer tracking. The ultimate goal here is to remove the dependency we currently have on audio driver power to get PSR working. Since with audio driver runtime PM disabled the Hardware tracking believes graphics is fully active and prevent PSR Entry, or in other words continuously exit PSR. So, the idea is to transfer the PSR exit responsability from the HW tracking to the SW tracking (frontbuffer tracking), who is really mature right now. However with LPSP masked out there might be cases where we could miss exit from HW tracking since it can be relying on this, like a specific case reported at our mailing list who user reported he would miss screen updates if scrolling firefox in a Gnome environment when i915 runtimepm was enabled. So before masking out LPSP again to make us independent from the audio driver we need to make sure that all our cases are coverred from the frontbuffer tracking perspective, where the flush means invalidate and flush. Without this patch for HSW, BDW and SKL we just do the invalidate part when the flush wasn't originated by a page flip because we were trusting the HW tracking for the flip case. So let's rely more on frontbuffer tracking and do the invalidation regardless the origin as expected for all platforms. v2: Improve commit message as suggested by Paulo. v3: Another attempt to let commit message more clear. Cc: Paulo Zanoni Signed-off-by: Rodrigo Vivi Reviewed-by: Paulo Zanoni Reviewed-by: Damien Lespiau damien.lespiau@intel.com Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_psr.c | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 9c64807e88cf..452ac3745560 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -734,25 +734,9 @@ void intel_psr_flush(struct drm_device *dev, frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe); dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits; - if (HAS_DDI(dev)) { - /* - * By definition every flush should mean invalidate + flush, - * however on core platforms let's minimize the - * disable/re-enable so we can avoid the invalidate when flip - * originated the flush. - */ - if (frontbuffer_bits && origin != ORIGIN_FLIP) - intel_psr_exit(dev); - } else { - /* - * On Valleyview and Cherryview we don't use hardware tracking - * so any plane updates or cursor moves don't result in a PSR - * invalidating. Which means we need to manually fake this in - * software for all flushes. - */ - if (frontbuffer_bits) - intel_psr_exit(dev); - } + /* By definition flush = invalidate + flush */ + if (frontbuffer_bits) + intel_psr_exit(dev); if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits) if (!work_busy(&dev_priv->psr.work.work)) From bb929cbc1f58c72eaf7981281dbb024ad92ef24d Mon Sep 17 00:00:00 2001 From: Rodrigo Vivi Date: Wed, 18 Nov 2015 11:21:55 -0800 Subject: [PATCH 08/70] drm/i915: PSR: Mask LPSP hw tracking back again. When we introduced PSR we let LPSP masked allowing us to get PSR independently from the audio runtime PM. However in one of the attempts to get PSR enabled by default one user reported one specific case where he would miss screen updates if scrolling the firefox in a Gnome environment when i915 runtime pm was enabled. So for this specific case that (I could never create an i-g-t test case) we decided to remove the LPSP mask and let HW tracking taking care of this case. The mask got removed later by my commit 09108b90f04 ("drm/i915: PSR: Remove Low Power HW tracking mask.") So we started depending on audio driver again, what is bad. With previous commit "drm/i915: PSR: Let's rely more on frontbuffer tracking." we transfered the PSR exit responsability totally to SW frontbuffer tracking. So now can safelly shut off a bit the HW tracking, or at least this case that makes us to depend on other drivers. v2: Update commit message since this patch by itself doesn't solve the bugzilla entries. v3: Another attempt to improve commit message. Cc: Paulo Zanoni Tested-by: Brian Norris Signed-off-by: Rodrigo Vivi Reviewed-by: Paulo Zanoni Reviewed-by: Damien Lespiau damien.lespiau@intel.com Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_psr.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 452ac3745560..a02dd3015b91 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -411,9 +411,14 @@ void intel_psr_enable(struct intel_dp *intel_dp) skl_psr_setup_su_vsc(intel_dp); } - /* Avoid continuous PSR exit by masking memup and hpd */ + /* + * Per Spec: Avoid continuous PSR exit by masking MEMUP and HPD. + * Also mask LPSP to avoid dependency on other drivers that + * might block runtime_pm besides preventing other hw tracking + * issues now we can rely on frontbuffer tracking. + */ I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP | - EDP_PSR_DEBUG_MASK_HPD); + EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP); /* Enable PSR on the panel */ hsw_psr_enable_sink(intel_dp); From 05eec3c2709d8966cbfcc7cd395f37889c492678 Mon Sep 17 00:00:00 2001 From: Rodrigo Vivi Date: Mon, 23 Nov 2015 14:16:40 -0800 Subject: [PATCH 09/70] drm/i915: Remove PSR Perf Counter for SKL+ Whenever DMC firmware put the HW into DC State a bunch of registers including this perf counter is reset to 0. Even with PSR active and working we could still read "Performance_Counter: 0" what will misslead people to believe PSR is broken. For instance on SKL we can only see PC10 residency with screen on if PSR is working properly. However Performance_Counter was showing 0. Even if it restored properly on DC6 exit we don't want to give users the wrong impression that PSR is not working while we know for sure it is. So, it is better to remove this counter information while we don't have a better way to track PSR residency. Signed-off-by: Rodrigo Vivi Reviewed-by: Durgadoss R Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_debugfs.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index b28da6fd7540..a728ff11e389 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2582,8 +2582,11 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) } seq_puts(m, "\n"); - /* CHV PSR has no kind of performance counter */ - if (HAS_DDI(dev)) { + /* + * VLV/CHV PSR has no kind of performance counter + * SKL+ Perf counter is reset to 0 everytime DC state is entered + */ + if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { psrperf = I915_READ(EDP_PSR_PERF_CNT) & EDP_PSR_PERF_CNT_MASK; From b6e4d53490504ee43a765e71a8fd8337af137623 Mon Sep 17 00:00:00 2001 From: Rodrigo Vivi Date: Mon, 23 Nov 2015 14:19:32 -0800 Subject: [PATCH 10/70] drm/i915: Also disable PSR on Sink when disabling it on Source. It is not a bad idea to disable the PSR feature on Sink when we are disabling on the Source. v2: Move dpcd write inside mutex protected area as suggested by Sonika. Cc: Sonika Jindal Suggested-by: Sonika Jindal Signed-off-by: Rodrigo Vivi Reviewed-by: Sonika Jindal Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_psr.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index a02dd3015b91..b6609e648f75 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -524,11 +524,15 @@ void intel_psr_disable(struct intel_dp *intel_dp) return; } + /* Disable PSR on Source */ if (HAS_DDI(dev)) hsw_psr_disable(intel_dp); else vlv_psr_disable(intel_dp); + /* Disable PSR on Sink */ + drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0); + dev_priv->psr.enabled = NULL; mutex_unlock(&dev_priv->psr.lock); From 3860b2ecfc96251455091739b92217e4f01b96c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Fri, 20 Nov 2015 22:09:18 +0200 Subject: [PATCH 11/70] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We still get spurious pipe underruns on ILK/SNB/IVB under two circumstances when dealing with PCH ports: * When the pipe has been disabled, but FDI RX/TX is still enabled * During FDI link training Both cases seem to happen at least when we do VGA+HDMI cloning from the same pipe. I don't think I've seen them when not cloning, but can't be 100% sure. Disable underrun reporting around those places to eliminate the dmesg errors. Testcase: igt/kms_setmode/basic-clone-single-crtc Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448050160-14124-1-git-send-email-ville.syrjala@linux.intel.com Acked-by: Chris Wilson --- drivers/gpu/drm/i915/intel_display.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 33cf197df753..9c8718366cc2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4137,6 +4137,12 @@ static void ironlake_pch_enable(struct drm_crtc *crtc) I915_WRITE(FDI_RX_TUSIZE1(pipe), I915_READ(PIPE_DATA_M1(pipe)) & TU_SIZE_MASK); + /* + * Sometimes spurious CPU pipe underruns happen during FDI + * training, at least with VGA+HDMI cloning. Suppress them. + */ + intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); + /* For PCH output, training FDI link */ dev_priv->display.fdi_link_train(crtc); @@ -4170,6 +4176,8 @@ static void ironlake_pch_enable(struct drm_crtc *crtc) intel_fdi_normal_train(crtc); + intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true); + /* For PCH DP, enable TRANS_DP_CTL */ if (HAS_PCH_CPT(dev) && intel_crtc->config->has_dp_encoder) { const struct drm_display_mode *adjusted_mode = @@ -5062,12 +5070,22 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) drm_crtc_vblank_off(crtc); assert_vblank_disabled(crtc); + /* + * Sometimes spurious CPU pipe underruns happen when the + * pipe is already disabled, but FDI RX/TX is still enabled. + * Happens at least with VGA+HDMI cloning. Suppress them. + */ + if (intel_crtc->config->has_pch_encoder) + intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); + intel_disable_pipe(intel_crtc); ironlake_pfit_disable(intel_crtc, false); - if (intel_crtc->config->has_pch_encoder) + if (intel_crtc->config->has_pch_encoder) { ironlake_fdi_disable(crtc); + intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true); + } for_each_encoder_on_crtc(dev, crtc, encoder) if (encoder->post_disable) From 7864578a70ac8af640b02d2caf3555aba9048f6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Fri, 20 Nov 2015 22:09:19 +0200 Subject: [PATCH 12/70] drm/i915: Disable CPU underruns around eDP port and vdd enable on ILK-IVB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We sometimes get a spurious CPU pipe underrun somewhere between enabling port A and enabling vdd for the panel. Observed on both ILK and IVB with port A eDP. Suppress FIFO underrun reporting around the port and vdd enable to avoid the dmesg errors. Not sure if port D eDP would suffer from the same issue, but assume that it doesn't until proven differently. Testcase: igt/kms_setmode Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448050160-14124-2-git-send-email-ville.syrjala@linux.intel.com Acked-by: Chris Wilson --- drivers/gpu/drm/i915/intel_dp.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index bec443a629da..2805f0d4ba45 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2697,6 +2697,15 @@ static void intel_enable_dp(struct intel_encoder *encoder) if (IS_VALLEYVIEW(dev)) vlv_init_panel_power_sequencer(intel_dp); + /* + * We get an occasional spurious underrun between the port + * enable and vdd enable, when enabling port A eDP. + * + * FIXME: Not sure if this applies to (PCH) port D eDP as well + */ + if (port == PORT_A) + intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); + intel_dp_enable_port(intel_dp); if (port == PORT_A && IS_GEN5(dev_priv)) { @@ -2714,6 +2723,9 @@ static void intel_enable_dp(struct intel_encoder *encoder) edp_panel_on(intel_dp); edp_panel_vdd_off(intel_dp, true); + if (port == PORT_A) + intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true); + pps_unlock(intel_dp); if (IS_VALLEYVIEW(dev)) { From 9bbc8258ae5914af1986561767d971417cee7a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Fri, 20 Nov 2015 22:09:20 +0200 Subject: [PATCH 13/70] drm/i915: Check for underruns after crtc disable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To get a better idea if underruns occurred during crtc disabling, let's check for them explicitly. This helps in cases where the error interrupt isn't active, or there is no underrun interrupt support at all. Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448050160-14124-3-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9c8718366cc2..3df22f0c84df 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13402,6 +13402,13 @@ static int intel_atomic_commit(struct drm_device *dev, dev_priv->display.crtc_disable(crtc); intel_crtc->active = false; intel_disable_shared_dpll(intel_crtc); + + /* + * Underruns don't always raise + * interrupts, so check manually. + */ + intel_check_cpu_fifo_underruns(dev_priv); + intel_check_pch_fifo_underruns(dev_priv); } } From 6c03a6bd0dd836db388feb28fda1868037491ee7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Fri, 20 Nov 2015 22:35:41 +0200 Subject: [PATCH 14/70] drm/i915: Don't register CRT connector when it's fused off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On some machines the CRT connector may be fused off. The weird thing about this setup is that the ADPA register works otherwise normally, except the enable bit is hardwired to 0. No one knows of any fuse register that would tell us if this is the case, so the only thing we can do (apart from a blacklist) is to try and set the enable bit and see if it sticks. If not, we don't register the connector at all. Obviously if the bit is already set when loading the driver we can just assume it works. I've smoke tested this approach on several machines (GMCH and PCH), some with actual CRT connectors, some with shadow connectors, and obviously the machine (IVB) with the fused off connector. So far I've not seen any ill effects from this probe. The main benefit is that we can actually run igt on machines with fused off connectors, without totally upsetting the state checker. Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448051741-22771-1-git-send-email-ville.syrjala@linux.intel.com Acked-by: Chris Wilson --- drivers/gpu/drm/i915/intel_crt.c | 33 ++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 27b3e610e8f0..12008af797bd 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -777,11 +777,37 @@ void intel_crt_init(struct drm_device *dev) struct intel_crt *crt; struct intel_connector *intel_connector; struct drm_i915_private *dev_priv = dev->dev_private; + i915_reg_t adpa_reg; + u32 adpa; /* Skip machines without VGA that falsely report hotplug events */ if (dmi_check_system(intel_no_crt)) return; + if (HAS_PCH_SPLIT(dev)) + adpa_reg = PCH_ADPA; + else if (IS_VALLEYVIEW(dev)) + adpa_reg = VLV_ADPA; + else + adpa_reg = ADPA; + + adpa = I915_READ(adpa_reg); + if ((adpa & ADPA_DAC_ENABLE) == 0) { + /* + * On some machines (some IVB at least) CRT can be + * fused off, but there's no known fuse bit to + * indicate that. On these machine the ADPA register + * works normally, except the DAC enable bit won't + * take. So the only way to tell is attempt to enable + * it and see what happens. + */ + I915_WRITE(adpa_reg, adpa | ADPA_DAC_ENABLE | + ADPA_HSYNC_CNTL_DISABLE | ADPA_VSYNC_CNTL_DISABLE); + if ((I915_READ(adpa_reg) & ADPA_DAC_ENABLE) == 0) + return; + I915_WRITE(adpa_reg, adpa); + } + crt = kzalloc(sizeof(struct intel_crt), GFP_KERNEL); if (!crt) return; @@ -815,12 +841,7 @@ void intel_crt_init(struct drm_device *dev) connector->interlace_allowed = 1; connector->doublescan_allowed = 0; - if (HAS_PCH_SPLIT(dev)) - crt->adpa_reg = PCH_ADPA; - else if (IS_VALLEYVIEW(dev)) - crt->adpa_reg = VLV_ADPA; - else - crt->adpa_reg = ADPA; + crt->adpa_reg = adpa_reg; crt->base.compute_config = intel_crt_compute_config; if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev)) { From 2e35c8833bc737a0c87a18e9b8c0de8495467d97 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Tue, 24 Nov 2015 18:47:25 +0200 Subject: [PATCH 15/70] drm/i915: remove duplicate definition of for_each_power_domain commit b04c5bd6fda54703e56f29569e4bca489d6c5a5c Author: Borun Fu Date: Sat Jul 12 10:02:27 2014 +0530 drm/i915: Power gating display wells during i915_pm_suspend moved for_each_power_domain from intel_display.c to i915_drv.h but we still have the definition around in intel_display.c, due to a merge conflict resolution gone wrong in commit 4dac3edfe68e5e1b3c2216b84ba160572420fa40 Merge: 487777673e35 e05444be705b Author: Daniel Vetter Date: Tue Jul 29 20:49:36 2014 +0200 Merge remote-tracking branch 'airlied/drm-next' into drm-intel-next Just remove the extra definition left behind. Reviewed-by: Daniel Vetter Signed-off-by: Jani Nikula Link: http://patchwork.freedesktop.org/patch/msgid/1448383645-7615-1-git-send-email-jani.nikula@intel.com --- drivers/gpu/drm/i915/intel_display.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3df22f0c84df..79fc520b018b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5232,10 +5232,6 @@ static enum intel_display_power_domain port_to_aux_power_domain(enum port port) } } -#define for_each_power_domain(domain, mask) \ - for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) \ - if ((1 << (domain)) & (mask)) - enum intel_display_power_domain intel_display_port_power_domain(struct intel_encoder *intel_encoder) { From 39bfcd5235e07e95ad3e70eab8e0b85db181de9e Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 26 Nov 2015 12:03:51 +0100 Subject: [PATCH 16/70] drm/i915: more virtual south bridge detection Commit "30c964a drm/i915: Detect virtual south bridge" detects and handles the southbridge emulated by vmware esx. Add the ich9 south bridge emulated by 'qemu -M q35'. Signed-off-by: Gerd Hoffmann Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.c | 3 ++- drivers/gpu/drm/i915/i915_drv.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 649e20aaa3ff..90faa8e03fca 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -565,7 +565,8 @@ void intel_detect_pch(struct drm_device *dev) DRM_DEBUG_KMS("Found SunrisePoint LP PCH\n"); WARN_ON(!IS_SKYLAKE(dev) && !IS_KABYLAKE(dev)); - } else if (id == INTEL_PCH_P2X_DEVICE_ID_TYPE) { + } else if ((id == INTEL_PCH_P2X_DEVICE_ID_TYPE) || + (id == INTEL_PCH_QEMU_DEVICE_ID_TYPE)) { dev_priv->pch_type = intel_virt_detect_pch(dev); } else continue; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3d8741eff7d3..439226d39437 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2609,6 +2609,7 @@ struct drm_i915_cmd_table { #define INTEL_PCH_SPT_DEVICE_ID_TYPE 0xA100 #define INTEL_PCH_SPT_LP_DEVICE_ID_TYPE 0x9D00 #define INTEL_PCH_P2X_DEVICE_ID_TYPE 0x7100 +#define INTEL_PCH_QEMU_DEVICE_ID_TYPE 0x2900 /* qemu q35 has 2918 */ #define INTEL_PCH_TYPE(dev) (__I915__(dev)->pch_type) #define HAS_PCH_SPT(dev) (INTEL_PCH_TYPE(dev) == PCH_SPT) From 6a39d7c986be4fd18eb019e9cdbf774ec36c9f77 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Wed, 25 Nov 2015 16:47:22 +0200 Subject: [PATCH 17/70] drm/i915: fix the SDE irq dmesg warnings properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We had the "The master control interrupt lied (SDE)!" check and error message in place for a long time without any problems, until commit aaf5ec2e51ab1d9c5e962b4728a1107ed3ff7a3e Author: Sonika Jindal Date: Wed Jul 8 17:07:47 2015 +0530 drm/i915: Handle HPD when it has actually occurred caused the errors to start happening. This was bisected and reported, but the error message was silenced in commit 97e5ed1111dcc5300a0f59a55248cd243937a8ab Author: Daniel Vetter Date: Fri Oct 23 10:56:12 2015 +0200 drm/i915: shut up gen8+ SDE irq dmesg noise shooting the messenger while the debugging for why Sonika's commit triggered the errors was still in progress. It looks like we need to read and acknowledge the PCH_PORT_HOTPLUG register even though the hotplug trigger indicates there isn't a hotplug irq to handle. The PCH doesn't seem to really ack the the interrupt to the CPU unless we touch the hotplug register. Cc: Ville Syrjälä Cc: Sonika Jindal Cc: Daniel Vetter Reviewed-by: Ville Syrjälä Acked-by: Daniel Vetter Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92084 Fixes: aaf5ec2e51ab ("drm/i915: Handle HPD when it has actually occurred") [Jani: added a comment and amended the commit message while applying] Signed-off-by: Jani Nikula Link: http://patchwork.freedesktop.org/patch/msgid/1448462843-32739-1-git-send-email-jani.nikula@intel.com --- drivers/gpu/drm/i915/i915_irq.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index c8ba94968aaf..671f76ccd8df 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1824,8 +1824,24 @@ static void ibx_hpd_irq_handler(struct drm_device *dev, u32 hotplug_trigger, struct drm_i915_private *dev_priv = to_i915(dev); u32 dig_hotplug_reg, pin_mask = 0, long_mask = 0; + /* + * Somehow the PCH doesn't seem to really ack the interrupt to the CPU + * unless we touch the hotplug register, even if hotplug_trigger is + * zero. Not acking leads to "The master control interrupt lied (SDE)!" + * errors. + */ dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG); + if (!hotplug_trigger) { + u32 mask = PORTA_HOTPLUG_STATUS_MASK | + PORTD_HOTPLUG_STATUS_MASK | + PORTC_HOTPLUG_STATUS_MASK | + PORTB_HOTPLUG_STATUS_MASK; + dig_hotplug_reg &= ~mask; + } + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg); + if (!hotplug_trigger) + return; intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger, dig_hotplug_reg, hpd, @@ -1840,8 +1856,7 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir) int pipe; u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK; - if (hotplug_trigger) - ibx_hpd_irq_handler(dev, hotplug_trigger, hpd_ibx); + ibx_hpd_irq_handler(dev, hotplug_trigger, hpd_ibx); if (pch_iir & SDE_AUDIO_POWER_MASK) { int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK) >> @@ -1934,8 +1949,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir) int pipe; u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT; - if (hotplug_trigger) - ibx_hpd_irq_handler(dev, hotplug_trigger, hpd_cpt); + ibx_hpd_irq_handler(dev, hotplug_trigger, hpd_cpt); if (pch_iir & SDE_AUDIO_POWER_MASK_CPT) { int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK_CPT) >> From 820da7ae46332fa709b171eb7ba57cbd023fa6df Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Wed, 25 Nov 2015 16:47:23 +0200 Subject: [PATCH 18/70] Revert "drm/i915: shut up gen8+ SDE irq dmesg noise" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 97e5ed1111dcc5300a0f59a55248cd243937a8ab Author: Daniel Vetter Date: Fri Oct 23 10:56:12 2015 +0200 drm/i915: shut up gen8+ SDE irq dmesg noise With the proper fix ("drm/i915: fix the SDE irq dmesg warnings properly") reliably in place, bring back the error message. Cc: Daniel Vetter Reviewed-by: Ville Syrjälä Acked-by: Daniel Vetter Signed-off-by: Jani Nikula Link: http://patchwork.freedesktop.org/patch/msgid/1448462843-32739-2-git-send-email-jani.nikula@intel.com --- drivers/gpu/drm/i915/i915_irq.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 671f76ccd8df..0677e87c89d7 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2365,13 +2365,9 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg) spt_irq_handler(dev, pch_iir); else cpt_irq_handler(dev, pch_iir); - } else { - /* - * Like on previous PCH there seems to be something - * fishy going on with forwarding PCH interrupts. - */ - DRM_DEBUG_DRIVER("The master control interrupt lied (SDE)!\n"); - } + } else + DRM_ERROR("The master control interrupt lied (SDE)!\n"); + } I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); From 144432618e4f9f3d24eaa9de0a1b6a5bee00989e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Mon, 23 Nov 2015 18:06:15 +0200 Subject: [PATCH 19/70] drm/i915: Make ibx_{enable,disable}_display_interrupt() static inlines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No reason why ibx_{enable,disable}_display_interrupt() couldn't be static inlines instead of cpp macros. Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448294777-13722-2-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 439226d39437..5ca8524e0130 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2758,10 +2758,17 @@ ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask); void ibx_display_interrupt_update(struct drm_i915_private *dev_priv, uint32_t interrupt_mask, uint32_t enabled_irq_mask); -#define ibx_enable_display_interrupt(dev_priv, bits) \ - ibx_display_interrupt_update((dev_priv), (bits), (bits)) -#define ibx_disable_display_interrupt(dev_priv, bits) \ - ibx_display_interrupt_update((dev_priv), (bits), 0) +static inline void +ibx_enable_display_interrupt(struct drm_i915_private *dev_priv, uint32_t bits) +{ + ibx_display_interrupt_update(dev_priv, bits, bits); +} +static inline void +ibx_disable_display_interrupt(struct drm_i915_private *dev_priv, uint32_t bits) +{ + ibx_display_interrupt_update(dev_priv, bits, 0); +} + /* i915_gem.c */ int i915_gem_create_ioctl(struct drm_device *dev, void *data, From fbdedaeaa7e43f3cf57617b713926eade2535202 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Mon, 23 Nov 2015 18:06:16 +0200 Subject: [PATCH 20/70] drm/i915: Make ironlake_{enable,disable}_display_irq() static inlines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ironlake_{enable,disable}_display_irq() each just call ilk_update_display_irq() so let's make them static inlines. While at it s/ironlake/ilk/ to make things shorter, and a bit more consistent with the ibx functions. Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448294777-13722-3-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++---- drivers/gpu/drm/i915/i915_irq.c | 24 ++++++---------------- drivers/gpu/drm/i915/intel_fifo_underrun.c | 8 ++++---- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5ca8524e0130..e0a2f9fcc8d8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2751,10 +2751,19 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv); void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv, uint32_t mask, uint32_t bits); -void -ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask); -void -ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask); +void ilk_update_display_irq(struct drm_i915_private *dev_priv, + uint32_t interrupt_mask, + uint32_t enabled_irq_mask); +static inline void +ilk_enable_display_irq(struct drm_i915_private *dev_priv, uint32_t bits) +{ + ilk_update_display_irq(dev_priv, bits, bits); +} +static inline void +ilk_disable_display_irq(struct drm_i915_private *dev_priv, uint32_t bits) +{ + ilk_update_display_irq(dev_priv, bits, 0); +} void ibx_display_interrupt_update(struct drm_i915_private *dev_priv, uint32_t interrupt_mask, uint32_t enabled_irq_mask); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 0677e87c89d7..db69bf54801a 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -215,9 +215,9 @@ void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv, * @interrupt_mask: mask of interrupt bits to update * @enabled_irq_mask: mask of interrupt bits to enable */ -static void ilk_update_display_irq(struct drm_i915_private *dev_priv, - uint32_t interrupt_mask, - uint32_t enabled_irq_mask) +void ilk_update_display_irq(struct drm_i915_private *dev_priv, + uint32_t interrupt_mask, + uint32_t enabled_irq_mask) { uint32_t new_val; @@ -239,18 +239,6 @@ static void ilk_update_display_irq(struct drm_i915_private *dev_priv, } } -void -ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask) -{ - ilk_update_display_irq(dev_priv, mask, mask); -} - -void -ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask) -{ - ilk_update_display_irq(dev_priv, mask, 0); -} - /** * ilk_update_gt_irq - update GTIMR * @dev_priv: driver private @@ -2655,7 +2643,7 @@ static int ironlake_enable_vblank(struct drm_device *dev, unsigned int pipe) DE_PIPE_VBLANK(pipe); spin_lock_irqsave(&dev_priv->irq_lock, irqflags); - ironlake_enable_display_irq(dev_priv, bit); + ilk_enable_display_irq(dev_priv, bit); spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); return 0; @@ -2710,7 +2698,7 @@ static void ironlake_disable_vblank(struct drm_device *dev, unsigned int pipe) DE_PIPE_VBLANK(pipe); spin_lock_irqsave(&dev_priv->irq_lock, irqflags); - ironlake_disable_display_irq(dev_priv, bit); + ilk_disable_display_irq(dev_priv, bit); spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); } @@ -3462,7 +3450,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev) * setup is guaranteed to run in single-threaded context. But we * need it to make the assert_spin_locked happy. */ spin_lock_irq(&dev_priv->irq_lock); - ironlake_enable_display_irq(dev_priv, DE_PCU_EVENT); + ilk_enable_display_irq(dev_priv, DE_PCU_EVENT); spin_unlock_irq(&dev_priv->irq_lock); } diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c index 7ae182d0594b..48bd079bdb06 100644 --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c @@ -128,9 +128,9 @@ static void ironlake_set_fifo_underrun_reporting(struct drm_device *dev, DE_PIPEB_FIFO_UNDERRUN; if (enable) - ironlake_enable_display_irq(dev_priv, bit); + ilk_enable_display_irq(dev_priv, bit); else - ironlake_disable_display_irq(dev_priv, bit); + ilk_disable_display_irq(dev_priv, bit); } static void ivybridge_check_fifo_underruns(struct intel_crtc *crtc) @@ -161,9 +161,9 @@ static void ivybridge_set_fifo_underrun_reporting(struct drm_device *dev, if (!ivb_can_enable_err_int(dev)) return; - ironlake_enable_display_irq(dev_priv, DE_ERR_INT_IVB); + ilk_enable_display_irq(dev_priv, DE_ERR_INT_IVB); } else { - ironlake_disable_display_irq(dev_priv, DE_ERR_INT_IVB); + ilk_disable_display_irq(dev_priv, DE_ERR_INT_IVB); if (old && I915_READ(GEN7_ERR_INT) & ERR_INT_FIFO_UNDERRUN(pipe)) { From 013d37520af735fd55f78e33edf55cc6fb3858eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Mon, 23 Nov 2015 18:06:17 +0200 Subject: [PATCH 21/70] drm/i915: Introduce bdw_{update,enable,disable}_pipe_irq() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pull the BDW+ DE pipe interrupt mask frobbing into a central place, like we have for other platforms. v2: Fix the kerneldoc (Daniel) Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448294777-13722-4-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 14 ++++++++ drivers/gpu/drm/i915/i915_irq.c | 41 ++++++++++++++++++---- drivers/gpu/drm/i915/intel_fifo_underrun.c | 8 ++--- 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e0a2f9fcc8d8..256a3bcc5f70 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2764,6 +2764,20 @@ ilk_disable_display_irq(struct drm_i915_private *dev_priv, uint32_t bits) { ilk_update_display_irq(dev_priv, bits, 0); } +void bdw_update_pipe_irq(struct drm_i915_private *dev_priv, + enum pipe pipe, + uint32_t interrupt_mask, + uint32_t enabled_irq_mask); +static inline void bdw_enable_pipe_irq(struct drm_i915_private *dev_priv, + enum pipe pipe, uint32_t bits) +{ + bdw_update_pipe_irq(dev_priv, pipe, bits, bits); +} +static inline void bdw_disable_pipe_irq(struct drm_i915_private *dev_priv, + enum pipe pipe, uint32_t bits) +{ + bdw_update_pipe_irq(dev_priv, pipe, bits, 0); +} void ibx_display_interrupt_update(struct drm_i915_private *dev_priv, uint32_t interrupt_mask, uint32_t enabled_irq_mask); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index db69bf54801a..62d885fa321a 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -437,6 +437,38 @@ static void bdw_update_port_irq(struct drm_i915_private *dev_priv, } } +/** + * bdw_update_pipe_irq - update DE pipe interrupt + * @dev_priv: driver private + * @pipe: pipe whose interrupt to update + * @interrupt_mask: mask of interrupt bits to update + * @enabled_irq_mask: mask of interrupt bits to enable + */ +void bdw_update_pipe_irq(struct drm_i915_private *dev_priv, + enum pipe pipe, + uint32_t interrupt_mask, + uint32_t enabled_irq_mask) +{ + uint32_t new_val; + + assert_spin_locked(&dev_priv->irq_lock); + + WARN_ON(enabled_irq_mask & ~interrupt_mask); + + if (WARN_ON(!intel_irqs_enabled(dev_priv))) + return; + + new_val = dev_priv->de_irq_mask[pipe]; + new_val &= ~interrupt_mask; + new_val |= (~enabled_irq_mask & interrupt_mask); + + if (new_val != dev_priv->de_irq_mask[pipe]) { + dev_priv->de_irq_mask[pipe] = new_val; + I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]); + POSTING_READ(GEN8_DE_PIPE_IMR(pipe)); + } +} + /** * ibx_display_interrupt_update - update SDEIMR * @dev_priv: driver private @@ -2668,10 +2700,9 @@ static int gen8_enable_vblank(struct drm_device *dev, unsigned int pipe) unsigned long irqflags; spin_lock_irqsave(&dev_priv->irq_lock, irqflags); - dev_priv->de_irq_mask[pipe] &= ~GEN8_PIPE_VBLANK; - I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]); - POSTING_READ(GEN8_DE_PIPE_IMR(pipe)); + bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK); spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + return 0; } @@ -2719,9 +2750,7 @@ static void gen8_disable_vblank(struct drm_device *dev, unsigned int pipe) unsigned long irqflags; spin_lock_irqsave(&dev_priv->irq_lock, irqflags); - dev_priv->de_irq_mask[pipe] |= GEN8_PIPE_VBLANK; - I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]); - POSTING_READ(GEN8_DE_PIPE_IMR(pipe)); + bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK); spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); } diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c index 48bd079bdb06..bda526660e20 100644 --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c @@ -178,14 +178,10 @@ static void broadwell_set_fifo_underrun_reporting(struct drm_device *dev, { struct drm_i915_private *dev_priv = dev->dev_private; - assert_spin_locked(&dev_priv->irq_lock); - if (enable) - dev_priv->de_irq_mask[pipe] &= ~GEN8_PIPE_FIFO_UNDERRUN; + bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); else - dev_priv->de_irq_mask[pipe] |= GEN8_PIPE_FIFO_UNDERRUN; - I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]); - POSTING_READ(GEN8_DE_PIPE_IMR(pipe)); + bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN); } static void ibx_set_fifo_underrun_reporting(struct drm_device *dev, From 81fd874e55035ea15144c00452fe4bbbc666bd64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 25 Nov 2015 16:21:30 +0200 Subject: [PATCH 22/70] drm/i915: Fix kerneldoc indent fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448461290-12333-1-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_guc_submission.c | 2 +- drivers/gpu/drm/i915/i915_irq.c | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index ed9f1002ab36..a057cbd78ecb 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -677,7 +677,7 @@ static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev, /** * gem_release_guc_obj() - Release gem object allocated for GuC usage * @obj: gem obj to be released - */ + */ static void gem_release_guc_obj(struct drm_i915_gem_object *obj) { if (!obj) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 62d885fa321a..e88d692583a5 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -288,11 +288,11 @@ static i915_reg_t gen6_pm_ier(struct drm_i915_private *dev_priv) } /** - * snb_update_pm_irq - update GEN6_PMIMR - * @dev_priv: driver private - * @interrupt_mask: mask of interrupt bits to update - * @enabled_irq_mask: mask of interrupt bits to enable - */ + * snb_update_pm_irq - update GEN6_PMIMR + * @dev_priv: driver private + * @interrupt_mask: mask of interrupt bits to update + * @enabled_irq_mask: mask of interrupt bits to enable + */ static void snb_update_pm_irq(struct drm_i915_private *dev_priv, uint32_t interrupt_mask, uint32_t enabled_irq_mask) @@ -406,11 +406,11 @@ void gen6_disable_rps_interrupts(struct drm_device *dev) } /** - * bdw_update_port_irq - update DE port interrupt - * @dev_priv: driver private - * @interrupt_mask: mask of interrupt bits to update - * @enabled_irq_mask: mask of interrupt bits to enable - */ + * bdw_update_port_irq - update DE port interrupt + * @dev_priv: driver private + * @interrupt_mask: mask of interrupt bits to update + * @enabled_irq_mask: mask of interrupt bits to enable + */ static void bdw_update_port_irq(struct drm_i915_private *dev_priv, uint32_t interrupt_mask, uint32_t enabled_irq_mask) From b13b8402523cb2717d4e9b9c39538b1fd48b29d5 Mon Sep 17 00:00:00 2001 From: Namrta Salonie Date: Fri, 27 Nov 2015 13:43:11 +0530 Subject: [PATCH 23/70] drm/i915: Fix possible null dereference in framebuffer_info debugfs function Found by static code analysis tool. v2: Inserted block instead of goto & renamed variables (Chris) v3: Aligned code as per the opening brace (Chris) Rebased on top of nightly (Daniel) Signed-off-by: Namrta Salonie Signed-off-by: Deepak S Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_debugfs.c | 34 +++++++++++++---------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index a728ff11e389..bfd57fb597dc 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1869,33 +1869,29 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data) { struct drm_info_node *node = m->private; struct drm_device *dev = node->minor->dev; - struct intel_fbdev *ifbdev = NULL; - struct intel_framebuffer *fb; + struct intel_framebuffer *fbdev_fb = NULL; struct drm_framebuffer *drm_fb; #ifdef CONFIG_DRM_FBDEV_EMULATION - struct drm_i915_private *dev_priv = dev->dev_private; + if (to_i915(dev)->fbdev) { + fbdev_fb = to_intel_framebuffer(to_i915(dev)->fbdev->helper.fb); - ifbdev = dev_priv->fbdev; - if (ifbdev) { - fb = to_intel_framebuffer(ifbdev->helper.fb); - - seq_printf(m, "fbcon size: %d x %d, depth %d, %d bpp, modifier 0x%llx, refcount %d, obj ", - fb->base.width, - fb->base.height, - fb->base.depth, - fb->base.bits_per_pixel, - fb->base.modifier[0], - atomic_read(&fb->base.refcount.refcount)); - describe_obj(m, fb->obj); - seq_putc(m, '\n'); - } + seq_printf(m, "fbcon size: %d x %d, depth %d, %d bpp, modifier 0x%llx, refcount %d, obj ", + fbdev_fb->base.width, + fbdev_fb->base.height, + fbdev_fb->base.depth, + fbdev_fb->base.bits_per_pixel, + fbdev_fb->base.modifier[0], + atomic_read(&fbdev_fb->base.refcount.refcount)); + describe_obj(m, fbdev_fb->obj); + seq_putc(m, '\n'); + } #endif mutex_lock(&dev->mode_config.fb_lock); drm_for_each_fb(drm_fb, dev) { - fb = to_intel_framebuffer(drm_fb); - if (ifbdev && &fb->base == ifbdev->helper.fb) + struct intel_framebuffer *fb = to_intel_framebuffer(drm_fb); + if (fb == fbdev_fb) continue; seq_printf(m, "user size: %d x %d, depth %d, %d bpp, modifier 0x%llx, refcount %d, obj ", From e3488e75b6d4d6dd2e9f43b1e1cb4bc11131c89a Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Fri, 27 Nov 2015 12:21:44 +0200 Subject: [PATCH 24/70] drm/i915/dsi: merge pre_pll_enable hook to pre_enable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For DSI, the pre_pll_enable and the pre_enable hooks are called back-to-back on all platforms that support DSI. The distinction is artificial for DSI, for which we enable the DSI PLL in the encoder hooks. Do everything in pre_enable, and remove DSI pre_pll_enable hook. Reviewed-by: Ville Syrjälä Reviewed-by: Daniel Vetter Signed-off-by: Jani Nikula Link: http://patchwork.freedesktop.org/patch/msgid/1448619706-21293-1-git-send-email-jani.nikula@intel.com --- drivers/gpu/drm/i915/intel_dsi.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index efb5a27dd49c..08c46c49c481 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -462,6 +462,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder) intel_panel_enable_backlight(intel_dsi->attached_connector); } +static void intel_dsi_prepare(struct intel_encoder *intel_encoder); + static void intel_dsi_pre_enable(struct intel_encoder *encoder) { struct drm_device *dev = encoder->base.dev; @@ -474,6 +476,9 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder) DRM_DEBUG_KMS("\n"); + intel_dsi_prepare(encoder); + intel_enable_dsi_pll(encoder); + /* Panel Enable over CRC PMIC */ if (intel_dsi->gpio_panel) gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1); @@ -1026,15 +1031,6 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder) } } -static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder) -{ - DRM_DEBUG_KMS("\n"); - - intel_dsi_prepare(encoder); - intel_enable_dsi_pll(encoder); - -} - static enum drm_connector_status intel_dsi_detect(struct drm_connector *connector, bool force) { @@ -1154,9 +1150,7 @@ void intel_dsi_init(struct drm_device *dev) drm_encoder_init(dev, encoder, &intel_dsi_funcs, DRM_MODE_ENCODER_DSI); - /* XXX: very likely not all of these are needed */ intel_encoder->compute_config = intel_dsi_compute_config; - intel_encoder->pre_pll_enable = intel_dsi_pre_pll_enable; intel_encoder->pre_enable = intel_dsi_pre_enable; intel_encoder->enable = intel_dsi_enable_nop; intel_encoder->disable = intel_dsi_pre_disable; From 70656900ab5c33cc5aff4dd45ca42087cdeb464b Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Fri, 27 Nov 2015 12:21:45 +0200 Subject: [PATCH 25/70] drm/i915: remove pre_pll_enable hook from DDI/gen9+ crtc enable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hook was added to cater for DSI, but with the hooks rearranged on the DSI encoder side, this is no longer needed. It was a bit silly anyway to have two hooks called back-to-back. Reviewed-by: Ville Syrjälä Reviewed-by: Daniel Vetter Signed-off-by: Jani Nikula Link: http://patchwork.freedesktop.org/patch/msgid/1448619706-21293-2-git-send-email-jani.nikula@intel.com --- drivers/gpu/drm/i915/intel_display.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 79fc520b018b..cde354288aee 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4981,8 +4981,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true); for_each_encoder_on_crtc(dev, crtc, encoder) { - if (encoder->pre_pll_enable) - encoder->pre_pll_enable(encoder); if (encoder->pre_enable) encoder->pre_enable(encoder); } From a65347ba6a4e07d276377a62b9b76ee64a277197 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Fri, 27 Nov 2015 12:21:46 +0200 Subject: [PATCH 26/70] drm/i915: add has_dsi_encoder to crtc state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DSI has quite a few special cases, like DP, so add it to crtc state. This way we can get rid of a number of intel_pipe_has_type() checks for DSI. This isn't necessarily the prettiest way, but it's a step towards being aligned with what's being done with other encoders. Reviewed-by: Ville Syrjälä Reviewed-by: Daniel Vetter Signed-off-by: Jani Nikula Link: http://patchwork.freedesktop.org/patch/msgid/1448619706-21293-3-git-send-email-jani.nikula@intel.com --- drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++------------------ drivers/gpu/drm/i915/intel_drv.h | 3 ++ drivers/gpu/drm/i915/intel_dsi.c | 8 +++-- 3 files changed, 26 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index cde354288aee..b95c325f9a89 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2130,7 +2130,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc) * need the check. */ if (HAS_GMCH_DISPLAY(dev_priv->dev)) - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI)) + if (crtc->config->has_dsi_encoder) assert_dsi_pll_enabled(dev_priv); else assert_pll_enabled(dev_priv, pipe); @@ -4636,7 +4636,7 @@ static void intel_crtc_load_lut(struct drm_crtc *crtc) return; if (HAS_GMCH_DISPLAY(dev_priv->dev)) { - if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI)) + if (intel_crtc->config->has_dsi_encoder) assert_dsi_pll_enabled(dev_priv); else assert_pll_enabled(dev_priv, pipe); @@ -4946,7 +4946,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) int pipe = intel_crtc->pipe, hsw_workaround_pipe; struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc->state); - bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI); if (WARN_ON(intel_crtc->active)) return; @@ -4988,7 +4987,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) if (intel_crtc->config->has_pch_encoder) dev_priv->display.fdi_link_train(crtc); - if (!is_dsi) + if (!intel_crtc->config->has_dsi_encoder) intel_ddi_enable_pipe_clock(intel_crtc); if (INTEL_INFO(dev)->gen >= 9) @@ -5003,7 +5002,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) intel_crtc_load_lut(crtc); intel_ddi_set_pipe_settings(crtc); - if (!is_dsi) + if (!intel_crtc->config->has_dsi_encoder) intel_ddi_enable_transcoder_func(crtc); intel_update_watermarks(crtc); @@ -5012,7 +5011,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) if (intel_crtc->config->has_pch_encoder) lpt_pch_enable(crtc); - if (intel_crtc->config->dp_encoder_is_mst && !is_dsi) + if (intel_crtc->config->dp_encoder_is_mst) intel_ddi_set_vc_payload_alloc(crtc, true); assert_vblank_disabled(crtc); @@ -5123,7 +5122,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_encoder *encoder; enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder; - bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI); if (intel_crtc->config->has_pch_encoder) intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A, @@ -5142,7 +5140,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc) if (intel_crtc->config->dp_encoder_is_mst) intel_ddi_set_vc_payload_alloc(crtc, false); - if (!is_dsi) + if (!intel_crtc->config->has_dsi_encoder) intel_ddi_disable_transcoder_func(dev_priv, cpu_transcoder); if (INTEL_INFO(dev)->gen >= 9) @@ -5150,7 +5148,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc) else ironlake_pfit_disable(intel_crtc, false); - if (!is_dsi) + if (!intel_crtc->config->has_dsi_encoder) intel_ddi_disable_pipe_clock(intel_crtc); if (intel_crtc->config->has_pch_encoder) { @@ -6152,13 +6150,10 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_encoder *encoder; int pipe = intel_crtc->pipe; - bool is_dsi; if (WARN_ON(intel_crtc->active)) return; - is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI); - if (intel_crtc->config->has_dp_encoder) intel_dp_set_m_n(intel_crtc, M1_N1); @@ -6181,7 +6176,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) if (encoder->pre_pll_enable) encoder->pre_pll_enable(encoder); - if (!is_dsi) { + if (!intel_crtc->config->has_dsi_encoder) { if (IS_CHERRYVIEW(dev)) { chv_prepare_pll(intel_crtc, intel_crtc->config); chv_enable_pll(intel_crtc, intel_crtc->config); @@ -6307,7 +6302,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) if (encoder->post_disable) encoder->post_disable(encoder); - if (!intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI)) { + if (!intel_crtc->config->has_dsi_encoder) { if (IS_CHERRYVIEW(dev)) chv_disable_pll(dev_priv, pipe); else if (IS_VALLEYVIEW(dev)) @@ -7920,8 +7915,6 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc, int refclk, num_connectors = 0; intel_clock_t clock; bool ok; - bool is_dsi = false; - struct intel_encoder *encoder; const intel_limit_t *limit; struct drm_atomic_state *state = crtc_state->base.state; struct drm_connector *connector; @@ -7931,26 +7924,14 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc, memset(&crtc_state->dpll_hw_state, 0, sizeof(crtc_state->dpll_hw_state)); - for_each_connector_in_state(state, connector, connector_state, i) { - if (connector_state->crtc != &crtc->base) - continue; - - encoder = to_intel_encoder(connector_state->best_encoder); - - switch (encoder->type) { - case INTEL_OUTPUT_DSI: - is_dsi = true; - break; - default: - break; - } - - num_connectors++; - } - - if (is_dsi) + if (crtc_state->has_dsi_encoder) return 0; + for_each_connector_in_state(state, connector, connector_state, i) { + if (connector_state->crtc == &crtc->base) + num_connectors++; + } + if (!crtc_state->clock_set) { refclk = i9xx_get_refclk(crtc_state, num_connectors); @@ -12600,6 +12581,8 @@ intel_pipe_config_compare(struct drm_device *dev, } else PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2); + PIPE_CONF_CHECK_I(has_dsi_encoder); + PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hdisplay); PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_htotal); PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hblank_start); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 7127332ae31f..6bc640b93eb0 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -395,6 +395,9 @@ struct intel_crtc_state { * accordingly. */ bool has_dp_encoder; + /* DSI has special cases */ + bool has_dsi_encoder; + /* Whether we should send NULL infoframes. Required for audio. */ bool has_hdmi_sink; diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 08c46c49c481..230957f9f663 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -266,16 +266,18 @@ static inline bool is_cmd_mode(struct intel_dsi *intel_dsi) } static bool intel_dsi_compute_config(struct intel_encoder *encoder, - struct intel_crtc_state *config) + struct intel_crtc_state *pipe_config) { struct intel_dsi *intel_dsi = container_of(encoder, struct intel_dsi, base); struct intel_connector *intel_connector = intel_dsi->attached_connector; struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode; - struct drm_display_mode *adjusted_mode = &config->base.adjusted_mode; + struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; DRM_DEBUG_KMS("\n"); + pipe_config->has_dsi_encoder = true; + if (fixed_mode) intel_fixed_panel_mode(fixed_mode, adjusted_mode); @@ -704,6 +706,8 @@ static void intel_dsi_get_config(struct intel_encoder *encoder, u32 pclk = 0; DRM_DEBUG_KMS("\n"); + pipe_config->has_dsi_encoder = true; + /* * DPLL_MD is not used in case of DSI, reading will get some default value * set dpll_md = 0 From 6b6985169017e554d80a8c2befdadbd81d13c2a2 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sat, 28 Nov 2015 11:05:39 +0100 Subject: [PATCH 27/70] drm/i915: fix fdi related fifo underruns on hsw MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similar to commit 37ca8d4ccd9860df0747aa2ea281a3c9c4bf8826 Author: Ville Syrjälä Date: Fri Oct 30 19:20:27 2015 +0200 drm/i915: Enable PCH FIFO underruns later on ILK/SNB/IVB we can only enable fifo underrun reporting when using the fdi/lpt after everything is set up and after a bit of waiting. The waiting is required, enabling it right after enabling encoders will first trigger an underrun on the pch and then, 1 frame later, an underrun on the cpu. Two vblank waits after encoder enabling seems enough to curb it. And similar to Author: Ville Syrjälä Date: Fri Nov 20 22:09:18 2015 +0200 drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB we also need to make sure cpu fifo underrun reporting is disabled when enabling the fdi rx/tx and pch transcoder&port. But somehow this is only needed when enabling, not also when disabling. Cc: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448705139-12534-1-git-send-email-daniel.vetter@ffwll.ch Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91578 Tested-by: Mika Kahola Reviewed-by: Mika Kahola Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b95c325f9a89..b3323262d9cd 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4978,7 +4978,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) intel_crtc->active = true; - intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true); + if (intel_crtc->config->has_pch_encoder) + intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); + else + intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true); + for_each_encoder_on_crtc(dev, crtc, encoder) { if (encoder->pre_enable) encoder->pre_enable(encoder); @@ -5022,9 +5026,13 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) intel_opregion_notify_encoder(encoder, true); } - if (intel_crtc->config->has_pch_encoder) + if (intel_crtc->config->has_pch_encoder) { + intel_wait_for_vblank(dev, pipe); + intel_wait_for_vblank(dev, pipe); + intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true); intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A, true); + } /* If we change the relative order between pipe/planes enabling, we need * to change the workaround. */ From cda0aaaf55b76e3515be1a9dcd7a1ca14d258ce5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Thu, 26 Nov 2015 18:27:07 +0200 Subject: [PATCH 28/70] drm/i915: Don't use crtc->config when reading out infoframe state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .get_config() hooks should not reference anything in crtc->config, everything should be based on the passed in pipe_config instead. So don't dig out the cpu_transcoder from crtc->config on ddi platfforms, and also avoid using the encoder->crtc link and instead look up the pipe via pipe_config->base.crtc. I don't think this will actually fix anything since during the initial state readout we set up the encoder->crtc link prior to calling .get_config(), and during the modeset state check the encoder->crtc ought to be correct anyway since it's that state we just programmed. But this seems the right thing to do anyway. While at it, do some house cleaning on the local variables in the .infoframe_enabled() hooks. Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448555227-31403-1-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ddi.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 3 +- drivers/gpu/drm/i915/intel_hdmi.c | 47 +++++++++++++++---------------- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 76ce7c2960b6..7f618cf5289c 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -3151,7 +3151,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder, pipe_config->has_hdmi_sink = true; intel_hdmi = enc_to_intel_hdmi(&encoder->base); - if (intel_hdmi->infoframe_enabled(&encoder->base)) + if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config)) pipe_config->has_infoframe = true; break; case TRANS_DDI_MODE_SELECT_DVI: diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 6bc640b93eb0..d82723b752cf 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -715,7 +715,8 @@ struct intel_hdmi { void (*set_infoframes)(struct drm_encoder *encoder, bool enable, const struct drm_display_mode *adjusted_mode); - bool (*infoframe_enabled)(struct drm_encoder *encoder); + bool (*infoframe_enabled)(struct drm_encoder *encoder, + const struct intel_crtc_state *pipe_config); }; struct intel_dp_mst_encoder; diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index bdd462e7c690..c3978bad5ca0 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -169,10 +169,10 @@ static void g4x_write_infoframe(struct drm_encoder *encoder, POSTING_READ(VIDEO_DIP_CTL); } -static bool g4x_infoframe_enabled(struct drm_encoder *encoder) +static bool g4x_infoframe_enabled(struct drm_encoder *encoder, + const struct intel_crtc_state *pipe_config) { - struct drm_device *dev = encoder->dev; - struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = to_i915(encoder->dev); struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder); u32 val = I915_READ(VIDEO_DIP_CTL); @@ -225,13 +225,13 @@ static void ibx_write_infoframe(struct drm_encoder *encoder, POSTING_READ(reg); } -static bool ibx_infoframe_enabled(struct drm_encoder *encoder) +static bool ibx_infoframe_enabled(struct drm_encoder *encoder, + const struct intel_crtc_state *pipe_config) { - struct drm_device *dev = encoder->dev; - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); + struct drm_i915_private *dev_priv = to_i915(encoder->dev); struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder); - i915_reg_t reg = TVIDEO_DIP_CTL(intel_crtc->pipe); + enum pipe pipe = to_intel_crtc(pipe_config->base.crtc)->pipe; + i915_reg_t reg = TVIDEO_DIP_CTL(pipe); u32 val = I915_READ(reg); if ((val & VIDEO_DIP_ENABLE) == 0) @@ -287,12 +287,12 @@ static void cpt_write_infoframe(struct drm_encoder *encoder, POSTING_READ(reg); } -static bool cpt_infoframe_enabled(struct drm_encoder *encoder) +static bool cpt_infoframe_enabled(struct drm_encoder *encoder, + const struct intel_crtc_state *pipe_config) { - struct drm_device *dev = encoder->dev; - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); - u32 val = I915_READ(TVIDEO_DIP_CTL(intel_crtc->pipe)); + struct drm_i915_private *dev_priv = to_i915(encoder->dev); + enum pipe pipe = to_intel_crtc(pipe_config->base.crtc)->pipe; + u32 val = I915_READ(TVIDEO_DIP_CTL(pipe)); if ((val & VIDEO_DIP_ENABLE) == 0) return false; @@ -341,13 +341,13 @@ static void vlv_write_infoframe(struct drm_encoder *encoder, POSTING_READ(reg); } -static bool vlv_infoframe_enabled(struct drm_encoder *encoder) +static bool vlv_infoframe_enabled(struct drm_encoder *encoder, + const struct intel_crtc_state *pipe_config) { - struct drm_device *dev = encoder->dev; - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); + struct drm_i915_private *dev_priv = to_i915(encoder->dev); struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder); - u32 val = I915_READ(VLV_TVIDEO_DIP_CTL(intel_crtc->pipe)); + enum pipe pipe = to_intel_crtc(pipe_config->base.crtc)->pipe; + u32 val = I915_READ(VLV_TVIDEO_DIP_CTL(pipe)); if ((val & VIDEO_DIP_ENABLE) == 0) return false; @@ -398,12 +398,11 @@ static void hsw_write_infoframe(struct drm_encoder *encoder, POSTING_READ(ctl_reg); } -static bool hsw_infoframe_enabled(struct drm_encoder *encoder) +static bool hsw_infoframe_enabled(struct drm_encoder *encoder, + const struct intel_crtc_state *pipe_config) { - struct drm_device *dev = encoder->dev; - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); - u32 val = I915_READ(HSW_TVIDEO_DIP_CTL(intel_crtc->config->cpu_transcoder)); + struct drm_i915_private *dev_priv = to_i915(encoder->dev); + u32 val = I915_READ(HSW_TVIDEO_DIP_CTL(pipe_config->cpu_transcoder)); return val & (VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW | @@ -927,7 +926,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder, if (tmp & HDMI_MODE_SELECT_HDMI) pipe_config->has_hdmi_sink = true; - if (intel_hdmi->infoframe_enabled(&encoder->base)) + if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config)) pipe_config->has_infoframe = true; if (tmp & SDVO_AUDIO_ENABLE) From 7905df29b959cb657bd07be4e481a86844c98b51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 25 Nov 2015 16:35:30 +0200 Subject: [PATCH 29/70] drm/i915: Use intel_pipe_will_have_type() in ironlake_crtc_compute_clock() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ironlake_crtc_compute_clock() gets called during atomic compute phase, so we must check the future pipe type instead of the current type. Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448462131-13321-1-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b3323262d9cd..598a6adbe1da 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8932,7 +8932,7 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc, memset(&crtc_state->dpll_hw_state, 0, sizeof(crtc_state->dpll_hw_state)); - is_lvds = intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS); + is_lvds = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS); WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)), "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev)); From 18024199579882265653bfe9e2b1a3dcb5697cd9 Mon Sep 17 00:00:00 2001 From: Matt Roper Date: Tue, 1 Dec 2015 09:26:58 -0800 Subject: [PATCH 30/70] drm/i915/bxt: Disable power well support BXT power well support is not yet stable. Starting with patch commit 9f836f9016ad5320e0c9230419d2102cf15a28aa Author: Patrik Jakobsson Date: Mon Nov 16 16:20:01 2015 +0100 drm/i915/gen9: Turn DC handling into a power well DPMS off operations may actually cause the entire system to powerdown or reboot. Disable power well support for now until Broxton gets fixes similar to what we have for SKL. Cc: Imre Deak Cc: Patrik Jakobsson References: http://lists.freedesktop.org/archives/intel-gfx/2015-November/081037.html Signed-off-by: Matt Roper Link: http://patchwork.freedesktop.org/patch/msgid/1448990818-11005-1-git-send-email-matthew.d.roper@intel.com Reviewed-by: Imre Deak --- drivers/gpu/drm/i915/intel_runtime_pm.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 3fa43af94946..ab833efb5f0c 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -1911,6 +1911,11 @@ sanitize_disable_power_well_option(const struct drm_i915_private *dev_priv, if (disable_power_well >= 0) return !!disable_power_well; + if (IS_BROXTON(dev_priv)) { + DRM_DEBUG_KMS("Disabling display power well support\n"); + return 0; + } + return 1; } From 2dd6982e471f68137afd864fb4de64c59c7f5388 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Tue, 1 Dec 2015 10:23:50 +0200 Subject: [PATCH 31/70] drm/i915/bxt: add support for setting backlight freq from vbt The only missing piece is the function to convert frequency to PWM register value. The PWM is based on 19.2 MHz clock, except for BXT A step, which is based on CDCLK, and which we ignore. Reviewed-by: Imre Deak Tested-by: Imre Deak Signed-off-by: Jani Nikula Link: http://patchwork.freedesktop.org/patch/msgid/1448958232-26520-1-git-send-email-jani.nikula@intel.com --- drivers/gpu/drm/i915/intel_panel.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index a24df35e11e7..2d1696386b0d 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1263,6 +1263,14 @@ static void intel_backlight_device_unregister(struct intel_connector *connector) } #endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */ +/* + * BXT: PWM clock frequency = 19.2 MHz. + */ +static u32 bxt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) +{ + return KHz(19200) / pwm_freq_hz; +} + /* * SPT: This value represents the period of the PWM stream in clock periods * multiplied by 16 (default increment) or 128 (alternate increment selected in @@ -1750,6 +1758,7 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel) panel->backlight.disable = bxt_disable_backlight; panel->backlight.set = bxt_set_backlight; panel->backlight.get = bxt_get_backlight; + panel->backlight.hz_to_pwm = bxt_hz_to_pwm; } else if (HAS_PCH_LPT(dev) || HAS_PCH_SPT(dev)) { panel->backlight.setup = lpt_setup_backlight; panel->backlight.enable = lpt_enable_backlight; From 5075222b04a28bff6bf1c66a7baced787be6b6de Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Tue, 1 Dec 2015 10:23:51 +0200 Subject: [PATCH 32/70] drm/i915: use default 200 Hz backlight frequency If the backlight modulation frequency can't be extracted from the registers or from VBT, use 200 Hz as the default. This may enable backlight on some machines that previously failed. Reviewed-by: Imre Deak Tested-by: Imre Deak Signed-off-by: Jani Nikula Link: http://patchwork.freedesktop.org/patch/msgid/1448958232-26520-2-git-send-email-jani.nikula@intel.com --- drivers/gpu/drm/i915/intel_panel.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 2d1696386b0d..2b25497333de 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1393,14 +1393,18 @@ static u32 get_backlight_max_vbt(struct intel_connector *connector) u16 pwm_freq_hz = dev_priv->vbt.backlight.pwm_freq_hz; u32 pwm; - if (!pwm_freq_hz) { - DRM_DEBUG_KMS("backlight frequency not specified in VBT\n"); + if (!panel->backlight.hz_to_pwm) { + DRM_DEBUG_KMS("backlight frequency conversion not supported\n"); return 0; } - if (!panel->backlight.hz_to_pwm) { - DRM_DEBUG_KMS("backlight frequency setting from VBT currently not supported on this platform\n"); - return 0; + if (pwm_freq_hz) { + DRM_DEBUG_KMS("VBT defined backlight frequency %u Hz\n", + pwm_freq_hz); + } else { + pwm_freq_hz = 200; + DRM_DEBUG_KMS("default backlight frequency %u Hz\n", + pwm_freq_hz); } pwm = panel->backlight.hz_to_pwm(connector, pwm_freq_hz); @@ -1409,8 +1413,6 @@ static u32 get_backlight_max_vbt(struct intel_connector *connector) return 0; } - DRM_DEBUG_KMS("backlight frequency %u Hz from VBT\n", pwm_freq_hz); - return pwm; } From d965e7ac7a196c27e299c598bcb678ab71132dc1 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Tue, 1 Dec 2015 10:23:52 +0200 Subject: [PATCH 33/70] drm/i915/bxt: backlight clock gating workaround MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per bspec, "Backlight PWM may stop in the asserted state, causing backlight to stay fully on. WA: Before disabling PWM, set CLKGATE_DIS_0 0x46530 bit 13 PWM1 Gating Dis (for PWM1) or bit 14 PWM2 Gating Dis (for PWM2). The bits can remain set without harm." (There's no workaround name for this.) This fixes some Broxton backlight issues. Signed-off-by: Imre Deak Reviewed-by: Ville Syrjälä [Jani: cleanup & commit message] Signed-off-by: Jani Nikula Link: http://patchwork.freedesktop.org/patch/msgid/1448958232-26520-3-git-send-email-jani.nikula@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 7 +++++++ drivers/gpu/drm/i915/intel_pm.c | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 1a12d44b9710..04439e888380 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2972,6 +2972,13 @@ enum skl_disp_power_wells { #define OGAMC1 _MMIO(0x30020) #define OGAMC0 _MMIO(0x30024) +/* + * GEN9 clock gating regs + */ +#define GEN9_CLKGATE_DIS_0 _MMIO(0x46530) +#define PWM2_GATING_DIS (1 << 14) +#define PWM1_GATING_DIS (1 << 13) + /* * Display engine regs */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 96f45d7b3e4b..612a8b462294 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -66,6 +66,14 @@ static void bxt_init_clock_gating(struct drm_device *dev) */ I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | GEN8_HDCUNIT_CLOCK_GATE_DISABLE_HDCREQ); + + /* + * Wa: Backlight PWM may stop in the asserted state, causing backlight + * to stay fully on. + */ + if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) + I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) | + PWM1_GATING_DIS | PWM2_GATING_DIS); } static void i915_pineview_get_mem_freq(struct drm_device *dev) From 245f586876eefc55438f0d65856fbe15de8dadca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Mon, 30 Nov 2015 16:23:42 +0200 Subject: [PATCH 34/70] drm/i915: Fix VBT backlight Hz to PWM conversion for PNV MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert the MHz number coming from intel_rawclk() into Hz in i9xx_hz_to_pwm() on PNV. Otherwise we'll get something totally bogus as a result. Fixes: aa17cdb4f836 ("drm/i915: initialize backlight max from VBT") Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448893432-6978-2-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/intel_panel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 2b25497333de..c3d3bba97651 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1343,7 +1343,7 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) int clock; if (IS_PINEVIEW(dev)) - clock = intel_hrawclk(dev); + clock = MHz(intel_hrawclk(dev)); else clock = 1000 * dev_priv->display.get_display_clock_speed(dev); From 83d83392e3e5432a9240e6b060119f01d27f2a30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Mon, 30 Nov 2015 16:23:43 +0200 Subject: [PATCH 35/70] drm/i915: Fix vbt PWM max setup for CTG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CTG uses hrawclk for backlight, so calculate the max based on that instead of cdclk. Fixes: aa17cdb4f836 ("drm/i915: initialize backlight max from VBT") Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448893432-6978-3-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/intel_panel.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index c3d3bba97651..d4beac3c19a4 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1352,13 +1352,19 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) /* * Gen4: This value represents the period of the PWM stream in display core - * clocks multiplied by 128. + * clocks ([DevCTG] HRAW clocks) multiplied by 128. + * */ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) { struct drm_device *dev = connector->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; - int clock = 1000 * dev_priv->display.get_display_clock_speed(dev); + int clock; + + if (IS_G4X(dev_priv)) + clock = MHz(intel_hrawclk(dev)); + else + clock = 1000 * dev_priv->display.get_display_clock_speed(dev); return clock / (pwm_freq_hz * 128); } From 56f5f7007dee07bdc109061c87a94eadd58e64b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Mon, 30 Nov 2015 16:23:44 +0200 Subject: [PATCH 36/70] drm/i915: Add HAS_PCH_LPT_H() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have HAS_PCH_LPT_LP() already, so add HAS_PCH_LPT_H() and use it where appropriate. Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448893432-6978-4-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_dp.c | 2 +- drivers/gpu/drm/i915/intel_panel.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 256a3bcc5f70..ba611c710860 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2615,6 +2615,7 @@ struct drm_i915_cmd_table { #define HAS_PCH_SPT(dev) (INTEL_PCH_TYPE(dev) == PCH_SPT) #define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT) #define HAS_PCH_LPT_LP(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) +#define HAS_PCH_LPT_H(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT) #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX) #define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 2805f0d4ba45..02bddb02e4da 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -711,7 +711,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index) if (index) return 0; return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000); - } else if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) { + } else if (HAS_PCH_LPT_H(dev_priv)) { /* Workaround for non-ULT HSW */ switch (index) { case 0: return 63; diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index d4beac3c19a4..eb42b388a5e9 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1308,7 +1308,7 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) else mul = 128; - if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) + if (HAS_PCH_LPT_H(dev_priv)) clock = MHz(135); /* LPT:H */ else clock = MHz(24); /* LPT:LP */ From a612c405563c44e61d28218741393d1dab8a6d4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Mon, 30 Nov 2015 16:23:45 +0200 Subject: [PATCH 37/70] drm/i915: Kill duplicated PNV .get_display_clock_speed() assignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Somehow we accumulated a duplicated .get_display_clock_speed() assignment for PNV in commit 34edce2fea69 ("drm/i915: Add cdclk extraction for g33, g965gm and g4x") No real harm on having two, we just never reach the second one, so simply kill it. Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448893432-6978-5-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Daniel Vetter Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 598a6adbe1da..4aa4cadeaf31 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14800,9 +14800,6 @@ static void intel_init_display(struct drm_device *dev) else if (IS_I945GM(dev) || IS_845G(dev)) dev_priv->display.get_display_clock_speed = i9xx_misc_get_display_clock_speed; - else if (IS_PINEVIEW(dev)) - dev_priv->display.get_display_clock_speed = - pnv_get_display_clock_speed; else if (IS_I915GM(dev)) dev_priv->display.get_display_clock_speed = i915gm_get_display_clock_speed; From fce18c4cf5fac09153dff8c8726d1eabd02e59ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Mon, 30 Nov 2015 16:23:46 +0200 Subject: [PATCH 38/70] drm/i915: Round the AUX clock divider to closest on all platforms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we round the AUX clock divider down on g4x, to closest on HSW/BDW port A, and up everywhere else. We are supposed to get as close to 2MHz as we can, so round to closest seems like the best option. Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448893432-6978-6-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/intel_dp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 02bddb02e4da..7e3ffc30a00e 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -681,7 +681,7 @@ static uint32_t i9xx_get_aux_clock_divider(struct intel_dp *intel_dp, int index) * The clock divider is based off the hrawclk, and would like to run at * 2MHz. So, take the hrawclk value and divide by 2 and use that */ - return index ? 0 : intel_hrawclk(dev) / 2; + return index ? 0 : DIV_ROUND_CLOSEST(intel_hrawclk(dev), 2); } static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index) @@ -694,10 +694,10 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index) return 0; if (intel_dig_port->port == PORT_A) { - return DIV_ROUND_UP(dev_priv->cdclk_freq, 2000); + return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000); } else { - return DIV_ROUND_UP(intel_pch_rawclk(dev), 2); + return DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2); } } @@ -719,7 +719,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index) default: return 0; } } else { - return index ? 0 : DIV_ROUND_UP(intel_pch_rawclk(dev), 2); + return index ? 0 : DIV_ROUND_CLOSEST(intel_pch_rawclk(dev), 2); } } From 4e7f8cfbd22755844094caaac2d9f49d2b77fd8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Mon, 30 Nov 2015 16:23:47 +0200 Subject: [PATCH 39/70] drm/i915: Use cached cdclk_freq for PWM calculations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No need to read out cdclk from the hardware, we have it already cached in dev_priv. Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1448893432-6978-7-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/intel_panel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index eb42b388a5e9..ae808b68a44f 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1345,7 +1345,7 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) if (IS_PINEVIEW(dev)) clock = MHz(intel_hrawclk(dev)); else - clock = 1000 * dev_priv->display.get_display_clock_speed(dev); + clock = 1000 * dev_priv->cdclk_freq; return clock / (pwm_freq_hz * 32); } @@ -1364,7 +1364,7 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) if (IS_G4X(dev_priv)) clock = MHz(intel_hrawclk(dev)); else - clock = 1000 * dev_priv->display.get_display_clock_speed(dev); + clock = 1000 * dev_priv->cdclk_freq; return clock / (pwm_freq_hz * 128); } From 65e472e43355488a4727db5dc9ce667d7d05b35e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Tue, 1 Dec 2015 23:28:55 +0200 Subject: [PATCH 40/70] drm/i915: Don't register the CRT connector when it's fused off on LPT-H MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LPT-H has a strap bit for fused off CRT block. Check it to see if we should register the CRT connector or not. Supposedly this also forces the ADAP enable bit to 0, so the detection we added in commit 6c03a6bd0dd8 ("drm/i915: Don't register CRT connector when it's fused off") should already catch it, but checking the fuse bit should at least do no harm. v2: Use HAS_PCH_LPT_H() (Paulo) Signed-off-by: Ville Syrjälä Reviewed-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/1449005335-15192-1-git-send-email-ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_display.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 04439e888380..d326c54ff667 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7556,6 +7556,7 @@ enum skl_disp_power_wells { #define SFUSE_STRAP _MMIO(0xc2014) #define SFUSE_STRAP_FUSE_LOCK (1<<13) #define SFUSE_STRAP_DISPLAY_DISABLED (1<<7) +#define SFUSE_STRAP_CRT_DISABLED (1<<6) #define SFUSE_STRAP_DDIB_DETECTED (1<<2) #define SFUSE_STRAP_DDIC_DETECTED (1<<1) #define SFUSE_STRAP_DDID_DETECTED (1<<0) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4aa4cadeaf31..bfb00e383587 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14257,6 +14257,9 @@ static bool intel_crt_present(struct drm_device *dev) if (IS_CHERRYVIEW(dev)) return false; + if (HAS_PCH_LPT_H(dev) && I915_READ(SFUSE_STRAP) & SFUSE_STRAP_CRT_DISABLED) + return false; + if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support) return false; From 70ac54d08244b9e9a416b0c76023dc6e9b73165e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Tue, 1 Dec 2015 23:29:56 +0200 Subject: [PATCH 41/70] drm/i915: Don't register CRT connector when DDI E can't be used MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On HSW/BDW DDI A and E share 2 lanes, so when DDI A requires the shared lanes DDI E can't be used. The lanes are not supposed to be dynamically switched between the two uses, so there's no point in registering the CRT connector when DDI E has no lanes. v2: Fix typos in the commit message (Paulo) Signed-off-by: Ville Syrjälä Reviewed-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/1449005396-15319-1-git-send-email-ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bfb00e383587..0ce988af1300 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14260,6 +14260,10 @@ static bool intel_crt_present(struct drm_device *dev) if (HAS_PCH_LPT_H(dev) && I915_READ(SFUSE_STRAP) & SFUSE_STRAP_CRT_DISABLED) return false; + /* DDI E can't be used if DDI A requires 4 lanes */ + if (HAS_DDI(dev) && I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) + return false; + if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support) return false; From e4abb733bb72ab447120d0887083b7dc4847c557 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Tue, 1 Dec 2015 23:31:33 +0200 Subject: [PATCH 42/70] drm/i915: Check VBT for CRT port presence on HSW/BDW MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unfortunatey there appear to quite a few HSW/BDW machines (eg. NUCs, Brix Pro) in the wild with LPT/WPT-H that have no physical CRT connector and non-working FDI. FDI training fails every single time on these machines. Dunno, maybe they just didn't bother wiring it up or something? Unfortunately all the fuse bits and whatnot are telling us that the CRT connector is present. And so what we get from this is tons of false positives from the CI systems due to VGA connector forcing. I've not found any way to detect this purely from hardware, so we have to resort to looking at the VBT int_crt_support bit. We used to check this bit on all platforms, but that broke all the old machines, so the check was then restricted to VLV only in commit 84b4e042c470 ("drm/i915: only apply crt_present check on VLV") Considering HSW and VLV VBT probably got defined around the same time, it should be reasonably safe to assume that the bits is sane for HSW/BDW as well. At least I have one copy of some VBT spec here that says it's meant for both VLV and HSW, and it knows about the bit (lists it being valid from version 155 onwards). Also I have two desktop machines with actual CRT ports and both have int_crt_support==1 in their VBTs. Also we already trust VBT >= 155 to tell us various details about the DDI ports, so trusting it a bit more seems reasonable. As far as VLV goes, the added VBT version check should be fine. Even if someone has some weird VLV machine with a very old VBT version, it just means they'll end up with a shadow CRT connector. IIRC the reason for eliminating the shadow CRT connector on VLV was to speed up display probing rather than fixing something more serious. v2: Move the platform checks into the VBT parsing code Also check that the VBT version is at least 155 v3: Improve commit message (Paulo) Cc: Chris Wilson Signed-off-by: Ville Syrjälä Reviewed-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/1449005493-15487-1-git-send-email-ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_bios.c | 5 ++++- drivers/gpu/drm/i915/intel_display.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index ce82f9c7df24..070470fe9a91 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -356,7 +356,10 @@ parse_general_features(struct drm_i915_private *dev_priv, general = find_section(bdb, BDB_GENERAL_FEATURES); if (general) { dev_priv->vbt.int_tv_support = general->int_tv_support; - dev_priv->vbt.int_crt_support = general->int_crt_support; + /* int_crt_support can't be trusted on earlier platforms */ + if (bdb->version >= 155 && + (HAS_DDI(dev_priv) || IS_VALLEYVIEW(dev_priv))) + dev_priv->vbt.int_crt_support = general->int_crt_support; dev_priv->vbt.lvds_use_ssc = general->enable_ssc; dev_priv->vbt.lvds_ssc_freq = intel_bios_ssc_frequency(dev, general->ssc_freq); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0ce988af1300..9f8ab09eb411 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14264,7 +14264,7 @@ static bool intel_crt_present(struct drm_device *dev) if (HAS_DDI(dev) && I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) return false; - if (IS_VALLEYVIEW(dev) && !dev_priv->vbt.int_crt_support) + if (!dev_priv->vbt.int_crt_support) return false; return true; From 79bd23dadf613fb5ebc1d9aa178fd7129c3e059c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Tue, 1 Dec 2015 23:32:07 +0200 Subject: [PATCH 43/70] drm/i915: Add "missing" break to haswell_get_ddi_pll() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While not technically needed on the last case in the switch statement, the 'break' makes it look better IMO. v2: Fixed a typo in the commit message (Paulo) Signed-off-by: Ville Syrjälä Reviewed-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/1449005527-15617-1-git-send-email-ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9f8ab09eb411..edc034dc38b2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9808,6 +9808,7 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv, break; case PORT_CLK_SEL_SPLL: pipe_config->shared_dpll = DPLL_ID_SPLL; + break; } } From 0aeb904888f52f48ca0cc7dfec2c2f540d2a762a Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Tue, 1 Dec 2015 16:29:25 +0200 Subject: [PATCH 44/70] drm/i915: simplify gmbus xfer error checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shorter, easier to follow code with no functional changes. In all cases, the return value ultimately comes from gmbus_wait_hw_status() anyway. Reviewed-by: Ville Syrjälä Signed-off-by: Jani Nikula Link: http://patchwork.freedesktop.org/patch/msgid/1448980166-23055-1-git-send-email-jani.nikula@intel.com --- drivers/gpu/drm/i915/intel_i2c.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 1110c83953cf..ccb522c176bd 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -505,17 +505,13 @@ retry: ret = gmbus_xfer_write(dev_priv, &msgs[i]); } + if (!ret) + ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_WAIT_PHASE, + GMBUS_HW_WAIT_EN); if (ret == -ETIMEDOUT) goto timeout; - if (ret == -ENXIO) + else if (ret) goto clear_err; - - ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_WAIT_PHASE, - GMBUS_HW_WAIT_EN); - if (ret == -ENXIO) - goto clear_err; - if (ret) - goto timeout; } /* Generate a STOP condition on the bus. Note that gmbus can't generata From bffce907d64042b5683ed3076e839d0249606264 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Tue, 1 Dec 2015 16:29:26 +0200 Subject: [PATCH 45/70] drm/i915: abstract i2c bit banging fallback in gmbus xfer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Choose between i2c bit banging and gmbus in a new higher level function, and let the i2c core retry the first time we fall back to bit banging. The i2c core imposes a timeout on -EAGAIN, but it defaults to 1 second, and shouldn't be a problem for us. Reviewed-by: Ville Syrjälä Signed-off-by: Jani Nikula Link: http://patchwork.freedesktop.org/patch/msgid/1448980166-23055-2-git-send-email-jani.nikula@intel.com --- drivers/gpu/drm/i915/intel_i2c.c | 39 ++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index ccb522c176bd..e26e22a72e3b 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -472,9 +472,7 @@ gmbus_xfer_index_read(struct drm_i915_private *dev_priv, struct i2c_msg *msgs) } static int -gmbus_xfer(struct i2c_adapter *adapter, - struct i2c_msg *msgs, - int num) +do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) { struct intel_gmbus *bus = container_of(adapter, struct intel_gmbus, @@ -483,14 +481,6 @@ gmbus_xfer(struct i2c_adapter *adapter, int i = 0, inc, try = 0; int ret = 0; - intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); - mutex_lock(&dev_priv->gmbus_mutex); - - if (bus->force_bit) { - ret = i2c_bit_algo.master_xfer(adapter, msgs, num); - goto out; - } - retry: I915_WRITE(GMBUS0, bus->reg0); @@ -585,13 +575,34 @@ timeout: bus->adapter.name, bus->reg0 & 0xff); I915_WRITE(GMBUS0, 0); - /* Hardware may not support GMBUS over these pins? Try GPIO bitbanging instead. */ + /* + * Hardware may not support GMBUS over these pins? Try GPIO bitbanging + * instead. Use EAGAIN to have i2c core retry. + */ bus->force_bit = 1; - ret = i2c_bit_algo.master_xfer(adapter, msgs, num); + ret = -EAGAIN; out: - mutex_unlock(&dev_priv->gmbus_mutex); + return ret; +} +static int +gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) +{ + struct intel_gmbus *bus = container_of(adapter, struct intel_gmbus, + adapter); + struct drm_i915_private *dev_priv = bus->dev_priv; + int ret; + + intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); + mutex_lock(&dev_priv->gmbus_mutex); + + if (bus->force_bit) + ret = i2c_bit_algo.master_xfer(adapter, msgs, num); + else + ret = do_gmbus_xfer(adapter, msgs, num); + + mutex_unlock(&dev_priv->gmbus_mutex); intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); return ret; From 90d5234fdbccbd03250a54acf1a1793654e7eebf Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Fri, 16 Oct 2015 16:44:43 -0300 Subject: [PATCH 46/70] drm/i915: fix the CFB size check In function find_compression_threshold() we try to over-allocate CFB space in order to reduce reallocations and fragmentation, and we're not considering that at the CFB size check. Consider it. There is also a longer-term plan to kill dev_priv->fbc.uncompressed_size, but this will come later. v2: Use drm_mm_node_allocated() (Chris). Reviewed-by: Chris Wilson Signed-off-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/ --- drivers/gpu/drm/i915/intel_fbc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 11fc5281e8ef..9eb94c04ce69 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -720,7 +720,8 @@ static int intel_fbc_setup_cfb(struct intel_crtc *crtc) size = intel_fbc_calculate_cfb_size(crtc); cpp = drm_format_plane_cpp(fb->pixel_format, 0); - if (size <= dev_priv->fbc.uncompressed_size) + if (drm_mm_node_allocated(&dev_priv->fbc.compressed_fb) && + size <= dev_priv->fbc.compressed_fb.size * dev_priv->fbc.threshold) return 0; /* Release any current block */ From e9c5fd26ac38ec64497b86d70472f8dd750410ad Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Tue, 13 Oct 2015 18:04:45 -0300 Subject: [PATCH 47/70] drm/i915: set dev_priv->fbc.crtc before scheduling the enable work This thing where we need to get the crtc either from the work structure or the fbc structure itself is confusing and unnecessary. Set fbc.crtc right when scheduling the enable work so we can always use it. The problem is not what gets passed and how to retrieve it. The problem is that when we're in the other parts of the code we always have to keep in mind that if FBC is already enabled we have to get the CRTC from place A, if FBC is scheduled we have to get the CRTC from place B, and if it's disabled there's no CRTC. Having a single place to retrieve the CRTC from allows us to treat the "is enabled" and "is scheduled" cases as the same case, reducing the mistake surface. I guess I should add this to the commit message. Besides the immediate advantages, this is also going to make one of the next commits much simpler. And even later, when we introduce enable/disable + activate/deactivate, this will be even simpler as we'll set the CRTC at enable time. So all the activate/deactivate/update code can just look at the single CRTC variable regardless of the current state. v2: Improve commit message (Chris). v3: Rebase after changing the patch order. Reviewed-by: Chris Wilson Signed-off-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/ --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/intel_fbc.c | 22 +++++++++------------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ba611c710860..e9ea7c44e492 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -921,7 +921,6 @@ struct i915_fbc { struct intel_fbc_work { struct delayed_work work; - struct intel_crtc *crtc; struct drm_framebuffer *fb; } *fbc_work; diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 9eb94c04ce69..70b55c0cf28e 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -334,14 +334,13 @@ bool intel_fbc_enabled(struct drm_i915_private *dev_priv) return dev_priv->fbc.enabled; } -static void intel_fbc_enable(struct intel_crtc *crtc, - const struct drm_framebuffer *fb) +static void intel_fbc_enable(const struct drm_framebuffer *fb) { - struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; + struct drm_i915_private *dev_priv = fb->dev->dev_private; + struct intel_crtc *crtc = dev_priv->fbc.crtc; dev_priv->fbc.enable_fbc(crtc); - dev_priv->fbc.crtc = crtc; dev_priv->fbc.fb_id = fb->base.id; dev_priv->fbc.y = crtc->base.y; } @@ -351,8 +350,8 @@ static void intel_fbc_work_fn(struct work_struct *__work) struct intel_fbc_work *work = container_of(to_delayed_work(__work), struct intel_fbc_work, work); - struct drm_i915_private *dev_priv = work->crtc->base.dev->dev_private; - struct drm_framebuffer *crtc_fb = work->crtc->base.primary->fb; + struct drm_i915_private *dev_priv = work->fb->dev->dev_private; + struct drm_framebuffer *crtc_fb = dev_priv->fbc.crtc->base.primary->fb; mutex_lock(&dev_priv->fbc.lock); if (work == dev_priv->fbc.fbc_work) { @@ -360,7 +359,7 @@ static void intel_fbc_work_fn(struct work_struct *__work) * the prior work. */ if (crtc_fb == work->fb) - intel_fbc_enable(work->crtc, work->fb); + intel_fbc_enable(work->fb); dev_priv->fbc.fbc_work = NULL; } @@ -400,15 +399,15 @@ static void intel_fbc_schedule_enable(struct intel_crtc *crtc) WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock)); intel_fbc_cancel_work(dev_priv); + dev_priv->fbc.crtc = crtc; work = kzalloc(sizeof(*work), GFP_KERNEL); if (work == NULL) { DRM_ERROR("Failed to allocate FBC work structure\n"); - intel_fbc_enable(crtc, crtc->base.primary->fb); + intel_fbc_enable(crtc->base.primary->fb); return; } - work->crtc = crtc; work->fb = crtc->base.primary->fb; INIT_DELAYED_WORK(&work->work, intel_fbc_work_fn); @@ -984,11 +983,8 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv, mutex_lock(&dev_priv->fbc.lock); - if (dev_priv->fbc.enabled) + if (dev_priv->fbc.enabled || dev_priv->fbc.fbc_work) fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe); - else if (dev_priv->fbc.fbc_work) - fbc_bits = INTEL_FRONTBUFFER_PRIMARY( - dev_priv->fbc.fbc_work->crtc->pipe); else fbc_bits = dev_priv->fbc.possible_framebuffer_bits; From 754d113304aa640b9b3ffa5535a1565b95e5fdb6 Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Tue, 13 Oct 2015 19:13:25 -0300 Subject: [PATCH 48/70] drm/i915: pass the crtc as an argument to intel_fbc_update() There's no need to reevaluate the status of every single crtc when a single crtc changes its state. With this, we're cutting the case where due to a change in pipe B, intel_fbc_update() is called, then intel_fbc_find_crtc() concludes FBC should be enabled on pipe A, then it completely rechecks the state of pipe A only to conclude FBC should remain enabled on pipe A. If any change on pipe A triggers a need to recompute whether FBC is valid on pipe A, then at some point someone is going to call intel_fbc_update(PIPE_A). The addition of intel_fbc_deactivate() is necessary so we keep track of the previously selected CRTC when we do invalidate/flush. We're also going to continue the enable/disable/activate/deactivate concept in the next patches. v2: Rebase. v3: Rebase after changing the patch order. Reviewed-by: Chris Wilson Signed-off-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/ --- drivers/gpu/drm/i915/intel_display.c | 3 +- drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/i915/intel_fbc.c | 68 ++++++++++++---------------- 3 files changed, 31 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index edc034dc38b2..353fd6827c4a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4792,7 +4792,6 @@ static void intel_post_plane_update(struct intel_crtc *crtc) { struct intel_crtc_atomic_commit *atomic = &crtc->atomic; struct drm_device *dev = crtc->base.dev; - struct drm_i915_private *dev_priv = dev->dev_private; if (atomic->wait_vblank) intel_wait_for_vblank(dev, crtc->pipe); @@ -4806,7 +4805,7 @@ static void intel_post_plane_update(struct intel_crtc *crtc) intel_update_watermarks(&crtc->base); if (atomic->update_fbc) - intel_fbc_update(dev_priv); + intel_fbc_update(crtc); if (atomic->post_enable_primary) intel_post_enable_primary(&crtc->base); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d82723b752cf..ae0a78b9a5c9 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1323,7 +1323,7 @@ static inline void intel_fbdev_restore_mode(struct drm_device *dev) /* intel_fbc.c */ bool intel_fbc_enabled(struct drm_i915_private *dev_priv); -void intel_fbc_update(struct drm_i915_private *dev_priv); +void intel_fbc_update(struct intel_crtc *crtc); void intel_fbc_init(struct drm_i915_private *dev_priv); void intel_fbc_disable(struct drm_i915_private *dev_priv); void intel_fbc_disable_crtc(struct intel_crtc *crtc); diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 70b55c0cf28e..263d450b9bd7 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -429,7 +429,7 @@ static void intel_fbc_schedule_enable(struct intel_crtc *crtc) schedule_delayed_work(&work->work, msecs_to_jiffies(50)); } -static void __intel_fbc_disable(struct drm_i915_private *dev_priv) +static void intel_fbc_deactivate(struct drm_i915_private *dev_priv) { WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock)); @@ -437,6 +437,11 @@ static void __intel_fbc_disable(struct drm_i915_private *dev_priv) if (dev_priv->fbc.enabled) dev_priv->fbc.disable_fbc(dev_priv); +} + +static void __intel_fbc_disable(struct drm_i915_private *dev_priv) +{ + intel_fbc_deactivate(dev_priv); dev_priv->fbc.crtc = NULL; } @@ -501,24 +506,6 @@ static bool crtc_is_valid(struct intel_crtc *crtc) return true; } -static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv) -{ - struct drm_crtc *crtc = NULL, *tmp_crtc; - enum pipe pipe; - - for_each_pipe(dev_priv, pipe) { - tmp_crtc = dev_priv->pipe_to_crtc_mapping[pipe]; - - if (crtc_is_valid(to_intel_crtc(tmp_crtc))) - crtc = tmp_crtc; - } - - if (!crtc) - return NULL; - - return crtc; -} - static bool multiple_pipes_ok(struct drm_i915_private *dev_priv) { enum pipe pipe; @@ -804,21 +791,28 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc) /** * __intel_fbc_update - enable/disable FBC as needed, unlocked - * @dev_priv: i915 device instance + * @crtc: the CRTC that triggered the update * * This function completely reevaluates the status of FBC, then enables, * disables or maintains it on the same state. */ -static void __intel_fbc_update(struct drm_i915_private *dev_priv) +static void __intel_fbc_update(struct intel_crtc *crtc) { - struct drm_crtc *drm_crtc = NULL; - struct intel_crtc *crtc; + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; struct drm_framebuffer *fb; struct drm_i915_gem_object *obj; const struct drm_display_mode *adjusted_mode; WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock)); + if (!multiple_pipes_ok(dev_priv)) { + set_no_fbc_reason(dev_priv, "more than one pipe active"); + goto out_disable; + } + + if (dev_priv->fbc.crtc != NULL && dev_priv->fbc.crtc != crtc) + return; + if (intel_vgpu_active(dev_priv->dev)) i915.enable_fbc = 0; @@ -832,18 +826,11 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) goto out_disable; } - drm_crtc = intel_fbc_find_crtc(dev_priv); - if (!drm_crtc) { + if (!crtc_is_valid(crtc)) { set_no_fbc_reason(dev_priv, "no output"); goto out_disable; } - if (!multiple_pipes_ok(dev_priv)) { - set_no_fbc_reason(dev_priv, "more than one pipe active"); - goto out_disable; - } - - crtc = to_intel_crtc(drm_crtc); fb = crtc->base.primary->fb; obj = intel_fb_obj(fb); adjusted_mode = &crtc->config->base.adjusted_mode; @@ -909,7 +896,8 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv) */ if (dev_priv->fbc.crtc == crtc && dev_priv->fbc.fb_id == fb->base.id && - dev_priv->fbc.y == crtc->base.y) + dev_priv->fbc.y == crtc->base.y && + dev_priv->fbc.enabled) return; if (intel_fbc_enabled(dev_priv)) { @@ -955,17 +943,19 @@ out_disable: /* * intel_fbc_update - enable/disable FBC as needed - * @dev_priv: i915 device instance + * @crtc: the CRTC that triggered the update * * This function reevaluates the overall state and enables or disables FBC. */ -void intel_fbc_update(struct drm_i915_private *dev_priv) +void intel_fbc_update(struct intel_crtc *crtc) { + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; + if (!fbc_supported(dev_priv)) return; mutex_lock(&dev_priv->fbc.lock); - __intel_fbc_update(dev_priv); + __intel_fbc_update(crtc); mutex_unlock(&dev_priv->fbc.lock); } @@ -991,7 +981,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv, dev_priv->fbc.busy_bits |= (fbc_bits & frontbuffer_bits); if (dev_priv->fbc.busy_bits) - __intel_fbc_disable(dev_priv); + intel_fbc_deactivate(dev_priv); mutex_unlock(&dev_priv->fbc.lock); } @@ -1009,9 +999,9 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv, dev_priv->fbc.busy_bits &= ~frontbuffer_bits; - if (!dev_priv->fbc.busy_bits) { - __intel_fbc_disable(dev_priv); - __intel_fbc_update(dev_priv); + if (!dev_priv->fbc.busy_bits && dev_priv->fbc.crtc) { + intel_fbc_deactivate(dev_priv); + __intel_fbc_update(dev_priv->fbc.crtc); } mutex_unlock(&dev_priv->fbc.lock); From 0e631adc1aefcae2a35c9d1649ecc263cb534df9 Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Wed, 14 Oct 2015 17:45:36 -0300 Subject: [PATCH 49/70] drm/i915: introduce is_active/activate/deactivate to the FBC terminology The long term goal is to have enable/disable as the higher level functions and activate/deactivate as the lower level functions, just like we do for PSR and for the CRTC. This way, we'll run enable and disable once per modeset, while update, activate and deactivate will be run many times. With this, we can move the checks and code that need to run only once per modeset to enable(), making the code simpler and possibly a little faster. This patch is just the first step on the conversion: it starts by converting the current low level functions from enable/disable to activate/deactivate. This patch by itself has no benefits other than making review and rebase easier. Please see the next patches for more details on the conversion. v2: - Rebase. - Improve commit message (Chris). v3: Rebase after changing the patch order. Reviewed-by: Chris Wilson Signed-off-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/ --- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 10 +-- drivers/gpu/drm/i915/intel_display.c | 4 +- drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/i915/intel_fbc.c | 114 +++++++++++++-------------- drivers/gpu/drm/i915/intel_pm.c | 2 +- 6 files changed, 66 insertions(+), 68 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index bfd57fb597dc..9cc57296cbb1 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1639,7 +1639,7 @@ static int i915_fbc_status(struct seq_file *m, void *unused) intel_runtime_pm_get(dev_priv); mutex_lock(&dev_priv->fbc.lock); - if (intel_fbc_enabled(dev_priv)) + if (intel_fbc_is_active(dev_priv)) seq_puts(m, "FBC enabled\n"); else seq_printf(m, "FBC disabled: %s\n", diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e9ea7c44e492..03d6823077bd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -915,9 +915,7 @@ struct i915_fbc { bool false_color; - /* Tracks whether the HW is actually enabled, not whether the feature is - * possible. */ - bool enabled; + bool active; struct intel_fbc_work { struct delayed_work work; @@ -926,9 +924,9 @@ struct i915_fbc { const char *no_fbc_reason; - bool (*fbc_enabled)(struct drm_i915_private *dev_priv); - void (*enable_fbc)(struct intel_crtc *crtc); - void (*disable_fbc)(struct drm_i915_private *dev_priv); + bool (*is_active)(struct drm_i915_private *dev_priv); + void (*activate)(struct intel_crtc *crtc); + void (*deactivate)(struct drm_i915_private *dev_priv); }; /** diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 353fd6827c4a..246a9f70bdc0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3174,8 +3174,8 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; - if (dev_priv->fbc.disable_fbc) - dev_priv->fbc.disable_fbc(dev_priv); + if (dev_priv->fbc.deactivate) + dev_priv->fbc.deactivate(dev_priv); dev_priv->display.update_primary_plane(crtc, fb, x, y); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index ae0a78b9a5c9..9d77e2aa327c 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1322,7 +1322,7 @@ static inline void intel_fbdev_restore_mode(struct drm_device *dev) #endif /* intel_fbc.c */ -bool intel_fbc_enabled(struct drm_i915_private *dev_priv); +bool intel_fbc_is_active(struct drm_i915_private *dev_priv); void intel_fbc_update(struct intel_crtc *crtc); void intel_fbc_init(struct drm_i915_private *dev_priv); void intel_fbc_disable(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 263d450b9bd7..d8486824291f 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -43,7 +43,7 @@ static inline bool fbc_supported(struct drm_i915_private *dev_priv) { - return dev_priv->fbc.enable_fbc != NULL; + return dev_priv->fbc.activate != NULL; } static inline bool fbc_on_pipe_a_only(struct drm_i915_private *dev_priv) @@ -64,11 +64,11 @@ static unsigned int get_crtc_fence_y_offset(struct intel_crtc *crtc) return crtc->base.y - crtc->adjusted_y; } -static void i8xx_fbc_disable(struct drm_i915_private *dev_priv) +static void i8xx_fbc_deactivate(struct drm_i915_private *dev_priv) { u32 fbc_ctl; - dev_priv->fbc.enabled = false; + dev_priv->fbc.active = false; /* Disable compression */ fbc_ctl = I915_READ(FBC_CONTROL); @@ -84,10 +84,10 @@ static void i8xx_fbc_disable(struct drm_i915_private *dev_priv) return; } - DRM_DEBUG_KMS("disabled FBC\n"); + DRM_DEBUG_KMS("deactivated FBC\n"); } -static void i8xx_fbc_enable(struct intel_crtc *crtc) +static void i8xx_fbc_activate(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; struct drm_framebuffer *fb = crtc->base.primary->fb; @@ -96,7 +96,7 @@ static void i8xx_fbc_enable(struct intel_crtc *crtc) int i; u32 fbc_ctl; - dev_priv->fbc.enabled = true; + dev_priv->fbc.active = true; /* Note: fbc.threshold == 1 for i8xx */ cfb_pitch = dev_priv->fbc.uncompressed_size / FBC_LL_SIZE; @@ -133,23 +133,23 @@ static void i8xx_fbc_enable(struct intel_crtc *crtc) fbc_ctl |= obj->fence_reg; I915_WRITE(FBC_CONTROL, fbc_ctl); - DRM_DEBUG_KMS("enabled FBC, pitch %d, yoff %d, plane %c\n", + DRM_DEBUG_KMS("activated FBC, pitch %d, yoff %d, plane %c\n", cfb_pitch, crtc->base.y, plane_name(crtc->plane)); } -static bool i8xx_fbc_enabled(struct drm_i915_private *dev_priv) +static bool i8xx_fbc_is_active(struct drm_i915_private *dev_priv) { return I915_READ(FBC_CONTROL) & FBC_CTL_EN; } -static void g4x_fbc_enable(struct intel_crtc *crtc) +static void g4x_fbc_activate(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; struct drm_framebuffer *fb = crtc->base.primary->fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); u32 dpfc_ctl; - dev_priv->fbc.enabled = true; + dev_priv->fbc.active = true; dpfc_ctl = DPFC_CTL_PLANE(crtc->plane) | DPFC_SR_EN; if (drm_format_plane_cpp(fb->pixel_format, 0) == 2) @@ -163,14 +163,14 @@ static void g4x_fbc_enable(struct intel_crtc *crtc) /* enable it... */ I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); - DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(crtc->plane)); + DRM_DEBUG_KMS("activated fbc on plane %c\n", plane_name(crtc->plane)); } -static void g4x_fbc_disable(struct drm_i915_private *dev_priv) +static void g4x_fbc_deactivate(struct drm_i915_private *dev_priv) { u32 dpfc_ctl; - dev_priv->fbc.enabled = false; + dev_priv->fbc.active = false; /* Disable compression */ dpfc_ctl = I915_READ(DPFC_CONTROL); @@ -178,11 +178,11 @@ static void g4x_fbc_disable(struct drm_i915_private *dev_priv) dpfc_ctl &= ~DPFC_CTL_EN; I915_WRITE(DPFC_CONTROL, dpfc_ctl); - DRM_DEBUG_KMS("disabled FBC\n"); + DRM_DEBUG_KMS("deactivated FBC\n"); } } -static bool g4x_fbc_enabled(struct drm_i915_private *dev_priv) +static bool g4x_fbc_is_active(struct drm_i915_private *dev_priv) { return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN; } @@ -194,7 +194,7 @@ static void intel_fbc_recompress(struct drm_i915_private *dev_priv) POSTING_READ(MSG_FBC_REND_STATE); } -static void ilk_fbc_enable(struct intel_crtc *crtc) +static void ilk_fbc_activate(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; struct drm_framebuffer *fb = crtc->base.primary->fb; @@ -203,7 +203,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) int threshold = dev_priv->fbc.threshold; unsigned int y_offset; - dev_priv->fbc.enabled = true; + dev_priv->fbc.active = true; dpfc_ctl = DPFC_CTL_PLANE(crtc->plane); if (drm_format_plane_cpp(fb->pixel_format, 0) == 2) @@ -239,14 +239,14 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) intel_fbc_recompress(dev_priv); - DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(crtc->plane)); + DRM_DEBUG_KMS("activated fbc on plane %c\n", plane_name(crtc->plane)); } -static void ilk_fbc_disable(struct drm_i915_private *dev_priv) +static void ilk_fbc_deactivate(struct drm_i915_private *dev_priv) { u32 dpfc_ctl; - dev_priv->fbc.enabled = false; + dev_priv->fbc.active = false; /* Disable compression */ dpfc_ctl = I915_READ(ILK_DPFC_CONTROL); @@ -254,16 +254,16 @@ static void ilk_fbc_disable(struct drm_i915_private *dev_priv) dpfc_ctl &= ~DPFC_CTL_EN; I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl); - DRM_DEBUG_KMS("disabled FBC\n"); + DRM_DEBUG_KMS("deactivated FBC\n"); } } -static bool ilk_fbc_enabled(struct drm_i915_private *dev_priv) +static bool ilk_fbc_is_active(struct drm_i915_private *dev_priv) { return I915_READ(ILK_DPFC_CONTROL) & DPFC_CTL_EN; } -static void gen7_fbc_enable(struct intel_crtc *crtc) +static void gen7_fbc_activate(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; struct drm_framebuffer *fb = crtc->base.primary->fb; @@ -271,7 +271,7 @@ static void gen7_fbc_enable(struct intel_crtc *crtc) u32 dpfc_ctl; int threshold = dev_priv->fbc.threshold; - dev_priv->fbc.enabled = true; + dev_priv->fbc.active = true; dpfc_ctl = 0; if (IS_IVYBRIDGE(dev_priv)) @@ -318,28 +318,28 @@ static void gen7_fbc_enable(struct intel_crtc *crtc) intel_fbc_recompress(dev_priv); - DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(crtc->plane)); + DRM_DEBUG_KMS("activated fbc on plane %c\n", plane_name(crtc->plane)); } /** - * intel_fbc_enabled - Is FBC enabled? + * intel_fbc_is_active - Is FBC active? * @dev_priv: i915 device instance * * This function is used to verify the current state of FBC. * FIXME: This should be tracked in the plane config eventually * instead of queried at runtime for most callers. */ -bool intel_fbc_enabled(struct drm_i915_private *dev_priv) +bool intel_fbc_is_active(struct drm_i915_private *dev_priv) { - return dev_priv->fbc.enabled; + return dev_priv->fbc.active; } -static void intel_fbc_enable(const struct drm_framebuffer *fb) +static void intel_fbc_activate(const struct drm_framebuffer *fb) { struct drm_i915_private *dev_priv = fb->dev->dev_private; struct intel_crtc *crtc = dev_priv->fbc.crtc; - dev_priv->fbc.enable_fbc(crtc); + dev_priv->fbc.activate(crtc); dev_priv->fbc.fb_id = fb->base.id; dev_priv->fbc.y = crtc->base.y; @@ -359,7 +359,7 @@ static void intel_fbc_work_fn(struct work_struct *__work) * the prior work. */ if (crtc_fb == work->fb) - intel_fbc_enable(work->fb); + intel_fbc_activate(work->fb); dev_priv->fbc.fbc_work = NULL; } @@ -391,7 +391,7 @@ static void intel_fbc_cancel_work(struct drm_i915_private *dev_priv) dev_priv->fbc.fbc_work = NULL; } -static void intel_fbc_schedule_enable(struct intel_crtc *crtc) +static void intel_fbc_schedule_activation(struct intel_crtc *crtc) { struct intel_fbc_work *work; struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; @@ -404,7 +404,7 @@ static void intel_fbc_schedule_enable(struct intel_crtc *crtc) work = kzalloc(sizeof(*work), GFP_KERNEL); if (work == NULL) { DRM_ERROR("Failed to allocate FBC work structure\n"); - intel_fbc_enable(crtc->base.primary->fb); + intel_fbc_activate(crtc->base.primary->fb); return; } @@ -435,8 +435,8 @@ static void intel_fbc_deactivate(struct drm_i915_private *dev_priv) intel_fbc_cancel_work(dev_priv); - if (dev_priv->fbc.enabled) - dev_priv->fbc.disable_fbc(dev_priv); + if (dev_priv->fbc.active) + dev_priv->fbc.deactivate(dev_priv); } static void __intel_fbc_disable(struct drm_i915_private *dev_priv) @@ -897,10 +897,10 @@ static void __intel_fbc_update(struct intel_crtc *crtc) if (dev_priv->fbc.crtc == crtc && dev_priv->fbc.fb_id == fb->base.id && dev_priv->fbc.y == crtc->base.y && - dev_priv->fbc.enabled) + dev_priv->fbc.active) return; - if (intel_fbc_enabled(dev_priv)) { + if (intel_fbc_is_active(dev_priv)) { /* We update FBC along two paths, after changing fb/crtc * configuration (modeswitching) and after page-flipping * finishes. For the latter, we know that not only did @@ -928,13 +928,13 @@ static void __intel_fbc_update(struct intel_crtc *crtc) __intel_fbc_disable(dev_priv); } - intel_fbc_schedule_enable(crtc); + intel_fbc_schedule_activation(crtc); dev_priv->fbc.no_fbc_reason = "FBC enabled (not necessarily active)"; return; out_disable: /* Multiple disables should be harmless */ - if (intel_fbc_enabled(dev_priv)) { + if (intel_fbc_is_active(dev_priv)) { DRM_DEBUG_KMS("unsupported config, disabling FBC\n"); __intel_fbc_disable(dev_priv); } @@ -973,7 +973,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv, mutex_lock(&dev_priv->fbc.lock); - if (dev_priv->fbc.enabled || dev_priv->fbc.fbc_work) + if (dev_priv->fbc.active || dev_priv->fbc.fbc_work) fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe); else fbc_bits = dev_priv->fbc.possible_framebuffer_bits; @@ -1018,7 +1018,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv) enum pipe pipe; mutex_init(&dev_priv->fbc.lock); - dev_priv->fbc.enabled = false; + dev_priv->fbc.active = false; if (!HAS_FBC(dev_priv)) { dev_priv->fbc.no_fbc_reason = "unsupported by this chipset"; @@ -1034,29 +1034,29 @@ void intel_fbc_init(struct drm_i915_private *dev_priv) } if (INTEL_INFO(dev_priv)->gen >= 7) { - dev_priv->fbc.fbc_enabled = ilk_fbc_enabled; - dev_priv->fbc.enable_fbc = gen7_fbc_enable; - dev_priv->fbc.disable_fbc = ilk_fbc_disable; + dev_priv->fbc.is_active = ilk_fbc_is_active; + dev_priv->fbc.activate = gen7_fbc_activate; + dev_priv->fbc.deactivate = ilk_fbc_deactivate; } else if (INTEL_INFO(dev_priv)->gen >= 5) { - dev_priv->fbc.fbc_enabled = ilk_fbc_enabled; - dev_priv->fbc.enable_fbc = ilk_fbc_enable; - dev_priv->fbc.disable_fbc = ilk_fbc_disable; + dev_priv->fbc.is_active = ilk_fbc_is_active; + dev_priv->fbc.activate = ilk_fbc_activate; + dev_priv->fbc.deactivate = ilk_fbc_deactivate; } else if (IS_GM45(dev_priv)) { - dev_priv->fbc.fbc_enabled = g4x_fbc_enabled; - dev_priv->fbc.enable_fbc = g4x_fbc_enable; - dev_priv->fbc.disable_fbc = g4x_fbc_disable; + dev_priv->fbc.is_active = g4x_fbc_is_active; + dev_priv->fbc.activate = g4x_fbc_activate; + dev_priv->fbc.deactivate = g4x_fbc_deactivate; } else { - dev_priv->fbc.fbc_enabled = i8xx_fbc_enabled; - dev_priv->fbc.enable_fbc = i8xx_fbc_enable; - dev_priv->fbc.disable_fbc = i8xx_fbc_disable; + dev_priv->fbc.is_active = i8xx_fbc_is_active; + dev_priv->fbc.activate = i8xx_fbc_activate; + dev_priv->fbc.deactivate = i8xx_fbc_deactivate; /* This value was pulled out of someone's hat */ I915_WRITE(FBC_CONTROL, 500 << FBC_CTL_INTERVAL_SHIFT); } /* We still don't have any sort of hardware state readout for FBC, so - * disable it in case the BIOS enabled it to make sure software matches - * the hardware state. */ - if (dev_priv->fbc.fbc_enabled(dev_priv)) - dev_priv->fbc.disable_fbc(dev_priv); + * deactivate it in case the BIOS activated it to make sure software + * matches the hardware state. */ + if (dev_priv->fbc.is_active(dev_priv)) + dev_priv->fbc.deactivate(dev_priv); } diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 612a8b462294..ee05ce8bf79a 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2430,7 +2430,7 @@ static void ilk_wm_merge(struct drm_device *dev, * enabled sometime later. */ if (IS_GEN5(dev) && !merged->fbc_wm_enabled && - intel_fbc_enabled(dev_priv)) { + intel_fbc_is_active(dev_priv)) { for (level = 2; level <= max_level; level++) { struct intel_wm_level *wm = &merged->wm[level]; From d029bcad6e4157be4551f396e6a417465eca3409 Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Thu, 15 Oct 2015 10:44:46 -0300 Subject: [PATCH 50/70] drm/i915: introduce intel_fbc_{enable,disable} The goal is to call FBC enable/disable only once per modeset, while activate/deactivate/update will be called multiple times. The enable() function will be responsible for deciding if a CRTC will have FBC on it and then it will "lock" FBC on this CRTC: it won't be possible to change FBC's CRTC until disable(). With this, all checks and resource acquisition that only need to be done once per modeset can be moved from update() to enable(). And then the update(), activate() and deactivate() code will also get simpler since they won't need to worry about the CRTC being changed. The disable() function will do the reverse operation of enable(). One of its features is that it should only be called while the pipe is already off. This guarantees that FBC is stopped and nothing is using the CFB. With this, the activate() and deactivate() functions just start and temporarily stop FBC. They are the ones touching the hardware enable bit, so HW state reflects dev_priv->crtc.active. The last function remaining is update(). A lot of times I thought about renaming update() to activate() or try_to_activate() since it's called when we want to activate FBC. The thing is that update() may not only decide to activate FBC, but also deactivate or keep it on the same state, so I'll leave this name for now. Moving code to enable() and disable() will also help in case we decide to move FBC to pipe_config or something else later. The current patch only puts the very basic code on enable() and disable(). The next commits will take care of moving more stuff from update() to the new functions. v2: - Rebase. - Improve commit message (Chris). v3: Rebase after changing the patch order. v4: Rebase again after upstream changes. Reviewed-by: Chris Wilson Signed-off-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/ --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 16 ++- drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_fbc.c | 196 +++++++++++++++++++-------- 4 files changed, 157 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 03d6823077bd..2493ad2f5c12 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -915,6 +915,7 @@ struct i915_fbc { bool false_color; + bool enabled; bool active; struct intel_fbc_work { diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 246a9f70bdc0..602e2be1c3d8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4820,7 +4820,7 @@ static void intel_pre_plane_update(struct intel_crtc *crtc) struct intel_crtc_atomic_commit *atomic = &crtc->atomic; if (atomic->disable_fbc) - intel_fbc_disable_crtc(crtc); + intel_fbc_deactivate(crtc); if (crtc->atomic.disable_ips) hsw_disable_ips(crtc); @@ -4928,6 +4928,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) if (intel_crtc->config->has_pch_encoder) intel_wait_for_vblank(dev, pipe); intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true); + + intel_fbc_enable(intel_crtc); } /* IPS only exists on ULT machines and is tied to pipe A. */ @@ -5040,6 +5042,8 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) intel_wait_for_vblank(dev, hsw_workaround_pipe); intel_wait_for_vblank(dev, hsw_workaround_pipe); } + + intel_fbc_enable(intel_crtc); } static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force) @@ -5120,6 +5124,8 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) } intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true); + + intel_fbc_disable_crtc(intel_crtc); } static void haswell_crtc_disable(struct drm_crtc *crtc) @@ -5170,6 +5176,8 @@ static void haswell_crtc_disable(struct drm_crtc *crtc) if (intel_crtc->config->has_pch_encoder) intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A, true); + + intel_fbc_disable_crtc(intel_crtc); } static void i9xx_pfit_enable(struct intel_crtc *crtc) @@ -6262,6 +6270,8 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) for_each_encoder_on_crtc(dev, crtc, encoder) encoder->enable(encoder); + + intel_fbc_enable(intel_crtc); } static void i9xx_pfit_disable(struct intel_crtc *crtc) @@ -6324,6 +6334,8 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) if (!IS_GEN2(dev)) intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); + + intel_fbc_disable_crtc(intel_crtc); } static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) @@ -11585,7 +11597,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, to_intel_plane(primary)->frontbuffer_bit); mutex_unlock(&dev->struct_mutex); - intel_fbc_disable_crtc(intel_crtc); + intel_fbc_deactivate(intel_crtc); intel_frontbuffer_flip_prepare(dev, to_intel_plane(primary)->frontbuffer_bit); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 9d77e2aa327c..3517cd1ce266 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1323,8 +1323,10 @@ static inline void intel_fbdev_restore_mode(struct drm_device *dev) /* intel_fbc.c */ bool intel_fbc_is_active(struct drm_i915_private *dev_priv); +void intel_fbc_deactivate(struct intel_crtc *crtc); void intel_fbc_update(struct intel_crtc *crtc); void intel_fbc_init(struct drm_i915_private *dev_priv); +void intel_fbc_enable(struct intel_crtc *crtc); void intel_fbc_disable(struct drm_i915_private *dev_priv); void intel_fbc_disable_crtc(struct intel_crtc *crtc); void intel_fbc_invalidate(struct drm_i915_private *dev_priv, diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index d8486824291f..6125c7b15966 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -399,7 +399,6 @@ static void intel_fbc_schedule_activation(struct intel_crtc *crtc) WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock)); intel_fbc_cancel_work(dev_priv); - dev_priv->fbc.crtc = crtc; work = kzalloc(sizeof(*work), GFP_KERNEL); if (work == NULL) { @@ -429,7 +428,7 @@ static void intel_fbc_schedule_activation(struct intel_crtc *crtc) schedule_delayed_work(&work->work, msecs_to_jiffies(50)); } -static void intel_fbc_deactivate(struct drm_i915_private *dev_priv) +static void __intel_fbc_deactivate(struct drm_i915_private *dev_priv) { WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock)); @@ -439,35 +438,13 @@ static void intel_fbc_deactivate(struct drm_i915_private *dev_priv) dev_priv->fbc.deactivate(dev_priv); } -static void __intel_fbc_disable(struct drm_i915_private *dev_priv) -{ - intel_fbc_deactivate(dev_priv); - dev_priv->fbc.crtc = NULL; -} - -/** - * intel_fbc_disable - disable FBC - * @dev_priv: i915 device instance - * - * This function disables FBC. - */ -void intel_fbc_disable(struct drm_i915_private *dev_priv) -{ - if (!fbc_supported(dev_priv)) - return; - - mutex_lock(&dev_priv->fbc.lock); - __intel_fbc_disable(dev_priv); - mutex_unlock(&dev_priv->fbc.lock); -} - /* - * intel_fbc_disable_crtc - disable FBC if it's associated with crtc + * intel_fbc_deactivate - deactivate FBC if it's associated with crtc * @crtc: the CRTC * - * This function disables FBC if it's associated with the provided CRTC. + * This function deactivates FBC if it's associated with the provided CRTC. */ -void intel_fbc_disable_crtc(struct intel_crtc *crtc) +void intel_fbc_deactivate(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; @@ -476,7 +453,7 @@ void intel_fbc_disable_crtc(struct intel_crtc *crtc) mutex_lock(&dev_priv->fbc.lock); if (dev_priv->fbc.crtc == crtc) - __intel_fbc_disable(dev_priv); + __intel_fbc_deactivate(dev_priv); mutex_unlock(&dev_priv->fbc.lock); } @@ -490,13 +467,18 @@ static void set_no_fbc_reason(struct drm_i915_private *dev_priv, DRM_DEBUG_KMS("Disabling FBC: %s\n", reason); } -static bool crtc_is_valid(struct intel_crtc *crtc) +static bool crtc_can_fbc(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; if (fbc_on_pipe_a_only(dev_priv) && crtc->pipe != PIPE_A) return false; + return true; +} + +static bool crtc_is_valid(struct intel_crtc *crtc) +{ if (!intel_crtc_active(&crtc->base)) return false; @@ -790,11 +772,11 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc) } /** - * __intel_fbc_update - enable/disable FBC as needed, unlocked + * __intel_fbc_update - activate/deactivate FBC as needed, unlocked * @crtc: the CRTC that triggered the update * - * This function completely reevaluates the status of FBC, then enables, - * disables or maintains it on the same state. + * This function completely reevaluates the status of FBC, then activates, + * deactivates or maintains it on the same state. */ static void __intel_fbc_update(struct intel_crtc *crtc) { @@ -810,22 +792,9 @@ static void __intel_fbc_update(struct intel_crtc *crtc) goto out_disable; } - if (dev_priv->fbc.crtc != NULL && dev_priv->fbc.crtc != crtc) + if (!dev_priv->fbc.enabled || dev_priv->fbc.crtc != crtc) return; - if (intel_vgpu_active(dev_priv->dev)) - i915.enable_fbc = 0; - - if (i915.enable_fbc < 0) { - set_no_fbc_reason(dev_priv, "disabled per chip default"); - goto out_disable; - } - - if (!i915.enable_fbc) { - set_no_fbc_reason(dev_priv, "disabled per module param"); - goto out_disable; - } - if (!crtc_is_valid(crtc)) { set_no_fbc_reason(dev_priv, "no output"); goto out_disable; @@ -924,8 +893,8 @@ static void __intel_fbc_update(struct intel_crtc *crtc) * disabling paths we do need to wait for a vblank at * some point. And we wait before enabling FBC anyway. */ - DRM_DEBUG_KMS("disabling active FBC for update\n"); - __intel_fbc_disable(dev_priv); + DRM_DEBUG_KMS("deactivating FBC for update\n"); + __intel_fbc_deactivate(dev_priv); } intel_fbc_schedule_activation(crtc); @@ -935,17 +904,17 @@ static void __intel_fbc_update(struct intel_crtc *crtc) out_disable: /* Multiple disables should be harmless */ if (intel_fbc_is_active(dev_priv)) { - DRM_DEBUG_KMS("unsupported config, disabling FBC\n"); - __intel_fbc_disable(dev_priv); + DRM_DEBUG_KMS("unsupported config, deactivating FBC\n"); + __intel_fbc_deactivate(dev_priv); } __intel_fbc_cleanup_cfb(dev_priv); } /* - * intel_fbc_update - enable/disable FBC as needed + * intel_fbc_update - activate/deactivate FBC as needed * @crtc: the CRTC that triggered the update * - * This function reevaluates the overall state and enables or disables FBC. + * This function reevaluates the overall state and activates or deactivates FBC. */ void intel_fbc_update(struct intel_crtc *crtc) { @@ -973,7 +942,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv, mutex_lock(&dev_priv->fbc.lock); - if (dev_priv->fbc.active || dev_priv->fbc.fbc_work) + if (dev_priv->fbc.enabled) fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe); else fbc_bits = dev_priv->fbc.possible_framebuffer_bits; @@ -981,7 +950,7 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv, dev_priv->fbc.busy_bits |= (fbc_bits & frontbuffer_bits); if (dev_priv->fbc.busy_bits) - intel_fbc_deactivate(dev_priv); + __intel_fbc_deactivate(dev_priv); mutex_unlock(&dev_priv->fbc.lock); } @@ -999,14 +968,128 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv, dev_priv->fbc.busy_bits &= ~frontbuffer_bits; - if (!dev_priv->fbc.busy_bits && dev_priv->fbc.crtc) { - intel_fbc_deactivate(dev_priv); + if (!dev_priv->fbc.busy_bits && dev_priv->fbc.enabled) { + __intel_fbc_deactivate(dev_priv); __intel_fbc_update(dev_priv->fbc.crtc); } mutex_unlock(&dev_priv->fbc.lock); } +/** + * intel_fbc_enable: tries to enable FBC on the CRTC + * @crtc: the CRTC + * + * This function checks if it's possible to enable FBC on the following CRTC, + * then enables it. Notice that it doesn't activate FBC. + */ +void intel_fbc_enable(struct intel_crtc *crtc) +{ + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; + + if (!fbc_supported(dev_priv)) + return; + + mutex_lock(&dev_priv->fbc.lock); + + if (dev_priv->fbc.enabled) { + WARN_ON(dev_priv->fbc.crtc == crtc); + goto out; + } + + WARN_ON(dev_priv->fbc.active); + WARN_ON(dev_priv->fbc.crtc != NULL); + + if (intel_vgpu_active(dev_priv->dev)) { + set_no_fbc_reason(dev_priv, "VGPU is active"); + goto out; + } + + if (i915.enable_fbc < 0) { + set_no_fbc_reason(dev_priv, "disabled per chip default"); + goto out; + } + + if (!i915.enable_fbc) { + set_no_fbc_reason(dev_priv, "disabled per module param"); + goto out; + } + + if (!crtc_can_fbc(crtc)) { + set_no_fbc_reason(dev_priv, "no enabled pipes can have FBC"); + goto out; + } + + DRM_DEBUG_KMS("Enabling FBC on pipe %c\n", pipe_name(crtc->pipe)); + dev_priv->fbc.no_fbc_reason = "FBC enabled but not active yet\n"; + + dev_priv->fbc.enabled = true; + dev_priv->fbc.crtc = crtc; +out: + mutex_unlock(&dev_priv->fbc.lock); +} + +/** + * __intel_fbc_disable - disable FBC + * @dev_priv: i915 device instance + * + * This is the low level function that actually disables FBC. Callers should + * grab the FBC lock. + */ +static void __intel_fbc_disable(struct drm_i915_private *dev_priv) +{ + struct intel_crtc *crtc = dev_priv->fbc.crtc; + + WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock)); + WARN_ON(!dev_priv->fbc.enabled); + WARN_ON(dev_priv->fbc.active); + assert_pipe_disabled(dev_priv, crtc->pipe); + + DRM_DEBUG_KMS("Disabling FBC on pipe %c\n", pipe_name(crtc->pipe)); + + dev_priv->fbc.enabled = false; + dev_priv->fbc.crtc = NULL; +} + +/** + * intel_fbc_disable_crtc - disable FBC if it's associated with crtc + * @crtc: the CRTC + * + * This function disables FBC if it's associated with the provided CRTC. + */ +void intel_fbc_disable_crtc(struct intel_crtc *crtc) +{ + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; + + if (!fbc_supported(dev_priv)) + return; + + mutex_lock(&dev_priv->fbc.lock); + if (dev_priv->fbc.crtc == crtc) { + WARN_ON(!dev_priv->fbc.enabled); + WARN_ON(dev_priv->fbc.active); + __intel_fbc_disable(dev_priv); + } + mutex_unlock(&dev_priv->fbc.lock); +} + +/** + * intel_fbc_disable - globally disable FBC + * @dev_priv: i915 device instance + * + * This function disables FBC regardless of which CRTC is associated with it. + */ +void intel_fbc_disable(struct drm_i915_private *dev_priv) +{ + if (!fbc_supported(dev_priv)) + return; + + mutex_lock(&dev_priv->fbc.lock); + if (dev_priv->fbc.enabled) + __intel_fbc_disable(dev_priv); + mutex_unlock(&dev_priv->fbc.lock); +} + /** * intel_fbc_init - Initialize FBC * @dev_priv: the i915 device @@ -1018,6 +1101,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv) enum pipe pipe; mutex_init(&dev_priv->fbc.lock); + dev_priv->fbc.enabled = false; dev_priv->fbc.active = false; if (!HAS_FBC(dev_priv)) { From c5ecd4691c797c098013da2bb6167d6bd93df6ae Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Thu, 15 Oct 2015 14:19:21 -0300 Subject: [PATCH 51/70] drm/i915: alloc/free the FBC CFB during enable/disable One of the problems with the current code is that it frees the CFB and releases its drm_mm node as soon as we flip FBC's enable bit. This is bad because after we disable FBC the hardware may still use the CFB for the rest of the frame, so in theory we should only release the drm_mm node one frame after we disable FBC. Otherwise, a stolen memory allocation done right after an FBC disable may result in either corrupted memory for the new owner of that memory region or corrupted screen/underruns in case the new owner changes it while the hardware is still reading it. This case is not exactly easy to reproduce since we currently don't do a lot of stolen memory allocations, but I see patches on the mailing list trying to expose stolen memory to user space, so races will be possible. I thought about three different approaches to solve this, and they all have downsides. The first approach would be to simply use multiple drm_mm nodes and freeing the unused ones only after a frame has passed. The problem with this approach is that since stolen memory is rather small, there's a risk we just won't be able to allocate a new CFB from stolen if the previous one was not freed yet. This could happen in case we quickly disable FBC from pipe A and decide to enable it on pipe B, or just if we change pipe A's fb stride while FBC is enabled. The second approach would be similar to the first one, but maintaining a single drm_mm node and keeping track of when it can be reused. This would remove the disadvantage of not having enough space for two nodes, but would create the new problem where we may not be able to enable FBC at the point intel_fbc_update() is called, so we would have to add more code to retry updating FBC after the time has passed. And that can quickly get too complex since we can get invalidate, flush, disable and other calls in the middle of the wait. Both solutions above - and also the current code - have the problem that we unnecessarily free+realloc FBC during invalidate+flush operations even if the CFB size doesn't change. The third option would be to move the allocation/deallocation to enable/disable. This makes sure that the pipe is always disabled when we allocate/deallocate the CFB, so there's no risk that the FBC hardware may read or write to the memory right after it is freed from drm_mm. The downside is that it is possible for user space to change the buffer stride without triggering a disable/enable - only deactivate/activate -, so we'll have to handle this case somehow - see igt's kms_frontbuffer_tracking test, fbc-stridechange subtest. It could be possible to implement a way to free+alloc the CFB during said stride change, but it would involve a lot of book-keeping - exactly as mentioned above - just for on case, so for now I'll keep it simple and just deactivate FBC. Besides, we may not even need to disable FBC since we do CFB over-allocation. Note from Chris: "Starting a fullscreen client that covers a single monitor in a multi-monitor setup will trigger a change in stride on one of the CRTCs (the monitors will be flipped independently).". It shouldn't be a huge problem if we lose FBC on multi-monitor setups since these setups already have problems reaching deep PC states anyway. v2: Rebase after changing the patch order. v3: - Remove references to the stride change case being "uncommon" and paste Chris' example. - Rebase after a change in a previous patch. Reviewed-by: Chris Wilson Signed-off-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/ --- drivers/gpu/drm/i915/intel_fbc.c | 134 ++++++++++++++++--------------- 1 file changed, 69 insertions(+), 65 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 6125c7b15966..958f9732dc69 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -64,6 +64,46 @@ static unsigned int get_crtc_fence_y_offset(struct intel_crtc *crtc) return crtc->base.y - crtc->adjusted_y; } +/* + * For SKL+, the plane source size used by the hardware is based on the value we + * write to the PLANE_SIZE register. For BDW-, the hardware looks at the value + * we wrote to PIPESRC. + */ +static void intel_fbc_get_plane_source_size(struct intel_crtc *crtc, + int *width, int *height) +{ + struct intel_plane_state *plane_state = + to_intel_plane_state(crtc->base.primary->state); + int w, h; + + if (intel_rotation_90_or_270(plane_state->base.rotation)) { + w = drm_rect_height(&plane_state->src) >> 16; + h = drm_rect_width(&plane_state->src) >> 16; + } else { + w = drm_rect_width(&plane_state->src) >> 16; + h = drm_rect_height(&plane_state->src) >> 16; + } + + if (width) + *width = w; + if (height) + *height = h; +} + +static int intel_fbc_calculate_cfb_size(struct intel_crtc *crtc, + struct drm_framebuffer *fb) +{ + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; + int lines; + + intel_fbc_get_plane_source_size(crtc, NULL, &lines); + if (INTEL_INFO(dev_priv)->gen >= 7) + lines = min(lines, 2048); + + /* Hardware needs the full buffer stride, not just the active area. */ + return lines * fb->pitches[0]; +} + static void i8xx_fbc_deactivate(struct drm_i915_private *dev_priv) { u32 fbc_ctl; @@ -558,11 +598,17 @@ again: } } -static int intel_fbc_alloc_cfb(struct drm_i915_private *dev_priv, int size, - int fb_cpp) +static int intel_fbc_alloc_cfb(struct intel_crtc *crtc) { + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; + struct drm_framebuffer *fb = crtc->base.primary->state->fb; struct drm_mm_node *uninitialized_var(compressed_llb); - int ret; + int size, fb_cpp, ret; + + WARN_ON(drm_mm_node_allocated(&dev_priv->fbc.compressed_fb)); + + size = intel_fbc_calculate_cfb_size(crtc, fb); + fb_cpp = drm_format_plane_cpp(fb->pixel_format, 0); ret = find_compression_threshold(dev_priv, &dev_priv->fbc.compressed_fb, size, fb_cpp); @@ -639,65 +685,6 @@ void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv) mutex_unlock(&dev_priv->fbc.lock); } -/* - * For SKL+, the plane source size used by the hardware is based on the value we - * write to the PLANE_SIZE register. For BDW-, the hardware looks at the value - * we wrote to PIPESRC. - */ -static void intel_fbc_get_plane_source_size(struct intel_crtc *crtc, - int *width, int *height) -{ - struct intel_plane_state *plane_state = - to_intel_plane_state(crtc->base.primary->state); - int w, h; - - if (intel_rotation_90_or_270(plane_state->base.rotation)) { - w = drm_rect_height(&plane_state->src) >> 16; - h = drm_rect_width(&plane_state->src) >> 16; - } else { - w = drm_rect_width(&plane_state->src) >> 16; - h = drm_rect_height(&plane_state->src) >> 16; - } - - if (width) - *width = w; - if (height) - *height = h; -} - -static int intel_fbc_calculate_cfb_size(struct intel_crtc *crtc) -{ - struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; - struct drm_framebuffer *fb = crtc->base.primary->fb; - int lines; - - intel_fbc_get_plane_source_size(crtc, NULL, &lines); - if (INTEL_INFO(dev_priv)->gen >= 7) - lines = min(lines, 2048); - - /* Hardware needs the full buffer stride, not just the active area. */ - return lines * fb->pitches[0]; -} - -static int intel_fbc_setup_cfb(struct intel_crtc *crtc) -{ - struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; - struct drm_framebuffer *fb = crtc->base.primary->fb; - int size, cpp; - - size = intel_fbc_calculate_cfb_size(crtc); - cpp = drm_format_plane_cpp(fb->pixel_format, 0); - - if (drm_mm_node_allocated(&dev_priv->fbc.compressed_fb) && - size <= dev_priv->fbc.compressed_fb.size * dev_priv->fbc.threshold) - return 0; - - /* Release any current block */ - __intel_fbc_cleanup_cfb(dev_priv); - - return intel_fbc_alloc_cfb(dev_priv, size, cpp); -} - static bool stride_is_valid(struct drm_i915_private *dev_priv, unsigned int stride) { @@ -853,8 +840,19 @@ static void __intel_fbc_update(struct intel_crtc *crtc) goto out_disable; } - if (intel_fbc_setup_cfb(crtc)) { - set_no_fbc_reason(dev_priv, "not enough stolen memory"); + /* It is possible for the required CFB size change without a + * crtc->disable + crtc->enable since it is possible to change the + * stride without triggering a full modeset. Since we try to + * over-allocate the CFB, there's a chance we may keep FBC enabled even + * if this happens, but if we exceed the current CFB size we'll have to + * disable FBC. Notice that it would be possible to disable FBC, wait + * for a frame, free the stolen node, then try to reenable FBC in case + * we didn't get any invalidate/deactivate calls, but this would require + * a lot of tracking just for a specific case. If we conclude it's an + * important case, we can implement it later. */ + if (intel_fbc_calculate_cfb_size(crtc, fb) > + dev_priv->fbc.compressed_fb.size * dev_priv->fbc.threshold) { + set_no_fbc_reason(dev_priv, "CFB requirements changed"); goto out_disable; } @@ -907,7 +905,6 @@ out_disable: DRM_DEBUG_KMS("unsupported config, deactivating FBC\n"); __intel_fbc_deactivate(dev_priv); } - __intel_fbc_cleanup_cfb(dev_priv); } /* @@ -1020,6 +1017,11 @@ void intel_fbc_enable(struct intel_crtc *crtc) goto out; } + if (intel_fbc_alloc_cfb(crtc)) { + set_no_fbc_reason(dev_priv, "not enough stolen memory"); + goto out; + } + DRM_DEBUG_KMS("Enabling FBC on pipe %c\n", pipe_name(crtc->pipe)); dev_priv->fbc.no_fbc_reason = "FBC enabled but not active yet\n"; @@ -1047,6 +1049,8 @@ static void __intel_fbc_disable(struct drm_i915_private *dev_priv) DRM_DEBUG_KMS("Disabling FBC on pipe %c\n", pipe_name(crtc->pipe)); + __intel_fbc_cleanup_cfb(dev_priv); + dev_priv->fbc.enabled = false; dev_priv->fbc.crtc = NULL; } From e6cd6dc104fc3611326bf69df92090b6b7c78f60 Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Fri, 16 Oct 2015 17:55:40 -0300 Subject: [PATCH 52/70] drm/i915: check for FBC planes in the same place as the pipes This moves the pre-gen4 check from update() to enable(). The HAS_DDI in the original code is not needed since only gen 2/3 have the plane swapping code. v2: Rebase. v3: Extract fbc_on_plane_a_only() (Chris). Reviewed-by: Chris Wilson Signed-off-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/ --- drivers/gpu/drm/i915/intel_fbc.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 958f9732dc69..8460e3d72b98 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -51,6 +51,11 @@ static inline bool fbc_on_pipe_a_only(struct drm_i915_private *dev_priv) return IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8; } +static inline bool fbc_on_plane_a_only(struct drm_i915_private *dev_priv) +{ + return INTEL_INFO(dev_priv)->gen < 4; +} + /* * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's @@ -514,6 +519,9 @@ static bool crtc_can_fbc(struct intel_crtc *crtc) if (fbc_on_pipe_a_only(dev_priv) && crtc->pipe != PIPE_A) return false; + if (fbc_on_plane_a_only(dev_priv) && crtc->plane != PLANE_A) + return false; + return true; } @@ -802,12 +810,6 @@ static void __intel_fbc_update(struct intel_crtc *crtc) goto out_disable; } - if ((INTEL_INFO(dev_priv)->gen < 4 || HAS_DDI(dev_priv)) && - crtc->plane != PLANE_A) { - set_no_fbc_reason(dev_priv, "FBC unsupported on plane"); - goto out_disable; - } - /* The use of a CPU fence is mandatory in order to detect writes * by the CPU to the scanout and trigger updates to the FBC. */ From 128d735606d4e43bc6a56432cf66a68c2753bdce Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Mon, 26 Oct 2015 16:27:49 -0200 Subject: [PATCH 53/70] drm/i915: use a single intel_fbc_work struct This was already on my TODO list, and was requested both by Chris and Ville, for different reasons. The advantages are avoiding a frequent malloc/free pair, and the locality of having the work structure embedded in dev_priv. The maximum used memory is also smaller since previously we could have multiple allocated intel_fbc_work structs at the same time, and now we'll always have a single one - the one embedded on dev_priv. Of course, we're now using a little more memory on the cases where there's nothing scheduled. The biggest challenge here is to keep everything synchronized the way it was before. Currently, when we try to activate FBC, we allocate a new intel_fbc_work structure. Then later when we conclude we must delay the FBC activation a little more, we allocate a new intel_fbc_work struct, and then adjust dev_priv->fbc.fbc_work to point to the new struct. So when the old work runs - at intel_fbc_work_fn() - it will check that dev_priv->fbc.fbc_work points to something else, so it does nothing. Everything is also protected by fbc.lock. Just cancelling the old delayed work doesn't work because we might just cancel it after the work function already started to run, but while it is still waiting to grab fbc.lock. That's why we use the "dev_priv->fbc.fbc_work == work" check described in the paragraph above. So now that we have a single work struct we have to introduce a new way to synchronize everything. So we're making the work function a normal work instead of a delayed work, and it will be responsible for sleeping the appropriate amount of time itself. This way, after it wakes up it can grab the lock, ask "were we delayed or cancelled?" and then go back to sleep, enable FBC or give up. v2: - Spelling fixes. - Rebase after changing the patch order. - Fix ms/jiffies confusion. Reviewed-by: Chris Wilson (v1) Signed-off-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/ --- drivers/gpu/drm/i915/i915_drv.h | 6 +- drivers/gpu/drm/i915/intel_fbc.c | 119 ++++++++++++++----------------- 2 files changed, 58 insertions(+), 67 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2493ad2f5c12..fff165a35ce7 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -919,9 +919,11 @@ struct i915_fbc { bool active; struct intel_fbc_work { - struct delayed_work work; + bool scheduled; + struct work_struct work; struct drm_framebuffer *fb; - } *fbc_work; + unsigned long enable_jiffies; + } work; const char *no_fbc_reason; diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 8460e3d72b98..07d3342b59de 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -392,71 +392,13 @@ static void intel_fbc_activate(const struct drm_framebuffer *fb) static void intel_fbc_work_fn(struct work_struct *__work) { - struct intel_fbc_work *work = - container_of(to_delayed_work(__work), - struct intel_fbc_work, work); - struct drm_i915_private *dev_priv = work->fb->dev->dev_private; - struct drm_framebuffer *crtc_fb = dev_priv->fbc.crtc->base.primary->fb; - - mutex_lock(&dev_priv->fbc.lock); - if (work == dev_priv->fbc.fbc_work) { - /* Double check that we haven't switched fb without cancelling - * the prior work. - */ - if (crtc_fb == work->fb) - intel_fbc_activate(work->fb); - - dev_priv->fbc.fbc_work = NULL; - } - mutex_unlock(&dev_priv->fbc.lock); - - kfree(work); -} - -static void intel_fbc_cancel_work(struct drm_i915_private *dev_priv) -{ - WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock)); - - if (dev_priv->fbc.fbc_work == NULL) - return; - - /* Synchronisation is provided by struct_mutex and checking of - * dev_priv->fbc.fbc_work, so we can perform the cancellation - * entirely asynchronously. - */ - if (cancel_delayed_work(&dev_priv->fbc.fbc_work->work)) - /* tasklet was killed before being run, clean up */ - kfree(dev_priv->fbc.fbc_work); - - /* Mark the work as no longer wanted so that if it does - * wake-up (because the work was already running and waiting - * for our mutex), it will discover that is no longer - * necessary to run. - */ - dev_priv->fbc.fbc_work = NULL; -} - -static void intel_fbc_schedule_activation(struct intel_crtc *crtc) -{ - struct intel_fbc_work *work; - struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; - - WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock)); - - intel_fbc_cancel_work(dev_priv); - - work = kzalloc(sizeof(*work), GFP_KERNEL); - if (work == NULL) { - DRM_ERROR("Failed to allocate FBC work structure\n"); - intel_fbc_activate(crtc->base.primary->fb); - return; - } - - work->fb = crtc->base.primary->fb; - INIT_DELAYED_WORK(&work->work, intel_fbc_work_fn); - - dev_priv->fbc.fbc_work = work; + struct drm_i915_private *dev_priv = + container_of(__work, struct drm_i915_private, fbc.work.work); + struct intel_fbc_work *work = &dev_priv->fbc.work; + struct intel_crtc *crtc = dev_priv->fbc.crtc; + int delay_ms = 50; +retry: /* Delay the actual enabling to let pageflipping cease and the * display to settle before starting the compression. Note that * this delay also serves a second purpose: it allows for a @@ -470,7 +412,52 @@ static void intel_fbc_schedule_activation(struct intel_crtc *crtc) * * WaFbcWaitForVBlankBeforeEnable:ilk,snb */ - schedule_delayed_work(&work->work, msecs_to_jiffies(50)); + wait_remaining_ms_from_jiffies(work->enable_jiffies, delay_ms); + + mutex_lock(&dev_priv->fbc.lock); + + /* Were we cancelled? */ + if (!work->scheduled) + goto out; + + /* Were we delayed again while this function was sleeping? */ + if (time_after(work->enable_jiffies + msecs_to_jiffies(delay_ms), + jiffies)) { + mutex_unlock(&dev_priv->fbc.lock); + goto retry; + } + + if (crtc->base.primary->fb == work->fb) + intel_fbc_activate(work->fb); + + work->scheduled = false; + +out: + mutex_unlock(&dev_priv->fbc.lock); +} + +static void intel_fbc_cancel_work(struct drm_i915_private *dev_priv) +{ + WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock)); + dev_priv->fbc.work.scheduled = false; +} + +static void intel_fbc_schedule_activation(struct intel_crtc *crtc) +{ + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; + struct intel_fbc_work *work = &dev_priv->fbc.work; + + WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock)); + + /* It is useless to call intel_fbc_cancel_work() in this function since + * we're not releasing fbc.lock, so it won't have an opportunity to grab + * it to discover that it was cancelled. So we just update the expected + * jiffy count. */ + work->fb = crtc->base.primary->fb; + work->scheduled = true; + work->enable_jiffies = jiffies; + + schedule_work(&work->work); } static void __intel_fbc_deactivate(struct drm_i915_private *dev_priv) @@ -1106,9 +1093,11 @@ void intel_fbc_init(struct drm_i915_private *dev_priv) { enum pipe pipe; + INIT_WORK(&dev_priv->fbc.work.work, intel_fbc_work_fn); mutex_init(&dev_priv->fbc.lock); dev_priv->fbc.enabled = false; dev_priv->fbc.active = false; + dev_priv->fbc.work.scheduled = false; if (!HAS_FBC(dev_priv)) { dev_priv->fbc.no_fbc_reason = "unsupported by this chipset"; From 559d9135835c39bec7fd580cae9ac73f005ae726 Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Mon, 26 Oct 2015 18:44:25 -0200 Subject: [PATCH 54/70] drm/i915: kill fbc.uncompressed_size Directly call intel_fbc_calculate_cfb_size() in the only place that actually needs it, and use the proper check before removing the stolen node. IMHO, this change makes our code easier to understand. v2: Use drm_mm_node_allocated() (Chris). Reviewed-by: Chris Wilson Signed-off-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/ --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/intel_fbc.c | 13 ++++--------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index fff165a35ce7..5e276f5d61da 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -902,7 +902,6 @@ struct i915_fbc { /* This is always the inner lock when overlapping with struct_mutex and * it's the outer lock when overlapping with stolen_lock. */ struct mutex lock; - unsigned long uncompressed_size; unsigned threshold; unsigned int fb_id; unsigned int possible_framebuffer_bits; diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 07d3342b59de..0ad9585b0487 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -144,7 +144,7 @@ static void i8xx_fbc_activate(struct intel_crtc *crtc) dev_priv->fbc.active = true; /* Note: fbc.threshold == 1 for i8xx */ - cfb_pitch = dev_priv->fbc.uncompressed_size / FBC_LL_SIZE; + cfb_pitch = intel_fbc_calculate_cfb_size(crtc, fb) / FBC_LL_SIZE; if (fb->pitches[0] < cfb_pitch) cfb_pitch = fb->pitches[0]; @@ -638,8 +638,6 @@ static int intel_fbc_alloc_cfb(struct intel_crtc *crtc) dev_priv->mm.stolen_base + compressed_llb->start); } - dev_priv->fbc.uncompressed_size = size; - DRM_DEBUG_KMS("reserved %llu bytes of contiguous stolen space for FBC, threshold: %d\n", dev_priv->fbc.compressed_fb.size, dev_priv->fbc.threshold); @@ -656,18 +654,15 @@ err_llb: static void __intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv) { - if (dev_priv->fbc.uncompressed_size == 0) - return; - - i915_gem_stolen_remove_node(dev_priv, &dev_priv->fbc.compressed_fb); + if (drm_mm_node_allocated(&dev_priv->fbc.compressed_fb)) + i915_gem_stolen_remove_node(dev_priv, + &dev_priv->fbc.compressed_fb); if (dev_priv->fbc.compressed_llb) { i915_gem_stolen_remove_node(dev_priv, dev_priv->fbc.compressed_llb); kfree(dev_priv->fbc.compressed_llb); } - - dev_priv->fbc.uncompressed_size = 0; } void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv) From 820bcabbf0acce7f3e7b72ed5e51b0a1d9c2046d Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Tue, 27 Oct 2015 14:54:22 -0200 Subject: [PATCH 55/70] drm/i915: get rid of FBC {,de}activation messages When running Cinnamon I see way too many pairs of these messages: many per second. Get rid of them as they're just telling us FBC is working as expected. We already have the messages for enable/disable, so we don't really need messages for activation/deactivation. v2: Rebase after changing the patch order. Reviewed-by: Chris Wilson Signed-off-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/ --- drivers/gpu/drm/i915/intel_fbc.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 0ad9585b0487..af621de700fb 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -128,8 +128,6 @@ static void i8xx_fbc_deactivate(struct drm_i915_private *dev_priv) DRM_DEBUG_KMS("FBC idle timed out\n"); return; } - - DRM_DEBUG_KMS("deactivated FBC\n"); } static void i8xx_fbc_activate(struct intel_crtc *crtc) @@ -177,9 +175,6 @@ static void i8xx_fbc_activate(struct intel_crtc *crtc) fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT; fbc_ctl |= obj->fence_reg; I915_WRITE(FBC_CONTROL, fbc_ctl); - - DRM_DEBUG_KMS("activated FBC, pitch %d, yoff %d, plane %c\n", - cfb_pitch, crtc->base.y, plane_name(crtc->plane)); } static bool i8xx_fbc_is_active(struct drm_i915_private *dev_priv) @@ -207,8 +202,6 @@ static void g4x_fbc_activate(struct intel_crtc *crtc) /* enable it... */ I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); - - DRM_DEBUG_KMS("activated fbc on plane %c\n", plane_name(crtc->plane)); } static void g4x_fbc_deactivate(struct drm_i915_private *dev_priv) @@ -222,8 +215,6 @@ static void g4x_fbc_deactivate(struct drm_i915_private *dev_priv) if (dpfc_ctl & DPFC_CTL_EN) { dpfc_ctl &= ~DPFC_CTL_EN; I915_WRITE(DPFC_CONTROL, dpfc_ctl); - - DRM_DEBUG_KMS("deactivated FBC\n"); } } @@ -283,8 +274,6 @@ static void ilk_fbc_activate(struct intel_crtc *crtc) } intel_fbc_recompress(dev_priv); - - DRM_DEBUG_KMS("activated fbc on plane %c\n", plane_name(crtc->plane)); } static void ilk_fbc_deactivate(struct drm_i915_private *dev_priv) @@ -298,8 +287,6 @@ static void ilk_fbc_deactivate(struct drm_i915_private *dev_priv) if (dpfc_ctl & DPFC_CTL_EN) { dpfc_ctl &= ~DPFC_CTL_EN; I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl); - - DRM_DEBUG_KMS("deactivated FBC\n"); } } @@ -362,8 +349,6 @@ static void gen7_fbc_activate(struct intel_crtc *crtc) I915_WRITE(DPFC_CPU_FENCE_OFFSET, get_crtc_fence_y_offset(crtc)); intel_fbc_recompress(dev_priv); - - DRM_DEBUG_KMS("activated fbc on plane %c\n", plane_name(crtc->plane)); } /** From ee7d6cfa4b15aafa1d87f913572f30dd64cdd85a Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Wed, 11 Nov 2015 14:46:22 -0200 Subject: [PATCH 56/70] drm/i915: only recompress FBC after flushing a drawing operation There's no need to stop and restart FBC, which is quite expensive as we have to revalidate the CRTC state. After flushing a drawing operation we know the CRTC state hasn't changed, so a nuke (recompress) should be fine. v2: Make it simpler (Chris). v3: Rewrite the patch again due to patch order changes. v4: Rewrite commit message (Chris). Reviewed-by: Chris Wilson Signed-off-by: Paulo Zanoni Link: http://patchwork.freedesktop.org/patch/msgid/ --- drivers/gpu/drm/i915/intel_fbc.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index af621de700fb..a1988a486b92 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -935,8 +935,12 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv, dev_priv->fbc.busy_bits &= ~frontbuffer_bits; if (!dev_priv->fbc.busy_bits && dev_priv->fbc.enabled) { - __intel_fbc_deactivate(dev_priv); - __intel_fbc_update(dev_priv->fbc.crtc); + if (origin != ORIGIN_FLIP && dev_priv->fbc.active) { + intel_fbc_recompress(dev_priv); + } else { + __intel_fbc_deactivate(dev_priv); + __intel_fbc_update(dev_priv->fbc.crtc); + } } mutex_unlock(&dev_priv->fbc.lock); From 5a843307cdf5ffa65a9f2382b3827e86576bbfe8 Mon Sep 17 00:00:00 2001 From: Alex Dai Date: Wed, 2 Dec 2015 16:56:29 -0800 Subject: [PATCH 57/70] drm/i915/guc: Clean up locks in GuC For now, remove the spinlocks that protected the GuC's statistics block and work queue; they are only accessed by code that already holds the global struct_mutex, and so are redundant (until the big struct_mutex rewrite!). The specific problem that the spinlocks caused was that if the work queue was full, the driver would try to spinwait for one jiffy, but with interrupts disabled the jiffy count would not advance, leading to a system hang. The issue was found using test case igt/gem_close_race. The new version will usleep() instead, still holding the struct_mutex but without any spinlocks. v4: Reorganize commit message (Dave Gordon) v3: Remove unnecessary whitespace churn v2: Clean up wq_lock too v1: Clean up host2guc lock as well Signed-off-by: Alex Dai Reviewed-by: Dave Gordon Link: http://patchwork.freedesktop.org/patch/msgid/1449104189-27591-1-git-send-email-yu.dai@intel.com Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_debugfs.c | 12 ++++----- drivers/gpu/drm/i915/i915_guc_submission.c | 31 +++++----------------- drivers/gpu/drm/i915/intel_guc.h | 4 --- 3 files changed, 12 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 9cc57296cbb1..a8721fccd8a0 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2469,15 +2469,15 @@ static int i915_guc_info(struct seq_file *m, void *data) if (!HAS_GUC_SCHED(dev_priv->dev)) return 0; + if (mutex_lock_interruptible(&dev->struct_mutex)) + return 0; + /* Take a local copy of the GuC data, so we can dump it at leisure */ - spin_lock(&dev_priv->guc.host2guc_lock); guc = dev_priv->guc; - if (guc.execbuf_client) { - spin_lock(&guc.execbuf_client->wq_lock); + if (guc.execbuf_client) client = *guc.execbuf_client; - spin_unlock(&guc.execbuf_client->wq_lock); - } - spin_unlock(&dev_priv->guc.host2guc_lock); + + mutex_unlock(&dev->struct_mutex); seq_printf(m, "GuC total action count: %llu\n", guc.action_count); seq_printf(m, "GuC action failure count: %u\n", guc.action_fail); diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index a057cbd78ecb..0d23785ba818 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -86,7 +86,6 @@ static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len) return -EINVAL; intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); - spin_lock(&dev_priv->guc.host2guc_lock); dev_priv->guc.action_count += 1; dev_priv->guc.action_cmd = data[0]; @@ -119,7 +118,6 @@ static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len) } dev_priv->guc.action_status = status; - spin_unlock(&dev_priv->guc.host2guc_lock); intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); return ret; @@ -292,16 +290,12 @@ static uint32_t select_doorbell_cacheline(struct intel_guc *guc) const uint32_t cacheline_size = cache_line_size(); uint32_t offset; - spin_lock(&guc->host2guc_lock); - /* Doorbell uses a single cache line within a page */ offset = offset_in_page(guc->db_cacheline); /* Moving to next cache line to reduce contention */ guc->db_cacheline += cacheline_size; - spin_unlock(&guc->host2guc_lock); - DRM_DEBUG_DRIVER("selected doorbell cacheline 0x%x, next 0x%x, linesize %u\n", offset, guc->db_cacheline, cacheline_size); @@ -322,13 +316,11 @@ static uint16_t assign_doorbell(struct intel_guc *guc, uint32_t priority) const uint16_t end = start + half; uint16_t id; - spin_lock(&guc->host2guc_lock); id = find_next_zero_bit(guc->doorbell_bitmap, end, start); if (id == end) id = GUC_INVALID_DOORBELL_ID; else bitmap_set(guc->doorbell_bitmap, id, 1); - spin_unlock(&guc->host2guc_lock); DRM_DEBUG_DRIVER("assigned %s priority doorbell id 0x%x\n", hi_pri ? "high" : "normal", id); @@ -338,9 +330,7 @@ static uint16_t assign_doorbell(struct intel_guc *guc, uint32_t priority) static void release_doorbell(struct intel_guc *guc, uint16_t id) { - spin_lock(&guc->host2guc_lock); bitmap_clear(guc->doorbell_bitmap, id, 1); - spin_unlock(&guc->host2guc_lock); } /* @@ -487,16 +477,13 @@ static int guc_get_workqueue_space(struct i915_guc_client *gc, u32 *offset) struct guc_process_desc *desc; void *base; u32 size = sizeof(struct guc_wq_item); - int ret = 0, timeout_counter = 200; + int ret = -ETIMEDOUT, timeout_counter = 200; base = kmap_atomic(i915_gem_object_get_page(gc->client_obj, 0)); desc = base + gc->proc_desc_offset; while (timeout_counter-- > 0) { - ret = wait_for_atomic(CIRC_SPACE(gc->wq_tail, desc->head, - gc->wq_size) >= size, 1); - - if (!ret) { + if (CIRC_SPACE(gc->wq_tail, desc->head, gc->wq_size) >= size) { *offset = gc->wq_tail; /* advance the tail for next workqueue item */ @@ -505,7 +492,11 @@ static int guc_get_workqueue_space(struct i915_guc_client *gc, u32 *offset) /* this will break the loop */ timeout_counter = 0; + ret = 0; } + + if (timeout_counter) + usleep_range(1000, 2000); }; kunmap_atomic(base); @@ -597,15 +588,12 @@ int i915_guc_submit(struct i915_guc_client *client, { struct intel_guc *guc = client->guc; enum intel_ring_id ring_id = rq->ring->id; - unsigned long flags; int q_ret, b_ret; /* Need this because of the deferred pin ctx and ring */ /* Shall we move this right after ring is pinned? */ lr_context_update(rq); - spin_lock_irqsave(&client->wq_lock, flags); - q_ret = guc_add_workqueue_item(client, rq); if (q_ret == 0) b_ret = guc_ring_doorbell(client); @@ -620,12 +608,8 @@ int i915_guc_submit(struct i915_guc_client *client, } else { client->retcode = 0; } - spin_unlock_irqrestore(&client->wq_lock, flags); - - spin_lock(&guc->host2guc_lock); guc->submissions[ring_id] += 1; guc->last_seqno[ring_id] = rq->seqno; - spin_unlock(&guc->host2guc_lock); return q_ret; } @@ -768,7 +752,6 @@ static struct i915_guc_client *guc_client_alloc(struct drm_device *dev, client->client_obj = obj; client->wq_offset = GUC_DB_SIZE; client->wq_size = GUC_WQ_SIZE; - spin_lock_init(&client->wq_lock); client->doorbell_offset = select_doorbell_cacheline(guc); @@ -871,8 +854,6 @@ int i915_guc_submission_init(struct drm_device *dev) if (!guc->ctx_pool_obj) return -ENOMEM; - spin_lock_init(&dev_priv->guc.host2guc_lock); - ida_init(&guc->ctx_ids); guc_create_log(guc); diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h index 5ba586683c87..822952235dcf 100644 --- a/drivers/gpu/drm/i915/intel_guc.h +++ b/drivers/gpu/drm/i915/intel_guc.h @@ -42,8 +42,6 @@ struct i915_guc_client { uint32_t wq_offset; uint32_t wq_size; - - spinlock_t wq_lock; /* Protects all data below */ uint32_t wq_tail; /* GuC submission statistics & status */ @@ -95,8 +93,6 @@ struct intel_guc { struct i915_guc_client *execbuf_client; - spinlock_t host2guc_lock; /* Protects all data below */ - DECLARE_BITMAP(doorbell_bitmap, GUC_MAX_DOORBELLS); uint32_t db_cacheline; /* Cyclic counter mod pagesize */ From 6d65ba943a2d1e4292a07ca7ddb6c5138b9efa5d Mon Sep 17 00:00:00 2001 From: Nick Hoath Date: Tue, 1 Dec 2015 14:48:57 +0000 Subject: [PATCH 58/70] drm/i915: Extend LRC pinning to cover GPU context writeback Use the first retired request on a new context to unpin the old context. This ensures that the hw context remains bound until it has been written back to by the GPU. Now that the context is pinned until later in the request/context lifecycle, it no longer needs to be pinned from context_queue to retire_requests. This fixes an issue with GuC submission where the GPU might not have finished writing back the context before it is unpinned. This results in a GPU hang. v2: Moved the new pin to cover GuC submission (Alex Dai) Moved the new unpin to request_retire to fix coverage leak v3: Added switch to default context if freeing a still pinned context just in case the hw was actually still using it v4: Unwrapped context unpin to allow calling without a request v5: Only create a switch to idle context if the ring doesn't already have a request pending on it (Alex Dai) Rename unsaved to dirty to avoid double negatives (Dave Gordon) Changed _no_req postfix to __ prefix for consistency (Dave Gordon) Split out per engine cleanup from context_free as it was getting unwieldy Corrected locking (Dave Gordon) v6: Removed some bikeshedding (Mika Kuoppala) Added explanation of the GuC hang that this fixes (Daniel Vetter) v7: Removed extra per request pinning from ring reset code (Alex Dai) Added forced ring unpin/clean in error case in context free (Alex Dai) Signed-off-by: Nick Hoath Issue: VIZ-4277 Cc: Daniel Vetter Cc: David Gordon Cc: Chris Wilson Cc: Alex Dai Cc: Mika Kuoppala Reviewed-by: Alex Dai Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 7 +- drivers/gpu/drm/i915/intel_lrc.c | 136 +++++++++++++++++++++++++------ drivers/gpu/drm/i915/intel_lrc.h | 1 + 4 files changed, 118 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5e276f5d61da..f95f36767649 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -884,6 +884,7 @@ struct intel_context { struct { struct drm_i915_gem_object *state; struct intel_ringbuffer *ringbuf; + bool dirty; int pin_count; } engine[I915_NUM_RINGS]; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 33adc8f8ab20..6356c347ede0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1354,6 +1354,9 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request) { trace_i915_gem_request_retire(request); + if (i915.enable_execlists) + intel_lr_context_complete_check(request); + /* We know the GPU must have read the request to have * sent us the seqno + interrupt, so use the position * of tail of the request to update the last known position @@ -2764,10 +2767,6 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv, struct drm_i915_gem_request, execlist_link); list_del(&submit_req->execlist_link); - - if (submit_req->ctx != ring->default_context) - intel_lr_context_unpin(submit_req); - i915_gem_request_unreference(submit_req); } spin_unlock_irq(&ring->execlist_lock); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 4ebafab53f30..c3504a09340c 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -571,9 +571,6 @@ static int execlists_context_queue(struct drm_i915_gem_request *request) struct drm_i915_gem_request *cursor; int num_elements = 0; - if (request->ctx != ring->default_context) - intel_lr_context_pin(request); - i915_gem_request_reference(request); spin_lock_irq(&ring->execlist_lock); @@ -737,6 +734,13 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request) if (intel_ring_stopped(ring)) return; + if (request->ctx != ring->default_context) { + if (!request->ctx->engine[ring->id].dirty) { + intel_lr_context_pin(request); + request->ctx->engine[ring->id].dirty = true; + } + } + if (dev_priv->guc.execbuf_client) i915_guc_submit(dev_priv->guc.execbuf_client, request); else @@ -963,12 +967,6 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring) spin_unlock_irq(&ring->execlist_lock); list_for_each_entry_safe(req, tmp, &retired_list, execlist_link) { - struct intel_context *ctx = req->ctx; - struct drm_i915_gem_object *ctx_obj = - ctx->engine[ring->id].state; - - if (ctx_obj && (ctx != ring->default_context)) - intel_lr_context_unpin(req); list_del(&req->execlist_link); i915_gem_request_unreference(req); } @@ -1063,21 +1061,39 @@ reset_pin_count: return ret; } -void intel_lr_context_unpin(struct drm_i915_gem_request *rq) +static void __intel_lr_context_unpin(struct intel_engine_cs *ring, + struct intel_context *ctx) { - struct intel_engine_cs *ring = rq->ring; - struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state; - struct intel_ringbuffer *ringbuf = rq->ringbuf; - + struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state; + struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf; if (ctx_obj) { WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex)); - if (--rq->ctx->engine[ring->id].pin_count == 0) { + if (--ctx->engine[ring->id].pin_count == 0) { intel_unpin_ringbuffer_obj(ringbuf); i915_gem_object_ggtt_unpin(ctx_obj); } } } +void intel_lr_context_unpin(struct drm_i915_gem_request *rq) +{ + __intel_lr_context_unpin(rq->ring, rq->ctx); +} + +void intel_lr_context_complete_check(struct drm_i915_gem_request *req) +{ + struct intel_engine_cs *ring = req->ring; + + if (ring->last_context && ring->last_context != req->ctx && + ring->last_context->engine[ring->id].dirty) { + __intel_lr_context_unpin( + ring, + ring->last_context); + ring->last_context->engine[ring->id].dirty = false; + } + ring->last_context = req->ctx; +} + static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req) { int ret, i; @@ -2351,6 +2367,76 @@ populate_lr_context(struct intel_context *ctx, struct drm_i915_gem_object *ctx_o return 0; } +/** + * intel_lr_context_clean_ring() - clean the ring specific parts of an LRC + * @ctx: the LR context being freed. + * @ring: the engine being cleaned + * @ctx_obj: the hw context being unreferenced + * @ringbuf: the ringbuf being freed + * + * Take care of cleaning up the per-engine backing + * objects and the logical ringbuffer. + */ +static void +intel_lr_context_clean_ring(struct intel_context *ctx, + struct intel_engine_cs *ring, + struct drm_i915_gem_object *ctx_obj, + struct intel_ringbuffer *ringbuf) +{ + int ret; + + WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex)); + + if (ctx == ring->default_context) { + intel_unpin_ringbuffer_obj(ringbuf); + i915_gem_object_ggtt_unpin(ctx_obj); + } + + if (ctx->engine[ring->id].dirty) { + struct drm_i915_gem_request *req = NULL; + + /** + * If there is already a request pending on + * this ring, wait for that to complete, + * otherwise create a switch to idle request + */ + if (list_empty(&ring->request_list)) { + int ret; + + ret = i915_gem_request_alloc( + ring, + ring->default_context, + &req); + if (!ret) + i915_add_request(req); + else + DRM_DEBUG("Failed to ensure context saved"); + } else { + req = list_first_entry( + &ring->request_list, + typeof(*req), list); + } + if (req) { + ret = i915_wait_request(req); + if (ret != 0) { + /** + * If we get here, there's probably been a ring + * reset, so we just clean up the dirty flag.& + * pin count. + */ + ctx->engine[ring->id].dirty = false; + __intel_lr_context_unpin( + ring, + ctx); + } + } + } + + WARN_ON(ctx->engine[ring->id].pin_count); + intel_ringbuffer_free(ringbuf); + drm_gem_object_unreference(&ctx_obj->base); +} + /** * intel_lr_context_free() - free the LRC specific bits of a context * @ctx: the LR context to free. @@ -2363,7 +2449,7 @@ void intel_lr_context_free(struct intel_context *ctx) { int i; - for (i = 0; i < I915_NUM_RINGS; i++) { + for (i = 0; i < I915_NUM_RINGS; ++i) { struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state; if (ctx_obj) { @@ -2371,13 +2457,10 @@ void intel_lr_context_free(struct intel_context *ctx) ctx->engine[i].ringbuf; struct intel_engine_cs *ring = ringbuf->ring; - if (ctx == ring->default_context) { - intel_unpin_ringbuffer_obj(ringbuf); - i915_gem_object_ggtt_unpin(ctx_obj); - } - WARN_ON(ctx->engine[ring->id].pin_count); - intel_ringbuffer_free(ringbuf); - drm_gem_object_unreference(&ctx_obj->base); + intel_lr_context_clean_ring(ctx, + ring, + ctx_obj, + ringbuf); } } } @@ -2539,5 +2622,12 @@ void intel_lr_context_reset(struct drm_device *dev, ringbuf->head = 0; ringbuf->tail = 0; + + if (ctx->engine[ring->id].dirty) { + __intel_lr_context_unpin( + ring, + ctx); + ctx->engine[ring->id].dirty = false; + } } } diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index 0b821b91723a..fd1b6b43bacc 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -91,6 +91,7 @@ void intel_lr_context_reset(struct drm_device *dev, struct intel_context *ctx); uint64_t intel_lr_context_descriptor(struct intel_context *ctx, struct intel_engine_cs *ring); +void intel_lr_context_complete_check(struct drm_i915_gem_request *req); /* Execlists */ int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists); From fd8e058a27f319d4560529d54bcbf8aec2d72017 Mon Sep 17 00:00:00 2001 From: Alex Goins Date: Wed, 25 Nov 2015 18:43:38 -0800 Subject: [PATCH 59/70] i915: wait for fence in mmio_flip_work_func If a buffer is backed by dmabuf, wait on its reservation object's exclusive fence before flipping. v2: First commit v3: Remove object_name_lock acquire v4: Move wait ahead of mark_page_flip_active Use crtc->primary->fb to get GEM object instead of pending_flip_obj use_mmio_flip() return true when exclusive fence is attached Wait only on exclusive fences, interruptible with no timeout v5: Move wait from do_mmio_flip to mmio_flip_work_func Style tweaks to more closely match rest of file v6: Change back to unintteruptible wait to match __i915_wait_request due to inability to properly handle interrupted wait. Warn on error code from waiting. v7: No change v8: Test for !reservation_object_signaled_rcu(test_all=FALSE) instead of obj->base.dma_buf->resv->fence_excl Link: https://patchwork.kernel.org/patch/7704181/ Signed-off-by: Alex Goins Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 602e2be1c3d8..0ad20eccbf62 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -44,6 +44,8 @@ #include #include #include +#include +#include /* Primary plane formats for gen <= 3 */ static const uint32_t i8xx_primary_formats[] = { @@ -11204,6 +11206,10 @@ static bool use_mmio_flip(struct intel_engine_cs *ring, return true; else if (i915.enable_execlists) return true; + else if (obj->base.dma_buf && + !reservation_object_test_signaled_rcu(obj->base.dma_buf->resv, + false)) + return true; else return ring != i915_gem_request_get_ring(obj->last_write_req); } @@ -11318,6 +11324,9 @@ static void intel_mmio_flip_work_func(struct work_struct *work) { struct intel_mmio_flip *mmio_flip = container_of(work, struct intel_mmio_flip, work); + struct intel_framebuffer *intel_fb = + to_intel_framebuffer(mmio_flip->crtc->base.primary->fb); + struct drm_i915_gem_object *obj = intel_fb->obj; if (mmio_flip->req) { WARN_ON(__i915_wait_request(mmio_flip->req, @@ -11327,6 +11336,12 @@ static void intel_mmio_flip_work_func(struct work_struct *work) i915_gem_request_unreference__unlocked(mmio_flip->req); } + /* For framebuffer backed by dmabuf, wait for fence */ + if (obj->base.dma_buf) + WARN_ON(reservation_object_wait_timeout_rcu(obj->base.dma_buf->resv, + false, false, + MAX_SCHEDULE_TIMEOUT) < 0); + intel_do_mmio_flip(mmio_flip); kfree(mmio_flip); } From 3c28ff22f6e20cc946cb5a2b77d14862999f7ba0 Mon Sep 17 00:00:00 2001 From: Alex Goins Date: Wed, 25 Nov 2015 18:43:39 -0800 Subject: [PATCH 60/70] i915: wait for fence in prepare_plane_fb In intel_prepare_plane_fb, if fb is backed by dma-buf, wait for exclusive fence v2: First commit v3: Remove object_name_lock acquire Move wait from intel_atomic_commit() to intel_prepare_plane_fb() v4: Wait only on exclusive fences, interruptible with no timeout v5: Style tweaks to more closely match rest of file v6: Properly handle interrupted waits v7: No change v8: No change Link: https://patchwork.kernel.org/patch/7704181/ Signed-off-by: Alex Goins Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0ad20eccbf62..01aae086b532 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13690,6 +13690,17 @@ intel_prepare_plane_fb(struct drm_plane *plane, return ret; } + /* For framebuffer backed by dmabuf, wait for fence */ + if (obj && obj->base.dma_buf) { + ret = reservation_object_wait_timeout_rcu(obj->base.dma_buf->resv, + false, true, + MAX_SCHEDULE_TIMEOUT); + if (ret == -ERESTARTSYS) + return ret; + + WARN_ON(ret < 0); + } + if (!obj) { ret = 0; } else if (plane->type == DRM_PLANE_TYPE_CURSOR && From 63ba534ecf78c301df87b25267406002cc1af14a Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Tue, 24 Nov 2015 11:29:03 +0100 Subject: [PATCH 61/70] drm/i915: Handle cdclk limits on broadwell. As the comment indicates this can only fail gracefully when called from compute_config. Fortunately this is now what's happening, so the fixme can be removed and the DRM_ERROR downgraded. Link: http://patchwork.freedesktop.org/patch/msgid/1448360945-5723-3-git-send-email-maarten.lankhorst@linux.intel.com Signed-off-by: Maarten Lankhorst Reviewed-by: Chris Wilson --- drivers/gpu/drm/i915/intel_display.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 01aae086b532..3a13a498722a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9719,14 +9719,10 @@ static int broadwell_modeset_calc_cdclk(struct drm_atomic_state *state) else cdclk = 337500; - /* - * FIXME move the cdclk caclulation to - * compute_config() so we can fail gracegully. - */ if (cdclk > dev_priv->max_cdclk_freq) { - DRM_ERROR("requested cdclk (%d kHz) exceeds max (%d kHz)\n", - cdclk, dev_priv->max_cdclk_freq); - cdclk = dev_priv->max_cdclk_freq; + DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", + cdclk, dev_priv->max_cdclk_freq); + return -EINVAL; } to_intel_atomic_state(state)->cdclk = cdclk; From 89f41f4f90741fe94b6da9d4d366628a9b0be8f1 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 1 Dec 2015 17:09:50 +0100 Subject: [PATCH 62/70] drm/i915: Remove superfluous NULL check to_intel_crtc() always returns a non-NULL pointer. Signed-off-by: Takashi Iwai Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1448986198-3488-2-git-send-email-tiwai@suse.de --- drivers/gpu/drm/i915/intel_audio.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 9aa83e71b792..97302457e7f9 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -653,10 +653,6 @@ static int i915_audio_component_sync_audio_rate(struct device *dev, intel_dig_port = enc_to_dig_port(&intel_encoder->base); if (port == intel_dig_port->port) { crtc = to_intel_crtc(intel_encoder->base.crtc); - if (!crtc) { - DRM_DEBUG_KMS("%s: crtc is NULL\n", __func__); - continue; - } pipe = crtc->pipe; break; } From 6a8beeffed3b2d33151150e3a03696e697f16d46 Mon Sep 17 00:00:00 2001 From: Wayne Boyer Date: Wed, 2 Dec 2015 13:28:14 -0800 Subject: [PATCH 63/70] drm/i915: Clean up device info structure definitions Beginning with gen7, newer devices repetitively redefine values for the device info structure members. This patch simplifies the structure definitions by grouping member value definitions into the existing GEN7_FEATURES #define and into the new GEN7_LP_FEATURES and HSW_FEATURES #defines. Specifically, GEN_DEFAULT_PIPEOFFSETS and IVB_CURSOR_OFFSETS are added to GEN7_FEATURES and subsequent IVB definitions are simplified. VLV_FEATURES is defined to differentiate and simplify the gen7 low power (LP) devices. HSW_FEATURES is defined and used to simplify all HSW+ devices except for LP. v2: Use VLV_FEATURES for the gen7 low power devices. (Jani) Cc: Rodrigo Vivi Signed-off-by: Wayne Boyer Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1449091694-7681-1-git-send-email-wayne.boyer@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 137 ++++++++------------------------ 1 file changed, 35 insertions(+), 102 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 90faa8e03fca..d2d7e2461fa6 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -228,121 +228,83 @@ static const struct intel_device_info intel_sandybridge_m_info = { .need_gfx_hws = 1, .has_hotplug = 1, \ .has_fbc = 1, \ .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ - .has_llc = 1 + .has_llc = 1, \ + GEN_DEFAULT_PIPEOFFSETS, \ + IVB_CURSOR_OFFSETS static const struct intel_device_info intel_ivybridge_d_info = { GEN7_FEATURES, .is_ivybridge = 1, - GEN_DEFAULT_PIPEOFFSETS, - IVB_CURSOR_OFFSETS, }; static const struct intel_device_info intel_ivybridge_m_info = { GEN7_FEATURES, .is_ivybridge = 1, .is_mobile = 1, - GEN_DEFAULT_PIPEOFFSETS, - IVB_CURSOR_OFFSETS, }; static const struct intel_device_info intel_ivybridge_q_info = { GEN7_FEATURES, .is_ivybridge = 1, .num_pipes = 0, /* legal, last one wins */ - GEN_DEFAULT_PIPEOFFSETS, - IVB_CURSOR_OFFSETS, }; +#define VLV_FEATURES \ + .gen = 7, .num_pipes = 2, \ + .need_gfx_hws = 1, .has_hotplug = 1, \ + .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ + .display_mmio_offset = VLV_DISPLAY_BASE, \ + GEN_DEFAULT_PIPEOFFSETS, \ + CURSOR_OFFSETS + static const struct intel_device_info intel_valleyview_m_info = { - GEN7_FEATURES, - .is_mobile = 1, - .num_pipes = 2, + VLV_FEATURES, .is_valleyview = 1, - .display_mmio_offset = VLV_DISPLAY_BASE, - .has_fbc = 0, /* legal, last one wins */ - .has_llc = 0, /* legal, last one wins */ - GEN_DEFAULT_PIPEOFFSETS, - CURSOR_OFFSETS, + .is_mobile = 1, }; static const struct intel_device_info intel_valleyview_d_info = { - GEN7_FEATURES, - .num_pipes = 2, + VLV_FEATURES, .is_valleyview = 1, - .display_mmio_offset = VLV_DISPLAY_BASE, - .has_fbc = 0, /* legal, last one wins */ - .has_llc = 0, /* legal, last one wins */ - GEN_DEFAULT_PIPEOFFSETS, - CURSOR_OFFSETS, }; +#define HSW_FEATURES \ + GEN7_FEATURES, \ + .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, \ + .has_ddi = 1, \ + .has_fpga_dbg = 1 + static const struct intel_device_info intel_haswell_d_info = { - GEN7_FEATURES, + HSW_FEATURES, .is_haswell = 1, - .has_ddi = 1, - .has_fpga_dbg = 1, - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, - GEN_DEFAULT_PIPEOFFSETS, - IVB_CURSOR_OFFSETS, }; static const struct intel_device_info intel_haswell_m_info = { - GEN7_FEATURES, + HSW_FEATURES, .is_haswell = 1, .is_mobile = 1, - .has_ddi = 1, - .has_fpga_dbg = 1, - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, - GEN_DEFAULT_PIPEOFFSETS, - IVB_CURSOR_OFFSETS, }; static const struct intel_device_info intel_broadwell_d_info = { - .gen = 8, .num_pipes = 3, - .need_gfx_hws = 1, .has_hotplug = 1, - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, - .has_llc = 1, - .has_ddi = 1, - .has_fpga_dbg = 1, - .has_fbc = 1, - GEN_DEFAULT_PIPEOFFSETS, - IVB_CURSOR_OFFSETS, + HSW_FEATURES, + .gen = 8, }; static const struct intel_device_info intel_broadwell_m_info = { - .gen = 8, .is_mobile = 1, .num_pipes = 3, - .need_gfx_hws = 1, .has_hotplug = 1, - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, - .has_llc = 1, - .has_ddi = 1, - .has_fpga_dbg = 1, - .has_fbc = 1, - GEN_DEFAULT_PIPEOFFSETS, - IVB_CURSOR_OFFSETS, + HSW_FEATURES, + .gen = 8, .is_mobile = 1, }; static const struct intel_device_info intel_broadwell_gt3d_info = { - .gen = 8, .num_pipes = 3, - .need_gfx_hws = 1, .has_hotplug = 1, + HSW_FEATURES, + .gen = 8, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, - .has_llc = 1, - .has_ddi = 1, - .has_fpga_dbg = 1, - .has_fbc = 1, - GEN_DEFAULT_PIPEOFFSETS, - IVB_CURSOR_OFFSETS, }; static const struct intel_device_info intel_broadwell_gt3m_info = { - .gen = 8, .is_mobile = 1, .num_pipes = 3, - .need_gfx_hws = 1, .has_hotplug = 1, + HSW_FEATURES, + .gen = 8, .is_mobile = 1, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, - .has_llc = 1, - .has_ddi = 1, - .has_fpga_dbg = 1, - .has_fbc = 1, - GEN_DEFAULT_PIPEOFFSETS, - IVB_CURSOR_OFFSETS, }; static const struct intel_device_info intel_cherryview_info = { @@ -356,29 +318,15 @@ static const struct intel_device_info intel_cherryview_info = { }; static const struct intel_device_info intel_skylake_info = { + HSW_FEATURES, .is_skylake = 1, - .gen = 9, .num_pipes = 3, - .need_gfx_hws = 1, .has_hotplug = 1, - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, - .has_llc = 1, - .has_ddi = 1, - .has_fpga_dbg = 1, - .has_fbc = 1, - GEN_DEFAULT_PIPEOFFSETS, - IVB_CURSOR_OFFSETS, + .gen = 9, }; static const struct intel_device_info intel_skylake_gt3_info = { .is_skylake = 1, - .gen = 9, .num_pipes = 3, - .need_gfx_hws = 1, .has_hotplug = 1, + .gen = 9, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, - .has_llc = 1, - .has_ddi = 1, - .has_fpga_dbg = 1, - .has_fbc = 1, - GEN_DEFAULT_PIPEOFFSETS, - IVB_CURSOR_OFFSETS, }; static const struct intel_device_info intel_broxton_info = { @@ -396,33 +344,18 @@ static const struct intel_device_info intel_broxton_info = { }; static const struct intel_device_info intel_kabylake_info = { + HSW_FEATURES, .is_preliminary = 1, .is_kabylake = 1, .gen = 9, - .num_pipes = 3, - .need_gfx_hws = 1, .has_hotplug = 1, - .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, - .has_llc = 1, - .has_ddi = 1, - .has_fpga_dbg = 1, - .has_fbc = 1, - GEN_DEFAULT_PIPEOFFSETS, - IVB_CURSOR_OFFSETS, }; static const struct intel_device_info intel_kabylake_gt3_info = { + HSW_FEATURES, .is_preliminary = 1, .is_kabylake = 1, .gen = 9, - .num_pipes = 3, - .need_gfx_hws = 1, .has_hotplug = 1, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, - .has_llc = 1, - .has_ddi = 1, - .has_fpga_dbg = 1, - .has_fbc = 1, - GEN_DEFAULT_PIPEOFFSETS, - IVB_CURSOR_OFFSETS, }; /* From d8578836ad2ebaaf0a899c72d12201009e0ad1e5 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 4 Dec 2015 15:54:30 +0100 Subject: [PATCH 64/70] Revert "drm/i915: Remove superfluous NULL check" This reverts commit 89f41f4f90741fe94b6da9d4d366628a9b0be8f1. It's possible that ->crtc is NULL in here. Noticed by Ville. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_audio.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 97302457e7f9..9aa83e71b792 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -653,6 +653,10 @@ static int i915_audio_component_sync_audio_rate(struct device *dev, intel_dig_port = enc_to_dig_port(&intel_encoder->base); if (port == intel_dig_port->port) { crtc = to_intel_crtc(intel_encoder->base.crtc); + if (!crtc) { + DRM_DEBUG_KMS("%s: crtc is NULL\n", __func__); + continue; + } pipe = crtc->pipe; break; } From b6aa0873614dff74b01e18095fd335fbb1fb777c Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 2 Dec 2015 09:13:46 +0000 Subject: [PATCH 65/70] drm/i915: Fix RPS pointer passed from wait_ioctl to i915_wait_request In commit 2e1b873072dfe3bbcc158a9c21acde1ab0d36c55 [v4.2] Author: Chris Wilson Date: Mon Apr 27 13:41:22 2015 +0100 drm/i915: Convert RPS tracking to a intel_rps_client struct we converted the __i915_wait_request() to take a new intel_rps_client struct (rather than having to pass fake drm_i915_file_private structs). However, due to use of passing a void pointer, I didn't spot one callsite in wait-ioctl was passing the wrong pointer. Fwiw, the impact of this bug is zero. Along the rps path, we always first call list_empty(rps) which when we pass in the wrong pointer always evaluates to false and we return early and never chase the invalid pointers. The user visible impact is then wait-ioctl doesn't get the same waitboosting as the other interfaces (set-domain, throttle), which is a performance concern for the *very* few users of the wait interface. There is also a libdrm_intel patch to use the wait-ioctl for drm_intel_bo_wait_rendering() if anyone feels inclined to review libdrm_intel patches. Signed-off-by: Chris Wilson Cc: Daniel Vetter [danvet: Add Chris' explanation for why the impact of this is pretty close to 0.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 6356c347ede0..ef4dbe3d442d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3064,7 +3064,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) if (ret == 0) ret = __i915_wait_request(req[i], reset_counter, true, args->timeout_ns > 0 ? &args->timeout_ns : NULL, - file->driver_priv); + to_rps_client(file)); i915_gem_request_unreference__unlocked(req[i]); } return ret; From a9287dbc26569ec187b8bca0093e0ffe28abe843 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 4 Dec 2015 16:15:55 +0100 Subject: [PATCH 66/70] drm/i915: Restore skl_gt3 device info This was broken in commit 6a8beeffed3b2d33151150e3a03696e697f16d46 Author: Wayne Boyer Date: Wed Dec 2 13:28:14 2015 -0800 drm/i915: Clean up device info structure definitions and I didn't spot this while reviewing. We really need that CI farm up asap! Reported-by: Chris Wilson Cc: Chris Wilson Cc: Wayne Boyer Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index d2d7e2461fa6..46ac66484dc7 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -324,6 +324,7 @@ static const struct intel_device_info intel_skylake_info = { }; static const struct intel_device_info intel_skylake_gt3_info = { + HSW_FEATURES, .is_skylake = 1, .gen = 9, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, From 61ad992875639b0ac5866321e2edcba573f5f3a2 Mon Sep 17 00:00:00 2001 From: Deepak M Date: Fri, 4 Dec 2015 19:47:38 +0530 Subject: [PATCH 67/70] drm/i915: Correct the Ref clock value for BXT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reference clock for BXT is 19.2 MHz not 19.5 MHz, updating the correct value here. Signed-off-by: Deepak M Reviewed-by: Ville Syrjälä Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1449238659-12473-2-git-send-email-m.deepak@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d326c54ff667..206b213a74e1 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7714,7 +7714,7 @@ enum skl_disp_power_wells { #define BXT_DSI_PLL_RATIO_MAX 0x7D #define BXT_DSI_PLL_RATIO_MIN 0x22 #define BXT_DSI_PLL_RATIO_MASK 0xFF -#define BXT_REF_CLOCK_KHZ 19500 +#define BXT_REF_CLOCK_KHZ 19200 #define BXT_DSI_PLL_ENABLE _MMIO(0x46080) #define BXT_DSI_PLL_DO_ENABLE (1 << 31) From af3302b90775ca3389c93ab31458d696e8a8fa60 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 4 Dec 2015 17:27:15 +0100 Subject: [PATCH 68/70] Revert "drm/i915: Extend LRC pinning to cover GPU context writeback" This reverts commit 6d65ba943a2d1e4292a07ca7ddb6c5138b9efa5d. Mika Kuoppala traced down a use-after-free crash in module unload to this commit, because ring->last_context is leaked beyond when the context gets destroyed. Mika submitted a quick fix to patch that up in the context destruction code, but that's too much of a hack. The right fix is instead for the ring to hold a full reference onto it's last context, like we do for legacy contexts. Since this is causing a regression in BAT it gets reverted before we can close this. Cc: Nick Hoath Cc: Daniel Vetter Cc: David Gordon Cc: Chris Wilson Cc: Alex Dai Cc: Mika Kuoppala Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93248 Acked-by: Mika Kuoppala Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/i915_gem.c | 7 +- drivers/gpu/drm/i915/intel_lrc.c | 136 ++++++------------------------- drivers/gpu/drm/i915/intel_lrc.h | 1 - 4 files changed, 27 insertions(+), 118 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f95f36767649..5e276f5d61da 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -884,7 +884,6 @@ struct intel_context { struct { struct drm_i915_gem_object *state; struct intel_ringbuffer *ringbuf; - bool dirty; int pin_count; } engine[I915_NUM_RINGS]; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ef4dbe3d442d..a531cb83295c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1354,9 +1354,6 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request) { trace_i915_gem_request_retire(request); - if (i915.enable_execlists) - intel_lr_context_complete_check(request); - /* We know the GPU must have read the request to have * sent us the seqno + interrupt, so use the position * of tail of the request to update the last known position @@ -2767,6 +2764,10 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv, struct drm_i915_gem_request, execlist_link); list_del(&submit_req->execlist_link); + + if (submit_req->ctx != ring->default_context) + intel_lr_context_unpin(submit_req); + i915_gem_request_unreference(submit_req); } spin_unlock_irq(&ring->execlist_lock); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index c3504a09340c..4ebafab53f30 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -571,6 +571,9 @@ static int execlists_context_queue(struct drm_i915_gem_request *request) struct drm_i915_gem_request *cursor; int num_elements = 0; + if (request->ctx != ring->default_context) + intel_lr_context_pin(request); + i915_gem_request_reference(request); spin_lock_irq(&ring->execlist_lock); @@ -734,13 +737,6 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request) if (intel_ring_stopped(ring)) return; - if (request->ctx != ring->default_context) { - if (!request->ctx->engine[ring->id].dirty) { - intel_lr_context_pin(request); - request->ctx->engine[ring->id].dirty = true; - } - } - if (dev_priv->guc.execbuf_client) i915_guc_submit(dev_priv->guc.execbuf_client, request); else @@ -967,6 +963,12 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring) spin_unlock_irq(&ring->execlist_lock); list_for_each_entry_safe(req, tmp, &retired_list, execlist_link) { + struct intel_context *ctx = req->ctx; + struct drm_i915_gem_object *ctx_obj = + ctx->engine[ring->id].state; + + if (ctx_obj && (ctx != ring->default_context)) + intel_lr_context_unpin(req); list_del(&req->execlist_link); i915_gem_request_unreference(req); } @@ -1061,39 +1063,21 @@ reset_pin_count: return ret; } -static void __intel_lr_context_unpin(struct intel_engine_cs *ring, - struct intel_context *ctx) +void intel_lr_context_unpin(struct drm_i915_gem_request *rq) { - struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state; - struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf; + struct intel_engine_cs *ring = rq->ring; + struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state; + struct intel_ringbuffer *ringbuf = rq->ringbuf; + if (ctx_obj) { WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex)); - if (--ctx->engine[ring->id].pin_count == 0) { + if (--rq->ctx->engine[ring->id].pin_count == 0) { intel_unpin_ringbuffer_obj(ringbuf); i915_gem_object_ggtt_unpin(ctx_obj); } } } -void intel_lr_context_unpin(struct drm_i915_gem_request *rq) -{ - __intel_lr_context_unpin(rq->ring, rq->ctx); -} - -void intel_lr_context_complete_check(struct drm_i915_gem_request *req) -{ - struct intel_engine_cs *ring = req->ring; - - if (ring->last_context && ring->last_context != req->ctx && - ring->last_context->engine[ring->id].dirty) { - __intel_lr_context_unpin( - ring, - ring->last_context); - ring->last_context->engine[ring->id].dirty = false; - } - ring->last_context = req->ctx; -} - static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req) { int ret, i; @@ -2367,76 +2351,6 @@ populate_lr_context(struct intel_context *ctx, struct drm_i915_gem_object *ctx_o return 0; } -/** - * intel_lr_context_clean_ring() - clean the ring specific parts of an LRC - * @ctx: the LR context being freed. - * @ring: the engine being cleaned - * @ctx_obj: the hw context being unreferenced - * @ringbuf: the ringbuf being freed - * - * Take care of cleaning up the per-engine backing - * objects and the logical ringbuffer. - */ -static void -intel_lr_context_clean_ring(struct intel_context *ctx, - struct intel_engine_cs *ring, - struct drm_i915_gem_object *ctx_obj, - struct intel_ringbuffer *ringbuf) -{ - int ret; - - WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex)); - - if (ctx == ring->default_context) { - intel_unpin_ringbuffer_obj(ringbuf); - i915_gem_object_ggtt_unpin(ctx_obj); - } - - if (ctx->engine[ring->id].dirty) { - struct drm_i915_gem_request *req = NULL; - - /** - * If there is already a request pending on - * this ring, wait for that to complete, - * otherwise create a switch to idle request - */ - if (list_empty(&ring->request_list)) { - int ret; - - ret = i915_gem_request_alloc( - ring, - ring->default_context, - &req); - if (!ret) - i915_add_request(req); - else - DRM_DEBUG("Failed to ensure context saved"); - } else { - req = list_first_entry( - &ring->request_list, - typeof(*req), list); - } - if (req) { - ret = i915_wait_request(req); - if (ret != 0) { - /** - * If we get here, there's probably been a ring - * reset, so we just clean up the dirty flag.& - * pin count. - */ - ctx->engine[ring->id].dirty = false; - __intel_lr_context_unpin( - ring, - ctx); - } - } - } - - WARN_ON(ctx->engine[ring->id].pin_count); - intel_ringbuffer_free(ringbuf); - drm_gem_object_unreference(&ctx_obj->base); -} - /** * intel_lr_context_free() - free the LRC specific bits of a context * @ctx: the LR context to free. @@ -2449,7 +2363,7 @@ void intel_lr_context_free(struct intel_context *ctx) { int i; - for (i = 0; i < I915_NUM_RINGS; ++i) { + for (i = 0; i < I915_NUM_RINGS; i++) { struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state; if (ctx_obj) { @@ -2457,10 +2371,13 @@ void intel_lr_context_free(struct intel_context *ctx) ctx->engine[i].ringbuf; struct intel_engine_cs *ring = ringbuf->ring; - intel_lr_context_clean_ring(ctx, - ring, - ctx_obj, - ringbuf); + if (ctx == ring->default_context) { + intel_unpin_ringbuffer_obj(ringbuf); + i915_gem_object_ggtt_unpin(ctx_obj); + } + WARN_ON(ctx->engine[ring->id].pin_count); + intel_ringbuffer_free(ringbuf); + drm_gem_object_unreference(&ctx_obj->base); } } } @@ -2622,12 +2539,5 @@ void intel_lr_context_reset(struct drm_device *dev, ringbuf->head = 0; ringbuf->tail = 0; - - if (ctx->engine[ring->id].dirty) { - __intel_lr_context_unpin( - ring, - ctx); - ctx->engine[ring->id].dirty = false; - } } } diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index fd1b6b43bacc..0b821b91723a 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -91,7 +91,6 @@ void intel_lr_context_reset(struct drm_device *dev, struct intel_context *ctx); uint64_t intel_lr_context_descriptor(struct intel_context *ctx, struct intel_engine_cs *ring); -void intel_lr_context_complete_check(struct drm_i915_gem_request *req); /* Execlists */ int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists); From 15620206ae87ba9643ffa6f5ddb5471be7192006 Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Fri, 6 Nov 2015 14:11:16 +0200 Subject: [PATCH 69/70] drm/i915/skl: Add SKL GT4 PCI IDs Add Skylake Intel Graphics GT4 PCI IDs v2: Rebase Signed-off-by: Mika Kuoppala Reviewed-by: Damien Lespiau Signed-off-by: Damien Lespiau Link: http://patchwork.freedesktop.org/patch/msgid/1446811876-303-1-git-send-email-mika.kuoppala@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 1 + include/drm/i915_pciids.h | 13 ++++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 46ac66484dc7..e6935f1cb689 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -399,6 +399,7 @@ static const struct pci_device_id pciidlist[] = { INTEL_SKL_GT1_IDS(&intel_skylake_info), INTEL_SKL_GT2_IDS(&intel_skylake_info), INTEL_SKL_GT3_IDS(&intel_skylake_gt3_info), + INTEL_SKL_GT4_IDS(&intel_skylake_gt3_info), INTEL_BXT_IDS(&intel_broxton_info), INTEL_KBL_GT1_IDS(&intel_kabylake_info), INTEL_KBL_GT2_IDS(&intel_kabylake_info), diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index f1a113e35f98..f97020904717 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -279,12 +279,19 @@ #define INTEL_SKL_GT3_IDS(info) \ INTEL_VGA_DEVICE(0x1926, info), /* ULT GT3 */ \ INTEL_VGA_DEVICE(0x192B, info), /* Halo GT3 */ \ - INTEL_VGA_DEVICE(0x192A, info) /* SRV GT3 */ \ + INTEL_VGA_DEVICE(0x192A, info) /* SRV GT3 */ -#define INTEL_SKL_IDS(info) \ +#define INTEL_SKL_GT4_IDS(info) \ + INTEL_VGA_DEVICE(0x1932, info), /* DT GT4 */ \ + INTEL_VGA_DEVICE(0x193B, info), /* Halo GT4 */ \ + INTEL_VGA_DEVICE(0x193D, info), /* WKS GT4 */ \ + INTEL_VGA_DEVICE(0x193A, info) /* SRV GT4 */ + +#define INTEL_SKL_IDS(info) \ INTEL_SKL_GT1_IDS(info), \ INTEL_SKL_GT2_IDS(info), \ - INTEL_SKL_GT3_IDS(info) + INTEL_SKL_GT3_IDS(info), \ + INTEL_SKL_GT4_IDS(info) #define INTEL_BXT_IDS(info) \ INTEL_VGA_DEVICE(0x0A84, info), \ From 03a97d825573de7b6ff1b44f257345efbff2161a Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 4 Dec 2015 17:55:47 +0100 Subject: [PATCH 70/70] drm/i915: Update DRIVER_DATE to 20151204 Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5e276f5d61da..5b3830ef44df 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -57,7 +57,7 @@ #define DRIVER_NAME "i915" #define DRIVER_DESC "Intel Graphics" -#define DRIVER_DATE "20151120" +#define DRIVER_DATE "20151204" #undef WARN_ON /* Many gcc seem to no see through this and fall over :( */