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

[Sit, Michael Wei Hong]

> -----Original Message-----
> From: Alsa-devel <alsa-devel-bounces at alsa-project.org> On
> Behalf Of Sit, Michael Wei Hong
> Sent: Tuesday, 15 December, 2020 6:15 PM
> To: Lars-Peter Clausen <lars at metafoo.de>; 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
> 
> 
> 
> > -----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);
> > [...]
Any further comments on this patch set?