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

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?

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.

• 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?

• .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 */

/* 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...

• 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?

• 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;

+}