Skip to content

Commit

Permalink
ASoC: q6apm: fix under runs and fragment sizes
Browse files Browse the repository at this point in the history 
Merge series from srinivas.kandagatla@linaro.org:

On Qualcomm Audioreach setup, some of the audio artifacts are seen in
both recording and playback. These patches fix issues by
1. Adjusting the fragment size that dsp can service.
2. schedule available playback buffers in time for dsp to not hit under runs
3. remove some of the manual calculations done to get hardware pointer.

With these patches, am able to see significant Audio quality improvements.
I have few more patches to optimize the dsp drivers, but for now am
keeping this series simple to address the underruns and overruns issues
noticed in pipewire setup.

Any testing would be appreciated.

Please note that on pipewire min-latency has to be set to 512 which
reflects the DSP latency requirements of 10ms. You might see audio
artifacts like glitches if you try to play audio below 256 latency.
  • Loading branch information
Mark Brown committed Mar 31, 2025
2 parents 1ebd494 + a93dad6 commit cc80b44
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 29 deletions.
60 changes: 32 additions & 28 deletions sound/soc/qcom/qdsp6/q6apm-dai.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
#define PLAYBACK_MIN_PERIOD_SIZE 128
#define CAPTURE_MIN_NUM_PERIODS 2
#define CAPTURE_MAX_NUM_PERIODS 8
#define CAPTURE_MAX_PERIOD_SIZE 4096
#define CAPTURE_MIN_PERIOD_SIZE 320
#define CAPTURE_MAX_PERIOD_SIZE 65536
#define CAPTURE_MIN_PERIOD_SIZE 6144
#define BUFFER_BYTES_MAX (PLAYBACK_MAX_NUM_PERIODS * PLAYBACK_MAX_PERIOD_SIZE)
#define BUFFER_BYTES_MIN (PLAYBACK_MIN_NUM_PERIODS * PLAYBACK_MIN_PERIOD_SIZE)
#define COMPR_PLAYBACK_MAX_FRAGMENT_SIZE (128 * 1024)
Expand Down Expand Up @@ -64,12 +64,12 @@ struct q6apm_dai_rtd {
phys_addr_t phys;
unsigned int pcm_size;
unsigned int pcm_count;
unsigned int pos; /* Buffer position */
unsigned int periods;
unsigned int bytes_sent;
unsigned int bytes_received;
unsigned int copied_total;
uint16_t bits_per_sample;
snd_pcm_uframes_t queue_ptr;
bool next_track;
enum stream_state state;
struct q6apm_graph *graph;
Expand Down Expand Up @@ -123,25 +123,16 @@ static void event_handler(uint32_t opcode, uint32_t token, void *payload, void *
{
struct q6apm_dai_rtd *prtd = priv;
struct snd_pcm_substream *substream = prtd->substream;
unsigned long flags;

switch (opcode) {
case APM_CLIENT_EVENT_CMD_EOS_DONE:
prtd->state = Q6APM_STREAM_STOPPED;
break;
case APM_CLIENT_EVENT_DATA_WRITE_DONE:
spin_lock_irqsave(&prtd->lock, flags);
prtd->pos += prtd->pcm_count;
spin_unlock_irqrestore(&prtd->lock, flags);
snd_pcm_period_elapsed(substream);
if (prtd->state == Q6APM_STREAM_RUNNING)
q6apm_write_async(prtd->graph, prtd->pcm_count, 0, 0, 0);

break;
case APM_CLIENT_EVENT_DATA_READ_DONE:
spin_lock_irqsave(&prtd->lock, flags);
prtd->pos += prtd->pcm_count;
spin_unlock_irqrestore(&prtd->lock, flags);
snd_pcm_period_elapsed(substream);
if (prtd->state == Q6APM_STREAM_RUNNING)
q6apm_read(prtd->graph);
Expand Down Expand Up @@ -248,7 +239,6 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
}

prtd->pcm_count = snd_pcm_lib_period_bytes(substream);
prtd->pos = 0;
/* rate and channels are sent to audio driver */
ret = q6apm_graph_media_format_shmem(prtd->graph, &cfg);
if (ret < 0) {
Expand Down Expand Up @@ -294,6 +284,27 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
return 0;
}

static int q6apm_dai_ack(struct snd_soc_component *component, struct snd_pcm_substream *substream)
{
struct snd_pcm_runtime *runtime = substream->runtime;
struct q6apm_dai_rtd *prtd = runtime->private_data;
int i, ret = 0, avail_periods;

if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
avail_periods = (runtime->control->appl_ptr - prtd->queue_ptr)/runtime->period_size;
for (i = 0; i < avail_periods; i++) {
ret = q6apm_write_async(prtd->graph, prtd->pcm_count, 0, 0, NO_TIMESTAMP);
if (ret < 0) {
dev_err(component->dev, "Error queuing playback buffer %d\n", ret);
return ret;
}
prtd->queue_ptr += runtime->period_size;
}
}

return ret;
}

static int q6apm_dai_trigger(struct snd_soc_component *component,
struct snd_pcm_substream *substream, int cmd)
{
Expand All @@ -305,9 +316,6 @@ static int q6apm_dai_trigger(struct snd_soc_component *component,
case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_RESUME:
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
/* start writing buffers for playback only as we already queued capture buffers */
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
ret = q6apm_write_async(prtd->graph, prtd->pcm_count, 0, 0, 0);
break;
case SNDRV_PCM_TRIGGER_STOP:
/* TODO support be handled via SoftPause Module */
Expand Down Expand Up @@ -377,13 +385,14 @@ static int q6apm_dai_open(struct snd_soc_component *component,
}
}

ret = snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 32);
/* setup 10ms latency to accommodate DSP restrictions */
ret = snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_SIZE, 480);
if (ret < 0) {
dev_err(dev, "constraint for period bytes step ret = %d\n", ret);
goto err;
}

ret = snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 32);
ret = snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_BUFFER_SIZE, 480);
if (ret < 0) {
dev_err(dev, "constraint for buffer bytes step ret = %d\n", ret);
goto err;
Expand Down Expand Up @@ -428,16 +437,12 @@ static snd_pcm_uframes_t q6apm_dai_pointer(struct snd_soc_component *component,
struct snd_pcm_runtime *runtime = substream->runtime;
struct q6apm_dai_rtd *prtd = runtime->private_data;
snd_pcm_uframes_t ptr;
unsigned long flags;

spin_lock_irqsave(&prtd->lock, flags);
if (prtd->pos == prtd->pcm_size)
prtd->pos = 0;

ptr = bytes_to_frames(runtime, prtd->pos);
spin_unlock_irqrestore(&prtd->lock, flags);
ptr = q6apm_get_hw_pointer(prtd->graph, substream->stream) * runtime->period_size;
if (ptr)
return ptr - 1;

return ptr;
return 0;
}

static int q6apm_dai_hw_params(struct snd_soc_component *component,
Expand Down Expand Up @@ -652,8 +657,6 @@ static int q6apm_dai_compr_set_params(struct snd_soc_component *component,
prtd->pcm_size = runtime->fragments * runtime->fragment_size;
prtd->bits_per_sample = 16;

prtd->pos = 0;

if (prtd->next_track != true) {
memcpy(&prtd->codec, codec, sizeof(*codec));

Expand Down Expand Up @@ -836,6 +839,7 @@ static const struct snd_soc_component_driver q6apm_fe_dai_component = {
.hw_params = q6apm_dai_hw_params,
.pointer = q6apm_dai_pointer,
.trigger = q6apm_dai_trigger,
.ack = q6apm_dai_ack,
.compress_ops = &q6apm_dai_compress_ops,
.use_dai_pcm_id = true,
};
Expand Down
18 changes: 17 additions & 1 deletion sound/soc/qcom/qdsp6/q6apm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,19 @@ int q6apm_read(struct q6apm_graph *graph)
}
EXPORT_SYMBOL_GPL(q6apm_read);

int q6apm_get_hw_pointer(struct q6apm_graph *graph, int dir)
{
struct audioreach_graph_data *data;

if (dir == SNDRV_PCM_STREAM_PLAYBACK)
data = &graph->rx_data;
else
data = &graph->tx_data;

return (int)atomic_read(&data->hw_ptr);
}
EXPORT_SYMBOL_GPL(q6apm_get_hw_pointer);

static int graph_callback(struct gpr_resp_pkt *data, void *priv, int op)
{
struct data_cmd_rsp_rd_sh_mem_ep_data_buffer_done_v2 *rd_done;
Expand All @@ -520,7 +533,8 @@ static int graph_callback(struct gpr_resp_pkt *data, void *priv, int op)
done = data->payload;
phys = graph->rx_data.buf[token].phys;
mutex_unlock(&graph->lock);

/* token numbering starts at 0 */
atomic_set(&graph->rx_data.hw_ptr, token + 1);
if (lower_32_bits(phys) == done->buf_addr_lsw &&
upper_32_bits(phys) == done->buf_addr_msw) {
graph->result.opcode = hdr->opcode;
Expand Down Expand Up @@ -553,6 +567,8 @@ static int graph_callback(struct gpr_resp_pkt *data, void *priv, int op)
rd_done = data->payload;
phys = graph->tx_data.buf[hdr->token].phys;
mutex_unlock(&graph->lock);
/* token numbering starts at 0 */
atomic_set(&graph->tx_data.hw_ptr, hdr->token + 1);

if (upper_32_bits(phys) == rd_done->buf_addr_msw &&
lower_32_bits(phys) == rd_done->buf_addr_lsw) {
Expand Down
3 changes: 3 additions & 0 deletions sound/soc/qcom/qdsp6/q6apm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#ifndef __Q6APM_H__
#define __Q6APM_H__
#include <linux/types.h>
#include <linux/atomic.h>
#include <linux/slab.h>
#include <linux/wait.h>
#include <linux/kernel.h>
Expand Down Expand Up @@ -77,6 +78,7 @@ struct audioreach_graph_data {
uint32_t num_periods;
uint32_t dsp_buf;
uint32_t mem_map_handle;
atomic_t hw_ptr;
};

struct audioreach_graph {
Expand Down Expand Up @@ -150,4 +152,5 @@ int q6apm_enable_compress_module(struct device *dev, struct q6apm_graph *graph,
int q6apm_remove_initial_silence(struct device *dev, struct q6apm_graph *graph, uint32_t samples);
int q6apm_remove_trailing_silence(struct device *dev, struct q6apm_graph *graph, uint32_t samples);
int q6apm_set_real_module_id(struct device *dev, struct q6apm_graph *graph, uint32_t codec_id);
int q6apm_get_hw_pointer(struct q6apm_graph *graph, int dir);
#endif /* __APM_GRAPH_ */

0 comments on commit cc80b44

Please sign in to comment.