public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Paulo Zanoni <przanoni@gmail.com>
To: intel-gfx@lists.freedesktop.org
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: [PATCH 18/19] drm/i915: update the PC8 and runtime PM documentation.
Date: Thu, 19 Dec 2013 11:55:08 -0200	[thread overview]
Message-ID: <1387461309-2756-19-git-send-email-przanoni@gmail.com> (raw)
In-Reply-To: <1387461309-2756-1-git-send-email-przanoni@gmail.com>

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Now that PC8 got much simpler, there are less things to document.
Also, runtime PM already has a nice documentation, so we don't need to
re-explain it on our driver.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      | 56 +++++++++++-------------------------
 drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++
 2 files changed, 38 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 56434c6..b24b2cd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1254,48 +1254,24 @@ struct ilk_wm_values {
 };
 
 /*
- * This struct tracks the state needed for the Package C8+ feature.
+ * This struct helps tracking the state needed for runtime PM, which puts the
+ * device in PCI D3 state. Notice that when this happens, nothing on the
+ * graphics device works, even register access, so we don't get interrupts nor
+ * anything else.
  *
- * TODO: we're merging the Package C8+ feature with the runtime PM support. To
- * avoid having to update the documentation at each patch of the series, we'll
- * do a final update at the end.
+ * Every piece of our code that needs to actually touch the hardware needs to
+ * either call intel_runtime_pm_get or call intel_display_power_get with the
+ * appropriate power domain.
  *
- * Package states C8 and deeper are really deep PC states that can only be
- * reached when all the devices on the system allow it, so even if the graphics
- * device allows PC8+, it doesn't mean the system will actually get to these
- * states.
+ * The gpu_idle variable is just used as a way to make sure that we actually do
+ * put/get calls symmetrically. When we change it to true, we put the runtime PM
+ * reference, and when we change it to false, we get the reference. With this,
+ * we can call intel_runtime_pm_gpu_{idle,busy} asymmetrically without problems.
  *
- * Our driver only allows PC8+ when all the outputs are disabled, the power well
- * is disabled and the GPU is idle. When these conditions are met, we manually
- * do the other conditions: disable the interrupts, clocks and switch LCPLL
- * refclk to Fclk.
- *
- * When we really reach PC8 or deeper states (not just when we allow it) we lose
- * the state of some registers, so when we come back from PC8+ we need to
- * restore this state. We don't get into PC8+ if we're not in RC6, so we don't
- * need to take care of the registers kept by RC6.
- *
- * The interrupt disabling is part of the requirements. We can only leave the
- * PCH HPD interrupts enabled. If we're in PC8+ and we get another interrupt we
- * can lock the machine.
- *
- * Ideally every piece of our code that needs PC8+ disabled would call
- * hsw_disable_package_c8, which would increment disable_count and prevent the
- * system from reaching PC8+. But we don't have a symmetric way to do this for
- * everything, so we have the requirements_met and gpu_idle variables. When we
- * switch requirements_met or gpu_idle to true we decrease disable_count, and
- * increase it in the opposite case. The requirements_met variable is true when
- * all the CRTCs, encoders and the power well are disabled. The gpu_idle
- * variable is true when the GPU is idle.
- *
- * In addition to everything, we only actually enable PC8+ if disable_count
- * stays at zero for at least some seconds. This is implemented with the
- * enable_work variable. We do this so we don't enable/disable PC8 dozens of
- * consecutive times when all screens are disabled and some background app
- * queries the state of our connectors, or we have some application constantly
- * waking up to use the GPU. Only after the enable_work function actually
- * enables PC8+ the "enable" variable will become true, which means that it can
- * be false even if disable_count is 0.
+ * Our driver uses the autosuspend delay feature, which means we'll only really
+ * suspend if we stay with zero refcount for a certain amount of time. The
+ * default value is currently very conservative (see intel_init_runtime_pm), but
+ * it can be changed with the standard runtime PM files frmo sysfs.
  *
  * The irqs_disabled variable becomes true exactly after we disable the IRQs and
  * goes back to false exactly before we reenable the IRQs. We use this variable
@@ -1305,7 +1281,7 @@ struct ilk_wm_values {
  * inside struct regsave so when we restore the IRQs they will contain the
  * latest expected values.
  *
- * For more, read "Display Sequences for Package C8" on our documentation.
+ * For more, read the Documentation/power/runtime_pm.txt.
  */
 struct i915_runtime_pm {
 	bool suspended;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9f451a5..6b6be48 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6638,6 +6638,28 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
 	gen6_gt_force_wake_put_no_rpm(dev_priv, FORCEWAKE_ALL);
 }
 
+/*
+ * Package states C8 and deeper are really deep PC states that can only be
+ * reached when all the devices on the system allow it, so even if the graphics
+ * device allows PC8+, it doesn't mean the system will actually get to these
+ * states. Our driver only allows PC8+ when going into runtime PM.
+ *
+ * The requirements for PC8+ are that all the outputs are disabled, the power
+ * well is disabled and the GPU is idle, and these are also the requirements for
+ * runtime PM. When these conditions are met, we manually do the other
+ * conditions: disable the interrupts, clocks and switch LCPLL refclk to Fclk.
+ * If we're in PC8+ and we get an non-hotplug interrupt, we can hard hang the
+ * machine.
+ *
+ * When we really reach PC8 or deeper states (not just when we allow it) we lose
+ * the state of some registers, so when we come back from PC8+ we need to
+ * restore this state. We don't get into PC8+ if we're not in RC6, so we don't
+ * need to take care of the registers kept by RC6. Notice that this happens even
+ * if we don't put the device in PCI D3 state (which is what currently happens
+ * because of the runtime PM support).
+ *
+ * For more, read "Display Sequences for Package C8" on our documentation.
+ */
 void hsw_enable_pc8(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
-- 
1.8.3.1

  parent reply	other threads:[~2013-12-19 13:55 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-19 13:54 [PATCH 00/19] Make PC8 become part of runtime PM Paulo Zanoni
2013-12-19 13:54 ` [PATCH 01/19] drm/i915: rename modeset_update_power_wells Paulo Zanoni
2014-02-27 13:44   ` Imre Deak
2013-12-19 13:54 ` [PATCH 02/19] drm/i915: get/put runtime PM without holding rps.hw_lock Paulo Zanoni
2013-12-19 18:30   ` Jesse Barnes
2013-12-19 21:20     ` Daniel Vetter
2013-12-19 21:31       ` Paulo Zanoni
2014-02-27 13:45   ` Imre Deak
2013-12-19 13:54 ` [PATCH 03/19] drm/i915: add forcewake functions that don't touch runtime PM Paulo Zanoni
2014-02-27 14:43   ` Imre Deak
2014-02-27 14:48     ` Paulo Zanoni
2014-02-27 15:24       ` Imre Deak
2014-03-04 14:18       ` Daniel Vetter
2013-12-19 13:54 ` [PATCH 04/19] drm/i915: extract __hsw_do_{en, dis}able_package_c8 Paulo Zanoni
2014-02-27 14:49   ` Imre Deak
2013-12-19 13:54 ` [PATCH 05/19] drm/i915: make PC8 be part of runtime PM suspend/resume Paulo Zanoni
2013-12-19 13:54 ` [PATCH 06/19] drm/i915: get/put runtime PM when we get/put a power domain Paulo Zanoni
2014-01-24 19:58   ` Paulo Zanoni
2013-12-19 13:54 ` [PATCH 07/19] drm/i915: remove dev_priv->pc8.requirements_met Paulo Zanoni
2013-12-19 13:54 ` [PATCH 08/19] drm/i915: make gpu_idle be part of runtime PM, not PC8 Paulo Zanoni
2013-12-19 13:54 ` [PATCH 09/19] drm/i915: kill pc8.disable_count Paulo Zanoni
2013-12-19 13:55 ` [PATCH 10/19] drm/i915: remove an indirection level on PC8 functions Paulo Zanoni
2013-12-19 13:55 ` [PATCH 11/19] drm/i915: don't get/put PC8 reference on freeze/thaw Paulo Zanoni
2013-12-19 13:55 ` [PATCH 12/19] drm/i915: get/put runtime PM references for GMBUS and DP AUX Paulo Zanoni
2013-12-19 13:55 ` [PATCH 13/19] drm/i915: don't get/put PC8 when getting/putting power wells Paulo Zanoni
2013-12-19 13:55 ` [PATCH 14/19] drm/i915: remove dev_priv->pc8.enabled Paulo Zanoni
2013-12-19 13:55 ` [PATCH 15/19] drm/i915: move pc8.irqs_disabled to pm.irqs_disabled Paulo Zanoni
2013-12-19 13:55 ` [PATCH 16/19] drm/i915: kill struct i915_package_c8 Paulo Zanoni
2013-12-19 13:55 ` [PATCH 17/19] drm/i915: rename __hsw_do_{en, dis}able_pc8 Paulo Zanoni
2013-12-19 13:55 ` Paulo Zanoni [this message]
2013-12-19 13:55 ` [PATCH 19/19] drm/i915: init pm.suspended earlier Paulo Zanoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1387461309-2756-19-git-send-email-przanoni@gmail.com \
    --to=przanoni@gmail.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox