All of lore.kernel.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 15/16] drm/i915: update the PC8 and runtime PM documentation
Date: Fri,  7 Mar 2014 20:08:18 -0300	[thread overview]
Message-ID: <1394233699-3741-16-git-send-email-przanoni@gmail.com> (raw)
In-Reply-To: <1394233699-3741-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.

v2: - Rebase.
    - Fix typo (Jesse).

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      | 52 +++++++++---------------------------
 drivers/gpu/drm/i915/intel_display.c | 23 ++++++++++++++++
 2 files changed, 35 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b741298..70feb61 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1333,47 +1333,19 @@ 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.
- *
- * 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 variable. When we switch
- * requirements_met 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.
- *
- * 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 from 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
@@ -1383,7 +1355,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 53e480a..ed9233e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6790,6 +6790,29 @@ static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+/*
+ * 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 most interrupts are disabled, and these are also
+ * 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 the hardware
+ * documentation.
+ */
 void hsw_enable_pc8(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
-- 
1.8.5.3

  parent reply	other threads:[~2014-03-07 23:08 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-07 23:08 [PATCH 00/16] Merge PC8 with runtime PM, v3 Paulo Zanoni
2014-03-07 23:08 ` [PATCH 01/16] drm/i915: extract __hsw_do_{en, dis}able_package_c8 Paulo Zanoni
2014-03-07 23:08 ` [PATCH 02/16] drm/i915: make PC8 be part of runtime PM suspend/resume Paulo Zanoni
2014-03-19 15:07   ` Imre Deak
2014-03-19 15:45     ` Daniel Vetter
2014-03-07 23:08 ` [PATCH 03/16] drm/i915: get/put runtime PM when we get/put a power domain Paulo Zanoni
2014-03-07 23:08 ` [PATCH 04/16] drm/i915: remove dev_priv->pc8.requirements_met Paulo Zanoni
2014-03-19 15:14   ` Imre Deak
2014-03-07 23:08 ` [PATCH 05/16] drm/i915: get runtime PM references when the GPU is idle/busy Paulo Zanoni
2014-03-07 23:08 ` [PATCH 06/16] drm/i915: kill pc8.disable_count Paulo Zanoni
2014-03-07 23:08 ` [PATCH 07/16] drm/i915: remove an indirection level on PC8 functions Paulo Zanoni
2014-03-11  8:20   ` Chris Wilson
2014-03-11 14:56     ` Daniel Vetter
2014-03-07 23:08 ` [PATCH 08/16] drm/i915: don't get/put PC8 reference on freeze/thaw Paulo Zanoni
2014-03-07 23:08 ` [PATCH 09/16] drm/i915: make intel_aux_display_runtime_get get runtime PM, not PC8 Paulo Zanoni
2014-03-19 15:23   ` Imre Deak
2014-03-07 23:08 ` [PATCH 10/16] drm/i915: don't get/put PC8 when getting/putting power wells Paulo Zanoni
2014-03-07 23:08 ` [PATCH 11/16] drm/i915: remove dev_priv->pc8.enabled Paulo Zanoni
2014-03-07 23:08 ` [PATCH 12/16] drm/i915: move pc8.irqs_disabled to pm.irqs_disabled Paulo Zanoni
2014-03-07 23:08 ` [PATCH 13/16] drm/i915: kill struct i915_package_c8 Paulo Zanoni
2014-03-07 23:08 ` [PATCH 14/16] drm/i915: rename __hsw_do_{en, dis}able_pc8 Paulo Zanoni
2014-03-07 23:08 ` Paulo Zanoni [this message]
2014-03-07 23:08 ` [PATCH 16/16] 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=1394233699-3741-16-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.