Re: [PATCH] ASoC: amd: ps: fix for soundwire failures during hibernation exit sequence

["Mukunda,Vijendar" <vijendar.mukunda@amd.com>]

On 21/06/25 15:17, Vijendar Mukunda wrote:
> During the hibernate entry sequence, ACP registers will be reset to
> default values and acp ip will be completely powered off including acp
> SoundWire pads. During resume sequence, if acp SoundWire pad keeper enable
> register is not restored along with pad pulldown control register value,
> then SoundWire manager links won't be powered on correctly results in
> peripheral register access failures and completely audio function is
> broken.
>
> Add code to store the acp SoundWire pad keeper enable register and acp pad
> pulldown ctrl register values before entering into suspend state and
> restore the register values during resume sequence based on condition check
> for acp SoundWire pad keeper enable register for ACP6.3, ACP7.0 & ACP7.1
> platforms.
>
> Fixes: 491628388005 ("ASoC: amd: ps: add callback functions for acp pci driver pm ops")
> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
> ---
>  sound/soc/amd/ps/acp63.h     |  5 +++++
>  sound/soc/amd/ps/ps-common.c | 18 ++++++++++++++++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/sound/soc/amd/ps/acp63.h b/sound/soc/amd/ps/acp63.h
> index 85feae45c44c..babf2fa7ea27 100644
> --- a/sound/soc/amd/ps/acp63.h
> +++ b/sound/soc/amd/ps/acp63.h
> @@ -82,6 +82,7 @@
>  #define ACP63_SDW0_DMA_MAX_STREAMS	6
>  #define ACP63_SDW1_DMA_MAX_STREAMS	2
>  #define ACP63_P1_AUDIO_TX_THRESHOLD	6
> +#define ACP_SW_PAD_KEEPER_EN		0x0001454
>  
>  /*
>   * Below entries describes SDW0 instance DMA stream id and DMA irq bit mapping
> @@ -334,6 +335,8 @@ struct acp_hw_ops {
>   * @addr: pci ioremap address
>   * @reg_range: ACP reigister range
>   * @acp_rev: ACP PCI revision id
> + * @acp_sw_pad_keeper_en: store acp SoundWire pad keeper enable register value
> + * @acp_pad_pulldown_ctrl: store acp pad pulldown control register value
>   * @acp63_sdw0-dma_intr_stat: DMA interrupt status array for ACP6.3 platform SoundWire
>   * manager-SW0 instance
>   * @acp63_sdw_dma_intr_stat: DMA interrupt status array for ACP6.3 platform SoundWire
> @@ -367,6 +370,8 @@ struct acp63_dev_data {
>  	u32 addr;
>  	u32 reg_range;
>  	u32 acp_rev;
> +	u32 acp_sw_pad_keeper_en;
> +	u32 acp_pad_pulldown_ctrl;
>  	u16 acp63_sdw0_dma_intr_stat[ACP63_SDW0_DMA_MAX_STREAMS];
>  	u16 acp63_sdw1_dma_intr_stat[ACP63_SDW1_DMA_MAX_STREAMS];
>  	u16 acp70_sdw0_dma_intr_stat[ACP70_SDW0_DMA_MAX_STREAMS];
> diff --git a/sound/soc/amd/ps/ps-common.c b/sound/soc/amd/ps/ps-common.c
> index 1c89fb5fe1da..f18d2a0d83aa 100644
> --- a/sound/soc/amd/ps/ps-common.c
> +++ b/sound/soc/amd/ps/ps-common.c
> @@ -160,6 +160,8 @@ static int __maybe_unused snd_acp63_suspend(struct device *dev)
>  
>  	adata = dev_get_drvdata(dev);
>  	if (adata->is_sdw_dev) {
> +		adata->acp_sw_pad_keeper_en = readl(adata->acp63_base + ACP_SW_PAD_KEEPER_EN);
> +		adata->acp_pad_pulldown_ctrl = readl(adata->acp63_base + ACP_PAD_PULLDOWN_CTRL);
>  		adata->sdw_en_stat = check_acp_sdw_enable_status(adata);
>  		if (adata->sdw_en_stat) {
>  			writel(1, adata->acp63_base + ACP_ZSC_DSP_CTRL);
> @@ -197,6 +199,7 @@ static int __maybe_unused snd_acp63_runtime_resume(struct device *dev)
>  static int __maybe_unused snd_acp63_resume(struct device *dev)
>  {
>  	struct acp63_dev_data *adata;
> +	u32 acp_sw_pad_keeper_en;
>  	int ret;
>  
>  	adata = dev_get_drvdata(dev);
> @@ -209,6 +212,12 @@ static int __maybe_unused snd_acp63_resume(struct device *dev)
>  	if (ret)
>  		dev_err(dev, "ACP init failed\n");
>  
> +	acp_sw_pad_keeper_en = readl(adata->acp63_base + ACP_SW_PAD_KEEPER_EN);
> +	dev_dbg(dev, "ACP_SW_PAD_KEEPER_EN:0x%x\n", acp_sw_pad_keeper_en);
> +	if (!acp_sw_pad_keeper_en) {
> +		writel(adata->acp_sw_pad_keeper_en, adata->acp63_base + ACP_SW_PAD_KEEPER_EN);
> +		writel(adata->acp_pad_pulldown_ctrl, adata->acp63_base + ACP_PAD_PULLDOWN_CTRL);
> +	}
>  	return ret;
>  }
>  
> @@ -408,6 +417,8 @@ static int __maybe_unused snd_acp70_suspend(struct device *dev)
>  
>  	adata = dev_get_drvdata(dev);
>  	if (adata->is_sdw_dev) {
> +		adata->acp_sw_pad_keeper_en = readl(adata->acp63_base + ACP_SW0_PAD_KEEPER_EN);
As register offsets, ACP_SW_PAD_KEEPER_EN & ACP_SW0_PAD_KEEPER_EN
are same, will use the same register macro and post the v2 version.


> +		adata->acp_pad_pulldown_ctrl = readl(adata->acp63_base + ACP_PAD_PULLDOWN_CTRL);
>  		adata->sdw_en_stat = check_acp_sdw_enable_status(adata);
>  		if (adata->sdw_en_stat) {
>  			writel(1, adata->acp63_base + ACP_ZSC_DSP_CTRL);
> @@ -445,6 +456,7 @@ static int __maybe_unused snd_acp70_runtime_resume(struct device *dev)
>  static int __maybe_unused snd_acp70_resume(struct device *dev)
>  {
>  	struct acp63_dev_data *adata;
> +	u32 acp_sw_pad_keeper_en;
>  	int ret;
>  
>  	adata = dev_get_drvdata(dev);
> @@ -459,6 +471,12 @@ static int __maybe_unused snd_acp70_resume(struct device *dev)
>  	if (ret)
>  		dev_err(dev, "ACP init failed\n");
>  
> +	acp_sw_pad_keeper_en = readl(adata->acp63_base + ACP_SW_PAD_KEEPER_EN);
> +	dev_dbg(dev, "ACP_SW_PAD_KEEPER_EN:0x%x\n", acp_sw_pad_keeper_en);
> +	if (!acp_sw_pad_keeper_en) {
> +		writel(adata->acp_sw_pad_keeper_en, adata->acp63_base + ACP_SW0_PAD_KEEPER_EN);
> +		writel(adata->acp_pad_pulldown_ctrl, adata->acp63_base + ACP_PAD_PULLDOWN_CTRL);
> +	}
>  	return ret;
>  }
>