[PATCH 2/2] ASoC: Intel: KMB: Enable DMA transfer mode

[Sit, Michael Wei Hong]

> -----Original Message-----
> From: Lars-Peter Clausen <lars at metafoo.de>
> Sent: Tuesday, 15 December, 2020 5:41 PM
> To: Sit, Michael Wei Hong <michael.wei.hong.sit at intel.com>;
> alsa-devel at alsa-project.org
> Cc: Rojewski, Cezary <cezary.rojewski at intel.com>;
> tiwai at suse.com; Sia, Jee Heng <jee.heng.sia at intel.com>; pierre-
> louis.bossart at linux.intel.com; liam.r.girdwood at linux.intel.com;
> broonie at kernel.org
> 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);
> [...]