Bug 28745

Summary:

RS780: dynpm makes my cat levitate (div by 0)

Product:

DRI

Reporter:

Rafał Miłecki <zajec5>

Component:

DRM/Radeon

Assignee:

Default DRI bug account <dri-devel>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

medium

Version:

unspecified

Hardware:

Other

OS:

All

Whiteboard:

i915 platform:

i915 features:

Attachments:

Description	Flags
dmesg with div by 0 call trace	none
dmesg with some own debugging messages	none
vbios	none
dmesg: power states picked by driver	none
fix div by 0	none
dmesg with patch	none

Description Rafał Miłecki 2010-06-24 10:30:33 UTC

drm-radeon-testing, RS780

[  379.169870] [drm:radeon_set_power_state], Setting: e: 0
[  379.169938] divide error: 0000 [#1] PREEMPT SMP
(...)
[  379.170077] Call Trace:
[  379.170105]  [<f96e20e6>] ? rs690_bandwidth_update+0x96/0xb60 [radeon]

Will provide more info.

Comment 1 Rafał Miłecki 2010-06-24 10:31:53 UTC

Created attachment 36467 [details]
dmesg with div by 0 call trace

Comment 2 Rafał Miłecki 2010-06-25 10:48:21 UTC

Created attachment 36489 [details]
dmesg with some own debugging messages

Alex could you take a look at this?

Comment 3 Rafał Miłecki 2010-06-25 11:05:40 UTC

Created attachment 36495 [details]
vbios

Comment 4 Rafał Miłecki 2010-06-25 11:07:38 UTC

Created attachment 36496 [details]
dmesg: power states picked by driver

Please note downclocked mode is marked as performance one :|

Comment 5 Alex Deucher 2010-06-25 12:08:13 UTC

Created attachment 36497 [details] [review]
fix div by 0

This should fix the issue.  IGP chips have the same clocks for the low performance and battery states, so it doesn't matter which we choose.

Comment 6 Rafał Miłecki 2010-06-25 13:09:52 UTC

Created attachment 36498 [details]
dmesg with patch

It works fine, hope that's what you expected :)

Can you submit to Dave?

Comment 7 Rafał Miłecki 2010-06-25 13:13:53 UTC

Could you by the way explain that magic cycling through the power states? 

I thought of doing same patch for r100.c, but there exists another condition:

if (rdev->pm.current_power_state_index == 0)

is that alright? I guess patch for r100.c is nod needed then?

Comment 8 Alex Deucher 2010-06-25 13:20:32 UTC

(In reply to comment #6)
> Created an attachment (id=36498) [details]
> dmesg with patch
> 
> It works fine, hope that's what you expected :)
> 
> Can you submit to Dave?

Will do.

(In reply to comment #7)
> Could you by the way explain that magic cycling through the power states? 
> 
> I thought of doing same patch for r100.c, but there exists another condition:
> 
> if (rdev->pm.current_power_state_index == 0)
> 
> is that alright? I guess patch for r100.c is nod needed then?

Yes, r100 doesn't need a similar patch as the power states are ordered differently.

Comment 9 Alex Deucher 2010-06-25 13:22:22 UTC

Patch sent.

Comment 10 Rafał Miłecki 2010-06-25 13:39:52 UTC

(In reply to comment #7)
> Could you by the way explain that magic cycling through the power states? 

Ping? :)

Why do we cycle through the (all?) power states?

Comment 11 Alex Deucher 2010-06-25 14:11:50 UTC

(In reply to comment #10)
> 
> Why do we cycle through the (all?) power states?

It's a simple algo :)
I suppose the alternative would be to just switch between a low and high one.  It's easier for the plls to lock after a small change, so there's less of a delay when making small changes to the clocks, but that's about it.  If we had more advanced pm algos there might be advantages to staying in intermediate states for certain reasons (e.g., to maintain enough bandwidth or performance for certain tasks, etc.).

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.