Dynamic clock cause hard lockup on Radeon 7000 + SMP

[Kristian Høgsberg]

Kristian Høgsberg wrote:
> Alex Deucher wrote:
> 
>> On Tue, 23 Nov 2004 18:27:44 -0500, Alex Deucher 
>> <alexdeucher at gmail.com> wrote:
>>
>>> On Tue, 23 Nov 2004 17:46:24 -0500, Kristian Høgsberg 
>>> <krh at bitplanet.net> wrote:
>>>
>>>
>>>
>>>>
>>>> Alex Deucher wrote:
>>>>
>>>>> On Tue, 23 Nov 2004 17:25:00 -0500, Kristian Høgsberg 
>>>>> <krh at bitplanet.net> wrote:
>>>>>
>>>>>
>>>>>> Alex Deucher wrote:
>>>>>>
>>>>>>
>>>>>>> On Tue, 23 Nov 2004 16:26:14 -0500, Michel Dänzer 
>>>>>>> <michel at daenzer.net> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> On Tue, 2004-11-23 at 15:06 -0500, Alex Deucher wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> On Tue, 23 Nov 2004 15:01:08 -0500, Alex Deucher 
>>>>>>>>> <alexdeucher at gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> On Tue, 23 Nov 2004 14:21:18 -0500, Kristian Høgsberg 
>>>>>>>>>> <krh at bitplanet.net> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> -            if ( !info->HasCRTC2 ) {
>>>>>>>>> +           if ((info->ChipFamily == CHIP_FAMILY_RV100) &&
>>>>>>>>> !(info->IsMobility)) {
>>>>>>>>> +               break;
>>>>>>>>> +           } else if ( !info->HasCRTC2 ) {
>>>>>>>>
>>>>>>>>
>>>>>>>> It should probably simply be enabled only on Mobility chips.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> right now it's not enabled for _any_ chips by default, however, the
>>>>>>> not enabled case which sets the clocks explicitly to "on" rather 
>>>>>>> than
>>>>>>> "dynamic" seems to cause lockups for Kristian.  Kristian, does it 
>>>>>>> lock
>>>>>>> up when you turn dynamicclocks on (sets clocks for dynamic) on your
>>>>>>> board?
>>>>>>
>>>>>>
>>>>>> Hmm, no, enabling dynamic clocks also makes the crash go away.  I was
>>>>>> wondering why not enabling dynamic clocks (ie no "DynamicClocks" 
>>>>>> option)
>>>>>> also need to change the PLL bits. I was expecting that in that 
>>>>>> case it
>>>>>> could just leave it as is (i.e. 6.7.0 behaviour).  Or in code:
>>>>>>
>>>>>>    if (xf86ReturnOptValBool(info->Options, OPTION_DYNAMIC_CLOCKS,
>>>>>> FALSE)) {
>>>>>>        RADEONSetDynamicClock(pScrn, 1);
>>>>>>    }
>>>>>>
>>>>>> instead of
>>>>>>
>>>>>>    if (xf86ReturnOptValBool(info->Options, OPTION_DYNAMIC_CLOCKS,
>>>>>> FALSE)) {
>>>>>>        RADEONSetDynamicClock(pScrn, 1);
>>>>>>    } else {
>>>>>>        RADEONSetDynamicClock(pScrn, 0);
>>>>>>    }
>>>>>
>>>>>
>>>>> the problem was that certain asics apparently need to have their
>>>>> clocks forced on rather than left in their default configuration.  the
>>>>> old 6.7 code forced on several clocks in the accel init code in radeon
>>>>> _accel.c.  that code was removed or commented out in favor of the code
>>>>> here.
>>>>
>>>>
>>>> OK, so the right fix would be to port that clock configuration from 6.7
>>>> radeon_accel.c into the 'case 0:' part of RADEONSetDynamicClock()?
>>>>
>>>
>>> it seems that way.  I'll try and take a loot at it this week at some
>>> point and come up with a better patch unless someone beats me to it.
>>>
>>> Alex
>>
>>
>>
>> See if this works for you (also added to bugzilla).
> 
> 
> Alex, this is awesome, thanks a lot.  I'll give it a try Monday after 
> thanksgiving.

OK, it seems to work too.  I wasn't able to crash the box with the 
latest version of the patch.  I updated the bugzilla bug, but I didn't 
close the bug just yet.  I think we should add this to HEAD and 6.8.2.

cheers,
Kristian