Subject: [PATCH 1/3] ASoC: amd: use DMA addr rather than CPU pa for audio buffer

We shouldn't assume CPU physical address we get from page_to_phys()
is same as DMA address we get from dma_alloc_coherent(). On x86_64,
we won't run into any problem with the assumption when dma_ops is
nommu_dma_ops. However, DMA address is IOVA when IOMMU is enabled.
And it's most likely different from CPU physical address when AMD
IOMMU is not in passthrough mode.

Signed-off-by: Vijendar Mukunda <[email protected]>
Tested-by: RAVULAPATI VISHNU VARDHAN RAO <[email protected]>
---
sound/soc/amd/raven/acp3x-pcm-dma.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/sound/soc/amd/raven/acp3x-pcm-dma.c b/sound/soc/amd/raven/acp3x-pcm-dma.c
index a4ade6b..49c4872 100644
--- a/sound/soc/amd/raven/acp3x-pcm-dma.c
+++ b/sound/soc/amd/raven/acp3x-pcm-dma.c
@@ -31,7 +31,7 @@ struct i2s_stream_instance {
u16 num_pages;
u16 channels;
u32 xfer_resolution;
- struct page *pg;
+ dma_addr_t dma_addr;
u64 bytescount;
void __iomem *acp3x_base;
};
@@ -211,11 +211,13 @@ static irqreturn_t i2s_irq_handler(int irq, void *dev_id)
static void config_acp3x_dma(struct i2s_stream_instance *rtd, int direction)
{
u16 page_idx;
- u64 addr;
+ dma_addr_t addr;
u32 low, high, val, acp_fifo_addr;
- struct page *pg = rtd->pg;

- /* 8 scratch registers used to map one 64 bit address */
+ addr = rtd->dma_addr;
+ /* 8 scratch registers used to map one 64 bit address.
+ * For 2 pages (8192 * 2 bytes), it will be 16 registers.
+ */
if (direction == SNDRV_PCM_STREAM_PLAYBACK)
val = 0;
else
@@ -229,7 +231,6 @@ static void config_acp3x_dma(struct i2s_stream_instance *rtd, int direction)

for (page_idx = 0; page_idx < rtd->num_pages; page_idx++) {
/* Load the low address of page int ACP SRAM through SRBM */
- addr = page_to_phys(pg);
low = lower_32_bits(addr);
high = upper_32_bits(addr);

@@ -239,7 +240,7 @@ static void config_acp3x_dma(struct i2s_stream_instance *rtd, int direction)
+ 4);
/* Move to next physically contiguos page */
val += 8;
- pg++;
+ addr += PAGE_SIZE;
}

if (direction == SNDRV_PCM_STREAM_PLAYBACK) {
@@ -340,10 +341,8 @@ static int acp3x_dma_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)
{
int status;
- u64 size;
- struct page *pg;
- struct snd_pcm_runtime *runtime = substream->runtime;
- struct i2s_stream_instance *rtd = runtime->private_data;
+ uint64_t size;
+ struct i2s_stream_instance *rtd = substream->runtime->private_data;

if (!rtd)
return -EINVAL;
@@ -355,8 +354,8 @@ static int acp3x_dma_hw_params(struct snd_pcm_substream *substream,

memset(substream->runtime->dma_area, 0, params_buffer_bytes(params));
pg = virt_to_page(substream->dma_buffer.area);
- if (pg) {
- rtd->pg = pg;
+ if (substream->dma_buffer.area) {
+ rtd->dma_addr = substream->dma_buffer.addr;
rtd->num_pages = (PAGE_ALIGN(size) >> PAGE_SHIFT);
config_acp3x_dma(rtd, substream->stream);
status = 0;
--
2.7.4


Subject: [PATCH 3/3] ASoC : amd: reduced period size

Reduced period size and offsets.

Signed-off-by:Ravulapati, Vishnu vardhan rao <[email protected]>
---
sound/soc/amd/raven/acp3x.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/amd/raven/acp3x.h b/sound/soc/amd/raven/acp3x.h
index 4f2cadd..033e2a9 100644
--- a/sound/soc/amd/raven/acp3x.h
+++ b/sound/soc/amd/raven/acp3x.h
@@ -23,17 +23,17 @@
#define ACP_SRAM_PTE_OFFSET 0x02050000
#define PAGE_SIZE_4K_ENABLE 0x2
#define MEM_WINDOW_START 0x4000000
-#define PLAYBACK_FIFO_ADDR_OFFSET 0x400
-#define CAPTURE_FIFO_ADDR_OFFSET 0x500
+#define PLAYBACK_FIFO_ADDR_OFFSET 0x00
+#define CAPTURE_FIFO_ADDR_OFFSET 0x04

#define PLAYBACK_MIN_NUM_PERIODS 2
#define PLAYBACK_MAX_NUM_PERIODS 8
-#define PLAYBACK_MAX_PERIOD_SIZE 16384
-#define PLAYBACK_MIN_PERIOD_SIZE 4096
+#define PLAYBACK_MAX_PERIOD_SIZE 8192
+#define PLAYBACK_MIN_PERIOD_SIZE 1024
#define CAPTURE_MIN_NUM_PERIODS 2
#define CAPTURE_MAX_NUM_PERIODS 8
-#define CAPTURE_MAX_PERIOD_SIZE 16384
-#define CAPTURE_MIN_PERIOD_SIZE 4096
+#define CAPTURE_MAX_PERIOD_SIZE 8192
+#define CAPTURE_MIN_PERIOD_SIZE 1024

#define MAX_BUFFER (PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS)
#define MIN_BUFFER MAX_BUFFER
--
2.7.4

2019-07-31 20:34:58

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 3/3] ASoC : amd: reduced period size

On Mon, Jul 29, 2019 at 05:38:31PM +0530, Ravulapati Vishnu vardhan rao wrote:
> Reduced period size and offsets.

Why?

> #define PLAYBACK_MAX_NUM_PERIODS 8
> -#define PLAYBACK_MAX_PERIOD_SIZE 16384
> -#define PLAYBACK_MIN_PERIOD_SIZE 4096
> +#define PLAYBACK_MAX_PERIOD_SIZE 8192
> +#define PLAYBACK_MIN_PERIOD_SIZE 1024

Is there a need to also reduce the maximum, the hardware culd support it
before?


Attachments:
(No filename) (434.00 B)
signature.asc (499.00 B)
Download all attachments

2019-08-01 13:18:09

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/3] ASoC: amd: use DMA addr rather than CPU pa for audio buffer

On Mon, Jul 29, 2019 at 05:38:29PM +0530, Ravulapati Vishnu vardhan rao wrote:
> We shouldn't assume CPU physical address we get from page_to_phys()
> is same as DMA address we get from dma_alloc_coherent(). On x86_64,
> we won't run into any problem with the assumption when dma_ops is
> nommu_dma_ops. However, DMA address is IOVA when IOMMU is enabled.
> And it's most likely different from CPU physical address when AMD
> IOMMU is not in passthrough mode.

This doesn't build for me:

sound/soc/amd/raven/acp3x-pcm-dma.c: In function ‘acp3x_dma_hw_params’:
sound/soc/amd/raven/acp3x-pcm-dma.c:356:2: error: ‘pg’ undeclared (first use in this function)
pg = virt_to_page(substream->dma_buffer.area);
^~
sound/soc/amd/raven/acp3x-pcm-dma.c:356:2: note: each undeclared identifier is reported only once for each function it appears in

Please check and resend.


Attachments:
(No filename) (891.00 B)
signature.asc (499.00 B)
Download all attachments