Direct Rendering Infrastructure

W dniu 17 grudnia 2009 03:02 użytkownik Rafał Miłecki
<zajec5@...> napisał:
> V2: reorganize functions, fix modesetting calls
> V3: rebase patch, use radeon's workqueue
> V4: enable on tested chipsets only, request VBLANK IRQs
> V5: enable PM on older hardware (IRQs, mode_fixup, dpms)
> V6: use separate dynpm module parameter
>
> Tested on my M82==RV620 and my new M96==RV730. Also tested on RV515 by
> Sedat where already posted patch for AtomBIOS's mutex was needed.

Also Nezmer's M92==45xx was downclocked fine.

And:
<bjaglin> Zajec: looks like your patch is working just fine on my
RV250, down and upclocking when running/stopping glxgears :)

-- 
Rafał

2009/12/16 Rafał Miłecki <zajec5@...>:
> V2: reorganize functions, fix modesetting calls
> V3: rebase patch, use radeon's workqueue
> V4: enable on tested chipsets only, request VBLANK IRQs
> V5: enable PM on older hardware (IRQs, mode_fixup, dpms)
> V6: use separate dynpm module parameter
>
> Tested on my M82==RV620 and my new M96==RV730. Also tested on RV515 by
> Sedat where already posted patch for AtomBIOS's mutex was needed.

How does re-clocking happen if no crtcs are active?  We will want to
be much more aggressive when the displays are off.

Alex

v6 runs fine on rv515. Thanks Rafal.

--- Speedup in 3D: Running Anholt's openarena benchmark demo ---

$ openarena +exec anholt 2>&1 | egrep -e '[0-9]+ frames'
NEW: 840 frames 10.2 seconds 82.1 fps 4.0/12.2/56.0/5.4 ms
OLD: 840 frames 37.4 seconds 22.4 fps 15.0/44.6/97.0/13.4 ms

Even playing a youtube music-video in firefox parallelly reduces fps down to 63.

System description:
[1] kernel: upstream 2.6.32-git13 (incl. drm-linus)
+0001-drm-radeon-kms-add-dynamic-engine-reclocking-V6.patch
+0001-drm-radeon-kms-prevent-parallel-AtomBIOS-calls.patch
[2] ddx: xf86-video-ati 6.12.99 (git20091210.f198590)
[3] libdrm 2.4.16
[4] mesa-7.8: Using r300g dri/statetracker
commit 09cef45393c14d2b02529cb3cbea194bdfc06bf3
"r600 : clean a bit to prepare to enable gl2."
[5] xorg-server 1.7.3.901
[6] KMS enabled (modeset=1) and DynPM enabled (dynpm=1)

Thanks to all contributor to Gallium3D Radeon (r300g).

- Sedat -

2009/12/17 Rafał Miłecki <zajec5@...>:
> V2: reorganize functions, fix modesetting calls
> V3: rebase patch, use radeon's workqueue
> V4: enable on tested chipsets only, request VBLANK IRQs
> V5: enable PM on older hardware (IRQs, mode_fixup, dpms)
> V6: use separate dynpm module parameter
>
> Tested on my M82==RV620 and my new M96==RV730. Also tested on RV515 by
> Sedat where already posted patch for AtomBIOS's mutex was needed.
>
> --
> Rafał
>

W dniu 17 grudnia 2009 06:43 użytkownik Alex Deucher
<alexdeucher@...> napisał:
> 2009/12/16 Rafał Miłecki <zajec5@...>:
>> V2: reorganize functions, fix modesetting calls
>> V3: rebase patch, use radeon's workqueue
>> V4: enable on tested chipsets only, request VBLANK IRQs
>> V5: enable PM on older hardware (IRQs, mode_fixup, dpms)
>> V6: use separate dynpm module parameter
>>
>> Tested on my M82==RV620 and my new M96==RV730. Also tested on RV515 by
>> Sedat where already posted patch for AtomBIOS's mutex was needed.
>
> How does re-clocking happen if no crtcs are active?  We will want to
> be much more aggressive when the displays are off.

Currently we treat such situation as 1 crtc active. That is something
I want to improve in next patch when this one will be commitet. I hope
it is not stopping issue.

-- 
Rafał

2009/12/17 Rafał Miłecki <zajec5@...>:
> W dniu 17 grudnia 2009 06:43 użytkownik Alex Deucher
> <alexdeucher@...> napisał:
>> 2009/12/16 Rafał Miłecki <zajec5@...>:
>>> V2: reorganize functions, fix modesetting calls
>>> V3: rebase patch, use radeon's workqueue
>>> V4: enable on tested chipsets only, request VBLANK IRQs
>>> V5: enable PM on older hardware (IRQs, mode_fixup, dpms)
>>> V6: use separate dynpm module parameter
>>>
>>> Tested on my M82==RV620 and my new M96==RV730. Also tested on RV515 by
>>> Sedat where already posted patch for AtomBIOS's mutex was needed.
>>
>> How does re-clocking happen if no crtcs are active?  We will want to
>> be much more aggressive when the displays are off.
>
> Currently we treat such situation as 1 crtc active. That is something
> I want to improve in next patch when this one will be commitet. I hope
> it is not stopping issue.
>

The issue you won't get vblank interrupts if the crtcs are off, so no
reclocking will occur.

Alex

W dniu 18 grudnia 2009 01:08 użytkownik Alex Deucher
<alexdeucher@...> napisał:
> 2009/12/17 Rafał Miłecki <zajec5@...>:
>> W dniu 17 grudnia 2009 06:43 użytkownik Alex Deucher
>> <alexdeucher@...> napisał:
>>> 2009/12/16 Rafał Miłecki <zajec5@...>:
>>>> V2: reorganize functions, fix modesetting calls
>>>> V3: rebase patch, use radeon's workqueue
>>>> V4: enable on tested chipsets only, request VBLANK IRQs
>>>> V5: enable PM on older hardware (IRQs, mode_fixup, dpms)
>>>> V6: use separate dynpm module parameter
>>>>
>>>> Tested on my M82==RV620 and my new M96==RV730. Also tested on RV515 by
>>>> Sedat where already posted patch for AtomBIOS's mutex was needed.
>>>
>>> How does re-clocking happen if no crtcs are active?  We will want to
>>> be much more aggressive when the displays are off.
>>
>> Currently we treat such situation as 1 crtc active. That is something
>> I want to improve in next patch when this one will be commitet. I hope
>> it is not stopping issue.
>>
>
> The issue you won't get vblank interrupts if the crtcs are off, so no
> reclocking will occur.

Whoops, nice catch!

-- 
Rafał

W dniu 18 grudnia 2009 01:08 użytkownik Alex Deucher
<alexdeucher@...> napisał:
> 2009/12/17 Rafał Miłecki <zajec5@...>:
>> W dniu 17 grudnia 2009 06:43 użytkownik Alex Deucher
>> <alexdeucher@...> napisał:
>>> 2009/12/16 Rafał Miłecki <zajec5@...>:
>>>> V2: reorganize functions, fix modesetting calls
>>>> V3: rebase patch, use radeon's workqueue
>>>> V4: enable on tested chipsets only, request VBLANK IRQs
>>>> V5: enable PM on older hardware (IRQs, mode_fixup, dpms)
>>>> V6: use separate dynpm module parameter
>>>>
>>>> Tested on my M82==RV620 and my new M96==RV730. Also tested on RV515 by
>>>> Sedat where already posted patch for AtomBIOS's mutex was needed.
>>>
>>> How does re-clocking happen if no crtcs are active?  We will want to
>>> be much more aggressive when the displays are off.
>>
>> Currently we treat such situation as 1 crtc active. That is something
>> I want to improve in next patch when this one will be commitet. I hope
>> it is not stopping issue.
>>
>
> The issue you won't get vblank interrupts if the crtcs are off, so no
> reclocking will occur.

Actually you discovered another bug in my patch. We can not do following:
if (mode == DRM_MODE_DPMS_OFF)
        radeon_pm_compute_clocks(rdev);
in radeon_encoders.c because we won't react on DPMS on. My bad I had
screensaver disabled.


-- 
Rafał

On Thu, Dec 17, 2009 at 03:02:51AM +0100, Rafał Miłecki wrote:
> V2: reorganize functions, fix modesetting calls
> V3: rebase patch, use radeon's workqueue
> V4: enable on tested chipsets only, request VBLANK IRQs
> V5: enable PM on older hardware (IRQs, mode_fixup, dpms)
> V6: use separate dynpm module parameter
> 
> Tested on my M82==RV620 and my new M96==RV730. Also tested on RV515 by
> Sedat where already posted patch for AtomBIOS's mutex was needed.
> 
> -- 
> Rafał

Few comments.

I think it would be better to change :
RADEON_PM_ACTION_NONE -> PM_NONE ...
We already know we are in the radeon module so this is redundant
to put it in the name again. And i don't think PM_NONE will conflict
with other kernel symbol. Same apply to all other name.

I wish to convert radeon module to use dev_err, dev_info, ... instead
of DRM_DEBUG, DRM_INFO. Motivation behind that is multi GPU computer.
It's not common yet but i think we will want to support that in the
future.

Convertion to match this can be done as a latter patch. There is
few things that might be improved but that shouldn't matter that
much.

Good works,

Cheers,
Jerome

W dniu 18 grudnia 2009 13:24 użytkownik Jerome Glisse
<glisse@...> napisał:
> I think it would be better to change :
> RADEON_PM_ACTION_NONE -> PM_NONE ...
> We already know we are in the radeon module so this is redundant
> to put it in the name again. And i don't think PM_NONE will conflict
> with other kernel symbol. Same apply to all other name.

Thanks. I guess you meant PM_ACTION_NONE? I think it'll easier to
understand code when we include "ACTION" string.


> I wish to convert radeon module to use dev_err, dev_info, ... instead
> of DRM_DEBUG, DRM_INFO. Motivation behind that is multi GPU computer.
> It's not common yet but i think we will want to support that in the
> future.
>
> Convertion to match this can be done as a latter patch. There is
> few things that might be improved but that shouldn't matter that
> much.

As that's general issue (we use DRM_*... everywhere) I would prefer to
make this patch follow general design for now.


Thanks for comments.

-- 
Rafał

On Fri, Dec 18, 2009 at 02:30:34PM +0100, Rafał Miłecki wrote:
> W dniu 18 grudnia 2009 13:24 użytkownik Jerome Glisse
> <glisse@...> napisał:
> > I think it would be better to change :
> > RADEON_PM_ACTION_NONE -> PM_NONE ...
> > We already know we are in the radeon module so this is redundant
> > to put it in the name again. And i don't think PM_NONE will conflict
> > with other kernel symbol. Same apply to all other name.
> 
> Thanks. I guess you meant PM_ACTION_NONE? I think it'll easier to
> understand code when we include "ACTION" string.
> 
> 
> > I wish to convert radeon module to use dev_err, dev_info, ... instead
> > of DRM_DEBUG, DRM_INFO. Motivation behind that is multi GPU computer.
> > It's not common yet but i think we will want to support that in the
> > future.
> >
> > Convertion to match this can be done as a latter patch. There is
> > few things that might be improved but that shouldn't matter that
> > much.
> 
> As that's general issue (we use DRM_*... everywhere) I would prefer to
> make this patch follow general design for now.
> 
> 
> Thanks for comments.
> 
> -- 
> Rafał
> 

No we don't use DRM_* everywhere. I slowly convert to dev_* when
doing a patch. But as i said i could do the conversion myself latter.

Cheers,
Jerome

W dniu 18 grudnia 2009 15:18 użytkownik Jerome Glisse
<glisse@...> napisał:
> On Fri, Dec 18, 2009 at 02:30:34PM +0100, Rafał Miłecki wrote:
>> W dniu 18 grudnia 2009 13:24 użytkownik Jerome Glisse
>> <glisse@...> napisał:
>> > I think it would be better to change :
>> > RADEON_PM_ACTION_NONE -> PM_NONE ...
>> > We already know we are in the radeon module so this is redundant
>> > to put it in the name again. And i don't think PM_NONE will conflict
>> > with other kernel symbol. Same apply to all other name.
>>
>> Thanks. I guess you meant PM_ACTION_NONE? I think it'll easier to
>> understand code when we include "ACTION" string.
>>
>>
>> > I wish to convert radeon module to use dev_err, dev_info, ... instead
>> > of DRM_DEBUG, DRM_INFO. Motivation behind that is multi GPU computer.
>> > It's not common yet but i think we will want to support that in the
>> > future.
>> >
>> > Convertion to match this can be done as a latter patch. There is
>> > few things that might be improved but that shouldn't matter that
>> > much.
>>
>> As that's general issue (we use DRM_*... everywhere) I would prefer to
>> make this patch follow general design for now.
>>
>>
>> Thanks for comments.
>>
>> --
>> Rafał
>>
>
> No we don't use DRM_* everywhere. I slowly convert to dev_* when
> doing a patch. But as i said i could do the conversion myself latter.

Ups, sorry, didn't notice that :)

-- 
Rafał