Skip to content

[WIP] Video plugin improvements - #32

Merged
yarda merged 1 commit into
redhat-performance:masterfrom
olysonek:radeon
Sep 19, 2017
Merged

[WIP] Video plugin improvements#32
yarda merged 1 commit into
redhat-performance:masterfrom
olysonek:radeon

Conversation

@olysonek

Copy link
Copy Markdown
Contributor

Support the 'dpm' power method on Radeon GPUs. Implement automatic selection of the best power method and use it in the balanced profile.

Resolves: rhbz#1417659

@olysonek olysonek changed the title Video plugin improvements [WIP] Video plugin improvements Apr 18, 2017
@olysonek

Copy link
Copy Markdown
Contributor Author

Do not merge. I doesn't work.

@yarda

yarda commented Apr 26, 2017

Copy link
Copy Markdown
Contributor

Is it ready for merge?

@yarda

yarda commented Apr 26, 2017

Copy link
Copy Markdown
Contributor

I do not have HW to test :)

@olysonek

Copy link
Copy Markdown
Contributor Author

I don't know, I can't test it either. Let's wait for Oliver.

@oliverhenshaw oliverhenshaw left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This look generally right to me - thanks.

A couple of niggles though - adding "radeon_dpm_state=balanced" is not enough to avoid complaints in tuned.log and from "tuned-adm verify". I need to add "radeon_powersave=dpm" too.

Secondly, "tuned-adm verify" doesn't seem to do anything to check radeon_dpm_state - I tried setting it to a nonsense value and although I got an error when changing profile I didn't see any complaints when I ran the verify subcommand.

Comment thread tuned/plugins/plugin_video.py Outdated
elif value == "dpm":
if not sim and self._get_radeon_powersave(device) != "dpm":
log.error("Cannot set dpm power method through the video plugin." \
+ "Use the sysctl plugin with the 'radeon.dpm=1' option")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is strictly correct. The radeon.dpm option is a boot-time parameter, not one that can be changed with sysctl.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I guess I meant to say the bootloader plugin.

Support changing the 'power_dpm_state' parameter of Radeon
GPUs. It only works if the 'dpm' power method is enabled.
Note that the 'dpm' power method cannot be enabled through
the video plugin, but it can be set using the bootloader plugin
using 'cmdline = radeon.dpm=1'.

The video plugin now recognizes a new 'radeon_dpm_state' option
which changes the dpm state. Possible values are 'battery',
'balanced' and 'performance' [1].

[video]
radeon_dpm_state=balanced

[1] https://wiki.archlinux.org/index.php/ATI#Dynamic_power_management

Resolves: rhbz#1417659

Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
@olysonek

olysonek commented Jul 4, 2017

Copy link
Copy Markdown
Contributor Author

A couple of niggles though - adding "radeon_dpm_state=balanced" is not enough to avoid complaints in
tuned.log and from "tuned-adm verify". I need to add "radeon_powersave=dpm" too.

I don't understand. What exactly are you seeing in the log? Adding "radeon_powersave=dpm" should actually cause an error message.

Secondly, "tuned-adm verify" doesn't seem to do anything to check radeon_dpm_state - I tried setting it to a nonsense value and although I got an error when changing profile I didn't see any complaints when I ran the verify subcommand.

The verify command seems to behave badly in some cases when setting nonsensical values. The same happens in the CPU plugin with governor for example. We'll have to deal with that later in some more generic way. But I think the verify command should now correctly detect that the current value is different from a legal value in the tuned profile.

@yarda

yarda commented Sep 19, 2017

Copy link
Copy Markdown
Contributor

Thanks, LGTM, I will also probably update the profiles to use the DPM by default.

@yarda
yarda merged commit dd6460b into redhat-performance:master Sep 19, 2017
yarda added a commit that referenced this pull request Sep 19, 2017
Now, in addition to the already supported Radeon power methods, it also
supports:

radeon_powersave=DPM

where DPM can be 'battery', 'balanced', or 'performance'.

It also supports fallback, so one can write:

radeon_powersave=balanced, auto

where it tries 'balanced' DPM profile and if it fails or is not
supported it fallbacks to the non DPM 'auto' profile.

As delimiters colon, semilocon, comma or whitespace can be used.

It also switched upstream profiles to prefer DPM.

#32

Related: rhbz#1417659

Signed-off-by: Jaroslav Škarvada <jskarvad@redhat.com>
@yarda

yarda commented Sep 19, 2017

Copy link
Copy Markdown
Contributor

I rewrote it a bit in eef09f1. Unfortunately I do not have compatible video card, so I wasn't able to test it.

@olysonek
olysonek deleted the radeon branch September 19, 2017 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants