Rework PCM ops callback mechanism

- Fix issues regarding PCM ops callbacks via AVIRT
- As PCMs are created, the PCM ops are set according to the Audio
  Path's needs. A default PCM ops table is applied for all PCMs,
  which includes callbacks such as open, and hw_free, since AVIRT
  requires these for intermediate processing, whether or not a given
  Audio Path may need them.
- Separate PCM ops into separate playback and capture ops tables

Signed-off-by: Mark Farrugia <mark.farrugia@fiberdyne.com.au>

6 files changed, 73 insertions, 240 deletions

diff --git a/core.c b/core.c
index f3b33d1..35998c0 100644
--- a/core.c
+++ b/core.c
@@ -26,6 +26,10 @@ MODULE_LICENSE("GPL v2");
 #define D_PRINTK(fmt, args...) DDEBUG(D_LOGNAME, fmt, ##args)
 #define D_ERRORK(fmt, args...) DERROR(D_LOGNAME, fmt, ##args)
 
+#define PCM_OPS_SET(pcm_ops_ap, pcm_ops, cb)                                   \
+	((pcm_ops_ap->cb) ? ((*pcm_ops)->cb = pcm_ops_ap->cb) :                \
+			    ((*pcm_ops)->cb = NULL));
+
 #define SND_AVIRT_DRIVER "snd_avirt"
 
 static struct snd_avirt_core core = {
@@ -219,18 +223,42 @@ static void pcm_private_data_free(struct snd_pcm *pcm)
 static struct snd_pcm *pcm_create(struct snd_avirt_stream *stream)
 {
 	struct snd_avirt_private_data *avirt_private_data;
-	bool playback = false, capture = false;
+	struct snd_avirt_audiopath *audiopath;
+	struct snd_pcm_ops *pcm_ops, *pcm_ops_ap;
 	struct snd_pcm *pcm;
+	bool playback = false, capture = false;
 	int err;
 
-	/** Special case: loopback */
-	if (!strcmp(stream->map, "ap_loopback")) {
+	audiopath = snd_avirt_audiopath_get(stream->map);
+	if (!audiopath) {
+		D_ERRORK("Cannot find Audio Path uid: '%s'!", stream->map);
+		return ERR_PTR(-EFAULT);
+	}
+
+	if (!stream->direction) {
+		pcm_ops_ap = (struct snd_pcm_ops *)audiopath->pcm_playback_ops;
 		playback = true;
+	} else {
+		pcm_ops_ap = (struct snd_pcm_ops *)audiopath->pcm_capture_ops;
 		capture = true;
-		(&pcm_ops)->copy_user = NULL;
-	} else if (!stream->direction) {
+	}
+	pcm_ops = &pcm_ops_avirt;
+
+	/* Set PCM ops for the Audio Path*/
+	PCM_OPS_SET(pcm_ops_ap, &pcm_ops, close);
+	PCM_OPS_SET(pcm_ops_ap, &pcm_ops, prepare);
+	PCM_OPS_SET(pcm_ops_ap, &pcm_ops, trigger);
+	PCM_OPS_SET(pcm_ops_ap, &pcm_ops, pointer);
+	PCM_OPS_SET(pcm_ops_ap, &pcm_ops, get_time_info);
+	PCM_OPS_SET(pcm_ops_ap, &pcm_ops, fill_silence);
+	PCM_OPS_SET(pcm_ops_ap, &pcm_ops, copy_user);
+	PCM_OPS_SET(pcm_ops_ap, &pcm_ops, copy_kernel);
+	PCM_OPS_SET(pcm_ops_ap, &pcm_ops, mmap);
+	PCM_OPS_SET(pcm_ops_ap, &pcm_ops, ack);
+
+	/** Special case: loopback */
+	if (!strcmp(stream->map, "ap_loopback")) {
 		playback = true;
-	} else {
 		capture = true;
 	}
 
@@ -242,9 +270,9 @@ static struct snd_pcm *pcm_create(struct snd_avirt_stream *stream)
 
 	/** Register driver callbacks */
 	if (playback)
-		snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &pcm_ops);
+		snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, pcm_ops);
 	if (capture)
-		snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &pcm_ops);
+		snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, pcm_ops);
 
 	pcm->info_flags = 0;
 	strcpy(pcm->name, stream->name);
diff --git a/core.h b/core.h
index fe2aaab..40252e6 100644
--- a/core.h
+++ b/core.h
@@ -14,7 +14,7 @@
 
 #include "utils.h"
 
-extern struct snd_pcm_ops pcm_ops;
+extern struct snd_pcm_ops pcm_ops_avirt;
 
 struct snd_avirt_core {
 	int version[3];
diff --git a/dummy/dummy.c b/dummy/dummy.c
index 872ade9..f683b92 100644
--- a/dummy/dummy.c
+++ b/dummy/dummy.c
@@ -272,7 +272,8 @@ static struct snd_avirt_audiopath dummyap_module = {
 	.name = "Dummy Audio Path",
 	.version = { 0, 0, 1 },
 	.hw = &dummyap_hw,
-	.pcm_ops = &dummyap_pcm_ops,
+	.pcm_playback_ops = &dummyap_pcm_ops,
+	.pcm_capture_ops = &dummyap_pcm_ops,
 	.configure = dummy_configure,
 };
 
diff --git a/loopback/loopback.c b/loopback/loopback.c
index 9646706..4181392 100644
--- a/loopback/loopback.c
+++ b/loopback/loopback.c
@@ -1090,7 +1090,8 @@ static struct snd_avirt_audiopath loopbackap_module = {
 	.name = "Loopback Audio Path",
 	.version = { 0, 0, 1 },
 	.hw = &loopbackap_pcm_hardware,
-	.pcm_ops = &loopbackap_pcm_ops,
+	.pcm_playback_ops = &loopbackap_pcm_ops,
+	.pcm_capture_ops = &loopbackap_pcm_ops,
 	.configure = loopbackap_configure,
 };
 
diff --git a/pcm.c b/pcm.c
index 509eae7..411db40 100644
--- a/pcm.c
+++ b/pcm.c
@@ -15,10 +15,20 @@
 #define D_PRINTK(fmt, args...) DDEBUG(D_LOGNAME, fmt, ##args)
 #define D_ERRORK(fmt, args...) DERROR(D_LOGNAME, fmt, ##args)
 
-#define DO_AUDIOPATH_CB(ap, callback, substream, ...)                  \
-	(((ap)->pcm_ops->callback) ?                                   \
-		 (ap)->pcm_ops->callback((substream), ##__VA_ARGS__) : \
-		 0)
+/**
+ * DO_AUDIOPATH_CB - Used to call into an Audio Path's ALSA PCM callback from
+ * AVIRT, where AVIRT's own callback must be attended to first.
+ */
+#define DO_AUDIOPATH_CB(ap, callback, substream, ...)                          \
+	((!substream->stream) ?                                                \
+		 (((ap)->pcm_playback_ops->callback) ?                         \
+			  (ap)->pcm_playback_ops->callback((substream),        \
+							   ##__VA_ARGS__) :    \
+			  0) :                                                 \
+		 (((ap)->pcm_capture_ops->callback) ?                          \
+			  (ap)->pcm_capture_ops->callback((substream),         \
+							  ##__VA_ARGS__) :     \
+			  0))
 
 #define PRIVATE_DATA(substream)                                                \
 	((struct snd_avirt_private_data *)substream->private_data)
@@ -57,9 +67,14 @@ static int pcm_open(struct snd_pcm_substream *substream)
 	struct snd_pcm_hardware *hw;
 	unsigned int chans = 0;
 
+	// Find the Audio Path mapped to this device
 	stream = snd_avirt_stream_find_by_device(substream->pcm->device);
+	if (IS_ERR_VALUE(stream) || !stream)
+		return PTR_ERR(stream);
 	audiopath = snd_avirt_audiopath_get(stream->map);
 	CHK_NULL_V(audiopath, "Cannot find Audio Path uid: '%s'!", stream->map);
+
+	// Set the private data's Audio Path
 	avirt_private_data = substream->private_data;
 	avirt_private_data->audiopath = audiopath;
 
@@ -67,10 +82,6 @@ static int pcm_open(struct snd_pcm_substream *substream)
 	hw = &substream->runtime->hw;
 	memcpy(hw, audiopath->hw, sizeof(struct snd_pcm_hardware));
 
-	stream = snd_avirt_stream_find_by_device(substream->pcm->device);
-	if (IS_ERR_VALUE(stream) || !stream)
-		return PTR_ERR(stream);
-
 	// Setup remaining hw properties
 	chans = stream->channels;
 	hw->channels_min = chans;
@@ -81,22 +92,6 @@ static int pcm_open(struct snd_pcm_substream *substream)
 }
 
 /**
- * pcm_close - Implements 'close' callback for PCM middle layer
- * @substream: pointer to ALSA PCM substream
- *
- * This is called when a PCM substream is closed.
- *
- * Returns 0 on success or error code otherwise.
- */
-static int pcm_close(struct snd_pcm_substream *substream)
-{
-	// Do additional Audio Path 'close' callback
-	return DO_AUDIOPATH_CB(
-		(struct snd_avirt_audiopath *)PRIVATE_DATA(substream)->audiopath,
-        close, substream);
-}
-
-/**
  * pcm_hw_params - Implements 'hw_params' callback for PCM middle layer
  * @substream: pointer to ALSA PCM substream
  * @hw_params: contains the hardware parameters for the PCM
@@ -111,23 +106,19 @@ static int pcm_hw_params(struct snd_pcm_substream *substream,
 {
 	int retval;
 	size_t bufsz;
-	struct snd_avirt_audiopath *audiopath;
-	struct snd_avirt_stream *stream;
 
-	stream = snd_avirt_stream_find_by_device(substream->pcm->device);
-	if (IS_ERR_VALUE(stream) || !stream)
-		return PTR_ERR(stream);
-
-	if ((params_channels(hw_params) > stream->channels) ||
-	    (params_channels(hw_params) < stream->channels)) {
+	/* Check supported channels */
+	if ((params_channels(hw_params) <
+	     substream->runtime->hw.channels_min) ||
+	    (params_channels(hw_params) >
+	     substream->runtime->hw.channels_max)) {
 		D_ERRORK("Requested number of channels: %d not supported",
 			 params_channels(hw_params));
 		return -EINVAL;
 	}
 
-	audiopath =
-		(struct snd_avirt_audiopath *)PRIVATE_DATA(substream)->audiopath;
-	bufsz = params_buffer_bytes(hw_params) * audiopath->hw->periods_max;
+	bufsz = params_buffer_bytes(hw_params) *
+		substream->runtime->hw.periods_max;
 
 	retval = snd_pcm_lib_alloc_vmalloc_buffer(substream, bufsz);
 	if (retval < 0)
@@ -152,208 +143,19 @@ static int pcm_hw_free(struct snd_pcm_substream *substream)
 	// Do additional Audio Path 'hw_free' callback
 	err = DO_AUDIOPATH_CB(
 		(struct snd_avirt_audiopath *)PRIVATE_DATA(substream)->audiopath,
-        hw_free, substream);
+		hw_free, substream);
 
 	return snd_pcm_lib_free_vmalloc_buffer(substream);
 }
 
 /**
- * pcm_prepare - Implements 'prepare' callback for PCM middle layer
- * @substream: pointer to ALSA PCM substream
- *
- * The format rate, sample rate, etc., can be set here. This callback can be
- * called many times at each setup. This function is also used to handle overrun
- * and underrun conditions when we try and resync the DMA (if we're using DMA).
- *
- * Returns 0 on success or error code otherwise.
+ * Default PCM ops table. Selected PCM ops must pass through AVIRT before
+ * calling back to their respective Audio Paths
  */
-static int pcm_prepare(struct snd_pcm_substream *substream)
-{
-
-	// Do additional Audio Path 'prepare' callback
-	return DO_AUDIOPATH_CB(
-		(struct snd_avirt_audiopath *)PRIVATE_DATA(substream)->audiopath,
-        prepare, substream);
-}
-
-/**
- * pcm_trigger - Implements 'trigger' callback for PCM middle layer
- * @substream: pointer to ALSA PCM substream
- * @cmd: action to be performed (start or stop)
- *
- * This is called when the PCM is started, stopped or paused. The action
- * indicated action is specified in the second argument, SNDRV_PCM_TRIGGER_XXX
- *
- * Returns 0 on success or error code otherwise.
- */
-static int pcm_trigger(struct snd_pcm_substream *substream, int cmd)
-{
-
-    switch (cmd) {
-	case SNDRV_PCM_TRIGGER_START:
-	case SNDRV_PCM_TRIGGER_RESUME:
-	case SNDRV_PCM_TRIGGER_STOP:
-	case SNDRV_PCM_TRIGGER_SUSPEND:
-		break;
-	default:
-		D_ERRORK("Invalid trigger cmd: %d", cmd);
-		return -EINVAL;
-	}
-
-	// Do additional Audio Path 'trigger' callback
-	return DO_AUDIOPATH_CB(
-		(struct snd_avirt_audiopath *)PRIVATE_DATA(substream)->audiopath,
-			trigger, substream, cmd);
-}
-
-/**
- * pcm_pointer - Implements 'pointer' callback for PCM middle layer
- * @substream: pointer to ALSA PCM substream
- *
- * This gets called when the user space needs a DMA buffer index. IO errors will
- * be generated if the index does not increment, or drives beyond the frame
- * threshold of the buffer itself.
- *
- * Returns the current hardware buffer frame index.
- */
-static snd_pcm_uframes_t pcm_pointer(struct snd_pcm_substream *substream)
-{
-	// Do additional Audio Path 'pointer' callback
-	return DO_AUDIOPATH_CB(
-		(struct snd_avirt_audiopath *)PRIVATE_DATA(substream)->audiopath,
-			pointer, substream);
-}
-
-/**
- * pcm_pointer - Implements 'get_time_info' callback for PCM middle layer
- * @substream: pointer to ALSA PCM substream
- * @system_ts
- * @audio_ts
- * @audio_tstamp_config
- * @audio_tstamp_report
- *
- * Generic way to get system timestamp and audio timestamp info
- *
- * Returns 0 on success or error code otherwise
- */
-static int
-pcm_get_time_info(struct snd_pcm_substream *substream,
-		  struct timespec *system_ts, struct timespec *audio_ts,
-		  struct snd_pcm_audio_tstamp_config *audio_tstamp_config,
-		  struct snd_pcm_audio_tstamp_report *audio_tstamp_report)
-{
-	return DO_AUDIOPATH_CB(
-		(struct snd_avirt_audiopath *)PRIVATE_DATA(substream)->audiopath,
-		get_time_info, substream, system_ts, audio_ts,
-		audio_tstamp_config, audio_tstamp_report);
-}
-
-/**
- * pcm_copy_user - Implements 'copy_user' callback for PCM middle layer
- * @substream: pointer to ALSA PCM substream
- * @channel:
- * @pos: The offset in the DMA
- * @src: Audio PCM data from the user space
- * @count:
- *
- * This is where we need to copy user audio PCM data into the sound driver
- *
- * Returns 0 on success or error code otherwise.
- *
- */
-static int pcm_copy_user(struct snd_pcm_substream *substream, int channel,
-			 snd_pcm_uframes_t pos, void __user *src,
-			 snd_pcm_uframes_t count)
-{
-
-	// struct snd_pcm_runtime *runtime;
-	// int offset;
-
-	// runtime = substream->runtime;
-	// offset = frames_to_bytes(runtime, pos);
-
-	// Do additional Audio Path 'copy_user' callback
-	return DO_AUDIOPATH_CB(
-		(struct snd_avirt_audiopath *)PRIVATE_DATA(substream)->audiopath,
-			copy_user, substream, channel, pos, src, count);
-}
-
-/**
- * pcm_copy_kernel - Implements 'copy_kernel' callback for PCM middle layer
- * @substream: pointer to ALSA PCM substream
- * @channel:
- * @pos: The offset in the DMA
- * @src: Audio PCM data from the user space
- * @count:
- *
- * This is where we need to copy kernel audio PCM data into the sound driver
- *
- * Returns 0 on success or error code otherwise.
- *
- */
-static int pcm_copy_kernel(struct snd_pcm_substream *substream, int channel,
-			   unsigned long pos, void *buf, unsigned long count)
-{
-	return DO_AUDIOPATH_CB(
-		(struct snd_avirt_audiopath *)PRIVATE_DATA(substream)->audiopath,
-			copy_kernel, substream, channel, pos, buf, count);
-}
-
-/**
- * pcm_ack - Implements 'ack' callback for PCM middle layer
- * @substream: pointer to ALSA PCM substream
- *
- * This is where we need to ack
- *
- * Returns 0 on success or error code otherwise.
- *
- */
-static int pcm_ack(struct snd_pcm_substream *substream)
-{
-	return DO_AUDIOPATH_CB(
-		(struct snd_avirt_audiopath *)PRIVATE_DATA(substream)->audiopath,
-			ack, substream);
-}
-
-static int pcm_silence(struct snd_pcm_substream *substream, int channel,
-		       snd_pcm_uframes_t pos, snd_pcm_uframes_t count)
-{
-	return DO_AUDIOPATH_CB(
-		(struct snd_avirt_audiopath *)PRIVATE_DATA(substream)->audiopath,
-			fill_silence, substream, channel, pos, count);
-}
-
-/**
- * pcm_mmap - Implements 'mmap' callback for PCM middle layer
- * @substream: pointer to ALSA PCM substream
- *
- * This is where we need to ack
- *
- * Returns 0 on success or error code otherwise.
- *
- */
-static int pcm_mmap(struct snd_pcm_substream *substream,
-		    struct vm_area_struct *vma)
-{
-	return DO_AUDIOPATH_CB(
-		(struct snd_avirt_audiopath *)PRIVATE_DATA(substream)->audiopath,
-			mmap, substream, vma);
-}
-
-struct snd_pcm_ops pcm_ops = {
+struct snd_pcm_ops pcm_ops_avirt = {
 	.open = pcm_open,
-	.close = pcm_close,
 	.ioctl = snd_pcm_lib_ioctl,
 	.hw_params = pcm_hw_params,
 	.hw_free = pcm_hw_free,
-	.prepare = pcm_prepare,
-	.trigger = pcm_trigger,
-	.pointer = pcm_pointer,
-	.get_time_info = pcm_get_time_info,
-	.fill_silence = pcm_silence,
-	.copy_user = pcm_copy_user,
-	.copy_kernel = pcm_copy_kernel,
 	.page = snd_pcm_lib_get_vmalloc_page,
-	.ack = pcm_ack,
-	.mmap = pcm_mmap,
 };
diff --git a/sound/avirt.h b/sound/avirt.h
index fd4ae32..0ebac86 100644
--- a/sound/avirt.h
+++ b/sound/avirt.h
@@ -44,7 +44,8 @@ struct snd_avirt_audiopath {
 	const char *name; /* Pretty name */
 	unsigned int version[3]; /* Version - Major.Minor.Ext */
 	const struct snd_pcm_hardware *hw; /* ALSA PCM HW conf */
-	const struct snd_pcm_ops *pcm_ops; /* ALSA PCM op table */
+	const struct snd_pcm_ops *pcm_playback_ops; /* ALSA PCM playback ops */
+	const struct snd_pcm_ops *pcm_capture_ops; /* ALSA PCM capture ops */
 	snd_avirt_audiopath_configure configure; /* Config callback function */
 
 	void *context;