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

[Martin Peres]

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 at 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?