login
Search
I think you should instead fix your hardware or maybe change --
How can I know if this laptop is broken or not? According to Jesse's reply, the BIOS lowered the limit, which could explain why it hits the limit more often: intel ips 0000:00:1f.6: Warning: CPU TDP doesn't match expected value (found 25, expected 35) The thermal throttling seems to be also managed by the BIOS (and there are no settings about it on the BIOS setup that I remember): CPU: Physical Processor ID: 0 CPU: Processor Core ID: 0 mce: CPU supports 9 MCE banks CPU0: Thermal monitoring handled by SMI using mwait in idle threads. [...] CPU0: Intel(R) Core(TM) i3 CPU M 330 @ 2.13GHz stepping 02 Booting Node 0, Processors #1 CPU1: Thermal monitoring handled by SMI #2 CPU2: Thermal monitoring handled by SMI #3 Ok. CPU3: Thermal monitoring handled by SMI Brought up 4 CPUs And ACPI is of no help, it shows two thermal zones, one always showing 27 C, the other always showing 0 C (and even though that is below the reported thresholds of 55 C and 71 C for the fans, the fan still speeds up and down on its own, showing that it is just the reporting that is broken). The fan does slow down to almost nothing (or even off) when idle and spins up to a strong hot breeze when compiling the kernel (make -j8), so the thermal monitoring on the BIOS seems to be working fine. I would expect broken hardware to run the fan on high speed all the time (not cooling enough) or low speed all the time (thermal sensor broken). The coretemp module (which is not autoloaded) seems to be more helpful; it shows between 43000 and 45000 for temp1_input on both cores when idle, going up to 50000-56000 when lightly loaded (I have not looked at it yet while doing a heavy compile). Is there a way to know if all this is just an oddness of this model, or if there is something which is not working quite right? (All the output above is from 2.6.35.3; I am not running 2.3.36-rc2+ right now because it hangs on resume, and I have not yet had the time to -- Cesar ...
Perhaps you might try this patch and get a bit more information.
It seems a sensible patch and perhaps should be applied anyway.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/platform/x86/intel_ips.c | 58 +++++++++++++++++++++++++++++--------
1 files changed, 45 insertions(+), 13 deletions(-)
diff --git a/drivers/platform/x86/intel_ips.c b/drivers/platform/x86/intel_ips.c
index 9024480..73f9ad1 100644
--- a/drivers/platform/x86/intel_ips.c
+++ b/drivers/platform/x86/intel_ips.c
@@ -598,17 +598,36 @@ static bool mcp_exceeded(struct ips_driver *ips)
{
unsigned long flags;
bool ret = false;
+ u16 mcp_avg_temp;
+ u16 mcp_temp_limit;
+ u16 mcp_power_limit;
+ u32 cpu_avg_power;
+ u32 mch_avg_power;
spin_lock_irqsave(&ips->turbo_status_lock, flags);
- if (ips->mcp_avg_temp > (ips->mcp_temp_limit * 100))
- ret = true;
- if (ips->cpu_avg_power + ips->mch_avg_power > ips->mcp_power_limit)
- ret = true;
+
+ mcp_avg_temp = ips->mcp_avg_temp;
+ mcp_temp_limit = ips->mcp_temp_limit;
+ mcp_power_limit = ips->mcp_power_limit;
+ cpu_avg_power = ips->cpu_avg_power;
+ mch_avg_power = ips->mch_avg_power;
+
spin_unlock_irqrestore(&ips->turbo_status_lock, flags);
- if (ret)
+ if ((cpu_avg_power + mch_avg_power) > mcp_power_limit) {
dev_info(&ips->dev->dev,
- "MCP power or thermal limit exceeded\n");
+ "MCP power limit %d exceeded: %d\n",
+ mcp_power_limit,
+ cpu_avg_power + mch_avg_power);
+ ret = true;
+ }
+ if (mcp_avg_temp > (mcp_temp_limit * 100)) {
+ dev_info(&ips->dev->dev,
+ "MCP thermal limit %d exceeded: %d\n",
+ mcp_temp_limit * 100,
+ mcp_avg_temp);
+ ret = true;
+ }
return ret;
}
@@ -623,20 +642,33 @@ static bool mcp_exceeded(struct ips_driver *ips)
static bool cpu_exceeded(struct ips_driver *ips, int cpu)
{
unsigned long flags;
- int avg;
bool ret = false;
+ int avg;
+ int core_temp_limit;
+ u16 core_power_limit;
+ u32 cpu_avg_power;
...Running with it right now. Unless I missed one, the messages do happen exactly every five seconds. Here are the first few lines of the dmesg (grepping for 'intel ips'), and it does seem a bit strange: $ dmesg | fgrep intel\ ips intel ips 0000:00:1f.6: Warning: CPU TDP doesn't match expected value (found 25, expected 35) intel ips 0000:00:1f.6: PCI INT C -> GSI 18 (level, low) -> IRQ 18 intel ips 0000:00:1f.6: IPS driver initialized, MCP temp limit 65535 intel ips 0000:00:1f.6: MCP power limit 65535 exceeded: 25615533 intel ips 0000:00:1f.6: CPU power limit 8183 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 6276 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 7952 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 7155 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 5928 exceeded: 0 intel ips 0000:00:1f.6: MCP power limit 65535 exceeded: 70848 intel ips 0000:00:1f.6: CPU power limit 6430 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 6474 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 5508 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 6569 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 5250 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 5023 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 6209 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 7276 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 9027 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 7008 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 5478 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 6658 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 5192 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 6347 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 5506 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 4447 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 4462 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 4382 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 4862 exceeded: 0 intel ips 0000:00:1f.6: CPU power limit 5218 ...
I added a logging message whenever the turbo limits change
and logging messages for power/temp on MCH for completeness.
Maybe this will show something useful like when/how
CPU power limit gets set to 0.
drivers/platform/x86/intel_ips.c | 105 +++++++++++++++++++++++++++++++-------
1 files changed, 87 insertions(+), 18 deletions(-)
diff --git a/drivers/platform/x86/intel_ips.c b/drivers/platform/x86/intel_ips.c
index 9024480..9c65e66 100644
--- a/drivers/platform/x86/intel_ips.c
+++ b/drivers/platform/x86/intel_ips.c
@@ -598,17 +598,36 @@ static bool mcp_exceeded(struct ips_driver *ips)
{
unsigned long flags;
bool ret = false;
+ u16 mcp_avg_temp;
+ u16 mcp_temp_limit;
+ u16 mcp_power_limit;
+ u32 cpu_avg_power;
+ u32 mch_avg_power;
spin_lock_irqsave(&ips->turbo_status_lock, flags);
- if (ips->mcp_avg_temp > (ips->mcp_temp_limit * 100))
- ret = true;
- if (ips->cpu_avg_power + ips->mch_avg_power > ips->mcp_power_limit)
- ret = true;
+
+ mcp_avg_temp = ips->mcp_avg_temp;
+ mcp_temp_limit = ips->mcp_temp_limit;
+ mcp_power_limit = ips->mcp_power_limit;
+ cpu_avg_power = ips->cpu_avg_power;
+ mch_avg_power = ips->mch_avg_power;
+
spin_unlock_irqrestore(&ips->turbo_status_lock, flags);
- if (ret)
+ if ((cpu_avg_power + mch_avg_power) > mcp_power_limit) {
+ dev_info(&ips->dev->dev,
+ "MCP power limit %u exceeded: cpu:%u + mch:%u\n",
+ mcp_power_limit,
+ cpu_avg_power, mch_avg_power);
+ ret = true;
+ }
+ if (mcp_avg_temp > (mcp_temp_limit * 100)) {
dev_info(&ips->dev->dev,
- "MCP power or thermal limit exceeded\n");
+ "MCP thermal limit %d exceeded: %d\n",
+ mcp_temp_limit * 100,
+ mcp_avg_temp);
+ ret = true;
+ }
return ret;
}
@@ -623,20 +642,33 @@ static bool mcp_exceeded(struct ips_driver *ips)
static bool cpu_exceeded(struct ips_driver *ips, int cpu)
{
unsigned long flags;
- int avg;
bool ret = false;
+ int avg;
+ int core_temp_limit;
+ u16 core_power_limit;
+ u32 cpu_avg_power;
...Running with it right now, did not help much: $ dmesg | fgrep 'intel ips' intel ips 0000:00:1f.6: Warning: CPU TDP doesn't match expected value (found 25, expected 35) intel ips 0000:00:1f.6: PCI INT C -> GSI 18 (level, low) -> IRQ 18 intel ips 0000:00:1f.6: IPS driver initialized, MCP temp limit 65535 intel ips 0000:00:1f.6: MCP power limit 65535 exceeded: cpu:8058 + mch:23392829 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5675 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 6369 intel ips 0000:00:1f.6: MCP power limit 65535 exceeded: cpu:5095 + mch:65379 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 7387 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 8326 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5943 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 6428 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5775 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 7061 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5153 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5098 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5208 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 7500 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 9144 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 6722 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 7156 intel ips 0000:00:1f.6: MCP power limit 65535 exceeded: cpu:5693 + mch:64598 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5856 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 4209 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 4726 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5259 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5212 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 4862 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5281 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 4235 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 4897 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5257 intel ips 0000:00:1f.6: MCP power limit ...
I believe all these limits should always have non-zero values.
So I still think you've hardware problems, but I suppose it
could be the driver not reading the right registers or some
such. It seems odd that the driver never printed a logging
message for either of the polling or irq methods to read the
device cpu and thermal limits.
Jesse or any Intel folk, can you verify or suggest anything
better?
If cpu_power_limit, or any _limit, is not set perhaps changing
the test style to verify limit and adding a printed_once alert
for each 0 value limit. At least that'd shut up the continuous
logging but at least give a notification message.
if (limit) {
if (measured_val > limit)
dev_info(foo)
} else
dev_alert_once()
Maybe something like this:
drivers/platform/x86/intel_ips.c | 160 +++++++++++++++++++++++++++++++++-----
1 files changed, 140 insertions(+), 20 deletions(-)
diff --git a/drivers/platform/x86/intel_ips.c b/drivers/platform/x86/intel_ips.c
index 9024480..10eba04 100644
--- a/drivers/platform/x86/intel_ips.c
+++ b/drivers/platform/x86/intel_ips.c
@@ -598,17 +598,53 @@ static bool mcp_exceeded(struct ips_driver *ips)
{
unsigned long flags;
bool ret = false;
+ static bool printed_zero_mcp_power_limit;
+ static bool printed_zero_mcp_temp_limit;
+ u16 mcp_avg_temp;
+ u16 mcp_temp_limit;
+ u16 mcp_power_limit;
+ u32 cpu_avg_power;
+ u32 mch_avg_power;
spin_lock_irqsave(&ips->turbo_status_lock, flags);
- if (ips->mcp_avg_temp > (ips->mcp_temp_limit * 100))
- ret = true;
- if (ips->cpu_avg_power + ips->mch_avg_power > ips->mcp_power_limit)
- ret = true;
+
+ mcp_avg_temp = ips->mcp_avg_temp;
+ mcp_temp_limit = ips->mcp_temp_limit;
+ mcp_power_limit = ips->mcp_power_limit;
+ cpu_avg_power = ips->cpu_avg_power;
+ mch_avg_power = ips->mch_avg_power;
+
spin_unlock_irqrestore(&ips->turbo_status_lock, flags);
- if (ret)
- dev_info(&ips->dev->dev,
- "MCP power or thermal limit exceeded\n");
+ if (mcp_power_limit) {
+ if ...Come on, no blaming the BIOS? ;-)
If I read the code with your previous patch correctly, show_turbo_limits
will never be called if poll_turbo_status is false but no interrupt
happens. And we know no interrupt happened (at least not with nonzero
register values), because the interrupt handler does two dev_info()
right at the beginning. So the limits could still be the ones initially
set at ips_probe().
Wouldn't it make more sense to do the alert when the limit is set,
instead of when it is used? Also, it should still treat it as limit
exceeded (better safe than sorry). Something like:
if (measured_val > limit) {
if (limit)
dev_info(...);
ret = true;
}
--
Cesar Eduardo Barros
cesarb@cesarb.net
cesar.barros@gmail.com
--
You seem to have dropped the CC list by accident, adding it back. Here it is: intel ips 0000:00:1f.6: Warning: CPU TDP doesn't match expected value (found 25, expected 35) intel ips 0000:00:1f.6: PCI INT C -> GSI 18 (level, low) -> IRQ 18 intel ips 0000:00:1f.6: show_turbo_limits:ips_probe cte:1 gte:1 cpt:0 mpl:65535 mtl:65535 mpl:65535 intel ips 0000:00:1f.6: IPS driver initialized, MCP temp limit 65535 intel ips 0000:00:1f.6: MCP power limit 65535 exceeded: cpu:8004 + mch:25353039 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 4841 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5283 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5586 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 6077 intel ips 0000:00:1f.6: MCP power limit 65535 exceeded: cpu:5871 + mch:64538 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5466 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 8589 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5744 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 4859 intel ips 0000:00:1f.6: MCP power limit 65535 exceeded: cpu:4834 + mch:62385 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 4874 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5356 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 6557 intel ips 0000:00:1f.6: MCP power limit 65535 exceeded: cpu:7589 + mch:59343 intel ips 0000:00:1f.6: MCP power limit 65535 exceeded: cpu:5536 + mch:60020 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 6676 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 4401 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5634 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 4038 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 4700 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5086 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 4930 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 4697 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5034 intel ips 0000:00:1f.6: CPU power limit 0 exceeded: 5381 intel ...
It seems I made some mistake when copying and pasting the CC: list, I fixed it on this message. -- Cesar Eduardo Barros cesarb@cesarb.net cesar.barros@gmail.com --
So ips->core_power_limit is still 0. Is it bad bios or driver? I expect the bios/hw is faulty but fingers are easy to point. Your choice on how to minimize your current logging problem. Changing the message level to dev_dbg probably isn't the right thing to do overall, but it may suit you right now. Thanks for testing. --
The solution here probably is not less logging. The best solution IMO would be to do some sanity checking when loading the module, and if the values do not make sense, print something to the log and return -ENODEV. I expect that, when it works as it should, the first read while loading the module already returns sane values, so a sanity check there should not have many false positives. OTOH, it is best to not load the module when you think things are strange. -- Cesar Eduardo Barros cesarb@cesarb.net cesar.barros@gmail.com --
As long as your sanity checking won't make the module fail to load in the following scenario: 1. environment temperature control fails, room starts to heat up 2. things go south, server reboots due to exceeded temperature limits 3. OS boots in an overheat situation 4. module refuse to load because it expects to never start in a overheating situation. If the sanity checks will cause (4), then don't add them. rate-limit the thermal alarms (issue them only once every T, and only if temperature has increased more than, say, 5°C from the last alarm). If a given platform is buggy crap (or just el-cheapo trash that overheats all the time) to the point that the module is useless, blacklist it by DMI What good is an alarm module that refuses to load when there is an alarm condition happening already? -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh --
I have not read the datasheet (I do not even know if it is available to the public; I have not looked), but I would not expect to see a power limit of 0 even if the CPU is on fire. Of course, you have to be more cautious when validating the current temperature (and even then, if it This is not an alarm module; AFAIK it is a module for the feature in recent Intel CPU/GPU chips which allow you to overclock it a bit as long as the thermal and power limit has not been exceeded: config INTEL_IPS tristate "Intel Intelligent Power Sharing" depends on ACPI ---help--- Intel Calpella platforms support dynamic power sharing between the CPU and GPU, maximizing performance in a given TDP. This driver, along with the CPU frequency and i915 drivers, provides that functionality. If in doubt, say Y here; it will only load on supported platforms. If the module is not loaded, it simply will not be able to go above its nominal clock, so refusing to load it is not that much of a problem. -- Cesar Eduardo Barros cesarb@cesarb.net cesar.barros@gmail.com --
On Sat, 28 Aug 2010 16:07:08 -0300 It sounds like either your BIOS values are wrong or the driver isn't fetching them correctly on your platform. It's possible that the main issue here is bad thermal limits. There's obviously a relationship between power and thermal output, but the driver tries to monitor both. However it's up to the BIOS to provide the driver with accurate thermal limits, as well as accurate power limits. The power limits sound reasonable at 25W, thus the informational output about 35W vs 25W (35W is what the MCP can handle, but some platforms are designed to handle less, so they clamp it down a bit). But the temp limits look all wrong. I'll see if I can find info on getting better data into the driver... Thanks, -- Jesse Barnes, Intel Open Source Technology Center --
If you need more information from this laptop (dmidecode, ACPI AML, etc), just ask. -- Cesar Eduardo Barros cesarb@cesarb.net cesar.barros@gmail.com --
On Fri, 27 Aug 2010 19:21:44 -0700
Yes, we do need something like this. It turns out the BIOS can
optionally program several of these values, and we need to sanity check
them. If they're not valid (e.g. a core power limit of 0 or MCP temp
limit of 0xffff) we need to use the default values in the limit structs.
I think the programmed limits are valid if they're nonzero and less
than one of the available default limits. If they're not valid, we
should just use the default values. I was thinking something like the
below for MCP, but Joe you may want to just update your patch instead
since it's more complete.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
diff --git a/drivers/platform/x86/intel_ips.c b/drivers/platform/x86/intel_ips.c
index 9024480..ec72e80 100644
--- a/drivers/platform/x86/intel_ips.c
+++ b/drivers/platform/x86/intel_ips.c
@@ -662,6 +662,17 @@ static bool mch_exceeded(struct ips_driver *ips)
return ret;
}
+static void clamp_mcp_temp_limit(struct ips_driver *ips)
+{
+ /*
+ * BIOS may or may not program an MCP limit. Clamp it to the
+ * lowest available value.
+ */
+ if (ips->mcp_temp_limit < ips->core_temp_limit ||
+ ips->mcp_temp_limit < ips->mch_temp_limit)
+ ips->mcp_temp_limit = min(ips->core_temp_limit, ips->mch_temp_limit);
+}
+
/**
* update_turbo_limits - get various limits & settings from regs
* @ips: IPS driver struct
@@ -686,6 +697,7 @@ static void update_turbo_limits(struct ips_driver *ips)
ips->mcp_temp_limit = thm_readw(THM_PTL);
ips->mcp_power_limit = thm_readw(THM_MPPC);
+ clamp_mcp_temp_limit(ips);
/* Ignore BIOS CPU vs GPU pref */
}
@@ -1155,6 +1167,7 @@ static irqreturn_t ips_irq_handler(int irq, void *arg)
STS_PTL_SHIFT;
ips->mcp_power_limit = (tc1 & STS_PPL_MASK) >>
STS_PPL_SHIFT;
+ clamp_mcp_temp_limit(ips);
spin_unlock(&ips->turbo_status_lock);
thm_writeb(THM_SEC, SEC_ACK);
--
You have my patch yes? You can probably integrate and validate it better than I can with your proposed change. I don't have any of the hardware to test. If I can otherwise help somehow, let me know. cheers, Joe --
On Thu, 23 Sep 2010 13:47:05 -0700 Ah ok, I'll fix it up and respin then, thanks. -- Jesse Barnes, Intel Open Source Technology Center --
