On 2017-09-15 — 17:11, Karol Herbst wrote: > This function will be used to update the current clock state. > > This will happen for various reasons: > * Temperature changes > * User changes clocking state > * Load changes > > v2: remove parameter name > > Signed-off-by: Karol Herbst <karolher...@gmail.com> > --- > drm/nouveau/include/nvkm/subdev/clk.h | 1 + > drm/nouveau/nvkm/subdev/clk/base.c | 26 ++++++++++++++++---------- > 2 files changed, 17 insertions(+), 10 deletions(-) > > diff --git a/drm/nouveau/include/nvkm/subdev/clk.h > b/drm/nouveau/include/nvkm/subdev/clk.h > index e5275f74..ce3bbcfe 100644 > --- a/drm/nouveau/include/nvkm/subdev/clk.h > +++ b/drm/nouveau/include/nvkm/subdev/clk.h > @@ -123,6 +123,7 @@ int nvkm_clk_ustate(struct nvkm_clk *, int req, int pwr); > int nvkm_clk_astate(struct nvkm_clk *, int req, int rel, bool wait); > int nvkm_clk_dstate(struct nvkm_clk *, int req, int rel); > int nvkm_clk_tstate(struct nvkm_clk *, u8 temperature); > +int nvkm_clk_update(struct nvkm_clk *, bool wait); > > int nv04_clk_new(struct nvkm_device *, int, struct nvkm_clk **); > int nv40_clk_new(struct nvkm_device *, int, struct nvkm_clk **); > diff --git a/drm/nouveau/nvkm/subdev/clk/base.c > b/drm/nouveau/nvkm/subdev/clk/base.c > index e4c8d310..ecff3ff3 100644 > --- a/drm/nouveau/nvkm/subdev/clk/base.c > +++ b/drm/nouveau/nvkm/subdev/clk/base.c > @@ -296,7 +296,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei) > } > > static void > -nvkm_pstate_work(struct work_struct *work) > +nvkm_clk_update_work(struct work_struct *work) > { > struct nvkm_clk *clk = container_of(work, typeof(*clk), work); > struct nvkm_subdev *subdev = &clk->subdev; > @@ -332,9 +332,15 @@ nvkm_pstate_work(struct work_struct *work) > nvkm_notify_get(&clk->pwrsrc_ntfy); > } > > -static int > -nvkm_pstate_calc(struct nvkm_clk *clk, bool wait) > +int > +nvkm_clk_update(struct nvkm_clk *clk, bool wait) > { > + if (!clk) > + return -EINVAL; > + > + if (!clk->allow_reclock) > + return -ENODEV; > + > atomic_set(&clk->waiting, 1); > schedule_work(&clk->work); > if (wait) > @@ -524,7 +530,7 @@ nvkm_clk_ustate(struct nvkm_clk *clk, int req, int pwr) > if (ret >= 0) { > if (ret -= 2, pwr) clk->ustate_ac = ret; > else clk->ustate_dc = ret; > - return nvkm_pstate_calc(clk, true); > + return nvkm_clk_update(clk, true); > } > return ret; > } > @@ -536,7 +542,7 @@ nvkm_clk_astate(struct nvkm_clk *clk, int req, int rel, > bool wait) > if ( rel) clk->astate += rel; > clk->astate = min(clk->astate, clk->state_nr - 1); > clk->astate = max(clk->astate, 0); > - return nvkm_pstate_calc(clk, wait); > + return nvkm_clk_update(clk, wait); > } > > int > @@ -545,7 +551,7 @@ nvkm_clk_tstate(struct nvkm_clk *clk, u8 temp) > if (clk->temp == temp) > return 0; > clk->temp = temp; > - return nvkm_pstate_calc(clk, false); > + return nvkm_clk_update(clk, false); > } > > int > @@ -555,7 +561,7 @@ nvkm_clk_dstate(struct nvkm_clk *clk, int req, int rel) > if ( rel) clk->dstate += rel; > clk->dstate = min(clk->dstate, clk->state_nr - 1); > clk->dstate = max(clk->dstate, 0); > - return nvkm_pstate_calc(clk, true); > + return nvkm_clk_update(clk, true); > } > > static int > @@ -563,7 +569,7 @@ nvkm_clk_pwrsrc(struct nvkm_notify *notify) > { > struct nvkm_clk *clk = > container_of(notify, typeof(*clk), pwrsrc_ntfy); > - nvkm_pstate_calc(clk, false); > + nvkm_clk_update(clk, false); > return NVKM_NOTIFY_DROP;
Same here as below, shouldn’t there be some error checking on what `nvkm_clk_update()` returned, as it can fail? > } > > @@ -618,7 +624,7 @@ nvkm_clk_init(struct nvkm_subdev *subdev) > clk->dstate = 0; > clk->pstate = -1; > clk->temp = 90; /* reasonable default value */ > - nvkm_pstate_calc(clk, true); > + nvkm_clk_update(clk, true); > return 0; `nvkm_pstate_calc()` did not return any error code, but `nvkm_clk_update()` now can, so shouldn’t the function return the return value of `nvkm_clk_update()` instead? Or at least do some error checking on what `nvkm_clk_update()` returned? > } > > @@ -675,7 +681,7 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct > nvkm_device *device, > clk->ustate_dc = -1; > clk->allow_reclock = allow_reclock; > > - INIT_WORK(&clk->work, nvkm_pstate_work); > + INIT_WORK(&clk->work, nvkm_clk_update_work); > init_waitqueue_head(&clk->wait); > atomic_set(&clk->waiting, 0); > > -- > 2.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
signature.asc
Description: PGP signature
_______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau