Messages in this thread | ![/](/images/icornerl.gif) | | Subject | Re: [alsa-devel] [PATCH v3 3/6] ASoC: amd: Enabling I2S instance in DMA and DAI | From | Pierre-Louis Bossart <> | Date | Thu, 7 Nov 2019 09:22:25 -0600 |
| |
> if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { > - val = rv_readl(rtd->acp3x_base + mmACP_BTTDM_ITER); > - val = val | (rtd->xfer_resolution << 3); > - rv_writel(val, rtd->acp3x_base + mmACP_BTTDM_ITER); > + switch (rtd->i2s_instance) { > + case I2S_BT_INSTANCE: > + val = rv_readl(rtd->acp3x_base + mmACP_BTTDM_ITER); > + val = val | (rtd->xfer_resolution << 3); > + rv_writel(val, rtd->acp3x_base + mmACP_BTTDM_ITER); > + break; > + case I2S_SP_INSTANCE: > + default: > + val = rv_readl(rtd->acp3x_base + mmACP_I2STDM_ITER); > + val = val | (rtd->xfer_resolution << 3); > + rv_writel(val, rtd->acp3x_base + mmACP_I2STDM_ITER); > + } > } else { > - val = rv_readl(rtd->acp3x_base + mmACP_BTTDM_IRER); > - val = val | (rtd->xfer_resolution << 3); > - rv_writel(val, rtd->acp3x_base + mmACP_BTTDM_IRER); > + switch (rtd->i2s_instance) { > + case I2S_BT_INSTANCE: > + val = rv_readl(rtd->acp3x_base + mmACP_BTTDM_IRER); > + val = val | (rtd->xfer_resolution << 3); > + rv_writel(val, rtd->acp3x_base + mmACP_BTTDM_IRER); > + break; > + case I2S_SP_INSTANCE: > + default: > + val = rv_readl(rtd->acp3x_base + mmACP_I2STDM_IRER); > + val = val | (rtd->xfer_resolution << 3); > + rv_writel(val, rtd->acp3x_base + mmACP_I2STDM_IRER); > + }
You could reduce the code by setting the address in the switch case, then perform the read/modify/write outisde of the switch.
> @@ -131,33 +168,104 @@ static int acp3x_i2s_trigger(struct snd_pcm_substream *substream, > rtd->bytescount = acp_get_byte_count(rtd, > substream->stream); > if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { > - rv_writel(period_bytes, rtd->acp3x_base + > + switch (rtd->i2s_instance) { > + case I2S_BT_INSTANCE: > + rv_writel(period_bytes, rtd->acp3x_base + > mmACP_BT_TX_INTR_WATERMARK_SIZE); > - val = rv_readl(rtd->acp3x_base + mmACP_BTTDM_ITER); > - val = val | BIT(0); > - rv_writel(val, rtd->acp3x_base + mmACP_BTTDM_ITER); > + val = rv_readl(rtd->acp3x_base + > + mmACP_BTTDM_ITER); > + val = val | BIT(0); > + rv_writel(val, rtd->acp3x_base + > + mmACP_BTTDM_ITER); > + rv_writel(1, rtd->acp3x_base + > + mmACP_BTTDM_IER); > + break; > + case I2S_SP_INSTANCE: > + default: > + rv_writel(period_bytes, rtd->acp3x_base + > + mmACP_I2S_TX_INTR_WATERMARK_SIZE); > + val = rv_readl(rtd->acp3x_base + > + mmACP_I2STDM_ITER); > + val = val | BIT(0); > + rv_writel(val, rtd->acp3x_base + > + mmACP_I2STDM_ITER); > + rv_writel(1, rtd->acp3x_base + > + mmACP_I2STDM_IER); > + } > } else { > - rv_writel(period_bytes, rtd->acp3x_base + > + switch (rtd->i2s_instance) { > + case I2S_BT_INSTANCE: > + rv_writel(period_bytes, rtd->acp3x_base + > mmACP_BT_RX_INTR_WATERMARK_SIZE); > - val = rv_readl(rtd->acp3x_base + mmACP_BTTDM_IRER); > - val = val | BIT(0); > - rv_writel(val, rtd->acp3x_base + mmACP_BTTDM_IRER); > + val = rv_readl(rtd->acp3x_base + > + mmACP_BTTDM_IRER); > + val = val | BIT(0); > + rv_writel(val, rtd->acp3x_base + > + mmACP_BTTDM_IRER); > + rv_writel(1, rtd->acp3x_base + > + mmACP_BTTDM_IER); > + break; > + case I2S_SP_INSTANCE: > + default: > + rv_writel(period_bytes, rtd->acp3x_base + > + mmACP_I2S_RX_INTR_WATERMARK_SIZE); > + val = rv_readl(rtd->acp3x_base + > + mmACP_I2STDM_IRER); > + val = val | BIT(0); > + rv_writel(val, rtd->acp3x_base + > + mmACP_I2STDM_IRER); > + rv_writel(1, rtd->acp3x_base + > + mmACP_I2STDM_IER); > + }
same here, you could set 3 addresses in the switch cases, and perform the operations outside of the switch.
> } > - rv_writel(1, rtd->acp3x_base + mmACP_BTTDM_IER); > break; > case SNDRV_PCM_TRIGGER_STOP: > case SNDRV_PCM_TRIGGER_SUSPEND: > case SNDRV_PCM_TRIGGER_PAUSE_PUSH: > if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { > - val = rv_readl(rtd->acp3x_base + mmACP_BTTDM_ITER); > - val = val & ~BIT(0); > - rv_writel(val, rtd->acp3x_base + mmACP_BTTDM_ITER); > + switch (rtd->i2s_instance) { > + case I2S_BT_INSTANCE: > + val = rv_readl(rtd->acp3x_base + > + mmACP_BTTDM_ITER); > + val = val & ~BIT(0); > + rv_writel(val, rtd->acp3x_base + > + mmACP_BTTDM_ITER); > + rv_writel(0, rtd->acp3x_base + > + mmACP_BTTDM_IER); > + break; > + case I2S_SP_INSTANCE: > + default: > + val = rv_readl(rtd->acp3x_base + > + mmACP_I2STDM_ITER); > + val = val & ~BIT(0); > + rv_writel(val, rtd->acp3x_base + > + mmACP_I2STDM_ITER); > + rv_writel(0, rtd->acp3x_base + > + mmACP_I2STDM_IER); > + } > + > } else { > - val = rv_readl(rtd->acp3x_base + mmACP_BTTDM_IRER); > - val = val & ~BIT(0); > - rv_writel(val, rtd->acp3x_base + mmACP_BTTDM_IRER); > + switch (rtd->i2s_instance) { > + case I2S_BT_INSTANCE: > + val = rv_readl(rtd->acp3x_base + > + mmACP_BTTDM_IRER); > + val = val & ~BIT(0); > + rv_writel(val, rtd->acp3x_base + > + mmACP_BTTDM_IRER); > + rv_writel(0, rtd->acp3x_base + > + mmACP_BTTDM_IER); > + break; > + case I2S_SP_INSTANCE: > + default: > + val = rv_readl(rtd->acp3x_base + > + mmACP_I2STDM_IRER); > + val = val & ~BIT(0); > + rv_writel(val, rtd->acp3x_base + > + mmACP_I2STDM_IRER); > + rv_writel(0, rtd->acp3x_base + > + mmACP_I2STDM_IER); > + }
and here too...
> if (direction == SNDRV_PCM_STREAM_PLAYBACK) { > - /* Config ringbuffer */ > - rv_writel(MEM_WINDOW_START, rtd->acp3x_base + > - mmACP_BT_TX_RINGBUFADDR); > - rv_writel(MAX_BUFFER, rtd->acp3x_base + > - mmACP_BT_TX_RINGBUFSIZE); > - rv_writel(DMA_SIZE, rtd->acp3x_base + mmACP_BT_TX_DMA_SIZE); > - > - /* Config audio fifo */ > - acp_fifo_addr = ACP_SRAM_PTE_OFFSET + (rtd->num_pages * 8) > - + PLAYBACK_FIFO_ADDR_OFFSET; > - rv_writel(acp_fifo_addr, rtd->acp3x_base + > - mmACP_BT_TX_FIFOADDR); > - rv_writel(FIFO_SIZE, rtd->acp3x_base + mmACP_BT_TX_FIFOSIZE); > + switch (rtd->i2s_instance) { > + case I2S_BT_INSTANCE: > + /* Config ringbuffer */ > + rv_writel(I2S_BT_TX_MEM_WINDOW_START, > + rtd->acp3x_base + mmACP_BT_TX_RINGBUFADDR); > + rv_writel(MAX_BUFFER, rtd->acp3x_base + > + mmACP_BT_TX_RINGBUFSIZE); > + rv_writel(DMA_SIZE, > + rtd->acp3x_base + mmACP_BT_TX_DMA_SIZE); > + > + /* Config audio fifo */ > + acp_fifo_addr = ACP_SRAM_PTE_OFFSET + > + BT_PB_FIFO_ADDR_OFFSET; > + rv_writel(acp_fifo_addr, > + rtd->acp3x_base + mmACP_BT_TX_FIFOADDR); > + rv_writel(FIFO_SIZE, > + rtd->acp3x_base + mmACP_BT_TX_FIFOSIZE); > + /* Enable watermark/period interrupt to host */ > + rv_writel(BIT(BT_TX_THRESHOLD), > + rtd->acp3x_base + mmACP_EXTERNAL_INTR_CNTL); > + break; > + > + case I2S_SP_INSTANCE: > + default: > + /* Config ringbuffer */ > + rv_writel(I2S_SP_TX_MEM_WINDOW_START, > + rtd->acp3x_base + mmACP_I2S_TX_RINGBUFADDR); > + rv_writel(MAX_BUFFER, > + rtd->acp3x_base + mmACP_I2S_TX_RINGBUFSIZE); > + rv_writel(DMA_SIZE, > + rtd->acp3x_base + mmACP_I2S_TX_DMA_SIZE); > + > + /* Config audio fifo */ > + acp_fifo_addr = ACP_SRAM_PTE_OFFSET + > + SP_PB_FIFO_ADDR_OFFSET; > + rv_writel(acp_fifo_addr, > + rtd->acp3x_base + mmACP_I2S_TX_FIFOADDR); > + rv_writel(FIFO_SIZE, > + rtd->acp3x_base + mmACP_I2S_TX_FIFOSIZE); > + /* Enable watermark/period interrupt to host */ > + rv_writel(BIT(I2S_TX_THRESHOLD), > + rtd->acp3x_base + mmACP_EXTERNAL_INTR_CNTL); > + } > } else { > - /* Config ringbuffer */ > - rv_writel(MEM_WINDOW_START + MAX_BUFFER, rtd->acp3x_base + > - mmACP_BT_RX_RINGBUFADDR); > - rv_writel(MAX_BUFFER, rtd->acp3x_base + > - mmACP_BT_RX_RINGBUFSIZE); > - rv_writel(DMA_SIZE, rtd->acp3x_base + mmACP_BT_RX_DMA_SIZE); > - > - /* Config audio fifo */ > - acp_fifo_addr = ACP_SRAM_PTE_OFFSET + > - (rtd->num_pages * 8) + CAPTURE_FIFO_ADDR_OFFSET; > - rv_writel(acp_fifo_addr, rtd->acp3x_base + > - mmACP_BT_RX_FIFOADDR); > - rv_writel(FIFO_SIZE, rtd->acp3x_base + mmACP_BT_RX_FIFOSIZE); > + switch (rtd->i2s_instance) { > + case I2S_BT_INSTANCE: > + /* Config ringbuffer */ > + rv_writel(I2S_BT_RX_MEM_WINDOW_START, > + rtd->acp3x_base + mmACP_BT_RX_RINGBUFADDR); > + rv_writel(MAX_BUFFER, > + rtd->acp3x_base + mmACP_BT_RX_RINGBUFSIZE); > + rv_writel(DMA_SIZE, > + rtd->acp3x_base + mmACP_BT_RX_DMA_SIZE); > + > + /* Config audio fifo */ > + acp_fifo_addr = ACP_SRAM_PTE_OFFSET + > + BT_CAPT_FIFO_ADDR_OFFSET; > + rv_writel(acp_fifo_addr, > + rtd->acp3x_base + mmACP_BT_RX_FIFOADDR); > + rv_writel(FIFO_SIZE, > + rtd->acp3x_base + mmACP_BT_RX_FIFOSIZE); > + /* Enable watermark/period interrupt to host */ > + rv_writel(BIT(BT_RX_THRESHOLD), > + rtd->acp3x_base + mmACP_EXTERNAL_INTR_CNTL); > + break; > + > + case I2S_SP_INSTANCE: > + default: > + /* Config ringbuffer */ > + rv_writel(I2S_SP_RX_MEM_WINDOW_START, > + rtd->acp3x_base + mmACP_I2S_RX_RINGBUFADDR); > + rv_writel(MAX_BUFFER, > + rtd->acp3x_base + mmACP_I2S_RX_RINGBUFSIZE); > + rv_writel(DMA_SIZE, > + rtd->acp3x_base + mmACP_I2S_RX_DMA_SIZE); > + > + /* Config audio fifo */ > + acp_fifo_addr = ACP_SRAM_PTE_OFFSET + > + SP_CAPT_FIFO_ADDR_OFFSET; > + rv_writel(acp_fifo_addr, > + rtd->acp3x_base + mmACP_I2S_RX_FIFOADDR); > + rv_writel(FIFO_SIZE, > + rtd->acp3x_base + mmACP_I2S_RX_FIFOSIZE); > + /* Enable watermark/period interrupt to host */ > + rv_writel(BIT(I2S_RX_THRESHOLD), > + rtd->acp3x_base + mmACP_EXTERNAL_INTR_CNTL); > + }
and here too. You are doing the same operations with just different offsets.
| ![\](/images/icornerr.gif) |