This patch set updates the AMD GPU and Audio CoProcessor (ACP) audio drivers and the designware i2s driver for Stoney (ST). ST is an APU similar to Carrizo (CZ) which already has ACP audio support. The i2s controller and ACP audio DMA engine are part of the GPU and both need updating so I would like to upstream the whole patch set via one tree if possible.
The current code is based on drm-next, but I'm happy to rebase on whatever tree this ends up going through if there are any problems applying. The entire patch set can be viewed here: https://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-next-4.13-acp
Thanks!
Alex
Akshu Agrawal (1): ASoC: AMD: Add machine driver for cz rt5650
Vijendar Mukunda (7): drm/amd/amdgpu: Added asic_type as ACP DMA driver platform data ASoC: dwc: Added a quirk DW_I2S_QUIRK_16BIT_IDX_OVERRIDE to dwc driver drm/amd/amdgpu: Added a dwc quirk for Stoney platform ASoC: AMD: added condition checks for CZ specific code ASoC: AMD: DMA driver changes for Stoney Platform ASoC: AMD: Buffer related changes for Stoney drm/amd/amdgpu: Disable ACP Power Gating for Stoney platform
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 54 +++++--- include/sound/designware_i2s.h | 1 + sound/soc/amd/Kconfig | 7 ++ sound/soc/amd/Makefile | 2 + sound/soc/amd/acp-pcm-dma.c | 215 ++++++++++++++++++++++++-------- sound/soc/amd/acp-rt5645.c | 210 +++++++++++++++++++++++++++++++ sound/soc/amd/acp.h | 9 ++ sound/soc/dwc/dwc-i2s.c | 6 + 8 files changed, 432 insertions(+), 72 deletions(-) create mode 100644 sound/soc/amd/acp-rt5645.c
From: Vijendar Mukunda Vijendar.Mukunda@amd.com
asic_type information is passed to ACP DMA Driver as platform data. We need this to determine whether the asic is Carrizo (CZ) or Stoney (ST) in the acp sound driver.
Reviewed-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 4 ++++ sound/soc/amd/acp-pcm-dma.c | 8 ++------ sound/soc/amd/acp.h | 7 +++++++ 3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index 06879d1..7a2a765 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -262,6 +262,7 @@ static int acp_hw_init(void *handle) uint64_t acp_base; struct device *dev; struct i2s_platform_data *i2s_pdata; + u32 asic_type;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -271,6 +272,7 @@ static int acp_hw_init(void *handle) if (!ip_block) return -EINVAL;
+ asic_type = adev->asic_type; r = amd_acp_hw_init(adev->acp.cgs_device, ip_block->version->major, ip_block->version->minor); /* -ENODEV means board uses AZ rather than ACP */ @@ -355,6 +357,8 @@ static int acp_hw_init(void *handle) adev->acp.acp_cell[0].name = "acp_audio_dma"; adev->acp.acp_cell[0].num_resources = 4; adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0]; + adev->acp.acp_cell[0].platform_data = &asic_type; + adev->acp.acp_cell[0].pdata_size = sizeof(asic_type);
adev->acp.acp_cell[1].name = "designware-i2s"; adev->acp.acp_cell[1].num_resources = 1; diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index 08b1399..dcbf997 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -73,12 +73,6 @@ static const struct snd_pcm_hardware acp_pcm_hardware_capture = { .periods_max = CAPTURE_MAX_NUM_PERIODS, };
-struct audio_drv_data { - struct snd_pcm_substream *play_stream; - struct snd_pcm_substream *capture_stream; - void __iomem *acp_mmio; -}; - static u32 acp_reg_read(void __iomem *acp_mmio, u32 reg) { return readl(acp_mmio + (reg * 4)); @@ -916,6 +910,7 @@ static int acp_audio_probe(struct platform_device *pdev) int status; struct audio_drv_data *audio_drv_data; struct resource *res; + const u32 *pdata = pdev->dev.platform_data;
audio_drv_data = devm_kzalloc(&pdev->dev, sizeof(struct audio_drv_data), GFP_KERNEL); @@ -932,6 +927,7 @@ static int acp_audio_probe(struct platform_device *pdev)
audio_drv_data->play_stream = NULL; audio_drv_data->capture_stream = NULL; + audio_drv_data->asic_type = *pdata;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) { diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h index 330832e..28cf914 100644 --- a/sound/soc/amd/acp.h +++ b/sound/soc/amd/acp.h @@ -84,6 +84,13 @@ struct audio_substream_data { void __iomem *acp_mmio; };
+struct audio_drv_data { + struct snd_pcm_substream *play_stream; + struct snd_pcm_substream *capture_stream; + void __iomem *acp_mmio; + u32 asic_type; +}; + enum { ACP_TILE_P1 = 0, ACP_TILE_P2,
Am 23.06.2017 um 18:34 schrieb Alex Deucher:
From: Vijendar Mukunda Vijendar.Mukunda@amd.com
asic_type information is passed to ACP DMA Driver as platform data. We need this to determine whether the asic is Carrizo (CZ) or Stoney (ST) in the acp sound driver.
Reviewed-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 4 ++++ sound/soc/amd/acp-pcm-dma.c | 8 ++------ sound/soc/amd/acp.h | 7 +++++++ 3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index 06879d1..7a2a765 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -262,6 +262,7 @@ static int acp_hw_init(void *handle) uint64_t acp_base; struct device *dev; struct i2s_platform_data *i2s_pdata;
u32 asic_type;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -271,6 +272,7 @@ static int acp_hw_init(void *handle) if (!ip_block) return -EINVAL;
- asic_type = adev->asic_type; r = amd_acp_hw_init(adev->acp.cgs_device, ip_block->version->major, ip_block->version->minor); /* -ENODEV means board uses AZ rather than ACP */
@@ -355,6 +357,8 @@ static int acp_hw_init(void *handle) adev->acp.acp_cell[0].name = "acp_audio_dma"; adev->acp.acp_cell[0].num_resources = 4; adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0];
- adev->acp.acp_cell[0].platform_data = &asic_type;
- adev->acp.acp_cell[0].pdata_size = sizeof(asic_type);
Have the painkillers jellyfied my brain or are you setting a pointer in the amdgpu device structure to some variable on the stack?
If it's just for initialization then that's probably ok, but I would reset the pointer at the end of the function.
Otherwise somebody might have the idea to dereference it later on and that can only lead to trouble.
Christian.
adev->acp.acp_cell[1].name = "designware-i2s"; adev->acp.acp_cell[1].num_resources = 1; diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index 08b1399..dcbf997 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -73,12 +73,6 @@ static const struct snd_pcm_hardware acp_pcm_hardware_capture = { .periods_max = CAPTURE_MAX_NUM_PERIODS, };
-struct audio_drv_data {
- struct snd_pcm_substream *play_stream;
- struct snd_pcm_substream *capture_stream;
- void __iomem *acp_mmio;
-};
- static u32 acp_reg_read(void __iomem *acp_mmio, u32 reg) { return readl(acp_mmio + (reg * 4));
@@ -916,6 +910,7 @@ static int acp_audio_probe(struct platform_device *pdev) int status; struct audio_drv_data *audio_drv_data; struct resource *res;
const u32 *pdata = pdev->dev.platform_data;
audio_drv_data = devm_kzalloc(&pdev->dev, sizeof(struct audio_drv_data), GFP_KERNEL);
@@ -932,6 +927,7 @@ static int acp_audio_probe(struct platform_device *pdev)
audio_drv_data->play_stream = NULL; audio_drv_data->capture_stream = NULL;
audio_drv_data->asic_type = *pdata;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) {
diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h index 330832e..28cf914 100644 --- a/sound/soc/amd/acp.h +++ b/sound/soc/amd/acp.h @@ -84,6 +84,13 @@ struct audio_substream_data { void __iomem *acp_mmio; };
+struct audio_drv_data {
- struct snd_pcm_substream *play_stream;
- struct snd_pcm_substream *capture_stream;
- void __iomem *acp_mmio;
- u32 asic_type;
+};
- enum { ACP_TILE_P1 = 0, ACP_TILE_P2,
On Fri, Jun 23, 2017 at 12:43 PM, Christian König deathsimple@vodafone.de wrote:
Am 23.06.2017 um 18:34 schrieb Alex Deucher:
From: Vijendar Mukunda Vijendar.Mukunda@amd.com
asic_type information is passed to ACP DMA Driver as platform data. We need this to determine whether the asic is Carrizo (CZ) or Stoney (ST) in the acp sound driver.
Reviewed-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 4 ++++ sound/soc/amd/acp-pcm-dma.c | 8 ++------ sound/soc/amd/acp.h | 7 +++++++ 3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index 06879d1..7a2a765 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -262,6 +262,7 @@ static int acp_hw_init(void *handle) uint64_t acp_base; struct device *dev; struct i2s_platform_data *i2s_pdata;
u32 asic_type; struct amdgpu_device *adev = (struct amdgpu_device *)handle;
asic_type = adev->asic_type; r = amd_acp_hw_init(adev->acp.cgs_device, ip_block->version->major,ip_block->version->minor); /* -ENODEV means board uses AZ rather than ACP */ @@ -355,6 +357,8 @@ static int acp_hw_init(void *handle) adev->acp.acp_cell[0].name = "acp_audio_dma"; adev->acp.acp_cell[0].num_resources = 4; adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0];
adev->acp.acp_cell[0].platform_data = &asic_type;adev->acp.acp_cell[0].pdata_size = sizeof(asic_type);Have the painkillers jellyfied my brain or are you setting a pointer in the amdgpu device structure to some variable on the stack?
If it's just for initialization then that's probably ok, but I would reset the pointer at the end of the function.
Otherwise somebody might have the idea to dereference it later on and that can only lead to trouble.
I think it's ok because the hotplug of the audio device is triggered from this function, so the audio device should be probed by the time this variable goes out of scope. That said, it would probably be better to use the asic_type from the GPU driver instance directly rather than a stack variable.
Alex
Christian.
adev->acp.acp_cell[1].name = "designware-i2s"; adev->acp.acp_cell[1].num_resources = 1;diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index 08b1399..dcbf997 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -73,12 +73,6 @@ static const struct snd_pcm_hardware acp_pcm_hardware_capture = { .periods_max = CAPTURE_MAX_NUM_PERIODS, }; -struct audio_drv_data {
struct snd_pcm_substream *play_stream;struct snd_pcm_substream *capture_stream;void __iomem *acp_mmio;-};
- static u32 acp_reg_read(void __iomem *acp_mmio, u32 reg) { return readl(acp_mmio + (reg * 4));
@@ -916,6 +910,7 @@ static int acp_audio_probe(struct platform_device *pdev) int status; struct audio_drv_data *audio_drv_data; struct resource *res;
const u32 *pdata = pdev->dev.platform_data; audio_drv_data = devm_kzalloc(&pdev->dev, sizeof(structaudio_drv_data), GFP_KERNEL); @@ -932,6 +927,7 @@ static int acp_audio_probe(struct platform_device *pdev) audio_drv_data->play_stream = NULL; audio_drv_data->capture_stream = NULL;
audio_drv_data->asic_type = *pdata; res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) {diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h index 330832e..28cf914 100644 --- a/sound/soc/amd/acp.h +++ b/sound/soc/amd/acp.h @@ -84,6 +84,13 @@ struct audio_substream_data { void __iomem *acp_mmio; }; +struct audio_drv_data {
struct snd_pcm_substream *play_stream;struct snd_pcm_substream *capture_stream;void __iomem *acp_mmio;u32 asic_type;+};
- enum { ACP_TILE_P1 = 0, ACP_TILE_P2,
Am 23.06.2017 um 19:05 schrieb Alex Deucher:
On Fri, Jun 23, 2017 at 12:43 PM, Christian König deathsimple@vodafone.de wrote:
Am 23.06.2017 um 18:34 schrieb Alex Deucher:
From: Vijendar Mukunda Vijendar.Mukunda@amd.com
asic_type information is passed to ACP DMA Driver as platform data. We need this to determine whether the asic is Carrizo (CZ) or Stoney (ST) in the acp sound driver.
Reviewed-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 4 ++++ sound/soc/amd/acp-pcm-dma.c | 8 ++------ sound/soc/amd/acp.h | 7 +++++++ 3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index 06879d1..7a2a765 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -262,6 +262,7 @@ static int acp_hw_init(void *handle) uint64_t acp_base; struct device *dev; struct i2s_platform_data *i2s_pdata;
u32 asic_type; struct amdgpu_device *adev = (struct amdgpu_device *)handle;
asic_type = adev->asic_type; r = amd_acp_hw_init(adev->acp.cgs_device, ip_block->version->major,ip_block->version->minor); /* -ENODEV means board uses AZ rather than ACP */ @@ -355,6 +357,8 @@ static int acp_hw_init(void *handle) adev->acp.acp_cell[0].name = "acp_audio_dma"; adev->acp.acp_cell[0].num_resources = 4; adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0];
adev->acp.acp_cell[0].platform_data = &asic_type;adev->acp.acp_cell[0].pdata_size = sizeof(asic_type);Have the painkillers jellyfied my brain or are you setting a pointer in the amdgpu device structure to some variable on the stack?
If it's just for initialization then that's probably ok, but I would reset the pointer at the end of the function.
Otherwise somebody might have the idea to dereference it later on and that can only lead to trouble.
I think it's ok because the hotplug of the audio device is triggered from this function, so the audio device should be probed by the time this variable goes out of scope. That said, it would probably be better to use the asic_type from the GPU driver instance directly rather than a stack variable.
Yeah, agree. But keeping a stale reference in to a stack variable in the driver struct is still ugly like hell.
At bare minimum set this to NULL at the end of the function, or even better use a intptr_t to encode the asic type into the pointer.
Christian.
Alex
Christian.
adev->acp.acp_cell[1].name = "designware-i2s"; adev->acp.acp_cell[1].num_resources = 1;diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index 08b1399..dcbf997 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -73,12 +73,6 @@ static const struct snd_pcm_hardware acp_pcm_hardware_capture = { .periods_max = CAPTURE_MAX_NUM_PERIODS, }; -struct audio_drv_data {
struct snd_pcm_substream *play_stream;struct snd_pcm_substream *capture_stream;void __iomem *acp_mmio;-};
- static u32 acp_reg_read(void __iomem *acp_mmio, u32 reg) { return readl(acp_mmio + (reg * 4));
@@ -916,6 +910,7 @@ static int acp_audio_probe(struct platform_device *pdev) int status; struct audio_drv_data *audio_drv_data; struct resource *res;
const u32 *pdata = pdev->dev.platform_data; audio_drv_data = devm_kzalloc(&pdev->dev, sizeof(structaudio_drv_data), GFP_KERNEL); @@ -932,6 +927,7 @@ static int acp_audio_probe(struct platform_device *pdev) audio_drv_data->play_stream = NULL; audio_drv_data->capture_stream = NULL;
audio_drv_data->asic_type = *pdata; res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) {diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h index 330832e..28cf914 100644 --- a/sound/soc/amd/acp.h +++ b/sound/soc/amd/acp.h @@ -84,6 +84,13 @@ struct audio_substream_data { void __iomem *acp_mmio; }; +struct audio_drv_data {
struct snd_pcm_substream *play_stream;struct snd_pcm_substream *capture_stream;void __iomem *acp_mmio;u32 asic_type;+};
- enum { ACP_TILE_P1 = 0, ACP_TILE_P2,
On Fri, Jun 23, 2017 at 01:05:37PM -0400, Alex Deucher wrote:
On Fri, Jun 23, 2017 at 12:43 PM, Christian König
Have the painkillers jellyfied my brain or are you setting a pointer in the amdgpu device structure to some variable on the stack?
If it's just for initialization then that's probably ok, but I would reset the pointer at the end of the function.
Otherwise somebody might have the idea to dereference it later on and that can only lead to trouble.
I think it's ok because the hotplug of the audio device is triggered from this function, so the audio device should be probed by the time this variable goes out of scope. That said, it would probably be
No, that's in no way safe. There is no guarantee that probe is going to happen on any particular schedule, if the other driver isn't registered yet it'll need to wait for that to happen for example. You will often get away with it but that's at best going to be vulnerable to framework changes (what if someone decides to kick off the probe on a separate CPU in an effort to reduce boot time?).
better to use the asic_type from the GPU driver instance directly rather than a stack variable.
Yes.
From: Vijendar Mukunda Vijendar.Mukunda@amd.com
Added quirk DW_I2S_QUIRK_16BIT_IDX_OVERRIDE to Designware driver. This quirk will set idx value to 1.
By setting this quirk, it will override supported format as 16 bit resolution and bus width as 2 Bytes.
Reviewed-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com --- include/sound/designware_i2s.h | 1 + sound/soc/dwc/dwc-i2s.c | 6 ++++++ 2 files changed, 7 insertions(+)
diff --git a/include/sound/designware_i2s.h b/include/sound/designware_i2s.h index 5681855..830f5ca 100644 --- a/include/sound/designware_i2s.h +++ b/include/sound/designware_i2s.h @@ -47,6 +47,7 @@ struct i2s_platform_data {
#define DW_I2S_QUIRK_COMP_REG_OFFSET (1 << 0) #define DW_I2S_QUIRK_COMP_PARAM1 (1 << 1) + #define DW_I2S_QUIRK_16BIT_IDX_OVERRIDE (1 << 2) unsigned int quirks; unsigned int i2s_reg_comp1; unsigned int i2s_reg_comp2; diff --git a/sound/soc/dwc/dwc-i2s.c b/sound/soc/dwc/dwc-i2s.c index 9c46e41..9160676 100644 --- a/sound/soc/dwc/dwc-i2s.c +++ b/sound/soc/dwc/dwc-i2s.c @@ -496,6 +496,8 @@ static int dw_configure_dai(struct dw_i2s_dev *dev, idx = COMP1_TX_WORDSIZE_0(comp1); if (WARN_ON(idx >= ARRAY_SIZE(formats))) return -EINVAL; + if (dev->quirks & DW_I2S_QUIRK_16BIT_IDX_OVERRIDE) + idx = 1; dw_i2s_dai->playback.channels_min = MIN_CHANNEL_NUM; dw_i2s_dai->playback.channels_max = 1 << (COMP1_TX_CHANNELS(comp1) + 1); @@ -508,6 +510,8 @@ static int dw_configure_dai(struct dw_i2s_dev *dev, idx = COMP2_RX_WORDSIZE_0(comp2); if (WARN_ON(idx >= ARRAY_SIZE(formats))) return -EINVAL; + if (dev->quirks & DW_I2S_QUIRK_16BIT_IDX_OVERRIDE) + idx = 1; dw_i2s_dai->capture.channels_min = MIN_CHANNEL_NUM; dw_i2s_dai->capture.channels_max = 1 << (COMP1_RX_CHANNELS(comp1) + 1); @@ -543,6 +547,8 @@ static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev, if (ret < 0) return ret;
+ if (dev->quirks & DW_I2S_QUIRK_16BIT_IDX_OVERRIDE) + idx = 1; /* Set DMA slaves info */ dev->play_dma_data.pd.data = pdata->play_dma_data; dev->capture_dma_data.pd.data = pdata->capture_dma_data;
The patch
ASoC: dwc: Added a quirk DW_I2S_QUIRK_16BIT_IDX_OVERRIDE to dwc driver
has been applied to the asoc tree at
git://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 286345eef97ea8f4ea223410f025ed35f265e506 Mon Sep 17 00:00:00 2001
From: Vijendar Mukunda Vijendar.Mukunda@amd.com Date: Fri, 23 Jun 2017 12:35:00 -0400 Subject: [PATCH] ASoC: dwc: Added a quirk DW_I2S_QUIRK_16BIT_IDX_OVERRIDE to dwc driver
Added quirk DW_I2S_QUIRK_16BIT_IDX_OVERRIDE to Designware driver. This quirk will set idx value to 1.
By setting this quirk, it will override supported format as 16 bit resolution and bus width as 2 Bytes.
Reviewed-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Mark Brown broonie@kernel.org --- include/sound/designware_i2s.h | 1 + sound/soc/dwc/dwc-i2s.c | 6 ++++++ 2 files changed, 7 insertions(+)
diff --git a/include/sound/designware_i2s.h b/include/sound/designware_i2s.h index 5681855396c4..830f5caa915c 100644 --- a/include/sound/designware_i2s.h +++ b/include/sound/designware_i2s.h @@ -47,6 +47,7 @@ struct i2s_platform_data {
#define DW_I2S_QUIRK_COMP_REG_OFFSET (1 << 0) #define DW_I2S_QUIRK_COMP_PARAM1 (1 << 1) + #define DW_I2S_QUIRK_16BIT_IDX_OVERRIDE (1 << 2) unsigned int quirks; unsigned int i2s_reg_comp1; unsigned int i2s_reg_comp2; diff --git a/sound/soc/dwc/dwc-i2s.c b/sound/soc/dwc/dwc-i2s.c index 9c46e4112026..916067638180 100644 --- a/sound/soc/dwc/dwc-i2s.c +++ b/sound/soc/dwc/dwc-i2s.c @@ -496,6 +496,8 @@ static int dw_configure_dai(struct dw_i2s_dev *dev, idx = COMP1_TX_WORDSIZE_0(comp1); if (WARN_ON(idx >= ARRAY_SIZE(formats))) return -EINVAL; + if (dev->quirks & DW_I2S_QUIRK_16BIT_IDX_OVERRIDE) + idx = 1; dw_i2s_dai->playback.channels_min = MIN_CHANNEL_NUM; dw_i2s_dai->playback.channels_max = 1 << (COMP1_TX_CHANNELS(comp1) + 1); @@ -508,6 +510,8 @@ static int dw_configure_dai(struct dw_i2s_dev *dev, idx = COMP2_RX_WORDSIZE_0(comp2); if (WARN_ON(idx >= ARRAY_SIZE(formats))) return -EINVAL; + if (dev->quirks & DW_I2S_QUIRK_16BIT_IDX_OVERRIDE) + idx = 1; dw_i2s_dai->capture.channels_min = MIN_CHANNEL_NUM; dw_i2s_dai->capture.channels_max = 1 << (COMP1_RX_CHANNELS(comp1) + 1); @@ -543,6 +547,8 @@ static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev, if (ret < 0) return ret;
+ if (dev->quirks & DW_I2S_QUIRK_16BIT_IDX_OVERRIDE) + idx = 1; /* Set DMA slaves info */ dev->play_dma_data.pd.data = pdata->play_dma_data; dev->capture_dma_data.pd.data = pdata->capture_dma_data;
From: Vijendar Mukunda Vijendar.Mukunda@amd.com
Added DW_I2S_QUIRK_16BIT_IDX_OVERRIDE quirk for Stoney.
Supported format and bus width for I2S controller read from I2S Component Parameter registers. These are ready only registers.
For Stoney, I2S Component Parameter registers are programmed to support 32 bit format and 4 bytes bus width only.
By setting this quirk, it will override 32 bit format with 16 bit format and 2 bytes as bus width for Stoney.
Reviewed-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index 7a2a765..86b68f7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -321,14 +321,25 @@ static int acp_hw_init(void *handle) return -ENOMEM; }
- i2s_pdata[0].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET; + if (adev->asic_type == CHIP_STONEY) { + i2s_pdata[0].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET | + DW_I2S_QUIRK_16BIT_IDX_OVERRIDE; + } else { + i2s_pdata[0].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET; + } i2s_pdata[0].cap = DWC_I2S_PLAY; i2s_pdata[0].snd_rates = SNDRV_PCM_RATE_8000_96000; i2s_pdata[0].i2s_reg_comp1 = ACP_I2S_COMP1_PLAY_REG_OFFSET; i2s_pdata[0].i2s_reg_comp2 = ACP_I2S_COMP2_PLAY_REG_OFFSET; + if (adev->asic_type == CHIP_STONEY) { + i2s_pdata[1].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET | + DW_I2S_QUIRK_COMP_PARAM1 | + DW_I2S_QUIRK_16BIT_IDX_OVERRIDE; + } else { + i2s_pdata[1].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET | + DW_I2S_QUIRK_COMP_PARAM1; + }
- i2s_pdata[1].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET | - DW_I2S_QUIRK_COMP_PARAM1; i2s_pdata[1].cap = DWC_I2S_RECORD; i2s_pdata[1].snd_rates = SNDRV_PCM_RATE_8000_96000; i2s_pdata[1].i2s_reg_comp1 = ACP_I2S_COMP1_CAP_REG_OFFSET;
On Fri, Jun 23, 2017 at 12:35:01PM -0400, Alex Deucher wrote:
- if (adev->asic_type == CHIP_STONEY) {
i2s_pdata[1].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET |DW_I2S_QUIRK_COMP_PARAM1 |DW_I2S_QUIRK_16BIT_IDX_OVERRIDE;- } else {
i2s_pdata[1].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET |DW_I2S_QUIRK_COMP_PARAM1;- }
Quirks like this are better written as switch statements since that makes it easier to handle further variants in future.
From: Vijendar Mukunda Vijendar.Mukunda@amd.com
Added condition checks for CZ specific code based on asic_type. Stoney specific code will be added in a future commit.
Reviewed-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com --- sound/soc/amd/acp-pcm-dma.c | 62 ++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 23 deletions(-)
diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index dcbf997..e48ae5d 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -34,6 +34,8 @@
#define MAX_BUFFER (PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS) #define MIN_BUFFER MAX_BUFFER +#define CHIP_STONEY 14 +#define CHIP_CARRIZO 13
static const struct snd_pcm_hardware acp_pcm_hardware_playback = { .info = SNDRV_PCM_INFO_INTERLEAVED | @@ -419,7 +421,7 @@ static void acp_set_sram_bank_state(void __iomem *acp_mmio, u16 bank, }
/* Initialize and bring ACP hardware to default state. */ -static int acp_init(void __iomem *acp_mmio) +static int acp_init(void __iomem *acp_mmio, u32 asic_type) { u16 bank; u32 val, count, sram_pte_offset; @@ -494,8 +496,10 @@ static int acp_init(void __iomem *acp_mmio) * Now, turn off all of them. This can't be done in 'poweron' of * ACP pm domain, as this requires ACP to be initialized. */ - for (bank = 1; bank < 48; bank++) - acp_set_sram_bank_state(acp_mmio, bank, false); + if (asic_type == CHIP_CARRIZO) { + for (bank = 1; bank < 48; bank++) + acp_set_sram_bank_state(acp_mmio, bank, false); + }
return 0; } @@ -646,14 +650,18 @@ static int acp_dma_open(struct snd_pcm_substream *substream)
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { intr_data->play_stream = substream; - for (bank = 1; bank <= 4; bank++) - acp_set_sram_bank_state(intr_data->acp_mmio, bank, - true); + if (intr_data->asic_type == CHIP_CARRIZO) { + for (bank = 1; bank <= 4; bank++) + acp_set_sram_bank_state(intr_data->acp_mmio, + bank, true); + } } else { intr_data->capture_stream = substream; - for (bank = 5; bank <= 8; bank++) - acp_set_sram_bank_state(intr_data->acp_mmio, bank, - true); + if (intr_data->asic_type == CHIP_CARRIZO) { + for (bank = 5; bank <= 8; bank++) + acp_set_sram_bank_state(intr_data->acp_mmio, + bank, true); + } }
return 0; @@ -869,14 +877,18 @@ static int acp_dma_close(struct snd_pcm_substream *substream)
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { adata->play_stream = NULL; - for (bank = 1; bank <= 4; bank++) - acp_set_sram_bank_state(adata->acp_mmio, bank, - false); - } else { + if (adata->asic_type == CHIP_CARRIZO) { + for (bank = 1; bank <= 4; bank++) + acp_set_sram_bank_state(adata->acp_mmio, bank, + false); + } + } else { adata->capture_stream = NULL; - for (bank = 5; bank <= 8; bank++) - acp_set_sram_bank_state(adata->acp_mmio, bank, - false); + if (adata->asic_type == CHIP_CARRIZO) { + for (bank = 5; bank <= 8; bank++) + acp_set_sram_bank_state(adata->acp_mmio, bank, + false); + } }
/* Disable ACP irq, when the current stream is being closed and @@ -945,7 +957,7 @@ static int acp_audio_probe(struct platform_device *pdev) dev_set_drvdata(&pdev->dev, audio_drv_data);
/* Initialize the ACP */ - acp_init(audio_drv_data->acp_mmio); + acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
status = snd_soc_register_platform(&pdev->dev, &acp_asoc_platform); if (status != 0) { @@ -976,19 +988,23 @@ static int acp_pcm_resume(struct device *dev) u16 bank; struct audio_drv_data *adata = dev_get_drvdata(dev);
- acp_init(adata->acp_mmio); + acp_init(adata->acp_mmio, adata->asic_type);
if (adata->play_stream && adata->play_stream->runtime) { - for (bank = 1; bank <= 4; bank++) - acp_set_sram_bank_state(adata->acp_mmio, bank, + if (adata->asic_type == CHIP_CARRIZO) { + for (bank = 1; bank <= 4; bank++) + acp_set_sram_bank_state(adata->acp_mmio, bank, true); + } config_acp_dma(adata->acp_mmio, adata->play_stream->runtime->private_data); } if (adata->capture_stream && adata->capture_stream->runtime) { - for (bank = 5; bank <= 8; bank++) - acp_set_sram_bank_state(adata->acp_mmio, bank, + if (adata->asic_type == CHIP_CARRIZO) { + for (bank = 5; bank <= 8; bank++) + acp_set_sram_bank_state(adata->acp_mmio, bank, true); + } config_acp_dma(adata->acp_mmio, adata->capture_stream->runtime->private_data); } @@ -1009,7 +1025,7 @@ static int acp_pcm_runtime_resume(struct device *dev) { struct audio_drv_data *adata = dev_get_drvdata(dev);
- acp_init(adata->acp_mmio); + acp_init(adata->acp_mmio, adata->asic_type); acp_reg_write(1, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB); return 0; }
On Fri, Jun 23, 2017 at 12:35:02PM -0400, Alex Deucher wrote:
Reviewed-by: Alex Deucher alexander.deucher@amd.com
index dcbf997..e48ae5d 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -34,6 +34,8 @@
#define MAX_BUFFER (PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS) #define MIN_BUFFER MAX_BUFFER +#define CHIP_STONEY 14 +#define CHIP_CARRIZO 13
These defines are being added in the middle of a file but CHIP_STONEY is also used in another file in the previous patch (and apparently extensively throughout the DRM driver already). This is obviously not good, we shouldn't have multiple copies of the definition.
- } else {
if (adata->asic_type == CHIP_CARRIZO) {for (bank = 1; bank <= 4; bank++)acp_set_sram_bank_state(adata->acp_mmio, bank,false);
I'm not seeing any poweroff cases for other chips being added, and again switch statements please.
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Wednesday, June 28, 2017 11:36 PM To: Alex Deucher Cc: dri-devel@lists.freedesktop.org; amd-gfx@lists.freedesktop.org; alsa-devel@alsa-project.org; airlied@gmail.com; Mukunda, Vijendar; rajeevkumar.linux@gmail.com; lgirdwood@gmail.com; tiwai@suse.de; perex@perex.cz; Deucher, Alexander Subject: Re: [PATCH 4/8] ASoC: AMD: added condition checks for CZ specific code
On Fri, Jun 23, 2017 at 12:35:02PM -0400, Alex Deucher wrote:
Reviewed-by: Alex Deucher alexander.deucher@amd.com
index dcbf997..e48ae5d 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -34,6 +34,8 @@
#define MAX_BUFFER (PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS) #define MIN_BUFFER MAX_BUFFER +#define CHIP_STONEY 14 +#define CHIP_CARRIZO 13
These defines are being added in the middle of a file but CHIP_STONEY is also used in another file in the previous patch (and apparently extensively throughout the DRM driver already). This is obviously not good, >we shouldn't have multiple copies of the definition.
- } else {
if (adata->asic_type == CHIP_CARRIZO) {for (bank = 1; bank <= 4; bank++)acp_set_sram_bank_state(adata->acp_mmio, bank,false);
I'm not seeing any poweroff cases for other chips being added, and again switch statements please.
We will modify code to use single definition for CHIP_STONEY and CHIP_CARRIZO. There are only two chip sets based on ACP 2.x design(Carrizo and Stoney). Our future Chip sets going to use different design based on next ACP IP version.
In the current patch, Condition checks added for Carrizo for setting SRAM BANK state. Memory Gating is disabled in Stoney,i.e SRAM Bank's won't be turned off. The default state for SRAM banks is ON. As Memory Gating is disabled, there is no need to add condition checks for Stoney to set SRAM Bank state.
Apart from Memory Gating, there are few more differences between Stoney and Carrizo chip sets. Stoney specific DMA driver changes submitted in a separate patch.
Vijendar
On Thu, Jun 29, 2017 at 12:58:02PM +0000, Mukunda, Vijendar wrote:
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Wednesday, June 28, 2017 11:36 PM To: Alex Deucher
Please fix your mail client to quote mails in a more normal fashion, this looks pretty broken...
These defines are being added in the middle of a file but CHIP_STONEY is also used in another file in the previous patch (and apparently extensively throughout the DRM driver already). This is obviously not good, >we shouldn't have multiple copies of the definition.
...especially in that it's reflowing the message it's replying to to cause 80 column problems and has serious problems in that regard itself.
We will modify code to use single definition for CHIP_STONEY and CHIP_CARRIZO. There are only two chip sets based on ACP 2.x design(Carrizo and Stoney). Our future Chip sets going to use different design based on next ACP IP version.
Write the code well, that way we don't have bad patterns in the codebase and if plans change with regard to new variants you're covered.
In the current patch, Condition checks added for Carrizo for setting SRAM BANK state. Memory Gating is disabled in Stoney,i.e SRAM Bank's won't be turned off. The default state for SRAM banks is ON. As Memory Gating is disabled, there is no need to add condition checks for Stoney to set SRAM Bank state.
Some documentation of this in the code would be good.
From: Vijendar Mukunda Vijendar.Mukunda@amd.com
Added DMA driver changes for Stoney platform. Below are the key differences between Stoney and CZ: - Memory Gating is disabled - SRAM Banks won't be turned off - No Of SRAM Banks reduced to 6 - DAGB Garlic Interface used - 16 bit resolution is supported. - SRAM bank 1 & SRAM bank 2 will be used for playback scenario. - SRAM Bank 3 & SRAM Bank 4 will be used for Capture scenario.
Reviewed-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com --- sound/soc/amd/acp-pcm-dma.c | 81 +++++++++++++++++++++++++++++++++------------ sound/soc/amd/acp.h | 2 ++ 2 files changed, 61 insertions(+), 22 deletions(-)
diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index e48ae5d..5b5e95d 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -139,8 +139,8 @@ static void config_dma_descriptor_in_sram(void __iomem *acp_mmio, * system memory <-> ACP SRAM */ static void set_acp_sysmem_dma_descriptors(void __iomem *acp_mmio, - u32 size, int direction, - u32 pte_offset) + u32 size, int direction, + u32 pte_offset, u32 asic_type) { u16 i; u16 dma_dscr_idx = PLAYBACK_START_DMA_DESCR_CH12; @@ -154,20 +154,38 @@ static void set_acp_sysmem_dma_descriptors(void __iomem *acp_mmio, (size / 2) - (i * (size/2)); dmadscr[i].src = ACP_INTERNAL_APERTURE_WINDOW_0_ADDRESS + (pte_offset * SZ_4K) + (i * (size/2)); - dmadscr[i].xfer_val |= - (ACP_DMA_ATTRIBUTES_DAGB_ONION_TO_SHAREDMEM << 16) | - (size / 2); + if (asic_type == CHIP_STONEY) { + dmadscr[i].xfer_val |= + (ACP_DMA_ATTRIBUTES_DAGB_GARLIC_TO_SHAREDMEM << 16) | + (size / 2); + } else { + dmadscr[i].xfer_val |= + (ACP_DMA_ATTRIBUTES_DAGB_ONION_TO_SHAREDMEM << 16) | + (size / 2); + } } else { dma_dscr_idx = CAPTURE_START_DMA_DESCR_CH14 + i; - dmadscr[i].src = ACP_SHARED_RAM_BANK_5_ADDRESS + - (i * (size/2)); - dmadscr[i].dest = ACP_INTERNAL_APERTURE_WINDOW_0_ADDRESS - + (pte_offset * SZ_4K) + - (i * (size/2)); - dmadscr[i].xfer_val |= - BIT(22) | - (ACP_DMA_ATTRIBUTES_SHAREDMEM_TO_DAGB_ONION << 16) | - (size / 2); + if (asic_type == CHIP_STONEY) { + dmadscr[i].src = ACP_SHARED_RAM_BANK_3_ADDRESS + + (i * (size/2)); + dmadscr[i].dest = + ACP_INTERNAL_APERTURE_WINDOW_0_ADDRESS + + (pte_offset * SZ_4K) + (i * (size/2)); + dmadscr[i].xfer_val |= + BIT(22) | + (ACP_DMA_ATTRIBUTES_SHARED_MEM_TO_DAGB_GARLIC << 16) | + (size / 2); + } else { + dmadscr[i].src = ACP_SHARED_RAM_BANK_5_ADDRESS + + (i * (size/2)); + dmadscr[i].dest = + ACP_INTERNAL_APERTURE_WINDOW_0_ADDRESS + + (pte_offset * SZ_4K) + (i * (size/2)); + dmadscr[i].xfer_val |= + BIT(22) | + (ACP_DMA_ATTRIBUTES_SHAREDMEM_TO_DAGB_ONION << 16) | + (size / 2); + } } config_dma_descriptor_in_sram(acp_mmio, dma_dscr_idx, &dmadscr[i]); @@ -188,7 +206,8 @@ static void set_acp_sysmem_dma_descriptors(void __iomem *acp_mmio, * ACP SRAM <-> I2S */ static void set_acp_to_i2s_dma_descriptors(void __iomem *acp_mmio, - u32 size, int direction) + u32 size, int direction, + u32 asic_type) {
u16 i; @@ -209,8 +228,15 @@ static void set_acp_to_i2s_dma_descriptors(void __iomem *acp_mmio, dma_dscr_idx = CAPTURE_START_DMA_DESCR_CH15 + i; /* dmadscr[i].src is unused by hardware. */ dmadscr[i].src = 0; - dmadscr[i].dest = ACP_SHARED_RAM_BANK_5_ADDRESS + + if (asic_type == CHIP_STONEY) { + dmadscr[i].dest = + ACP_SHARED_RAM_BANK_3_ADDRESS + (i * (size / 2)); + } else { + dmadscr[i].dest = + ACP_SHARED_RAM_BANK_5_ADDRESS + + (i * (size / 2)); + } dmadscr[i].xfer_val |= BIT(22) | (FROM_ACP_I2S_1 << 16) | (size / 2); } @@ -266,7 +292,8 @@ static void acp_pte_config(void __iomem *acp_mmio, struct page *pg, }
static void config_acp_dma(void __iomem *acp_mmio, - struct audio_substream_data *audio_config) + struct audio_substream_data *audio_config, + u32 asic_type) { u32 pte_offset;
@@ -280,11 +307,12 @@ static void config_acp_dma(void __iomem *acp_mmio,
/* Configure System memory <-> ACP SRAM DMA descriptors */ set_acp_sysmem_dma_descriptors(acp_mmio, audio_config->size, - audio_config->direction, pte_offset); + audio_config->direction, pte_offset, + asic_type);
/* Configure ACP SRAM <-> I2S DMA descriptors */ set_acp_to_i2s_dma_descriptors(acp_mmio, audio_config->size, - audio_config->direction); + audio_config->direction, asic_type); }
/* Start a given DMA channel transfer */ @@ -500,6 +528,11 @@ static int acp_init(void __iomem *acp_mmio, u32 asic_type) for (bank = 1; bank < 48; bank++) acp_set_sram_bank_state(acp_mmio, bank, false); } + if (asic_type == CHIP_STONEY) { + val = acp_reg_read(acp_mmio, mmACP_I2S_16BIT_RESOLUTION_EN); + val |= 0x03; + acp_reg_write(val, acp_mmio, mmACP_I2S_16BIT_RESOLUTION_EN); + }
return 0; } @@ -675,6 +708,8 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream, struct page *pg; struct snd_pcm_runtime *runtime; struct audio_substream_data *rtd; + struct snd_soc_pcm_runtime *prtd = substream->private_data; + struct audio_drv_data *adata = dev_get_drvdata(prtd->platform->dev);
runtime = substream->runtime; rtd = runtime->private_data; @@ -702,7 +737,7 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream, rtd->num_of_pages = PAGE_ALIGN(size) >> PAGE_SHIFT; rtd->direction = substream->stream;
- config_acp_dma(rtd->acp_mmio, rtd); + config_acp_dma(rtd->acp_mmio, rtd, adata->asic_type); status = 0; } else { status = -ENOMEM; @@ -997,7 +1032,8 @@ static int acp_pcm_resume(struct device *dev) true); } config_acp_dma(adata->acp_mmio, - adata->play_stream->runtime->private_data); + adata->play_stream->runtime->private_data, + adata->asic_type); } if (adata->capture_stream && adata->capture_stream->runtime) { if (adata->asic_type == CHIP_CARRIZO) { @@ -1006,7 +1042,8 @@ static int acp_pcm_resume(struct device *dev) true); } config_acp_dma(adata->acp_mmio, - adata->capture_stream->runtime->private_data); + adata->capture_stream->runtime->private_data, + adata->asic_type); } acp_reg_write(1, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB); return 0; diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h index 28cf914..a330a99 100644 --- a/sound/soc/amd/acp.h +++ b/sound/soc/amd/acp.h @@ -19,6 +19,7 @@
/* Capture SRAM address (as a source in dma descriptor) */ #define ACP_SHARED_RAM_BANK_5_ADDRESS 0x400A000 +#define ACP_SHARED_RAM_BANK_3_ADDRESS 0x4006000
#define ACP_DMA_RESET_TIME 10000 #define ACP_CLOCK_EN_TIME_OUT_VALUE 0x000000FF @@ -67,6 +68,7 @@ #define CAPTURE_START_DMA_DESCR_CH15 6 #define CAPTURE_END_DMA_DESCR_CH15 7
+#define mmACP_I2S_16BIT_RESOLUTION_EN 0x5209 enum acp_dma_priority_level { /* 0x0 Specifies the DMA channel is given normal priority */ ACP_DMA_PRIORITY_LEVEL_NORMAL = 0x0,
From: Vijendar Mukunda Vijendar.Mukunda@amd.com
Stoney uses 16kb SRAM memory for playback and 16Kb for capture. Modified Max buffer size to have the correct mapping between System Memory and SRAM.
Added snd_pcm_hardware structures for playback and capture for Stoney.
Reviewed-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com --- sound/soc/amd/acp-pcm-dma.c | 68 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 5 deletions(-)
diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index 5b5e95d..c9bb618 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -34,6 +34,10 @@
#define MAX_BUFFER (PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS) #define MIN_BUFFER MAX_BUFFER +#define ST_PLAYBACK_MAX_PERIOD_SIZE 8192 +#define ST_CAPTURE_MAX_PERIOD_SIZE 8192 +#define ST_MAX_BUFFER (ST_PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS) +#define ST_MIN_BUFFER ST_MAX_BUFFER #define CHIP_STONEY 14 #define CHIP_CARRIZO 13
@@ -75,6 +79,44 @@ static const struct snd_pcm_hardware acp_pcm_hardware_capture = { .periods_max = CAPTURE_MAX_NUM_PERIODS, };
+static const struct snd_pcm_hardware acp_st_pcm_hardware_playback = { + .info = SNDRV_PCM_INFO_INTERLEAVED | + SNDRV_PCM_INFO_BLOCK_TRANSFER | SNDRV_PCM_INFO_MMAP | + SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_BATCH | + SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME, + .formats = SNDRV_PCM_FMTBIT_S16_LE | + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE, + .channels_min = 1, + .channels_max = 8, + .rates = SNDRV_PCM_RATE_8000_96000, + .rate_min = 8000, + .rate_max = 96000, + .buffer_bytes_max = ST_MAX_BUFFER, + .period_bytes_min = PLAYBACK_MIN_PERIOD_SIZE, + .period_bytes_max = ST_PLAYBACK_MAX_PERIOD_SIZE, + .periods_min = PLAYBACK_MIN_NUM_PERIODS, + .periods_max = PLAYBACK_MAX_NUM_PERIODS, +}; + +static const struct snd_pcm_hardware acp_st_pcm_hardware_capture = { + .info = SNDRV_PCM_INFO_INTERLEAVED | + SNDRV_PCM_INFO_BLOCK_TRANSFER | SNDRV_PCM_INFO_MMAP | + SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_BATCH | + SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME, + .formats = SNDRV_PCM_FMTBIT_S16_LE | + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE, + .channels_min = 1, + .channels_max = 2, + .rates = SNDRV_PCM_RATE_8000_48000, + .rate_min = 8000, + .rate_max = 48000, + .buffer_bytes_max = ST_MAX_BUFFER, + .period_bytes_min = CAPTURE_MIN_PERIOD_SIZE, + .period_bytes_max = ST_CAPTURE_MAX_PERIOD_SIZE, + .periods_min = CAPTURE_MIN_NUM_PERIODS, + .periods_max = CAPTURE_MAX_NUM_PERIODS, +}; + static u32 acp_reg_read(void __iomem *acp_mmio, u32 reg) { return readl(acp_mmio + (reg * 4)); @@ -657,10 +699,17 @@ static int acp_dma_open(struct snd_pcm_substream *substream) if (adata == NULL) return -ENOMEM;
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - runtime->hw = acp_pcm_hardware_playback; - else - runtime->hw = acp_pcm_hardware_capture; + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + if (intr_data->asic_type == CHIP_STONEY) + runtime->hw = acp_st_pcm_hardware_playback; + else + runtime->hw = acp_pcm_hardware_playback; + } else { + if (intr_data->asic_type == CHIP_STONEY) + runtime->hw = acp_st_pcm_hardware_capture; + else + runtime->hw = acp_pcm_hardware_capture; + }
ret = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS); @@ -894,10 +943,19 @@ static int acp_dma_trigger(struct snd_pcm_substream *substream, int cmd)
static int acp_dma_new(struct snd_soc_pcm_runtime *rtd) { - return snd_pcm_lib_preallocate_pages_for_all(rtd->pcm, + struct audio_drv_data *adata = dev_get_drvdata(rtd->platform->dev); + + if (adata->asic_type == CHIP_STONEY) { + return snd_pcm_lib_preallocate_pages_for_all(rtd->pcm, + SNDRV_DMA_TYPE_DEV, + NULL, MIN_BUFFER, + ST_MAX_BUFFER); + } else { + return snd_pcm_lib_preallocate_pages_for_all(rtd->pcm, SNDRV_DMA_TYPE_DEV, NULL, MIN_BUFFER, MAX_BUFFER); + } }
static int acp_dma_close(struct snd_pcm_substream *substream)
From: Vijendar Mukunda Vijendar.Mukunda@amd.com
Power Gating is disabled in Stoney platform.
Reviewed-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index 86b68f7..0e512fa 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -287,19 +287,20 @@ static int acp_hw_init(void *handle) return 0; else if (r) return r; + if (asic_type != CHIP_STONEY) { + adev->acp.acp_genpd = kzalloc(sizeof(struct acp_pm_domain), GFP_KERNEL); + if (adev->acp.acp_genpd == NULL) + return -ENOMEM;
- adev->acp.acp_genpd = kzalloc(sizeof(struct acp_pm_domain), GFP_KERNEL); - if (adev->acp.acp_genpd == NULL) - return -ENOMEM; - - adev->acp.acp_genpd->gpd.name = "ACP_AUDIO"; - adev->acp.acp_genpd->gpd.power_off = acp_poweroff; - adev->acp.acp_genpd->gpd.power_on = acp_poweron; + adev->acp.acp_genpd->gpd.name = "ACP_AUDIO"; + adev->acp.acp_genpd->gpd.power_off = acp_poweroff; + adev->acp.acp_genpd->gpd.power_on = acp_poweron;
- adev->acp.acp_genpd->cgs_dev = adev->acp.cgs_device; + adev->acp.acp_genpd->cgs_dev = adev->acp.cgs_device;
- pm_genpd_init(&adev->acp.acp_genpd->gpd, NULL, false); + pm_genpd_init(&adev->acp.acp_genpd->gpd, NULL, false); + }
adev->acp.acp_cell = kzalloc(sizeof(struct mfd_cell) * ACP_DEVS, GFP_KERNEL); @@ -388,12 +389,14 @@ static int acp_hw_init(void *handle) if (r) return r;
- for (i = 0; i < ACP_DEVS ; i++) { - dev = get_mfd_cell_dev(adev->acp.acp_cell[i].name, i); - r = pm_genpd_add_device(&adev->acp.acp_genpd->gpd, dev); - if (r) { - dev_err(dev, "Failed to add dev to genpd\n"); - return r; + if (asic_type != CHIP_STONEY) { + for (i = 0; i < ACP_DEVS ; i++) { + dev = get_mfd_cell_dev(adev->acp.acp_cell[i].name, i); + r = pm_genpd_add_device(&adev->acp.acp_genpd->gpd, dev); + if (r) { + dev_err(dev, "Failed to add dev to genpd\n"); + return r; + } } }
From: Akshu Agrawal akshu.agrawal@amd.com
The driver is used for AMD board using rt5650 codec.
Reviewed-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Akshu Agrawal akshu.agrawal@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com --- sound/soc/amd/Kconfig | 7 ++ sound/soc/amd/Makefile | 2 + sound/soc/amd/acp-rt5645.c | 210 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 219 insertions(+) create mode 100644 sound/soc/amd/acp-rt5645.c
diff --git a/sound/soc/amd/Kconfig b/sound/soc/amd/Kconfig index 78187eb..eb0ae60 100644 --- a/sound/soc/amd/Kconfig +++ b/sound/soc/amd/Kconfig @@ -2,3 +2,10 @@ config SND_SOC_AMD_ACP tristate "AMD Audio Coprocessor support" help This option enables ACP DMA support on AMD platform. +config SND_SOC_AMD_CZ_RT5645_MACH + tristate "AMD CZ support for RT5645" + select SND_SOC_RT5645 + select SND_SOC_AMD_ACP + depends on I2C_DESIGNWARE_PLATFORM + help + This option enables machine driver for rt5645. diff --git a/sound/soc/amd/Makefile b/sound/soc/amd/Makefile index 1a66ec0..eed64ff 100644 --- a/sound/soc/amd/Makefile +++ b/sound/soc/amd/Makefile @@ -1,3 +1,5 @@ snd-soc-acp-pcm-objs := acp-pcm-dma.o +snd-soc-acp-rt5645-mach-objs := acp-rt5645.o
obj-$(CONFIG_SND_SOC_AMD_ACP) += snd-soc-acp-pcm.o +obj-$(CONFIG_SND_SOC_AMD_CZ_RT5645_MACH) += snd-soc-acp-rt5645-mach.o diff --git a/sound/soc/amd/acp-rt5645.c b/sound/soc/amd/acp-rt5645.c new file mode 100644 index 0000000..ebaafad --- /dev/null +++ b/sound/soc/amd/acp-rt5645.c @@ -0,0 +1,210 @@ +/* + * Machine driver for AMD ACP Audio engine using Realtek RT5645 codec + * + * Copyright 2017 Advanced Micro Devices, Inc. + * + * This file is modified from rt288 machine driver + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + * + */ + +#include <sound/core.h> +#include <sound/soc.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc-dapm.h> +#include <sound/jack.h> +#include <linux/gpio.h> +#include <linux/module.h> +#include <linux/i2c.h> +#include <linux/acpi.h> + +#include "../codecs/rt5645.h" + +#define CZ_PLAT_CLK 24000000 + +static struct snd_soc_jack cz_jack; + +static int cz_aif1_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + int ret = 0; + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *codec_dai = rtd->codec_dai; + + ret = snd_soc_dai_set_pll(codec_dai, 0, RT5645_PLL1_S_MCLK, + CZ_PLAT_CLK, params_rate(params) * 512); + if (ret < 0) { + dev_err(rtd->dev, "can't set codec pll: %d\n", ret); + return ret; + } + + ret = snd_soc_dai_set_sysclk(codec_dai, RT5645_SCLK_S_PLL1, + params_rate(params) * 512, SND_SOC_CLOCK_OUT); + if (ret < 0) { + dev_err(rtd->dev, "can't set codec sysclk: %d\n", ret); + return ret; + } + + return ret; +} + +static int cz_init(struct snd_soc_pcm_runtime *rtd) +{ + int ret; + struct snd_soc_card *card; + struct snd_soc_codec *codec; + + codec = rtd->codec; + card = rtd->card; + + ret = snd_soc_card_jack_new(card, "Headset Jack", + SND_JACK_HEADPHONE | SND_JACK_MICROPHONE | + SND_JACK_BTN_0 | SND_JACK_BTN_1 | + SND_JACK_BTN_2 | SND_JACK_BTN_3, + &cz_jack, NULL, 0); + if (ret) { + dev_err(card->dev, "HP jack creation failed %d\n", ret); + return ret; + } + + rt5645_set_jack_detect(codec, &cz_jack, &cz_jack, &cz_jack); + + return 0; +} + +static struct snd_soc_ops cz_aif1_ops = { + .hw_params = cz_aif1_hw_params, +}; + +static struct snd_soc_dai_link cz_dai_rt5650[] = { + { + .name = "amd-rt5645-play", + .stream_name = "RT5645_AIF1", + .platform_name = "acp_audio_dma.0.auto", + .cpu_dai_name = "designware-i2s.1.auto", + .codec_dai_name = "rt5645-aif1", + .codec_name = "i2c-10EC5650:00", + .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF + | SND_SOC_DAIFMT_CBM_CFM, + .init = cz_init, + .ops = &cz_aif1_ops, + }, + { + .name = "amd-rt5645-cap", + .stream_name = "RT5645_AIF1", + .platform_name = "acp_audio_dma.0.auto", + .cpu_dai_name = "designware-i2s.2.auto", + .codec_dai_name = "rt5645-aif1", + .codec_name = "i2c-10EC5650:00", + .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF + | SND_SOC_DAIFMT_CBM_CFM, + .init = cz_init, + .ops = &cz_aif1_ops, + }, +}; + +static const struct snd_soc_dapm_widget cz_widgets[] = { + SND_SOC_DAPM_HP("Headphones", NULL), + SND_SOC_DAPM_SPK("Speakers", NULL), + SND_SOC_DAPM_MIC("Headset Mic", NULL), + SND_SOC_DAPM_MIC("Int Mic", NULL), +}; + +static const struct snd_soc_dapm_route cz_audio_route[] = { + {"Headphones", NULL, "HPOL"}, + {"Headphones", NULL, "HPOR"}, + {"RECMIXL", NULL, "Headset Mic"}, + {"RECMIXR", NULL, "Headset Mic"}, + {"Speakers", NULL, "SPOL"}, + {"Speakers", NULL, "SPOR"}, + {"DMIC L2", NULL, "Int Mic"}, + {"DMIC R2", NULL, "Int Mic"}, +}; + +static const struct snd_kcontrol_new cz_mc_controls[] = { + SOC_DAPM_PIN_SWITCH("Headphones"), + SOC_DAPM_PIN_SWITCH("Speakers"), + SOC_DAPM_PIN_SWITCH("Headset Mic"), + SOC_DAPM_PIN_SWITCH("Int Mic"), +}; + +static struct snd_soc_card cz_card = { + .name = "acprt5650", + .owner = THIS_MODULE, + .dai_link = cz_dai_rt5650, + .num_links = ARRAY_SIZE(cz_dai_rt5650), + .dapm_widgets = cz_widgets, + .num_dapm_widgets = ARRAY_SIZE(cz_widgets), + .dapm_routes = cz_audio_route, + .num_dapm_routes = ARRAY_SIZE(cz_audio_route), + .controls = cz_mc_controls, + .num_controls = ARRAY_SIZE(cz_mc_controls), +}; + +static int cz_probe(struct platform_device *pdev) +{ + int ret; + struct snd_soc_card *card; + + card = &cz_card; + cz_card.dev = &pdev->dev; + platform_set_drvdata(pdev, card); + ret = snd_soc_register_card(card); + if (ret) { + dev_err(&pdev->dev, + "snd_soc_register_card(%s) failed: %d\n", + cz_card.name, ret); + return ret; + } + return 0; +} + +static int cz_remove(struct platform_device *pdev) +{ + struct snd_soc_card *card; + + card = platform_get_drvdata(pdev); + snd_soc_unregister_card(card); + + return 0; +} + +static const struct acpi_device_id cz_audio_acpi_match[] = { + { "I2SC1002", 0 }, + {}, +}; + +static struct platform_driver cz_pcm_driver = { + .driver = { + .name = "cz-rt5645", + .acpi_match_table = ACPI_PTR(cz_audio_acpi_match), + .pm = &snd_soc_pm_ops, + }, + .probe = cz_probe, + .remove = cz_remove, +}; + +module_platform_driver(cz_pcm_driver); + +MODULE_AUTHOR("akshu.agrawal@amd.com"); +MODULE_DESCRIPTION("cz-rt5645 audio support"); +MODULE_LICENSE("GPL v2");
+static const struct acpi_device_id cz_audio_acpi_match[] = {
- { "I2SC1002", 0 },
This one goes on my list of _HID that don't follow ACPI/PCI vendorID/PartID conventions. AMD shoud use the "AMDI" ACPI ID or the 0x1002 PCI ID for the 4 first characters, if everyone does what they feel like one day we'll have a conflict between devices and probe the wrong driver...
- {},
+};
+static struct platform_driver cz_pcm_driver = {
- .driver = {
.name = "cz-rt5645",.acpi_match_table = ACPI_PTR(cz_audio_acpi_match),.pm = &snd_soc_pm_ops,- },
- .probe = cz_probe,
- .remove = cz_remove,
+};
+module_platform_driver(cz_pcm_driver);
+MODULE_AUTHOR("akshu.agrawal@amd.com"); +MODULE_DESCRIPTION("cz-rt5645 audio support"); +MODULE_LICENSE("GPL v2");
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Christian König -
Mark Brown -
Mukunda, Vijendar -
Pierre-Louis Bossart