[Nouveau] [RFC PATCH 04/29] clk: Rename nvkm_pstate_calc to nvkm_clk_update and export it

[Pierre Moreau]

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 <karolherbst at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/b22f82ac/attachment.sig>