Bug 26347 - powermanagement on rs780 not working
powermanagement on rs780 not working
Status: RESOLVED FIXED
Product: DRI
Classification: Unclassified
Component: DRM/Radeon
DRI git
x86-64 (AMD64) Linux (All)
: medium normal
Assigned To: Default DRI bug account
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-01-31 04:44 UTC by Marc Dietrich
Modified: 2010-03-15 11:27 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
output of /var/log/messages (47.95 KB, text/plain)
2010-01-31 05:01 UTC, Marc Dietrich
no flags Details
video bios (58.50 KB, application/octet-stream)
2010-02-02 13:32 UTC, Marc Dietrich
no flags Details
log with atom_debug=1 (32.87 KB, text/plain)
2010-02-04 13:44 UTC, Marc Dietrich
no flags Details
drm/radeon/kms: add PM quirk for Asus Radeon HD 3200 (1.90 KB, patch)
2010-03-04 14:19 UTC, Rafał Miłecki
no flags Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Dietrich 2010-01-31 04:44:25 UTC
drm detects 2 power states, but they are never switched.
see attached log.
Comment 1 Marc Dietrich 2010-01-31 05:01:22 UTC
Created attachment 32946 [details]
output of /var/log/messages
Comment 2 Alex Deucher 2010-02-02 11:12:13 UTC
please attach your video bios:
(as root):
cd /sys/bus/pci/devices/<pci bus id>
echo 1 > rom
cat rom > /tmp/vbios.rom
echo 0 > rom
Comment 3 Marc Dietrich 2010-02-02 13:32:07 UTC
Created attachment 33014 [details]
video bios
Comment 4 Marc Dietrich 2010-02-02 13:35:36 UTC
I guess I know what's wrong. bootlog shows:

kernel: [    1.651833] [drm] 2 Power State(s)
kernel: [    1.651835] [drm] State 0 Default (default)
kernel: [    1.651836] [drm]  1 Clock Mode(s)
kernel: [    1.651837] [drm]          0 engine: 300000
kernel: [    1.651839] [drm] State 1 Performance
kernel: [    1.651840] [drm]  1 Clock Mode(s)
kernel: [    1.651841] [drm]          0 engine: 500000
kernel: [    1.651846] [drm] radeon: dynamic power management enabled

which means the default mode is the low clock rate and there is only one clock per state. looking at radeon_pm.c, when going upclock, the default state is loaded (in my case the low power one) and then the highest clock is choosen (which is 300 Mhz and therefore wrong).

Somehow, the logic is wrong here.
Comment 5 Marc Dietrich 2010-02-04 13:44:14 UTC
Created attachment 33083 [details]
log with atom_debug=1
Comment 6 Marc Dietrich 2010-02-04 13:44:49 UTC
I boldly applied this patch:

--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -180,7 +180,7 @@ static void radeon_get_power_state(struct radeon_device *rdev,
                        radeon_pick_clock_mode(rdev, rdev->pm.requested_power_state, POWER_MODE_TYPE_MID);
                break;
        case PM_ACTION_UPCLOCK:
-               rdev->pm.requested_power_state = radeon_pick_power_state(rdev, POWER_STATE_TYPE_DEFAULT);
+               rdev->pm.requested_power_state = radeon_pick_power_state(rdev, POWER_STATE_TYPE_PERFORMANCE);
                rdev->pm.requested_power_state->requested_clock_mode =
                        radeon_pick_clock_mode(rdev, rdev->pm.requested_power_state, POWER_MODE_TYPE_HIGH);
                break;

and now I get "switching messages" in my log. Unforunately, the performance does not change notably.

So I set atom_debug=1 and attached the log in the hope it may help...
Comment 7 Rafał Miłecki 2010-02-04 14:02:28 UTC
Marc: please boot (modprobe) without dynpm (or dynpm=0) and provide output of following:

cat /sys/kernel/debug/dri/0/radeon_pm_info
Comment 8 Marc Dietrich 2010-02-04 14:10:48 UTC
cat /sys/kernel/debug/dri/0/radeon_pm_info with dynclk=0

state: PM_STATE_DISABLED
default engine clock: 500000 kHz
current engine clock: 494040 kHz
default memory clock: 400000 kHz

that is different to enabled dynclk (see comment #1)!
Comment 9 Tobias Jakobi 2010-02-04 14:27:34 UTC
This looks exactly like my (remaining) problem here: https://bugs.freedesktop.org/show_bug.cgi?id=26329

radeon DRM currently seems to lack some way to switch the power state to a reasonable setting. E.g. for me the default power state only contains one clock and this one is also the highest one the GPU/mem does (officially) support.

So currently I'm always running in "performance"-mode, although the "performance powerstate" is a different one (state 0) and would be a MUCH saner choice.
Comment 10 Rafał Miłecki 2010-02-04 14:33:14 UTC
Driver believes it's currently running 300MHz as this state is called default (incorrectly).

Decision of dynamic power management is to downclock. Driver looks for some low mode (for downclocking) and it finds 300MHz... but then it believe it's mode we are already running.

My first idea is we should not set current mode until we really reclock... but let me think about this yet.
Comment 11 Tobias Jakobi 2010-02-04 15:51:29 UTC
I checked the Windows driver behaviour with my friend (who owns a integrated Radeon HD in his laptop) and it seems to go like this:
The driver selects a powerstate based on the energy-level of the system (by watching for ACPI events).

With the information from DRM radeon we found out that the VGA BIOS does feature 4 power states. But only state 1 and 2 are used by the Windows driver, state 0 and 3 are never used.

If the laptop is running on battery mode the driver switches to power state 1, if the laptop is running on AC it's switched to power state 2.

I don't have all details memorized, but the maximum GPU clock for this chip is 500MHz and the AC power state has both 500MHz and 330MHz CPU clocks.
The battery power state has 330MHz and 110MHz GPU clocks.

Again, the powerstate never changes if one does not fiddle with the AC cord.

Doing some testing with FurMark the driver only reclocks with the clocks from the current power state. If it's on battery and there is no load on the GPU it's downclocked to 100MHz - going up to 330MHz once FurMark kicks in. Same goes for the AC power state.

We need the same for DRM radeon: Something (devicenode in /sys/ e.g.) that can select the power state, so I can configure acpid to select the appropriate powerstate on AC/battery changes OR do it manually.
Then the driver should only reclock with the clocks that are inside the clock group belonging to the power state.

Creating custom power states would be an additional goodie, so you can tweak the hardware even more (something like VERY low power states - example: reducing mem clock on my card since all the existant powerstates always clock it at MAX).
Comment 12 Tobias Jakobi 2010-02-04 16:00:07 UTC
Looking at the code in radeon_pm.c the current approach looks fundamentally wrong to me:

radeon_pm_set_clocks_locked seems to call radeon_set_power_state to issue clock changes. This just doesn't look right to me and is obviously not what the Windows driver is doing.

Or am I just misunderstanding the concept of power states?
Comment 13 Rafał Miłecki 2010-03-03 14:43:43 UTC
(In reply to comment #11)
> I don't have all details memorized, but the maximum GPU clock for this chip is
> 500MHz and the AC power state has both 500MHz and 330MHz CPU clocks.
> The battery power state has 330MHz and 110MHz GPU clocks.

It already shows driver uses at least 3 states. And you few lines earlier wrote:

> But only state 1 and 2 are used by the Windows driver,
> state 0 and 3 are never used.

Anyway, I don't think it's really important to understand Windows driver. We may eventually need smarter reclocking algorithm.



(In reply to comment #12)
> Looking at the code in radeon_pm.c the current approach looks fundamentally
> wrong to me:
> 
> radeon_pm_set_clocks_locked seems to call radeon_set_power_state to issue clock
> changes. This just doesn't look right to me and is obviously not what the
> Windows driver is doing.
> 
> Or am I just misunderstanding the concept of power states?

I guess you are. What does look incorrect for you in that?
Comment 14 Rafał Miłecki 2010-03-03 14:50:00 UTC
Marc: please provide output of:
# lspci -nnv

You can cut it to VGA part.
Comment 15 Tobias Jakobi 2010-03-04 05:24:20 UTC
@Rafał Miłecki:

Currently (nearly) everything in PM looks wrong to me.

First of all the user has no way to configure the power management. I can't force the card into low-power mode like I can on Windows. Nor can I force the card into high-power mode if I need the performance e.g. for games (even on Windows there are situation where I don't want dynamic clock changes because I want a steady framerate).

There is currently no way to tell the driver: I'm in this situation and I need that much performance. And that's (at least from my understanding) the main reason for the different "power states" the card offers.

This problem extends to mobile systems. On these the driver has currently no knowledge about the battery/AC-adapter situation. If we want the driver to react to ACPI events (like AC unplug/plug events) (and I really think we SHOULD react to that) we need to expose power state selection to userspace.

> Anyway, I don't think it's really important to understand Windows driver. We
> may eventually need smarter reclocking algorithm.
I think it very much is, because the Windows driver actually does reclocking right (no artifacts, no sudden gamespeed slowdowns when reclocking occurs) and offers the user the ability to configure the reclocking behaviour.

I agree that we may need a smarter algorithm for WHEN to do reclocking, but we should adapt to the Windows driver for WHICH clock/voltage/etc. to select.

The current PM implementation on linux does too much "automagic", which fails in most cases. It ignores the concept of "power states" in the sense that the term "power state" doesn't really matter to the driver - it switches between them anyway.
Comment 16 Marc Dietrich 2010-03-04 09:58:33 UTC
output of lspci -vnn

01:05.0 VGA compatible controller [0300]: ATI Technologies Inc Radeon HD 3200 Graphics [1002:9610]
        Subsystem: ASUSTeK Computer Inc. Device [1043:82f1]
        Flags: bus master, fast devsel, latency 0, IRQ 18
        Memory at d0000000 (32-bit, prefetchable) [size=256M]
        I/O ports at d000 [size=256]
        Memory at fbef0000 (32-bit, non-prefetchable) [size=64K]
        Memory at fbd00000 (32-bit, non-prefetchable) [size=1M]
        Capabilities: [50] Power Management version 3
        Capabilities: [a0] Message Signalled Interrupts: Mask- 64bit+ Queue=0/0 Enable-
        Kernel driver in use: radeon
        Kernel modules: radeon

I also updated the kernel to 2.6.33 + drm-linus from yesterday:

relevant parts:

[    1.474500] [drm] Initialized drm 1.1.0 20060810                                                                                                   
[    1.501991] [drm] radeon defaulting to kernel modesetting.                                                                                         
[    1.502076] [drm] radeon kernel modesetting enabled.                                                                                               
[    1.502212] radeon 0000:01:05.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18                                                                        
[    1.502302] radeon 0000:01:05.0: setting latency timer to 64                                                                                       
[    1.504998] [drm] radeon: Initializing kernel modesetting.                                                                                         
[    1.505230] [drm] register mmio base: 0xFBEF0000                                                                                                   
[    1.505313] [drm] register mmio size: 65536                                                                                                        
[    1.505984] ATOM BIOS: 113                                                                                                                         
[    1.506091] [drm] Clocks initialized !                                                                                                             
[    1.506175] [drm] 2 Power State(s)                                                                                                                 
[    1.506257] [drm] State 0 Default (default)                                                                                                        
[    1.506340] [drm]    1 Clock Mode(s)                                                                                                               
[    1.506422] [drm]            0 engine: 300000                                                                                                      
[    1.506504] [drm] State 1 Performance                                                                                                              
[    1.506587] [drm]    1 Clock Mode(s)                                                                                                               
[    1.506663] [drm]            0 engine: 500000                                                                                                      
[    1.506749] [drm] radeon: dynamic power management enabled                                                                                         
[    1.506830] [drm] radeon: power management initialized                                                                                             
[    1.506930] radeon 0000:01:05.0: VRAM: 256M 0xC0000000 - 0xCFFFFFFF (256M used)                                                                    
[    1.507057] radeon 0000:01:05.0: GTT: 512M 0xA0000000 - 0xBFFFFFFF                                                                                 
[    1.507343] [drm] Detected VRAM RAM=256M, BAR=256M                                                                                                 
[    1.507429] [drm] RAM width 32bits DDR                                                                                                             
[    1.507589] [TTM] Zone  kernel: Available graphics memory: 893658 kiB.                                                                             
[    1.507687] [drm] radeon: 256M of VRAM memory ready                                                                                                
[    1.507770] [drm] radeon: 512M of GTT memory ready.                                                                                                
[    1.507879] [drm] radeon: irq initialized.                                                                                                         
[    1.507962] [drm] GART: num cpu pages 131072, num gpu pages 131072                                                                                 
[    1.509428] [drm] Loading RS780 Microcode                                                                                                          
[    1.509506] platform radeon_cp.0: firmware: requesting radeon/RS780_pfp.bin                                                                        
[    1.511192] platform radeon_cp.0: firmware: requesting radeon/RS780_me.bin                                                                         
[    1.513200] platform radeon_cp.0: firmware: requesting radeon/R600_rlc.bin                                                                         
[    1.551581] [drm] ring test succeeded in 0 usecs                                                                                                   
[    1.551961] [drm] radeon: ib pool ready.                                                                                                           
[    1.552123] [drm] ib test succeeded in 0 usecs                                                                                                     
[    1.552206] [drm] Enabling audio support                                                                                                           
[    1.552382] [drm] Radeon Display Connectors                                                                                                        
[    1.552529] [drm] Connector 0:                                                                                                                     
[    1.552613] [drm]   VGA                                                                                                                            
[    1.552690] [drm]   DDC: 0x7e40 0x7e40 0x7e44 0x7e44 0x7e48 0x7e48 0x7e4c 0x7e4c                                                                   
[    1.552811] [drm]   Encoders:                                                                                                                      
[    1.552883] [drm]     CRT1: INTERNAL_KLDSCP_DAC1                                                                                                   
[    1.552965] [drm] Connector 1:                                                                                                                     
[    1.553045] [drm]   DVI-D                                                                                                                          
[    1.553126] [drm]   HPD3                                                                                                                           
[    1.553209] [drm]   DDC: 0x7e50 0x7e50 0x7e54 0x7e54 0x7e58 0x7e58 0x7e5c 0x7e5c                                                                   
[    1.553329] [drm]   Encoders:                                                                                                                      
[    1.553401] [drm]     DFP3: INTERNAL_KLDSCP_LVTMA                                                                                                  
[    1.606052] [drm] Requested: e: 50000 m: 0 p: 16                                                                                                   
[    1.606138] [drm] Setting: e: 50000 m: 0 p: 16                                                                                                     
[    1.723272] [drm] fb mappable at 0xD0141000                                                                                                        
[    1.723368] [drm] vram apper at 0xD0000000                                                                                                         
[    1.723443] [drm] size 9216000                                                                                                                     
[    1.723524] [drm] fb depth is 24                                                                                                                   
[    1.723596] [drm]    pitch is 7680                                                                                                                 
[    1.726270] [drm] Requested: e: 30000 m: 0 p: 16                                                                                                   
[    1.924054] [drm] Setting: e: 30000 m: 0 p: 16                                                                                                     
[    1.971710] Console: switching to colour frame buffer device 240x75                                                                                
[    2.007685] fb0: radeondrmfb frame buffer device                                                                                                   
[    2.007689] registered panic notifier                                                                                                              
[    2.007911] [drm] Initialized radeon 2.1.0 20080528 for 0000:01:05.0 on minor 0                                                                    
[    2.245139] [drm] Requested: e: 50000 m: 0 p: 16                                                                                                   
[    2.448043] [drm] not in vbl for pm change 00020002 00000000 at entry                                                                              
[    2.448222] [drm] Setting: e: 50000 m: 0 p: 16                                                                                                     
[    2.448453] [drm] not in vbl for pm change 00020002 00000000 at exit                                                                               
[   77.580020] [drm] Requested: e: 30000 m: 0 p: 16                                                                                                   
[   77.780025] [drm] not in vbl for pm change 00020002 00000000 at entry                                                                              
[   77.780029] [drm] Setting: e: 30000 m: 0 p: 16                                                                                                     
[   77.780128] [drm] not in vbl for pm change 00020002 00000000 at exit                                                                               
[   78.084021] [drm] Requested: e: 50000 m: 0 p: 16                                                                                                   
[   78.284024] [drm] not in vbl for pm change 00020002 00000000 at entry                                                                              
[   78.284027] [drm] Setting: e: 50000 m: 0 p: 16                                                                                                     
[   78.284129] [drm] not in vbl for pm change 00020002 00000000 at exit                                                                               
.....

root@AX2-5200p:/sys/kernel/debug/dri/0# cat radeon_pm_info
state: PM_STATE_ACTIVE
default engine clock: 500000 kHz
current engine clock: 494040 kHz
default memory clock: 400000 kHz
PCIE lanes: 0

^^^^^^^
this one shows wrong information
Comment 17 Alex Deucher 2010-03-04 11:23:03 UTC
(In reply to comment #16)
> root@AX2-5200p:/sys/kernel/debug/dri/0# cat radeon_pm_info
> state: PM_STATE_ACTIVE
> default engine clock: 500000 kHz
> current engine clock: 494040 kHz
> default memory clock: 400000 kHz
> PCIE lanes: 0
> 
> ^^^^^^^
> this one shows wrong information
> 

It's correct.  The RS780 is integrated into the northbridge and is not connected via PCIE so there are no lanes to change.
Comment 18 Marc Dietrich 2010-03-04 11:36:52 UTC
ah - sorry. I didn't meant the #lanes but the engine clock. dmesg reports 300 and 500 MHz available with 300 MHz default. radeon_pm_info says 500 is default and I'm using it! this sounds contradictory.
Comment 19 Rafał Miłecki 2010-03-04 13:51:04 UTC
(In reply to comment #15)
> @Rafał Miłecki:
> 
> Currently (nearly) everything in PM looks wrong to me.

Uhm.


> First of all the user has no way to configure the power management. I can't
> force the card into low-power mode like I can on Windows. Nor can I force the
> card into high-power mode if I need the performance e.g. for games (even on
> Windows there are situation where I don't want dynamic clock changes because I
> want a steady framerate).

What you want is static PM, I never claimed we have that. I focus on dynpm.


> There is currently no way to tell the driver: I'm in this situation and I need
> that much performance. And that's (at least from my understanding) the main
> reason for the different "power states" the card offers.

No. Power states in AtomBIOS are for both: dynamic and static PM. How do you imagine dynamic PM without knowing valid modes?


> This problem extends to mobile systems. On these the driver has currently no
> knowledge about the battery/AC-adapter situation. If we want the driver to
> react to ACPI events (like AC unplug/plug events) (and I really think we SHOULD
> react to that) we need to expose power state selection to userspace.

We may use AC/battery state info in future, when doing much more advanced PM. For now there is not any usage of such info.


> > Anyway, I don't think it's really important to understand Windows driver. We
> > may eventually need smarter reclocking algorithm.
> I think it very much is, because the Windows driver actually does reclocking
> right (no artifacts, no sudden gamespeed slowdowns when reclocking occurs) and
> offers the user the ability to configure the reclocking behaviour.

You won't get info about how to do reclocking to avoid artifacts/corruptions from watching Catalyst. Unless you're going to RE it. About providing UI for static PM I don't think there is much we can learn from Catalyst. We just need someone who will implement that.


> I agree that we may need a smarter algorithm for WHEN to do reclocking, but we
> should adapt to the Windows driver for WHICH clock/voltage/etc. to select.

I believe we known most of flags of power states. If we meet some flags we don't know we may eventually try to find out when Catalyst uses mode with such flag.


> The current PM implementation on linux does too much "automagic", which fails
> in most cases. It ignores the concept of "power states" in the sense that the
> term "power state" doesn't really matter to the driver - it switches between
> them anyway.

Well, what really more would you like to use from power states? They just provide clocks and voltage with some flags (which we don't parse fully yet). Don't see much more magic about them we could use.
Comment 20 Rafał Miłecki 2010-03-04 14:19:43 UTC
Created attachment 33771 [details] [review]
drm/radeon/kms: add PM quirk for Asus Radeon HD 3200

Marc: can you try this, please?
Comment 21 Marc Dietrich 2010-03-05 07:34:18 UTC
Rafał,

this seems to work. radeon_pm_info shows proper switching. Also I get some more frames in glxgears (538->588) so I guess the gpu really runs faster now. However, the "not in vbl for pm change 00020002 00000000 at entry/exit" is still there. Don't know if this is important. 

Thanks!
Comment 22 Tobias Jakobi 2010-03-07 12:10:10 UTC
@Rafał:

> Uhm.
Just to make sure - I don't mean to criticize you here. I appreciate your work but at least from my point of view there are some big issues here. I'm still waiting for some AMD guy to reply here (Alex?) to shed some light on the issue.


> What you want is static PM, I never claimed we have that. I focus on dynpm.
No, I don't just want static PM (that would come for free if the reclocking is implement similar to the Windows driver). I want some cpufreq like (however simplified a lot) interface in userspace to configure PM (and also dynpm).


> No. Power states in AtomBIOS are for both: dynamic and static PM.
I never said it wasn't. Of course the tables include both "dynamic" and "static" states - I never doubt that since it's obvious from the kernel log / DRM module output.


> How do you imagine dynamic PM without knowing valid modes?
I don't think you get my point. The power state entries are profiles which are based on the "need" of the current system state.

See my post about the Windows driver behaviour on my friend's laptop system.

There are in total 4 power states defined by the BIOS (in his case!). The driver only uses 2 (lets call them A and B) of them (unimportant for my point).

A is the state that is selected when the laptop is plugged to AC. When plugged to AC the system doesn't have to worry so much about power consumption which is reflected in the clocks of power state (high). Of course the whole PM is still dynamic since the power state A has more than 1 entry and the entries aren't identical.

Same goes for power state B which is selected (by the driver) when the system is running on battery. On battery the driver should worry about power consumption since each battery has limited capacity. This is again reflected in the clocks of power state B. But again it's dynamic - only the individual clocks are lower.


> About providing UI for
> static PM I don't think there is much we can learn from Catalyst. We just need
> someone who will implement that.
I may be totally wrong here - but isn't that just fixing the specific power state (lets call it C) and let the driver only switch between the entries of C (with the effect that no effective clock changes occur)?


> Well, what really more would you like to use from power states? They just
> provide clocks and voltage with some flags (which we don't parse fully yet).
> Don't see much more magic about them we could use.
Then why does AMD define the notion of a "power state" at all? Why are entries _grouped_ together to a power state? Why not just leave the whole notion of "power state" away and just put the entries (without grouping) into the BIOS?

Again, why this grouping if (according to you) there is no meaning to it in the first place?

Greets,
Tobias
Comment 23 Marc Dietrich 2010-03-13 10:52:23 UTC
Rafał, 

what about your original idea in comment #10? Instead of making quirks for broken bioses, just set the engine to some know value (e.g. default powerstate or boot state) and then update the current mode?
Comment 24 Alex Deucher 2010-03-14 08:28:43 UTC
I have some ideas about reworking the power mode selection.  I'll post patches when they are ready.
Comment 25 Rafał Miłecki 2010-03-14 14:00:41 UTC
@Marc: as for VBLANK errors, I get some even when checking VBLANK state directly in IRQ handler. So I can not debug that easily if we have some mistake or sth in IRQ. Anyway if you do not see corruptions, just live with it.

@Tobias: maybe you have some good ideas indeed, I guess I didn't understand you in some place.
Anyway I don't totally agree with you/fglrx. AFAIU fglrx does not use the lowest state/mode when AC is plugged, right? That's fine for power consumption, but not heat. I still want driver to downclock to the lowest state/mode even when I'm on AC cable to make my notebook cooler and more silent (slower fan).

@Marc: I think I submitted patch for this


Anyway I don't feel too comfortable with DRM development, so won't do any serious patches for some time. Please ping AMD for problems/features.
Comment 26 Marc Dietrich 2010-03-15 11:27:50 UTC
Thanks Rafał for your efforts!

I tested the patches from Alex today and everything is working so far (beside some vblank reports).

Hope it will be ready for 2.6.35 -> Closing.