From 12ecd33394d18b9c3f99a131284473bc3466b325 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh@NetBSD.org>
Date: Thu, 6 Apr 2023 13:14:06 +0000
Subject: [PATCH] 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
---
sys/dev/hdaudio/hdafg.c | 82 +++++++++++++++++++++++++++++++++--------
1 file changed, 66 insertions(+), 16 deletions(-)
diff --git a/sys/dev/hdaudio/hdafg.c b/sys/dev/hdaudio/hdafg.c
index 4d81af81ecf8..560d01716eb6 100644
--- a/sys/dev/hdaudio/hdafg.c
+++ b/sys/dev/hdaudio/hdafg.c
@@ -71,6 +71,9 @@ __KERNEL_RCSID(0, "$NetBSD: hdafg.c,v 1.29 2023/01/05 09:57:39 kardel Exp $");
#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_softc *sc)
}
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 *sc)
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)
@@ -3651,8 +3680,19 @@ hdafg_hp_switch_init(struct hdafg_softc *sc)
hda_trace1(sc, "]\n");
}
if (enable) {
+ mutex_init(&sc->sc_jack_lock, MUTEX_DEFAULT, IPL_NONE);
+ cv_init(&sc->sc_jack_cv, "hdafghp");
sc->sc_jack_polling = true;
- hdafg_hp_switch_handler(sc);
+ 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, "kthread_create: %d",
+ error);
+ sc->sc_jack_polling = false;
+ cv_destroy(&sc->sc_jack_cv);
+ mutex_destroy(&sc->sc_jack_lock);
+ }
} else
hda_trace(sc, "jack detect not enabled\n");
}
@@ -3675,10 +3715,6 @@ hdafg_attach(device_t parent, device_t self, void *opaque)
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 +3861,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 +3920,9 @@ hdafg_suspend(device_t self, const pmf_qual_t *qual)
{
struct hdafg_softc *sc = device_private(self);
- callout_halt(&sc->sc_jack_callout, NULL);
+ mutex_enter(&sc->sc_jack_lock);
+ sc->sc_jack_suspended = true;
+ mutex_exit(&sc->sc_jack_lock);
return true;
}
@@ -3907,8 +3953,12 @@ hdafg_resume(device_t self, const pmf_qual_t *qual)
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);
+ sc->sc_jack_suspended = false;
+ cv_broadcast(&sc->sc_jack_cv);
+ mutex_exit(&sc->sc_jack_lock);
+ }
return true;
}