Subject: touchpanel bug: even better patch
To: None <port-hpcarm@netbsd.org>
From: Emmanuel Dreyfus <manu@netbsd.org>
List: port-hpcarm
Date: 09/09/2002 23:20:24
I modified j720ssp.c in an attempt to disable tp interrupt once the
screen is touched and then poll the tp using a callout. When the callout
function detects the screen is not touched anymore, it disable the
polling and restore the interrupt.

The result is a much better compromise between interupt overhead and
move accuracy. I also raised the poll frequency to 25 Hz, it's more
comfortable than 10 Hz.

One last issue before converting the keyboard handling to this: If I run
top with a null delay (In top, hit s then request 0 seconds), I can see
the time spent in interrput raise to about 80% when I touch the screen.
I wonder if j720tp_disable really does its job.

Oh, and one more bug to fix: ddb is not usable for more than one
command. you quickly get a crash.

Index: j720ssp.c
===================================================================
RCS file: /cvsroot/syssrc/sys/arch/hpcarm/dev/j720ssp.c,v
retrieving revision 1.8
diff -U4 -r1.8 j720ssp.c
--- j720ssp.c   2002/07/22 20:55:48     1.8
+++ j720ssp.c   2002/09/09 21:18:25
@@ -114,17 +114,16 @@
        struct tpcalib_softc sc_tpcalib;
 
        void *sc_kbdsi;
        void *sc_tpsi;
-       struct callout sc_tptimeout;
+       struct callout sc_tppoll;
        int sc_enabled;
 };
 
 int j720kbd_intr(void *);
 int j720tp_intr(void *);
 void j720kbdsoft(void *);
-void j720tpsoft(void *);
-void j720tp_timeout(void *);
+void j720tppoll(void *);
 int j720lcdparam(void *, int, long, void *);
 static void j720kbd_read(struct j720ssp_softc *, char *);
 static int j720ssp_readwrite(struct j720ssp_softc *, int, int, int *);
 
@@ -277,12 +276,11 @@
                    (caddr_t)&j720_default_calib, 0, 0);
        }
 
        j720tp_disable(sc);
-       callout_init(&sc->sc_tptimeout);
+       callout_init(&sc->sc_tppoll);
 
        /* Setup touchpad interrupt */
-       sc->sc_tpsi = softintr_establish(IPL_SOFTCLOCK, j720tpsoft, sc);
        sa11x0_intr_establish(0, 9, 1, IPL_BIO, j720tp_intr, sc);
 
        /* LCD control is on the same bus */
        config_hook(CONFIG_HOOK_SET, CONFIG_HOOK_BRIGHTNESS,
@@ -381,9 +379,12 @@
        struct j720ssp_softc *sc = arg;
 
        bus_space_write_4(sc->sc_iot, sc->sc_gpioh, SAGPIO_EDR, 1 << 9);
 
-       softintr_schedule(sc->sc_tpsi);
+       if (!callout_pending(&sc->sc_tppoll)) {
+               j720tp_disable(sc);
+               callout_reset(&sc->sc_tppoll, hz / 25, j720tppoll, sc);
+       }
 
        return (1);
 }
 
@@ -465,13 +466,25 @@
 printf("j720kbd_read: error %x\n", data);
 }
 
 void
-j720tpsoft(void *arg)
+j720tppoll(void *arg)
 {
        struct j720ssp_softc *sc = arg;
        int buf[8], data, i, x, y;
 
+       /* 
+        * If touch panel is not touched anymore, 
+        * stop polling and re-enable interrupt 
+        */
+       if (bus_space_read_4(sc->sc_iot, 
+           sc->sc_gpioh, SAGPIO_PLR) & (1 << 9)) {
+               wsmouse_input(sc->sc_wsmousedev, 0, 0, 0, 0, 0);
+               callout_stop(&sc->sc_tppoll);
+               j720tp_enable(sc);
+               return;
+       }
+
        bus_space_write_4(sc->sc_iot, sc->sc_gpioh, SAGPIO_PCR,
0x2000000);
 
        /* send read touchpanel command */
        if (j720ssp_readwrite(sc, 1, 0x500, &data) < 0 ||
@@ -495,18 +508,18 @@
                buf[i + 3] |= buf[7] & 0x300;
                buf[7] >>= 2;
        }
 #if 0
-       printf("j720tpsoft: %d %d %d  %d %d %d\n", buf[0], buf[1],
buf[2],
+       printf("j720tppoll: %d %d %d  %d %d %d\n", buf[0], buf[1],
buf[2],
            buf[3], buf[4], buf[5]);
 #endif
 
        /* XXX buf[1], buf[2], ... should also be used */
        tpcalib_trans(&sc->sc_tpcalib, buf[1], buf[4], &x, &y);
        wsmouse_input(sc->sc_wsmousedev, 1, x, y, 0,
            WSMOUSE_INPUT_ABSOLUTE_X | WSMOUSE_INPUT_ABSOLUTE_Y);
 
-       callout_reset(&sc->sc_tptimeout, hz / 10, j720tp_timeout, sc);
+       callout_reset(&sc->sc_tppoll, hz / 25, j720tppoll, sc);
 
        return;
 
 out:
@@ -516,27 +529,9 @@
        /* reset SSP */
        bus_space_write_4(sc->sc_iot, sc->sc_ssph, SASSP_CR0, 0x307);
        delay(100);
        bus_space_write_4(sc->sc_iot, sc->sc_ssph, SASSP_CR0, 0x387);
-       printf("j720tpsoft: error %x\n", data);
-}
-
-void
-j720tp_timeout(void *arg)
-{
-       struct j720ssp_softc *sc = arg;
-
-#if 0
-       /* XXX I don't this this is necessary (untested) */
-       if (bus_space_read_4(sc->sc_iot, sc->sc_gpioh, SAGPIO_PLR) &
-           (1 << 9)) {
-               /* Touchpad is still pressed */
-               callout_reset(&sc->sc_tptimeout, hz / 10,
j720tp_timeout, sc);
-               return;
-       }
-#endif
-
-       wsmouse_input(sc->sc_wsmousedev, 0, 0, 0, 0, 0);
+       printf("j720tppoll: error %x\n", data);
 }
 
 static int
 j720tp_enable(void *arg) {

-- 
Emmanuel Dreyfus.
"Le 80x86 n'est pas si complexe - il n'a simplement pas de sens"
(Mike Johnson, responsable de la conception x86 chez AMD) 
manu@netbsd.org