Martin Peres
2016-Apr-20 20:53 UTC
[Nouveau] [PATCH v4 27/37] clk: make pstate a pointer to nvkm_pstate
On 18/04/16 22:14, Karol Herbst wrote:> we will access the current set cstate at least every second and this safes ussaves> some CPU cycles looking them up every second. > > Signed-off-by: Karol Herbst <nouveau at karolherbst.de> > --- > drm/nouveau/include/nvkm/subdev/clk.h | 2 +- > drm/nouveau/nvkm/engine/device/ctrl.c | 5 ++++- > drm/nouveau/nvkm/subdev/clk/base.c | 12 ++++++++---- > drm/nouveau/nvkm/subdev/pmu/gk20a.c | 23 +++++++---------------- > 4 files changed, 20 insertions(+), 22 deletions(-) > > diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h > index db52e65..4fb2c1b 100644 > --- a/drm/nouveau/include/nvkm/subdev/clk.h > +++ b/drm/nouveau/include/nvkm/subdev/clk.h > @@ -91,7 +91,7 @@ struct nvkm_clk { > > struct nvkm_notify pwrsrc_ntfy; > int pwrsrc; > - int pstate; /* current */ > + struct nvkm_pstate *pstate; /* current */ > int ustate_ac; /* user-requested (-1 disabled, -2 perfmon) */ > int ustate_dc; /* user-requested (-1 disabled, -2 perfmon) */ > int astate; /* perfmon adjustment (base) */ > diff --git a/drm/nouveau/nvkm/engine/device/ctrl.c b/drm/nouveau/nvkm/engine/device/ctrl.c > index 039e8a4..cb85266 100644 > --- a/drm/nouveau/nvkm/engine/device/ctrl.c > +++ b/drm/nouveau/nvkm/engine/device/ctrl.c > @@ -52,7 +52,10 @@ nvkm_control_mthd_pstate_info(struct nvkm_control *ctrl, void *data, u32 size) > args->v0.ustate_ac = clk->ustate_ac; > args->v0.ustate_dc = clk->ustate_dc; > args->v0.pwrsrc = clk->pwrsrc; > - args->v0.pstate = clk->pstate; > + if (clk->pstate) > + args->v0.pstate = clk->pstate->pstate; > + else > + args->v0.pstate = -1; > } else { > args->v0.count = 0; > args->v0.ustate_ac = NVIF_CONTROL_PSTATE_INFO_V0_USTATE_DISABLE; > diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c > index 3867ab7..762dfe2 100644 > --- a/drm/nouveau/nvkm/subdev/clk/base.c > +++ b/drm/nouveau/nvkm/subdev/clk/base.c > @@ -288,7 +288,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei) > } > > nvkm_debug(subdev, "setting performance state %d\n", pstatei); > - clk->pstate = pstatei; > + clk->pstate = pstate; > > nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width); > > @@ -317,15 +317,19 @@ nvkm_clk_update_work(struct work_struct *work) > return; > clk->pwrsrc = power_supply_is_system_supplied(); > > + if (clk->pstate) > + pstate = clk->pstate->pstate; > + else > + pstate = -1; > nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d\n", > - clk->pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc, > + pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc, > clk->astate); > > pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc; > if (clk->state_nr && pstate != -1) { > pstate = (pstate < 0) ? clk->astate : pstate; > } else { > - pstate = clk->pstate = -1; > + pstate = -1;Isn't "clk->pstate = NULL;" missing here? Why did you change this code otherwise?> } > > nvkm_trace(subdev, "-> %d\n", pstate); > @@ -609,7 +613,7 @@ nvkm_clk_init(struct nvkm_subdev *subdev) > return clk->func->init(clk); > > clk->astate = clk->state_nr - 1; > - clk->pstate = -1; > + clk->pstate = NULL; > nvkm_clk_update(clk, true); > return 0; > } > diff --git a/drm/nouveau/nvkm/subdev/pmu/gk20a.c b/drm/nouveau/nvkm/subdev/pmu/gk20a.c > index f996d90..6f0d290 100644 > --- a/drm/nouveau/nvkm/subdev/pmu/gk20a.c > +++ b/drm/nouveau/nvkm/subdev/pmu/gk20a.c > @@ -57,24 +57,21 @@ gk20a_pmu_dvfs_target(struct gk20a_pmu *pmu, int *state) > } > > static int > -gk20a_pmu_dvfs_get_cur_state(struct gk20a_pmu *pmu, int *state) > -{ > - struct nvkm_clk *clk = pmu->base.subdev.device->clk; > - > - *state = clk->pstate; > - return 0; > -} > - > -static int > gk20a_pmu_dvfs_get_target_state(struct gk20a_pmu *pmu, > int *state, int load) > { > struct gk20a_pmu_dvfs_data *data = pmu->data; > struct nvkm_clk *clk = pmu->base.subdev.device->clk; > + struct nvkm_pstate *pstate = clk->pstate; > int cur_level, level; > > + if (!pstate) { > + *state = 0; > + return 1; > + } > + > /* For GK20A, the performance level is directly mapped to pstate */ > - level = cur_level = clk->pstate; > + level = cur_level = clk->pstate->pstate; > > if (load > data->p_load_max) { > level = min(clk->state_nr - 1, level + (clk->state_nr / 3)); > @@ -150,12 +147,6 @@ gk20a_pmu_dvfs_work(struct nvkm_alarm *alarm) > nvkm_trace(subdev, "utilization = %d %%, avg_load = %d %%\n", > utilization, data->avg_load); > > - ret = gk20a_pmu_dvfs_get_cur_state(pmu, &state); > - if (ret) { > - nvkm_warn(subdev, "failed to get current state\n"); > - goto resched; > - } > - > if (gk20a_pmu_dvfs_get_target_state(pmu, &state, data->avg_load)) { > nvkm_trace(subdev, "set new state to %d\n", state); > gk20a_pmu_dvfs_target(pmu, &state);
karol herbst
2016-Apr-20 21:46 UTC
[Nouveau] [PATCH v4 27/37] clk: make pstate a pointer to nvkm_pstate
2016-04-20 20:53 GMT+00:00 Martin Peres <martin.peres at free.fr>:> On 18/04/16 22:14, Karol Herbst wrote: >> >> we will access the current set cstate at least every second and this safes >> us > > saves >> >> some CPU cycles looking them up every second. >> >> Signed-off-by: Karol Herbst <nouveau at karolherbst.de> >> --- >> drm/nouveau/include/nvkm/subdev/clk.h | 2 +- >> drm/nouveau/nvkm/engine/device/ctrl.c | 5 ++++- >> drm/nouveau/nvkm/subdev/clk/base.c | 12 ++++++++---- >> drm/nouveau/nvkm/subdev/pmu/gk20a.c | 23 +++++++---------------- >> 4 files changed, 20 insertions(+), 22 deletions(-) >> >> diff --git a/drm/nouveau/include/nvkm/subdev/clk.h >> b/drm/nouveau/include/nvkm/subdev/clk.h >> index db52e65..4fb2c1b 100644 >> --- a/drm/nouveau/include/nvkm/subdev/clk.h >> +++ b/drm/nouveau/include/nvkm/subdev/clk.h >> @@ -91,7 +91,7 @@ struct nvkm_clk { >> struct nvkm_notify pwrsrc_ntfy; >> int pwrsrc; >> - int pstate; /* current */ >> + struct nvkm_pstate *pstate; /* current */ >> int ustate_ac; /* user-requested (-1 disabled, -2 perfmon) */ >> int ustate_dc; /* user-requested (-1 disabled, -2 perfmon) */ >> int astate; /* perfmon adjustment (base) */ >> diff --git a/drm/nouveau/nvkm/engine/device/ctrl.c >> b/drm/nouveau/nvkm/engine/device/ctrl.c >> index 039e8a4..cb85266 100644 >> --- a/drm/nouveau/nvkm/engine/device/ctrl.c >> +++ b/drm/nouveau/nvkm/engine/device/ctrl.c >> @@ -52,7 +52,10 @@ nvkm_control_mthd_pstate_info(struct nvkm_control >> *ctrl, void *data, u32 size) >> args->v0.ustate_ac = clk->ustate_ac; >> args->v0.ustate_dc = clk->ustate_dc; >> args->v0.pwrsrc = clk->pwrsrc; >> - args->v0.pstate = clk->pstate; >> + if (clk->pstate) >> + args->v0.pstate = clk->pstate->pstate; >> + else >> + args->v0.pstate = -1; >> } else { >> args->v0.count = 0; >> args->v0.ustate_ac >> NVIF_CONTROL_PSTATE_INFO_V0_USTATE_DISABLE; >> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c >> b/drm/nouveau/nvkm/subdev/clk/base.c >> index 3867ab7..762dfe2 100644 >> --- a/drm/nouveau/nvkm/subdev/clk/base.c >> +++ b/drm/nouveau/nvkm/subdev/clk/base.c >> @@ -288,7 +288,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei) >> } >> nvkm_debug(subdev, "setting performance state %d\n", pstatei); >> - clk->pstate = pstatei; >> + clk->pstate = pstate; >> nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width); >> @@ -317,15 +317,19 @@ nvkm_clk_update_work(struct work_struct *work) >> return; >> clk->pwrsrc = power_supply_is_system_supplied(); >> + if (clk->pstate) >> + pstate = clk->pstate->pstate; >> + else >> + pstate = -1; >> nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d\n", >> - clk->pstate, clk->pwrsrc, clk->ustate_ac, >> clk->ustate_dc, >> + pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc, >> clk->astate); >> pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc; >> if (clk->state_nr && pstate != -1) { >> pstate = (pstate < 0) ? clk->astate : pstate; >> } else { >> - pstate = clk->pstate = -1; >> + pstate = -1; > > > Isn't "clk->pstate = NULL;" missing here? Why did you change this code > otherwise? >clk->pstate will never be set as long as they are no states>> } >> nvkm_trace(subdev, "-> %d\n", pstate); >> @@ -609,7 +613,7 @@ nvkm_clk_init(struct nvkm_subdev *subdev) >> return clk->func->init(clk); >> clk->astate = clk->state_nr - 1; >> - clk->pstate = -1; >> + clk->pstate = NULL; >> nvkm_clk_update(clk, true); >> return 0; >> } >> diff --git a/drm/nouveau/nvkm/subdev/pmu/gk20a.c >> b/drm/nouveau/nvkm/subdev/pmu/gk20a.c >> index f996d90..6f0d290 100644 >> --- a/drm/nouveau/nvkm/subdev/pmu/gk20a.c >> +++ b/drm/nouveau/nvkm/subdev/pmu/gk20a.c >> @@ -57,24 +57,21 @@ gk20a_pmu_dvfs_target(struct gk20a_pmu *pmu, int >> *state) >> } >> static int >> -gk20a_pmu_dvfs_get_cur_state(struct gk20a_pmu *pmu, int *state) >> -{ >> - struct nvkm_clk *clk = pmu->base.subdev.device->clk; >> - >> - *state = clk->pstate; >> - return 0; >> -} >> - >> -static int >> gk20a_pmu_dvfs_get_target_state(struct gk20a_pmu *pmu, >> int *state, int load) >> { >> struct gk20a_pmu_dvfs_data *data = pmu->data; >> struct nvkm_clk *clk = pmu->base.subdev.device->clk; >> + struct nvkm_pstate *pstate = clk->pstate; >> int cur_level, level; >> + if (!pstate) { >> + *state = 0; >> + return 1; >> + } >> + >> /* For GK20A, the performance level is directly mapped to pstate >> */ >> - level = cur_level = clk->pstate; >> + level = cur_level = clk->pstate->pstate; >> if (load > data->p_load_max) { >> level = min(clk->state_nr - 1, level + (clk->state_nr / >> 3)); >> @@ -150,12 +147,6 @@ gk20a_pmu_dvfs_work(struct nvkm_alarm *alarm) >> nvkm_trace(subdev, "utilization = %d %%, avg_load = %d %%\n", >> utilization, data->avg_load); >> - ret = gk20a_pmu_dvfs_get_cur_state(pmu, &state); >> - if (ret) { >> - nvkm_warn(subdev, "failed to get current state\n"); >> - goto resched; >> - } >> - >> if (gk20a_pmu_dvfs_get_target_state(pmu, &state, data->avg_load)) >> { >> nvkm_trace(subdev, "set new state to %d\n", state); >> gk20a_pmu_dvfs_target(pmu, &state); > > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
Martin Peres
2016-Apr-20 22:56 UTC
[Nouveau] [PATCH v4 27/37] clk: make pstate a pointer to nvkm_pstate
On 21/04/16 00:46, karol herbst wrote:> 2016-04-20 20:53 GMT+00:00 Martin Peres <martin.peres at free.fr>: >> On 18/04/16 22:14, Karol Herbst wrote: >>> we will access the current set cstate at least every second and this safes >>> us >> saves >>> some CPU cycles looking them up every second. >>> >>> Signed-off-by: Karol Herbst <nouveau at karolherbst.de> >>> --- >>> drm/nouveau/include/nvkm/subdev/clk.h | 2 +- >>> drm/nouveau/nvkm/engine/device/ctrl.c | 5 ++++- >>> drm/nouveau/nvkm/subdev/clk/base.c | 12 ++++++++---- >>> drm/nouveau/nvkm/subdev/pmu/gk20a.c | 23 +++++++---------------- >>> 4 files changed, 20 insertions(+), 22 deletions(-) >>> >>> diff --git a/drm/nouveau/include/nvkm/subdev/clk.h >>> b/drm/nouveau/include/nvkm/subdev/clk.h >>> index db52e65..4fb2c1b 100644 >>> --- a/drm/nouveau/include/nvkm/subdev/clk.h >>> +++ b/drm/nouveau/include/nvkm/subdev/clk.h >>> @@ -91,7 +91,7 @@ struct nvkm_clk { >>> struct nvkm_notify pwrsrc_ntfy; >>> int pwrsrc; >>> - int pstate; /* current */ >>> + struct nvkm_pstate *pstate; /* current */ >>> int ustate_ac; /* user-requested (-1 disabled, -2 perfmon) */ >>> int ustate_dc; /* user-requested (-1 disabled, -2 perfmon) */ >>> int astate; /* perfmon adjustment (base) */ >>> diff --git a/drm/nouveau/nvkm/engine/device/ctrl.c >>> b/drm/nouveau/nvkm/engine/device/ctrl.c >>> index 039e8a4..cb85266 100644 >>> --- a/drm/nouveau/nvkm/engine/device/ctrl.c >>> +++ b/drm/nouveau/nvkm/engine/device/ctrl.c >>> @@ -52,7 +52,10 @@ nvkm_control_mthd_pstate_info(struct nvkm_control >>> *ctrl, void *data, u32 size) >>> args->v0.ustate_ac = clk->ustate_ac; >>> args->v0.ustate_dc = clk->ustate_dc; >>> args->v0.pwrsrc = clk->pwrsrc; >>> - args->v0.pstate = clk->pstate; >>> + if (clk->pstate) >>> + args->v0.pstate = clk->pstate->pstate; >>> + else >>> + args->v0.pstate = -1; >>> } else { >>> args->v0.count = 0; >>> args->v0.ustate_ac >>> NVIF_CONTROL_PSTATE_INFO_V0_USTATE_DISABLE; >>> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c >>> b/drm/nouveau/nvkm/subdev/clk/base.c >>> index 3867ab7..762dfe2 100644 >>> --- a/drm/nouveau/nvkm/subdev/clk/base.c >>> +++ b/drm/nouveau/nvkm/subdev/clk/base.c >>> @@ -288,7 +288,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei) >>> } >>> nvkm_debug(subdev, "setting performance state %d\n", pstatei); >>> - clk->pstate = pstatei; >>> + clk->pstate = pstate; >>> nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width); >>> @@ -317,15 +317,19 @@ nvkm_clk_update_work(struct work_struct *work) >>> return; >>> clk->pwrsrc = power_supply_is_system_supplied(); >>> + if (clk->pstate) >>> + pstate = clk->pstate->pstate; >>> + else >>> + pstate = -1; >>> nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d\n", >>> - clk->pstate, clk->pwrsrc, clk->ustate_ac, >>> clk->ustate_dc, >>> + pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc, >>> clk->astate); >>> pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc; >>> if (clk->state_nr && pstate != -1) { >>> pstate = (pstate < 0) ? clk->astate : pstate; >>> } else { >>> - pstate = clk->pstate = -1; >>> + pstate = -1; >> >> Isn't "clk->pstate = NULL;" missing here? Why did you change this code >> otherwise? >> > clk->pstate will never be set as long as they are no statesI see. Then, the patch is: Reviewed-by: Martin Peres <martin.peres at free.fr>> >>> } >>> nvkm_trace(subdev, "-> %d\n", pstate); >>> @@ -609,7 +613,7 @@ nvkm_clk_init(struct nvkm_subdev *subdev) >>> return clk->func->init(clk); >>> clk->astate = clk->state_nr - 1; >>> - clk->pstate = -1; >>> + clk->pstate = NULL; >>> nvkm_clk_update(clk, true); >>> return 0; >>> } >>> diff --git a/drm/nouveau/nvkm/subdev/pmu/gk20a.c >>> b/drm/nouveau/nvkm/subdev/pmu/gk20a.c >>> index f996d90..6f0d290 100644 >>> --- a/drm/nouveau/nvkm/subdev/pmu/gk20a.c >>> +++ b/drm/nouveau/nvkm/subdev/pmu/gk20a.c >>> @@ -57,24 +57,21 @@ gk20a_pmu_dvfs_target(struct gk20a_pmu *pmu, int >>> *state) >>> } >>> static int >>> -gk20a_pmu_dvfs_get_cur_state(struct gk20a_pmu *pmu, int *state) >>> -{ >>> - struct nvkm_clk *clk = pmu->base.subdev.device->clk; >>> - >>> - *state = clk->pstate; >>> - return 0; >>> -} >>> - >>> -static int >>> gk20a_pmu_dvfs_get_target_state(struct gk20a_pmu *pmu, >>> int *state, int load) >>> { >>> struct gk20a_pmu_dvfs_data *data = pmu->data; >>> struct nvkm_clk *clk = pmu->base.subdev.device->clk; >>> + struct nvkm_pstate *pstate = clk->pstate; >>> int cur_level, level; >>> + if (!pstate) { >>> + *state = 0; >>> + return 1; >>> + } >>> + >>> /* For GK20A, the performance level is directly mapped to pstate >>> */ >>> - level = cur_level = clk->pstate; >>> + level = cur_level = clk->pstate->pstate; >>> if (load > data->p_load_max) { >>> level = min(clk->state_nr - 1, level + (clk->state_nr / >>> 3)); >>> @@ -150,12 +147,6 @@ gk20a_pmu_dvfs_work(struct nvkm_alarm *alarm) >>> nvkm_trace(subdev, "utilization = %d %%, avg_load = %d %%\n", >>> utilization, data->avg_load); >>> - ret = gk20a_pmu_dvfs_get_cur_state(pmu, &state); >>> - if (ret) { >>> - nvkm_warn(subdev, "failed to get current state\n"); >>> - goto resched; >>> - } >>> - >>> if (gk20a_pmu_dvfs_get_target_state(pmu, &state, data->avg_load)) >>> { >>> nvkm_trace(subdev, "set new state to %d\n", state); >>> gk20a_pmu_dvfs_target(pmu, &state); >> >> _______________________________________________ >> Nouveau mailing list >> Nouveau at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/nouveau
Seemingly Similar Threads
- [PATCH v4 27/37] clk: make pstate a pointer to nvkm_pstate
- [PATCH 3/9] clk: Make pstate a pointer to nvkm_pstate
- [RFC PATCH 06/29] clk: Make pstate a pointer to nvkm_pstate
- [RFC PATCH 06/29] clk: Make pstate a pointer to nvkm_pstate
- [PATCH v4 27/37] clk: make pstate a pointer to nvkm_pstate