-----Original Message----- From: Alsa-devel alsa-devel-bounces@alsa-project.org On Behalf Of Sit, Michael Wei Hong Sent: Tuesday, 15 December, 2020 6:15 PM To: Lars-Peter Clausen lars@metafoo.de; alsa-devel@alsa- project.org Cc: Rojewski, Cezary cezary.rojewski@intel.com; tiwai@suse.com; Sia, Jee Heng jee.heng.sia@intel.com; pierre- louis.bossart@linux.intel.com; liam.r.girdwood@linux.intel.com; broonie@kernel.org Subject: RE: [PATCH 2/2] ASoC: Intel: KMB: Enable DMA transfer mode
-----Original Message----- From: Lars-Peter Clausen lars@metafoo.de Sent: Tuesday, 15 December, 2020 5:41 PM To: Sit, Michael Wei Hong michael.wei.hong.sit@intel.com; alsa-devel@alsa-project.org Cc: Rojewski, Cezary cezary.rojewski@intel.com;
tiwai@suse.com; Sia,
Jee Heng jee.heng.sia@intel.com; pierre- louis.bossart@linux.intel.com; liam.r.girdwood@linux.intel.com; broonie@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?