VRM(VRD) detection versus CPUID

[Rudolf Marek]

On Fri, 2 Jul 2004, Jean Delvare wrote:

>
> >typedef struct {
> >u8 vendor;
> >u8 eff_family;
> >u8 eff_model;
> >vrm_t vrm_type;
> >} vrm_models_t;
> >#define ANY 0xFF
>
> What is 0xFF is ever a valid value for eff_model?

No, model is 4 bit :)

>
> >vrm_models_t vrm_models[] = {
> >                {X86_VENDOR_AMD,0x6,ANY,VRM_INTEL_9x}, //athlon duron etc
> >                {X86_VENDOR_AMD,0xF,4,VRM_INTEL_9x}, //Athlon 64
> >                {X86_VENDOR_AMD,0xF,5,VRM_AMD_OPTERON},
> >                {X86_VENDOR_INTEL,0x6,0x9,VRM_INTEL_85},  /* 0.13um too */
> >                {X86_VENDOR_INTEL,0x6,0xB,VRM_INTEL_85},  /* 0xB Tualatin */
> >                {X86_VENDOR_INTEL,0x6,ANY,VRM_INTEL_8x},  /* any P6 */
> >                {X86_VENDOR_INTEL,0x7,ANY,VRM_UNKNOWN},   /* Itanium */
> >                {X86_VENDOR_INTEL,0xF,0x3,VRM_INTEL_10x}, /* P4 Prescott */
> >                {X86_VENDOR_INTEL,0xF,ANY,VRM_INTEL_9x},  /* P4 before
> >Prescott*/
> >                {X86_VENDOR_INTEL,0x10,ANY,VRM_INTEL_ITANIUM2} /*Itanium 2*/
> >                };
> >#define num_models (sizeof(vrm_models) / sizeof(vrm_models_t) )
>
> I think that the usual practice for this kind of lookup tables is to have
> an "empty" final line and rely on it to detect the end of the table.
> Your method looks OK too, but maybe there's a reason why kernel folks
> seem to prefer the empty last line?

I will take a look to kernel code.

>
> >vrm_t find_vrm(u8 ef,u8 em,u8 vendor) {
> >int i;
> >
> >for (i=0;i<num_models;i++) {
> >    if (vrm_models[i].vendor==vendor)
> >       if
> >((vrm_models[i].eff_family==ef)&&((vrm_models[i].eff_model==ef)|| \
>
>                                                              ^^
>                                                              em
>
oops :)  I tested only the ANY case for this PII Thanks.

> >       ((vrm_models[i].eff_model==ANY)))) return vrm_models[i].vrm_type;
> >    }
> >return VRM_UNKNOWN;
> >}
>
> >static vrm_t whichVRM(void) {
> >struct cpuinfo_x86 *c = cpu_data;
> >
> >u32 eax;
> >u8 f,m;
>
> Please use longer variable names in the final code. Short varible names
> make the code harder to read.

I will do.
>
> >    eax = cpuid_eax(1);
> >    f = ((eax & 0x00000F00)>>8);
> >    m = ((eax & 0x000000F0)>>4);
> >    if (f == 0xF) {
> >    f+=((eax & 0x00F00000)>>20);
>
> No <<4 here?


No according to AMD paper is family "additive", model is "concat" so
thatswhy it is shifted by 4;

> >    m+=((eax & 0x000F0000)>>16)<<4;
> >    }
> >return find_vrm(f,m,c->x86_vendor);
>
> Looks overwise really good,

I'm not sure if we can leave 0x000F0000 such things there...

>I'll test this on as many systems as
> possible soon.
>
Ok thank you.

Rudolf