* [PATCH] ACPI: implement acpi_os_get_timer() according the spec @ 2013-05-13 10:27 Mika Westerberg 2013-05-13 11:38 ` Rafael J. Wysocki 0 siblings, 1 reply; 7+ messages in thread From: Mika Westerberg @ 2013-05-13 10:27 UTC (permalink / raw) To: linux-acpi; +Cc: Rafael J. Wysocki, Len Brown, Mika Westerberg, linux-kernel ACPI Timer() opcode should return monotonically increasing clock with 100ns granularity. Implement this with the help of ktime_get(). Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/acpi/osl.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 586e7e9..2a22170 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -835,19 +835,7 @@ void acpi_os_stall(u32 us) */ u64 acpi_os_get_timer(void) { - static u64 t; - -#ifdef CONFIG_HPET - /* TBD: use HPET if available */ -#endif - -#ifdef CONFIG_X86_PM_TIMER - /* TBD: default to PM timer if HPET was not available */ -#endif - if (!t) - printk(KERN_ERR PREFIX "acpi_os_get_timer() TBD\n"); - - return ++t; + return ktime_to_ns(ktime_get()) / 100; } acpi_status acpi_os_read_port(acpi_io_address port, u32 * value, u32 width) -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: implement acpi_os_get_timer() according the spec 2013-05-13 10:27 [PATCH] ACPI: implement acpi_os_get_timer() according the spec Mika Westerberg @ 2013-05-13 11:38 ` Rafael J. Wysocki 2013-05-13 11:44 ` Mika Westerberg 0 siblings, 1 reply; 7+ messages in thread From: Rafael J. Wysocki @ 2013-05-13 11:38 UTC (permalink / raw) To: Mika Westerberg; +Cc: linux-acpi, Rafael J. Wysocki, Len Brown, linux-kernel On Monday, May 13, 2013 01:27:51 PM Mika Westerberg wrote: > ACPI Timer() opcode should return monotonically increasing clock with 100ns > granularity. Implement this with the help of ktime_get(). > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> That looks reasobable. Have you tested it? Rafael > --- > drivers/acpi/osl.c | 14 +------------- > 1 file changed, 1 insertion(+), 13 deletions(-) > > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c > index 586e7e9..2a22170 100644 > --- a/drivers/acpi/osl.c > +++ b/drivers/acpi/osl.c > @@ -835,19 +835,7 @@ void acpi_os_stall(u32 us) > */ > u64 acpi_os_get_timer(void) > { > - static u64 t; > - > -#ifdef CONFIG_HPET > - /* TBD: use HPET if available */ > -#endif > - > -#ifdef CONFIG_X86_PM_TIMER > - /* TBD: default to PM timer if HPET was not available */ > -#endif > - if (!t) > - printk(KERN_ERR PREFIX "acpi_os_get_timer() TBD\n"); > - > - return ++t; > + return ktime_to_ns(ktime_get()) / 100; > } > > acpi_status acpi_os_read_port(acpi_io_address port, u32 * value, u32 width) > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: implement acpi_os_get_timer() according the spec 2013-05-13 11:38 ` Rafael J. Wysocki @ 2013-05-13 11:44 ` Mika Westerberg 2013-05-20 10:25 ` Mika Westerberg 0 siblings, 1 reply; 7+ messages in thread From: Mika Westerberg @ 2013-05-13 11:44 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: linux-acpi, Rafael J. Wysocki, Len Brown, linux-kernel On Mon, May 13, 2013 at 01:38:46PM +0200, Rafael J. Wysocki wrote: > On Monday, May 13, 2013 01:27:51 PM Mika Westerberg wrote: > > ACPI Timer() opcode should return monotonically increasing clock with 100ns > > granularity. Implement this with the help of ktime_get(). > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > That looks reasobable. Have you tested it? Very lightly. Basically I added some debug printks() between two successsive calls of Timer() and it seemed like it returned correct time. It is certainly better than returning t+1 every time Timer() is called :) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: implement acpi_os_get_timer() according the spec 2013-05-13 11:44 ` Mika Westerberg @ 2013-05-20 10:25 ` Mika Westerberg 2013-05-20 11:07 ` Rafael J. Wysocki 0 siblings, 1 reply; 7+ messages in thread From: Mika Westerberg @ 2013-05-20 10:25 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: linux-acpi, Rafael J. Wysocki, Len Brown, linux-kernel On Mon, May 13, 2013 at 02:44:32PM +0300, Mika Westerberg wrote: > On Mon, May 13, 2013 at 01:38:46PM +0200, Rafael J. Wysocki wrote: > > On Monday, May 13, 2013 01:27:51 PM Mika Westerberg wrote: > > > ACPI Timer() opcode should return monotonically increasing clock with 100ns > > > granularity. Implement this with the help of ktime_get(). > > > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > > That looks reasobable. Have you tested it? > > Very lightly. Basically I added some debug printks() between two > successsive calls of Timer() and it seemed like it returned correct time. > > It is certainly better than returning t+1 every time Timer() is called :) I did somewhat better test for this. I added following ASL code: ... Store(Timer, Local1) Sleep(10) Divide(Subtract(Timer, Local1), 10000,, Local1) Sleep(Local1) Store(Timer, Local1) Sleep(200) Divide(Subtract(Timer, Local1), 10000,, Local1) Sleep(Local1) Store(Timer, Local1) Sleep(1300) Divide(Subtract(Timer, Local1), 10000,, Local1) Sleep(Local1) The second sleep should be pretty close to the first one. Without this patch I get: [ 11.488100] ACPI: acpi_os_get_timer() TBD [ 11.492150] ACPI: Sleep(10) [ 11.502993] ACPI: Sleep(0) [ 11.506315] ACPI: Sleep(200) [ 11.706237] ACPI: Sleep(0) [ 11.709550] ACPI: Sleep(1300) [ 13.008929] ACPI: Sleep(0) With the patch applied I get: [ 11.486786] ACPI: Sleep(10) [ 11.499029] ACPI: Sleep(12) [ 11.512350] ACPI: Sleep(200) [ 11.712282] ACPI: Sleep(200) [ 11.912170] ACPI: Sleep(1300) [ 13.211577] ACPI: Sleep(1300) The above looks much more correct to me. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: implement acpi_os_get_timer() according the spec 2013-05-20 10:25 ` Mika Westerberg @ 2013-05-20 11:07 ` Rafael J. Wysocki 2013-05-20 11:28 ` [PATCH v2] " Mika Westerberg 0 siblings, 1 reply; 7+ messages in thread From: Rafael J. Wysocki @ 2013-05-20 11:07 UTC (permalink / raw) To: Mika Westerberg; +Cc: linux-acpi, Rafael J. Wysocki, Len Brown, linux-kernel On Monday, May 20, 2013 01:25:30 PM Mika Westerberg wrote: > On Mon, May 13, 2013 at 02:44:32PM +0300, Mika Westerberg wrote: > > On Mon, May 13, 2013 at 01:38:46PM +0200, Rafael J. Wysocki wrote: > > > On Monday, May 13, 2013 01:27:51 PM Mika Westerberg wrote: > > > > ACPI Timer() opcode should return monotonically increasing clock with 100ns > > > > granularity. Implement this with the help of ktime_get(). > > > > > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > > > > That looks reasobable. Have you tested it? > > > > Very lightly. Basically I added some debug printks() between two > > successsive calls of Timer() and it seemed like it returned correct time. > > > > It is certainly better than returning t+1 every time Timer() is called :) > > I did somewhat better test for this. I added following ASL code: > > ... > Store(Timer, Local1) > Sleep(10) > Divide(Subtract(Timer, Local1), 10000,, Local1) > Sleep(Local1) > > Store(Timer, Local1) > Sleep(200) > Divide(Subtract(Timer, Local1), 10000,, Local1) > Sleep(Local1) > > Store(Timer, Local1) > Sleep(1300) > Divide(Subtract(Timer, Local1), 10000,, Local1) > Sleep(Local1) > > The second sleep should be pretty close to the first one. > > Without this patch I get: > > [ 11.488100] ACPI: acpi_os_get_timer() TBD > [ 11.492150] ACPI: Sleep(10) > [ 11.502993] ACPI: Sleep(0) > [ 11.506315] ACPI: Sleep(200) > [ 11.706237] ACPI: Sleep(0) > [ 11.709550] ACPI: Sleep(1300) > [ 13.008929] ACPI: Sleep(0) > > With the patch applied I get: > > [ 11.486786] ACPI: Sleep(10) > [ 11.499029] ACPI: Sleep(12) > [ 11.512350] ACPI: Sleep(200) > [ 11.712282] ACPI: Sleep(200) > [ 11.912170] ACPI: Sleep(1300) > [ 13.211577] ACPI: Sleep(1300) > > The above looks much more correct to me. That demostrates the problem nicely. Can you please add the above info to the patch changelog? Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] ACPI: implement acpi_os_get_timer() according the spec 2013-05-20 11:07 ` Rafael J. Wysocki @ 2013-05-20 11:28 ` Mika Westerberg 2013-05-23 7:27 ` [PATCH v3] " Mika Westerberg 0 siblings, 1 reply; 7+ messages in thread From: Mika Westerberg @ 2013-05-20 11:28 UTC (permalink / raw) To: linux-kernel; +Cc: Rafael J. Wysocki, Len Brown, linux-acpi, Mika Westerberg ACPI Timer() opcode should return monotonically increasing clock with 100ns granularity according the ACPI 5.0 spec. Testing the current Timer() implementation with following ASL code (and an additional debug print in acpi_os_sleep() to get the sleep times dumped out to dmesg): // Test: 10ms Store(Timer, Local1) Sleep(10) Divide(Subtract(Timer, Local1), 10000,, Local1) Sleep(Local1) // Test: 200ms Store(Timer, Local1) Sleep(200) Divide(Subtract(Timer, Local1), 10000,, Local1) Sleep(Local1) // Test 1300ms Store(Timer, Local1) Sleep(1300) Divide(Subtract(Timer, Local1), 10000,, Local1) Sleep(Local1) The second sleep value is calculated using Timer(). If the implementation is good enough we should be able to get the second value pretty close to the first. However, the current Timer() gives pretty bad sleep times: [ 11.488100] ACPI: acpi_os_get_timer() TBD [ 11.492150] ACPI: Sleep(10) [ 11.502993] ACPI: Sleep(0) [ 11.506315] ACPI: Sleep(200) [ 11.706237] ACPI: Sleep(0) [ 11.709550] ACPI: Sleep(1300) [ 13.008929] ACPI: Sleep(0) Fix this with the help of ktime_get(). Once the fix is applied and run against the same ASL code we get: [ 11.486786] ACPI: Sleep(10) [ 11.499029] ACPI: Sleep(12) [ 11.512350] ACPI: Sleep(200) [ 11.712282] ACPI: Sleep(200) [ 11.912170] ACPI: Sleep(1300) [ 13.211577] ACPI: Sleep(1300) That is much more closer to the values we expected. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/acpi/osl.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index e721863..cdda7cf 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -835,19 +835,7 @@ void acpi_os_stall(u32 us) */ u64 acpi_os_get_timer(void) { - static u64 t; - -#ifdef CONFIG_HPET - /* TBD: use HPET if available */ -#endif - -#ifdef CONFIG_X86_PM_TIMER - /* TBD: default to PM timer if HPET was not available */ -#endif - if (!t) - printk(KERN_ERR PREFIX "acpi_os_get_timer() TBD\n"); - - return ++t; + return ktime_to_ns(ktime_get()) / 100; } acpi_status acpi_os_read_port(acpi_io_address port, u32 * value, u32 width) -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3] ACPI: implement acpi_os_get_timer() according the spec 2013-05-20 11:28 ` [PATCH v2] " Mika Westerberg @ 2013-05-23 7:27 ` Mika Westerberg 0 siblings, 0 replies; 7+ messages in thread From: Mika Westerberg @ 2013-05-23 7:27 UTC (permalink / raw) To: linux-kernel; +Cc: Rafael J. Wysocki, Len Brown, linux-acpi, Mika Westerberg ACPI Timer() opcode should return monotonically increasing clock with 100ns granularity according the ACPI 5.0 spec. Testing the current Timer() implementation with following ASL code (and an additional debug print in acpi_os_sleep() to get the sleep times dumped out to dmesg): // Test: 10ms Store(Timer, Local1) Sleep(10) Divide(Subtract(Timer, Local1), 10000,, Local1) Sleep(Local1) // Test: 200ms Store(Timer, Local1) Sleep(200) Divide(Subtract(Timer, Local1), 10000,, Local1) Sleep(Local1) // Test 1300ms Store(Timer, Local1) Sleep(1300) Divide(Subtract(Timer, Local1), 10000,, Local1) Sleep(Local1) The second sleep value is calculated using Timer(). If the implementation is good enough we should be able to get the second value pretty close to the first. However, the current Timer() gives pretty bad sleep times: [ 11.488100] ACPI: acpi_os_get_timer() TBD [ 11.492150] ACPI: Sleep(10) [ 11.502993] ACPI: Sleep(0) [ 11.506315] ACPI: Sleep(200) [ 11.706237] ACPI: Sleep(0) [ 11.709550] ACPI: Sleep(1300) [ 13.008929] ACPI: Sleep(0) Fix this with the help of ktime_get(). Once the fix is applied and run against the same ASL code we get: [ 11.486786] ACPI: Sleep(10) [ 11.499029] ACPI: Sleep(12) [ 11.512350] ACPI: Sleep(200) [ 11.712282] ACPI: Sleep(200) [ 11.912170] ACPI: Sleep(1300) [ 13.211577] ACPI: Sleep(1300) That is much more closer to the values we expected. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- Difference to the previous version is that now we use do_div() instead so that i386 build won't break. This was found by kbuild test robot. In addition I tested this on both i386 and x86_64. drivers/acpi/osl.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index e721863..c290769 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -835,19 +835,9 @@ void acpi_os_stall(u32 us) */ u64 acpi_os_get_timer(void) { - static u64 t; - -#ifdef CONFIG_HPET - /* TBD: use HPET if available */ -#endif - -#ifdef CONFIG_X86_PM_TIMER - /* TBD: default to PM timer if HPET was not available */ -#endif - if (!t) - printk(KERN_ERR PREFIX "acpi_os_get_timer() TBD\n"); - - return ++t; + u64 time_ns = ktime_to_ns(ktime_get()); + do_div(time_ns, 100); + return time_ns; } acpi_status acpi_os_read_port(acpi_io_address port, u32 * value, u32 width) -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-05-23 7:27 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-05-13 10:27 [PATCH] ACPI: implement acpi_os_get_timer() according the spec Mika Westerberg 2013-05-13 11:38 ` Rafael J. Wysocki 2013-05-13 11:44 ` Mika Westerberg 2013-05-20 10:25 ` Mika Westerberg 2013-05-20 11:07 ` Rafael J. Wysocki 2013-05-20 11:28 ` [PATCH v2] " Mika Westerberg 2013-05-23 7:27 ` [PATCH v3] " Mika Westerberg
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.