| Message ID | 1391763814-6644-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 07, 2014 at 10:03:33AM +0100, Daniel Vetter wrote: > The kernel will round it, so if we don't we'll have a spurious > mismatch. Happens on my machine here with 650-1300MHz range, where the > midpoint is 975. > > Cc: Jeff McGee <jeff.mcgee@intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > tests/pm_rps.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/tests/pm_rps.c b/tests/pm_rps.c > index 467038104ec6..27e758755e3f 100644 > --- a/tests/pm_rps.c > +++ b/tests/pm_rps.c > @@ -350,6 +350,9 @@ static void min_max_config(void (*check)(void)) > { > int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2; > > + /* hw (and so kernel) currently rounds to 50 MHz ... */ s/rounds/truncates/ or if it really does round, you need to adjust the calculation. > + fmid = fmid / 50 * 50; > + > log("\nCheck original min and max...\n"); > check(); -Chris
On Fri, Feb 7, 2014 at 10:33 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Fri, Feb 07, 2014 at 10:03:33AM +0100, Daniel Vetter wrote: >> The kernel will round it, so if we don't we'll have a spurious >> mismatch. Happens on my machine here with 650-1300MHz range, where the >> midpoint is 975. >> >> Cc: Jeff McGee <jeff.mcgee@intel.com> >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> --- >> tests/pm_rps.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/tests/pm_rps.c b/tests/pm_rps.c >> index 467038104ec6..27e758755e3f 100644 >> --- a/tests/pm_rps.c >> +++ b/tests/pm_rps.c >> @@ -350,6 +350,9 @@ static void min_max_config(void (*check)(void)) >> { >> int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2; >> >> + /* hw (and so kernel) currently rounds to 50 MHz ... */ > > s/rounds/truncates/ or if it really does round, you need to adjust the > calculation. We just need to use something divisible by 50 so that the value we write and the one we get match up. Whether it's truncating or rounding doesn't matter really. -Daniel
On Fri, Feb 07, 2014 at 11:15:15AM +0100, Daniel Vetter wrote: > On Fri, Feb 7, 2014 at 10:33 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > On Fri, Feb 07, 2014 at 10:03:33AM +0100, Daniel Vetter wrote: > >> The kernel will round it, so if we don't we'll have a spurious > >> mismatch. Happens on my machine here with 650-1300MHz range, where the > >> midpoint is 975. > >> > >> Cc: Jeff McGee <jeff.mcgee@intel.com> > >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > >> --- > >> tests/pm_rps.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/tests/pm_rps.c b/tests/pm_rps.c > >> index 467038104ec6..27e758755e3f 100644 > >> --- a/tests/pm_rps.c > >> +++ b/tests/pm_rps.c > >> @@ -350,6 +350,9 @@ static void min_max_config(void (*check)(void)) > >> { > >> int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2; > >> > >> + /* hw (and so kernel) currently rounds to 50 MHz ... */ > > > > s/rounds/truncates/ or if it really does round, you need to adjust the > > calculation. > > We just need to use something divisible by 50 so that the value we > write and the one we get match up. Whether it's truncating or rounding > doesn't matter really. > -Daniel Darn, I considered this possibility but forgot to account for it in the test. I think what I was going to do was to create another writeval variant which doesn't do read back matching check. This was because I didn't want to assume that all systems use a 50 Mhz frequency increment (do they all?). -Jeff
On Fri, Feb 07, 2014 at 08:44:14AM -0600, Jeff McGee wrote: > On Fri, Feb 07, 2014 at 11:15:15AM +0100, Daniel Vetter wrote: > > On Fri, Feb 7, 2014 at 10:33 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > On Fri, Feb 07, 2014 at 10:03:33AM +0100, Daniel Vetter wrote: > > >> The kernel will round it, so if we don't we'll have a spurious > > >> mismatch. Happens on my machine here with 650-1300MHz range, where the > > >> midpoint is 975. > > >> > > >> Cc: Jeff McGee <jeff.mcgee@intel.com> > > >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > >> --- > > >> tests/pm_rps.c | 3 +++ > > >> 1 file changed, 3 insertions(+) > > >> > > >> diff --git a/tests/pm_rps.c b/tests/pm_rps.c > > >> index 467038104ec6..27e758755e3f 100644 > > >> --- a/tests/pm_rps.c > > >> +++ b/tests/pm_rps.c > > >> @@ -350,6 +350,9 @@ static void min_max_config(void (*check)(void)) > > >> { > > >> int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2; > > >> > > >> + /* hw (and so kernel) currently rounds to 50 MHz ... */ > > > > > > s/rounds/truncates/ or if it really does round, you need to adjust the > > > calculation. > > > > We just need to use something divisible by 50 so that the value we > > write and the one we get match up. Whether it's truncating or rounding > > doesn't matter really. > > -Daniel > > Darn, I considered this possibility but forgot to account for it in the test. > I think what I was going to do was to create another writeval variant > which doesn't do read back matching check. This was because I didn't want to > assume that all systems use a 50 Mhz frequency increment (do they all?). VLV sure doesn't.
On Fri, Feb 07, 2014 at 04:52:30PM +0200, Ville Syrjälä wrote: > On Fri, Feb 07, 2014 at 08:44:14AM -0600, Jeff McGee wrote: > > On Fri, Feb 07, 2014 at 11:15:15AM +0100, Daniel Vetter wrote: > > > On Fri, Feb 7, 2014 at 10:33 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > On Fri, Feb 07, 2014 at 10:03:33AM +0100, Daniel Vetter wrote: > > > >> The kernel will round it, so if we don't we'll have a spurious > > > >> mismatch. Happens on my machine here with 650-1300MHz range, where the > > > >> midpoint is 975. > > > >> > > > >> Cc: Jeff McGee <jeff.mcgee@intel.com> > > > >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > >> --- > > > >> tests/pm_rps.c | 3 +++ > > > >> 1 file changed, 3 insertions(+) > > > >> > > > >> diff --git a/tests/pm_rps.c b/tests/pm_rps.c > > > >> index 467038104ec6..27e758755e3f 100644 > > > >> --- a/tests/pm_rps.c > > > >> +++ b/tests/pm_rps.c > > > >> @@ -350,6 +350,9 @@ static void min_max_config(void (*check)(void)) > > > >> { > > > >> int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2; > > > >> > > > >> + /* hw (and so kernel) currently rounds to 50 MHz ... */ > > > > > > > > s/rounds/truncates/ or if it really does round, you need to adjust the > > > > calculation. > > > > > > We just need to use something divisible by 50 so that the value we > > > write and the one we get match up. Whether it's truncating or rounding > > > doesn't matter really. > > > -Daniel > > > > Darn, I considered this possibility but forgot to account for it in the test. > > I think what I was going to do was to create another writeval variant > > which doesn't do read back matching check. This was because I didn't want to > > assume that all systems use a 50 Mhz frequency increment (do they all?). > > VLV sure doesn't. Well I've hacked around it for now. But I agree that an unchecked write-val for fmid would be the better approach. -Daniel
diff --git a/tests/pm_rps.c b/tests/pm_rps.c index 467038104ec6..27e758755e3f 100644 --- a/tests/pm_rps.c +++ b/tests/pm_rps.c @@ -350,6 +350,9 @@ static void min_max_config(void (*check)(void)) { int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2; + /* hw (and so kernel) currently rounds to 50 MHz ... */ + fmid = fmid / 50 * 50; + log("\nCheck original min and max...\n"); check();
The kernel will round it, so if we don't we'll have a spurious mismatch. Happens on my machine here with 650-1300MHz range, where the midpoint is 975. Cc: Jeff McGee <jeff.mcgee@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- tests/pm_rps.c | 3 +++ 1 file changed, 3 insertions(+)