On Fri, Dec 11, 2020 at 10:01:35AM +0100, Alexandre Ratchov wrote:
> On Fri, Dec 11, 2020 at 09:07:45AM +0100, Marcus Glocker wrote:
> >
> > After doing some deeper analyzes in to asmc_wait() I agree to that.
> > Something seems to go fundamental wrong there. In every asmc_update()
> > execution, I can see asmc_wait() timeout 9 times, always on the
> > ASMC_ACCEPT check. That means out of ~48ms to execute asmc_update(),
> > we spend 45ms waiting for timeouts!
> >
> > When this can be fixed, I guess we should be fine with the remaining
> > ~3ms to execute asmc_update().
> >
>
> Even 3ms seems the wrong order of magnitude; that's ~3000000 CPU
> cycles on a 1GHz processor. This is 3 usb frames, so this will still
> hurt audio in low-latency corner cases.
Right. I'm happy to continue the discussion on what could be the right
approach to get there. But first I would like to remove the unnecessary
delays/timeouts we currently see in asmc(4). Therefore ...
... with this diff, which is basically doing the same as the Linux
driver, I can get the asmc_update() function down to 2-3ms, without
getting any timeout in the update code. This allows me to stream
uaudio(4), run uvideo(4), and compile a kernel with '-j 4' in
parallel on my iMac11,3 without seeing freezes.
Testers, comments, OKs?
Index: sys/dev/acpi/asmc.c
===================================================================
RCS file: /cvs/src/sys/dev/acpi/asmc.c,v
retrieving revision 1.2
diff -u -p -u -p -r1.2 asmc.c
--- sys/dev/acpi/asmc.c 13 Sep 2020 14:11:28 -0000 1.2
+++ sys/dev/acpi/asmc.c 11 Dec 2020 19:10:22 -0000
@@ -58,6 +58,8 @@
#define ASMC_MAXLIGHT 2 /* left and right light sensor */
#define ASMC_MAXMOTION 3 /* x y z axis motion sensors */
+#define ASMC_DELAY_LOOP 200 /* ASMC_DELAY_LOOP * 10us = 2ms */
+
struct asmc_prod {
const char *pr_name;
uint8_t pr_light;
@@ -434,36 +436,41 @@ asmc_status(struct asmc_softc *sc)
}
static int
-asmc_wait(struct asmc_softc *sc, uint8_t mask, uint8_t val)
+asmc_write(struct asmc_softc *sc, uint8_t off, uint8_t val)
{
int i;
+ uint8_t status;
- for (i = 0; i < 500; i++) { /* wait up to 5 ms */
- if ((bus_space_read_1(sc->sc_iot, sc->sc_ioh, ASMC_COMMAND) &
- mask) == val)
+ bus_space_write_1(sc->sc_iot, sc->sc_ioh, off, val);
+ for (i = 0; i < ASMC_DELAY_LOOP; i++) {
+ status = bus_space_read_1(sc->sc_iot, sc->sc_ioh, ASMC_COMMAND);
+ if (status & ASMC_IBF)
+ continue;
+ if (status & ASMC_ACCEPT)
return 0;
delay(10);
+ bus_space_write_1(sc->sc_iot, sc->sc_ioh, off, val);
}
- return ETIMEDOUT;
-}
-static int
-asmc_write(struct asmc_softc *sc, uint8_t off, uint8_t val)
-{
- if (asmc_wait(sc, ASMC_IBF, 0))
- return 1;
- bus_space_write_1(sc->sc_iot, sc->sc_ioh, off, val);
- if (asmc_wait(sc, ASMC_ACCEPT, ASMC_ACCEPT))
- return 1;
- return 0;
+ return ETIMEDOUT;
}
static int
asmc_read(struct asmc_softc *sc, uint8_t off, uint8_t *buf)
{
- if (asmc_wait(sc, ASMC_OBF, ASMC_OBF))
- return 1;
+ int i;
+ uint8_t status;
+
+ for (i = 0; i < ASMC_DELAY_LOOP; i++) {
+ status = bus_space_read_1(sc->sc_iot, sc->sc_ioh, ASMC_COMMAND);
+ if (status & ASMC_OBF)
+ break;
+ delay(10);
+ }
+ if (i == ASMC_DELAY_LOOP)
+ return ETIMEDOUT;
*buf = bus_space_read_1(sc->sc_iot, sc->sc_ioh, off);
+
return 0;
}
