[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAA+D8AN_xej-DWHbWKMCwgZVVcW9N2mHrK=4GUyoRiGLXBAoTw@mail.gmail.com>
Date: Thu, 5 Feb 2026 10:36:01 +0800
From: Shengjiu Wang <shengjiu.wang@...il.com>
To: Frank Li <Frank.li@....com>
Cc: Shengjiu Wang <shengjiu.wang@....com>, lgirdwood@...il.com, broonie@...nel.org,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, shawnguo@...nel.org,
s.hauer@...gutronix.de, kernel@...gutronix.de, festevam@...il.com,
linux-sound@...r.kernel.org, devicetree@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Xiubo.Lee@...il.com, nicoleotsuka@...il.com, perex@...ex.cz, tiwai@...e.com,
linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v3 2/4] ASoC: fsl_asrc_m2m: Add option to start ASRC
before DMA device for M2M
On Thu, Feb 5, 2026 at 12:30 AM Frank Li <Frank.li@....com> wrote:
>
> On Wed, Feb 04, 2026 at 09:41:55AM +0800, Shengjiu Wang wrote:
> > On Wed, Feb 4, 2026 at 1:53 AM Frank Li <Frank.li@....com> wrote:
> > >
> > > On Tue, Feb 03, 2026 at 11:13:43AM +0800, Shengjiu Wang wrote:
> > > > There is a limitation on i.MX952 that dma request is not cleared at the
> > > > end of conversion with dma slave mode. Which causes sample is dropped
> > > > from the input fifo on the second time if dma is triggered before the
> > > > client device and EDMA may copy wrong data from output fifo as the output
> > > > fifo is not ready in the beginning.
> > > >
> > > > The solution is to trigger asrc before dma on i.MX952, and add delay to
> > > > wait output data is generated then start the EDMA for output, otherwise
> > > > the m2m function has noise issues.
> > > >
> > > > So add an option to start ASRC first for M2M before ASRC is enabled on
> > > > i.MX952.
> > > >
> > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@....com>
> > > > ---
> > > > sound/soc/fsl/fsl_asrc.c | 23 +++++++++++++++++++++++
> > > > sound/soc/fsl/fsl_asrc.h | 4 ++++
> > > > sound/soc/fsl/fsl_asrc_common.h | 4 ++++
> > > > sound/soc/fsl/fsl_asrc_m2m.c | 8 +++++++-
> > > > 4 files changed, 38 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> > > > index 92fb16f7be45..b6d4f1e09e2e 100644
> > > > --- a/sound/soc/fsl/fsl_asrc.c
> > > > +++ b/sound/soc/fsl/fsl_asrc.c
> > > > @@ -1078,6 +1078,27 @@ static unsigned int fsl_asrc_get_output_fifo_size(struct fsl_asrc_pair *pair)
> > > > return val >> ASRFSTi_OUTPUT_FIFO_SHIFT;
> > > > }
> > > >
> > > > +static bool fsl_asrc_m2m_output_ready(struct fsl_asrc_pair *pair)
> > > > +{
> > > > + struct fsl_asrc *asrc = pair->asrc;
> > > > + enum asrc_pair_index index = pair->index;
> > > > + int retry = 1000;
> > > > + u32 val;
> > > > + int ret;
> > > > +
> > > > + /* Check output fifo status if it exceeds the watermark. */
> > > > + ret = regmap_read_poll_timeout(asrc->regmap, REG_ASRFST(index), val,
> > > > + (ASRFSTi_OUTPUT_FIFO_FILL(val) >= ASRC_M2M_OUTPUTFIFO_WML) ||
> > > > + (--retry == 0), 0, USEC_PER_SEC);
> > >
> > > are sure need "retry"? there are timeout, which should be equal to 'retry'.
> >
> > Yes, "retry" is what I want, timeout is not what I expected.
>
> Okay, but it will be strange. you polling a register, you suppose it will
> be finished below 1000 IO read, which is certain time in SoC, for example
> 1ms. Compare to timeout us value, 1000 is hard to understand why you choose
> this value.
It can be removed by reducing the timeout time.
will update in the next version.
best regards
Shengjiu Wang
>
> Frank
> >
> > Best regards
> > Shengjiu Wang
> >
> > >
> > > Frank
> > > > +
> > > > + if (ret || !retry) {
> > > > + pair_warn("output is not ready\n");
> > > > + return false;
> > > > + }
> > > > +
> > > > + return true;
> > > > +}
> > > > +
> > > > static int fsl_asrc_m2m_prepare(struct fsl_asrc_pair *pair)
> > > > {
> > > > struct fsl_asrc_pair_priv *pair_priv = pair->private;
> > > > @@ -1275,6 +1296,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
> > > >
> > > > asrc_priv->soc = of_device_get_match_data(&pdev->dev);
> > > > asrc->use_edma = asrc_priv->soc->use_edma;
> > > > + asrc->start_before_dma = asrc_priv->soc->start_before_dma;
> > > > asrc->get_dma_channel = fsl_asrc_get_dma_channel;
> > > > asrc->request_pair = fsl_asrc_request_pair;
> > > > asrc->release_pair = fsl_asrc_release_pair;
> > > > @@ -1289,6 +1311,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
> > > > asrc->m2m_get_maxburst = fsl_asrc_m2m_get_maxburst;
> > > > asrc->m2m_pair_resume = fsl_asrc_m2m_pair_resume;
> > > > asrc->m2m_get_cap = fsl_asrc_m2m_get_cap;
> > > > + asrc->m2m_output_ready = fsl_asrc_m2m_output_ready;
> > > >
> > > > if (of_device_is_compatible(np, "fsl,imx35-asrc")) {
> > > > asrc_priv->clk_map[IN] = input_clk_map_imx35;
> > > > diff --git a/sound/soc/fsl/fsl_asrc.h b/sound/soc/fsl/fsl_asrc.h
> > > > index 1c492eb237f5..60b6865ca952 100644
> > > > --- a/sound/soc/fsl/fsl_asrc.h
> > > > +++ b/sound/soc/fsl/fsl_asrc.h
> > > > @@ -257,6 +257,8 @@
> > > > #define ASRFSTi_OUTPUT_FIFO_WIDTH 7
> > > > #define ASRFSTi_OUTPUT_FIFO_SHIFT 12
> > > > #define ASRFSTi_OUTPUT_FIFO_MASK (((1 << ASRFSTi_OUTPUT_FIFO_WIDTH) - 1) << ASRFSTi_OUTPUT_FIFO_SHIFT)
> > > > +#define ASRFSTi_OUTPUT_FIFO_FILL(v) \
> > > > + (((v) & ASRFSTi_OUTPUT_FIFO_MASK) >> ASRFSTi_OUTPUT_FIFO_SHIFT)
> > > > #define ASRFSTi_IAEi_SHIFT 11
> > > > #define ASRFSTi_IAEi_MASK (1 << ASRFSTi_IAEi_SHIFT)
> > > > #define ASRFSTi_IAEi (1 << ASRFSTi_IAEi_SHIFT)
> > > > @@ -432,10 +434,12 @@ struct dma_block {
> > > > *
> > > > * @use_edma: using edma as dma device or not
> > > > * @channel_bits: width of ASRCNCR register for each pair
> > > > + * @start_before_dma: start asrc before dma
> > > > */
> > > > struct fsl_asrc_soc_data {
> > > > bool use_edma;
> > > > unsigned int channel_bits;
> > > > + bool start_before_dma;
> > > > };
> > > >
> > > > /**
> > > > diff --git a/sound/soc/fsl/fsl_asrc_common.h b/sound/soc/fsl/fsl_asrc_common.h
> > > > index 0cd595b0f629..c8a1a2b5915d 100644
> > > > --- a/sound/soc/fsl/fsl_asrc_common.h
> > > > +++ b/sound/soc/fsl/fsl_asrc_common.h
> > > > @@ -107,6 +107,7 @@ struct fsl_asrc_pair {
> > > > * @asrc_rate: default sample rate for ASoC Back-Ends
> > > > * @asrc_format: default sample format for ASoC Back-Ends
> > > > * @use_edma: edma is used
> > > > + * @start_before_dma: start asrc before dma
> > > > * @get_dma_channel: function pointer
> > > > * @request_pair: function pointer
> > > > * @release_pair: function pointer
> > > > @@ -116,6 +117,7 @@ struct fsl_asrc_pair {
> > > > * @m2m_start: function pointer
> > > > * @m2m_unprepare: function pointer
> > > > * @m2m_stop: function pointer
> > > > + * @m2m_output_ready: function pointer, check output fifo ready or not
> > > > * @m2m_calc_out_len: function pointer
> > > > * @m2m_get_maxburst: function pointer
> > > > * @m2m_pair_suspend: function pointer
> > > > @@ -143,6 +145,7 @@ struct fsl_asrc {
> > > > int asrc_rate;
> > > > snd_pcm_format_t asrc_format;
> > > > bool use_edma;
> > > > + bool start_before_dma;
> > > >
> > > > struct dma_chan *(*get_dma_channel)(struct fsl_asrc_pair *pair, bool dir);
> > > > int (*request_pair)(int channels, struct fsl_asrc_pair *pair);
> > > > @@ -154,6 +157,7 @@ struct fsl_asrc {
> > > > int (*m2m_start)(struct fsl_asrc_pair *pair);
> > > > int (*m2m_unprepare)(struct fsl_asrc_pair *pair);
> > > > int (*m2m_stop)(struct fsl_asrc_pair *pair);
> > > > + bool (*m2m_output_ready)(struct fsl_asrc_pair *pair);
> > > >
> > > > int (*m2m_calc_out_len)(struct fsl_asrc_pair *pair, int input_buffer_length);
> > > > int (*m2m_get_maxburst)(u8 dir, struct fsl_asrc_pair *pair);
> > > > diff --git a/sound/soc/fsl/fsl_asrc_m2m.c b/sound/soc/fsl/fsl_asrc_m2m.c
> > > > index f46881f71e43..77999526dd9e 100644
> > > > --- a/sound/soc/fsl/fsl_asrc_m2m.c
> > > > +++ b/sound/soc/fsl/fsl_asrc_m2m.c
> > > > @@ -253,15 +253,21 @@ static int asrc_m2m_device_run(struct fsl_asrc_pair *pair, struct snd_compr_task
> > > > reinit_completion(&pair->complete[IN]);
> > > > reinit_completion(&pair->complete[OUT]);
> > > >
> > > > + if (asrc->start_before_dma)
> > > > + asrc->m2m_start(pair);
> > > > +
> > > > /* Submit DMA request */
> > > > dmaengine_submit(pair->desc[IN]);
> > > > dma_async_issue_pending(pair->desc[IN]->chan);
> > > > if (out_dma_len > 0) {
> > > > + if (asrc->start_before_dma && asrc->m2m_output_ready)
> > > > + asrc->m2m_output_ready(pair);
> > > > dmaengine_submit(pair->desc[OUT]);
> > > > dma_async_issue_pending(pair->desc[OUT]->chan);
> > > > }
> > > >
> > > > - asrc->m2m_start(pair);
> > > > + if (!asrc->start_before_dma)
> > > > + asrc->m2m_start(pair);
> > > >
> > > > if (!wait_for_completion_interruptible_timeout(&pair->complete[IN], 10 * HZ)) {
> > > > dev_err(dev, "out DMA task timeout\n");
> > > > --
> > > > 2.34.1
> > > >
Powered by blists - more mailing lists