All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] ASoC: use DMA addr rather than CPU pa for acp_audio_dma
@ 2018-12-04 22:42 Yu Zhao
  2018-12-04 22:42 ` [PATCH 2/2] ASoC: use dma_ops of parent device " Yu Zhao
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Yu Zhao @ 2018-12-04 22:42 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, aroslav Kysela, Takashi Iwai
  Cc: Daniel Kurtz, Vijendar Mukunda, Akshu Agrawal, Alex Deucher,
	alsa-devel, linux-kernel, Yu Zhao

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: Yu Zhao <yuzhao@google.com>
---
 sound/soc/amd/acp-pcm-dma.c | 15 +++++----------
 sound/soc/amd/acp.h         |  2 +-
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index cdebab2f8ce5..fd3db4c37882 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -303,11 +303,10 @@ static void set_acp_to_i2s_dma_descriptors(void __iomem *acp_mmio, u32 size,
 }
 
 /* Create page table entries in ACP SRAM for the allocated memory */
-static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
+static void acp_pte_config(void __iomem *acp_mmio, dma_addr_t addr,
 			   u16 num_of_pages, u32 pte_offset)
 {
 	u16 page_idx;
-	u64 addr;
 	u32 low;
 	u32 high;
 	u32 offset;
@@ -317,7 +316,6 @@ static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
 		/* Load the low address of page int ACP SRAM through SRBM */
 		acp_reg_write((offset + (page_idx * 8)),
 			      acp_mmio, mmACP_SRBM_Targ_Idx_Addr);
-		addr = page_to_phys(pg);
 
 		low = lower_32_bits(addr);
 		high = upper_32_bits(addr);
@@ -333,7 +331,7 @@ static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
 		acp_reg_write(high, acp_mmio, mmACP_SRBM_Targ_Idx_Data);
 
 		/* Move to next physically contiguos page */
-		pg++;
+		addr += PAGE_SIZE;
 	}
 }
 
@@ -343,7 +341,7 @@ static void config_acp_dma(void __iomem *acp_mmio,
 {
 	u16 ch_acp_sysmem, ch_acp_i2s;
 
-	acp_pte_config(acp_mmio, rtd->pg, rtd->num_of_pages,
+	acp_pte_config(acp_mmio, rtd->dma_addr, rtd->num_of_pages,
 		       rtd->pte_offset);
 
 	if (rtd->direction == SNDRV_PCM_STREAM_PLAYBACK) {
@@ -850,7 +848,6 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 	int status;
 	uint64_t size;
 	u32 val = 0;
-	struct page *pg;
 	struct snd_pcm_runtime *runtime;
 	struct audio_substream_data *rtd;
 	struct snd_soc_pcm_runtime *prtd = substream->private_data;
@@ -986,16 +983,14 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 		return status;
 
 	memset(substream->runtime->dma_area, 0, params_buffer_bytes(params));
-	pg = virt_to_page(substream->dma_buffer.area);
 
-	if (pg) {
+	if (substream->dma_buffer.area) {
 		acp_set_sram_bank_state(rtd->acp_mmio, 0, true);
 		/* Save for runtime private data */
-		rtd->pg = pg;
+		rtd->dma_addr = substream->dma_buffer.addr;
 		rtd->order = get_order(size);
 
 		/* Fill the page table entries in ACP SRAM */
-		rtd->pg = pg;
 		rtd->size = size;
 		rtd->num_of_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
 		rtd->direction = substream->stream;
diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h
index dbbb1a85638d..e5ab6c6040a6 100644
--- a/sound/soc/amd/acp.h
+++ b/sound/soc/amd/acp.h
@@ -123,7 +123,7 @@ enum acp_dma_priority_level {
 };
 
 struct audio_substream_data {
-	struct page *pg;
+	dma_addr_t dma_addr;
 	unsigned int order;
 	u16 num_of_pages;
 	u16 i2s_instance;
-- 
2.20.0.rc1.387.gf8505762e3-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] ASoC: use dma_ops of parent device for acp_audio_dma
  2018-12-04 22:42 [PATCH 1/2] ASoC: use DMA addr rather than CPU pa for acp_audio_dma Yu Zhao
@ 2018-12-04 22:42 ` Yu Zhao
  2018-12-05  6:45     ` Mukunda, Vijendar
  2018-12-06 20:26     ` Mark Brown
  2018-12-05  6:42 ` [PATCH 1/2] ASoC: use DMA addr rather than CPU pa for acp_audio_dma Mukunda, Vijendar
  2018-12-06 20:26   ` Mark Brown
  2 siblings, 2 replies; 9+ messages in thread
From: Yu Zhao @ 2018-12-04 22:42 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, aroslav Kysela, Takashi Iwai
  Cc: Daniel Kurtz, Vijendar Mukunda, Akshu Agrawal, Alex Deucher,
	alsa-devel, linux-kernel, Yu Zhao

AMD platform device acp_audio_dma can only be created by parent PCI
device driver (drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c). Pass struct
device of the parent to snd_pcm_lib_preallocate_pages() so
dma_alloc_coherent() can use correct dma_ops. Otherwise, it will
use default dma_ops which is nommu_dma_ops on x86_64 even when
IOMMU is enabled and set to non passthrough mode.

Though platform device inherits some dma related fields during its
creation in mfd_add_device(), we can't simply pass its struct device
to snd_pcm_lib_preallocate_pages() because dma_ops is not among the
inherited fields. Even it were, drivers/iommu/amd_iommu.c would
ignore it because get_device_id() doesn't handle platform device.

This change shouldn't give us any trouble even struct device of the
parent becomes null or represents some non PCI device in the future,
because get_dma_ops() correctly handles null struct device or uses
the default dma_ops if struct device doesn't have it set.

Signed-off-by: Yu Zhao <yuzhao@google.com>
---
 sound/soc/amd/acp-pcm-dma.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index fd3db4c37882..f4011bebc7ec 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -1146,18 +1146,21 @@ static int acp_dma_new(struct snd_soc_pcm_runtime *rtd)
 	struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd,
 								    DRV_NAME);
 	struct audio_drv_data *adata = dev_get_drvdata(component->dev);
+	struct device *parent = component->dev->parent;
 
 	switch (adata->asic_type) {
 	case CHIP_STONEY:
 		ret = snd_pcm_lib_preallocate_pages_for_all(rtd->pcm,
 							    SNDRV_DMA_TYPE_DEV,
-							    NULL, ST_MIN_BUFFER,
+							    parent,
+							    ST_MIN_BUFFER,
 							    ST_MAX_BUFFER);
 		break;
 	default:
 		ret = snd_pcm_lib_preallocate_pages_for_all(rtd->pcm,
 							    SNDRV_DMA_TYPE_DEV,
-							    NULL, MIN_BUFFER,
+							    parent,
+							    MIN_BUFFER,
 							    MAX_BUFFER);
 		break;
 	}
-- 
2.20.0.rc1.387.gf8505762e3-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] ASoC: use DMA addr rather than CPU pa for acp_audio_dma
  2018-12-04 22:42 [PATCH 1/2] ASoC: use DMA addr rather than CPU pa for acp_audio_dma Yu Zhao
  2018-12-04 22:42 ` [PATCH 2/2] ASoC: use dma_ops of parent device " Yu Zhao
@ 2018-12-05  6:42 ` Mukunda, Vijendar
  2018-12-06 20:26   ` Mark Brown
  2 siblings, 0 replies; 9+ messages in thread
From: Mukunda, Vijendar @ 2018-12-05  6:42 UTC (permalink / raw)
  To: Yu Zhao, Liam Girdwood, Mark Brown, aroslav Kysela, Takashi Iwai
  Cc: Daniel Kurtz, Agrawal, Akshu, Deucher, Alexander, alsa-devel,
	linux-kernel



On 05/12/18 4:12 AM, Yu Zhao 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 is really a good fix. We will apply this patch changes for Raven 
platform also.

Thanks,
Vijendar
> 
> Signed-off-by: Yu Zhao <yuzhao@google.com>
> ---
>   sound/soc/amd/acp-pcm-dma.c | 15 +++++----------
>   sound/soc/amd/acp.h         |  2 +-
>   2 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
> index cdebab2f8ce5..fd3db4c37882 100644
> --- a/sound/soc/amd/acp-pcm-dma.c
> +++ b/sound/soc/amd/acp-pcm-dma.c
> @@ -303,11 +303,10 @@ static void set_acp_to_i2s_dma_descriptors(void __iomem *acp_mmio, u32 size,
>   }
>   
>   /* Create page table entries in ACP SRAM for the allocated memory */
> -static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
> +static void acp_pte_config(void __iomem *acp_mmio, dma_addr_t addr,
>   			   u16 num_of_pages, u32 pte_offset)
>   {
>   	u16 page_idx;
> -	u64 addr;
>   	u32 low;
>   	u32 high;
>   	u32 offset;
> @@ -317,7 +316,6 @@ static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
>   		/* Load the low address of page int ACP SRAM through SRBM */
>   		acp_reg_write((offset + (page_idx * 8)),
>   			      acp_mmio, mmACP_SRBM_Targ_Idx_Addr);
> -		addr = page_to_phys(pg);
>   
>   		low = lower_32_bits(addr);
>   		high = upper_32_bits(addr);
> @@ -333,7 +331,7 @@ static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
>   		acp_reg_write(high, acp_mmio, mmACP_SRBM_Targ_Idx_Data);
>   
>   		/* Move to next physically contiguos page */
> -		pg++;
> +		addr += PAGE_SIZE;
>   	}
>   }
>   
> @@ -343,7 +341,7 @@ static void config_acp_dma(void __iomem *acp_mmio,
>   {
>   	u16 ch_acp_sysmem, ch_acp_i2s;
>   
> -	acp_pte_config(acp_mmio, rtd->pg, rtd->num_of_pages,
> +	acp_pte_config(acp_mmio, rtd->dma_addr, rtd->num_of_pages,
>   		       rtd->pte_offset);
>   
>   	if (rtd->direction == SNDRV_PCM_STREAM_PLAYBACK) {
> @@ -850,7 +848,6 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
>   	int status;
>   	uint64_t size;
>   	u32 val = 0;
> -	struct page *pg;
>   	struct snd_pcm_runtime *runtime;
>   	struct audio_substream_data *rtd;
>   	struct snd_soc_pcm_runtime *prtd = substream->private_data;
> @@ -986,16 +983,14 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
>   		return status;
>   
>   	memset(substream->runtime->dma_area, 0, params_buffer_bytes(params));
> -	pg = virt_to_page(substream->dma_buffer.area);
>   
> -	if (pg) {
> +	if (substream->dma_buffer.area) {
>   		acp_set_sram_bank_state(rtd->acp_mmio, 0, true);
>   		/* Save for runtime private data */
> -		rtd->pg = pg;
> +		rtd->dma_addr = substream->dma_buffer.addr;
>   		rtd->order = get_order(size);
>   
>   		/* Fill the page table entries in ACP SRAM */
> -		rtd->pg = pg;
>   		rtd->size = size;
>   		rtd->num_of_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>   		rtd->direction = substream->stream;
> diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h
> index dbbb1a85638d..e5ab6c6040a6 100644
> --- a/sound/soc/amd/acp.h
> +++ b/sound/soc/amd/acp.h
> @@ -123,7 +123,7 @@ enum acp_dma_priority_level {
>   };
>   
>   struct audio_substream_data {
> -	struct page *pg;
> +	dma_addr_t dma_addr;
>   	unsigned int order;
>   	u16 num_of_pages;
>   	u16 i2s_instance;
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] ASoC: use dma_ops of parent device for acp_audio_dma
  2018-12-04 22:42 ` [PATCH 2/2] ASoC: use dma_ops of parent device " Yu Zhao
@ 2018-12-05  6:45     ` Mukunda, Vijendar
  2018-12-06 20:26     ` Mark Brown
  1 sibling, 0 replies; 9+ messages in thread
From: Mukunda, Vijendar @ 2018-12-05  6:45 UTC (permalink / raw)
  To: Yu Zhao, Liam Girdwood, Mark Brown, aroslav Kysela, Takashi Iwai
  Cc: Daniel Kurtz, Agrawal, Akshu, Deucher, Alexander, alsa-devel,
	linux-kernel



On 05/12/18 4:12 AM, Yu Zhao wrote:
> AMD platform device acp_audio_dma can only be created by parent PCI
> device driver (drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c). Pass struct
> device of the parent to snd_pcm_lib_preallocate_pages() so
> dma_alloc_coherent() can use correct dma_ops. Otherwise, it will
> use default dma_ops which is nommu_dma_ops on x86_64 even when
> IOMMU is enabled and set to non passthrough mode.
> 
> Though platform device inherits some dma related fields during its
> creation in mfd_add_device(), we can't simply pass its struct device
> to snd_pcm_lib_preallocate_pages() because dma_ops is not among the
> inherited fields. Even it were, drivers/iommu/amd_iommu.c would
> ignore it because get_device_id() doesn't handle platform device.
> 
> This change shouldn't give us any trouble even struct device of the
> parent becomes null or represents some non PCI device in the future,
> because get_dma_ops() correctly handles null struct device or uses
> the default dma_ops if struct device doesn't have it set.
> 

This is really a good fix. We will also apply this fix for Raven platform.

Thanks,
Vijendar
> Signed-off-by: Yu Zhao <yuzhao@google.com>
> ---
>   sound/soc/amd/acp-pcm-dma.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
> index fd3db4c37882..f4011bebc7ec 100644
> --- a/sound/soc/amd/acp-pcm-dma.c
> +++ b/sound/soc/amd/acp-pcm-dma.c
> @@ -1146,18 +1146,21 @@ static int acp_dma_new(struct snd_soc_pcm_runtime *rtd)
>   	struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd,
>   								    DRV_NAME);
>   	struct audio_drv_data *adata = dev_get_drvdata(component->dev);
> +	struct device *parent = component->dev->parent;
>   
>   	switch (adata->asic_type) {
>   	case CHIP_STONEY:
>   		ret = snd_pcm_lib_preallocate_pages_for_all(rtd->pcm,
>   							    SNDRV_DMA_TYPE_DEV,
> -							    NULL, ST_MIN_BUFFER,
> +							    parent,
> +							    ST_MIN_BUFFER,
>   							    ST_MAX_BUFFER);
>   		break;
>   	default:
>   		ret = snd_pcm_lib_preallocate_pages_for_all(rtd->pcm,
>   							    SNDRV_DMA_TYPE_DEV,
> -							    NULL, MIN_BUFFER,
> +							    parent,
> +							    MIN_BUFFER,
>   							    MAX_BUFFER);
>   		break;
>   	}
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] ASoC: use dma_ops of parent device for acp_audio_dma
@ 2018-12-05  6:45     ` Mukunda, Vijendar
  0 siblings, 0 replies; 9+ messages in thread
From: Mukunda, Vijendar @ 2018-12-05  6:45 UTC (permalink / raw)
  To: Yu Zhao, Liam Girdwood, Mark Brown, aroslav Kysela, Takashi Iwai
  Cc: Deucher, Alexander, alsa-devel, linux-kernel, Daniel Kurtz,
	Agrawal, Akshu



On 05/12/18 4:12 AM, Yu Zhao wrote:
> AMD platform device acp_audio_dma can only be created by parent PCI
> device driver (drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c). Pass struct
> device of the parent to snd_pcm_lib_preallocate_pages() so
> dma_alloc_coherent() can use correct dma_ops. Otherwise, it will
> use default dma_ops which is nommu_dma_ops on x86_64 even when
> IOMMU is enabled and set to non passthrough mode.
> 
> Though platform device inherits some dma related fields during its
> creation in mfd_add_device(), we can't simply pass its struct device
> to snd_pcm_lib_preallocate_pages() because dma_ops is not among the
> inherited fields. Even it were, drivers/iommu/amd_iommu.c would
> ignore it because get_device_id() doesn't handle platform device.
> 
> This change shouldn't give us any trouble even struct device of the
> parent becomes null or represents some non PCI device in the future,
> because get_dma_ops() correctly handles null struct device or uses
> the default dma_ops if struct device doesn't have it set.
> 

This is really a good fix. We will also apply this fix for Raven platform.

Thanks,
Vijendar
> Signed-off-by: Yu Zhao <yuzhao@google.com>
> ---
>   sound/soc/amd/acp-pcm-dma.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
> index fd3db4c37882..f4011bebc7ec 100644
> --- a/sound/soc/amd/acp-pcm-dma.c
> +++ b/sound/soc/amd/acp-pcm-dma.c
> @@ -1146,18 +1146,21 @@ static int acp_dma_new(struct snd_soc_pcm_runtime *rtd)
>   	struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd,
>   								    DRV_NAME);
>   	struct audio_drv_data *adata = dev_get_drvdata(component->dev);
> +	struct device *parent = component->dev->parent;
>   
>   	switch (adata->asic_type) {
>   	case CHIP_STONEY:
>   		ret = snd_pcm_lib_preallocate_pages_for_all(rtd->pcm,
>   							    SNDRV_DMA_TYPE_DEV,
> -							    NULL, ST_MIN_BUFFER,
> +							    parent,
> +							    ST_MIN_BUFFER,
>   							    ST_MAX_BUFFER);
>   		break;
>   	default:
>   		ret = snd_pcm_lib_preallocate_pages_for_all(rtd->pcm,
>   							    SNDRV_DMA_TYPE_DEV,
> -							    NULL, MIN_BUFFER,
> +							    parent,
> +							    MIN_BUFFER,
>   							    MAX_BUFFER);
>   		break;
>   	}
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Applied "ASoC: use dma_ops of parent device for acp_audio_dma" to the asoc tree
  2018-12-04 22:42 ` [PATCH 2/2] ASoC: use dma_ops of parent device " Yu Zhao
@ 2018-12-06 20:26     ` Mark Brown
  2018-12-06 20:26     ` Mark Brown
  1 sibling, 0 replies; 9+ messages in thread
From: Mark Brown @ 2018-12-06 20:26 UTC (permalink / raw)
  To: Yu Zhao
  Cc: Mark Brown, Liam Girdwood, Mark Brown, aroslav Kysela,
	Takashi Iwai, alsa-devel, linux-kernel, Daniel Kurtz,
	Akshu Agrawal, Vijendar Mukunda, Alex Deucher, alsa-devel

The patch

   ASoC: use dma_ops of parent device for acp_audio_dma

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 23aa128bb28d9da69bb1bdb2b70e50128857884a Mon Sep 17 00:00:00 2001
From: Yu Zhao <yuzhao@google.com>
Date: Tue, 4 Dec 2018 15:42:53 -0700
Subject: [PATCH] ASoC: use dma_ops of parent device for acp_audio_dma

AMD platform device acp_audio_dma can only be created by parent PCI
device driver (drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c). Pass struct
device of the parent to snd_pcm_lib_preallocate_pages() so
dma_alloc_coherent() can use correct dma_ops. Otherwise, it will
use default dma_ops which is nommu_dma_ops on x86_64 even when
IOMMU is enabled and set to non passthrough mode.

Though platform device inherits some dma related fields during its
creation in mfd_add_device(), we can't simply pass its struct device
to snd_pcm_lib_preallocate_pages() because dma_ops is not among the
inherited fields. Even it were, drivers/iommu/amd_iommu.c would
ignore it because get_device_id() doesn't handle platform device.

This change shouldn't give us any trouble even struct device of the
parent becomes null or represents some non PCI device in the future,
because get_dma_ops() correctly handles null struct device or uses
the default dma_ops if struct device doesn't have it set.

Signed-off-by: Yu Zhao <yuzhao@google.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/amd/acp-pcm-dma.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index fd3db4c37882..f4011bebc7ec 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -1146,18 +1146,21 @@ static int acp_dma_new(struct snd_soc_pcm_runtime *rtd)
 	struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd,
 								    DRV_NAME);
 	struct audio_drv_data *adata = dev_get_drvdata(component->dev);
+	struct device *parent = component->dev->parent;
 
 	switch (adata->asic_type) {
 	case CHIP_STONEY:
 		ret = snd_pcm_lib_preallocate_pages_for_all(rtd->pcm,
 							    SNDRV_DMA_TYPE_DEV,
-							    NULL, ST_MIN_BUFFER,
+							    parent,
+							    ST_MIN_BUFFER,
 							    ST_MAX_BUFFER);
 		break;
 	default:
 		ret = snd_pcm_lib_preallocate_pages_for_all(rtd->pcm,
 							    SNDRV_DMA_TYPE_DEV,
-							    NULL, MIN_BUFFER,
+							    parent,
+							    MIN_BUFFER,
 							    MAX_BUFFER);
 		break;
 	}
-- 
2.19.0.rc2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Applied "ASoC: use dma_ops of parent device for acp_audio_dma" to the asoc tree
@ 2018-12-06 20:26     ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2018-12-06 20:26 UTC (permalink / raw)
  To: Yu Zhao; +Cc: Mark Brown, Liam Girdwood

The patch

   ASoC: use dma_ops of parent device for acp_audio_dma

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 23aa128bb28d9da69bb1bdb2b70e50128857884a Mon Sep 17 00:00:00 2001
From: Yu Zhao <yuzhao@google.com>
Date: Tue, 4 Dec 2018 15:42:53 -0700
Subject: [PATCH] ASoC: use dma_ops of parent device for acp_audio_dma

AMD platform device acp_audio_dma can only be created by parent PCI
device driver (drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c). Pass struct
device of the parent to snd_pcm_lib_preallocate_pages() so
dma_alloc_coherent() can use correct dma_ops. Otherwise, it will
use default dma_ops which is nommu_dma_ops on x86_64 even when
IOMMU is enabled and set to non passthrough mode.

Though platform device inherits some dma related fields during its
creation in mfd_add_device(), we can't simply pass its struct device
to snd_pcm_lib_preallocate_pages() because dma_ops is not among the
inherited fields. Even it were, drivers/iommu/amd_iommu.c would
ignore it because get_device_id() doesn't handle platform device.

This change shouldn't give us any trouble even struct device of the
parent becomes null or represents some non PCI device in the future,
because get_dma_ops() correctly handles null struct device or uses
the default dma_ops if struct device doesn't have it set.

Signed-off-by: Yu Zhao <yuzhao@google.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/amd/acp-pcm-dma.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index fd3db4c37882..f4011bebc7ec 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -1146,18 +1146,21 @@ static int acp_dma_new(struct snd_soc_pcm_runtime *rtd)
 	struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd,
 								    DRV_NAME);
 	struct audio_drv_data *adata = dev_get_drvdata(component->dev);
+	struct device *parent = component->dev->parent;
 
 	switch (adata->asic_type) {
 	case CHIP_STONEY:
 		ret = snd_pcm_lib_preallocate_pages_for_all(rtd->pcm,
 							    SNDRV_DMA_TYPE_DEV,
-							    NULL, ST_MIN_BUFFER,
+							    parent,
+							    ST_MIN_BUFFER,
 							    ST_MAX_BUFFER);
 		break;
 	default:
 		ret = snd_pcm_lib_preallocate_pages_for_all(rtd->pcm,
 							    SNDRV_DMA_TYPE_DEV,
-							    NULL, MIN_BUFFER,
+							    parent,
+							    MIN_BUFFER,
 							    MAX_BUFFER);
 		break;
 	}
-- 
2.19.0.rc2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Applied "ASoC: use DMA addr rather than CPU pa for acp_audio_dma" to the asoc tree
  2018-12-04 22:42 [PATCH 1/2] ASoC: use DMA addr rather than CPU pa for acp_audio_dma Yu Zhao
@ 2018-12-06 20:26   ` Mark Brown
  2018-12-05  6:42 ` [PATCH 1/2] ASoC: use DMA addr rather than CPU pa for acp_audio_dma Mukunda, Vijendar
  2018-12-06 20:26   ` Mark Brown
  2 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2018-12-06 20:26 UTC (permalink / raw)
  To: Yu Zhao
  Cc: Mark Brown, Liam Girdwood, Mark Brown, aroslav Kysela,
	Takashi Iwai, alsa-devel, linux-kernel, Daniel Kurtz,
	Akshu Agrawal, Vijendar Mukunda, Alex Deucher, alsa-devel

The patch

   ASoC: use DMA addr rather than CPU pa for acp_audio_dma

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From d6d08273996b3363178b920ccfa74acabaf67963 Mon Sep 17 00:00:00 2001
From: Yu Zhao <yuzhao@google.com>
Date: Tue, 4 Dec 2018 15:42:52 -0700
Subject: [PATCH] ASoC: use DMA addr rather than CPU pa for acp_audio_dma

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: Yu Zhao <yuzhao@google.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/amd/acp-pcm-dma.c | 15 +++++----------
 sound/soc/amd/acp.h         |  2 +-
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index cdebab2f8ce5..fd3db4c37882 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -303,11 +303,10 @@ static void set_acp_to_i2s_dma_descriptors(void __iomem *acp_mmio, u32 size,
 }
 
 /* Create page table entries in ACP SRAM for the allocated memory */
-static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
+static void acp_pte_config(void __iomem *acp_mmio, dma_addr_t addr,
 			   u16 num_of_pages, u32 pte_offset)
 {
 	u16 page_idx;
-	u64 addr;
 	u32 low;
 	u32 high;
 	u32 offset;
@@ -317,7 +316,6 @@ static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
 		/* Load the low address of page int ACP SRAM through SRBM */
 		acp_reg_write((offset + (page_idx * 8)),
 			      acp_mmio, mmACP_SRBM_Targ_Idx_Addr);
-		addr = page_to_phys(pg);
 
 		low = lower_32_bits(addr);
 		high = upper_32_bits(addr);
@@ -333,7 +331,7 @@ static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
 		acp_reg_write(high, acp_mmio, mmACP_SRBM_Targ_Idx_Data);
 
 		/* Move to next physically contiguos page */
-		pg++;
+		addr += PAGE_SIZE;
 	}
 }
 
@@ -343,7 +341,7 @@ static void config_acp_dma(void __iomem *acp_mmio,
 {
 	u16 ch_acp_sysmem, ch_acp_i2s;
 
-	acp_pte_config(acp_mmio, rtd->pg, rtd->num_of_pages,
+	acp_pte_config(acp_mmio, rtd->dma_addr, rtd->num_of_pages,
 		       rtd->pte_offset);
 
 	if (rtd->direction == SNDRV_PCM_STREAM_PLAYBACK) {
@@ -850,7 +848,6 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 	int status;
 	uint64_t size;
 	u32 val = 0;
-	struct page *pg;
 	struct snd_pcm_runtime *runtime;
 	struct audio_substream_data *rtd;
 	struct snd_soc_pcm_runtime *prtd = substream->private_data;
@@ -986,16 +983,14 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 		return status;
 
 	memset(substream->runtime->dma_area, 0, params_buffer_bytes(params));
-	pg = virt_to_page(substream->dma_buffer.area);
 
-	if (pg) {
+	if (substream->dma_buffer.area) {
 		acp_set_sram_bank_state(rtd->acp_mmio, 0, true);
 		/* Save for runtime private data */
-		rtd->pg = pg;
+		rtd->dma_addr = substream->dma_buffer.addr;
 		rtd->order = get_order(size);
 
 		/* Fill the page table entries in ACP SRAM */
-		rtd->pg = pg;
 		rtd->size = size;
 		rtd->num_of_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
 		rtd->direction = substream->stream;
diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h
index dbbb1a85638d..e5ab6c6040a6 100644
--- a/sound/soc/amd/acp.h
+++ b/sound/soc/amd/acp.h
@@ -123,7 +123,7 @@ enum acp_dma_priority_level {
 };
 
 struct audio_substream_data {
-	struct page *pg;
+	dma_addr_t dma_addr;
 	unsigned int order;
 	u16 num_of_pages;
 	u16 i2s_instance;
-- 
2.19.0.rc2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Applied "ASoC: use DMA addr rather than CPU pa for acp_audio_dma" to the asoc tree
@ 2018-12-06 20:26   ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2018-12-06 20:26 UTC (permalink / raw)
  To: Yu Zhao; +Cc: Mark Brown, Liam Girdwood

The patch

   ASoC: use DMA addr rather than CPU pa for acp_audio_dma

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From d6d08273996b3363178b920ccfa74acabaf67963 Mon Sep 17 00:00:00 2001
From: Yu Zhao <yuzhao@google.com>
Date: Tue, 4 Dec 2018 15:42:52 -0700
Subject: [PATCH] ASoC: use DMA addr rather than CPU pa for acp_audio_dma

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: Yu Zhao <yuzhao@google.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/amd/acp-pcm-dma.c | 15 +++++----------
 sound/soc/amd/acp.h         |  2 +-
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index cdebab2f8ce5..fd3db4c37882 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -303,11 +303,10 @@ static void set_acp_to_i2s_dma_descriptors(void __iomem *acp_mmio, u32 size,
 }
 
 /* Create page table entries in ACP SRAM for the allocated memory */
-static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
+static void acp_pte_config(void __iomem *acp_mmio, dma_addr_t addr,
 			   u16 num_of_pages, u32 pte_offset)
 {
 	u16 page_idx;
-	u64 addr;
 	u32 low;
 	u32 high;
 	u32 offset;
@@ -317,7 +316,6 @@ static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
 		/* Load the low address of page int ACP SRAM through SRBM */
 		acp_reg_write((offset + (page_idx * 8)),
 			      acp_mmio, mmACP_SRBM_Targ_Idx_Addr);
-		addr = page_to_phys(pg);
 
 		low = lower_32_bits(addr);
 		high = upper_32_bits(addr);
@@ -333,7 +331,7 @@ static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
 		acp_reg_write(high, acp_mmio, mmACP_SRBM_Targ_Idx_Data);
 
 		/* Move to next physically contiguos page */
-		pg++;
+		addr += PAGE_SIZE;
 	}
 }
 
@@ -343,7 +341,7 @@ static void config_acp_dma(void __iomem *acp_mmio,
 {
 	u16 ch_acp_sysmem, ch_acp_i2s;
 
-	acp_pte_config(acp_mmio, rtd->pg, rtd->num_of_pages,
+	acp_pte_config(acp_mmio, rtd->dma_addr, rtd->num_of_pages,
 		       rtd->pte_offset);
 
 	if (rtd->direction == SNDRV_PCM_STREAM_PLAYBACK) {
@@ -850,7 +848,6 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 	int status;
 	uint64_t size;
 	u32 val = 0;
-	struct page *pg;
 	struct snd_pcm_runtime *runtime;
 	struct audio_substream_data *rtd;
 	struct snd_soc_pcm_runtime *prtd = substream->private_data;
@@ -986,16 +983,14 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 		return status;
 
 	memset(substream->runtime->dma_area, 0, params_buffer_bytes(params));
-	pg = virt_to_page(substream->dma_buffer.area);
 
-	if (pg) {
+	if (substream->dma_buffer.area) {
 		acp_set_sram_bank_state(rtd->acp_mmio, 0, true);
 		/* Save for runtime private data */
-		rtd->pg = pg;
+		rtd->dma_addr = substream->dma_buffer.addr;
 		rtd->order = get_order(size);
 
 		/* Fill the page table entries in ACP SRAM */
-		rtd->pg = pg;
 		rtd->size = size;
 		rtd->num_of_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
 		rtd->direction = substream->stream;
diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h
index dbbb1a85638d..e5ab6c6040a6 100644
--- a/sound/soc/amd/acp.h
+++ b/sound/soc/amd/acp.h
@@ -123,7 +123,7 @@ enum acp_dma_priority_level {
 };
 
 struct audio_substream_data {
-	struct page *pg;
+	dma_addr_t dma_addr;
 	unsigned int order;
 	u16 num_of_pages;
 	u16 i2s_instance;
-- 
2.19.0.rc2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-12-06 20:27 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-04 22:42 [PATCH 1/2] ASoC: use DMA addr rather than CPU pa for acp_audio_dma Yu Zhao
2018-12-04 22:42 ` [PATCH 2/2] ASoC: use dma_ops of parent device " Yu Zhao
2018-12-05  6:45   ` Mukunda, Vijendar
2018-12-05  6:45     ` Mukunda, Vijendar
2018-12-06 20:26   ` Applied "ASoC: use dma_ops of parent device for acp_audio_dma" to the asoc tree Mark Brown
2018-12-06 20:26     ` Mark Brown
2018-12-05  6:42 ` [PATCH 1/2] ASoC: use DMA addr rather than CPU pa for acp_audio_dma Mukunda, Vijendar
2018-12-06 20:26 ` Applied "ASoC: use DMA addr rather than CPU pa for acp_audio_dma" to the asoc tree Mark Brown
2018-12-06 20:26   ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.