2010/2/17 Rafał Miłecki <zaj...@gmail.com>:
> We kept requested and current modes in many places, depending on current
> state.
> That was useless, one place for holding that is enough.
>
> Signed-off-by: Rafał Miłecki <zaj...@gmail.com>
> ---
> Tested on my RV620, no problems. Alex: can you review this patch? It's your
> code I modify/remove in it.
NACK. Why are you replacing pointers with copies of of the power
state structs? The idea is to keep one array of power states and
pointers to the current one, default one, and requested one. Then
comparing power states is just comparing pointers and when you change
the power state, you just update the pointer rather then memcpying the
entire struct.
Alex
> ---
> drivers/gpu/drm/radeon/radeon.h | 13 +++---
> drivers/gpu/drm/radeon/radeon_atombios.c | 22 +++-------
> drivers/gpu/drm/radeon/radeon_combios.c | 9 +++-
> drivers/gpu/drm/radeon/radeon_pm.c | 66
> +++++++++++++++++-------------
> 4 files changed, 56 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 993cdf2..b533411 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -652,9 +652,6 @@ struct radeon_power_state {
> struct radeon_pm_clock_info clock_info[8];
> /* number of valid clock modes in this power state */
> int num_clock_modes;
> - /* currently selected clock mode */
> - struct radeon_pm_clock_info *current_clock_mode;
> - struct radeon_pm_clock_info *requested_clock_mode;
> struct radeon_pm_clock_info *default_clock_mode;
> /* non clock info about this state */
> struct radeon_pm_non_clock_info non_clock_info;
> @@ -684,10 +681,12 @@ struct radeon_pm {
> /* XXX: use a define for num power modes */
> struct radeon_power_state power_state[8];
> /* number of valid power states */
> - int num_power_states;
> - struct radeon_power_state *current_power_state;
> - struct radeon_power_state *requested_power_state;
> - struct radeon_power_state *default_power_state;
> + int num_power_states;
> + struct radeon_pm_clock_info current_clock_mode;
> + int current_pcie_lanes;
> + struct radeon_pm_clock_info requested_clock_mode;
> + int requested_pcie_lanes;
> + struct radeon_power_state *default_power_state;
> };
>
>
> diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c
> b/drivers/gpu/drm/radeon/radeon_atombios.c
> index 4f7dbce..b8e7e81 100644
> --- a/drivers/gpu/drm/radeon/radeon_atombios.c
> +++ b/drivers/gpu/drm/radeon/radeon_atombios.c
> @@ -1441,7 +1441,6 @@ void radeon_atombios_get_power_modes(struct
> radeon_device *rdev)
> power_info = (union power_info *)(mode_info->atom_context->bios +
> data_offset);
>
> rdev->pm.default_power_state = NULL;
> - rdev->pm.current_power_state = NULL;
>
> if (power_info) {
> if (frev < 4) {
> @@ -1508,11 +1507,8 @@ void radeon_atombios_get_power_modes(struct
> radeon_device *rdev)
>
> rdev->pm.power_state[state_index].type =
>
> POWER_STATE_TYPE_DEFAULT;
> rdev->pm.default_power_state =
> &rdev->pm.power_state[state_index];
> - rdev->pm.current_power_state
> = &rdev->pm.power_state[state_index];
>
> rdev->pm.power_state[state_index].default_clock_mode =
>
> &rdev->pm.power_state[state_index].clock_info[0];
> -
> rdev->pm.power_state[state_index].current_clock_mode =
> -
> &rdev->pm.power_state[state_index].clock_info[0];
> }
> state_index++;
> break;
> @@ -1577,11 +1573,8 @@ void radeon_atombios_get_power_modes(struct
> radeon_device *rdev)
>
> rdev->pm.power_state[state_index].type =
>
> POWER_STATE_TYPE_DEFAULT;
> rdev->pm.default_power_state =
> &rdev->pm.power_state[state_index];
> - rdev->pm.current_power_state
> = &rdev->pm.power_state[state_index];
>
> rdev->pm.power_state[state_index].default_clock_mode =
>
> &rdev->pm.power_state[state_index].clock_info[0];
> -
> rdev->pm.power_state[state_index].current_clock_mode =
> -
> &rdev->pm.power_state[state_index].clock_info[0];
> }
> state_index++;
> break;
> @@ -1652,11 +1645,8 @@ void radeon_atombios_get_power_modes(struct
> radeon_device *rdev)
>
> rdev->pm.power_state[state_index].type =
>
> POWER_STATE_TYPE_DEFAULT;
> rdev->pm.default_power_state =
> &rdev->pm.power_state[state_index];
> - rdev->pm.current_power_state
> = &rdev->pm.power_state[state_index];
>
> rdev->pm.power_state[state_index].default_clock_mode =
>
> &rdev->pm.power_state[state_index].clock_info[0];
> -
> rdev->pm.power_state[state_index].current_clock_mode =
> -
> &rdev->pm.power_state[state_index].clock_info[0];
> }
> state_index++;
> break;
> @@ -1756,11 +1746,8 @@ void radeon_atombios_get_power_modes(struct
> radeon_device *rdev)
>
> rdev->pm.power_state[state_index].type =
>
> POWER_STATE_TYPE_DEFAULT;
> rdev->pm.default_power_state =
> &rdev->pm.power_state[state_index];
> - rdev->pm.current_power_state
> = &rdev->pm.power_state[state_index];
>
> rdev->pm.power_state[state_index].default_clock_mode =
>
> &rdev->pm.power_state[state_index].clock_info[mode_index - 1];
> -
> rdev->pm.power_state[state_index].current_clock_mode =
> -
> &rdev->pm.power_state[state_index].clock_info[mode_index - 1];
> }
> state_index++;
> }
> @@ -1779,18 +1766,21 @@ void radeon_atombios_get_power_modes(struct
> radeon_device *rdev)
> rdev->pm.power_state[state_index].clock_info[0].sclk =
> rdev->clock.default_sclk;
> rdev->pm.power_state[state_index].default_clock_mode =
> &rdev->pm.power_state[state_index].clock_info[0];
> - rdev->pm.power_state[state_index].current_clock_mode =
> - &rdev->pm.power_state[state_index].clock_info[0];
> rdev->pm.power_state[state_index].clock_info[0].voltage.type =
> VOLTAGE_NONE;
> if (rdev->asic->get_pcie_lanes)
>
> rdev->pm.power_state[state_index].non_clock_info.pcie_lanes =
> radeon_get_pcie_lanes(rdev);
> else
>
> rdev->pm.power_state[state_index].non_clock_info.pcie_lanes = 16;
> rdev->pm.default_power_state =
> &rdev->pm.power_state[state_index];
> - rdev->pm.current_power_state =
> &rdev->pm.power_state[state_index];
> state_index++;
> }
> rdev->pm.num_power_states = state_index;
> +
> + memcpy((unsigned char *)&rdev->pm.current_clock_mode,
> + (unsigned char
> *)rdev->pm.default_power_state->default_clock_mode,
> + sizeof(struct radeon_pm_clock_info));
> + rdev->pm.current_pcie_lanes =
> + rdev->pm.default_power_state->non_clock_info.pcie_lanes;
> }
>
> void radeon_atom_set_clock_gating(struct radeon_device *rdev, int enable)
> diff --git a/drivers/gpu/drm/radeon/radeon_combios.c
> b/drivers/gpu/drm/radeon/radeon_combios.c
> index 9989d22..efc190e 100644
> --- a/drivers/gpu/drm/radeon/radeon_combios.c
> +++ b/drivers/gpu/drm/radeon/radeon_combios.c
> @@ -2358,7 +2358,6 @@ void radeon_combios_get_power_modes(struct
> radeon_device *rdev)
> int state_index = 0;
>
> rdev->pm.default_power_state = NULL;
> - rdev->pm.current_power_state = NULL;
>
> if (rdev->flags & RADEON_IS_MOBILITY) {
> offset = combios_get_table_offset(dev,
> COMBIOS_POWERPLAY_INFO_TABLE);
> @@ -2447,15 +2446,19 @@ default_mode:
> rdev->pm.power_state[state_index].clock_info[0].mclk =
> rdev->clock.default_mclk;
> rdev->pm.power_state[state_index].clock_info[0].sclk =
> rdev->clock.default_sclk;
> rdev->pm.power_state[state_index].default_clock_mode =
> &rdev->pm.power_state[state_index].clock_info[0];
> - rdev->pm.power_state[state_index].current_clock_mode =
> &rdev->pm.power_state[state_index].clock_info[0];
> rdev->pm.power_state[state_index].clock_info[0].voltage.type =
> VOLTAGE_NONE;
> if (rdev->asic->get_pcie_lanes)
> rdev->pm.power_state[state_index].non_clock_info.pcie_lanes =
> radeon_get_pcie_lanes(rdev);
> else
> rdev->pm.power_state[state_index].non_clock_info.pcie_lanes =
> 16;
> rdev->pm.default_power_state = &rdev->pm.power_state[state_index];
> - rdev->pm.current_power_state = &rdev->pm.power_state[state_index];
> rdev->pm.num_power_states = state_index + 1;
> +
> + memcpy((unsigned char *)&rdev->pm.current_clock_mode,
> + (unsigned char
> *)rdev->pm.default_power_state->default_clock_mode,
> + sizeof(struct radeon_pm_clock_info));
> + rdev->pm.current_pcie_lanes =
> + rdev->pm.default_power_state->non_clock_info.pcie_lanes;
> }
>
> void radeon_external_tmds_setup(struct drm_encoder *encoder)
> diff --git a/drivers/gpu/drm/radeon/radeon_pm.c
> b/drivers/gpu/drm/radeon/radeon_pm.c
> index 520197f..8426aff 100644
> --- a/drivers/gpu/drm/radeon/radeon_pm.c
> +++ b/drivers/gpu/drm/radeon/radeon_pm.c
> @@ -162,51 +162,61 @@ static struct radeon_pm_clock_info *
> radeon_pick_clock_mode(struct radeon_device
> static void radeon_get_power_state(struct radeon_device *rdev,
> enum radeon_pm_action action)
> {
> + struct radeon_power_state *requested_power_state;
> + struct radeon_pm_clock_info *requested_clock_mode;
> +
> switch (action) {
> - case PM_ACTION_NONE:
> - default:
> - rdev->pm.requested_power_state = rdev->pm.current_power_state;
> - rdev->pm.requested_power_state->requested_clock_mode =
> - rdev->pm.requested_power_state->current_clock_mode;
> - break;
> case PM_ACTION_MINIMUM:
> - rdev->pm.requested_power_state =
> radeon_pick_power_state(rdev, POWER_STATE_TYPE_BATTERY);
> - rdev->pm.requested_power_state->requested_clock_mode =
> - radeon_pick_clock_mode(rdev,
> rdev->pm.requested_power_state, POWER_MODE_TYPE_LOW);
> + requested_power_state = radeon_pick_power_state(rdev,
> POWER_STATE_TYPE_BATTERY);
> + requested_clock_mode = radeon_pick_clock_mode(rdev,
> requested_power_state, POWER_MODE_TYPE_LOW);
> break;
> case PM_ACTION_DOWNCLOCK:
> - rdev->pm.requested_power_state =
> radeon_pick_power_state(rdev, POWER_STATE_TYPE_POWERSAVE);
> - rdev->pm.requested_power_state->requested_clock_mode =
> - radeon_pick_clock_mode(rdev,
> rdev->pm.requested_power_state, POWER_MODE_TYPE_MID);
> + requested_power_state = radeon_pick_power_state(rdev,
> POWER_STATE_TYPE_POWERSAVE);
> + requested_clock_mode = radeon_pick_clock_mode(rdev,
> requested_power_state, POWER_MODE_TYPE_MID);
> break;
> case PM_ACTION_UPCLOCK:
> - rdev->pm.requested_power_state =
> radeon_pick_power_state(rdev, POWER_STATE_TYPE_DEFAULT);
> - rdev->pm.requested_power_state->requested_clock_mode =
> - radeon_pick_clock_mode(rdev,
> rdev->pm.requested_power_state, POWER_MODE_TYPE_HIGH);
> + requested_power_state = radeon_pick_power_state(rdev,
> POWER_STATE_TYPE_DEFAULT);
> + requested_clock_mode = radeon_pick_clock_mode(rdev,
> requested_power_state, POWER_MODE_TYPE_HIGH);
> break;
> + case PM_ACTION_NONE:
> + default:
> + DRM_ERROR("Trying to get power state for not defined
> action!\n");
> + return;
> }
> - DRM_INFO("Requested: e: %d m: %d p: %d\n",
> - rdev->pm.requested_power_state->requested_clock_mode->sclk,
> - rdev->pm.requested_power_state->requested_clock_mode->mclk,
> - rdev->pm.requested_power_state->non_clock_info.pcie_lanes);
> +
> + memcpy((unsigned char *)&rdev->pm.requested_clock_mode,
> + (unsigned char *)requested_clock_mode,
> + sizeof(struct radeon_pm_clock_info));
> + rdev->pm.requested_pcie_lanes =
> + requested_power_state->non_clock_info.pcie_lanes;
> +
> + DRM_INFO("Requested: eng: %d; mem: %d; lanes: %d\n",
> + rdev->pm.requested_clock_mode.sclk,
> + rdev->pm.requested_clock_mode.mclk,
> + rdev->pm.requested_pcie_lanes);
> }
>
> static void radeon_set_power_state(struct radeon_device *rdev)
> {
> - if (rdev->pm.requested_power_state == rdev->pm.current_power_state)
> - return;
> + struct radeon_pm_clock_info *req = &rdev->pm.requested_clock_mode;
> + struct radeon_pm_clock_info *curr = &rdev->pm.current_clock_mode;
> +
> + /*DRM_INFO("Current: eng: %d; mem: %d; lanes: %d\n",
> + curr->sclk, curr->mclk, rdev->pm.current_pcie_lanes);*/
> +
> + DRM_INFO("Setting: eng: %d; mem: %d; lanes: %d\n",
> + req->sclk, req->mclk, rdev->pm.requested_pcie_lanes);
>
> - DRM_INFO("Setting: e: %d m: %d p: %d\n",
> - rdev->pm.requested_power_state->requested_clock_mode->sclk,
> - rdev->pm.requested_power_state->requested_clock_mode->mclk,
> - rdev->pm.requested_power_state->non_clock_info.pcie_lanes);
> /* set pcie lanes */
> /* set voltage */
> +
> /* set engine clock */
> - radeon_set_engine_clock(rdev,
> rdev->pm.requested_power_state->requested_clock_mode->sclk);
> - /* set memory clock */
> + if (req->sclk != curr->sclk) {
> + radeon_set_engine_clock(rdev, req->sclk);
> + curr->sclk = req->sclk;
> + }
>
> - rdev->pm.current_power_state = rdev->pm.requested_power_state;
> + /* set memory clock */
> }
>
> int radeon_pm_init(struct radeon_device *rdev)
> --
> 1.6.4.2
>
>
------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
