[v2,06/22] volt: parse the both max voltage entries

Submitted by Karol Herbst on March 21, 2016, 4:16 p.m.

Details

Message ID 1458577000-6615-7-git-send-email-nouveau@karolherbst.de
State New
Headers show
Series "Volting/Clocking improvements for Fermi and newer" ( rev: 2 ) in Nouveau

Not browsing as part of any series.

Commit Message

Karol Herbst March 21, 2016, 4:16 p.m.
these entries specify a maximum voltage nvidia never exceeds, we shouldn't do
that, too.

Signed-off-by: Karol Herbst <nouveau@karolherbst.de>
---
 drm/nouveau/include/nvkm/subdev/bios/vmap.h |  2 ++
 drm/nouveau/include/nvkm/subdev/volt.h      |  2 ++
 drm/nouveau/nvkm/subdev/bios/vmap.c         |  5 +++++
 drm/nouveau/nvkm/subdev/volt/base.c         | 11 +++++++++++
 4 files changed, 20 insertions(+)

Patch hide | download patch | download mbox

diff --git a/drm/nouveau/include/nvkm/subdev/bios/vmap.h b/drm/nouveau/include/nvkm/subdev/bios/vmap.h
index 6633c6d..48fe71d 100644
--- a/drm/nouveau/include/nvkm/subdev/bios/vmap.h
+++ b/drm/nouveau/include/nvkm/subdev/bios/vmap.h
@@ -1,6 +1,8 @@ 
 #ifndef __NVBIOS_VMAP_H__
 #define __NVBIOS_VMAP_H__
 struct nvbios_vmap {
+	u8  max0;
+	u8  max1;
 };
 
 u16 nvbios_vmap_table(struct nvkm_bios *, u8 *ver, u8 *hdr, u8 *cnt, u8 *len);
diff --git a/drm/nouveau/include/nvkm/subdev/volt.h b/drm/nouveau/include/nvkm/subdev/volt.h
index fc68825..3e0f8da 100644
--- a/drm/nouveau/include/nvkm/subdev/volt.h
+++ b/drm/nouveau/include/nvkm/subdev/volt.h
@@ -15,6 +15,8 @@  struct nvkm_volt {
 
 	u32 max_uv;
 	u32 min_uv;
+	u8 max0_vid;
+	u8 max1_vid;
 };
 
 int nvkm_volt_map_min(struct nvkm_volt *volt, u8 id);
diff --git a/drm/nouveau/nvkm/subdev/bios/vmap.c b/drm/nouveau/nvkm/subdev/bios/vmap.c
index 2f13db7..f5463b1 100644
--- a/drm/nouveau/nvkm/subdev/bios/vmap.c
+++ b/drm/nouveau/nvkm/subdev/bios/vmap.c
@@ -61,7 +61,12 @@  nvbios_vmap_parse(struct nvkm_bios *bios, u8 *ver, u8 *hdr, u8 *cnt, u8 *len,
 	memset(info, 0x00, sizeof(*info));
 	switch (!!vmap * *ver) {
 	case 0x10:
+		info->max0 = 0xff;
+		info->max1 = 0xff;
+		break;
 	case 0x20:
+		info->max0 = nvbios_rd08(bios, vmap + 0x7);
+		info->max1 = nvbios_rd08(bios, vmap + 0x8);
 		break;
 	}
 	return vmap;
diff --git a/drm/nouveau/nvkm/subdev/volt/base.c b/drm/nouveau/nvkm/subdev/volt/base.c
index 71094a9..205dfcf 100644
--- a/drm/nouveau/nvkm/subdev/volt/base.c
+++ b/drm/nouveau/nvkm/subdev/volt/base.c
@@ -217,9 +217,20 @@  nvkm_volt_ctor(const struct nvkm_volt_func *func, struct nvkm_device *device,
 
 	/* Assuming the non-bios device should build the voltage table later */
 	if (bios) {
+		u8 ver, hdr, cnt, len;
+		struct nvbios_vmap vmap;
+
 		nvkm_volt_parse_bios(bios, volt);
 		nvkm_debug(&volt->subdev, "min: %iuv max: %iuv\n",
 			   volt->min_uv, volt->max_uv);
+
+		if (nvbios_vmap_parse(bios, &ver, &hdr, &cnt, &len, &vmap)) {
+			volt->max0_vid = vmap.max0;
+			volt->max1_vid = vmap.max1;
+		} else {
+			volt->max0_vid = 0xff;
+			volt->max1_vid = 0xff;
+		}
 	}
 
 	if (volt->vid_nr) {

Comments

On 21/03/16 18:16, Karol Herbst wrote:
> these entries specify a maximum voltage nvidia never exceeds, we shouldn't do
> that, too.
>
> Signed-off-by: Karol Herbst <nouveau@karolherbst.de>
> ---
>   drm/nouveau/include/nvkm/subdev/bios/vmap.h |  2 ++
>   drm/nouveau/include/nvkm/subdev/volt.h      |  2 ++
>   drm/nouveau/nvkm/subdev/bios/vmap.c         |  5 +++++
>   drm/nouveau/nvkm/subdev/volt/base.c         | 11 +++++++++++
>   4 files changed, 20 insertions(+)
>
> diff --git a/drm/nouveau/include/nvkm/subdev/bios/vmap.h b/drm/nouveau/include/nvkm/subdev/bios/vmap.h
> index 6633c6d..48fe71d 100644
> --- a/drm/nouveau/include/nvkm/subdev/bios/vmap.h
> +++ b/drm/nouveau/include/nvkm/subdev/bios/vmap.h
> @@ -1,6 +1,8 @@
>   #ifndef __NVBIOS_VMAP_H__
>   #define __NVBIOS_VMAP_H__
>   struct nvbios_vmap {
> +	u8  max0;
> +	u8  max1;
>   };
>   
>   u16 nvbios_vmap_table(struct nvkm_bios *, u8 *ver, u8 *hdr, u8 *cnt, u8 *len);
> diff --git a/drm/nouveau/include/nvkm/subdev/volt.h b/drm/nouveau/include/nvkm/subdev/volt.h
> index fc68825..3e0f8da 100644
> --- a/drm/nouveau/include/nvkm/subdev/volt.h
> +++ b/drm/nouveau/include/nvkm/subdev/volt.h
> @@ -15,6 +15,8 @@ struct nvkm_volt {
>   
>   	u32 max_uv;
>   	u32 min_uv;
> +	u8 max0_vid;
> +	u8 max1_vid;
>   };
>   
>   int nvkm_volt_map_min(struct nvkm_volt *volt, u8 id);
> diff --git a/drm/nouveau/nvkm/subdev/bios/vmap.c b/drm/nouveau/nvkm/subdev/bios/vmap.c
> index 2f13db7..f5463b1 100644
> --- a/drm/nouveau/nvkm/subdev/bios/vmap.c
> +++ b/drm/nouveau/nvkm/subdev/bios/vmap.c
> @@ -61,7 +61,12 @@ nvbios_vmap_parse(struct nvkm_bios *bios, u8 *ver, u8 *hdr, u8 *cnt, u8 *len,
>   	memset(info, 0x00, sizeof(*info));
>   	switch (!!vmap * *ver) {
>   	case 0x10:
> +		info->max0 = 0xff;
> +		info->max1 = 0xff;
> +		break;
>   	case 0x20:
> +		info->max0 = nvbios_rd08(bios, vmap + 0x7);
> +		info->max1 = nvbios_rd08(bios, vmap + 0x8);
>   		break;
>   	}
>   	return vmap;
> diff --git a/drm/nouveau/nvkm/subdev/volt/base.c b/drm/nouveau/nvkm/subdev/volt/base.c
> index 71094a9..205dfcf 100644
> --- a/drm/nouveau/nvkm/subdev/volt/base.c
> +++ b/drm/nouveau/nvkm/subdev/volt/base.c
> @@ -217,9 +217,20 @@ nvkm_volt_ctor(const struct nvkm_volt_func *func, struct nvkm_device *device,
>   
>   	/* Assuming the non-bios device should build the voltage table later */
>   	if (bios) {
> +		u8 ver, hdr, cnt, len;
> +		struct nvbios_vmap vmap;
> +
>   		nvkm_volt_parse_bios(bios, volt);
>   		nvkm_debug(&volt->subdev, "min: %iuv max: %iuv\n",
>   			   volt->min_uv, volt->max_uv);
> +
> +		if (nvbios_vmap_parse(bios, &ver, &hdr, &cnt, &len, &vmap)) {
> +			volt->max0_vid = vmap.max0;
> +			volt->max1_vid = vmap.max1;
> +		} else {
> +			volt->max0_vid = 0xff;
> +			volt->max1_vid = 0xff;
> +		}
>   	}
>   
>   	if (volt->vid_nr) {

This is really peculiar that NVIDIA would have 2 max_vid in the same 
entry :s

How did you reverse that? Did you really see no differences? Maybe the 
two max_vid are for different temperatures?
On 28/03/16 23:49, Martin Peres wrote:
> On 21/03/16 18:16, Karol Herbst wrote:
>> these entries specify a maximum voltage nvidia never exceeds, we shouldn't do
>> that, too.
>>
>> Signed-off-by: Karol Herbst <nouveau@karolherbst.de>
>> ---
>>    drm/nouveau/include/nvkm/subdev/bios/vmap.h |  2 ++
>>    drm/nouveau/include/nvkm/subdev/volt.h      |  2 ++
>>    drm/nouveau/nvkm/subdev/bios/vmap.c         |  5 +++++
>>    drm/nouveau/nvkm/subdev/volt/base.c         | 11 +++++++++++
>>    4 files changed, 20 insertions(+)
>>
>> diff --git a/drm/nouveau/include/nvkm/subdev/bios/vmap.h b/drm/nouveau/include/nvkm/subdev/bios/vmap.h
>> index 6633c6d..48fe71d 100644
>> --- a/drm/nouveau/include/nvkm/subdev/bios/vmap.h
>> +++ b/drm/nouveau/include/nvkm/subdev/bios/vmap.h
>> @@ -1,6 +1,8 @@
>>    #ifndef __NVBIOS_VMAP_H__
>>    #define __NVBIOS_VMAP_H__
>>    struct nvbios_vmap {
>> +	u8  max0;
>> +	u8  max1;
>>    };
>>    
>>    u16 nvbios_vmap_table(struct nvkm_bios *, u8 *ver, u8 *hdr, u8 *cnt, u8 *len);
>> diff --git a/drm/nouveau/include/nvkm/subdev/volt.h b/drm/nouveau/include/nvkm/subdev/volt.h
>> index fc68825..3e0f8da 100644
>> --- a/drm/nouveau/include/nvkm/subdev/volt.h
>> +++ b/drm/nouveau/include/nvkm/subdev/volt.h
>> @@ -15,6 +15,8 @@ struct nvkm_volt {
>>    
>>    	u32 max_uv;
>>    	u32 min_uv;
>> +	u8 max0_vid;
>> +	u8 max1_vid;
>>    };
>>    
>>    int nvkm_volt_map_min(struct nvkm_volt *volt, u8 id);
>> diff --git a/drm/nouveau/nvkm/subdev/bios/vmap.c b/drm/nouveau/nvkm/subdev/bios/vmap.c
>> index 2f13db7..f5463b1 100644
>> --- a/drm/nouveau/nvkm/subdev/bios/vmap.c
>> +++ b/drm/nouveau/nvkm/subdev/bios/vmap.c
>> @@ -61,7 +61,12 @@ nvbios_vmap_parse(struct nvkm_bios *bios, u8 *ver, u8 *hdr, u8 *cnt, u8 *len,
>>    	memset(info, 0x00, sizeof(*info));
>>    	switch (!!vmap * *ver) {
>>    	case 0x10:
>> +		info->max0 = 0xff;
>> +		info->max1 = 0xff;
>> +		break;
>>    	case 0x20:
>> +		info->max0 = nvbios_rd08(bios, vmap + 0x7);
>> +		info->max1 = nvbios_rd08(bios, vmap + 0x8);
>>    		break;
>>    	}
>>    	return vmap;
>> diff --git a/drm/nouveau/nvkm/subdev/volt/base.c b/drm/nouveau/nvkm/subdev/volt/base.c
>> index 71094a9..205dfcf 100644
>> --- a/drm/nouveau/nvkm/subdev/volt/base.c
>> +++ b/drm/nouveau/nvkm/subdev/volt/base.c
>> @@ -217,9 +217,20 @@ nvkm_volt_ctor(const struct nvkm_volt_func *func, struct nvkm_device *device,
>>    
>>    	/* Assuming the non-bios device should build the voltage table later */
>>    	if (bios) {
>> +		u8 ver, hdr, cnt, len;
>> +		struct nvbios_vmap vmap;
>> +
>>    		nvkm_volt_parse_bios(bios, volt);
>>    		nvkm_debug(&volt->subdev, "min: %iuv max: %iuv\n",
>>    			   volt->min_uv, volt->max_uv);
>> +
>> +		if (nvbios_vmap_parse(bios, &ver, &hdr, &cnt, &len, &vmap)) {
>> +			volt->max0_vid = vmap.max0;
>> +			volt->max1_vid = vmap.max1;
>> +		} else {
>> +			volt->max0_vid = 0xff;
>> +			volt->max1_vid = 0xff;
>> +		}
>>    	}
>>    
>>    	if (volt->vid_nr) {
> This is really peculiar that NVIDIA would have 2 max_vid in the same
> entry :s
>
> How did you reverse that? Did you really see no differences? Maybe the
> two max_vid are for different temperatures?
Ok, after discussions on IRC, I understand better. Max0/1 is actually 
not a vid but an entry in the voltage mapping table, which has different 
coefficient for the temperature. This allows OEMs to clamp the maximum 
voltage with not one but two different functions. I can see why it is OK.

Can you explain this a little more? Maybe as a comment in the code?