Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 27 Dec 2016 19:08:08 +0000 (UTC)
From:      Oleksandr Tymoshenko <gonzo@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r310636 - head/sys/arm/broadcom/bcm2835
Message-ID:  <201612271908.uBRJ88DE046230@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: gonzo
Date: Tue Dec 27 19:08:08 2016
New Revision: 310636
URL: https://svnweb.freebsd.org/changeset/base/310636

Log:
  [rpi] Fix bcm2835_audio locking and samples starvation
  
  Rework general approach to locking and working with audio worker thread:
  
  - Use flags to signal requested worker action
  - Fix submitted buffer calculations to avoid samples starvation
  - Protect buffer pointers with locks to fix race condition between callback
    and audio worker thread
  - Remove unnecessary vchi_service_use
  - Do not use lock to serialize VCHI requests since only one thread issues them now
  - Fix unloading signaling per hselasky@ suggestion
  - Add output to detect inconsistent callback data caused by possible firmware bug
    https://github.com/raspberrypi/firmware/issues/696
  - Add stats/debug sysctls to troubleshoot possible bugs
  
  PR:		213687, 205979, 215194
  MFC after:	1 week

Modified:
  head/sys/arm/broadcom/bcm2835/bcm2835_audio.c

Modified: head/sys/arm/broadcom/bcm2835/bcm2835_audio.c
==============================================================================
--- head/sys/arm/broadcom/bcm2835/bcm2835_audio.c	Tue Dec 27 18:23:16 2016	(r310635)
+++ head/sys/arm/broadcom/bcm2835/bcm2835_audio.c	Tue Dec 27 19:08:08 2016	(r310636)
@@ -40,13 +40,33 @@
 
 SND_DECLARE_FILE("$FreeBSD$");
 
+/* Audio destination */
 #define	DEST_AUTO		0
 #define	DEST_HEADPHONES		1
 #define	DEST_HDMI		2
 
+/* Playback state */
+#define	PLAYBACK_IDLE		0
+#define	PLAYBACK_PLAYING	1
+#define	PLAYBACK_STOPPING	2
+
+/* Worker thread state */
+#define	WORKER_RUNNING		0
+#define	WORKER_STOPPING		1
+#define	WORKER_STOPPED		2
+
+/*
+ * Worker thread flags, set to 1 in flags_pending
+ * when driver requests one or another operation
+ * from worker. Cleared to 0 once worker performs
+ * the operations.
+ */
+#define	AUDIO_PARAMS		(1 << 0)
+#define	AUDIO_PLAY		(1 << 1)
+#define	AUDIO_STOP		(1 << 2)
+
 #define	VCHIQ_AUDIO_PACKET_SIZE	4000
-#define	VCHIQ_AUDIO_BUFFER_SIZE	128000
-#define	VCHIQ_AUDIO_PREBUFFER	10 /* Number of pre-buffered audio messages */
+#define	VCHIQ_AUDIO_BUFFER_SIZE	10*VCHIQ_AUDIO_PACKET_SIZE
 
 #define	VCHIQ_AUDIO_MAX_VOLUME	
 /* volume in terms of 0.01dB */
@@ -77,22 +97,25 @@ static struct pcmchan_caps bcm2835_audio
 
 struct bcm2835_audio_info;
 
-#define	PLAYBACK_IDLE		0
-#define	PLAYBACK_STARTING	1
-#define	PLAYBACK_PLAYING	2
-#define	PLAYBACK_STOPPING	3
-
 struct bcm2835_audio_chinfo {
 	struct bcm2835_audio_info *parent;
 	struct pcm_channel *channel;
 	struct snd_dbuf *buffer;
 	uint32_t fmt, spd, blksz;
 
-	uint32_t complete_pos;
-	uint32_t free_buffer;
-	uint32_t buffered_ptr;
+	/* Pointer to first unsubmitted sample */
+	uint32_t unsubmittedptr;
+	/*
+	 * Number of bytes in "submitted but not played"
+	 * pseudo-buffer
+	 */
+	int available_space;
 	int playback_state;
-	int prebuffered;
+	uint64_t callbacks;
+	uint64_t submitted_samples;
+	uint64_t retrieved_samples;
+	uint64_t underruns;
+	int starved;
 };
 
 struct bcm2835_audio_info {
@@ -100,32 +123,25 @@ struct bcm2835_audio_info {
 	unsigned int bufsz;
     	struct bcm2835_audio_chinfo pch;
 	uint32_t dest, volume;
-	struct mtx *lock;
 	struct intr_config_hook intr_hook;
 
 	/* VCHI data */
-	struct sx vchi_lock;
-
 	VCHI_INSTANCE_T vchi_instance;
 	VCHI_CONNECTION_T *vchi_connection;
 	VCHI_SERVICE_HANDLE_T vchi_handle;
 
-	struct sx worker_lock;
+	struct mtx lock;
 	struct cv worker_cv;
 
-	bool parameters_update_pending;
-	bool controls_update_pending;
+	uint32_t flags_pending;
 
-	/* Unloadign module */
-	int unloading;
+	/* Worker thread state */
+	int worker_state;
 };
 
-#define bcm2835_audio_lock(_ess) snd_mtxlock((_ess)->lock)
-#define bcm2835_audio_unlock(_ess) snd_mtxunlock((_ess)->lock)
-#define bcm2835_audio_lock_assert(_ess) snd_mtxassert((_ess)->lock)
-
-#define VCHIQ_VCHI_LOCK(sc)		sx_xlock(&(sc)->vchi_lock)
-#define VCHIQ_VCHI_UNLOCK(sc)		sx_xunlock(&(sc)->vchi_lock)
+#define BCM2835_AUDIO_LOCK(sc)		mtx_lock(&(sc)->lock)
+#define BCM2835_AUDIO_LOCKED(sc)	mtx_assert(&(sc)->lock, MA_OWNED)
+#define BCM2835_AUDIO_UNLOCK(sc)	mtx_unlock(&(sc)->lock)
 
 static const char *
 dest_description(uint32_t dest)
@@ -149,6 +165,36 @@ dest_description(uint32_t dest)
 }
 
 static void
+bcm2835_worker_update_params(struct bcm2835_audio_info *sc)
+{
+
+	BCM2835_AUDIO_LOCKED(sc);
+
+	sc->flags_pending |= AUDIO_PARAMS;
+	cv_signal(&sc->worker_cv);
+}
+
+static void
+bcm2835_worker_play_start(struct bcm2835_audio_info *sc)
+{
+	BCM2835_AUDIO_LOCK(sc);
+	sc->flags_pending &= ~(AUDIO_STOP);
+	sc->flags_pending |= AUDIO_PLAY;
+	cv_signal(&sc->worker_cv);
+	BCM2835_AUDIO_UNLOCK(sc);
+}
+
+static void
+bcm2835_worker_play_stop(struct bcm2835_audio_info *sc)
+{
+	BCM2835_AUDIO_LOCK(sc);
+	sc->flags_pending &= ~(AUDIO_PLAY);
+	sc->flags_pending |= AUDIO_STOP;
+	cv_signal(&sc->worker_cv);
+	BCM2835_AUDIO_UNLOCK(sc);
+}
+
+static void
 bcm2835_audio_callback(void *param, const VCHI_CALLBACK_REASON_T reason, void *msg_handle)
 {
 	struct bcm2835_audio_info *sc = (struct bcm2835_audio_info *)param;
@@ -163,7 +209,7 @@ bcm2835_audio_callback(void *param, cons
 	    &m, sizeof m, &msg_len, VCHI_FLAGS_NONE);
 	if (m.type == VC_AUDIO_MSG_TYPE_RESULT) {
 		if (m.u.result.success) {
-			device_printf(sc->dev, 
+			device_printf(sc->dev,
 			    "msg type %08x failed\n",
 			    m.type);
 		}
@@ -172,13 +218,35 @@ bcm2835_audio_callback(void *param, cons
 
 		int count = m.u.complete.count & 0xffff;
 		int perr = (m.u.complete.count & (1U << 30)) != 0;
-
-		ch->complete_pos = (ch->complete_pos + count) % sndbuf_getsize(ch->buffer);
-		ch->free_buffer += count;
-		chn_intr(sc->pch.channel);
-
-		if (perr || ch->free_buffer >= VCHIQ_AUDIO_PACKET_SIZE)
-			cv_signal(&sc->worker_cv);
+		ch->callbacks++;
+		if (perr)
+			ch->underruns++;
+
+		BCM2835_AUDIO_LOCK(sc);
+		if (ch->playback_state != PLAYBACK_IDLE) {
+			/* Prevent LOR */
+			BCM2835_AUDIO_UNLOCK(sc);
+			chn_intr(sc->pch.channel);
+			BCM2835_AUDIO_LOCK(sc);
+		}
+		/* We should check again, state might have changed */
+		if (ch->playback_state != PLAYBACK_IDLE) {
+			if (!perr) {
+				if ((ch->available_space + count)> VCHIQ_AUDIO_BUFFER_SIZE) {
+					device_printf(sc->dev, "inconsistent data in callback:\n");
+					device_printf(sc->dev, "available_space == %d, count = %d, perr=%d\n",
+					    ch->available_space, count, perr);
+					device_printf(sc->dev,
+					    "retrieved_samples = %lld, submitted_samples = %lld\n",
+					    ch->retrieved_samples, ch->submitted_samples);
+				}
+				ch->available_space += count;
+				ch->retrieved_samples += count;
+			}
+			if (perr || (ch->available_space >= VCHIQ_AUDIO_PACKET_SIZE))
+				cv_signal(&sc->worker_cv);
+		}
+		BCM2835_AUDIO_UNLOCK(sc);
 	} else
 		printf("%s: unknown m.type: %d\n", __func__, m.type);
 }
@@ -218,10 +286,7 @@ bcm2835_audio_init(struct bcm2835_audio_
 	status = vchi_service_open(sc->vchi_instance, &params,
 	    &sc->vchi_handle);
 
-	if (status == 0)
-		/* Finished with the service for now */
-		vchi_service_release(sc->vchi_handle);
-	else
+	if (status != 0)
 		sc->vchi_handle = VCHIQ_SERVICE_HANDLE_INVALID;
 }
 
@@ -231,10 +296,10 @@ bcm2835_audio_release(struct bcm2835_aud
 	int success;
 
 	if (sc->vchi_handle != VCHIQ_SERVICE_HANDLE_INVALID) {
-		vchi_service_use(sc->vchi_handle);
 		success = vchi_service_close(sc->vchi_handle);
 		if (success != 0)
 			printf("vchi_service_close failed: %d\n", success);
+		vchi_service_release(sc->vchi_handle);
 		sc->vchi_handle = VCHIQ_SERVICE_HANDLE_INVALID;
 	}
 
@@ -244,12 +309,9 @@ bcm2835_audio_release(struct bcm2835_aud
 static void
 bcm2835_audio_reset_channel(struct bcm2835_audio_chinfo *ch)
 {
-	ch->free_buffer = VCHIQ_AUDIO_BUFFER_SIZE;
-	ch->playback_state = 0;
-	ch->buffered_ptr = 0;
-	ch->complete_pos = 0;
-	ch->prebuffered = 0;
 
+	ch->available_space = VCHIQ_AUDIO_BUFFER_SIZE;
+	ch->unsubmittedptr = 0;
 	sndbuf_reset(ch->buffer);
 }
 
@@ -260,21 +322,14 @@ bcm2835_audio_start(struct bcm2835_audio
 	int ret;
 	struct bcm2835_audio_info *sc = ch->parent;
 
-	VCHIQ_VCHI_LOCK(sc);
 	if (sc->vchi_handle != VCHIQ_SERVICE_HANDLE_INVALID) {
-		vchi_service_use(sc->vchi_handle);
-
 		m.type = VC_AUDIO_MSG_TYPE_START;
 		ret = vchi_msg_queue(sc->vchi_handle,
 		    &m, sizeof m, VCHI_FLAGS_BLOCK_UNTIL_QUEUED, NULL);
 
 		if (ret != 0)
 			printf("%s: vchi_msg_queue failed (err %d)\n", __func__, ret);
-
-		vchi_service_release(sc->vchi_handle);
 	}
-	VCHIQ_VCHI_UNLOCK(sc);
-
 }
 
 static void
@@ -284,10 +339,7 @@ bcm2835_audio_stop(struct bcm2835_audio_
 	int ret;
 	struct bcm2835_audio_info *sc = ch->parent;
 
-	VCHIQ_VCHI_LOCK(sc);
 	if (sc->vchi_handle != VCHIQ_SERVICE_HANDLE_INVALID) {
-		vchi_service_use(sc->vchi_handle);
-
 		m.type = VC_AUDIO_MSG_TYPE_STOP;
 		m.u.stop.draining = 0;
 
@@ -296,10 +348,7 @@ bcm2835_audio_stop(struct bcm2835_audio_
 
 		if (ret != 0)
 			printf("%s: vchi_msg_queue failed (err %d)\n", __func__, ret);
-
-		vchi_service_release(sc->vchi_handle);
 	}
-	VCHIQ_VCHI_UNLOCK(sc);
 }
 
 static void
@@ -308,20 +357,14 @@ bcm2835_audio_open(struct bcm2835_audio_
 	VC_AUDIO_MSG_T m;
 	int ret;
 
-	VCHIQ_VCHI_LOCK(sc);
 	if (sc->vchi_handle != VCHIQ_SERVICE_HANDLE_INVALID) {
-		vchi_service_use(sc->vchi_handle);
-
 		m.type = VC_AUDIO_MSG_TYPE_OPEN;
 		ret = vchi_msg_queue(sc->vchi_handle,
 		    &m, sizeof m, VCHI_FLAGS_BLOCK_UNTIL_QUEUED, NULL);
 
 		if (ret != 0)
 			printf("%s: vchi_msg_queue failed (err %d)\n", __func__, ret);
-
-		vchi_service_release(sc->vchi_handle);
 	}
-	VCHIQ_VCHI_UNLOCK(sc);
 }
 
 static void
@@ -330,10 +373,7 @@ bcm2835_audio_update_controls(struct bcm
 	VC_AUDIO_MSG_T m;
 	int ret, db;
 
-	VCHIQ_VCHI_LOCK(sc);
 	if (sc->vchi_handle != VCHIQ_SERVICE_HANDLE_INVALID) {
-		vchi_service_use(sc->vchi_handle);
-
 		m.type = VC_AUDIO_MSG_TYPE_CONTROL;
 		m.u.control.dest = dest;
 		if (volume > 99)
@@ -346,10 +386,7 @@ bcm2835_audio_update_controls(struct bcm
 
 		if (ret != 0)
 			printf("%s: vchi_msg_queue failed (err %d)\n", __func__, ret);
-
-		vchi_service_release(sc->vchi_handle);
 	}
-	VCHIQ_VCHI_UNLOCK(sc);
 }
 
 static void
@@ -358,10 +395,7 @@ bcm2835_audio_update_params(struct bcm28
 	VC_AUDIO_MSG_T m;
 	int ret;
 
-	VCHIQ_VCHI_LOCK(sc);
 	if (sc->vchi_handle != VCHIQ_SERVICE_HANDLE_INVALID) {
-		vchi_service_use(sc->vchi_handle);
-
 		m.type = VC_AUDIO_MSG_TYPE_CONFIG;
 		m.u.config.channels = AFMT_CHANNEL(fmt);
 		m.u.config.samplerate = speed;
@@ -372,76 +406,48 @@ bcm2835_audio_update_params(struct bcm28
 
 		if (ret != 0)
 			printf("%s: vchi_msg_queue failed (err %d)\n", __func__, ret);
-
-		vchi_service_release(sc->vchi_handle);
 	}
-	VCHIQ_VCHI_UNLOCK(sc);
 }
 
-static __inline uint32_t
-vchiq_unbuffered_bytes(struct bcm2835_audio_chinfo *ch)
+static bool
+bcm2835_audio_buffer_should_sleep(struct bcm2835_audio_chinfo *ch)
 {
-	uint32_t size, ready, readyptr, readyend;
-
-	size = sndbuf_getsize(ch->buffer);
-	readyptr = sndbuf_getreadyptr(ch->buffer);
-	ready = sndbuf_getready(ch->buffer);
+	
+	if (ch->playback_state != PLAYBACK_PLAYING)
+		return (true);
 
-	readyend = readyptr + ready;
-	/* Normal case */
-	if (ch->buffered_ptr >= readyptr) {
-		if (readyend > ch->buffered_ptr)
-			return readyend - ch->buffered_ptr;
-		else
-			return 0;
+	/* Not enough data */
+	if (sndbuf_getready(ch->buffer) < VCHIQ_AUDIO_PACKET_SIZE) {
+		printf("starve\n");
+		ch->starved++;
+		return (true);
 	}
-	else { /* buffered_ptr overflow */
-		if (readyend > ch->buffered_ptr + size)
-			return readyend - ch->buffered_ptr - size;
-		else
-			return 0;
+
+	/* Not enough free space */
+	if (ch->available_space < VCHIQ_AUDIO_PACKET_SIZE) {
+		return (true);
 	}
+
+	return (false);
 }
 
 static void
-bcm2835_audio_write_samples(struct bcm2835_audio_chinfo *ch)
+bcm2835_audio_write_samples(struct bcm2835_audio_chinfo *ch, void *buf, uint32_t count)
 {
 	struct bcm2835_audio_info *sc = ch->parent;
 	VC_AUDIO_MSG_T m;
-	void *buf;
-	uint32_t count, size;
 	int ret;
 
-	VCHIQ_VCHI_LOCK(sc);
 	if (sc->vchi_handle == VCHIQ_SERVICE_HANDLE_INVALID) {
-		VCHIQ_VCHI_UNLOCK(sc);
 		return;
 	}
 
-	vchi_service_use(sc->vchi_handle);
-
-	size = sndbuf_getsize(ch->buffer);
-	count = vchiq_unbuffered_bytes(ch);
-	buf = (uint8_t*)sndbuf_getbuf(ch->buffer) + ch->buffered_ptr;
-
-	if (ch->buffered_ptr + count > size)
-		count = size - ch->buffered_ptr;
-
-	if (count < VCHIQ_AUDIO_PACKET_SIZE)
-		goto done;
-
-	count = min(count, ch->free_buffer);
-	count -= count % VCHIQ_AUDIO_PACKET_SIZE;
-
 	m.type = VC_AUDIO_MSG_TYPE_WRITE;
 	m.u.write.count = count;
 	m.u.write.max_packet = VCHIQ_AUDIO_PACKET_SIZE;
 	m.u.write.callback = NULL;
 	m.u.write.cookie = ch;
-	if (buf)
-		m.u.write.silence = 0;
-	else
-		m.u.write.silence = 1;
+	m.u.write.silence = 0;
 
 	ret = vchi_msg_queue(sc->vchi_handle,
 	    &m, sizeof m, VCHI_FLAGS_BLOCK_UNTIL_QUEUED, NULL);
@@ -449,25 +455,16 @@ bcm2835_audio_write_samples(struct bcm28
 	if (ret != 0)
 		printf("%s: vchi_msg_queue failed (err %d)\n", __func__, ret);
 
-	if (buf) {
-		while (count > 0) {
-			int bytes = MIN((int)m.u.write.max_packet, (int)count);
-			ch->free_buffer -= bytes;
-			ch->buffered_ptr += bytes;
-			ch->buffered_ptr = ch->buffered_ptr % size;
-			ret = vchi_msg_queue(sc->vchi_handle,
-			    buf, bytes, VCHI_FLAGS_BLOCK_UNTIL_QUEUED, NULL);
-			if (ret != 0)
-				printf("%s: vchi_msg_queue failed: %d\n",
-				    __func__, ret);
-			buf = (char *)buf + bytes;
-			count -= bytes;
-		}
+	while (count > 0) {
+		int bytes = MIN((int)m.u.write.max_packet, (int)count);
+		ret = vchi_msg_queue(sc->vchi_handle,
+		    buf, bytes, VCHI_FLAGS_BLOCK_UNTIL_QUEUED, NULL);
+		if (ret != 0)
+			printf("%s: vchi_msg_queue failed: %d\n",
+			    __func__, ret);
+		buf = (char *)buf + bytes;
+		count -= bytes;
 	}
-done:
-
-	vchi_service_release(sc->vchi_handle);
-	VCHIQ_VCHI_UNLOCK(sc);
 }
 
 static void
@@ -477,70 +474,98 @@ bcm2835_audio_worker(void *data)
 	struct bcm2835_audio_chinfo *ch = &sc->pch;
 	uint32_t speed, format;
 	uint32_t volume, dest;
-	bool parameters_changed, controls_changed;
+	uint32_t flags;
+	uint32_t count, size, readyptr;
+	uint8_t *buf;
 
-	sx_slock(&sc->worker_lock);
-	while(1) {
+	ch->playback_state = PLAYBACK_IDLE;
 
-		if (sc->unloading)
+	while (1) {
+		if (sc->worker_state != WORKER_RUNNING)
 			break;
 
-		parameters_changed = false;
-		controls_changed = false;
-		bcm2835_audio_lock(sc);
-		if (sc->parameters_update_pending) {
-			/* TODO: update parameters */
+		BCM2835_AUDIO_LOCK(sc);
+		/*
+		 * wait until there are flags set or buffer is ready
+		 * to consume more samples
+		 */
+		while ((sc->flags_pending == 0) &&
+		    bcm2835_audio_buffer_should_sleep(ch)) {
+			cv_wait_sig(&sc->worker_cv, &sc->lock);
+		}
+		flags = sc->flags_pending;
+		/* Clear pending flags */
+		sc->flags_pending = 0;
+		BCM2835_AUDIO_UNLOCK(sc);
+
+		/* Requested to change parameters */
+		if (flags & AUDIO_PARAMS) {
+			BCM2835_AUDIO_LOCK(sc);
 			speed = ch->spd;
 			format = ch->fmt;
-			sc->parameters_update_pending = false;
-			parameters_changed = true;
-		}
-
-		if (sc->controls_update_pending) {
 			volume = sc->volume;
 			dest = sc->dest;
-			sc->controls_update_pending = false;
-			controls_changed = true;
+			BCM2835_AUDIO_UNLOCK(sc);
+			if (ch->playback_state == PLAYBACK_IDLE)
+				bcm2835_audio_update_params(sc, format, speed);
+			bcm2835_audio_update_controls(sc, volume, dest);
 		}
 
-		bcm2835_audio_unlock(sc);
-
-		if (parameters_changed) {
-			bcm2835_audio_update_params(sc, format, speed);
+		/* Requested to stop playback */
+		if ((flags & AUDIO_STOP) &&
+		    (ch->playback_state == PLAYBACK_PLAYING)) {
+			bcm2835_audio_stop(ch);
+			BCM2835_AUDIO_LOCK(sc);
+			bcm2835_audio_reset_channel(&sc->pch);
+			ch->playback_state = PLAYBACK_IDLE;
+			BCM2835_AUDIO_UNLOCK(sc);
+			continue;
 		}
 
-		if (controls_changed) {
-			bcm2835_audio_update_controls(sc, volume, dest);
+		/* Requested to start playback */
+		if ((flags & AUDIO_PLAY) &&
+		    (ch->playback_state == PLAYBACK_IDLE)) {
+			BCM2835_AUDIO_LOCK(sc);
+			ch->playback_state = PLAYBACK_PLAYING;
+			BCM2835_AUDIO_UNLOCK(sc);
+			bcm2835_audio_start(ch);
 		}
 
-		if (ch->playback_state == PLAYBACK_IDLE) {
-			cv_wait_sig(&sc->worker_cv, &sc->worker_lock);
+		if (ch->playback_state == PLAYBACK_IDLE)
 			continue;
-		}
 
-		if (ch->playback_state == PLAYBACK_STOPPING) {
-			bcm2835_audio_stop(ch);
-			bcm2835_audio_reset_channel(&sc->pch);
-			ch->playback_state = PLAYBACK_IDLE;
+		if (sndbuf_getready(ch->buffer) == 0)
 			continue;
-		}
 
-		if (ch->free_buffer < vchiq_unbuffered_bytes(ch)) {
-			cv_timedwait_sig(&sc->worker_cv, &sc->worker_lock, 10);
+		count = sndbuf_getready(ch->buffer);
+		size = sndbuf_getsize(ch->buffer);
+		readyptr = sndbuf_getreadyptr(ch->buffer);
+
+		BCM2835_AUDIO_LOCK(sc);
+		if (readyptr + count > size)
+			count = size - readyptr;
+		count = min(count, ch->available_space);
+		count -= (count % VCHIQ_AUDIO_PACKET_SIZE);
+		BCM2835_AUDIO_UNLOCK(sc);
+
+		if (count < VCHIQ_AUDIO_PACKET_SIZE)
 			continue;
-		}
 
-		bcm2835_audio_write_samples(ch);
+		buf = (uint8_t*)sndbuf_getbuf(ch->buffer) + readyptr;
 
-		if (ch->playback_state == PLAYBACK_STARTING) {
-			ch->prebuffered++;
-			if (ch->prebuffered == VCHIQ_AUDIO_PREBUFFER) {
-				bcm2835_audio_start(ch);
-				ch->playback_state = PLAYBACK_PLAYING;
-			}
-		}
+		bcm2835_audio_write_samples(ch, buf, count);
+		BCM2835_AUDIO_LOCK(sc);
+		ch->unsubmittedptr = (ch->unsubmittedptr + count) % sndbuf_getsize(ch->buffer);
+		ch->available_space -= count;
+		ch->submitted_samples += count;
+		KASSERT(ch->available_space >= 0, ("ch->available_space == %d\n", ch->available_space));
+		BCM2835_AUDIO_UNLOCK(sc);
 	}
-	sx_sunlock(&sc->worker_lock);
+
+	BCM2835_AUDIO_LOCK(sc);
+	sc->worker_state = WORKER_STOPPED;
+	cv_signal(&sc->worker_cv);
+	BCM2835_AUDIO_UNLOCK(sc);
 
 	kproc_exit(0);
 }
@@ -550,6 +575,7 @@ bcm2835_audio_create_worker(struct bcm28
 {
 	struct proc *newp;
 
+	sc->worker_state = WORKER_RUNNING;
 	if (kproc_create(bcm2835_audio_worker, (void*)sc, &newp, 0, 0,
 	    "bcm2835_audio_worker") != 0) {
 		printf("failed to create bcm2835_audio_worker\n");
@@ -585,8 +611,9 @@ bcmchan_init(kobj_t obj, void *devinfo, 
 		return NULL;
 	}
 
-	sc->parameters_update_pending = true;
-	cv_signal(&sc->worker_cv);
+	BCM2835_AUDIO_LOCK(sc);
+	bcm2835_worker_update_params(sc);
+	BCM2835_AUDIO_UNLOCK(sc);
 
 	return ch;
 }
@@ -610,12 +637,10 @@ bcmchan_setformat(kobj_t obj, void *data
 	struct bcm2835_audio_chinfo *ch = data;
 	struct bcm2835_audio_info *sc = ch->parent;
 
-	bcm2835_audio_lock(sc);
+	BCM2835_AUDIO_LOCK(sc);
 	ch->fmt = format;
-	sc->parameters_update_pending = true;
-	bcm2835_audio_unlock(sc);
-
-	cv_signal(&sc->worker_cv);
+	bcm2835_worker_update_params(sc);
+	BCM2835_AUDIO_UNLOCK(sc);
 
 	return 0;
 }
@@ -626,12 +651,10 @@ bcmchan_setspeed(kobj_t obj, void *data,
 	struct bcm2835_audio_chinfo *ch = data;
 	struct bcm2835_audio_info *sc = ch->parent;
 
-	bcm2835_audio_lock(sc);
+	BCM2835_AUDIO_LOCK(sc);
 	ch->spd = speed;
-	sc->parameters_update_pending = true;
-	bcm2835_audio_unlock(sc);
-
-	cv_signal(&sc->worker_cv);
+	bcm2835_worker_update_params(sc);
+	BCM2835_AUDIO_UNLOCK(sc);
 
 	return ch->spd;
 }
@@ -653,25 +676,18 @@ bcmchan_trigger(kobj_t obj, void *data, 
 	if (!PCMTRIG_COMMON(go))
 		return (0);
 
-
 	switch (go) {
 	case PCMTRIG_START:
-		bcm2835_audio_lock(sc);
-		bcm2835_audio_reset_channel(ch);
-		ch->playback_state = PLAYBACK_STARTING;
-		bcm2835_audio_unlock(sc);
 		/* kickstart data flow */
 		chn_intr(sc->pch.channel);
-		/* wakeup worker thread */
-		cv_signal(&sc->worker_cv);
+		ch->submitted_samples = 0;
+		ch->retrieved_samples = 0;
+		bcm2835_worker_play_start(sc);
 		break;
 
 	case PCMTRIG_STOP:
 	case PCMTRIG_ABORT:
-		bcm2835_audio_lock(sc);
-		ch->playback_state = PLAYBACK_STOPPING;
-		bcm2835_audio_unlock(sc);
-		cv_signal(&sc->worker_cv);
+		bcm2835_worker_play_stop(sc);
 		break;
 
 	default:
@@ -687,11 +703,9 @@ bcmchan_getptr(kobj_t obj, void *data)
 	struct bcm2835_audio_info *sc = ch->parent;
 	uint32_t ret;
 
-	bcm2835_audio_lock(sc);
-
-	ret = ch->complete_pos - (ch->complete_pos % VCHIQ_AUDIO_PACKET_SIZE);
-
-	bcm2835_audio_unlock(sc);
+	BCM2835_AUDIO_LOCK(sc);
+	ret = ch->unsubmittedptr;
+	BCM2835_AUDIO_UNLOCK(sc);
 
 	return ret;
 }
@@ -734,11 +748,11 @@ bcmmix_set(struct snd_mixer *m, unsigned
 
 	switch (dev) {
 	case SOUND_MIXER_VOLUME:
-		bcm2835_audio_lock(sc);
+		BCM2835_AUDIO_LOCK(sc);
 		sc->volume = left;
-		sc->controls_update_pending = true;
-		bcm2835_audio_unlock(sc);
-		cv_signal(&sc->worker_cv);
+		bcm2835_worker_update_params(sc);
+		BCM2835_AUDIO_UNLOCK(sc);
+
 		break;
 
 	default:
@@ -771,13 +785,13 @@ sysctl_bcm2835_audio_dest(SYSCTL_HANDLER
 	if ((val < 0) || (val > 2))
 		return (EINVAL);
 
-	bcm2835_audio_lock(sc);
+	BCM2835_AUDIO_LOCK(sc);
 	sc->dest = val;
-	sc->controls_update_pending = true;
-	bcm2835_audio_unlock(sc);
+	bcm2835_worker_update_params(sc);
+	BCM2835_AUDIO_UNLOCK(sc);
 
-	cv_signal(&sc->worker_cv);
-	device_printf(sc->dev, "destination set to %s\n", dest_description(val));
+	if (bootverbose)
+		device_printf(sc->dev, "destination set to %s\n", dest_description(val));
 
 	return (0);
 }
@@ -799,6 +813,24 @@ vchi_audio_sysctl_init(struct bcm2835_au
 	    CTLFLAG_RW | CTLTYPE_UINT, sc, sizeof(*sc),
 	    sysctl_bcm2835_audio_dest, "IU", "audio destination, "
 	    "0 - auto, 1 - headphones, 2 - HDMI");
+	SYSCTL_ADD_UQUAD(ctx, tree, OID_AUTO, "callbacks",
+			CTLFLAG_RD, &sc->pch.callbacks,
+			"callbacks total");
+	SYSCTL_ADD_UQUAD(ctx, tree, OID_AUTO, "submitted",
+			CTLFLAG_RD, &sc->pch.submitted_samples,
+			"last play submitted samples");
+	SYSCTL_ADD_UQUAD(ctx, tree, OID_AUTO, "retrieved",
+			CTLFLAG_RD, &sc->pch.retrieved_samples,
+			"last play retrieved samples");
+	SYSCTL_ADD_UQUAD(ctx, tree, OID_AUTO, "underruns",
+			CTLFLAG_RD, &sc->pch.underruns,
+			"callback underruns");
+	SYSCTL_ADD_INT(ctx, tree, OID_AUTO, "freebuffer",
+			CTLFLAG_RD, &sc->pch.available_space,
+			sc->pch.available_space, "callbacks total");
+	SYSCTL_ADD_INT(ctx, tree, OID_AUTO, "starved",
+			CTLFLAG_RD, &sc->pch.starved,
+			sc->pch.starved, "number of starved conditions");
 }
 
 static void
@@ -816,7 +848,6 @@ bcm2835_audio_probe(device_t dev)
 	return (BUS_PROBE_DEFAULT);
 }
 
-
 static void
 bcm2835_audio_delayed_init(void *xsc)
 {
@@ -837,7 +868,7 @@ bcm2835_audio_delayed_init(void *xsc)
 		goto no;
 	}
 
-    	if (pcm_register(sc->dev, sc, 1, 1)) {
+    	if (pcm_register(sc->dev, sc, 1, 0)) {
 		device_printf(sc->dev, "pcm_register failed\n");
 		goto no;
 	}
@@ -865,14 +896,12 @@ bcm2835_audio_attach(device_t dev)
 	sc->dev = dev;
 	sc->bufsz = VCHIQ_AUDIO_BUFFER_SIZE;
 
-	sc->lock = snd_mtxcreate(device_get_nameunit(dev), "bcm2835_audio softc");
-
-	sx_init(&sc->vchi_lock, device_get_nameunit(dev));
-	sx_init(&sc->worker_lock, "bcm_audio_worker_lock");
+	mtx_init(&sc->lock, device_get_nameunit(dev),
+	    "bcm_audio_lock", MTX_DEF);
 	cv_init(&sc->worker_cv, "worker_cv");
 	sc->vchi_handle = VCHIQ_SERVICE_HANDLE_INVALID;
 
-	/* 
+	/*
 	 * We need interrupts enabled for VCHI to work properly,
 	 * so delay initialization until it happens.
 	 */
@@ -896,26 +925,23 @@ bcm2835_audio_detach(device_t dev)
 	sc = pcm_getdevinfo(dev);
 
 	/* Stop worker thread */
-	sx_xlock(&sc->worker_lock);
-	sc->unloading = 1;
-	sx_xunlock(&sc->worker_lock);
+	BCM2835_AUDIO_LOCK(sc);
+	sc->worker_state = WORKER_STOPPING;
 	cv_signal(&sc->worker_cv);
+	/* Wait for thread to exit */
+	while (sc->worker_state != WORKER_STOPPED)
+		cv_wait_sig(&sc->worker_cv, &sc->lock);
+	BCM2835_AUDIO_UNLOCK(sc);
 
 	r = pcm_unregister(dev);
 	if (r)
 		return r;
 
-	sx_destroy(&sc->vchi_lock);
-	sx_destroy(&sc->worker_lock);
+	mtx_destroy(&sc->lock);
 	cv_destroy(&sc->worker_cv);
 
 	bcm2835_audio_release(sc);
 
-	if (sc->lock) {
-		snd_mtxfree(sc->lock);
-		sc->lock = NULL;
-	}
-
     	free(sc, M_DEVBUF);
 
 	return 0;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201612271908.uBRJ88DE046230>