date:20171008

Re: [Nouveau] [RFC PATCH 15/29] bios: add thermal policies table

2017-10-08 Thread Karol Herbst

On Sun, Oct 8, 2017 at 8:40 PM, Pierre Moreau  wrote:
> Looking good. It would be nice to have some defines/enums for the different
> modes. Some comments about t0, t1 and t2 would be nice. I saw you are using t0
> in patch 16, but I have no idea why use t0 rather than t1 or t2.
>

I think I will remove t1 and t2 for now, because I have no idea when
they are used. I also have no idea what those modes are doing except
changing things in a strange way. The only thing I know for sure is
the mode == 1 case and the throttle limits. This table also defines
how fast we should throttle, but for now, until we figure out more, we
go with the jump to min immediately approach.

> Otherwise,
> Acked-by: Pierre Moreau 
>
> On 2017-09-15 — 17:11, Karol Herbst wrote:
>> Signed-off-by: Karol Herbst 
>> ---
>>  .../include/nvkm/subdev/bios/thermal_policies.h| 27 
>>  drm/nouveau/nvkm/subdev/bios/Kbuild|  1 +
>>  drm/nouveau/nvkm/subdev/bios/thermal_policies.c| 81 
>> ++
>>  3 files changed, 109 insertions(+)
>>  create mode 100644 drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h
>>  create mode 100644 drm/nouveau/nvkm/subdev/bios/thermal_policies.c
>>
>> diff --git a/drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h 
>> b/drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h
>> new file mode 100644
>> index ..c9215fdd
>> --- /dev/null
>> +++ b/drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h
>> @@ -0,0 +1,27 @@
>> +#ifndef __NVBIOS_THERMAL_POLICIES_H__
>> +#define __NVBIOS_THERMAL_POLICIES_H__
>> +
>> +struct nvbios_thermal_policies_header {
>> + u32 offset;
>> +
>> + u8 version;
>> + u8 hlen;
>> + u8 ecount;
>> + u8 elen;
>> +};
>> +struct nvbios_thermal_policies_entry {
>> + u8  mode;
>> + u16 t0;
>> + u16 t1;
>> + u16 t2;
>> + s16 down_offset;
>> + s16 up_offset;
>> +};
>> +
>> +int nvbios_thermal_policies_parse(struct nvkm_bios *,
>> +   struct nvbios_thermal_policies_header *);
>> +int nvbios_thermal_policies_entry(struct nvkm_bios *,
>> +   struct nvbios_thermal_policies_header *,
>> +   u8 idx,
>> +   struct nvbios_thermal_policies_entry *);
>> +#endif
>> diff --git a/drm/nouveau/nvkm/subdev/bios/Kbuild 
>> b/drm/nouveau/nvkm/subdev/bios/Kbuild
>> index 6b4f1e06..38f31dd0 100644
>> --- a/drm/nouveau/nvkm/subdev/bios/Kbuild
>> +++ b/drm/nouveau/nvkm/subdev/bios/Kbuild
>> @@ -30,6 +30,7 @@ nvkm-y += nvkm/subdev/bios/shadowramin.o
>>  nvkm-y += nvkm/subdev/bios/shadowrom.o
>>  nvkm-y += nvkm/subdev/bios/timing.o
>>  nvkm-y += nvkm/subdev/bios/therm.o
>> +nvkm-y += nvkm/subdev/bios/thermal_policies.o
>>  nvkm-y += nvkm/subdev/bios/vmap.o
>>  nvkm-y += nvkm/subdev/bios/volt.o
>>  nvkm-y += nvkm/subdev/bios/vpstate.o
>> diff --git a/drm/nouveau/nvkm/subdev/bios/thermal_policies.c 
>> b/drm/nouveau/nvkm/subdev/bios/thermal_policies.c
>> new file mode 100644
>> index ..5105194e
>> --- /dev/null
>> +++ b/drm/nouveau/nvkm/subdev/bios/thermal_policies.c
>> @@ -0,0 +1,81 @@
>> +/*
>> + * Copyright 2017 Karol Herbst
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included 
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + *
>> + * Authors: Karol Herbst
>> + */
>> +#include 
>> +#include 
>> +#include 
>> +
>> +static u32
>> +nvbios_thermal_policies_offset(struct nvkm_bios *b)
>> +{
>> + struct bit_entry bit_P;
>> +
>> + if (!bit_entry(b, 'P', _P)) {
>> + if (bit_P.version == 2 && bit_P.length >= 0x50)
>> + return nvbios_rd32(b, bit_P.offset + 0x50);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +int
>> +nvbios_thermal_policies_parse(struct nvkm_bios *b, struct 
>> nvbios_thermal_policies_header *h)
>> +{
>> + if (!h)
>> +

Re: [Nouveau] [RFC PATCH 16/29] clk: parse thermal policies for throttling thresholds

2017-10-08 Thread Karol Herbst

On Sun, Oct 8, 2017 at 8:36 PM, Pierre Moreau  wrote:
> On 2017-09-15 — 17:11, Karol Herbst wrote:
>> v2: use min_t
>>
>> Signed-off-by: Karol Herbst 
>> ---
>>  drm/nouveau/include/nvkm/subdev/clk.h |  2 ++
>>  drm/nouveau/nvkm/subdev/clk/base.c| 42 
>> +++
>>  2 files changed, 44 insertions(+)
>>
>> diff --git a/drm/nouveau/include/nvkm/subdev/clk.h 
>> b/drm/nouveau/include/nvkm/subdev/clk.h
>> index f35518c3..f5ff1fd9 100644
>> --- a/drm/nouveau/include/nvkm/subdev/clk.h
>> +++ b/drm/nouveau/include/nvkm/subdev/clk.h
>> @@ -104,6 +104,8 @@ struct nvkm_clk {
>>   struct nvkm_cstate *cstate;
>>   int exp_cstateid;
>>   u8  temp;
>> + u8  max_temp;
>> + u8  relax_temp;
>
> I guess those should be int, similar to the changes you made in patch 1.
>
>>
>>   bool allow_reclock;
>>  #define NVKM_CLK_BOOST_NONE 0x0
>> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c 
>> b/drm/nouveau/nvkm/subdev/clk/base.c
>> index 54188d2b..54e14936 100644
>> --- a/drm/nouveau/nvkm/subdev/clk/base.c
>> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
>> @@ -27,6 +27,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -659,6 +660,44 @@ nvkm_clk = {
>>   .fini = nvkm_clk_fini,
>>  };
>>
>> +static void
>> +nvkm_clk_parse_max_temp(struct nvkm_clk *clk)
>> +{
>> + struct nvkm_subdev *subdev = >subdev;
>> + struct nvkm_bios *bios = subdev->device->bios;
>> + struct nvbios_thermal_policies_header header;
>> + struct nvbios_thermal_policies_entry entry;
>> + u8 i;
>> + s16 mt = 0xff;
>> + s16 rt = 0xff;
>
> Why 0xff? I mean, sure, there is a high possibility that the GPU won’t like
> temperatures above 255℃, but still.

just a hack to properly check if we set any values we can use and make
available.

>
>> +
>> + if (nvbios_thermal_policies_parse(bios, ))
>> + return;
>> +
>> + if (!header.ecount)
>> + return;
>> +
>> + for (i = 0; i < header.ecount; i++) {
>> + if (nvbios_thermal_policies_entry(bios, , i, ))
>> + return;
>> +
>> + if (entry.mode != 1)
>> + continue;
>> +
>> + mt = min_t(s16, mt, (entry.t0 + entry.down_offset) / 32);
>> + rt = min_t(s16, rt, (entry.t0 + entry.up_offset) / 32);
>> + }
>> +
>> + if (mt == 0xff || rt == 0xff)
>> + return;
>> +
>> + clk->max_temp = mt;
>> + clk->relax_temp = rt;
>> +
>> + nvkm_debug(subdev, "setting up sw throttling thresholds (%u/%u°C)\n",
>> +clk->max_temp, clk->relax_temp);
>> +}
>> +
>>  int
>>  nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device,
>> int index, bool allow_reclock, struct nvkm_clk *clk)
>> @@ -733,6 +772,9 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct 
>> nvkm_device *device,
>>
>>   clk->boost_mode = nvkm_longopt(device->cfgopt, "NvBoost",
>>  NVKM_CLK_BOOST_NONE);
>> +
>> + nvkm_clk_parse_max_temp(clk);
>> +
>>   return 0;
>>  }
>>
>> --
>> 2.14.1
>>
>> ___
>> Nouveau mailing list
>> Nouveau@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [RFC PATCH 17/29] clk: thermal throttling

2017-10-08 Thread Pierre Moreau

Reviewed-by: Pierre Moreau 

On 2017-09-15 — 17:11, Karol Herbst wrote:
> v2: make message about relaxed throttling an info
> rework reporting about current clk state
> 
> Signed-off-by: Karol Herbst 
> ---
>  drm/nouveau/include/nvkm/subdev/clk.h |  1 +
>  drm/nouveau/nvkm/subdev/clk/base.c| 44 
> +--
>  2 files changed, 33 insertions(+), 12 deletions(-)
> 
> diff --git a/drm/nouveau/include/nvkm/subdev/clk.h 
> b/drm/nouveau/include/nvkm/subdev/clk.h
> index f5ff1fd9..b79bf657 100644
> --- a/drm/nouveau/include/nvkm/subdev/clk.h
> +++ b/drm/nouveau/include/nvkm/subdev/clk.h
> @@ -106,6 +106,7 @@ struct nvkm_clk {
>   u8  temp;
>   u8  max_temp;
>   u8  relax_temp;
> + bool throttled;
>  
>   bool allow_reclock;
>  #define NVKM_CLK_BOOST_NONE 0x0
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c 
> b/drm/nouveau/nvkm/subdev/clk/base.c
> index 54e14936..72ca5e0c 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -279,7 +279,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid)
>   struct nvkm_fb *fb = subdev->device->fb;
>   struct nvkm_pci *pci = subdev->device->pci;
>   struct nvkm_pstate *pstate;
> - int ret;
> + int ret, cstate;
>  
>   if (pstateid == NVKM_CLK_PSTATE_DEFAULT)
>   return 0;
> @@ -308,7 +308,12 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid)
>   ram->func->tidy(ram);
>   }
>  
> - return nvkm_cstate_prog(clk, pstate, clk->exp_cstateid);
> + if (clk->throttled)
> + cstate = list_first_entry(>list, struct nvkm_cstate, 
> head)->id;
> + else
> + cstate = clk->exp_cstateid;
> +
> + return nvkm_cstate_prog(clk, pstate, cstate);
>  }
>  
>  static void
> @@ -322,22 +327,20 @@ 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 = NVKM_CLK_PSTATE_DEFAULT;
> - nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d C %d T %d°C\n",
> -pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
> -clk->astate, clk->exp_cstateid, clk->temp);
> -
>   pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;
>   if (clk->state_nr && pstate != -1) {
> - pstate = (pstate < 0) ? clk->astate : pstate;
> + if (clk->throttled)
> + pstate = list_first_entry(>states, struct 
> nvkm_pstate, head)->pstate;
> + else
> + pstate = (pstate < 0) ? clk->astate : pstate;
>   } else {
>   pstate = NVKM_CLK_PSTATE_DEFAULT;
>   }
>  
> - nvkm_trace(subdev, "-> %d\n", pstate);
> + nvkm_trace(subdev, "PWR %d U(AC) %d U(DC) %d A %d C %d T %d°C\n",
> +clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
> +clk->astate, clk->exp_cstateid, clk->temp);
> +
>   if (!clk->pstate || pstate != clk->pstate->pstate) {
>   int ret = nvkm_pstate_prog(clk, pstate);
>   if (ret) {
> @@ -564,9 +567,25 @@ nvkm_clk_astate(struct nvkm_clk *clk, int req, int rel, 
> bool wait)
>  int
>  nvkm_clk_tstate(struct nvkm_clk *clk, u8 temp)
>  {
> + struct nvkm_subdev *subdev = >subdev;
>   if (clk->temp == temp)
>   return 0;
>   clk->temp = temp;
> + if (clk->max_temp && clk->relax_temp) {
> + if (!clk->throttled && temp > clk->max_temp) {
> + nvkm_warn(subdev,
> +   "temperature (%d C) hit the 'downclock' "
> +   "threshold\n",
> +   temp);
> + clk->throttled = true;
> + } else if (clk->throttled && temp < clk->relax_temp) {
> + nvkm_info(subdev,
> +   "temperature (%d C) went below the "
> +   "'relax' threshold\n",
> +   temp);
> + clk->throttled = false;
> + }
> + }
>   return nvkm_clk_update(clk, false);
>  }
>  
> @@ -727,6 +746,7 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct 
> nvkm_device *device,
>   clk->ustate_dc = -1;
>   clk->exp_cstateid = NVKM_CLK_CSTATE_DEFAULT;
>   clk->temp = 90; /* reasonable default value */
> + clk->throttled = false;
>  
>   clk->allow_reclock = allow_reclock;
>  
> -- 
> 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

Re: [Nouveau] [RFC PATCH 15/29] bios: add thermal policies table

2017-10-08 Thread Pierre Moreau

Looking good. It would be nice to have some defines/enums for the different
modes. Some comments about t0, t1 and t2 would be nice. I saw you are using t0
in patch 16, but I have no idea why use t0 rather than t1 or t2.

Otherwise,
Acked-by: Pierre Moreau 

On 2017-09-15 — 17:11, Karol Herbst wrote:
> Signed-off-by: Karol Herbst 
> ---
>  .../include/nvkm/subdev/bios/thermal_policies.h| 27 
>  drm/nouveau/nvkm/subdev/bios/Kbuild|  1 +
>  drm/nouveau/nvkm/subdev/bios/thermal_policies.c| 81 
> ++
>  3 files changed, 109 insertions(+)
>  create mode 100644 drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h
>  create mode 100644 drm/nouveau/nvkm/subdev/bios/thermal_policies.c
> 
> diff --git a/drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h 
> b/drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h
> new file mode 100644
> index ..c9215fdd
> --- /dev/null
> +++ b/drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h
> @@ -0,0 +1,27 @@
> +#ifndef __NVBIOS_THERMAL_POLICIES_H__
> +#define __NVBIOS_THERMAL_POLICIES_H__
> +
> +struct nvbios_thermal_policies_header {
> + u32 offset;
> +
> + u8 version;
> + u8 hlen;
> + u8 ecount;
> + u8 elen;
> +};
> +struct nvbios_thermal_policies_entry {
> + u8  mode;
> + u16 t0;
> + u16 t1;
> + u16 t2;
> + s16 down_offset;
> + s16 up_offset;
> +};
> +
> +int nvbios_thermal_policies_parse(struct nvkm_bios *,
> +   struct nvbios_thermal_policies_header *);
> +int nvbios_thermal_policies_entry(struct nvkm_bios *,
> +   struct nvbios_thermal_policies_header *,
> +   u8 idx,
> +   struct nvbios_thermal_policies_entry *);
> +#endif
> diff --git a/drm/nouveau/nvkm/subdev/bios/Kbuild 
> b/drm/nouveau/nvkm/subdev/bios/Kbuild
> index 6b4f1e06..38f31dd0 100644
> --- a/drm/nouveau/nvkm/subdev/bios/Kbuild
> +++ b/drm/nouveau/nvkm/subdev/bios/Kbuild
> @@ -30,6 +30,7 @@ nvkm-y += nvkm/subdev/bios/shadowramin.o
>  nvkm-y += nvkm/subdev/bios/shadowrom.o
>  nvkm-y += nvkm/subdev/bios/timing.o
>  nvkm-y += nvkm/subdev/bios/therm.o
> +nvkm-y += nvkm/subdev/bios/thermal_policies.o
>  nvkm-y += nvkm/subdev/bios/vmap.o
>  nvkm-y += nvkm/subdev/bios/volt.o
>  nvkm-y += nvkm/subdev/bios/vpstate.o
> diff --git a/drm/nouveau/nvkm/subdev/bios/thermal_policies.c 
> b/drm/nouveau/nvkm/subdev/bios/thermal_policies.c
> new file mode 100644
> index ..5105194e
> --- /dev/null
> +++ b/drm/nouveau/nvkm/subdev/bios/thermal_policies.c
> @@ -0,0 +1,81 @@
> +/*
> + * Copyright 2017 Karol Herbst
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Authors: Karol Herbst
> + */
> +#include 
> +#include 
> +#include 
> +
> +static u32
> +nvbios_thermal_policies_offset(struct nvkm_bios *b)
> +{
> + struct bit_entry bit_P;
> +
> + if (!bit_entry(b, 'P', _P)) {
> + if (bit_P.version == 2 && bit_P.length >= 0x50)
> + return nvbios_rd32(b, bit_P.offset + 0x50);
> + }
> +
> + return 0;
> +}
> +
> +int
> +nvbios_thermal_policies_parse(struct nvkm_bios *b, struct 
> nvbios_thermal_policies_header *h)
> +{
> + if (!h)
> + return -EINVAL;
> +
> + h->offset = nvbios_thermal_policies_offset(b);
> + if (!h->offset)
> + return -ENODEV;
> +
> + h->version = nvbios_rd08(b, h->offset);
> + switch (h->version) {
> + case 0x10:
> + h->hlen = nvbios_rd08(b, h->offset + 0x1);
> + h->elen = nvbios_rd08(b, h->offset + 0x2);
> + h->ecount   = nvbios_rd08(b, h->offset + 0x3);
> + return 0;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +int
> +nvbios_thermal_policies_entry(struct nvkm_bios *b, struct 
>

Re: [Nouveau] [RFC PATCH 16/29] clk: parse thermal policies for throttling thresholds

2017-10-08 Thread Pierre Moreau

On 2017-09-15 — 17:11, Karol Herbst wrote:
> v2: use min_t
> 
> Signed-off-by: Karol Herbst 
> ---
>  drm/nouveau/include/nvkm/subdev/clk.h |  2 ++
>  drm/nouveau/nvkm/subdev/clk/base.c| 42 
> +++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/drm/nouveau/include/nvkm/subdev/clk.h 
> b/drm/nouveau/include/nvkm/subdev/clk.h
> index f35518c3..f5ff1fd9 100644
> --- a/drm/nouveau/include/nvkm/subdev/clk.h
> +++ b/drm/nouveau/include/nvkm/subdev/clk.h
> @@ -104,6 +104,8 @@ struct nvkm_clk {
>   struct nvkm_cstate *cstate;
>   int exp_cstateid;
>   u8  temp;
> + u8  max_temp;
> + u8  relax_temp;

I guess those should be int, similar to the changes you made in patch 1.

>  
>   bool allow_reclock;
>  #define NVKM_CLK_BOOST_NONE 0x0
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c 
> b/drm/nouveau/nvkm/subdev/clk/base.c
> index 54188d2b..54e14936 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -659,6 +660,44 @@ nvkm_clk = {
>   .fini = nvkm_clk_fini,
>  };
>  
> +static void
> +nvkm_clk_parse_max_temp(struct nvkm_clk *clk)
> +{
> + struct nvkm_subdev *subdev = >subdev;
> + struct nvkm_bios *bios = subdev->device->bios;
> + struct nvbios_thermal_policies_header header;
> + struct nvbios_thermal_policies_entry entry;
> + u8 i;
> + s16 mt = 0xff;
> + s16 rt = 0xff;

Why 0xff? I mean, sure, there is a high possibility that the GPU won’t like
temperatures above 255℃, but still.

> +
> + if (nvbios_thermal_policies_parse(bios, ))
> + return;
> +
> + if (!header.ecount)
> + return;
> +
> + for (i = 0; i < header.ecount; i++) {
> + if (nvbios_thermal_policies_entry(bios, , i, ))
> + return;
> +
> + if (entry.mode != 1)
> + continue;
> +
> + mt = min_t(s16, mt, (entry.t0 + entry.down_offset) / 32);
> + rt = min_t(s16, rt, (entry.t0 + entry.up_offset) / 32);
> + }
> +
> + if (mt == 0xff || rt == 0xff)
> + return;
> +
> + clk->max_temp = mt;
> + clk->relax_temp = rt;
> +
> + nvkm_debug(subdev, "setting up sw throttling thresholds (%u/%u°C)\n",
> +clk->max_temp, clk->relax_temp);
> +}
> +
>  int
>  nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device,
> int index, bool allow_reclock, struct nvkm_clk *clk)
> @@ -733,6 +772,9 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct 
> nvkm_device *device,
>  
>   clk->boost_mode = nvkm_longopt(device->cfgopt, "NvBoost",
>  NVKM_CLK_BOOST_NONE);
> +
> + nvkm_clk_parse_max_temp(clk);
> +
>   return 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

Re: [Nouveau] [RFC PATCH 14/29] therm: Trigger reclock in temperature daemon

2017-10-08 Thread Pierre Moreau

Reviewed-by: Pierre Moreau 

On 2017-09-15 — 17:11, Karol Herbst wrote:
> Depending on the temperature, cstates might become unreachable or the maped
> voltage of a cstate changes. We want to adjust to that.
> 
> Signed-off-by: Karol Herbst 
> Reviewed-by: Martin Peres 
> ---
>  drm/nouveau/nvkm/subdev/therm/base.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drm/nouveau/nvkm/subdev/therm/base.c 
> b/drm/nouveau/nvkm/subdev/therm/base.c
> index 59f01fec..c90b9cfc 100644
> --- a/drm/nouveau/nvkm/subdev/therm/base.c
> +++ b/drm/nouveau/nvkm/subdev/therm/base.c
> @@ -23,6 +23,8 @@
>   */
>  #include "priv.h"
>  
> +#include 
> +
>  int
>  nvkm_therm_temp_get(struct nvkm_therm *therm, int *val)
>  {
> @@ -168,9 +170,15 @@ nvkm_therm_alarm(struct nvkm_alarm *alarm)
>   int temp;
>   struct nvkm_therm *therm =
>  container_of(alarm, struct nvkm_therm, alarm);
> + struct nvkm_clk *clk = therm->subdev.device->clk;
> +
>   if (nvkm_therm_temp_get(therm, ) >= 0)
>   therm->last_temp = temp;
> +
>   nvkm_therm_update(therm, therm->last_temp, -1);
> +
> + if (clk)
> + nvkm_clk_tstate(clk, therm->last_temp);
>  }
>  
>  int
> -- 
> 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

Re: [Nouveau] [RFC PATCH 09/29] clk: Set clocks to pre suspend state after suspend

2017-10-08 Thread Pierre Moreau

Reviewed-by: Pierre Moreau 

On 2017-09-15 — 17:11, Karol Herbst wrote:
> The idea is to clear out the saved state, because after a resume we can't
> know what the GPU is clocked to. The reclock is triggered by the call to
> nvkm_clk_update later in nvkm_clk_init.
> 
> v2: convert to C style comments
> 
> Signed-off-by: Karol Herbst 
> Reviewed-by: Martin Peres 
> ---
>  drm/nouveau/nvkm/subdev/clk/base.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c 
> b/drm/nouveau/nvkm/subdev/clk/base.c
> index 1d71bf09..54188d2b 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -625,11 +625,10 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
>   if (clk->func->init)
>   return clk->func->init(clk);
>  
> - clk->astate = NVKM_CLK_PSTATE_DEFAULT;
> + /* after a resume we have no idea what clocks are set, reset the state
> +  */
>   clk->pstate = NULL;
> - clk->exp_cstateid = NVKM_CLK_CSTATE_DEFAULT;
>   clk->cstate = NULL;
> - clk->temp = 90; /* reasonable default value */
>   nvkm_clk_update(clk, true);
>   return 0;
>  }
> @@ -683,8 +682,13 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct 
> nvkm_device *device,
>   clk->func = func;
>   INIT_LIST_HEAD(>states);
>   clk->domains = func->domains;
> +
> + clk->astate = NVKM_CLK_PSTATE_DEFAULT;
>   clk->ustate_ac = -1;
>   clk->ustate_dc = -1;
> + clk->exp_cstateid = NVKM_CLK_CSTATE_DEFAULT;
> + clk->temp = 90; /* reasonable default value */
> +
>   clk->allow_reclock = allow_reclock;
>  
>   INIT_WORK(>work, nvkm_clk_update_work);
> -- 
> 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

Re: [Nouveau] [RFC PATCH 08/29] clk: We should pass the pstate id around not the index in the list

2017-10-08 Thread Karol Herbst

On Sun, Oct 8, 2017 at 4:23 PM, Pierre Moreau  wrote:
> On 2017-09-15 — 17:11, Karol Herbst wrote:
>> This makes the code easier, because we can compare the id with
>> pstate->pstate and saves us from the trouble of iterating over the pstates
>> to match the index.
>
> I don’t remember whether I have already done  this comment before, but I am 
> not
> sure where your iterating over the pstates savings are, as in this patch at
> least, you iterate as much as you did before.
>

yeah I think I ended up adding one loop again to check whether the
input was actual an existing pstate or not, but it shouldn't be needed
anymore. It was required before, now it's only there for error
checking.

> A few more comments below
>
>> v2: reword commit message
>>
>> Signed-off-by: Karol Herbst 
>> Reviewed-by: Martin Peres 
>> ---
>>  drm/nouveau/nouveau_debugfs.c  |  6 +--
>>  drm/nouveau/nvkm/subdev/clk/base.c | 78 
>> +++---
>>  2 files changed, 41 insertions(+), 43 deletions(-)
>>
>> diff --git a/drm/nouveau/nouveau_debugfs.c b/drm/nouveau/nouveau_debugfs.c
>> index 963a4dba..27281c4e 100644
>> --- a/drm/nouveau/nouveau_debugfs.c
>> +++ b/drm/nouveau/nouveau_debugfs.c
>> @@ -96,11 +96,11 @@ nouveau_debugfs_pstate_get(struct seq_file *m, void 
>> *data)
>>   } while (attr.index);
>>
>>   if (state >= 0) {
>> - if (info.ustate_ac == state)
>> + if (info.ustate_ac == attr.state)
>>   seq_printf(m, " AC");
>> - if (info.ustate_dc == state)
>> + if (info.ustate_dc == attr.state)
>>   seq_printf(m, " DC");
>> - if (info.pstate == state)
>> + if (info.pstate == attr.state)
>>   seq_printf(m, " *");
>>   } else {
>>   if (info.ustate_ac < -1)
>> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c 
>> b/drm/nouveau/nvkm/subdev/clk/base.c
>> index d37c13b7..1d71bf09 100644
>> --- a/drm/nouveau/nvkm/subdev/clk/base.c
>> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
>> @@ -272,23 +272,26 @@ nvkm_cstate_new(struct nvkm_clk *clk, int idx, struct 
>> nvkm_pstate *pstate)
>>   * P-States
>>   
>> */
>>  static int
>> -nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
>> +nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid)
>>  {
>>   struct nvkm_subdev *subdev = >subdev;
>>   struct nvkm_fb *fb = subdev->device->fb;
>>   struct nvkm_pci *pci = subdev->device->pci;
>>   struct nvkm_pstate *pstate;
>> - int ret, idx = 0;
>> + int ret;
>>
>> - if (pstatei == NVKM_CLK_PSTATE_DEFAULT)
>> + if (pstateid == NVKM_CLK_PSTATE_DEFAULT)
>>   return 0;
>>
>>   list_for_each_entry(pstate, >states, head) {
>> - if (idx++ == pstatei)
>> + if (pstate->pstate == pstateid)
>>   break;
>>   }
>>
>> - nvkm_debug(subdev, "setting performance state %d\n", pstatei);
>> + if (!pstate)
>> + return -EINVAL;
>> +
>> + nvkm_debug(subdev, "setting performance state %x\n", pstateid);
>>   clk->pstate = pstate;
>>
>>   nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width);
>> @@ -329,7 +332,6 @@ nvkm_clk_update_work(struct work_struct *work)
>>   pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;
>>   if (clk->state_nr && pstate != -1) {
>>   pstate = (pstate < 0) ? clk->astate : pstate;
>> - pstate = min(pstate, clk->state_nr - 1);
>>   } else {
>>   pstate = NVKM_CLK_PSTATE_DEFAULT;
>>   }
>> @@ -490,33 +492,10 @@ nvkm_pstate_new(struct nvkm_clk *clk, int idx)
>>  
>> /**
>>   * Adjustment triggers
>>   
>> */
>> -static int
>> -nvkm_clk_ustate_update(struct nvkm_clk *clk, int req)
>> -{
>> - struct nvkm_pstate *pstate;
>> - int i = 0;
>> -
>> - if (!clk->allow_reclock)
>> - return -ENOSYS;
>> -
>> - if (req != -1 && req != -2) {
>> - list_for_each_entry(pstate, >states, head) {
>> - if (pstate->pstate == req)
>> - break;
>> - i++;
>> - }
>> -
>> - if (pstate->pstate != req)
>> - return -EINVAL;
>> - req = i;
>> - }
>> -
>> - return req + 2;
>> -}
>> -
>>  static int
>>  nvkm_clk_nstate(struct nvkm_clk *clk, const char *mode, int arglen)
>>  {
>> + struct nvkm_pstate *pstate;
>>   int ret = 1;
>>
>>   if (clk->allow_reclock && !strncasecmpz(mode, "auto", arglen))
>> @@ -528,27 +507,46 @@ nvkm_clk_nstate(struct nvkm_clk

Re: [Nouveau] [RFC PATCH 08/29] clk: We should pass the pstate id around not the index in the list

2017-10-08 Thread Pierre Moreau

On 2017-09-15 — 17:11, Karol Herbst wrote:
> This makes the code easier, because we can compare the id with
> pstate->pstate and saves us from the trouble of iterating over the pstates
> to match the index.

I don’t remember whether I have already done  this comment before, but I am not
sure where your iterating over the pstates savings are, as in this patch at
least, you iterate as much as you did before.

A few more comments below

> v2: reword commit message
> 
> Signed-off-by: Karol Herbst 
> Reviewed-by: Martin Peres 
> ---
>  drm/nouveau/nouveau_debugfs.c  |  6 +--
>  drm/nouveau/nvkm/subdev/clk/base.c | 78 
> +++---
>  2 files changed, 41 insertions(+), 43 deletions(-)
> 
> diff --git a/drm/nouveau/nouveau_debugfs.c b/drm/nouveau/nouveau_debugfs.c
> index 963a4dba..27281c4e 100644
> --- a/drm/nouveau/nouveau_debugfs.c
> +++ b/drm/nouveau/nouveau_debugfs.c
> @@ -96,11 +96,11 @@ nouveau_debugfs_pstate_get(struct seq_file *m, void *data)
>   } while (attr.index);
>  
>   if (state >= 0) {
> - if (info.ustate_ac == state)
> + if (info.ustate_ac == attr.state)
>   seq_printf(m, " AC");
> - if (info.ustate_dc == state)
> + if (info.ustate_dc == attr.state)
>   seq_printf(m, " DC");
> - if (info.pstate == state)
> + if (info.pstate == attr.state)
>   seq_printf(m, " *");
>   } else {
>   if (info.ustate_ac < -1)
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c 
> b/drm/nouveau/nvkm/subdev/clk/base.c
> index d37c13b7..1d71bf09 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -272,23 +272,26 @@ nvkm_cstate_new(struct nvkm_clk *clk, int idx, struct 
> nvkm_pstate *pstate)
>   * P-States
>   
> */
>  static int
> -nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
> +nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid)
>  {
>   struct nvkm_subdev *subdev = >subdev;
>   struct nvkm_fb *fb = subdev->device->fb;
>   struct nvkm_pci *pci = subdev->device->pci;
>   struct nvkm_pstate *pstate;
> - int ret, idx = 0;
> + int ret;
>  
> - if (pstatei == NVKM_CLK_PSTATE_DEFAULT)
> + if (pstateid == NVKM_CLK_PSTATE_DEFAULT)
>   return 0;
>  
>   list_for_each_entry(pstate, >states, head) {
> - if (idx++ == pstatei)
> + if (pstate->pstate == pstateid)
>   break;
>   }
>  
> - nvkm_debug(subdev, "setting performance state %d\n", pstatei);
> + if (!pstate)
> + return -EINVAL;
> +
> + nvkm_debug(subdev, "setting performance state %x\n", pstateid);
>   clk->pstate = pstate;
>  
>   nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width);
> @@ -329,7 +332,6 @@ nvkm_clk_update_work(struct work_struct *work)
>   pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;
>   if (clk->state_nr && pstate != -1) {
>   pstate = (pstate < 0) ? clk->astate : pstate;
> - pstate = min(pstate, clk->state_nr - 1);
>   } else {
>   pstate = NVKM_CLK_PSTATE_DEFAULT;
>   }
> @@ -490,33 +492,10 @@ nvkm_pstate_new(struct nvkm_clk *clk, int idx)
>  
> /**
>   * Adjustment triggers
>   
> */
> -static int
> -nvkm_clk_ustate_update(struct nvkm_clk *clk, int req)
> -{
> - struct nvkm_pstate *pstate;
> - int i = 0;
> -
> - if (!clk->allow_reclock)
> - return -ENOSYS;
> -
> - if (req != -1 && req != -2) {
> - list_for_each_entry(pstate, >states, head) {
> - if (pstate->pstate == req)
> - break;
> - i++;
> - }
> -
> - if (pstate->pstate != req)
> - return -EINVAL;
> - req = i;
> - }
> -
> - return req + 2;
> -}
> -
>  static int
>  nvkm_clk_nstate(struct nvkm_clk *clk, const char *mode, int arglen)
>  {
> + struct nvkm_pstate *pstate;
>   int ret = 1;
>  
>   if (clk->allow_reclock && !strncasecmpz(mode, "auto", arglen))
> @@ -528,27 +507,46 @@ nvkm_clk_nstate(struct nvkm_clk *clk, const char *mode, 
> int arglen)
>  
>   ((char *)mode)[arglen] = '\0';
>   if (!kstrtol(mode, 0, )) {
> - ret = nvkm_clk_ustate_update(clk, v);
> + ret = v;
>   if (ret < 0)
>   ret = 1;
>   }
>   ((char *)mode)[arglen] = save;
>   }
>  
> - return ret - 2;
> +

Re: [Nouveau] [RFC PATCH 06/29] clk: Make pstate a pointer to nvkm_pstate

2017-10-08 Thread Karol Herbst

On Sun, Oct 8, 2017 at 1:47 PM, Pierre Moreau  wrote:
> The patch seems fine but I found it super confusing that sometimes `pstate` is
> a pointer (for example `clk->pstate`), sometimes it is an int (for example
> `args->v0.pstate`).
>

yeah I still plan to clean the names up when I find some time to
finish those patches for real

> On 2017-09-15 — 17:11, Karol Herbst wrote:
>> We will access the current cstate at least every second and this saves us
>> some CPU cycles looking them up every second.
>>
>> v2: Rewording commit message.
>>
>> Signed-off-by: Karol Herbst 
>> Reviewed-by: Martin Peres 
>> ---
>>  drm/nouveau/include/nvkm/subdev/clk.h |  4 +++-
>>  drm/nouveau/nvkm/engine/device/ctrl.c |  5 -
>>  drm/nouveau/nvkm/subdev/clk/base.c| 17 -
>>  drm/nouveau/nvkm/subdev/pmu/gk20a.c   | 18 +++---
>>  4 files changed, 26 insertions(+), 18 deletions(-)
>>
>> diff --git a/drm/nouveau/include/nvkm/subdev/clk.h 
>> b/drm/nouveau/include/nvkm/subdev/clk.h
>> index 1340f5b8..ec537e08 100644
>> --- a/drm/nouveau/include/nvkm/subdev/clk.h
>> +++ b/drm/nouveau/include/nvkm/subdev/clk.h
>> @@ -10,6 +10,8 @@ struct nvkm_pll_vals;
>>  #define NVKM_CLK_CSTATE_BASE-2 /* pstate base */
>>  #define NVKM_CLK_CSTATE_HIGHEST -3 /* highest possible */
>>
>> +#define NVKM_CLK_PSTATE_DEFAULT -1
>> +
>>  enum nv_clk_src {
>>   nv_clk_src_crystal,
>>   nv_clk_src_href,
>> @@ -95,7 +97,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 b0ece71a..da70626c 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 = NVKM_CLK_PSTATE_DEFAULT;
>>   } 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 07d530ed..0d4d9fdf 100644
>> --- a/drm/nouveau/nvkm/subdev/clk/base.c
>> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
>> @@ -271,13 +271,16 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
>>   struct nvkm_pstate *pstate;
>>   int ret, idx = 0;
>>
>> + if (pstatei == NVKM_CLK_PSTATE_DEFAULT)
>> + return 0;
>> +
>>   list_for_each_entry(pstate, >states, head) {
>>   if (idx++ == pstatei)
>>   break;
>>   }
>>
>>   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);
>>
>> @@ -306,8 +309,12 @@ 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 = NVKM_CLK_PSTATE_DEFAULT;
>>   nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d T %d°C\n",
>> -clk->pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
>> +pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
>>  clk->astate, clk->temp);
>>
>>   pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;
>
> In the if-statement following the previous line, the -1 could be replaced by
> NVKM_CLK_PSTATE_DEFAULT.
>
>> @@ -315,11 +322,11 @@ nvkm_clk_update_work(struct work_struct *work)
>>   pstate = (pstate < 0) ? clk->astate : pstate;
>
> Can `pstate` have negative values other than -1?

not yet, but >=0 means a pstate is selected by an id, <0 but a type,
but it might be that we'll never need more than the default one.

>
>>   pstate = min(pstate, clk->state_nr - 1);
>>   } else {
>> - pstate = clk->pstate = -1;
>> + pstate = NVKM_CLK_PSTATE_DEFAULT;
>
> Shouldn’t you also reset `clk->pstate` to NULL? Or maybe it is
> `nvkm_pstate_prog()` which should do it if
> `pstatei == NVKM_CLK_PSTATE_DEFAULT`.
>

no. We can't go back to the default clocks except we run devinit
again.

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

2017-10-08 Thread Karol Herbst

On Sun, Oct 8, 2017 at 12:41 PM, Pierre Moreau  wrote:
> 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 
>> ---
>>  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 = >subdev;
>> @@ -332,9 +332,15 @@ nvkm_pstate_work(struct work_struct *work)
>>   nvkm_notify_get(>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(>waiting, 1);
>>   schedule_work(>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?
>

it doesn't matter here, because it shouldn't change the return value
of that function.

>>  }
>>
>> @@ -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?

I don't want nvkm_clk_init to fail just because we don't allow
reclocking (which is basically the only reason it may return !0 here)

>
>>  }
>>
>> @@ -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(>work, nvkm_pstate_work);
>> +

Re: [Nouveau] [RFC PATCH 07/29] clk: Hold information about the current cstate status

2017-10-08 Thread Pierre Moreau

Acked-by: Pierre Moreau 

On 2017-09-15 — 17:11, Karol Herbst wrote:
> Later we will have situations where the expected and the current state
> isn't the same.
> 
> Signed-off-by: Karol Herbst 
> Reviewed-by: Martin Peres 
> ---
>  drm/nouveau/include/nvkm/subdev/clk.h |  2 ++
>  drm/nouveau/nvkm/subdev/clk/base.c| 32 +---
>  2 files changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drm/nouveau/include/nvkm/subdev/clk.h 
> b/drm/nouveau/include/nvkm/subdev/clk.h
> index ec537e08..f35518c3 100644
> --- a/drm/nouveau/include/nvkm/subdev/clk.h
> +++ b/drm/nouveau/include/nvkm/subdev/clk.h
> @@ -101,6 +101,8 @@ struct nvkm_clk {
>   int ustate_ac; /* user-requested (-1 disabled, -2 perfmon) */
>   int ustate_dc; /* user-requested (-1 disabled, -2 perfmon) */
>   int astate; /* perfmon adjustment (base) */
> + struct nvkm_cstate *cstate;
> + int exp_cstateid;
>   u8  temp;
>  
>   bool allow_reclock;
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c 
> b/drm/nouveau/nvkm/subdev/clk/base.c
> index 0d4d9fdf..d37c13b7 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -146,9 +146,14 @@ static struct nvkm_cstate *
>  nvkm_cstate_get(struct nvkm_clk *clk, struct nvkm_pstate *pstate, int 
> cstatei)
>  {
>   struct nvkm_cstate *cstate;
> - if (cstatei == NVKM_CLK_CSTATE_HIGHEST)
> + switch (cstatei) {
> + case NVKM_CLK_CSTATE_HIGHEST:
>   return list_last_entry(>list, typeof(*cstate), head);
> - else {
> + case NVKM_CLK_CSTATE_BASE:
> + return >base;
> + case NVKM_CLK_CSTATE_DEFAULT:
> + return NULL;
> + default:
>   list_for_each_entry(cstate, >list, head) {
>   if (cstate->id == cstatei)
>   return cstate;
> @@ -167,6 +172,9 @@ nvkm_cstate_prog(struct nvkm_clk *clk, struct nvkm_pstate 
> *pstate, int cstatei)
>   struct nvkm_cstate *cstate;
>   int ret;
>  
> + if (cstatei == NVKM_CLK_CSTATE_DEFAULT)
> + return 0;
> +
>   if (!list_empty(>list)) {
>   cstate = nvkm_cstate_get(clk, pstate, cstatei);
>   cstate = nvkm_cstate_find_best(clk, pstate, cstate);
> @@ -193,6 +201,7 @@ nvkm_cstate_prog(struct nvkm_clk *clk, struct nvkm_pstate 
> *pstate, int cstatei)
>  
>   ret = clk->func->calc(clk, cstate);
>   if (ret == 0) {
> + clk->cstate = cstate;
>   ret = clk->func->prog(clk);
>   clk->func->tidy(clk);
>   }
> @@ -295,7 +304,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
>   ram->func->tidy(ram);
>   }
>  
> - return nvkm_cstate_prog(clk, pstate, NVKM_CLK_CSTATE_HIGHEST);
> + return nvkm_cstate_prog(clk, pstate, clk->exp_cstateid);
>  }
>  
>  static void
> @@ -313,9 +322,9 @@ nvkm_clk_update_work(struct work_struct *work)
>   pstate = clk->pstate->pstate;
>   else
>   pstate = NVKM_CLK_PSTATE_DEFAULT;
> - nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d T %d°C\n",
> + nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d C %d T %d°C\n",
>  pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
> -clk->astate, clk->temp);
> +clk->astate, clk->exp_cstateid, clk->temp);
>  
>   pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;
>   if (clk->state_nr && pstate != -1) {
> @@ -536,6 +545,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;
> + clk->exp_cstateid = NVKM_CLK_CSTATE_HIGHEST;
>   return nvkm_clk_update(clk, true);
>   }
>   return ret;
> @@ -548,6 +558,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);
> + clk->exp_cstateid = NVKM_CLK_CSTATE_BASE;
>   return nvkm_clk_update(clk, wait);
>  }
>  
> @@ -618,6 +629,8 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
>  
>   clk->astate = clk->state_nr - 1;
>   clk->pstate = NULL;
> + clk->exp_cstateid = NVKM_CLK_CSTATE_DEFAULT;
> + clk->cstate = NULL;
>   clk->temp = 90; /* reasonable default value */
>   nvkm_clk_update(clk, true);
>   return 0;
> @@ -701,15 +714,20 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct 
> nvkm_device *device,
>   if (mode) {
>   clk->ustate_ac = nvkm_clk_nstate(clk, mode, arglen);
>   clk->ustate_dc = nvkm_clk_nstate(clk, mode, arglen);
> + clk->exp_cstateid = NVKM_CLK_CSTATE_HIGHEST;
>   }
>  
>   mode = nvkm_stropt(device->cfgopt, "NvClkModeAC", );
> - if (mode)
> +

Re: [Nouveau] [RFC PATCH 06/29] clk: Make pstate a pointer to nvkm_pstate

2017-10-08 Thread Pierre Moreau

The patch seems fine but I found it super confusing that sometimes `pstate` is
a pointer (for example `clk->pstate`), sometimes it is an int (for example
`args->v0.pstate`).

On 2017-09-15 — 17:11, Karol Herbst wrote:
> We will access the current cstate at least every second and this saves us
> some CPU cycles looking them up every second.
> 
> v2: Rewording commit message.
> 
> Signed-off-by: Karol Herbst 
> Reviewed-by: Martin Peres 
> ---
>  drm/nouveau/include/nvkm/subdev/clk.h |  4 +++-
>  drm/nouveau/nvkm/engine/device/ctrl.c |  5 -
>  drm/nouveau/nvkm/subdev/clk/base.c| 17 -
>  drm/nouveau/nvkm/subdev/pmu/gk20a.c   | 18 +++---
>  4 files changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/drm/nouveau/include/nvkm/subdev/clk.h 
> b/drm/nouveau/include/nvkm/subdev/clk.h
> index 1340f5b8..ec537e08 100644
> --- a/drm/nouveau/include/nvkm/subdev/clk.h
> +++ b/drm/nouveau/include/nvkm/subdev/clk.h
> @@ -10,6 +10,8 @@ struct nvkm_pll_vals;
>  #define NVKM_CLK_CSTATE_BASE-2 /* pstate base */
>  #define NVKM_CLK_CSTATE_HIGHEST -3 /* highest possible */
>  
> +#define NVKM_CLK_PSTATE_DEFAULT -1
> +
>  enum nv_clk_src {
>   nv_clk_src_crystal,
>   nv_clk_src_href,
> @@ -95,7 +97,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 b0ece71a..da70626c 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 = NVKM_CLK_PSTATE_DEFAULT;
>   } 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 07d530ed..0d4d9fdf 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -271,13 +271,16 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
>   struct nvkm_pstate *pstate;
>   int ret, idx = 0;
>  
> + if (pstatei == NVKM_CLK_PSTATE_DEFAULT)
> + return 0;
> +
>   list_for_each_entry(pstate, >states, head) {
>   if (idx++ == pstatei)
>   break;
>   }
>  
>   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);
>  
> @@ -306,8 +309,12 @@ 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 = NVKM_CLK_PSTATE_DEFAULT;
>   nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d T %d°C\n",
> -clk->pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
> +pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
>  clk->astate, clk->temp);
>  
>   pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;

In the if-statement following the previous line, the -1 could be replaced by
NVKM_CLK_PSTATE_DEFAULT.

> @@ -315,11 +322,11 @@ nvkm_clk_update_work(struct work_struct *work)
>   pstate = (pstate < 0) ? clk->astate : pstate;

Can `pstate` have negative values other than -1?

>   pstate = min(pstate, clk->state_nr - 1);
>   } else {
> - pstate = clk->pstate = -1;
> + pstate = NVKM_CLK_PSTATE_DEFAULT;

Shouldn’t you also reset `clk->pstate` to NULL? Or maybe it is
`nvkm_pstate_prog()` which should do it if
`pstatei == NVKM_CLK_PSTATE_DEFAULT`.

>   }
>  
>   nvkm_trace(subdev, "-> %d\n", pstate);
> - if (pstate != clk->pstate) {
> + if (!clk->pstate || pstate != clk->pstate->pstate) {
>   int ret = nvkm_pstate_prog(clk, pstate);
>   if (ret) {
>   nvkm_error(subdev, "error setting pstate %d: %d\n",
> @@ -610,7 +617,7 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
>   return clk->func->init(clk);
>  
>   clk->astate = clk->state_nr - 1;
> - clk->pstate

Re: [Nouveau] [RFC PATCH 05/29] clk: Remove dstate

2017-10-08 Thread Pierre Moreau

This patch could be applied before patch 04, that way patch 04 does not need to
update a function (`nvkm_clk_dstate()`) that will be removed in the next patch.

Reviewed-by: Pierre Moreau 

On 2017-09-15 — 17:11, Karol Herbst wrote:
> We won't need it now, because we will adjust the clocks depending on engine
> loads later on anyway or a static lookup table. It also simplifies the
> clocking logic.
> 
> This code was nowhere used anyway and just a mock up.
> 
> v2: fixed typo in commit message
> 
> Signed-off-by: Karol Herbst 
> Reviewed-by: Martin Peres 
> ---
>  drm/nouveau/include/nvkm/subdev/clk.h |  2 --
>  drm/nouveau/nvkm/subdev/clk/base.c| 16 ++--
>  2 files changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/drm/nouveau/include/nvkm/subdev/clk.h 
> b/drm/nouveau/include/nvkm/subdev/clk.h
> index ce3bbcfe..1340f5b8 100644
> --- a/drm/nouveau/include/nvkm/subdev/clk.h
> +++ b/drm/nouveau/include/nvkm/subdev/clk.h
> @@ -99,7 +99,6 @@ struct nvkm_clk {
>   int ustate_ac; /* user-requested (-1 disabled, -2 perfmon) */
>   int ustate_dc; /* user-requested (-1 disabled, -2 perfmon) */
>   int astate; /* perfmon adjustment (base) */
> - int dstate; /* display adjustment (min+) */
>   u8  temp;
>  
>   bool allow_reclock;
> @@ -121,7 +120,6 @@ struct nvkm_clk {
>  int nvkm_clk_read(struct nvkm_clk *, enum nv_clk_src);
>  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);
>  
> diff --git a/drm/nouveau/nvkm/subdev/clk/base.c 
> b/drm/nouveau/nvkm/subdev/clk/base.c
> index ecff3ff3..07d530ed 100644
> --- a/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -306,15 +306,14 @@ nvkm_clk_update_work(struct work_struct *work)
>   return;
>   clk->pwrsrc = power_supply_is_system_supplied();
>  
> - nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d T %d°C D %d\n",
> + nvkm_trace(subdev, "P %d PWR %d U(AC) %d U(DC) %d A %d T %d°C\n",
>  clk->pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc,
> -clk->astate, clk->temp, clk->dstate);
> +clk->astate, clk->temp);
>  
>   pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc;
>   if (clk->state_nr && pstate != -1) {
>   pstate = (pstate < 0) ? clk->astate : pstate;
>   pstate = min(pstate, clk->state_nr - 1);
> - pstate = max(pstate, clk->dstate);
>   } else {
>   pstate = clk->pstate = -1;
>   }
> @@ -554,16 +553,6 @@ nvkm_clk_tstate(struct nvkm_clk *clk, u8 temp)
>   return nvkm_clk_update(clk, false);
>  }
>  
> -int
> -nvkm_clk_dstate(struct nvkm_clk *clk, int req, int rel)
> -{
> - if (!rel) clk->dstate  = req;
> - if ( rel) clk->dstate += rel;
> - clk->dstate = min(clk->dstate, clk->state_nr - 1);
> - clk->dstate = max(clk->dstate, 0);
> - return nvkm_clk_update(clk, true);
> -}
> -
>  static int
>  nvkm_clk_pwrsrc(struct nvkm_notify *notify)
>  {
> @@ -621,7 +610,6 @@ nvkm_clk_init(struct nvkm_subdev *subdev)
>   return clk->func->init(clk);
>  
>   clk->astate = clk->state_nr - 1;
> - clk->dstate = 0;
>   clk->pstate = -1;
>   clk->temp = 90; /* reasonable default value */
>   nvkm_clk_update(clk, true);
> -- 
> 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

Re: [Nouveau] [RFC PATCH 03/29] subdev/volt/gk104: return error when read fails

2017-10-08 Thread Karol Herbst

On Sun, Oct 8, 2017 at 12:29 PM, Pierre Moreau  wrote:
> On 2017-09-15 — 17:11, Karol Herbst wrote:
>> While my gpu was powered off, hwmon returned 0.6V as the current voltage.
>> If nvkm_rd32 fails for any reason, return the error.
>>
>> With that sensors will display a "N/A" instead of 0.6V.
>
> Small nitpick, add a comma between “that” and “sensors”.
> Otherwise,
>
> Reviewed-by: Pierre Moreau 
>

I think in the end I am not _that_ happy with that change. nvkm_rd32
can't return an error code, but as we interpret the full register as
the value, values above 0x7fff are most likely an error on either
our side or the hardware side. When the GPU is suspended we get
0x out, but this code shouldn't be called in the first place
when the GPU is suspended. We can't really trust runpm as well due to
race conditions, waking up the GPU is another terrible idea.

>>
>> Signed-off-by: Karol Herbst 
>> ---
>>  drm/nouveau/nvkm/subdev/volt/gk104.c | 7 ++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drm/nouveau/nvkm/subdev/volt/gk104.c 
>> b/drm/nouveau/nvkm/subdev/volt/gk104.c
>> index 1c744e02..53a7af9d 100644
>> --- a/drm/nouveau/nvkm/subdev/volt/gk104.c
>> +++ b/drm/nouveau/nvkm/subdev/volt/gk104.c
>> @@ -40,10 +40,15 @@ gk104_volt_get(struct nvkm_volt *base)
>>  {
>>   struct nvbios_volt *bios = _volt(base)->bios;
>>   struct nvkm_device *device = base->subdev.device;
>> - u32 div, duty;
>> + int div, duty;
>>
>>   div  = nvkm_rd32(device, 0x20340);
>> + if (div < 0)
>> + return div;
>> +
>>   duty = nvkm_rd32(device, 0x20344);
>> + if (duty < 0)
>> + return duty;
>>
>>   return bios->base + bios->pwm_range * duty / div;
>>  }
>> --
>> 2.14.1
>>
>> ___
>> Nouveau mailing list
>> Nouveau@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

2017-10-08 Thread 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 
> ---
>  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 = >subdev;
> @@ -332,9 +332,15 @@ nvkm_pstate_work(struct work_struct *work)
>   nvkm_notify_get(>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(>waiting, 1);
>   schedule_work(>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(>work, nvkm_pstate_work);
> + INIT_WORK(>work, nvkm_clk_update_work);
>   init_waitqueue_head(>wait);
>   atomic_set(>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

Re: [Nouveau] [RFC PATCH 01/29] therm: split return code and value in nvkm_get_temp

2017-10-08 Thread Karol Herbst

On Sun, Oct 8, 2017 at 12:16 PM, Pierre Moreau  wrote:
> As you changed the return value of `temp_get()` to solely be the error code, 
> or
> absence of an error, I would change all those tests that checked whether the
> returned value was strictly less, or greater than, 0 to now only compare
> against 0 (no error). For example,
>
>   if (therm && therm->attr_get && nvkm_therm_temp_get(therm, ) < 0)
>   if (therm->func->temp_get(therm, ) >= 0)
>
> could become
>
>   if (therm && therm->attr_get && nvkm_therm_temp_get(therm, ))
>   if (!therm->func->temp_get(therm, ))
>

right, makes sense.

> to match other error checking code.
>
> More comments below
>
> On 2017-09-15 — 17:11, Karol Herbst wrote:
>> The current hwmon code doesn't check if the returned value was actually an
>> error.
>>
>> Since Kepler temperature sensors are able to report negative values.
>
> I assume those negative values are not for error reporting, but more if you
> buried your GPU in the snow somewhere in the Antarctic and still want a valid
> temperature to be reported.

well, nobody ever tested that, but at least on Kepler technically we
have two new registers related to therm, one offset and one value. If
the offset is 0x60, value needs to be substracted by that offset to
get the real temperature. No idea if that works for real though, but
why would they add it anyhow if not?

>
>> Since Pascal (and maybe earlier) we have sensors with improved precision.
>>
>> Adjust the nvkm_get_temp method to be able to deal with those changes
>> and let hwmon return an error properly.
>>
>> Signed-off-by: Karol Herbst 
>> ---
>>  drm/nouveau/include/nvkm/subdev/therm.h |  2 +-
>>  drm/nouveau/nouveau_hwmon.c | 15 +--
>>  drm/nouveau/nvkm/subdev/therm/base.c| 19 ++-
>>  drm/nouveau/nvkm/subdev/therm/g84.c | 11 ++-
>>  drm/nouveau/nvkm/subdev/therm/gp100.c   |  9 +
>>  drm/nouveau/nvkm/subdev/therm/nv40.c|  9 +++--
>>  drm/nouveau/nvkm/subdev/therm/nv50.c|  9 +++--
>>  drm/nouveau/nvkm/subdev/therm/priv.h|  4 ++--
>>  drm/nouveau/nvkm/subdev/therm/temp.c| 16 
>>  9 files changed, 55 insertions(+), 39 deletions(-)
>>
>> diff --git a/drm/nouveau/include/nvkm/subdev/therm.h 
>> b/drm/nouveau/include/nvkm/subdev/therm.h
>> index 9841f076..8c84017f 100644
>> --- a/drm/nouveau/include/nvkm/subdev/therm.h
>> +++ b/drm/nouveau/include/nvkm/subdev/therm.h
>> @@ -86,7 +86,7 @@ struct nvkm_therm {
>>   int (*attr_set)(struct nvkm_therm *, enum nvkm_therm_attr_type, int);
>>  };
>>
>> -int nvkm_therm_temp_get(struct nvkm_therm *);
>> +int nvkm_therm_temp_get(struct nvkm_therm *, int *);
>>  int nvkm_therm_fan_sense(struct nvkm_therm *);
>>  int nvkm_therm_cstate(struct nvkm_therm *, int, int);
>>
>> diff --git a/drm/nouveau/nouveau_hwmon.c b/drm/nouveau/nouveau_hwmon.c
>> index 7c965648..f1914d9a 100644
>> --- a/drm/nouveau/nouveau_hwmon.c
>> +++ b/drm/nouveau/nouveau_hwmon.c
>> @@ -326,8 +326,9 @@ nouveau_temp_is_visible(const void *data, u32 attr, int 
>> channel)
>>  {
>>   struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
>>   struct nvkm_therm *therm = nvxx_therm(>client.device);
>> + int val;
>>
>> - if (therm && therm->attr_get && nvkm_therm_temp_get(therm) < 0)
>> + if (therm && therm->attr_get && nvkm_therm_temp_get(therm, ) < 0)
>>   return 0;
>>
>>   switch (attr) {
>> @@ -421,15 +422,16 @@ nouveau_temp_read(struct device *dev, u32 attr, int 
>> channel, long *val)
>>   struct drm_device *drm_dev = dev_get_drvdata(dev);
>>   struct nouveau_drm *drm = nouveau_drm(drm_dev);
>>   struct nvkm_therm *therm = nvxx_therm(>client.device);
>> - int ret;
>> + int ret = 0;
>> + int temp;
>>
>>   if (!therm || !therm->attr_get)
>>   return -EOPNOTSUPP;
>>
>>   switch (attr) {
>>   case hwmon_temp_input:
>> - ret = nvkm_therm_temp_get(therm);
>> - *val = ret < 0 ? ret : (ret * 1000);
>> + ret = nvkm_therm_temp_get(therm, );
>> + *val = temp * 1000;
>>   break;
>>   case hwmon_temp_max:
>>   *val = therm->attr_get(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK)
>> @@ -459,7 +461,7 @@ nouveau_temp_read(struct device *dev, u32 attr, int 
>> channel, long *val)
>>   return -EOPNOTSUPP;
>>   }
>>
>> - return 0;
>> + return ret;
>>  }
>>
>>  static int
>> @@ -713,6 +715,7 @@ nouveau_hwmon_init(struct drm_device *dev)
>>   struct device *hwmon_dev;
>>   int ret = 0;
>>   int i = 0;
>> + int val;
>>
>>   hwmon = drm->hwmon = kzalloc(sizeof(*hwmon), GFP_KERNEL);
>>   if (!hwmon)
>> @@ -720,7 +723,7 @@ nouveau_hwmon_init(struct drm_device *dev)
>>   hwmon->dev = dev;
>>
>>   if (therm && therm->attr_get && therm->attr_set) {
>> - if (nvkm_therm_temp_get(therm) >= 0)
>> + if

Re: [Nouveau] [RFC PATCH 03/29] subdev/volt/gk104: return error when read fails

2017-10-08 Thread Pierre Moreau

On 2017-09-15 — 17:11, Karol Herbst wrote:
> While my gpu was powered off, hwmon returned 0.6V as the current voltage.
> If nvkm_rd32 fails for any reason, return the error.
> 
> With that sensors will display a "N/A" instead of 0.6V.

Small nitpick, add a comma between “that” and “sensors”.
Otherwise,

Reviewed-by: Pierre Moreau 

> 
> Signed-off-by: Karol Herbst 
> ---
>  drm/nouveau/nvkm/subdev/volt/gk104.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drm/nouveau/nvkm/subdev/volt/gk104.c 
> b/drm/nouveau/nvkm/subdev/volt/gk104.c
> index 1c744e02..53a7af9d 100644
> --- a/drm/nouveau/nvkm/subdev/volt/gk104.c
> +++ b/drm/nouveau/nvkm/subdev/volt/gk104.c
> @@ -40,10 +40,15 @@ gk104_volt_get(struct nvkm_volt *base)
>  {
>   struct nvbios_volt *bios = _volt(base)->bios;
>   struct nvkm_device *device = base->subdev.device;
> - u32 div, duty;
> + int div, duty;
>  
>   div  = nvkm_rd32(device, 0x20340);
> + if (div < 0)
> + return div;
> +
>   duty = nvkm_rd32(device, 0x20344);
> + if (duty < 0)
> + return duty;
>  
>   return bios->base + bios->pwm_range * duty / div;
>  }
> -- 
> 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

Re: [Nouveau] [RFC PATCH 02/29] hwmon: properly check for errors

2017-10-08 Thread Pierre Moreau

Reviewed-by: Pierre Moreau 

On 2017-09-15 — 17:11, Karol Herbst wrote:
> Otherwise hwmon interprets error codes as real values.
> 
> Signed-off-by: Karol Herbst 
> ---
>  drm/nouveau/nouveau_hwmon.c | 33 ++---
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drm/nouveau/nouveau_hwmon.c b/drm/nouveau/nouveau_hwmon.c
> index f1914d9a..0d75c14d 100644
> --- a/drm/nouveau/nouveau_hwmon.c
> +++ b/drm/nouveau/nouveau_hwmon.c
> @@ -470,18 +470,23 @@ nouveau_fan_read(struct device *dev, u32 attr, int 
> channel, long *val)
>   struct drm_device *drm_dev = dev_get_drvdata(dev);
>   struct nouveau_drm *drm = nouveau_drm(drm_dev);
>   struct nvkm_therm *therm = nvxx_therm(>client.device);
> + int ret;
>  
>   if (!therm)
>   return -EOPNOTSUPP;
>  
>   switch (attr) {
>   case hwmon_fan_input:
> - *val = nvkm_therm_fan_sense(therm);
> + ret = nvkm_therm_fan_sense(therm);
>   break;
>   default:
>   return -EOPNOTSUPP;
>   }
>  
> + if (ret < 0)
> + return ret;
> +
> + *val = ret;
>   return 0;
>  }
>  
> @@ -491,7 +496,7 @@ nouveau_in_read(struct device *dev, u32 attr, int 
> channel, long *val)
>   struct drm_device *drm_dev = dev_get_drvdata(dev);
>   struct nouveau_drm *drm = nouveau_drm(drm_dev);
>   struct nvkm_volt *volt = nvxx_volt(>client.device);
> - int ret;
> + int ret = 0;
>  
>   if (!volt)
>   return -EOPNOTSUPP;
> @@ -499,7 +504,8 @@ nouveau_in_read(struct device *dev, u32 attr, int 
> channel, long *val)
>   switch (attr) {
>   case hwmon_in_input:
>   ret = nvkm_volt_get(volt);
> - *val = ret < 0 ? ret : (ret / 1000);
> + if (ret >= 0)
> + *val = ret / 1000;
>   break;
>   case hwmon_in_min:
>   *val = volt->min_uv > 0 ? (volt->min_uv / 1000) : -ENODEV;
> @@ -511,7 +517,7 @@ nouveau_in_read(struct device *dev, u32 attr, int 
> channel, long *val)
>   return -EOPNOTSUPP;
>   }
>  
> - return 0;
> + return ret;
>  }
>  
>  static int
> @@ -520,21 +526,26 @@ nouveau_pwm_read(struct device *dev, u32 attr, int 
> channel, long *val)
>   struct drm_device *drm_dev = dev_get_drvdata(dev);
>   struct nouveau_drm *drm = nouveau_drm(drm_dev);
>   struct nvkm_therm *therm = nvxx_therm(>client.device);
> + int ret;
>  
>   if (!therm || !therm->attr_get || !therm->fan_get)
>   return -EOPNOTSUPP;
>  
>   switch (attr) {
>   case hwmon_pwm_enable:
> - *val = therm->attr_get(therm, NVKM_THERM_ATTR_FAN_MODE);
> + ret = therm->attr_get(therm, NVKM_THERM_ATTR_FAN_MODE);
>   break;
>   case hwmon_pwm_input:
> - *val = therm->fan_get(therm);
> + ret = therm->fan_get(therm);
>   break;
>   default:
>   return -EOPNOTSUPP;
>   }
>  
> + if (ret < 0)
> + return ret;
> +
> + *val = ret;
>   return 0;
>  }
>  
> @@ -544,18 +555,26 @@ nouveau_power_read(struct device *dev, u32 attr, int 
> channel, long *val)
>   struct drm_device *drm_dev = dev_get_drvdata(dev);
>   struct nouveau_drm *drm = nouveau_drm(drm_dev);
>   struct nvkm_iccsense *iccsense = nvxx_iccsense(>client.device);
> + int ret;
>  
>   if (!iccsense)
>   return -EOPNOTSUPP;
>  
>   switch (attr) {
>   case hwmon_power_input:
> - *val = nvkm_iccsense_read_all(iccsense);
> + ret = nvkm_iccsense_read_all(iccsense);
> + if (ret < 0)
> + return ret;
> + *val = ret;
>   break;
>   case hwmon_power_max:
> + if (iccsense->power_w_max <= 0)
> + return -ENODEV;
>   *val = iccsense->power_w_max;
>   break;
>   case hwmon_power_crit:
> + if (iccsense->power_w_crit <= 0)
> + return -ENODEV;
>   *val = iccsense->power_w_crit;
>   break;
>   default:
> -- 
> 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

Re: [Nouveau] [RFC PATCH 01/29] therm: split return code and value in nvkm_get_temp

2017-10-08 Thread Pierre Moreau

As you changed the return value of `temp_get()` to solely be the error code, or
absence of an error, I would change all those tests that checked whether the
returned value was strictly less, or greater than, 0 to now only compare
against 0 (no error). For example,

  if (therm && therm->attr_get && nvkm_therm_temp_get(therm, ) < 0)
  if (therm->func->temp_get(therm, ) >= 0)

could become

  if (therm && therm->attr_get && nvkm_therm_temp_get(therm, ))
  if (!therm->func->temp_get(therm, ))

to match other error checking code.

More comments below

On 2017-09-15 — 17:11, Karol Herbst wrote:
> The current hwmon code doesn't check if the returned value was actually an
> error.
> 
> Since Kepler temperature sensors are able to report negative values.

I assume those negative values are not for error reporting, but more if you
buried your GPU in the snow somewhere in the Antarctic and still want a valid
temperature to be reported.

> Since Pascal (and maybe earlier) we have sensors with improved precision.
> 
> Adjust the nvkm_get_temp method to be able to deal with those changes
> and let hwmon return an error properly.
> 
> Signed-off-by: Karol Herbst 
> ---
>  drm/nouveau/include/nvkm/subdev/therm.h |  2 +-
>  drm/nouveau/nouveau_hwmon.c | 15 +--
>  drm/nouveau/nvkm/subdev/therm/base.c| 19 ++-
>  drm/nouveau/nvkm/subdev/therm/g84.c | 11 ++-
>  drm/nouveau/nvkm/subdev/therm/gp100.c   |  9 +
>  drm/nouveau/nvkm/subdev/therm/nv40.c|  9 +++--
>  drm/nouveau/nvkm/subdev/therm/nv50.c|  9 +++--
>  drm/nouveau/nvkm/subdev/therm/priv.h|  4 ++--
>  drm/nouveau/nvkm/subdev/therm/temp.c| 16 
>  9 files changed, 55 insertions(+), 39 deletions(-)
> 
> diff --git a/drm/nouveau/include/nvkm/subdev/therm.h 
> b/drm/nouveau/include/nvkm/subdev/therm.h
> index 9841f076..8c84017f 100644
> --- a/drm/nouveau/include/nvkm/subdev/therm.h
> +++ b/drm/nouveau/include/nvkm/subdev/therm.h
> @@ -86,7 +86,7 @@ struct nvkm_therm {
>   int (*attr_set)(struct nvkm_therm *, enum nvkm_therm_attr_type, int);
>  };
>  
> -int nvkm_therm_temp_get(struct nvkm_therm *);
> +int nvkm_therm_temp_get(struct nvkm_therm *, int *);
>  int nvkm_therm_fan_sense(struct nvkm_therm *);
>  int nvkm_therm_cstate(struct nvkm_therm *, int, int);
>  
> diff --git a/drm/nouveau/nouveau_hwmon.c b/drm/nouveau/nouveau_hwmon.c
> index 7c965648..f1914d9a 100644
> --- a/drm/nouveau/nouveau_hwmon.c
> +++ b/drm/nouveau/nouveau_hwmon.c
> @@ -326,8 +326,9 @@ nouveau_temp_is_visible(const void *data, u32 attr, int 
> channel)
>  {
>   struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
>   struct nvkm_therm *therm = nvxx_therm(>client.device);
> + int val;
>  
> - if (therm && therm->attr_get && nvkm_therm_temp_get(therm) < 0)
> + if (therm && therm->attr_get && nvkm_therm_temp_get(therm, ) < 0)
>   return 0;
>  
>   switch (attr) {
> @@ -421,15 +422,16 @@ nouveau_temp_read(struct device *dev, u32 attr, int 
> channel, long *val)
>   struct drm_device *drm_dev = dev_get_drvdata(dev);
>   struct nouveau_drm *drm = nouveau_drm(drm_dev);
>   struct nvkm_therm *therm = nvxx_therm(>client.device);
> - int ret;
> + int ret = 0;
> + int temp;
>  
>   if (!therm || !therm->attr_get)
>   return -EOPNOTSUPP;
>  
>   switch (attr) {
>   case hwmon_temp_input:
> - ret = nvkm_therm_temp_get(therm);
> - *val = ret < 0 ? ret : (ret * 1000);
> + ret = nvkm_therm_temp_get(therm, );
> + *val = temp * 1000;
>   break;
>   case hwmon_temp_max:
>   *val = therm->attr_get(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK)
> @@ -459,7 +461,7 @@ nouveau_temp_read(struct device *dev, u32 attr, int 
> channel, long *val)
>   return -EOPNOTSUPP;
>   }
>  
> - return 0;
> + return ret;
>  }
>  
>  static int
> @@ -713,6 +715,7 @@ nouveau_hwmon_init(struct drm_device *dev)
>   struct device *hwmon_dev;
>   int ret = 0;
>   int i = 0;
> + int val;
>  
>   hwmon = drm->hwmon = kzalloc(sizeof(*hwmon), GFP_KERNEL);
>   if (!hwmon)
> @@ -720,7 +723,7 @@ nouveau_hwmon_init(struct drm_device *dev)
>   hwmon->dev = dev;
>  
>   if (therm && therm->attr_get && therm->attr_set) {
> - if (nvkm_therm_temp_get(therm) >= 0)
> + if (nvkm_therm_temp_get(therm, ) >= 0)
>   special_groups[i++] = _auto_point_sensor_group;
>   if (therm->fan_get && therm->fan_get(therm) >= 0)
>   special_groups[i++] = _fan_sensor_group;
> diff --git a/drm/nouveau/nvkm/subdev/therm/base.c 
> b/drm/nouveau/nvkm/subdev/therm/base.c
> index f27fc6d0..8fa691fb 100644
> --- a/drm/nouveau/nvkm/subdev/therm/base.c
> +++ b/drm/nouveau/nvkm/subdev/therm/base.c
> @@ -24,22 +24,26 @@
>  #include "priv.h"
>  
>  int
>

Re: [Nouveau] [RFC PATCH 15/29] bios: add thermal policies table

Re: [Nouveau] [RFC PATCH 16/29] clk: parse thermal policies for throttling thresholds

Re: [Nouveau] [RFC PATCH 17/29] clk: thermal throttling

Re: [Nouveau] [RFC PATCH 15/29] bios: add thermal policies table

Re: [Nouveau] [RFC PATCH 16/29] clk: parse thermal policies for throttling thresholds

Re: [Nouveau] [RFC PATCH 14/29] therm: Trigger reclock in temperature daemon

Re: [Nouveau] [RFC PATCH 09/29] clk: Set clocks to pre suspend state after suspend

Re: [Nouveau] [RFC PATCH 08/29] clk: We should pass the pstate id around not the index in the list

Re: [Nouveau] [RFC PATCH 08/29] clk: We should pass the pstate id around not the index in the list

Re: [Nouveau] [RFC PATCH 06/29] clk: Make pstate a pointer to nvkm_pstate

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

Re: [Nouveau] [RFC PATCH 07/29] clk: Hold information about the current cstate status

Re: [Nouveau] [RFC PATCH 06/29] clk: Make pstate a pointer to nvkm_pstate

Re: [Nouveau] [RFC PATCH 05/29] clk: Remove dstate

Re: [Nouveau] [RFC PATCH 03/29] subdev/volt/gk104: return error when read fails

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

Re: [Nouveau] [RFC PATCH 01/29] therm: split return code and value in nvkm_get_temp

Re: [Nouveau] [RFC PATCH 03/29] subdev/volt/gk104: return error when read fails

Re: [Nouveau] [RFC PATCH 02/29] hwmon: properly check for errors

Re: [Nouveau] [RFC PATCH 01/29] therm: split return code and value in nvkm_get_temp

20 matches

Site Navigation

Mail list logo

Footer information