Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/hdaudio hdafg(4): Do hotplug detection in kthread, n...



details:   https://anonhg.NetBSD.org/src/rev/43fffa2ceb03
branches:  trunk
changeset: 378076:43fffa2ceb03
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Tue Jul 18 13:35:57 2023 +0000

description:
hdafg(4): Do hotplug detection in kthread, not callout.

This can sometimes take a while (~1ms), and the logic to suspend the
callout on device suspend/resume was racy (PR kern/57322).

XXX pullup-8
XXX pullup-9
XXX pullup-10

diffstat:

 sys/dev/hdaudio/hdafg.c |  100 +++++++++++++++++++++++++++++++++++++----------
 1 files changed, 78 insertions(+), 22 deletions(-)

diffs (193 lines):

diff -r f1573d366877 -r 43fffa2ceb03 sys/dev/hdaudio/hdafg.c
--- a/sys/dev/hdaudio/hdafg.c   Tue Jul 18 12:34:25 2023 +0000
+++ b/sys/dev/hdaudio/hdafg.c   Tue Jul 18 13:35:57 2023 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: hdafg.c,v 1.29 2023/01/05 09:57:39 kardel Exp $ */
+/* $NetBSD: hdafg.c,v 1.30 2023/07/18 13:35:57 riastradh Exp $ */
 
 /*
  * Copyright (c) 2009 Precedence Technologies Ltd <support%precedence.co.uk@localhost>
@@ -60,7 +60,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: hdafg.c,v 1.29 2023/01/05 09:57:39 kardel Exp $");
+__KERNEL_RCSID(0, "$NetBSD: hdafg.c,v 1.30 2023/07/18 13:35:57 riastradh Exp $");
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -71,6 +71,9 @@
 #include <sys/bus.h>
 #include <sys/kmem.h>
 #include <sys/module.h>
+#include <sys/condvar.h>
+#include <sys/kthread.h>
+#include <sys/mutex.h>
 
 #include <sys/audioio.h>
 #include <dev/audio/audio_if.h>
@@ -311,8 +314,12 @@ struct hdafg_softc {
        int                             sc_pchan, sc_rchan;
        audio_params_t                  sc_pparam, sc_rparam;
 
-       struct callout                  sc_jack_callout;
+       kmutex_t                        sc_jack_lock;
+       kcondvar_t                      sc_jack_cv;
+       struct lwp                      *sc_jack_thread;
        bool                            sc_jack_polling;
+       bool                            sc_jack_suspended;
+       bool                            sc_jack_dying;
 
        struct {
                uint32_t                afg_cap;
@@ -3512,16 +3519,18 @@ hdafg_configure_encodings(struct hdafg_s
 }
 
 static void
-hdafg_hp_switch_handler(void *opaque)
+hdafg_hp_switch_handler(struct hdafg_softc *sc)
 {
-       struct hdafg_softc *sc = opaque;
        struct hdaudio_assoc *as = sc->sc_assocs;
        struct hdaudio_widget *w;
        uint32_t res = 0;
        int i, j;
 
+       KASSERT(sc->sc_jack_polling);
+       KASSERT(mutex_owned(&sc->sc_jack_lock));
+
        if (!device_is_active(sc->sc_dev))
-               goto resched;
+               return;
 
        for (i = 0; i < sc->sc_nassocs; i++) {
                if (as[i].as_digital != HDAFG_AS_ANALOG &&
@@ -3580,9 +3589,28 @@ hdafg_hp_switch_handler(void *opaque)
                        }
                }
        }
-
-resched:
-       callout_schedule(&sc->sc_jack_callout, HDAUDIO_HP_SENSE_PERIOD);
+}
+
+static void
+hdafg_hp_switch_thread(void *opaque)
+{
+       struct hdafg_softc *sc = opaque;
+
+       KASSERT(sc->sc_jack_polling);
+
+       mutex_enter(&sc->sc_jack_lock);
+       while (!sc->sc_jack_dying) {
+               if (sc->sc_jack_suspended) {
+                       cv_wait(&sc->sc_jack_cv, &sc->sc_jack_lock);
+                       continue;
+               }
+               hdafg_hp_switch_handler(sc);
+               (void)cv_timedwait(&sc->sc_jack_cv, &sc->sc_jack_lock,
+                   HDAUDIO_HP_SENSE_PERIOD);
+       }
+       mutex_exit(&sc->sc_jack_lock);
+
+       kthread_exit(0);
 }
 
 static void
@@ -3592,6 +3620,7 @@ hdafg_hp_switch_init(struct hdafg_softc 
        struct hdaudio_widget *w;
        bool enable = false;
        int i, j;
+       int error;
 
        for (i = 0; i < sc->sc_nassocs; i++) {
                if (as[i].as_hpredir < 0 && as[i].as_displaydev == false)
@@ -3650,11 +3679,24 @@ hdafg_hp_switch_init(struct hdafg_softc 
                        hda_trace1(sc, ",displayport");
                hda_trace1(sc, "]\n");
        }
-       if (enable) {
-               sc->sc_jack_polling = true;
-               hdafg_hp_switch_handler(sc);
-       } else
+       if (!enable) {
                hda_trace(sc, "jack detect not enabled\n");
+               return;
+       }
+
+       mutex_init(&sc->sc_jack_lock, MUTEX_DEFAULT, IPL_NONE);
+       cv_init(&sc->sc_jack_cv, "hdafghp");
+       sc->sc_jack_polling = true;
+       error = kthread_create(PRI_NONE, KTHREAD_MPSAFE, /*ci*/NULL,
+           hdafg_hp_switch_thread, sc, &sc->sc_jack_thread,
+           "%s hotplug detect", device_xname(sc->sc_dev));
+       if (error) {
+               aprint_error_dev(sc->sc_dev, "failed to create hotplug thread:"
+                   " %d", error);
+               sc->sc_jack_polling = false;
+               cv_destroy(&sc->sc_jack_cv);
+               mutex_destroy(&sc->sc_jack_lock);
+       }
 }
 
 static void
@@ -3675,10 +3717,6 @@ hdafg_attach(device_t parent, device_t s
        mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_NONE);
        mutex_init(&sc->sc_intr_lock, MUTEX_DEFAULT, IPL_SCHED);
 
-       callout_init(&sc->sc_jack_callout, 0);
-       callout_setfunc(&sc->sc_jack_callout,
-           hdafg_hp_switch_handler, sc);
-
        if (!pmf_device_register(self, hdafg_suspend, hdafg_resume))
                aprint_error_dev(self, "couldn't establish power handler\n");
 
@@ -3825,8 +3863,16 @@ hdafg_detach(device_t self, int flags)
        struct hdaudio_mixer *mx = sc->sc_mixers;
        int nid;
 
-       callout_halt(&sc->sc_jack_callout, NULL);
-       callout_destroy(&sc->sc_jack_callout);
+       if (sc->sc_jack_polling) {
+               int error __diagused;
+
+               mutex_enter(&sc->sc_jack_lock);
+               sc->sc_jack_dying = true;
+               cv_broadcast(&sc->sc_jack_cv);
+               mutex_exit(&sc->sc_jack_lock);
+               error = kthread_join(sc->sc_jack_thread);
+               KASSERTMSG(error == 0, "error=%d", error);
+       }
 
        if (sc->sc_config)
                prop_object_release(sc->sc_config);
@@ -3876,7 +3922,12 @@ hdafg_suspend(device_t self, const pmf_q
 {
        struct hdafg_softc *sc = device_private(self);
 
-       callout_halt(&sc->sc_jack_callout, NULL);
+       if (sc->sc_jack_polling) {
+               mutex_enter(&sc->sc_jack_lock);
+               KASSERT(!sc->sc_jack_suspended);
+               sc->sc_jack_suspended = true;
+               mutex_exit(&sc->sc_jack_lock);
+       }
 
        return true;
 }
@@ -3907,8 +3958,13 @@ hdafg_resume(device_t self, const pmf_qu
        hdafg_stream_connect(sc, AUMODE_PLAY);
        hdafg_stream_connect(sc, AUMODE_RECORD);
 
-       if (sc->sc_jack_polling)
-               hdafg_hp_switch_handler(sc);
+       if (sc->sc_jack_polling) {
+               mutex_enter(&sc->sc_jack_lock);
+               KASSERT(sc->sc_jack_suspended);
+               sc->sc_jack_suspended = false;
+               cv_broadcast(&sc->sc_jack_cv);
+               mutex_exit(&sc->sc_jack_lock);
+       }
 
        return true;
 }



Home | Main Index | Thread Index | Old Index