Commit a8a8bd547e6323c56295e1c5a03e30e765d42325

Authored by Paulo Zanoni
Committed by Daniel Vetter
1 parent b4d2a9a093

drm/i915: make PC8 be part of runtime PM suspend/resume

Currently, when our driver becomes idle for i915.pc8_timeout (default:
5s) we enable PC8, so we save some power, but not everything we can.
Then, while PC8 is enabled, if we stay idle for more
autosuspend_delay_ms (default: 10s) we'll enter runtime PM and put the
graphics device in D3 state, saving even more power. The two features
are separate things with increasing levels of power savings, but if we
disable PC8 we'll never get into D3.

While from the modularity point of view it would be nice to keep these
features as separate, we have reasons to merge them:
 - We are not aware of anybody wanting a "PC8 without D3" environment.
 - If we keep both features as separate, we'll have to to test both
   PC8 and PC8+D3 code paths. We're already having a major pain to
   make QA do automated testing of just one thing, testing both paths
   will cost even more.
 - Only Haswell+ supports PC8, so if we want to add runtime PM support
   to, for example, IVB, we'll have to copy some code from the PC8
   feature to runtime PM, so merging both features as a single thing
   will make it easier for enabling runtime PM on other platforms.

This patch only does the very basic steps required to have PC8 and
runtime PM merged on a single feature: the next patches will take care
of cleaning up everything.

v2: - Rebase.
v3: - Rebase.
    - Fully remove the deprecated i915 params since Daniel doesn't
      consider them as part of the ABI.
v4: - Rebase.
    - Fix typo in the commit message.
v5: - Rebase, again.
    - Add a huge comment explaining the different forcewake usage
      (Chris, Daniel).
    - Use open-coded forcewake functions (Daniel).

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Showing 7 changed files with 43 additions and 53 deletions Side-by-side Diff

drivers/gpu/drm/i915/i915_dma.c
... ... @@ -1823,8 +1823,6 @@
1823 1823 cancel_work_sync(&dev_priv->gpu_error.work);
1824 1824 i915_destroy_error_state(dev);
1825 1825  
1826   - cancel_delayed_work_sync(&dev_priv->pc8.enable_work);
1827   -
1828 1826 if (dev->pdev->msi_enabled)
1829 1827 pci_disable_msi(dev->pdev);
1830 1828  
drivers/gpu/drm/i915/i915_drv.c
... ... @@ -849,6 +849,9 @@
849 849  
850 850 DRM_DEBUG_KMS("Suspending device\n");
851 851  
  852 + if (HAS_PC8(dev))
  853 + __hsw_do_enable_pc8(dev_priv);
  854 +
852 855 i915_gem_release_all_mmaps(dev_priv);
853 856  
854 857 del_timer_sync(&dev_priv->gpu_error.hangcheck_timer);
... ... @@ -863,6 +866,7 @@
863 866 */
864 867 intel_opregion_notify_adapter(dev, PCI_D1);
865 868  
  869 + DRM_DEBUG_KMS("Device suspended\n");
866 870 return 0;
867 871 }
868 872  
... ... @@ -879,6 +883,10 @@
879 883 intel_opregion_notify_adapter(dev, PCI_D0);
880 884 dev_priv->pm.suspended = false;
881 885  
  886 + if (HAS_PC8(dev))
  887 + __hsw_do_disable_pc8(dev_priv);
  888 +
  889 + DRM_DEBUG_KMS("Device resumed\n");
882 890 return 0;
883 891 }
884 892  
drivers/gpu/drm/i915/i915_drv.h
... ... @@ -1339,6 +1339,10 @@
1339 1339 /*
1340 1340 * This struct tracks the state needed for the Package C8+ feature.
1341 1341 *
  1342 + * TODO: we're merging the Package C8+ feature with the runtime PM support. To
  1343 + * avoid having to update the documentation at each patch of the series, we'll
  1344 + * do a final update at the end.
  1345 + *
1342 1346 * Package states C8 and deeper are really deep PC states that can only be
1343 1347 * reached when all the devices on the system allow it, so even if the graphics
1344 1348 * device allows PC8+, it doesn't mean the system will actually get to these
... ... @@ -1392,7 +1396,6 @@
1392 1396 bool enabled;
1393 1397 int disable_count;
1394 1398 struct mutex lock;
1395   - struct delayed_work enable_work;
1396 1399  
1397 1400 struct {
1398 1401 uint32_t deimr;
... ... @@ -2095,8 +2098,6 @@
2095 2098 unsigned int preliminary_hw_support;
2096 2099 int disable_power_well;
2097 2100 int enable_ips;
2098   - int enable_pc8;
2099   - int pc8_timeout;
2100 2101 int invert_brightness;
2101 2102 int enable_cmd_parser;
2102 2103 /* leave bools at the end to not create holes */
drivers/gpu/drm/i915/i915_params.c
... ... @@ -42,8 +42,6 @@
42 42 .disable_power_well = 1,
43 43 .enable_ips = 1,
44 44 .fastboot = 0,
45   - .enable_pc8 = 1,
46   - .pc8_timeout = 5000,
47 45 .prefault_disable = 0,
48 46 .reset = true,
49 47 .invert_brightness = 0,
... ... @@ -134,14 +132,6 @@
134 132 module_param_named(fastboot, i915.fastboot, bool, 0600);
135 133 MODULE_PARM_DESC(fastboot,
136 134 "Try to skip unnecessary mode sets at boot time (default: false)");
137   -
138   -module_param_named(enable_pc8, i915.enable_pc8, int, 0600);
139   -MODULE_PARM_DESC(enable_pc8,
140   - "Enable support for low power package C states (PC8+) (default: true)");
141   -
142   -module_param_named(pc8_timeout, i915.pc8_timeout, int, 0600);
143   -MODULE_PARM_DESC(pc8_timeout,
144   - "Number of msecs of idleness required to enter PC8+ (default: 5000)");
145 135  
146 136 module_param_named(prefault_disable, i915.prefault_disable, bool, 0600);
147 137 MODULE_PARM_DESC(prefault_disable,
drivers/gpu/drm/i915/intel_display.c
... ... @@ -6960,6 +6960,7 @@
6960 6960 static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
6961 6961 {
6962 6962 uint32_t val;
  6963 + unsigned long irqflags;
6963 6964  
6964 6965 val = I915_READ(LCPLL_CTL);
6965 6966  
... ... @@ -6967,9 +6968,22 @@
6967 6968 LCPLL_POWER_DOWN_ALLOW)) == LCPLL_PLL_LOCK)
6968 6969 return;
6969 6970  
6970   - /* Make sure we're not on PC8 state before disabling PC8, otherwise
6971   - * we'll hang the machine! */
6972   - gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
  6971 + /*
  6972 + * Make sure we're not on PC8 state before disabling PC8, otherwise
  6973 + * we'll hang the machine. To prevent PC8 state, just enable force_wake.
  6974 + *
  6975 + * The other problem is that hsw_restore_lcpll() is called as part of
  6976 + * the runtime PM resume sequence, so we can't just call
  6977 + * gen6_gt_force_wake_get() because that function calls
  6978 + * intel_runtime_pm_get(), and we can't change the runtime PM refcount
  6979 + * while we are on the resume sequence. So to solve this problem we have
  6980 + * to call special forcewake code that doesn't touch runtime PM and
  6981 + * doesn't enable the forcewake delayed work.
  6982 + */
  6983 + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
  6984 + if (dev_priv->uncore.forcewake_count++ == 0)
  6985 + dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_ALL);
  6986 + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
6973 6987  
6974 6988 if (val & LCPLL_POWER_DOWN_ALLOW) {
6975 6989 val &= ~LCPLL_POWER_DOWN_ALLOW;
6976 6990  
6977 6991  
... ... @@ -7003,14 +7017,20 @@
7003 7017 DRM_ERROR("Switching back to LCPLL failed\n");
7004 7018 }
7005 7019  
7006   - gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
  7020 + /* See the big comment above. */
  7021 + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
  7022 + if (--dev_priv->uncore.forcewake_count == 0)
  7023 + dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL);
  7024 + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
7007 7025 }
7008 7026  
7009   -static void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
  7027 +void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv)
7010 7028 {
7011 7029 struct drm_device *dev = dev_priv->dev;
7012 7030 uint32_t val;
7013 7031  
  7032 + WARN_ON(!HAS_PC8(dev));
  7033 +
7014 7034 DRM_DEBUG_KMS("Enabling package C8+\n");
7015 7035  
7016 7036 dev_priv->pc8.enabled = true;
... ... @@ -7026,22 +7046,6 @@
7026 7046 hsw_disable_lcpll(dev_priv, true, true);
7027 7047 }
7028 7048  
7029   -void hsw_enable_pc8_work(struct work_struct *__work)
7030   -{
7031   - struct drm_i915_private *dev_priv =
7032   - container_of(to_delayed_work(__work), struct drm_i915_private,
7033   - pc8.enable_work);
7034   - struct drm_device *dev = dev_priv->dev;
7035   -
7036   - WARN_ON(!HAS_PC8(dev));
7037   -
7038   - if (dev_priv->pc8.enabled)
7039   - return;
7040   -
7041   - __hsw_do_enable_pc8(dev_priv);
7042   - intel_runtime_pm_put(dev_priv);
7043   -}
7044   -
7045 7049 static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
7046 7050 {
7047 7051 WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
7048 7052  
7049 7053  
... ... @@ -7052,15 +7056,16 @@
7052 7056 if (dev_priv->pc8.disable_count != 0)
7053 7057 return;
7054 7058  
7055   - schedule_delayed_work(&dev_priv->pc8.enable_work,
7056   - msecs_to_jiffies(i915.pc8_timeout));
  7059 + intel_runtime_pm_put(dev_priv);
7057 7060 }
7058 7061  
7059   -static void __hsw_do_disable_package_c8(struct drm_i915_private *dev_priv)
  7062 +void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv)
7060 7063 {
7061 7064 struct drm_device *dev = dev_priv->dev;
7062 7065 uint32_t val;
7063 7066  
  7067 + WARN_ON(!HAS_PC8(dev));
  7068 +
7064 7069 DRM_DEBUG_KMS("Disabling package C8+\n");
7065 7070  
7066 7071 hsw_restore_lcpll(dev_priv);
... ... @@ -7083,8 +7088,6 @@
7083 7088  
7084 7089 static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
7085 7090 {
7086   - struct drm_device *dev = dev_priv->dev;
7087   -
7088 7091 WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
7089 7092 WARN(dev_priv->pc8.disable_count < 0,
7090 7093 "pc8.disable_count: %d\n", dev_priv->pc8.disable_count);
7091 7094  
... ... @@ -7093,14 +7096,7 @@
7093 7096 if (dev_priv->pc8.disable_count != 1)
7094 7097 return;
7095 7098  
7096   - WARN_ON(!HAS_PC8(dev));
7097   -
7098   - cancel_delayed_work_sync(&dev_priv->pc8.enable_work);
7099   - if (!dev_priv->pc8.enabled)
7100   - return;
7101   -
7102 7099 intel_runtime_pm_get(dev_priv);
7103   - __hsw_do_disable_package_c8(dev_priv);
7104 7100 }
7105 7101  
7106 7102 void hsw_enable_package_c8(struct drm_i915_private *dev_priv)
... ... @@ -7156,9 +7152,6 @@
7156 7152 bool allow;
7157 7153  
7158 7154 if (!HAS_PC8(dev_priv->dev))
7159   - return;
7160   -
7161   - if (!i915.enable_pc8)
7162 7155 return;
7163 7156  
7164 7157 mutex_lock(&dev_priv->pc8.lock);
drivers/gpu/drm/i915/intel_drv.h
... ... @@ -725,7 +725,8 @@
725 725 unsigned int bpp,
726 726 unsigned int pitch);
727 727 void intel_display_handle_reset(struct drm_device *dev);
728   -void hsw_enable_pc8_work(struct work_struct *__work);
  728 +void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv);
  729 +void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv);
729 730 void hsw_enable_package_c8(struct drm_i915_private *dev_priv);
730 731 void hsw_disable_package_c8(struct drm_i915_private *dev_priv);
731 732 void intel_dp_get_m_n(struct intel_crtc *crtc,
drivers/gpu/drm/i915/intel_pm.c
... ... @@ -6161,7 +6161,6 @@
6161 6161 dev_priv->pc8.irqs_disabled = false;
6162 6162 dev_priv->pc8.enabled = false;
6163 6163 dev_priv->pc8.disable_count = 1; /* requirements_met */
6164   - INIT_DELAYED_WORK(&dev_priv->pc8.enable_work, hsw_enable_pc8_work);
6165 6164 INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
6166 6165 intel_gen6_powersave_work);
6167 6166 }