Patches simply add tracking of voltage and debug setting it. While it may look trivial and not important, having it would save a lot of time in debugging FDO bug #36081. This way I try to say it could be nice to have it in .35 as voltage stuff is new and we still may discover some bugs around it.
Rafał Miłecki (2): drm/radeon/kms/r600+: do not set the same voltage as current one drm/radeon/kms: add trivial debugging for voltage
drivers/gpu/drm/radeon/evergreen.c | 9 +++++++-- drivers/gpu/drm/radeon/r600.c | 9 +++++++-- drivers/gpu/drm/radeon/radeon.h | 1 + drivers/gpu/drm/radeon/radeon_pm.c | 4 ++++ drivers/gpu/drm/radeon/rv770.c | 9 +++++++-- 5 files changed, 26 insertions(+), 6 deletions(-)
We have similar tracking for engine and memory. We could think about some solution for older GPUs as well, however there are not strict voltage values set.
Signed-off-by: Rafał Miłecki zajec5@gmail.com --- drivers/gpu/drm/radeon/evergreen.c | 8 ++++++-- drivers/gpu/drm/radeon/r600.c | 8 ++++++-- drivers/gpu/drm/radeon/radeon.h | 1 + drivers/gpu/drm/radeon/radeon_pm.c | 2 ++ drivers/gpu/drm/radeon/rv770.c | 8 ++++++-- 5 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 174f718..c444808 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -46,8 +46,12 @@ void evergreen_pm_misc(struct radeon_device *rdev) struct radeon_power_state *ps = &rdev->pm.power_state[req_ps_idx]; struct radeon_voltage *voltage = &ps->clock_info[req_cm_idx].voltage;
- if ((voltage->type == VOLTAGE_SW) && voltage->voltage) - radeon_atom_set_voltage(rdev, voltage->voltage); + if (voltage->voltage != rdev->pm.current_vddc) { + if ((voltage->type == VOLTAGE_SW) && voltage->voltage) { + radeon_atom_set_voltage(rdev, voltage->voltage); + rdev->pm.current_vddc = voltage->voltage; + } + } }
void evergreen_pm_prepare(struct radeon_device *rdev) diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index 94c27d0..e49bebe 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -480,8 +480,12 @@ void r600_pm_misc(struct radeon_device *rdev) struct radeon_power_state *ps = &rdev->pm.power_state[req_ps_idx]; struct radeon_voltage *voltage = &ps->clock_info[req_cm_idx].voltage;
- if ((voltage->type == VOLTAGE_SW) && voltage->voltage) - radeon_atom_set_voltage(rdev, voltage->voltage); + if (voltage->voltage != rdev->pm.current_vddc) { + if ((voltage->type == VOLTAGE_SW) && voltage->voltage) { + radeon_atom_set_voltage(rdev, voltage->voltage); + rdev->pm.current_vddc = voltage->voltage; + } + } }
bool r600_gui_idle(struct radeon_device *rdev) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 084221d..04bc867 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -745,6 +745,7 @@ struct radeon_pm { int default_power_state_index; u32 current_sclk; u32 current_mclk; + u32 current_vddc; struct radeon_i2c_chan *i2c_bus; /* selected pm method */ enum radeon_pm_method pm_method; diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c index 0228126..a046fe7 100644 --- a/drivers/gpu/drm/radeon/radeon_pm.c +++ b/drivers/gpu/drm/radeon/radeon_pm.c @@ -381,6 +381,7 @@ void radeon_pm_suspend(struct radeon_device *rdev) rdev->pm.current_clock_mode_index = -1; rdev->pm.current_sclk = 0; rdev->pm.current_mclk = 0; + rdev->pm.current_vddc = 0; mutex_unlock(&rdev->pm.mutex); }
@@ -400,6 +401,7 @@ int radeon_pm_init(struct radeon_device *rdev) rdev->pm.dynpm_can_downclock = true; rdev->pm.current_sclk = 0; rdev->pm.current_mclk = 0; + rdev->pm.current_vddc = 0;
if (rdev->bios) { if (rdev->is_atom_bios) diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c index f002848..f310fa8 100644 --- a/drivers/gpu/drm/radeon/rv770.c +++ b/drivers/gpu/drm/radeon/rv770.c @@ -49,8 +49,12 @@ void rv770_pm_misc(struct radeon_device *rdev) struct radeon_power_state *ps = &rdev->pm.power_state[req_ps_idx]; struct radeon_voltage *voltage = &ps->clock_info[req_cm_idx].voltage;
- if ((voltage->type == VOLTAGE_SW) && voltage->voltage) - radeon_atom_set_voltage(rdev, voltage->voltage); + if (voltage->voltage != rdev->pm.current_vddc) { + if ((voltage->type == VOLTAGE_SW) && voltage->voltage) { + radeon_atom_set_voltage(rdev, voltage->voltage); + rdev->pm.current_vddc = voltage->voltage; + } + } }
/*
2010/6/6 Rafał Miłecki zajec5@gmail.com:
We have similar tracking for engine and memory. We could think about some solution for older GPUs as well, however there are not strict voltage values set.
Signed-off-by: Rafał Miłecki zajec5@gmail.com
I actually had almost the exact same patch in my tree (broken out from my voltage step patch), I just hadn't gotten around to sending it. Mine is against drm-linus and has the default voltage level set. See attached.
Alex
drivers/gpu/drm/radeon/evergreen.c | 8 ++++++-- drivers/gpu/drm/radeon/r600.c | 8 ++++++-- drivers/gpu/drm/radeon/radeon.h | 1 + drivers/gpu/drm/radeon/radeon_pm.c | 2 ++ drivers/gpu/drm/radeon/rv770.c | 8 ++++++-- 5 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 174f718..c444808 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -46,8 +46,12 @@ void evergreen_pm_misc(struct radeon_device *rdev) struct radeon_power_state *ps = &rdev->pm.power_state[req_ps_idx]; struct radeon_voltage *voltage = &ps->clock_info[req_cm_idx].voltage;
- if ((voltage->type == VOLTAGE_SW) && voltage->voltage)
- radeon_atom_set_voltage(rdev, voltage->voltage);
- if (voltage->voltage != rdev->pm.current_vddc) {
- if ((voltage->type == VOLTAGE_SW) && voltage->voltage) {
- radeon_atom_set_voltage(rdev, voltage->voltage);
- rdev->pm.current_vddc = voltage->voltage;
- }
- }
}
void evergreen_pm_prepare(struct radeon_device *rdev) diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index 94c27d0..e49bebe 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -480,8 +480,12 @@ void r600_pm_misc(struct radeon_device *rdev) struct radeon_power_state *ps = &rdev->pm.power_state[req_ps_idx]; struct radeon_voltage *voltage = &ps->clock_info[req_cm_idx].voltage;
- if ((voltage->type == VOLTAGE_SW) && voltage->voltage)
- radeon_atom_set_voltage(rdev, voltage->voltage);
- if (voltage->voltage != rdev->pm.current_vddc) {
- if ((voltage->type == VOLTAGE_SW) && voltage->voltage) {
- radeon_atom_set_voltage(rdev, voltage->voltage);
- rdev->pm.current_vddc = voltage->voltage;
- }
- }
}
bool r600_gui_idle(struct radeon_device *rdev) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 084221d..04bc867 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -745,6 +745,7 @@ struct radeon_pm { int default_power_state_index; u32 current_sclk; u32 current_mclk;
- u32 current_vddc;
struct radeon_i2c_chan *i2c_bus; /* selected pm method */ enum radeon_pm_method pm_method; diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c index 0228126..a046fe7 100644 --- a/drivers/gpu/drm/radeon/radeon_pm.c +++ b/drivers/gpu/drm/radeon/radeon_pm.c @@ -381,6 +381,7 @@ void radeon_pm_suspend(struct radeon_device *rdev) rdev->pm.current_clock_mode_index = -1; rdev->pm.current_sclk = 0; rdev->pm.current_mclk = 0;
- rdev->pm.current_vddc = 0;
mutex_unlock(&rdev->pm.mutex); }
@@ -400,6 +401,7 @@ int radeon_pm_init(struct radeon_device *rdev) rdev->pm.dynpm_can_downclock = true; rdev->pm.current_sclk = 0; rdev->pm.current_mclk = 0;
- rdev->pm.current_vddc = 0;
if (rdev->bios) { if (rdev->is_atom_bios) diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c index f002848..f310fa8 100644 --- a/drivers/gpu/drm/radeon/rv770.c +++ b/drivers/gpu/drm/radeon/rv770.c @@ -49,8 +49,12 @@ void rv770_pm_misc(struct radeon_device *rdev) struct radeon_power_state *ps = &rdev->pm.power_state[req_ps_idx]; struct radeon_voltage *voltage = &ps->clock_info[req_cm_idx].voltage;
- if ((voltage->type == VOLTAGE_SW) && voltage->voltage)
- radeon_atom_set_voltage(rdev, voltage->voltage);
- if (voltage->voltage != rdev->pm.current_vddc) {
- if ((voltage->type == VOLTAGE_SW) && voltage->voltage) {
- radeon_atom_set_voltage(rdev, voltage->voltage);
- rdev->pm.current_vddc = voltage->voltage;
- }
- }
}
/*
1.6.4.2
W dniu 7 czerwca 2010 07:00 użytkownik Alex Deucher alexdeucher@gmail.com napisał:
2010/6/6 Rafał Miłecki zajec5@gmail.com:
We have similar tracking for engine and memory. We could think about some solution for older GPUs as well, however there are not strict voltage values set.
Signed-off-by: Rafał Miłecki zajec5@gmail.com
I actually had almost the exact same patch in my tree (broken out from my voltage step patch), I just hadn't gotten around to sending it. Mine is against drm-linus and has the default voltage level set. See attached.
I know, actually I've stolen your name (curr_vddc).
Do you plan to include this in .35? If yes, it's fine. If not, I'd prefer to split your patch into two. The one I've sent and second that adds you step setting.
2010/6/7 Rafał Miłecki zajec5@gmail.com:
W dniu 7 czerwca 2010 07:00 użytkownik Alex Deucher alexdeucher@gmail.com napisał:
2010/6/6 Rafał Miłecki zajec5@gmail.com:
We have similar tracking for engine and memory. We could think about some solution for older GPUs as well, however there are not strict voltage values set.
Signed-off-by: Rafał Miłecki zajec5@gmail.com
I actually had almost the exact same patch in my tree (broken out from my voltage step patch), I just hadn't gotten around to sending it. Mine is against drm-linus and has the default voltage level set. See attached.
I know, actually I've stolen your name (curr_vddc).
Do you plan to include this in .35? If yes, it's fine. If not, I'd prefer to split your patch into two. The one I've sent and second that adds you step setting.
I'm not planning to include the whole step setting patch, but I did break out the current_vddc part and planned to push that (attached to the last email) for 2.6.35.
Alex
W dniu 7 czerwca 2010 18:09 użytkownik Alex Deucher alexdeucher@gmail.com napisał:
2010/6/7 Rafał Miłecki zajec5@gmail.com:
W dniu 7 czerwca 2010 07:00 użytkownik Alex Deucher alexdeucher@gmail.com napisał:
2010/6/6 Rafał Miłecki zajec5@gmail.com:
We have similar tracking for engine and memory. We could think about some solution for older GPUs as well, however there are not strict voltage values set.
Signed-off-by: Rafał Miłecki zajec5@gmail.com
I actually had almost the exact same patch in my tree (broken out from my voltage step patch), I just hadn't gotten around to sending it. Mine is against drm-linus and has the default voltage level set. See attached.
I know, actually I've stolen your name (curr_vddc).
Do you plan to include this in .35? If yes, it's fine. If not, I'd prefer to split your patch into two. The one I've sent and second that adds you step setting.
I'm not planning to include the whole step setting patch, but I did break out the current_vddc part and planned to push that (attached to the last email) for 2.6.35.
Ahh, indeed, sorry. I just wonder about setting current_vddc even when we do not call radeon_atom_set_voltage. This way we will have false value in current_vddc, if voltage change is not available (type != VOLTAGE_SW).
2010/6/7 Rafał Miłecki zajec5@gmail.com:
W dniu 7 czerwca 2010 18:09 użytkownik Alex Deucher alexdeucher@gmail.com napisał:
2010/6/7 Rafał Miłecki zajec5@gmail.com:
W dniu 7 czerwca 2010 07:00 użytkownik Alex Deucher alexdeucher@gmail.com napisał:
2010/6/6 Rafał Miłecki zajec5@gmail.com:
We have similar tracking for engine and memory. We could think about some solution for older GPUs as well, however there are not strict voltage values set.
Signed-off-by: Rafał Miłecki zajec5@gmail.com
I actually had almost the exact same patch in my tree (broken out from my voltage step patch), I just hadn't gotten around to sending it. Mine is against drm-linus and has the default voltage level set. See attached.
I know, actually I've stolen your name (curr_vddc).
Do you plan to include this in .35? If yes, it's fine. If not, I'd prefer to split your patch into two. The one I've sent and second that adds you step setting.
I'm not planning to include the whole step setting patch, but I did break out the current_vddc part and planned to push that (attached to the last email) for 2.6.35.
Ahh, indeed, sorry. I just wonder about setting current_vddc even when we do not call radeon_atom_set_voltage. This way we will have false value in current_vddc, if voltage change is not available (type != VOLTAGE_SW).
The default voltage level is set when the asic is initialized at post. I've changed the default behaviour to no set the default mode unconditionally at start up since it's already set by asicinit and it avoids the possibility that someone might get a hang on module load due to pm. See this patch: http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=commitdiff;h... Only VOLTAGE_SW uses current_vddc so it's irrelevant for other voltage types.
Alex
W dniu 7 czerwca 2010 18:24 użytkownik Alex Deucher alexdeucher@gmail.com napisał:
Only VOLTAGE_SW uses current_vddc so it's irrelevant for other voltage types.
I introduced displaying this value in radeom_pm_info debugfs in next patch.
2010/6/7 Rafał Miłecki zajec5@gmail.com:
W dniu 7 czerwca 2010 18:24 użytkownik Alex Deucher alexdeucher@gmail.com napisał:
Only VOLTAGE_SW uses current_vddc so it's irrelevant for other voltage types.
I introduced displaying this value in radeom_pm_info debugfs in next patch.
It should be set to 0 by default unless the asic supports VOLTAGE_SW, in which case, it will have a valid value. Other voltage types don't have any way of getting the actual voltage value; they use either either a low/high toggle or an index. So for debugfs, you can either only expose it for VOLTAGE_SW type or just have it always be 0 for other voltage types.
Alex
Signed-off-by: Rafał Miłecki zajec5@gmail.com --- drivers/gpu/drm/radeon/evergreen.c | 1 + drivers/gpu/drm/radeon/r600.c | 1 + drivers/gpu/drm/radeon/radeon_pm.c | 2 ++ drivers/gpu/drm/radeon/rv770.c | 1 + 4 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index c444808..ff21c97 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -50,6 +50,7 @@ void evergreen_pm_misc(struct radeon_device *rdev) if ((voltage->type == VOLTAGE_SW) && voltage->voltage) { radeon_atom_set_voltage(rdev, voltage->voltage); rdev->pm.current_vddc = voltage->voltage; + DRM_DEBUG("Setting: v: %d\n", voltage->voltage); } } } diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index e49bebe..1bcf22c 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -484,6 +484,7 @@ void r600_pm_misc(struct radeon_device *rdev) if ((voltage->type == VOLTAGE_SW) && voltage->voltage) { radeon_atom_set_voltage(rdev, voltage->voltage); rdev->pm.current_vddc = voltage->voltage; + DRM_DEBUG("Setting: v: %d\n", voltage->voltage); } } } diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c index a046fe7..31da562 100644 --- a/drivers/gpu/drm/radeon/radeon_pm.c +++ b/drivers/gpu/drm/radeon/radeon_pm.c @@ -707,6 +707,8 @@ static int radeon_debugfs_pm_info(struct seq_file *m, void *data) seq_printf(m, "default memory clock: %u0 kHz\n", rdev->clock.default_mclk); if (rdev->asic->get_memory_clock) seq_printf(m, "current memory clock: %u0 kHz\n", radeon_get_memory_clock(rdev)); + if (rdev->pm.current_vddc) + seq_printf(m, "voltage: %u mV\n", rdev->pm.current_vddc); if (rdev->asic->get_pcie_lanes) seq_printf(m, "PCIE lanes: %d\n", radeon_get_pcie_lanes(rdev));
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c index f310fa8..fd9bf4e 100644 --- a/drivers/gpu/drm/radeon/rv770.c +++ b/drivers/gpu/drm/radeon/rv770.c @@ -53,6 +53,7 @@ void rv770_pm_misc(struct radeon_device *rdev) if ((voltage->type == VOLTAGE_SW) && voltage->voltage) { radeon_atom_set_voltage(rdev, voltage->voltage); rdev->pm.current_vddc = voltage->voltage; + DRM_DEBUG("Setting: v: %d\n", voltage->voltage); } } }
2010/6/6 Rafał Miłecki zajec5@gmail.com:
Signed-off-by: Rafał Miłecki zajec5@gmail.com
Looks good.
Signed-off-by: Alex Deucher alexdeucher@gmail.com
drivers/gpu/drm/radeon/evergreen.c | 1 + drivers/gpu/drm/radeon/r600.c | 1 + drivers/gpu/drm/radeon/radeon_pm.c | 2 ++ drivers/gpu/drm/radeon/rv770.c | 1 + 4 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index c444808..ff21c97 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -50,6 +50,7 @@ void evergreen_pm_misc(struct radeon_device *rdev) if ((voltage->type == VOLTAGE_SW) && voltage->voltage) { radeon_atom_set_voltage(rdev, voltage->voltage); rdev->pm.current_vddc = voltage->voltage;
- DRM_DEBUG("Setting: v: %d\n", voltage->voltage);
} } } diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index e49bebe..1bcf22c 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -484,6 +484,7 @@ void r600_pm_misc(struct radeon_device *rdev) if ((voltage->type == VOLTAGE_SW) && voltage->voltage) { radeon_atom_set_voltage(rdev, voltage->voltage); rdev->pm.current_vddc = voltage->voltage;
- DRM_DEBUG("Setting: v: %d\n", voltage->voltage);
} } } diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c index a046fe7..31da562 100644 --- a/drivers/gpu/drm/radeon/radeon_pm.c +++ b/drivers/gpu/drm/radeon/radeon_pm.c @@ -707,6 +707,8 @@ static int radeon_debugfs_pm_info(struct seq_file *m, void *data) seq_printf(m, "default memory clock: %u0 kHz\n", rdev->clock.default_mclk); if (rdev->asic->get_memory_clock) seq_printf(m, "current memory clock: %u0 kHz\n", radeon_get_memory_clock(rdev));
- if (rdev->pm.current_vddc)
- seq_printf(m, "voltage: %u mV\n", rdev->pm.current_vddc);
if (rdev->asic->get_pcie_lanes) seq_printf(m, "PCIE lanes: %d\n", radeon_get_pcie_lanes(rdev));
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c index f310fa8..fd9bf4e 100644 --- a/drivers/gpu/drm/radeon/rv770.c +++ b/drivers/gpu/drm/radeon/rv770.c @@ -53,6 +53,7 @@ void rv770_pm_misc(struct radeon_device *rdev) if ((voltage->type == VOLTAGE_SW) && voltage->voltage) { radeon_atom_set_voltage(rdev, voltage->voltage); rdev->pm.current_vddc = voltage->voltage;
- DRM_DEBUG("Setting: v: %d\n", voltage->voltage);
} } } -- 1.6.4.2
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Rafał Miłecki