[src/trunk]: src/sys/dev/sbus fix audiomp bugs:

[mrg <mrg%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/a4cde2aefae0
branches:  trunk
changeset: 828317:a4cde2aefae0
user:      mrg <mrg%NetBSD.org@localhost>
date:      Fri Dec 08 07:47:00 2017 +0000

description:
fix audiomp bugs:
- switch from tsleep/wakeup to condvar
- fix locking in a bunch of places.  there were several locking
  against myself issues.

also:
- don't let dbri_process_interrupt_buffer() loop more than once
  over the array of intrs.

this fixes hangs when using audio on ss20 in -current, but does
not make audio work.  it eventually times out with eg:

dbri0: switching to control mode timed out (0 f6)

and may leave a sample in the audio buffer repeating.

diffstat:

 sys/dev/sbus/dbri.c    |  62 +++++++++++++++++++++++++++++++++----------------
 sys/dev/sbus/dbrivar.h |   5 +++-
 2 files changed, 46 insertions(+), 21 deletions(-)

diffs (253 lines):

diff -r b2367e394ea8 -r a4cde2aefae0 sys/dev/sbus/dbri.c
--- a/sys/dev/sbus/dbri.c       Fri Dec 08 05:22:23 2017 +0000
+++ b/sys/dev/sbus/dbri.c       Fri Dec 08 07:47:00 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: dbri.c,v 1.35 2013/10/19 21:00:32 mrg Exp $    */
+/*     $NetBSD: dbri.c,v 1.36 2017/12/08 07:47:00 mrg Exp $    */
 
 /*
  * Copyright (C) 1997 Rudolf Koenig (rfkoenig%immd4.informatik.uni-erlangen.de@localhost)
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: dbri.c,v 1.35 2013/10/19 21:00:32 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: dbri.c,v 1.36 2017/12/08 07:47:00 mrg Exp $");
 
 #include "audio.h"
 #if NAUDIO > 0
@@ -367,6 +367,10 @@
        mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_NONE);
        mutex_init(&sc->sc_intr_lock, MUTEX_DEFAULT, IPL_SCHED);
 
+#ifndef DBRI_SPIN
+       cv_init(&sc->sc_cv, "dbricv");
+#endif
+
        bus_intr_establish(sa->sa_bustag, sa->sa_pri, IPL_SCHED, dbri_intr,
            sc);
 
@@ -444,8 +448,11 @@
 {
        struct dbri_softc *sc = device_private(dev);
 
-       if (sc->sc_init_done != 0)
+       mutex_spin_enter(&sc->sc_intr_lock);
+       if (sc->sc_init_done != 0) {
+               mutex_spin_exit(&sc->sc_intr_lock);
                return 0;
+       }
 
        sc->sc_init_done = 1;
 
@@ -453,14 +460,19 @@
        if (mmcodec_init(sc) == -1) {
                printf("%s: no codec detected, aborting\n",
                    device_xname(dev));
+               mutex_spin_exit(&sc->sc_intr_lock);
                return 0;
        }
+       mutex_spin_exit(&sc->sc_intr_lock);
 
        /* Attach ourselves to the high level audio interface */
        audio_attach_mi(&dbri_hw_if, sc, sc->sc_dev);
 
        /* power down until open() */
+       mutex_spin_enter(&sc->sc_intr_lock);
        dbri_set_power(sc, 0);
+       mutex_spin_exit(&sc->sc_intr_lock);
+
        return 0;
 }
 
@@ -532,6 +544,8 @@
        bus_addr_t dmaaddr;
        int n;
 
+       KASSERT(mutex_owned(sc->sc_intr_lock));
+
        dbri_reset(sc);
 
        cmd = dbri_command_lock(sc);
@@ -562,6 +576,7 @@
        *(cmd++) = dmaaddr;
 
        dbri_command_send(sc, cmd);
+
        return (0);
 }
 
@@ -603,7 +618,7 @@
        bus_space_tag_t iot = sc->sc_iot;
        int maxloops = 1000000;
 
-       mutex_spin_enter(&sc->sc_intr_lock);
+       KASSERT(mutex_owned(sc->sc_intr_lock));
 
        sc->sc_locked--;
 
@@ -641,8 +656,6 @@
                }
        }
 
-       mutex_spin_exit(&sc->sc_intr_lock);
-
        return;
 }
 
@@ -650,6 +663,9 @@
 dbri_process_interrupt_buffer(struct dbri_softc *sc)
 {
        int32_t i;
+       int orig_irqp = sc->sc_irqp;
+
+       KASSERT(mutex_owned(sc->sc_intr_lock));
 
        while ((i = sc->sc_dma->intr[sc->sc_irqp]) != 0) {
                sc->sc_dma->intr[sc->sc_irqp] = 0;
@@ -661,6 +677,10 @@
                        sc->sc_irqp++;
 
                dbri_process_interrupt(sc, i);
+
+               /* don't loop more than once. */
+               if (orig_irqp == sc->sc_irqp)
+                       break;
        }
 
        return;
@@ -688,6 +708,7 @@
                int td;
                struct dbri_desc *dd;
 
+               DPRINTF("%s:%d tx complete\n", __func__, channel);
                td = sc->sc_pipe[channel].desc;
                dd = &sc->sc_desc[td];
 
@@ -696,7 +717,7 @@
                break;
        }
        case DBRI_INTR_FXDT:            /* fixed data change */
-               DPRINTF("dbri_intr: Fixed data change (%d: %x)\n", channel,
+               DPRINTF("%s:%d: Fixed data change: %x\n", __func__, channel,
                    val);
 #if 0
                printf("reg: %08x\n", sc->sc_mm.status);
@@ -706,8 +727,8 @@
                if (sc->sc_pipe[channel].prec)
                        *(sc->sc_pipe[channel].prec) = val;
 #ifndef DBRI_SPIN
-               DPRINTF("%s: wakeup %p\n", device_xname(sc->sc_dev), sc);
-               wakeup(sc);
+               DPRINTF("%s: cv_broadcast %p\n", device_xname(sc->sc_dev), sc);
+               cv_broadcast(&sc->sc_cv);
 #endif
                break;
        case DBRI_INTR_SBRI:
@@ -718,6 +739,7 @@
                int td;
                struct dbri_desc *dd;
 
+               DPRINTF("dbri_intr: buffer ready (%d)\n", channel);
                td = sc->sc_pipe[channel].desc;
                dd = &sc->sc_desc[td];
 
@@ -973,6 +995,8 @@
        int bail = 0;
 #if DBRI_SPIN
        int i;
+#else
+       int error;
 #endif
 
        /*
@@ -1034,8 +1058,12 @@
        }
 #else
        while (((sc->sc_mm.status & 0xe4) != 0x20) && (bail < 10)) {
-               DPRINTF("%s: tsleep %p\n", device_xname(sc->sc_dev), sc);
-               tsleep(sc, PCATCH | PZERO, "dbrifxdt", hz);
+               DPRINTF("%s: cv_wait_sig %p\n", device_xname(sc->sc_dev), sc);
+               error = cv_timedwait_sig(&sc->sc_cv, &sc->sc_intr_lock, hz);
+               if (error == EINTR) {
+                       DPRINTF("%s: interrupted\n", device_xname(sc->sc_dev));
+                       return -1;
+               }
                bail++;
        }
 #endif
@@ -1324,8 +1352,6 @@
        dd->callback = callback;
        dd->callback_args = callback_args;
 
-       mutex_spin_enter(&sc->sc_intr_lock);
-
        /* the pipe shouldn't be active */
        if (pipe_active(sc, pipe)) {
                aprint_error("pipe active (CDP)\n");
@@ -1360,8 +1386,6 @@
                DPRINTF("%s: starting DMA\n", __func__);
        }
 
-       mutex_spin_exit(&sc->sc_intr_lock);
-
        return;
 }
 
@@ -1421,8 +1445,6 @@
        dd->callback = callback;
        dd->callback_args = callback_args;
 
-       mutex_spin_enter(&sc->sc_intr_lock);
-
        /* the pipe shouldn't be active */
        if (pipe_active(sc, pipe)) {
                aprint_error("pipe active (CDP)\n");
@@ -1457,8 +1479,6 @@
                DPRINTF("%s: starting DMA\n", __func__);
        }
 
-       mutex_spin_exit(&sc->sc_intr_lock);
-
        return;
 }
 
@@ -2211,7 +2231,9 @@
 {
        struct dbri_softc *sc = device_private(self);
 
+       mutex_spin_enter(&sc->sc_intr_lock);
        dbri_set_power(sc, 0);
+       mutex_spin_exit(&sc->sc_intr_lock);
        return true;
 }
 
@@ -2226,8 +2248,8 @@
        if (sc->sc_playing) {
                volatile uint32_t *cmd;
 
+               mutex_spin_enter(&sc->sc_intr_lock);
                dbri_bring_up(sc);
-               mutex_spin_enter(&sc->sc_intr_lock);
                cmd = dbri_command_lock(sc);
                *(cmd++) = DBRI_CMD(DBRI_COMMAND_SDP,
                    0, sc->sc_pipe[4].sdp |
diff -r b2367e394ea8 -r a4cde2aefae0 sys/dev/sbus/dbrivar.h
--- a/sys/dev/sbus/dbrivar.h    Fri Dec 08 05:22:23 2017 +0000
+++ b/sys/dev/sbus/dbrivar.h    Fri Dec 08 07:47:00 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: dbrivar.h,v 1.13 2011/11/23 23:07:36 jmcneill Exp $    */
+/*     $NetBSD: dbrivar.h,v 1.14 2017/12/08 07:47:00 mrg Exp $ */
 
 /*
  * Copyright (C) 1997 Rudolf Koenig (rfkoenig%immd4.informatik.uni-erlangen.de@localhost)
@@ -169,6 +169,9 @@
 
        kmutex_t        sc_lock;
        kmutex_t        sc_intr_lock;
+#ifndef DBRI_SPIN
+       kcondvar_t      sc_cv;
+#endif
 };
 
 #define dbri_dma_off(member, elem)     \