diff mbox series

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

Message ID 20230111090222.2016499-9-Vijendar.Mukunda@amd.com (mailing list archive)
State New, archived
Headers show
Series Add soundwire support for Pink Sardine platform | expand

Commit Message

Vijendar Mukunda Jan. 11, 2023, 9:02 a.m. UTC
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(+)

Comments

Mark Brown Jan. 11, 2023, 1:04 p.m. UTC | #1
On Wed, Jan 11, 2023 at 02:32:11PM +0530, Vijendar Mukunda wrote:

> +static int acp63_sdw_dma_start(struct snd_pcm_substream *stream)
> +{
> +	struct sdw_stream_instance *sdw_ins;
> +	struct snd_soc_pcm_runtime *prtd;
> +	u32 stream_id;
> +	u32 sdw_dma_reg;
> +	u32 sdw_dma_en_stat_reg;
> +	u32 sdw_dma_stat;
> +	u32 val;
> +	int timeout = 0;
> +
> +	sdw_ins = stream->runtime->private_data;
> +	prtd = stream->private_data;
> +	stream_id = sdw_ins->stream_id;
> +	switch (stream_id) {
> +	case ACP_SDW_AUDIO_TX:
> +		sdw_dma_reg = ACP_SW_AUDIO_TX_EN;
> +		sdw_dma_en_stat_reg = ACP_SW_AUDIO_TX_EN_STATUS;
> +		break;

Not super urgent but if you're respinning then it looks like the
register selection here is the same in _dma_stop() so they could be
shared.  Indeed it generally feels like it might be nicer to have a
table of structs listing the registers needed per stream so all these
switch statements can be more like

	sdw_dma_reg = stream_registers[sdw_ins->stream_id];

That'd make each function smaller and I'd expect it'd be a bit easier to
add new streams for new hardware that way.
Vijendar Mukunda Jan. 11, 2023, 2:08 p.m. UTC | #2
On 11/01/23 18:34, Mark Brown wrote:
> On Wed, Jan 11, 2023 at 02:32:11PM +0530, Vijendar Mukunda wrote:
>
>> +static int acp63_sdw_dma_start(struct snd_pcm_substream *stream)
>> +{
>> +	struct sdw_stream_instance *sdw_ins;
>> +	struct snd_soc_pcm_runtime *prtd;
>> +	u32 stream_id;
>> +	u32 sdw_dma_reg;
>> +	u32 sdw_dma_en_stat_reg;
>> +	u32 sdw_dma_stat;
>> +	u32 val;
>> +	int timeout = 0;
>> +
>> +	sdw_ins = stream->runtime->private_data;
>> +	prtd = stream->private_data;
>> +	stream_id = sdw_ins->stream_id;
>> +	switch (stream_id) {
>> +	case ACP_SDW_AUDIO_TX:
>> +		sdw_dma_reg = ACP_SW_AUDIO_TX_EN;
>> +		sdw_dma_en_stat_reg = ACP_SW_AUDIO_TX_EN_STATUS;
>> +		break;
> Not super urgent but if you're respinning then it looks like the
> register selection here is the same in _dma_stop() so they could be
> shared.  Indeed it generally feels like it might be nicer to have a
> table of structs listing the registers needed per stream so all these
> switch statements can be more like
>
> 	sdw_dma_reg = stream_registers[sdw_ins->stream_id];
>
> That'd make each function smaller and I'd expect it'd be a bit easier to
> add new streams for new hardware that way.
will fix it in next version.
Pierre-Louis Bossart Jan. 11, 2023, 3:34 p.m. UTC | #3
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;
> +}
Vijendar Mukunda Jan. 13, 2023, 11:16 a.m. UTC | #4
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;
>> +}
Pierre-Louis Bossart Jan. 13, 2023, 5:05 p.m. UTC | #5
>>> +	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.

No, I wasn't asking about playing the same content to different sinks.

I was referring to playing/recording different content to/from different
devices.

Even when interfacing with a single device, there are interesting
topologies in the SDCA spec that would require multiple DMA transfers
conveying unrelated content (or processed content from the same source).
Vijendar Mukunda Jan. 16, 2023, 6:59 a.m. UTC | #6
On 13/01/23 22:35, Pierre-Louis Bossart wrote:
>>>> +	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.
> No, I wasn't asking about playing the same content to different sinks.
>
> I was referring to playing/recording different content to/from different
> devices.
Yes , it's possible. We have tested parallel RX and TX streams.
Please refer patch 2 review comments reply.

For ex, speaker playback we are using Audio_TX Channel whereas for
Headset playback we are using Headset_Tx channel.
>
> Even when interfacing with a single device, there are interesting
> topologies in the SDCA spec that would require multiple DMA transfers
> conveying unrelated content (or processed content from the same source).
>
diff mbox series

Patch

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,
+};
+
 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 {
+	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,
+	.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,
+	.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 */
+	acp63_writel(0x1, sdw_ins->acp_base + ACPAXI2AXI_ATU_CTRL);
+}
+
+static int acp63_configure_sdw_ringbuffer(void __iomem *acp_base, u32 stream_id, u32 size)
+{
+	u32 reg_dma_size;
+	u32 reg_fifo_addr;
+	u32 reg_fifo_size;
+	u32 reg_ring_buf_size;
+	u32 reg_ring_buf_addr;
+	u32 sdw_fifo_addr;
+	u32 sdw_ring_buf_addr;
+	u32 sdw_ring_buf_size;
+
+	switch (stream_id) {
+	case ACP_SDW_AUDIO_TX:
+		reg_dma_size = ACP_AUDIO_TX_DMA_SIZE;
+		reg_fifo_addr =	ACP_AUDIO_TX_FIFOADDR;
+		reg_fifo_size = ACP_AUDIO_TX_FIFOSIZE;
+		reg_ring_buf_size = ACP_AUDIO_TX_RINGBUFSIZE;
+		reg_ring_buf_addr = ACP_AUDIO_TX_RINGBUFADDR;
+		break;
+	case ACP_SDW_BT_TX:
+		reg_dma_size = ACP_BT_TX_DMA_SIZE;
+		reg_fifo_addr = ACP_BT_TX_FIFOADDR;
+		reg_fifo_size = ACP_BT_TX_FIFOSIZE;
+		reg_ring_buf_size = ACP_BT_TX_RINGBUFSIZE;
+		reg_ring_buf_addr = ACP_BT_TX_RINGBUFADDR;
+		break;
+	case ACP_SDW_HS_TX:
+		reg_dma_size = ACP_HS_TX_DMA_SIZE;
+		reg_fifo_addr = ACP_HS_TX_FIFOADDR;
+		reg_fifo_size = ACP_HS_TX_FIFOSIZE;
+		reg_ring_buf_size = ACP_HS_TX_RINGBUFSIZE;
+		reg_ring_buf_addr = ACP_HS_TX_RINGBUFADDR;
+		break;
+	case ACP_SDW1_BT_TX:
+		reg_dma_size = ACP_P1_BT_TX_DMA_SIZE;
+		reg_fifo_addr = ACP_P1_BT_TX_FIFOADDR;
+		reg_fifo_size = ACP_P1_BT_TX_FIFOSIZE;
+		reg_ring_buf_size = ACP_P1_BT_TX_RINGBUFSIZE;
+		reg_ring_buf_addr = ACP_P1_BT_TX_RINGBUFADDR;
+		break;
+	case ACP_SDW_AUDIO_RX:
+		reg_dma_size = ACP_AUDIO_RX_DMA_SIZE;
+		reg_fifo_addr = ACP_AUDIO_RX_FIFOADDR;
+		reg_fifo_size = ACP_AUDIO_RX_FIFOSIZE;
+		reg_ring_buf_size = ACP_AUDIO_RX_RINGBUFSIZE;
+		reg_ring_buf_addr = ACP_AUDIO_RX_RINGBUFADDR;
+		break;
+	case ACP_SDW_BT_RX:
+		reg_dma_size = ACP_BT_RX_DMA_SIZE;
+		reg_fifo_addr = ACP_BT_RX_FIFOADDR;
+		reg_fifo_size = ACP_BT_RX_FIFOSIZE;
+		reg_ring_buf_size = ACP_BT_RX_RINGBUFSIZE;
+		reg_ring_buf_addr = ACP_BT_RX_RINGBUFADDR;
+		break;
+	case ACP_SDW_HS_RX:
+		reg_dma_size = ACP_HS_RX_DMA_SIZE;
+		reg_fifo_addr = ACP_HS_RX_FIFOADDR;
+		reg_fifo_size = ACP_HS_RX_FIFOSIZE;
+		reg_ring_buf_size = ACP_HS_RX_RINGBUFSIZE;
+		reg_ring_buf_addr = ACP_HS_RX_RINGBUFADDR;
+		break;
+	case ACP_SDW1_BT_RX:
+		reg_dma_size = ACP_P1_BT_RX_DMA_SIZE;
+		reg_fifo_addr = ACP_P1_BT_RX_FIFOADDR;
+		reg_fifo_size = ACP_P1_BT_RX_FIFOSIZE;
+		reg_ring_buf_size = ACP_P1_BT_RX_RINGBUFSIZE;
+		reg_ring_buf_addr = ACP_P1_BT_RX_RINGBUFADDR;
+		break;
+	default:
+		return -EINVAL;
+	}
+	sdw_fifo_addr = ACP_SDW_FIFO_OFFSET * stream_id;
+	sdw_ring_buf_addr = SDW_MEM_WINDOW_START + (stream_id * ACP_SDW_RING_BUFF_ADDR_OFFSET);
+	sdw_ring_buf_size = size;
+	acp63_writel(sdw_ring_buf_size, acp_base + reg_ring_buf_size);
+	acp63_writel(sdw_ring_buf_addr, acp_base + reg_ring_buf_addr);
+	acp63_writel(sdw_fifo_addr, acp_base + reg_fifo_addr);
+	acp63_writel(SDW_DMA_SIZE, acp_base + reg_dma_size);
+	acp63_writel(SDW_FIFO_SIZE, acp_base + reg_fifo_size);
+	return 0;
+}
+
+static int acp63_sdw_dma_open(struct snd_soc_component *component,
+			      struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime;
+	struct sdw_dma_dev_data *sdw_dev_data;
+	struct sdw_stream_instance *sdw_stream_data;
+	struct snd_soc_dai *cpu_dai;
+	struct amd_sdwc_ctrl *ctrl;
+	struct snd_soc_pcm_runtime *prtd = substream->private_data;
+	int ret;
+
+	runtime = substream->runtime;
+	sdw_dev_data = dev_get_drvdata(component->dev);
+	cpu_dai = asoc_rtd_to_cpu(prtd, 0);
+	ctrl = snd_soc_dai_get_drvdata(cpu_dai);
+	sdw_stream_data = kzalloc(sizeof(*sdw_stream_data), GFP_KERNEL);
+	if (!sdw_stream_data)
+		return -EINVAL;
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		runtime->hw = acp63_sdw_hardware_playback;
+	else
+		runtime->hw = acp63_sdw_hardware_capture;
+	ret = snd_pcm_hw_constraint_integer(runtime,
+					    SNDRV_PCM_HW_PARAM_PERIODS);
+	if (ret < 0) {
+		dev_err(component->dev, "set integer constraint failed\n");
+		kfree(sdw_stream_data);
+		return ret;
+	}
+
+	if (ctrl->instance == ACP_SDW1)
+		sdw_stream_data->stream_id = cpu_dai->id + ACP_SDW0_MAX_DAI;
+	else
+		sdw_stream_data->stream_id = cpu_dai->id;
+	sdw_stream_data->acp_base = sdw_dev_data->acp_base;
+	runtime->private_data = sdw_stream_data;
+	return ret;
+}
+
+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;
+	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;
+	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;
+	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;
+}
+
+static u64 acp63_sdw_get_byte_count(struct sdw_stream_instance *rtd)
+{
+	union acp_sdw_dma_count byte_count;
+	u32 pos_low_reg, pos_high_reg;
+
+	byte_count.bytescount = 0;
+	switch (rtd->stream_id) {
+	case ACP_SDW_AUDIO_TX:
+		pos_low_reg = ACP_AUDIO_TX_LINEARPOSITIONCNTR_LOW;
+		pos_high_reg = ACP_AUDIO_TX_LINEARPOSITIONCNTR_HIGH;
+		break;
+	case ACP_SDW_BT_TX:
+		pos_low_reg = ACP_BT_TX_LINEARPOSITIONCNTR_LOW;
+		pos_high_reg = ACP_BT_TX_LINEARPOSITIONCNTR_HIGH;
+		break;
+	case ACP_SDW_HS_TX:
+		pos_low_reg = ACP_HS_TX_LINEARPOSITIONCNTR_LOW;
+		pos_high_reg = ACP_HS_TX_LINEARPOSITIONCNTR_HIGH;
+		break;
+	case ACP_SDW1_BT_TX:
+		pos_low_reg = ACP_P1_BT_TX_LINEARPOSITIONCNTR_LOW;
+		pos_high_reg = ACP_P1_BT_TX_LINEARPOSITIONCNTR_HIGH;
+		break;
+	case ACP_SDW_AUDIO_RX:
+		pos_low_reg = ACP_AUDIO_RX_LINEARPOSITIONCNTR_LOW;
+		pos_high_reg = ACP_AUDIO_RX_LINEARPOSITIONCNTR_HIGH;
+		break;
+	case ACP_SDW_BT_RX:
+		pos_low_reg = ACP_BT_RX_LINEARPOSITIONCNTR_LOW;
+		pos_high_reg = ACP_BT_RX_LINEARPOSITIONCNTR_HIGH;
+		break;
+	case ACP_SDW_HS_RX:
+		pos_low_reg = ACP_HS_RX_LINEARPOSITIONCNTR_LOW;
+		pos_high_reg = ACP_HS_RX_LINEARPOSITIONCNTR_HIGH;
+		break;
+	case ACP_SDW1_BT_RX:
+		pos_low_reg = ACP_P1_BT_RX_LINEARPOSITIONCNTR_LOW;
+		pos_high_reg = ACP_P1_BT_RX_LINEARPOSITIONCNTR_HIGH;
+		break;
+	default:
+		pr_err("%s Invalid stream id:%d\n", __func__, rtd->stream_id);
+		return -EINVAL;
+	}
+
+	if (pos_low_reg) {
+		byte_count.bcount.high = acp63_readl(rtd->acp_base + pos_high_reg);
+		byte_count.bcount.low = acp63_readl(rtd->acp_base + pos_low_reg);
+	}
+	return byte_count.bytescount;
+}
+
+static snd_pcm_uframes_t acp63_sdw_dma_pointer(struct snd_soc_component *comp,
+					       struct snd_pcm_substream *stream)
+{
+	struct sdw_stream_instance *sdw_ins;
+	u32 pos, buffersize;
+	u64 bytescount;
+
+	sdw_ins = stream->runtime->private_data;
+	buffersize = frames_to_bytes(stream->runtime,
+				     stream->runtime->buffer_size);
+	bytescount = acp63_sdw_get_byte_count(sdw_ins);
+	if (bytescount > sdw_ins->bytescount)
+		bytescount -= sdw_ins->bytescount;
+	pos = do_div(bytescount, buffersize);
+	return bytes_to_frames(stream->runtime, pos);
+}
+
+static int acp63_sdw_dma_new(struct snd_soc_component *component,
+			     struct snd_soc_pcm_runtime *rtd)
+{
+	struct device *parent = component->dev->parent;
+
+	snd_pcm_set_managed_buffer_all(rtd->pcm, SNDRV_DMA_TYPE_DEV,
+				       parent, SDW_MIN_BUFFER, SDW_MAX_BUFFER);
+	return 0;
+}
+
+static int acp63_sdw_dma_close(struct snd_soc_component *component,
+			       struct snd_pcm_substream *substream)
+{
+	struct sdw_dma_dev_data *sdw_dma_data;
+	struct snd_soc_pcm_runtime *prtd;
+	struct sdw_stream_instance *sdw_ins;
+
+	prtd = asoc_substream_to_rtd(substream);
+	component = snd_soc_rtdcom_lookup(prtd, DRV_NAME);
+	sdw_dma_data = dev_get_drvdata(component->dev);
+	sdw_ins = substream->runtime->private_data;
+	if (!sdw_ins)
+		return -EINVAL;
+	sdw_dma_data->sdw_stream[sdw_ins->stream_id] = NULL;
+	kfree(sdw_ins);
+	return 0;
+}
+
+static int acp63_sdw_dma_start(struct snd_pcm_substream *stream)
+{
+	struct sdw_stream_instance *sdw_ins;
+	struct snd_soc_pcm_runtime *prtd;
+	u32 stream_id;
+	u32 sdw_dma_reg;
+	u32 sdw_dma_en_stat_reg;
+	u32 sdw_dma_stat;
+	u32 val;
+	int timeout = 0;
+
+	sdw_ins = stream->runtime->private_data;
+	prtd = stream->private_data;
+	stream_id = sdw_ins->stream_id;
+	switch (stream_id) {
+	case ACP_SDW_AUDIO_TX:
+		sdw_dma_reg = ACP_SW_AUDIO_TX_EN;
+		sdw_dma_en_stat_reg = ACP_SW_AUDIO_TX_EN_STATUS;
+		break;
+	case ACP_SDW_BT_TX:
+		sdw_dma_reg = ACP_SW_BT_TX_EN;
+		sdw_dma_en_stat_reg = ACP_SW_BT_TX_EN_STATUS;
+		break;
+	case ACP_SDW_HS_TX:
+		sdw_dma_reg = ACP_SW_HEADSET_TX_EN;
+		sdw_dma_en_stat_reg = ACP_SW_HEADSET_TX_EN_STATUS;
+		break;
+	case ACP_SDW1_BT_TX:
+		sdw_dma_reg = ACP_P1_SW_BT_TX_EN;
+		sdw_dma_en_stat_reg = ACP_P1_SW_BT_TX_EN_STATUS;
+		break;
+	case ACP_SDW_AUDIO_RX:
+		sdw_dma_reg = ACP_SW_AUDIO_RX_EN;
+		sdw_dma_en_stat_reg = ACP_SW_AUDIO_RX_EN_STATUS;
+		break;
+	case ACP_SDW_BT_RX:
+		sdw_dma_reg = ACP_SW_BT_RX_EN;
+		sdw_dma_en_stat_reg = ACP_SW_BT_RX_EN_STATUS;
+		break;
+	case ACP_SDW_HS_RX:
+		sdw_dma_reg = ACP_SW_HEADSET_RX_EN;
+		sdw_dma_en_stat_reg = ACP_SW_HEADSET_RX_EN_STATUS;
+		break;
+	case ACP_SDW1_BT_RX:
+		sdw_dma_reg = ACP_P1_SW_BT_RX_EN;
+		sdw_dma_en_stat_reg = ACP_P1_SW_BT_RX_EN_STATUS;
+		break;
+	default:
+		return -EINVAL;
+	}
+	acp63_writel(0x01, sdw_ins->acp_base + sdw_dma_reg);
+	while (++timeout < ACP_COUNTER) {
+		sdw_dma_stat =  acp63_readl(sdw_ins->acp_base + sdw_dma_en_stat_reg);
+		if (sdw_dma_stat & BIT(0)) {
+			val = acp63_readl(sdw_ins->acp_base + sdw_dma_reg);
+			dev_dbg(prtd->dev, "%s stream_id:0x%x dma_reg[0x%x]:0x%x\n",
+				__func__, stream_id, sdw_dma_reg, val);
+			return 0;
+		}
+		udelay(ACP_DELAY_US);
+	}
+	return -ETIMEDOUT;
+}
+
+static int acp63_sdw_dma_stop(struct snd_pcm_substream *stream)
+{
+	struct sdw_stream_instance *sdw_ins;
+	struct snd_soc_pcm_runtime *prtd;
+	u32 stream_id;
+	u32 sdw_dma_reg;
+	u32 sdw_dma_en_stat_reg;
+	u32 sdw_dma_stat;
+	u32 val;
+	int timeout = 0;
+
+	prtd = stream->private_data;
+	sdw_ins = stream->runtime->private_data;
+	stream_id = sdw_ins->stream_id;
+	switch (stream_id) {
+	case ACP_SDW_AUDIO_TX:
+		sdw_dma_reg = ACP_SW_AUDIO_TX_EN;
+		sdw_dma_en_stat_reg = ACP_SW_AUDIO_TX_EN_STATUS;
+		break;
+	case ACP_SDW_BT_TX:
+		sdw_dma_reg = ACP_SW_BT_TX_EN;
+		sdw_dma_en_stat_reg = ACP_SW_BT_TX_EN_STATUS;
+		break;
+	case ACP_SDW_HS_TX:
+		sdw_dma_reg = ACP_SW_HEADSET_TX_EN;
+		sdw_dma_en_stat_reg = ACP_SW_HEADSET_TX_EN_STATUS;
+		break;
+	case ACP_SDW1_BT_TX:
+		sdw_dma_reg = ACP_P1_SW_BT_TX_EN;
+		sdw_dma_en_stat_reg = ACP_P1_SW_BT_TX_EN_STATUS;
+		break;
+	case ACP_SDW_AUDIO_RX:
+		sdw_dma_reg = ACP_SW_AUDIO_RX_EN;
+		sdw_dma_en_stat_reg = ACP_SW_AUDIO_RX_EN_STATUS;
+		break;
+	case ACP_SDW_BT_RX:
+		sdw_dma_reg = ACP_SW_BT_RX_EN;
+		sdw_dma_en_stat_reg = ACP_SW_BT_RX_EN_STATUS;
+		break;
+	case ACP_SDW_HS_RX:
+		sdw_dma_reg = ACP_SW_HEADSET_RX_EN;
+		sdw_dma_en_stat_reg = ACP_SW_HEADSET_RX_EN_STATUS;
+		break;
+	case ACP_SDW1_BT_RX:
+		sdw_dma_reg = ACP_P1_SW_BT_RX_EN;
+		sdw_dma_en_stat_reg = ACP_P1_SW_BT_RX_EN_STATUS;
+		break;
+	default:
+		return -EINVAL;
+	}
+	acp63_writel(0, sdw_ins->acp_base + sdw_dma_reg);
+	while (++timeout < ACP_COUNTER) {
+		sdw_dma_stat =  acp63_readl(sdw_ins->acp_base + sdw_dma_en_stat_reg);
+		if (sdw_dma_stat == 0) {
+			val = acp63_readl(sdw_ins->acp_base + sdw_dma_reg);
+			dev_dbg(prtd->dev, "%s stream_id:0x%x dma_reg[0x%x]:0x%x\n",
+				__func__, stream_id, sdw_dma_reg, val);
+			return 0;
+		}
+		udelay(ACP_DELAY_US);
+	}
+	return -ETIMEDOUT;
+}
+
+static int acp63_sdw_dma_trigger(struct snd_soc_component *comp,
+				 struct snd_pcm_substream *substream,
+				 int cmd)
+{
+	int ret;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+	case SNDRV_PCM_TRIGGER_RESUME:
+		ret = acp63_sdw_dma_start(substream);
+		break;
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_STOP:
+		ret = acp63_sdw_dma_stop(substream);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+	if (ret)
+		dev_err(comp->dev, "trigger %d failed: %d", cmd, ret);
+	return ret;
+}
+
 static const struct snd_soc_component_driver acp63_sdw_component = {
 	.name		= DRV_NAME,
+	.open		= acp63_sdw_dma_open,
+	.close		= acp63_sdw_dma_close,
+	.hw_params	= acp63_sdw_dma_hw_params,
+	.trigger	= acp63_sdw_dma_trigger,
+	.pointer	= acp63_sdw_dma_pointer,
+	.pcm_construct	= acp63_sdw_dma_new,
 };
 
 static int acp63_sdw_platform_probe(struct platform_device *pdev)