From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754613AbaHUL3K (ORCPT ); Thu, 21 Aug 2014 07:29:10 -0400 Received: from cantor2.suse.de ([195.135.220.15]:36977 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754511AbaHUL3F (ORCPT ); Thu, 21 Aug 2014 07:29:05 -0400 Date: Thu, 21 Aug 2014 13:29:03 +0200 Message-ID: From: Takashi Iwai To: Dmitry Torokhov Cc: Andreas Mohr , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, Vojtech Pavlik , Jiri Kosina Subject: Re: [PATCH 1/2] SOUND: kill gameport bits In-Reply-To: References: <20140820024638.GA25240@rhlx01.hs-esslingen.de> <20140820051815.GA1109@core.coreip.homeip.net> <20140820063130.GA11226@core.coreip.homeip.net> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org At Wed, 20 Aug 2014 09:05:58 +0200, Takashi Iwai wrote: > > > > > > Also, I'm left wondering why e.g. my Athlon XP system (a very popular > > > > > choice for longer times) would be affected by Cpufreq... > > > > > And there are no details on how exactly cpufreq is a problem or how this > > > > > timing issue could be fixed... > > > > > > > > If you take a look at gameport_measure_speed() in gameport.c you will see that > > > > it counts cycles for timing, which obviously does not work that well when CPU > > > > frequency changes. > > > > > > > > The bugs have been opened in bugzilla/reported on lists ages ago but nobody > > > > stepped up to fix that. > > > > > > Hm, can't we just use the standard ktime for measuring the time diff? > > > > We could use high-res timers, if they are available. Are they available on such > > old hardware and are they sufficiently fast to provide needed timings? I > > definitely do not have any hardware to est with. > > The boards aren't necessarily bound with the old hardware. PCI boards > run fine on the modern machines if they still have a PCI slot (how > lucky). And, the highres timer itself isn't so new... I did a quick hack and it seems working on my box. The patch is below. thanks, Takashi -- 8< -- From: Takashi Iwai Subject: [PATCH] Input: joystick - Use ktime for measuring timing The current codes in gameport and analog joystick drivers for the time accounting have a long-standing problem when the system is running with CPU freq; since the timing is measured via TSC or sample counter, the calculation isn't reliable. In this patch, as a simple fix, use the standard ktime to measure the timing. In case where no high resolution timer is available, use_ktime bool option is provided to both modules. Setting use_ktime=false switches to the old methods. Signed-off-by: Takashi Iwai --- drivers/input/gameport/gameport.c | 38 ++++++++++++++++++++- drivers/input/joystick/analog.c | 70 ++++++++++++++++++++++++++++----------- 2 files changed, 88 insertions(+), 20 deletions(-) diff --git a/drivers/input/gameport/gameport.c b/drivers/input/gameport/gameport.c index 24c41ba7d4e0..48d91f12e397 100644 --- a/drivers/input/gameport/gameport.c +++ b/drivers/input/gameport/gameport.c @@ -23,6 +23,7 @@ #include #include /* HZ */ #include +#include /*#include */ @@ -30,6 +31,9 @@ MODULE_AUTHOR("Vojtech Pavlik "); MODULE_DESCRIPTION("Generic gameport layer"); MODULE_LICENSE("GPL"); +static bool use_ktime = true; +module_param(use_ktime, bool, 0400); + /* * gameport_mutex protects entire gameport subsystem and is taken * every time gameport port or driver registrered or unregistered. @@ -76,6 +80,36 @@ static unsigned int get_time_pit(void) static int gameport_measure_speed(struct gameport *gameport) { + unsigned int i, t, tx; + u64 t1, t2; + unsigned long flags; + + if (gameport_open(gameport, NULL, GAMEPORT_MODE_RAW)) + return 0; + + tx = ~0; + + for (i = 0; i < 50; i++) { + local_irq_save(flags); + t1 = ktime_get_ns(); + for (t = 0; t < 50; t++) + gameport_read(gameport); + t2 = ktime_get_ns(); + local_irq_restore(flags); + udelay(i * 10); + if (t2 - t1 < tx) + tx = t2 - t1; + } + + gameport_close(gameport); + t = 1000000 * 50; + if (tx) + t /= tx; + return t; +} + +static int old_gameport_measure_speed(struct gameport *gameport) +{ #if defined(__i386__) unsigned int i, t, t1, t2, t3, tx; @@ -521,7 +555,9 @@ static void gameport_add_port(struct gameport *gameport) if (gameport->parent) gameport->parent->child = gameport; - gameport->speed = gameport_measure_speed(gameport); + gameport->speed = use_ktime ? + gameport_measure_speed(gameport) : + old_gameport_measure_speed(gameport); list_add_tail(&gameport->node, &gameport_list); diff --git a/drivers/input/joystick/analog.c b/drivers/input/joystick/analog.c index ab0fdcd36e18..723276d40b58 100644 --- a/drivers/input/joystick/analog.c +++ b/drivers/input/joystick/analog.c @@ -36,6 +36,7 @@ #include #include #include +#include #define DRIVER_DESC "Analog joystick and gamepad driver" @@ -43,6 +44,9 @@ MODULE_AUTHOR("Vojtech Pavlik "); MODULE_DESCRIPTION(DRIVER_DESC); MODULE_LICENSE("GPL"); +static bool use_ktime = true; +module_param(use_ktime, bool, 0400); + /* * Option parsing. */ @@ -171,6 +175,25 @@ static unsigned long analog_faketime = 0; #warning Precise timer not defined for this architecture. #endif +static inline u64 get_time(void) +{ + if (use_ktime) { + return ktime_get_ns(); + } else { + unsigned int x; + GET_TIME(x); + return x; + } +} + +static inline unsigned int delta(u64 x, u64 y) +{ + if (use_ktime) + return y - x; + else + return DELTA((unsigned int)x, (unsigned int)y); +} + /* * analog_decode() decodes analog joystick data and reports input events. */ @@ -226,7 +249,8 @@ static void analog_decode(struct analog *analog, int *axes, int *initial, int bu static int analog_cooked_read(struct analog_port *port) { struct gameport *gameport = port->gameport; - unsigned int time[4], start, loop, now, loopout, timeout; + u64 time[4], start, loop, now; + unsigned int loopout, timeout; unsigned char data[4], this, last; unsigned long flags; int i, j; @@ -236,7 +260,7 @@ static int analog_cooked_read(struct analog_port *port) local_irq_save(flags); gameport_trigger(gameport); - GET_TIME(now); + now = get_time(); local_irq_restore(flags); start = now; @@ -249,16 +273,16 @@ static int analog_cooked_read(struct analog_port *port) local_irq_disable(); this = gameport_read(gameport) & port->mask; - GET_TIME(now); + now = get_time(); local_irq_restore(flags); - if ((last ^ this) && (DELTA(loop, now) < loopout)) { + if ((last ^ this) && (delta(loop, now) < loopout)) { data[i] = last ^ this; time[i] = now; i++; } - } while (this && (i < 4) && (DELTA(start, now) < timeout)); + } while (this && (i < 4) && (delta(start, now) < timeout)); this <<= 4; @@ -266,7 +290,7 @@ static int analog_cooked_read(struct analog_port *port) this |= data[i]; for (j = 0; j < 4; j++) if (data[i] & (1 << j)) - port->axes[j] = (DELTA(start, time[i]) << ANALOG_FUZZ_BITS) / port->loop; + port->axes[j] = (delta(start, time[i]) << ANALOG_FUZZ_BITS) / port->loop; } return -(this != port->mask); @@ -365,31 +389,39 @@ static void analog_close(struct input_dev *dev) static void analog_calibrate_timer(struct analog_port *port) { struct gameport *gameport = port->gameport; - unsigned int i, t, tx, t1, t2, t3; + unsigned int i, t, tx; + u64 t1, t2, t3; unsigned long flags; - local_irq_save(flags); - GET_TIME(t1); + if (use_ktime) { + port->speed = 1000000; + } else { + local_irq_save(flags); + t1 = get_time(); #ifdef FAKE_TIME - analog_faketime += 830; + analog_faketime += 830; #endif - mdelay(1); - GET_TIME(t2); - GET_TIME(t3); - local_irq_restore(flags); + mdelay(1); + t2 = get_time(); + t3 = get_time(); + local_irq_restore(flags); - port->speed = DELTA(t1, t2) - DELTA(t2, t3); + port->speed = delta(t1, t2) - delta(t2, t3); + } tx = ~0; for (i = 0; i < 50; i++) { local_irq_save(flags); - GET_TIME(t1); - for (t = 0; t < 50; t++) { gameport_read(gameport); GET_TIME(t2); } - GET_TIME(t3); + t1 = get_time(); + for (t = 0; t < 50; t++) { + gameport_read(gameport); + t2 = get_time(); + } + t3 = get_time(); local_irq_restore(flags); udelay(i); - t = DELTA(t1, t2) - DELTA(t2, t3); + t = delta(t1, t2) - delta(t2, t3); if (t < tx) tx = t; } -- 2.0.4