From 31e3bcd1936bef02f1d39e4dffa09e0519943367 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Thu, 25 Apr 2013 17:23:55 +0200 Subject: [PATCH 1/5] ALSA: bcm2835: show card and subdev creation msg only on debug On driver probe a message showing that a card and its subdevices have been created is shown. Probably this message is not needed unless we have debug enabled on the driver. So, use the driver audio_info() debug macro instead of just printk(). Also is better to use dev_err() and pr_err() instead KERN_ERR. Signed-off-by: Javier Martinez Canillas --- sound/arm/bcm2835.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/sound/arm/bcm2835.c b/sound/arm/bcm2835.c index 9546fe61b28188..08628e506baa3f 100755 --- a/sound/arm/bcm2835.c +++ b/sound/arm/bcm2835.c @@ -110,20 +110,20 @@ static int __devinit snd_bcm2835_alsa_probe(struct platform_device *pdev) err = snd_bcm2835_create(g_card, pdev, &chip); if (err < 0) { - printk(KERN_ERR "Failed to create bcm2835 chip\n"); + dev_err(&pdev->dev, "Failed to create bcm2835 chip\n"); goto out_bcm2835_create; } g_chip = chip; err = snd_bcm2835_new_pcm(chip); if (err < 0) { - printk(KERN_ERR "Failed to create new BCM2835 pcm device\n"); + dev_err(&pdev->dev, "Failed to create new BCM2835 pcm device\n"); goto out_bcm2835_new_pcm; } err = snd_bcm2835_new_ctl(chip); if (err < 0) { - printk(KERN_ERR "Failed to create new BCM2835 ctl\n"); + dev_err(&pdev->dev, "Failed to create new BCM2835 ctl\n"); goto out_bcm2835_new_ctl; } @@ -139,14 +139,14 @@ static int __devinit snd_bcm2835_alsa_probe(struct platform_device *pdev) if (dev == 0) { err = snd_card_register(card); if (err < 0) { - printk(KERN_ERR - "Failed to register bcm2835 ALSA card \n"); + dev_err(&pdev->dev, + "Failed to register bcm2835 ALSA card \n"); goto out_card_register; } platform_set_drvdata(pdev, card); - printk(KERN_INFO "bcm2835 ALSA card created!\n"); + audio_info("bcm2835 ALSA card created!\n"); } else { - printk(KERN_INFO "bcm2835 ALSA chip created!\n"); + audio_info("bcm2835 ALSA chip created!\n"); platform_set_drvdata(pdev, (void *)dev); } @@ -160,11 +160,11 @@ static int __devinit snd_bcm2835_alsa_probe(struct platform_device *pdev) out_bcm2835_create: BUG_ON(!g_card); if (snd_card_free(g_card)) - printk(KERN_ERR "Failed to free Registered alsa card\n"); + dev_err(&pdev->dev, "Failed to free Registered alsa card\n"); g_card = NULL; out: dev = SNDRV_CARDS; /* stop more avail_substreams from being probed */ - printk(KERN_ERR "BCM2835 ALSA Probe failed !!\n"); + dev_err(&pdev->dev, "BCM2835 ALSA Probe failed !!\n"); return err; } @@ -326,49 +326,49 @@ static int __devinit bcm2835_alsa_device_init(void) int err; err = platform_driver_register(&bcm2835_alsa0_driver); if (err) { - printk("Error registering bcm2835_alsa0_driver %d .\n", err); + pr_err("Error registering bcm2835_alsa0_driver %d .\n", err); goto out; } err = platform_driver_register(&bcm2835_alsa1_driver); if (err) { - printk("Error registering bcm2835_alsa1_driver %d .\n", err); + pr_err("Error registering bcm2835_alsa0_driver %d .\n", err); goto unregister_0; } err = platform_driver_register(&bcm2835_alsa2_driver); if (err) { - printk("Error registering bcm2835_alsa2_driver %d .\n", err); + pr_err("Error registering bcm2835_alsa0_driver %d .\n", err); goto unregister_1; } err = platform_driver_register(&bcm2835_alsa3_driver); if (err) { - printk("Error registering bcm2835_alsa3_driver %d .\n", err); + pr_err("Error registering bcm2835_alsa0_driver %d .\n", err); goto unregister_2; } err = platform_driver_register(&bcm2835_alsa4_driver); if (err) { - printk("Error registering bcm2835_alsa4_driver %d .\n", err); + pr_err("Error registering bcm2835_alsa0_driver %d .\n", err); goto unregister_3; } err = platform_driver_register(&bcm2835_alsa5_driver); if (err) { - printk("Error registering bcm2835_alsa5_driver %d .\n", err); + pr_err("Error registering bcm2835_alsa0_driver %d .\n", err); goto unregister_4; } err = platform_driver_register(&bcm2835_alsa6_driver); if (err) { - printk("Error registering bcm2835_alsa6_driver %d .\n", err); + pr_err("Error registering bcm2835_alsa0_driver %d .\n", err); goto unregister_5; } err = platform_driver_register(&bcm2835_alsa7_driver); if (err) { - printk("Error registering bcm2835_alsa7_driver %d .\n", err); + pr_err("Error registering bcm2835_alsa0_driver %d .\n", err); goto unregister_6; } From a9cc473fd88cd3af0b948be0a9cac35e8ba761b9 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Wed, 24 Apr 2013 23:11:24 +0200 Subject: [PATCH 2/5] ALSA: bcm2835: use a completion instead of a semaphore for sync The kernel completion interface is a simple synchronization mechanism that is preferable over semaphores to signal the occurence of an event. Semaphores are optimized for the non-contention case since in most situations they will be available. But the wait for completion is the opposite case since the code calling down will wait on most cases. So, is better to use the completion mechanism that has been developed for this exact use case instead of semaphores. Signed-off-by: Javier Martinez Canillas --- sound/arm/bcm2835-vchiq.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/sound/arm/bcm2835-vchiq.c b/sound/arm/bcm2835-vchiq.c index 9ecb2d6a25fe0b..c7ccc154d64252 100755 --- a/sound/arm/bcm2835-vchiq.c +++ b/sound/arm/bcm2835-vchiq.c @@ -27,6 +27,7 @@ #include #include #include +#include #include "bcm2835.h" @@ -53,7 +54,7 @@ typedef struct opaque_AUDIO_INSTANCE_T { uint32_t num_connections; VCHI_SERVICE_HANDLE_T vchi_handle[VCHI_MAX_NUM_CONNECTIONS]; - struct semaphore msg_avail_event; + struct completion msg_avail_comp; struct mutex vchi_mutex; bcm2835_alsa_stream_t *alsa_stream; int32_t result; @@ -178,7 +179,7 @@ static void audio_vchi_callback(void *param, (" .. instance=%p, m.type=VC_AUDIO_MSG_TYPE_RESULT, success=%d\n", instance, m.u.result.success); instance->result = m.u.result.success; - up(&instance->msg_avail_event); + complete(&instance->msg_avail_comp); } else if (m.type == VC_AUDIO_MSG_TYPE_COMPLETE) { irq_handler_t callback = (irq_handler_t) m.u.complete.callback; LOG_DBG @@ -435,8 +436,8 @@ static int bcm2835_audio_set_ctls_chan(bcm2835_alsa_stream_t * alsa_stream, m.u.control.dest = chip->dest; m.u.control.volume = chip->volume; - /* Create the message available event */ - sema_init(&instance->msg_avail_event, 0); + /* Create the message available completion */ + init_completion(&instance->msg_avail_comp); /* Send the message to the videocore */ success = vchi_msg_queue(instance->vchi_handle[0], @@ -452,11 +453,10 @@ static int bcm2835_audio_set_ctls_chan(bcm2835_alsa_stream_t * alsa_stream, } /* We are expecting a reply from the videocore */ - if (down_interruptible(&instance->msg_avail_event)) { + ret = wait_for_completion_interruptible(&instance->msg_avail_comp); + if (ret) { LOG_ERR("%s: failed on waiting for event (status=%d)\n", __func__, success); - - ret = -1; goto unlock; } @@ -539,8 +539,8 @@ int bcm2835_audio_set_params(bcm2835_alsa_stream_t * alsa_stream, m.u.config.samplerate = samplerate; m.u.config.bps = bps; - /* Create the message available event */ - sema_init(&instance->msg_avail_event, 0); + /* Create the message available completion */ + init_completion(&instance->msg_avail_comp); /* Send the message to the videocore */ success = vchi_msg_queue(instance->vchi_handle[0], @@ -556,11 +556,10 @@ int bcm2835_audio_set_params(bcm2835_alsa_stream_t * alsa_stream, } /* We are expecting a reply from the videocore */ - if (down_interruptible(&instance->msg_avail_event)) { + ret = wait_for_completion_interruptible(&instance->msg_avail_comp); + if (ret) { LOG_ERR("%s: failed on waiting for event (status=%d)\n", __func__, success); - - ret = -1; goto unlock; } @@ -688,8 +687,8 @@ int bcm2835_audio_close(bcm2835_alsa_stream_t * alsa_stream) m.type = VC_AUDIO_MSG_TYPE_CLOSE; - /* Create the message available event */ - sema_init(&instance->msg_avail_event, 0); + /* Create the message available completion */ + init_completion(&instance->msg_avail_comp); /* Send the message to the videocore */ success = vchi_msg_queue(instance->vchi_handle[0], @@ -702,11 +701,11 @@ int bcm2835_audio_close(bcm2835_alsa_stream_t * alsa_stream) ret = -1; goto unlock; } - if (down_interruptible(&instance->msg_avail_event)) { + + ret = wait_for_completion_interruptible(&instance->msg_avail_comp); + if (ret) { LOG_ERR("%s: failed on waiting for event (status=%d)", __func__, success); - - ret = -1; goto unlock; } if (instance->result != 0) { From fd22e4c96c585a384f713d52a2b2cc9e14ec4a05 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Wed, 24 Apr 2013 13:40:53 +0200 Subject: [PATCH 3/5] ALSA: bcm2835: don't use magic numbers on audio start/stop workqueue The bcm2835 driver uses a workqueue to defer PCM playback start and stop. Commands are passed to the function as magic numbers so is more readable to use macro constants. Also, while being there change the generic name "x" to "cmd" for the var used to pass the start/stop commands. Signed-off-by: Javier Martinez Canillas --- sound/arm/bcm2835-vchiq.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/sound/arm/bcm2835-vchiq.c b/sound/arm/bcm2835-vchiq.c index c7ccc154d64252..c839546284265f 100755 --- a/sound/arm/bcm2835-vchiq.c +++ b/sound/arm/bcm2835-vchiq.c @@ -38,6 +38,9 @@ /* ---- Private Constants and Types ------------------------------------------ */ +#define BCM2835_AUDIO_STOP 0 +#define BCM2835_AUDIO_START 1 + /* Logging macros (for remapping to other logging mechanisms, i.e., printf) */ #ifdef AUDIO_DEBUG_ENABLE #define LOG_ERR( fmt, arg... ) pr_err( "%s:%d " fmt, __func__, __LINE__, ##arg) @@ -75,23 +78,23 @@ static int bcm2835_audio_start_worker(bcm2835_alsa_stream_t * alsa_stream); typedef struct { struct work_struct my_work; bcm2835_alsa_stream_t *alsa_stream; - int x; + int cmd; } my_work_t; static void my_wq_function(struct work_struct *work) { my_work_t *w = (my_work_t *) work; int ret = -9; - LOG_DBG(" .. IN %p:%d\n", w->alsa_stream, w->x); - switch (w->x) { - case 1: + LOG_DBG(" .. IN %p:%d\n", w->alsa_stream, w->cmd); + switch (w->cmd) { + case BCM2835_AUDIO_START: ret = bcm2835_audio_start_worker(w->alsa_stream); break; - case 2: + case BCM2835_AUDIO_STOP: ret = bcm2835_audio_stop_worker(w->alsa_stream); break; default: - LOG_ERR(" Unexpected work: %p:%d\n", w->alsa_stream, w->x); + LOG_ERR(" Unexpected work: %p:%d\n", w->alsa_stream, w->cmd); break; } kfree((void *)work); @@ -108,7 +111,7 @@ int bcm2835_audio_start(bcm2835_alsa_stream_t * alsa_stream) if (work) { INIT_WORK((struct work_struct *)work, my_wq_function); work->alsa_stream = alsa_stream; - work->x = 1; + work->cmd = BCM2835_AUDIO_START; if (queue_work (alsa_stream->my_wq, (struct work_struct *)work)) ret = 0; @@ -129,7 +132,7 @@ int bcm2835_audio_stop(bcm2835_alsa_stream_t * alsa_stream) if (work) { INIT_WORK((struct work_struct *)work, my_wq_function); work->alsa_stream = alsa_stream; - work->x = 2; + work->cmd = BCM2835_AUDIO_STOP; if (queue_work (alsa_stream->my_wq, (struct work_struct *)work)) ret = 0; From 24f64ad5010b10dedd83573c00446c39f963ef3e Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Thu, 25 Apr 2013 16:45:25 +0200 Subject: [PATCH 4/5] ALSA: bcm2835: defer bcm2835 PCM write using the workqueue Only PCM playback and stop are deferred using a workqueue on the ALSA bcm2835 driver. The actual writing of PCM samples is synchronous. This works well for the read/write transfer method since the snd_pcm_ops .copy function pointer runs on process context. But the snd_pcm_ops .ack function pointer used to implement direct read/write transfer using a memory-mapped I/O, runs with interrupts disabled. Since the Kernel to VideoCore interface driver has to be able to sleep, PCM write has to be deferred to in preparation to add mmap support to the driver. Signed-off-by: Javier Martinez Canillas --- sound/arm/bcm2835-vchiq.c | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/sound/arm/bcm2835-vchiq.c b/sound/arm/bcm2835-vchiq.c index c839546284265f..169d8999468fc1 100755 --- a/sound/arm/bcm2835-vchiq.c +++ b/sound/arm/bcm2835-vchiq.c @@ -40,6 +40,7 @@ #define BCM2835_AUDIO_STOP 0 #define BCM2835_AUDIO_START 1 +#define BCM2835_AUDIO_WRITE 2 /* Logging macros (for remapping to other logging mechanisms, i.e., printf) */ #ifdef AUDIO_DEBUG_ENABLE @@ -74,11 +75,15 @@ bool force_bulk = false; static int bcm2835_audio_stop_worker(bcm2835_alsa_stream_t * alsa_stream); static int bcm2835_audio_start_worker(bcm2835_alsa_stream_t * alsa_stream); +static int bcm2835_audio_write_worker(bcm2835_alsa_stream_t *alsa_stream, + uint32_t count, void *src); typedef struct { struct work_struct my_work; bcm2835_alsa_stream_t *alsa_stream; int cmd; + void *src; + uint32_t count; } my_work_t; static void my_wq_function(struct work_struct *work) @@ -93,6 +98,10 @@ static void my_wq_function(struct work_struct *work) case BCM2835_AUDIO_STOP: ret = bcm2835_audio_stop_worker(w->alsa_stream); break; + case BCM2835_AUDIO_WRITE: + ret = bcm2835_audio_write_worker(w->alsa_stream, w->count, + w->src); + break; default: LOG_ERR(" Unexpected work: %p:%d\n", w->alsa_stream, w->cmd); break; @@ -143,6 +152,30 @@ int bcm2835_audio_stop(bcm2835_alsa_stream_t * alsa_stream) return ret; } +int bcm2835_audio_write(bcm2835_alsa_stream_t *alsa_stream, + uint32_t count, void *src) +{ + int ret = -1; + LOG_DBG(" .. IN\n"); + if (alsa_stream->my_wq) { + my_work_t *work = kmalloc(sizeof(my_work_t), GFP_ATOMIC); + /*--- Queue some work (item 1) ---*/ + if (work) { + INIT_WORK((struct work_struct *)work, my_wq_function); + work->alsa_stream = alsa_stream; + work->cmd = BCM2835_AUDIO_WRITE; + work->src = src; + work->count = count; + if (queue_work + (alsa_stream->my_wq, (struct work_struct *)work)) + ret = 0; + } else + LOG_ERR(" .. Error: NULL work kmalloc\n"); + } + LOG_DBG(" .. OUT %d\n", ret); + return ret; +} + void my_workqueue_init(bcm2835_alsa_stream_t * alsa_stream) { alsa_stream->my_wq = create_workqueue("my_queue"); @@ -734,8 +767,8 @@ int bcm2835_audio_close(bcm2835_alsa_stream_t * alsa_stream) return ret; } -int bcm2835_audio_write(bcm2835_alsa_stream_t * alsa_stream, uint32_t count, - void *src) +int bcm2835_audio_write_worker(bcm2835_alsa_stream_t *alsa_stream, + uint32_t count, void *src) { VC_AUDIO_MSG_T m; AUDIO_INSTANCE_T *instance = alsa_stream->instance; From bad49c21520ddc40d512a0d424cbc94f20068e6c Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Sat, 20 Apr 2013 02:31:56 +0200 Subject: [PATCH 5/5] ALSA: bcm2835: add memory-mapped I/O mode for audio stream ALSA supports two transfers methods for PCM playback: Read/Write transfer where samples are writtern to the device using standard read and write functions and Direct Read/Write transfers where samples can be written directly to a mapped memory area and the driver is signaled once this has been done. The bcm2835 driver only supported Read/Write transfer method so this patch adds mmap support to the driver. The ARM CPU is not able to directly address the audio device hardware buffer so audio samples are sent to the device using a message passing interface (vchiq). Since hardware buffers can't be directly mapped to user-space memory, an intermediate buffer (using the PCM indirect API) is needed to store the audio samples and push them to the device through videocore. Signed-off-by: Javier Martinez Canillas --- sound/arm/bcm2835-pcm.c | 69 +++++++++++++++++++++++++---------------- sound/arm/bcm2835.h | 2 ++ 2 files changed, 45 insertions(+), 26 deletions(-) diff --git a/sound/arm/bcm2835-pcm.c b/sound/arm/bcm2835-pcm.c index 8d182f0f58f887..57f0b4a5604c87 100755 --- a/sound/arm/bcm2835-pcm.c +++ b/sound/arm/bcm2835-pcm.c @@ -19,7 +19,8 @@ /* hardware definition */ static struct snd_pcm_hardware snd_bcm2835_playback_hw = { - .info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER), + .info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER | + SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID), .formats = SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S16_LE, .rates = SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_8000_48000, .rate_min = 8000, @@ -251,6 +252,12 @@ static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream *substream) audio_info(" .. IN\n"); + memset(&alsa_stream->pcm_indirect, 0, sizeof(alsa_stream->pcm_indirect)); + + alsa_stream->pcm_indirect.hw_buffer_size = + alsa_stream->pcm_indirect.sw_buffer_size = + snd_pcm_lib_buffer_bytes(substream); + alsa_stream->buffer_size = snd_pcm_lib_buffer_bytes(substream); alsa_stream->period_size = snd_pcm_lib_period_bytes(substream); alsa_stream->pos = 0; @@ -263,6 +270,32 @@ static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream *substream) return 0; } +static void snd_bcm2835_pcm_transfer(struct snd_pcm_substream *substream, + struct snd_pcm_indirect *rec, size_t bytes) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + bcm2835_alsa_stream_t *alsa_stream = runtime->private_data; + void *src = (void *)(substream->runtime->dma_area + rec->sw_data); + int err; + + err = bcm2835_audio_write(alsa_stream, bytes, src); + if (err) + audio_error(" Failed to transfer to alsa device (%d)\n", err); + +} + +static int snd_bcm2835_pcm_ack(struct snd_pcm_substream *substream) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + bcm2835_alsa_stream_t *alsa_stream = runtime->private_data; + struct snd_pcm_indirect *pcm_indirect = &alsa_stream->pcm_indirect; + + pcm_indirect->hw_queue_size = runtime->hw.buffer_bytes_max; + snd_pcm_indirect_playback_transfer(substream, pcm_indirect, + snd_bcm2835_pcm_transfer); + return 0; +} + /* trigger callback */ static int snd_bcm2835_pcm_trigger(struct snd_pcm_substream *substream, int cmd) { @@ -279,6 +312,11 @@ static int snd_bcm2835_pcm_trigger(struct snd_pcm_substream *substream, int cmd) if (!alsa_stream->running) { err = bcm2835_audio_start(alsa_stream); if (err == 0) { + alsa_stream->pcm_indirect.hw_io = + alsa_stream->pcm_indirect.hw_data = + bytes_to_frames(runtime, + alsa_stream->pos); + substream->ops->ack(substream); alsa_stream->running = 1; alsa_stream->draining = 1; } else { @@ -327,30 +365,9 @@ snd_bcm2835_pcm_pointer(struct snd_pcm_substream *substream) alsa_stream->pos); audio_info(" .. OUT\n"); - return bytes_to_frames(runtime, alsa_stream->pos); -} - -static int snd_bcm2835_pcm_copy(struct snd_pcm_substream *substream, - int channel, snd_pcm_uframes_t pos, void *src, - snd_pcm_uframes_t count) -{ - int ret; - struct snd_pcm_runtime *runtime = substream->runtime; - bcm2835_alsa_stream_t *alsa_stream = runtime->private_data; - - audio_info(" .. IN\n"); - audio_debug("copy.......... (%d) hwptr=%d appl=%d pos=%d\n", - frames_to_bytes(runtime, count), frames_to_bytes(runtime, - runtime-> - status-> - hw_ptr), - frames_to_bytes(runtime, runtime->control->appl_ptr), - alsa_stream->pos); - ret = - bcm2835_audio_write(alsa_stream, frames_to_bytes(runtime, count), - src); - audio_info(" .. OUT\n"); - return ret; + return snd_pcm_indirect_playback_pointer(substream, + &alsa_stream->pcm_indirect, + alsa_stream->pos); } static int snd_bcm2835_pcm_lib_ioctl(struct snd_pcm_substream *substream, @@ -372,7 +389,7 @@ static struct snd_pcm_ops snd_bcm2835_playback_ops = { .prepare = snd_bcm2835_pcm_prepare, .trigger = snd_bcm2835_pcm_trigger, .pointer = snd_bcm2835_pcm_pointer, - .copy = snd_bcm2835_pcm_copy, + .ack = snd_bcm2835_pcm_ack, }; /* create a pcm device */ diff --git a/sound/arm/bcm2835.h b/sound/arm/bcm2835.h index b966e28588fbaa..08c763d2d34f7c 100755 --- a/sound/arm/bcm2835.h +++ b/sound/arm/bcm2835.h @@ -23,6 +23,7 @@ #include #include #include +#include #include /* @@ -110,6 +111,7 @@ typedef struct bcm2835_chip { typedef struct bcm2835_alsa_stream { bcm2835_chip_t *chip; struct snd_pcm_substream *substream; + struct snd_pcm_indirect pcm_indirect; struct semaphore buffers_update_sem; struct semaphore control_sem;