Skip to content

Commit

Permalink
ASoC: meson: tdm fixes
Browse files Browse the repository at this point in the history
Merge series from Jerome Brunet <jbrunet@baylibre.com>:

This patchset fixes 2 problems on TDM which both find a solution
by properly implementing the .trigger() callback for the TDM backend.

ATM, enabling the TDM formatters is done by the .prepare() callback
because handling the formatter is slow due to necessary calls to CCF.

The first problem affects the TDMIN. Because .prepare() is called on DPCM
backend first, the formatter are started before the FIFOs and this may
cause a random channel shifts if the TDMIN use multiple lanes with more
than 2 slots per lanes. Using trigger() allows to set the FE/BE order,
solving the problem.

There has already been an attempt to fix this 3y ago [1] and reverted [2]
It triggered a 'sleep in irq' error on the period IRQ. The solution is
to just use the bottom half of threaded IRQ. This is patch #1. Patch #2
and #3 remain mostly the same as 3y ago.

For TDMOUT, the problem is on pause. ATM pause only stops the FIFO and
the TDMOUT just starves. When it does, it will actually repeat the last
sample continuously. Depending on the platform, if there is no high-pass
filter on the analog path, this may translate to a constant position of
the speaker membrane. There is no audible glitch but it may damage the
speaker coil.

Properly stopping the TDMOUT in pause solves the problem. There is
behaviour change associated with that fix. Clocks used to be continuous
on pause because of the problem above. They will now be gated on pause by
default, as they should. The last change introduce the proper support for
continuous clocks, if needed.

[1]: https://lore.kernel.org/linux-amlogic/20211020114217.133153-1-jbrunet@baylibre.com
[2]: https://lore.kernel.org/linux-amlogic/20220421155725.2589089-1-narmstrong@baylibre.com
  • Loading branch information
broonie committed Apr 30, 2024
2 parents fbd741f + a5a8903 commit c5782bb
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 20 deletions.
1 change: 1 addition & 0 deletions sound/soc/meson/axg-card.c
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ static int axg_card_add_link(struct snd_soc_card *card, struct device_node *np,

dai_link->cpus = cpu;
dai_link->num_cpus = 1;
dai_link->nonatomic = true;

ret = meson_card_parse_dai(card, np, dai_link->cpus);
if (ret)
Expand Down
29 changes: 19 additions & 10 deletions sound/soc/meson/axg-fifo.c
Original file line number Diff line number Diff line change
Expand Up @@ -204,18 +204,26 @@ static irqreturn_t axg_fifo_pcm_irq_block(int irq, void *dev_id)
unsigned int status;

regmap_read(fifo->map, FIFO_STATUS1, &status);

status = FIELD_GET(STATUS1_INT_STS, status);
axg_fifo_ack_irq(fifo, status);

/* Use the thread to call period elapsed on nonatomic links */
if (status & FIFO_INT_COUNT_REPEAT)
snd_pcm_period_elapsed(ss);
else
dev_dbg(axg_fifo_dev(ss), "unexpected irq - STS 0x%02x\n",
status);
return IRQ_WAKE_THREAD;

/* Ack irqs */
axg_fifo_ack_irq(fifo, status);
dev_dbg(axg_fifo_dev(ss), "unexpected irq - STS 0x%02x\n",
status);

return IRQ_NONE;
}

static irqreturn_t axg_fifo_pcm_irq_block_thread(int irq, void *dev_id)
{
struct snd_pcm_substream *ss = dev_id;

snd_pcm_period_elapsed(ss);

return IRQ_RETVAL(status);
return IRQ_HANDLED;
}

int axg_fifo_pcm_open(struct snd_soc_component *component,
Expand Down Expand Up @@ -243,8 +251,9 @@ int axg_fifo_pcm_open(struct snd_soc_component *component,
if (ret)
return ret;

ret = request_irq(fifo->irq, axg_fifo_pcm_irq_block, 0,
dev_name(dev), ss);
ret = request_threaded_irq(fifo->irq, axg_fifo_pcm_irq_block,
axg_fifo_pcm_irq_block_thread,
IRQF_ONESHOT, dev_name(dev), ss);
if (ret)
return ret;

Expand Down
40 changes: 40 additions & 0 deletions sound/soc/meson/axg-tdm-formatter.c
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,46 @@ void axg_tdm_stream_free(struct axg_tdm_stream *ts)
}
EXPORT_SYMBOL_GPL(axg_tdm_stream_free);

int axg_tdm_stream_set_cont_clocks(struct axg_tdm_stream *ts,
unsigned int fmt)
{
int ret = 0;

if (fmt & SND_SOC_DAIFMT_CONT) {
/* Clock are already enabled - skipping */
if (ts->clk_enabled)
return 0;

ret = clk_prepare_enable(ts->iface->mclk);
if (ret)
return ret;

ret = clk_prepare_enable(ts->iface->sclk);
if (ret)
goto err_sclk;

ret = clk_prepare_enable(ts->iface->lrclk);
if (ret)
goto err_lrclk;

ts->clk_enabled = true;
return 0;
}

/* Clocks are already disabled - skipping */
if (!ts->clk_enabled)
return 0;

clk_disable_unprepare(ts->iface->lrclk);
err_lrclk:
clk_disable_unprepare(ts->iface->sclk);
err_sclk:
clk_disable_unprepare(ts->iface->mclk);
ts->clk_enabled = false;
return ret;
}
EXPORT_SYMBOL_GPL(axg_tdm_stream_set_cont_clocks);

MODULE_DESCRIPTION("Amlogic AXG TDM formatter driver");
MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>");
MODULE_LICENSE("GPL v2");
38 changes: 28 additions & 10 deletions sound/soc/meson/axg-tdm-interface.c
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ static int axg_tdm_iface_hw_params(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
struct axg_tdm_iface *iface = snd_soc_dai_get_drvdata(dai);
struct axg_tdm_stream *ts = snd_soc_dai_get_dma_data(dai, substream);
int ret;

switch (iface->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
Expand Down Expand Up @@ -346,27 +347,44 @@ static int axg_tdm_iface_hw_params(struct snd_pcm_substream *substream,
return ret;
}

return 0;
ret = axg_tdm_stream_set_cont_clocks(ts, iface->fmt);
if (ret)
dev_err(dai->dev, "failed to apply continuous clock setting\n");

return ret;
}

static int axg_tdm_iface_hw_free(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
struct axg_tdm_stream *ts = snd_soc_dai_get_dma_data(dai, substream);

/* Stop all attached formatters */
axg_tdm_stream_stop(ts);

return 0;
return axg_tdm_stream_set_cont_clocks(ts, 0);
}

static int axg_tdm_iface_prepare(struct snd_pcm_substream *substream,
static int axg_tdm_iface_trigger(struct snd_pcm_substream *substream,
int cmd,
struct snd_soc_dai *dai)
{
struct axg_tdm_stream *ts = snd_soc_dai_get_dma_data(dai, substream);
struct axg_tdm_stream *ts =
snd_soc_dai_get_dma_data(dai, substream);

switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_RESUME:
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
axg_tdm_stream_start(ts);
break;
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
case SNDRV_PCM_TRIGGER_STOP:
axg_tdm_stream_stop(ts);
break;
default:
return -EINVAL;
}

/* Force all attached formatters to update */
return axg_tdm_stream_reset(ts);
return 0;
}

static int axg_tdm_iface_remove_dai(struct snd_soc_dai *dai)
Expand Down Expand Up @@ -412,8 +430,8 @@ static const struct snd_soc_dai_ops axg_tdm_iface_ops = {
.set_fmt = axg_tdm_iface_set_fmt,
.startup = axg_tdm_iface_startup,
.hw_params = axg_tdm_iface_hw_params,
.prepare = axg_tdm_iface_prepare,
.hw_free = axg_tdm_iface_hw_free,
.trigger = axg_tdm_iface_trigger,
};

/* TDM Backend DAIs */
Expand Down
5 changes: 5 additions & 0 deletions sound/soc/meson/axg-tdm.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,17 @@ struct axg_tdm_stream {
unsigned int physical_width;
u32 *mask;
bool ready;

/* For continuous clock tracking */
bool clk_enabled;
};

struct axg_tdm_stream *axg_tdm_stream_alloc(struct axg_tdm_iface *iface);
void axg_tdm_stream_free(struct axg_tdm_stream *ts);
int axg_tdm_stream_start(struct axg_tdm_stream *ts);
void axg_tdm_stream_stop(struct axg_tdm_stream *ts);
int axg_tdm_stream_set_cont_clocks(struct axg_tdm_stream *ts,
unsigned int fmt);

static inline int axg_tdm_stream_reset(struct axg_tdm_stream *ts)
{
Expand Down

0 comments on commit c5782bb

Please sign in to comment.