[Nouveau] [PATCH v4 20/37] volt: add coefficients

[Martin Peres]

On 18/04/16 22:13, Karol Herbst wrote:
> I am sure that those are a bit different, but while testing the biggest error
> compared to nvidia was -1.5%.

Is this still true? I thought we were *much* closer now.

>
> Without this change we are most of the time around 10% below nvidias voltage,
> so this change causes no harm and improves the situation a lot already.

Yeah, this is definitely not up to date!

>
> These coefficients were REed by modifing the voltage map entries and by
> calculating the set voltage back until I was able to forecast which voltage
> nvidia sets for a given voltage map entry.
>
> v4: use better coefficients and speedo
>
> Signed-off-by: Karol Herbst <nouveau at karolherbst.de>
> ---
>   drm/nouveau/nvkm/subdev/volt/base.c | 38 +++++++++++++++++++++++++++++++++++--
>   1 file changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/drm/nouveau/nvkm/subdev/volt/base.c b/drm/nouveau/nvkm/subdev/volt/base.c
> index cecfac6..5e35d96 100644
> --- a/drm/nouveau/nvkm/subdev/volt/base.c
> +++ b/drm/nouveau/nvkm/subdev/volt/base.c
> @@ -110,13 +110,47 @@ nvkm_volt_map(struct nvkm_volt *volt, u8 id, u8 temp)
>   
>   	vmap = nvbios_vmap_entry_parse(bios, id, &ver, &len, &info);
>   	if (vmap) {
> +		s64 result;
> +
> +		if (volt->speedo < 0)
> +			return volt->speedo;

Hmm, so you will refuse reclocking if the speedo cannot be read... 
Fair-enough, but I would like to see a warning in the kernel logs.

> +
> +		if (ver == 0x10 || (ver == 0x20 && info.mode == 0)) {
> +			result  =  (s64)info.arg[0] / 10;
> +			result += ((s64)info.arg[1] * volt->speedo) / 10;
> +			result += ((s64)info.arg[2] * volt->speedo * volt->speedo) / 100000;
> +		} else if (ver == 0x20) {
> +			switch (info.mode) {
> +			/* 0x0 handled above! */
> +			case 0x1:
> +				result =  ((s64)info.arg[0] * 15625) >> 18;
> +				result += ((s64)info.arg[1] * volt->speedo * 15625) >> 18;
> +				result += ((s64)info.arg[2] * temp * 15625) >> 10;
> +				result += ((s64)info.arg[3] * volt->speedo * temp * 15625) >> 18;
> +				result += ((s64)info.arg[4] * volt->speedo * volt->speedo * 15625) >> 30;
> +				result += ((s64)info.arg[5] * temp * temp * 15625) >> 18;
> +				break;

Well, I can only say that these values got us really really close to 
what nvidia does... On around 10 GPUs... So... until we know better, 
let's stick to them.

Hopefully, we can figure out where this 15625 comes from. This patch is:

Reviewed-by: Martin Peres <martin.peres at free.fr>

> +			case 0x3:
> +				result = (info.min + info.max) / 2;
> +				break;
> +			case 0x2:
> +			default:
> +				result = info.min;
> +				break;
> +			}
> +		} else {
> +			return -ENODEV;
> +		}
> +
> +		result = min(max(result, (s64)info.min), (s64)info.max);
> +
>   		if (info.link != 0xff) {
>   			int ret = nvkm_volt_map(volt, info.link, temp);
>   			if (ret < 0)
>   				return ret;
> -			info.min += ret;
> +			result += ret;
>   		}
> -		return info.min;
> +		return result;
>   	}
>   
>   	return id ? id * 10000 : -ENODEV;