SEO

As I mentioned in the mp setperf mail there is a bug in est and powernow
with regard to how they choose what state corresponds to which
performance level requested by the user (or apmd on his or her behalf).

This diff will choose which frequency to select based on calculations
involving the number of states and not the frequencies of the states as
on some systems this leads to odd behaviour such as this.

sysclt hw.setperf=0 lowest state
sysctl hw.setperf=1-100 = highest state

With this diff this behaviour would be corrected so that:
sysctl hw.setperf=0-50 lowest state
sysctl hw.setperf=50-100 highest state

Index: sys/arch/i386/i386/est.c
================================================== =================
RCS file: /cvs/src/sys/arch/i386/i386/est.c,v
retrieving revision 1.27
diff -u -p -u -p -r1.27 est.c
--- sys/arch/i386/i386/est.c 22 Dec 2006 01:34:46 -0000 1.27
+++ sys/arch/i386/i386/est.c 20 Apr 2007 14:18:02 -0000
@@ -1076,19 +1076,15 @@ est_init(const char *cpu_device, int ven
void
est_setperf(int level)
{
- int low, high, i, fq;
+ int low, i;
uint64_t msr;

if (est_fqlist == NULL)
return;

- low = MSR2MHZ(est_fqlist->table[est_fqlist->n - 1], bus_clock);
- high = MSR2MHZ(est_fqlist->table[0], bus_clock);
- fq = low + (high - low) * level / 100;
+ low = est_fqlist->n - 1;
+ i = low - ((level - 1) / (100 / est_fqlist->n));

- for (i = est_fqlist->n - 1; i > 0; i--)
- if (MSR2MHZ(est_fqlist->table[i], bus_clock) >= fq)
- break;
msr = rdmsr(MSR_PERF_CTL);
msr &= ~0xffffULL;
msr |= est_fqlist->table[i];
Index: sys/arch/i386/i386/powernow-k7.c
================================================== =================
RCS file: /cvs/src/sys/arch/i386/i386/powernow-k7.c,v
retrieving revision 1.30
diff -u -p -u -p -r1.30 powernow-k7.c
--- sys/arch/i386/i386/powernow-k7.c 20 Dec 2006 17:50:40 -0000 1.30
+++ sys/arch/i386/i386/powernow-k7.c 20 Apr 2007 14:25:11 -0000
@@ -139,23 +139,16 @@ int k7pnow_states(struct k7pnow_cpu_stat
void
k7_powernow_setperf(int level)
{
- unsigned int i, low, high, freq;
+ unsigned int i;
int cvid, cfid, vid = 0, fid = 0;
uint64_t status, ctl;
struct k7pnow_cpu_state * cstate;

cstate = k7pnow_current_state;
- high = cstate->state_table[cstate->n_states - 1].freq;
- low = cstate->state_table[0].freq;
- freq = low + (high - low) * level / 100;
-
- for (i = 0; i < cstate->n_states; i++) {
- if (cstate->state_table[i].freq >= freq) {
- fid = cstate->state_table[i].fid;
- vid = cstate->state_table[i].vid;
- break;
- }
- }
+
+ i = (level - 1) / (100 / cstate->n_states);
+ fid = cstate->state_table[i].fid;
+ vid = cstate->state_table[i].vid;

if (fid == 0 || vid == 0)
return;
@@ -269,7 +262,7 @@ k7pnow_states(struct k7pnow_cpu_state *c

if (cpusig == pst->signature && fid == pst->fid
&& vid == pst->vid) {
-
+
if (abs(cstate->fsb - pst->fsb) > 5)
continue;
cstate->n_states = pst->n_states;
@@ -277,7 +270,8 @@ k7pnow_states(struct k7pnow_cpu_state *c
p + sizeof(struct pst_s),
cstate->n_states));
}
- p += sizeof(struct pst_s) + (2 * pst->n_states);
+ p += sizeof(struct pst_s) +
+ (2 * pst->n_states);
}
}
}
@@ -317,7 +311,7 @@ k7_powernow_init(void)
if (!cstate)
return;

- cstate->flags = cstate->n_states = 0;
+ cstate->flags = cstate->n_states = 0;
if (ci->ci_signature == AMD_ERRATA_A0_CPUSIG)
cstate->flags |= PN7_FLAG_ERRATA_A0;

@@ -330,7 +324,7 @@ k7_powernow_init(void)

/* if the base CPUID signature fails to match try, the extended one */
if (!k7pnow_states(cstate, ci->ci_signature, maxfid, startvid))
- k7pnow_states(cstate, regs[0], maxfid, startvid);
+ k7pnow_states(cstate, regs[0], maxfid, startvid);
if (cstate->n_states) {
if (cstate->flags & PN7_FLAG_DESKTOP_VRM)
techname = "Cool'n'Quiet K7";
@@ -342,8 +336,8 @@ k7_powernow_init(void)
state = &cstate->state_table[i-1];
printf(" %d", state->freq);
}
- printf(" MHz\n");
-
+ printf(" MHz\n");
+
k7pnow_current_state = cstate;
cpu_setperf = k7_powernow_setperf;
setperf_prio = 1;
Index: sys/arch/i386/i386/powernow-k8.c
================================================== =================
RCS file: /cvs/src/sys/arch/i386/i386/powernow-k8.c,v
retrieving revision 1.21
diff -u -p -u -p -r1.21 powernow-k8.c
--- sys/arch/i386/i386/powernow-k8.c 20 Dec 2006 17:50:40 -0000 1.21
+++ sys/arch/i386/i386/powernow-k8.c 20 Apr 2007 14:21:32 -0000
@@ -132,9 +132,6 @@ struct pst_s {
struct k8pnow_cpu_state *k8pnow_current_state = NULL;
extern int setperf_prio;

-/*
- * Prototypes
- */
int k8pnow_read_pending_wait(uint64_t *);
int k8pnow_decode_pst(struct k8pnow_cpu_state *, uint8_t *);
int k8pnow_states(struct k8pnow_cpu_state *, uint32_t, unsigned int,
@@ -158,7 +155,7 @@ k8pnow_read_pending_wait(uint64_t *statu
void
k8_powernow_setperf(int level)
{
- unsigned int i, low, high, freq;
+ unsigned int i;
uint64_t status;
int cfid, cvid, fid = 0, vid = 0, rvo;
u_int val;
@@ -175,18 +172,10 @@ k8_powernow_setperf(int level)
cvid = PN8_STA_CVID(status);

cstate = k8pnow_current_state;
- low = cstate->state_table[0].freq;
- high = cstate->state_table[cstate->n_states-1].freq;

- freq = low + (high - low) * level / 100;
-
- for (i = 0; i < cstate->n_states; i++) {
- if (cstate->state_table[i].freq >= freq) {
- fid = cstate->state_table[i].fid;
- vid = cstate->state_table[i].vid;
- break;
- }
- }
+ i = (level - 1) / (100 / cstate->n_states);
+ fid = cstate->state_table[i].fid;
+ vid = cstate->state_table[i].vid;

if (fid == cfid && vid == cvid)
return;
@@ -329,7 +318,8 @@ k8pnow_states(struct k8pnow_cpu_state *c
return (k8pnow_decode_pst(cstate,
p+= sizeof (struct pst_s)));
}
- p += sizeof(struct pst_s) + 2 * cstate->n_states;
+ p += sizeof(struct pst_s) + 2
+ * cstate->n_states;
}
}
}
@@ -360,11 +350,11 @@ k8_powernow_init(void)
cpuid(0x80000000, regs);
if (regs[0] < 0x80000007)
return;
-
+
cpuid(0x80000007, regs);
if (!(regs[3] & AMD_PN_FID_VID))
return;
-
+
/* Extended CPUID signature value */
cpuid(0x80000001, regs);