Trying to fix radeonfb suspending on IBM Thinkpad T41

Trying to fix radeonfb suspending on IBM Thinkpad T41

[Antti Andreimann]

Dear community!

The aim of this post is to discuss the radeonfb driver power management
issues. Enabling this feature dramatically reduces power consumtion
during ACPI suspend to ram. I would appriciate comments from people who
are more familiar with Radeon HW programming.

Long and boring background
--------------------------
I have a beast called IBM ThinkPad T41. Model No. 2373-2FG
Since 2.6.9 ACPI suspend to RAM (S3) has been working nicely, but the
power consumption during sleep is unacceptably high: the battery will
run dry in about 5-6 hours in sleep mode.
The root cause of this is the fact that not all hardware is properly
turned off. Namely, at least ethernet adapter, USB and most notably
graphics accelerator chip (Radeon Mobility M7 LW) remain powered on.
Unfortunately current radeonfb driver is hardcoded to do power
management only on PPC platform.
Volker Braun has some kernels on his page
(http://www.sas.upenn.edu/~vbraun/computing/T41/kernel.html) that
incorporate his patch to enable PM on other platforms as well. He also
owns a T41, but It's a bit different model and those kernels do not
resume properly on my laptop: the machine crashes hard on resume.
After a few hours of hacking around in radeonfb sources I think I found
the problem and fixed it for my hardware. The results are promising: the
power consumption during sleep is about 4-5 times lower: 15min ACPI
sleep consumes 1100mW/h without radeonfb and only 230mW/h with properly
patched radeonfb driver loaded.

A question to anyone familiar with radeon hardware programming
--------------------------------------------------------------
The radeonfb driver has a power management implementation that is used
on PPC platform (Macintosh laptops ;). The same implementation seems to
work fine on some Thinkpads, but crashes on others. In
drivers/video/aty/radeon_pm.c resides a function called:
radeon_pm_setup_for_suspend. This function has a following section that
manages to crash at least my laptop:

/* AGP PLL control */
OUTREG(BUS_CNTL1, INREG(BUS_CNTL1) |  BUS_CNTL1__AGPCLK_VALID);

OUTREG(BUS_CNTL1,
     (INREG(BUS_CNTL1) & ~BUS_CNTL1__MOBILE_PLATFORM_SEL_MASK)
     | (2<<BUS_CNTL1__MOBILE_PLATFORM_SEL__SHIFT));  // 440BX
OUTREG(CRTC_OFFSET_CNTL, (INREG(CRTC_OFFSET_CNTL) &
      ~CRTC_OFFSET_CNTL__CRTC_STEREO_SYNC_OUT_EN));

clk_pin_cntl &= ~CLK_PIN_CNTL__CG_CLK_TO_OUTPIN;
clk_pin_cntl |= CLK_PIN_CNTL__XTALIN_ALWAYS_ONb;
OUTPLL( pllCLK_PIN_CNTL, clk_pin_cntl);

It can be seen from the comments that this code is specific to Intel
440BX or similar chipset and this is most probably the reason it manages
to crash machines that are based on a different chip.

What exactly does this piece of code do and is it needed after all? My
machine seems to work fine without it.
If it is needed, is it possible to make it more generic?

-- 
 Antti Andreimann - Security Consultant
      Using Linux since 1993
  Member of ELUG since 29.01.2000

Re: Trying to fix radeonfb suspending on IBM Thinkpad T41

[Volker Braun]

I have no knowledge of the internals of the radeon family, but I am
under the impression that they require some hacks to work around bugs in
the silicon. There is a rather big patch coming, see

http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.11-
rc1/2.6.11-rc1-mm2/broken-out/radeonfb-massive-update-of-pm-code.patch

This patch also rewrites the questionable section in
radeon_pm_setup_for_suspend. I just found it and have not yet built a
new kernel, so I cannot comment on its effectiveness. 

For reference, the power management issues of the T41 have their own
bugzilla entry:

http://bugme.osdl.org/show_bug.cgi?id=3022

On a side note, since kernel 2.6.10 I have not been able to successfully
resume from acpi S3 + radeonfb any more (T41 2379-DJU, radeon mobility
M9) - works under 2.6.9 and 2.6.11-rc1 + vgaconsole. I'm still trying to
isolate the problem/waiting for some of the pm code to settle.

Best,
Volker

Re: Trying to fix radeonfb suspending on IBM Thinkpad T41

[Volker Braun]

Update: I compiled a kernel with the radeonfb-massive-update-of-pm-
code.patch. Now I can successfully resume from acpi S3 again. The power
drain issue remains, it still uses about 5W in the suspend state.

-Volker

Re: Trying to fix radeonfb suspending on IBM Thinkpad T41

[Antti Andreimann]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

Ühel kenal päeval (pühapäev, 23. jaanuar 2005, 16:54-0500), kirjutas
Volker Braun:
> Update: I compiled a kernel with the radeonfb-massive-update-of-pm-
> code.patch. Now I can successfully resume from acpi S3 again. The power
> drain issue remains, it still uses about 5W in the suspend state.

radeonfb power management is not enabled on PC platform.
I cooked up some code to turn D2 sleep mode on for selected PC-s and
managed to get my ThinkPad to work.
Try this patch on top of radeonfb-massive.
NB! The patch has a model specific triger, so You MUST add Your laptop
model number to the top of the drivers/video/aty/radeon_pm.c.

-- 
 Antti Andreimann - Security Expert
      Using Linux since 1993
  Member of ELUG since 29.01.2000

[-- Attachment #2: radeonfb-thinkpad-pm.patch --]
[-- Type: text/x-patch, Size: 1864 bytes --]

--- linux-2.6.11-rc2-tank/drivers/video/aty/radeon_pm.c.orig	2005-01-23 14:55:57.000000000 +0200
+++ linux-2.6.11-rc2-tank/drivers/video/aty/radeon_pm.c	2005-01-23 21:53:47.000000000 +0200
@@ -25,8 +25,31 @@
 #include <asm/pmac_feature.h>
 #endif
 
+/* For detecting supported PC laptops */
+#ifdef CONFIG_X86
+#include <linux/dmi.h>
+#endif
+
 #include "ati_ids.h"
 
+#ifdef CONFIG_X86
+/* This array holds a list of supported PC laptops.
+ * Currently only few IBM models are tested.
+ * If you want to experiment, use dmidecode to find out
+ * vendor and product codes for Your laptop.
+ */
+static struct dmi_system_id __devinitdata radeonfb_dmi_table[] = {
+	{
+		.ident = "IBM ThinkPad T41 (2373-2FG)",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "IBM"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "23732FG"),
+		},
+	},
+	{ },
+};
+#endif
+
 void radeon_pm_disable_dynamic_mode(struct radeonfb_info *rinfo)
 {
 	u32 tmp;
@@ -2748,6 +2771,24 @@
 		OUTREG(TV_DAC_CNTL, INREG(TV_DAC_CNTL) | 0x07000000);
 	}
 #endif /* defined(CONFIG_PM) && defined(CONFIG_PPC_OF) */
+
+/* The PM code also works on some PC laptops.
+ * Only a few models are actually tested so Your mileage may vary.
+ * We can do D2 on at least M7 and M9 on some IBM ThinkPad T41 models.
+ */
+#if defined(CONFIG_PM) && defined(CONFIG_X86)
+	if (dmi_check_system(radeonfb_dmi_table)) {
+		if (rinfo->is_mobility && rinfo->pm_reg &&
+		    rinfo->family <= CHIP_FAMILY_RV250)
+			rinfo->pm_mode |= radeon_pm_d2;
+
+		/* Power down TV DAC, that saves a significant amount of power,
+		 * we'll have something better once we actually have some TVOut
+		 * support
+		 */
+		OUTREG(TV_DAC_CNTL, INREG(TV_DAC_CNTL) | 0x07000000);
+	}
+#endif /* defined(CONFIG_PM) && defined(CONFIG_X86) */
 }
 
 void radeonfb_pm_exit(struct radeonfb_info *rinfo)

Re: Trying to fix radeonfb suspending on IBM Thinkpad T41

[Antti Andreimann]

[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]

Ühel kenal päeval (esmaspäev, 24. jaanuar 2005, 16:01-0500), kirjutas
Volker Braun:
> Hi,
> 
> This does the trick. In fact, i only need to get rid of the second
> OUTREG() call. No idea what that is good for. I'll update the bugzilla
> kernel entry with your patch and some additional information.

Ok, Great!

I have made another patch that disables this call altogether.
As far as I can understand it programs some registers that are related
to AGP clocking and according to some guys inside ATI, it is North
Bridge and Radeon chip dependent, but I don't know jack about ATI HW
programming so I might be seriously off here.
My laptop seems to do well without reprogramming this register and it
manages to crash Yours (and crashed mine with old driver) so I think it
should be left untouched until we get more information on how to do it
properly.

Can You please upload this patch to bugzilla instead?

If anyone tests the patch on some other thinkpad model please post it to
bugzilla so I we can get an impression how well it performs as well as
update the whitelist.

-- 
 Antti Andreimann - Security Expert
      Using Linux since 1993
  Member of ELUG since 29.01.2000

[-- Attachment #2: radeonfb-thinkpad-pm2.patch --]
[-- Type: text/x-patch, Size: 2877 bytes --]

--- linux-2.6.11-rc2-tank/drivers/video/aty/radeon_pm.c.orig	2005-01-24 23:52:17.000000000 +0200
+++ linux-2.6.11-rc2-tank/drivers/video/aty/radeon_pm.c	2005-01-25 00:00:24.000000000 +0200
@@ -25,8 +25,39 @@
 #include <asm/pmac_feature.h>
 #endif
 
+/* For detecting supported PC laptops */
+#ifdef CONFIG_X86
+#include <linux/dmi.h>
+#endif
+
 #include "ati_ids.h"
 
+#ifdef CONFIG_X86
+/* This array holds a list of supported PC laptops.
+ * Currently only few IBM models are tested.
+ * If you want to experiment, use dmidecode to find out
+ * vendor and product codes for Your laptop.
+ */
+static struct dmi_system_id __devinitdata radeonfb_dmi_table[] = {
+	{
+		.ident = "IBM ThinkPad T41 (2373-2FG)",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "IBM"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "23732FG"),
+		},
+	},
+	{
+		/* Reported by Volker Braun <vbraun@physics.upenn.edu> */
+		.ident = "IBM ThinkPad T41 (2379-DJU)",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "IBM"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "2379DJU"),
+		},
+	},
+	{ },
+};
+#endif
+
 void radeon_pm_disable_dynamic_mode(struct radeonfb_info *rinfo)
 {
 	u32 tmp;
@@ -853,6 +884,13 @@
 	tmp = INPLL( pllMCLK_MISC) | MCLK_MISC__EN_MCLK_TRISTATE_IN_SUSPEND;
 	OUTPLL( pllMCLK_MISC, tmp);
 
+	/* BUS_CNTL1__MOBILE_PLATORM_SEL setting is northbridge chipset
+	 * and radeon chip dependent. Thus we only enable it on Mac for
+	 * now (until we get more info on how to compute the correct 
+	 * value for various X86 bridges).
+	 */
+
+#ifdef CONFIG_PPC_PMAC
 	/* AGP PLL control */
 	if (rinfo->family <= CHIP_FAMILY_RV280) {
 		OUTREG(BUS_CNTL1, INREG(BUS_CNTL1) |  BUS_CNTL1__AGPCLK_VALID);
@@ -864,6 +902,7 @@
 		OUTREG(BUS_CNTL1, INREG(BUS_CNTL1));
 		OUTREG(BUS_CNTL1, (INREG(BUS_CNTL1) & ~0x4000) | 0x8000);
 	}
+#endif
 
 	OUTREG(CRTC_OFFSET_CNTL, (INREG(CRTC_OFFSET_CNTL)
 				  & ~CRTC_OFFSET_CNTL__CRTC_STEREO_SYNC_OUT_EN));
@@ -2748,6 +2787,24 @@
 		OUTREG(TV_DAC_CNTL, INREG(TV_DAC_CNTL) | 0x07000000);
 	}
 #endif /* defined(CONFIG_PM) && defined(CONFIG_PPC_OF) */
+
+/* The PM code also works on some PC laptops.
+ * Only a few models are actually tested so Your mileage may vary.
+ * We can do D2 on at least M7 and M9 on some IBM ThinkPad T41 models.
+ */
+#if defined(CONFIG_PM) && defined(CONFIG_X86)
+	if (dmi_check_system(radeonfb_dmi_table)) {
+		if (rinfo->is_mobility && rinfo->pm_reg &&
+		    rinfo->family <= CHIP_FAMILY_RV250)
+			rinfo->pm_mode |= radeon_pm_d2;
+
+		/* Power down TV DAC, that saves a significant amount of power,
+		 * we'll have something better once we actually have some TVOut
+		 * support
+		 */
+		OUTREG(TV_DAC_CNTL, INREG(TV_DAC_CNTL) | 0x07000000);
+	}
+#endif /* defined(CONFIG_PM) && defined(CONFIG_X86) */
 }
 
 void radeonfb_pm_exit(struct radeonfb_info *rinfo)