> -----Original Message-----
> From: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Sent: Tuesday, 15 December, 2020 5:41 PM
> To: Sit, Michael Wei Hong <michael.wei.hong.sit@xxxxxxxxx>;
> alsa-devel@xxxxxxxxxxxxxxxx
> Cc: Rojewski, Cezary <cezary.rojewski@xxxxxxxxx>;
> tiwai@xxxxxxxx; Sia, Jee Heng <jee.heng.sia@xxxxxxxxx>; pierre-
> louis.bossart@xxxxxxxxxxxxxxx; liam.r.girdwood@xxxxxxxxxxxxxxx;
> broonie@xxxxxxxxxx
> Subject: Re: [PATCH 2/2] ASoC: Intel: KMB: Enable DMA transfer
> mode
>
> On 12/15/20 6:33 AM, Michael Sit Wei Hong wrote:
> > [...]
> > +static inline void kmb_i2s_enable_dma(struct kmb_i2s_info
> *kmb_i2s,
> > +u32 stream) {
> > + u32 dma_reg;
> > +
> > + dma_reg = readl(kmb_i2s->i2s_base + I2S_DMACR);
> > + /* Enable DMA handshake for stream */
> > + if (stream == SNDRV_PCM_STREAM_PLAYBACK)
> > + dma_reg |= I2S_DMAEN_TXBLOCK;
> > + else
> > + dma_reg |= I2S_DMAEN_RXBLOCK;
> > +
> > + writel(dma_reg, kmb_i2s->i2s_base + I2S_DMACR); }
> > +
> > +static inline void kmb_i2s_disable_dma(struct kmb_i2s_info
> *kmb_i2s,
> > +u32 stream) {
> > + u32 dma_reg;
> > +
> > + dma_reg = readl(kmb_i2s->i2s_base + I2S_DMACR);
> > + /* Disable DMA handshake for stream */
> > + if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
> > + dma_reg &= ~I2S_DMAEN_TXBLOCK;
> > + writel(1, kmb_i2s->i2s_base + I2S_RTXDMA);
> > + } else {
> > + dma_reg &= ~I2S_DMAEN_RXBLOCK;
> > + writel(1, kmb_i2s->i2s_base + I2S_RRXDMA);
> > + }
> > + writel(dma_reg, kmb_i2s->i2s_base + I2S_DMACR);
>
> Does this need locking? I believe it is possible for the startup
> callback of the playback and capture stream to be called
> concurrently.
>
Pierre did raised this concern previously and checked that the upper layers has already a locking mechanism in place before this is called.
If we look at the uses of snd_soc_dai_startup() and snd_soc_dai_shutdown(), they are all protected by mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
> > +}
> > +
> > static void kmb_i2s_start(struct kmb_i2s_info *kmb_i2s,
> > struct snd_pcm_substream *substream)
> > {
> > @@ -356,7 +405,11 @@ static void kmb_i2s_start(struct
> kmb_i2s_info *kmb_i2s,
> > else
> > writel(1, kmb_i2s->i2s_base + IRER);
> >
> > - kmb_i2s_irq_trigger(kmb_i2s, substream->stream,
> config->chan_nr, true);
> > + if (kmb_i2s->use_pio)
> > + kmb_i2s_irq_trigger(kmb_i2s, substream-
> >stream,
> > + config->chan_nr, true);
> > + else
> > + kmb_i2s_enable_dma(kmb_i2s, substream-
> >stream);
> >
> > if (kmb_i2s->master)
> > writel(1, kmb_i2s->i2s_base + CER);
> [...]
