Re: [PATCH 08/19] ASoC: amd: ps: add soundwire dma driver dma ops

On 11/01/23 21:04, Pierre-Louis Bossart wrote:

On 1/11/23 03:02, Vijendar Mukunda wrote:

...

Add Soundwire DMA driver dma ops for Pink Sardine platform.

Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com

sound/soc/amd/ps/acp63.h | 61 ++++ sound/soc/amd/ps/ps-sdw-dma.c | 531 ++++++++++++++++++++++++++++++++++ 2 files changed, 592 insertions(+)

diff --git a/sound/soc/amd/ps/acp63.h b/sound/soc/amd/ps/acp63.h index b462320fdf2a..8963cfb6120d 100644 --- a/sound/soc/amd/ps/acp63.h +++ b/sound/soc/amd/ps/acp63.h @@ -67,6 +67,38 @@ #define ACP_SDW0_IRQ_MASK 21 #define ACP_SDW1_IRQ_MASK 2 #define ACP_ERROR_IRQ_MASK 29 +#define ACP_AUDIO_TX_THRESHOLD 28 +#define ACP_BT_TX_THRESHOLD 26 +#define ACP_HS_TX_THRESHOLD 24 +#define ACP_AUDIO_RX_THRESHOLD 27 +#define ACP_BT_RX_THRESHOLD 25 +#define ACP_HS_RX_THRESHOLD 23 +#define ACP_P1_BT_TX_THRESHOLD 6 +#define ACP_P1_BT_RX_THRESHOLD 5 +#define ACP_SDW_DMA_IRQ_MASK 0x1F800000 +#define ACP_P1_SDW_DMA_IRQ_MASK 0x60 +#define ACP63_SDW_MAX_STREAMS 8

+#define ACP_DELAY_US 5 +#define SDW_MEM_WINDOW_START 0x4800000 +#define ACP_SDW_SRAM_PTE_OFFSET 0x03800400 +#define SDW_PTE_OFFSET 0x400 +#define SDW_FIFO_SIZE 0x100 +#define SDW_DMA_SIZE 0x40 +#define ACP_SDW_FIFO_OFFSET 0x100 +#define ACP_SDW_RING_BUFF_ADDR_OFFSET (128 * 1024)

+#define SDW_PLAYBACK_MIN_NUM_PERIODS 2 +#define SDW_PLAYBACK_MAX_NUM_PERIODS 8 +#define SDW_PLAYBACK_MAX_PERIOD_SIZE 8192 +#define SDW_PLAYBACK_MIN_PERIOD_SIZE 1024 +#define SDW_CAPTURE_MIN_NUM_PERIODS 2 +#define SDW_CAPTURE_MAX_NUM_PERIODS 8 +#define SDW_CAPTURE_MAX_PERIOD_SIZE 8192 +#define SDW_CAPTURE_MIN_PERIOD_SIZE 1024

+#define SDW_MAX_BUFFER (SDW_PLAYBACK_MAX_PERIOD_SIZE * SDW_PLAYBACK_MAX_NUM_PERIODS) +#define SDW_MIN_BUFFER SDW_MAX_BUFFER

enum acp_config { ACP_CONFIG_0 = 0, @@ -93,6 +125,17 @@ enum acp_pdev_mask { ACP63_SDW_PDM_DEV_MASK, };

+enum channel_type {

• ACP_SDW_AUDIO_TX = 0,
• ACP_SDW_BT_TX,
• ACP_SDW_HS_TX,
• ACP_SDW_AUDIO_RX,
• ACP_SDW_BT_RX,
• ACP_SDW_HS_RX,
• ACP_SDW1_BT_TX,
• ACP_SDW1_BT_RX,

+};

this was defined in another patch already?

will drop this change.

...

struct pdm_stream_instance { u16 num_pages; u16 channels; @@ -139,4 +182,22 @@ struct acp63_dev_data { struct sdw_dma_dev_data { void __iomem *acp_base; struct mutex *acp_lock;

• struct snd_pcm_substream *sdw_stream[ACP63_SDW_MAX_STREAMS];

+};

+struct sdw_stream_instance {

sdw_stream is already a well-defined concept. Please use sdw_dma_stream or something less confusing naming-wise.

will rename it.

...

• u16 num_pages;
• u16 channels;
• u32 stream_id;
• dma_addr_t dma_addr;
• u64 bytescount;
• void __iomem *acp_base;

+};

+union acp_sdw_dma_count {

• struct {
• u32 low;
• u32 high;
• } bcount;
• u64 bytescount;

}; diff --git a/sound/soc/amd/ps/ps-sdw-dma.c b/sound/soc/amd/ps/ps-sdw-dma.c index 388a4b7df715..e94f76053c66 100644 --- a/sound/soc/amd/ps/ps-sdw-dma.c +++ b/sound/soc/amd/ps/ps-sdw-dma.c @@ -12,12 +12,543 @@ #include <sound/pcm_params.h> #include <sound/soc.h> #include <sound/soc-dai.h> +#include <linux/soundwire/sdw_amd.h> #include "acp63.h"

#define DRV_NAME "amd_ps_sdw_dma"

+static const struct snd_pcm_hardware acp63_sdw_hardware_playback = {

• .info = SNDRV_PCM_INFO_INTERLEAVED |

• SNDRV_PCM_INFO_BLOCK_TRANSFER |
  

• SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
  

• SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME,
  

• .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |

•    SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE,
  

• .channels_min = 2,
• .channels_max = 2,
• .rates = SNDRV_PCM_RATE_48000,
• .rate_min = 48000,
• .rate_max = 48000,

is this really limited to 2ch 48kHz? This doesn't align with the references to Bluetooth above?

As of now we are enabling solution for 48khz, 2Ch, 16bit. We will expand the coverage in the future.

...

• .buffer_bytes_max = SDW_PLAYBACK_MAX_NUM_PERIODS * SDW_PLAYBACK_MAX_PERIOD_SIZE,
• .period_bytes_min = SDW_PLAYBACK_MIN_PERIOD_SIZE,
• .period_bytes_max = SDW_PLAYBACK_MAX_PERIOD_SIZE,
• .periods_min = SDW_PLAYBACK_MIN_NUM_PERIODS,
• .periods_max = SDW_PLAYBACK_MAX_NUM_PERIODS,

+};

+static const struct snd_pcm_hardware acp63_sdw_hardware_capture = {

• .info = SNDRV_PCM_INFO_INTERLEAVED |

• SNDRV_PCM_INFO_BLOCK_TRANSFER |
  

• SNDRV_PCM_INFO_MMAP |
  

• SNDRV_PCM_INFO_MMAP_VALID |
  

• SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME,
  

• .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |

•    SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE,
  

• .channels_min = 2,
• .channels_max = 2,
• .rates = SNDRV_PCM_RATE_48000,
• .rate_min = 48000,
• .rate_max = 48000,

same here?

...

• .buffer_bytes_max = SDW_CAPTURE_MAX_NUM_PERIODS * SDW_CAPTURE_MAX_PERIOD_SIZE,
• .period_bytes_min = SDW_CAPTURE_MIN_PERIOD_SIZE,
• .period_bytes_max = SDW_CAPTURE_MAX_PERIOD_SIZE,
• .periods_min = SDW_CAPTURE_MIN_NUM_PERIODS,
• .periods_max = SDW_CAPTURE_MAX_NUM_PERIODS,

+};

+static void acp63_config_dma(struct sdw_stream_instance *sdw_ins, u32 stream_id) +{

• u16 page_idx;
• u32 low, high, val;
• dma_addr_t addr;
• addr = sdw_ins->dma_addr;
• val = SDW_PTE_OFFSET + (stream_id * 256);
• /* Group Enable */
• acp63_writel(ACP_SDW_SRAM_PTE_OFFSET | BIT(31), sdw_ins->acp_base +

•      ACPAXI2AXI_ATU_BASE_ADDR_GRP_2);
  

• acp63_writel(PAGE_SIZE_4K_ENABLE, sdw_ins->acp_base +

•      ACPAXI2AXI_ATU_PAGE_SIZE_GRP_2);
  

• for (page_idx = 0; page_idx < sdw_ins->num_pages; page_idx++) {

• /* Load the low address of page int ACP SRAM through SRBM */
  

• low = lower_32_bits(addr);
  

• high = upper_32_bits(addr);
  

• acp63_writel(low, sdw_ins->acp_base + ACP_SCRATCH_REG_0 + val);
  

• high |= BIT(31);
  

• acp63_writel(high, sdw_ins->acp_base + ACP_SCRATCH_REG_0 + val + 4);
  

• val += 8;
  

• addr += PAGE_SIZE;
  

• }
• /*cache Invalidation added for Testing */

Below register programming is required. We will remove the comment.

/* cache

...

• acp63_writel(0x1, sdw_ins->acp_base + ACPAXI2AXI_ATU_CTRL);

+} +static int acp63_sdw_dma_hw_params(struct snd_soc_component *component,

• 		   struct snd_pcm_substream *substream,
  

• 		   struct snd_pcm_hw_params *params)
  

+{

• struct sdw_stream_instance *sdw_stream_data;
• struct sdw_dma_dev_data *sdw_data;
• u32 period_bytes;
• u32 water_mark_size_reg;
• u32 irq_mask, ext_intr_ctrl;
• u64 size;
• u32 stream_id;
• u32 acp_ext_intr_cntl_reg;
• int ret;
• stream_id = 0;

useless initialization...

Will remove it.

...

• sdw_data = dev_get_drvdata(component->dev);
• sdw_stream_data = substream->runtime->private_data;
• if (!sdw_stream_data)

• return -EINVAL;
  

• stream_id = sdw_stream_data->stream_id;

... overriden here

...

• sdw_data->sdw_stream[stream_id] = substream;
• size = params_buffer_bytes(params);
• period_bytes = params_period_bytes(params);
• sdw_stream_data->dma_addr = substream->runtime->dma_addr;
• sdw_stream_data->num_pages = (PAGE_ALIGN(size) >> PAGE_SHIFT);
• acp63_config_dma(sdw_stream_data, stream_id);
• ret = acp63_configure_sdw_ringbuffer(sdw_stream_data->acp_base, stream_id, size);
• if (ret) {

• dev_err(component->dev, "Invalid channel type\n");
  

• return -EINVAL;
  

• }
• switch (stream_id) {
• case ACP_SDW_AUDIO_TX:

• water_mark_size_reg = ACP_AUDIO_TX_INTR_WATERMARK_SIZE;
  

• irq_mask = BIT(ACP_AUDIO_TX_THRESHOLD);
  

• acp_ext_intr_cntl_reg = ACP_EXTERNAL_INTR_CNTL;
  

• break;
  

so there's ONE resource to deal with external codecs? How does this work if you have a headset codec and an amplifier?

Are you referring to playing a same stream over headset codec and amplifier? It's all about channel selection from DMA perspective. We have tested speaker aggregation and headset playback use cases.

...

• case ACP_SDW_BT_TX:

• water_mark_size_reg = ACP_BT_TX_INTR_WATERMARK_SIZE;
  

• irq_mask = BIT(ACP_BT_TX_THRESHOLD);
  

• acp_ext_intr_cntl_reg = ACP_EXTERNAL_INTR_CNTL;
  

• break;
  

• case ACP_SDW_HS_TX:

• water_mark_size_reg = ACP_HS_TX_INTR_WATERMARK_SIZE;
  

• irq_mask = BIT(ACP_HS_TX_THRESHOLD);
  

• acp_ext_intr_cntl_reg = ACP_EXTERNAL_INTR_CNTL;
  

• break;
  

• case ACP_SDW1_BT_TX:

• water_mark_size_reg = ACP_P1_BT_TX_INTR_WATERMARK_SIZE;
  

• irq_mask = BIT(ACP_P1_BT_TX_THRESHOLD);
  

• acp_ext_intr_cntl_reg = ACP_EXTERNAL_INTR_CNTL1;
  

• break;
  

• case ACP_SDW_AUDIO_RX:

• water_mark_size_reg = ACP_AUDIO_RX_INTR_WATERMARK_SIZE;
  

• irq_mask = BIT(ACP_AUDIO_RX_THRESHOLD);
  

• acp_ext_intr_cntl_reg = ACP_EXTERNAL_INTR_CNTL;
  

• break;
  

• case ACP_SDW_BT_RX:

• water_mark_size_reg = ACP_BT_RX_INTR_WATERMARK_SIZE;
  

• irq_mask = BIT(ACP_BT_RX_THRESHOLD);
  

• acp_ext_intr_cntl_reg = ACP_EXTERNAL_INTR_CNTL;
  

• break;
  

• case ACP_SDW_HS_RX:

• water_mark_size_reg = ACP_HS_RX_INTR_WATERMARK_SIZE;
  

• irq_mask = BIT(ACP_HS_RX_THRESHOLD);
  

• acp_ext_intr_cntl_reg = ACP_EXTERNAL_INTR_CNTL;
  

• break;
  

• case ACP_SDW1_BT_RX:

• water_mark_size_reg = ACP_P1_BT_RX_INTR_WATERMARK_SIZE;
  

• irq_mask = BIT(ACP_P1_BT_RX_THRESHOLD);
  

• acp_ext_intr_cntl_reg = ACP_EXTERNAL_INTR_CNTL1;
  

• break;
  

• default:

• dev_err(component->dev, "%s: Invalid channel type\n", __func__);
  

• return -EINVAL;
  

• }
• ext_intr_ctrl = acp63_readl(sdw_stream_data->acp_base + acp_ext_intr_cntl_reg);
• ext_intr_ctrl |= irq_mask;
• acp63_writel(ext_intr_ctrl, sdw_stream_data->acp_base + acp_ext_intr_cntl_reg);
• acp63_writel(period_bytes, sdw_stream_data->acp_base + water_mark_size_reg);
• return 0;

+}