From: Paulo Zanoni <przanoni@gmail.com>
To: intel-gfx@lists.freedesktop.org
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: [PATCH 5/6] drm/i915: add i915.pc8_timeout function
Date: Mon, 19 Aug 2013 13:18:11 -0300 [thread overview]
Message-ID: <1376929092-3968-6-git-send-email-przanoni@gmail.com> (raw)
In-Reply-To: <1376929092-3968-1-git-send-email-przanoni@gmail.com>
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
We currently only enter PC8+ after all its required conditions are
met, there's no rendering, and we stay like that for at least 5
seconds.
I chose "5 seconds" because this value is conservative and won't make
us enter/leave PC8+ thousands of times after the screen is off: some
desktop environments have applications that wake up and do rendering
every 1-3 seconds, even when the screen is off and the machine is
completely idle.
But when I was testing my PC8+ patches I set the default value to
100ms so I could use the bad-behaving desktop environments to
stress-test my patches. I also thought it would be a good idea to ask
our power management team to test different values, but I'm pretty
sure they would ask me for an easy way to change the timeout. So to
help these 2 cases I decided to create an option that would make it
easier to change the default value. I also expect people making
specific products that use our driver could try to find the perfect
timeout for them.
Anyway, fixing the bad-behaving applications will always lead to
better power savings than just changing the timeout value: you need to
stop waking the Kernel, not quickly put it back to sleep again after
you wake it for nothing. Bad sleep leads to bad mood!
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 4 ++++
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 2 +-
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f197362..ad28a72 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -145,6 +145,10 @@ int i915_enable_pc8 __read_mostly = 0;
module_param_named(enable_pc8, i915_enable_pc8, int, 0600);
MODULE_PARM_DESC(enable_pc8, "Enable support for low power package C states (PC8+) (default: false)");
+int i915_pc8_timeout __read_mostly = 5000;
+module_param_named(pc8_timeout, i915_pc8_timeout, int, 0600);
+MODULE_PARM_DESC(pc8_timeout, "Number of msecs of idleness required to enter PC8+ (default: 5000)");
+
bool i915_prefault_disable __read_mostly;
module_param_named(prefault_disable, i915_prefault_disable, bool, 0600);
MODULE_PARM_DESC(prefault_disable,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 34c5af5..3138083 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1710,6 +1710,7 @@ extern int i915_disable_power_well __read_mostly;
extern int i915_enable_ips __read_mostly;
extern bool i915_fastboot __read_mostly;
extern int i915_enable_pc8 __read_mostly;
+extern int i915_pc8_timeout __read_mostly;
extern bool i915_prefault_disable __read_mostly;
extern int i915_suspend(struct drm_device *dev, pm_message_t state);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7d5b01a..ba3bdd5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6099,7 +6099,7 @@ static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
return;
schedule_delayed_work(&dev_priv->pc8.enable_work,
- msecs_to_jiffies(5 * 1000));
+ msecs_to_jiffies(i915_pc8_timeout));
}
static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
--
1.8.1.2
next prev parent reply other threads:[~2013-08-19 16:18 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-19 16:18 [PATCH 0/6] Final PC8+ patches Paulo Zanoni
2013-08-19 16:18 ` [PATCH 1/6] drm/i915: grab force_wake when restoring LCPLL Paulo Zanoni
2013-08-19 18:25 ` Chris Wilson
2013-08-19 19:01 ` Paulo Zanoni
2013-08-19 22:53 ` Ben Widawsky
2013-08-20 14:27 ` Paulo Zanoni
2013-08-19 16:18 ` [PATCH 2/6] drm/i915: fix SDEIMR assertion when disabling LCPLL Paulo Zanoni
2013-08-21 15:37 ` Rodrigo Vivi
2013-08-19 16:18 ` [PATCH 3/6] drm/i915: allow package C8+ states on Haswell (disabled) Paulo Zanoni
2013-08-19 18:29 ` Chris Wilson
2013-08-19 16:18 ` [PATCH 4/6] drm/i915: add i915_pc8_status debugfs file Paulo Zanoni
2013-08-21 15:42 ` Rodrigo Vivi
2013-08-19 16:18 ` Paulo Zanoni [this message]
2013-08-21 15:43 ` [PATCH 5/6] drm/i915: add i915.pc8_timeout function Rodrigo Vivi
2013-08-19 16:18 ` [PATCH 6/6] drm/i915: enable Package C8+ by default Paulo Zanoni
2013-08-21 15:43 ` Rodrigo Vivi
2013-08-22 14:13 ` Daniel Vetter
2013-08-19 16:20 ` [PATCH igt] tests: add pc8 Paulo Zanoni
2013-08-20 20:18 ` Daniel Vetter
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=1376929092-3968-6-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