Re: [PATCH v2] ASoC: amd: update pm_runtime enable sequence

Re: [PATCH v2] ASoC: amd: update pm_runtime enable sequence

[Limonciello, Mario]

On 6/23/2023 4:41 PM, Arun Gopal Kondaveeti wrote:
> pm_runtime_allow() is not needed for ACP child platform devices.
> Replace pm_runtime_allow() with pm_runtime_mark_last_busy()
> & pm_runtime_set_active() in pm_runtime enable sequence for
> ACP child platform drivers.

Can you explain what prompted this?

Does this fix a particular bug, or is it just to correct
things?  If it fixes a particular bug; I think it should be
split up across 5 patches (one for each APU) and then
appropriate Fixes tags applied for each on the code they're
fixing.

>
> Signed-off-by: Arun Gopal <arungopal.kondaveeti@amd.com>
> ---
>
> Changes since v1:
>      -- Updated commit message
>
> ---
>   sound/soc/amd/ps/ps-pdm-dma.c         | 3 ++-
>   sound/soc/amd/raven/acp3x-pcm-dma.c   | 3 ++-
>   sound/soc/amd/renoir/acp3x-pdm-dma.c  | 3 ++-
>   sound/soc/amd/vangogh/acp5x-pcm-dma.c | 4 ++--
>   sound/soc/amd/yc/acp6x-pdm-dma.c      | 3 ++-
>   5 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/amd/ps/ps-pdm-dma.c b/sound/soc/amd/ps/ps-pdm-dma.c
> index bdbbb797c74d..d48f7c5af289 100644
> --- a/sound/soc/amd/ps/ps-pdm-dma.c
> +++ b/sound/soc/amd/ps/ps-pdm-dma.c
> @@ -391,8 +391,9 @@ static int acp63_pdm_audio_probe(struct platform_device *pdev)
>   	}
>   	pm_runtime_set_autosuspend_delay(&pdev->dev, ACP_SUSPEND_DELAY_MS);
>   	pm_runtime_use_autosuspend(&pdev->dev);
> +	pm_runtime_mark_last_busy(&pdev->dev);
> +	pm_runtime_set_active(&pdev->dev);
>   	pm_runtime_enable(&pdev->dev);
> -	pm_runtime_allow(&pdev->dev);
>   	return 0;
>   }
>   
> diff --git a/sound/soc/amd/raven/acp3x-pcm-dma.c b/sound/soc/amd/raven/acp3x-pcm-dma.c
> index 7362dd15ad30..9538f3ffc5d9 100644
> --- a/sound/soc/amd/raven/acp3x-pcm-dma.c
> +++ b/sound/soc/amd/raven/acp3x-pcm-dma.c
> @@ -416,8 +416,9 @@ static int acp3x_audio_probe(struct platform_device *pdev)
>   
>   	pm_runtime_set_autosuspend_delay(&pdev->dev, 2000);
>   	pm_runtime_use_autosuspend(&pdev->dev);
> +	pm_runtime_mark_last_busy(&pdev->dev);
> +	pm_runtime_set_active(&pdev->dev);
>   	pm_runtime_enable(&pdev->dev);
> -	pm_runtime_allow(&pdev->dev);
>   	return 0;
>   }
>   
> diff --git a/sound/soc/amd/renoir/acp3x-pdm-dma.c b/sound/soc/amd/renoir/acp3x-pdm-dma.c
> index 4e299f96521f..c3b47e9bd239 100644
> --- a/sound/soc/amd/renoir/acp3x-pdm-dma.c
> +++ b/sound/soc/amd/renoir/acp3x-pdm-dma.c
> @@ -430,8 +430,9 @@ static int acp_pdm_audio_probe(struct platform_device *pdev)
>   	}
>   	pm_runtime_set_autosuspend_delay(&pdev->dev, ACP_SUSPEND_DELAY_MS);
>   	pm_runtime_use_autosuspend(&pdev->dev);
> +	pm_runtime_mark_last_busy(&pdev->dev);
> +	pm_runtime_set_active(&pdev->dev);
>   	pm_runtime_enable(&pdev->dev);
> -	pm_runtime_allow(&pdev->dev);
>   	return 0;
>   }
>   
> diff --git a/sound/soc/amd/vangogh/acp5x-pcm-dma.c b/sound/soc/amd/vangogh/acp5x-pcm-dma.c
> index 29901ee4bfe3..587dec5bb33d 100644
> --- a/sound/soc/amd/vangogh/acp5x-pcm-dma.c
> +++ b/sound/soc/amd/vangogh/acp5x-pcm-dma.c
> @@ -409,9 +409,9 @@ static int acp5x_audio_probe(struct platform_device *pdev)
>   	}
>   	pm_runtime_set_autosuspend_delay(&pdev->dev, 2000);
>   	pm_runtime_use_autosuspend(&pdev->dev);
> +	pm_runtime_mark_last_busy(&pdev->dev);
> +	pm_runtime_set_active(&pdev->dev);
>   	pm_runtime_enable(&pdev->dev);
> -	pm_runtime_allow(&pdev->dev);
> -
>   	return 0;
>   }
>   
> diff --git a/sound/soc/amd/yc/acp6x-pdm-dma.c b/sound/soc/amd/yc/acp6x-pdm-dma.c
> index d818eba48546..72c4591e451b 100644
> --- a/sound/soc/amd/yc/acp6x-pdm-dma.c
> +++ b/sound/soc/amd/yc/acp6x-pdm-dma.c
> @@ -383,8 +383,9 @@ static int acp6x_pdm_audio_probe(struct platform_device *pdev)
>   	}
>   	pm_runtime_set_autosuspend_delay(&pdev->dev, ACP_SUSPEND_DELAY_MS);
>   	pm_runtime_use_autosuspend(&pdev->dev);
> +	pm_runtime_mark_last_busy(&pdev->dev);
> +	pm_runtime_set_active(&pdev->dev);
>   	pm_runtime_enable(&pdev->dev);
> -	pm_runtime_allow(&pdev->dev);
>   	return 0;
>   }
>   

Re: [PATCH v2] ASoC: amd: update pm_runtime enable sequence

[Mark Brown]

On Sat, 24 Jun 2023 03:11:40 +0530, Arun Gopal Kondaveeti wrote:
> pm_runtime_allow() is not needed for ACP child platform devices.
> Replace pm_runtime_allow() with pm_runtime_mark_last_busy()
> & pm_runtime_set_active() in pm_runtime enable sequence for
> ACP child platform drivers.
> 
> 

Applied to

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

Thanks!

[1/1] ASoC: amd: update pm_runtime enable sequence
      commit: 154756319cc6f8b8b86241da02da6a8fcc6abd1f

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

Re: [PATCH v2] ASoC: amd: update pm_runtime enable sequence

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

On Fri, Jun 23, 2023 at 08:40:15AM -0500, Limonciello, Mario wrote:
> On 6/23/2023 4:41 PM, Arun Gopal Kondaveeti wrote:
> > pm_runtime_allow() is not needed for ACP child platform devices.
> > Replace pm_runtime_allow() with pm_runtime_mark_last_busy()
> > & pm_runtime_set_active() in pm_runtime enable sequence for
> > ACP child platform drivers.

> Can you explain what prompted this?

> Does this fix a particular bug, or is it just to correct
> things?  If it fixes a particular bug; I think it should be
> split up across 5 patches (one for each APU) and then
> appropriate Fixes tags applied for each on the code they're
> fixing.

I had already gone ahead and applied this before your mail - it seemed
better to get it into 6.5 than leave it waiting longer.  If there's a
need for backports that can be handled through the stable process.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH v2] ASoC: amd: update pm_runtime enable sequence

[Limonciello, Mario]

[AMD Official Use Only - General]

> On Fri, Jun 23, 2023 at 08:40:15AM -0500, Limonciello, Mario wrote:
> > On 6/23/2023 4:41 PM, Arun Gopal Kondaveeti wrote:
> > > pm_runtime_allow() is not needed for ACP child platform devices.
> > > Replace pm_runtime_allow() with pm_runtime_mark_last_busy()
> > > & pm_runtime_set_active() in pm_runtime enable sequence for
> > > ACP child platform drivers.
>
> > Can you explain what prompted this?
>
> > Does this fix a particular bug, or is it just to correct
> > things?  If it fixes a particular bug; I think it should be
> > split up across 5 patches (one for each APU) and then
> > appropriate Fixes tags applied for each on the code they're
> > fixing.
>
> I had already gone ahead and applied this before your mail - it seemed
> better to get it into 6.5 than leave it waiting longer.  If there's a
> need for backports that can be handled through the stable process.

Sounds good, thanks!

[PATCH v2] ASoC: amd: update pm_runtime enable sequence

[Arun Gopal Kondaveeti]

pm_runtime_allow() is not needed for ACP child platform devices.
Replace pm_runtime_allow() with pm_runtime_mark_last_busy()
& pm_runtime_set_active() in pm_runtime enable sequence for
ACP child platform drivers.

Signed-off-by: Arun Gopal <arungopal.kondaveeti@amd.com>
---

Changes since v1:
    -- Updated commit message

---
 sound/soc/amd/ps/ps-pdm-dma.c         | 3 ++-
 sound/soc/amd/raven/acp3x-pcm-dma.c   | 3 ++-
 sound/soc/amd/renoir/acp3x-pdm-dma.c  | 3 ++-
 sound/soc/amd/vangogh/acp5x-pcm-dma.c | 4 ++--
 sound/soc/amd/yc/acp6x-pdm-dma.c      | 3 ++-
 5 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/sound/soc/amd/ps/ps-pdm-dma.c b/sound/soc/amd/ps/ps-pdm-dma.c
index bdbbb797c74d..d48f7c5af289 100644
--- a/sound/soc/amd/ps/ps-pdm-dma.c
+++ b/sound/soc/amd/ps/ps-pdm-dma.c
@@ -391,8 +391,9 @@ static int acp63_pdm_audio_probe(struct platform_device *pdev)
 	}
 	pm_runtime_set_autosuspend_delay(&pdev->dev, ACP_SUSPEND_DELAY_MS);
 	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
-	pm_runtime_allow(&pdev->dev);
 	return 0;
 }
 
diff --git a/sound/soc/amd/raven/acp3x-pcm-dma.c b/sound/soc/amd/raven/acp3x-pcm-dma.c
index 7362dd15ad30..9538f3ffc5d9 100644
--- a/sound/soc/amd/raven/acp3x-pcm-dma.c
+++ b/sound/soc/amd/raven/acp3x-pcm-dma.c
@@ -416,8 +416,9 @@ static int acp3x_audio_probe(struct platform_device *pdev)
 
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 2000);
 	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
-	pm_runtime_allow(&pdev->dev);
 	return 0;
 }
 
diff --git a/sound/soc/amd/renoir/acp3x-pdm-dma.c b/sound/soc/amd/renoir/acp3x-pdm-dma.c
index 4e299f96521f..c3b47e9bd239 100644
--- a/sound/soc/amd/renoir/acp3x-pdm-dma.c
+++ b/sound/soc/amd/renoir/acp3x-pdm-dma.c
@@ -430,8 +430,9 @@ static int acp_pdm_audio_probe(struct platform_device *pdev)
 	}
 	pm_runtime_set_autosuspend_delay(&pdev->dev, ACP_SUSPEND_DELAY_MS);
 	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
-	pm_runtime_allow(&pdev->dev);
 	return 0;
 }
 
diff --git a/sound/soc/amd/vangogh/acp5x-pcm-dma.c b/sound/soc/amd/vangogh/acp5x-pcm-dma.c
index 29901ee4bfe3..587dec5bb33d 100644
--- a/sound/soc/amd/vangogh/acp5x-pcm-dma.c
+++ b/sound/soc/amd/vangogh/acp5x-pcm-dma.c
@@ -409,9 +409,9 @@ static int acp5x_audio_probe(struct platform_device *pdev)
 	}
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 2000);
 	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
-	pm_runtime_allow(&pdev->dev);
-
 	return 0;
 }
 
diff --git a/sound/soc/amd/yc/acp6x-pdm-dma.c b/sound/soc/amd/yc/acp6x-pdm-dma.c
index d818eba48546..72c4591e451b 100644
--- a/sound/soc/amd/yc/acp6x-pdm-dma.c
+++ b/sound/soc/amd/yc/acp6x-pdm-dma.c
@@ -383,8 +383,9 @@ static int acp6x_pdm_audio_probe(struct platform_device *pdev)
 	}
 	pm_runtime_set_autosuspend_delay(&pdev->dev, ACP_SUSPEND_DELAY_MS);
 	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
-	pm_runtime_allow(&pdev->dev);
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH v2] ASoC: amd: update pm_runtime enable sequence

[kondaveeti, Arungopal]

On 6/23/2023 7:10 PM, Limonciello, Mario wrote:
>
> On 6/23/2023 4:41 PM, Arun Gopal Kondaveeti wrote:
>> pm_runtime_allow() is not needed for ACP child platform devices.
>> Replace pm_runtime_allow() with pm_runtime_mark_last_busy()
>> & pm_runtime_set_active() in pm_runtime enable sequence for
>> ACP child platform drivers.
>
> Can you explain what prompted this?
>
> Does this fix a particular bug, or is it just to correct
> things?  If it fixes a particular bug; I think it should be
> split up across 5 patches (one for each APU) and then
> appropriate Fixes tags applied for each on the code they're
> fixing.

> it is not a fix.
just correct things.
>
>>
>> Signed-off-by: Arun Gopal <arungopal.kondaveeti@amd.com>
>> ---
>>
>> Changes since v1:
>>      -- Updated commit message
>>
>> ---
>>   sound/soc/amd/ps/ps-pdm-dma.c         | 3 ++-
>>   sound/soc/amd/raven/acp3x-pcm-dma.c   | 3 ++-
>>   sound/soc/amd/renoir/acp3x-pdm-dma.c  | 3 ++-
>>   sound/soc/amd/vangogh/acp5x-pcm-dma.c | 4 ++--
>>   sound/soc/amd/yc/acp6x-pdm-dma.c      | 3 ++-
>>   5 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/sound/soc/amd/ps/ps-pdm-dma.c 
>> b/sound/soc/amd/ps/ps-pdm-dma.c
>> index bdbbb797c74d..d48f7c5af289 100644
>> --- a/sound/soc/amd/ps/ps-pdm-dma.c
>> +++ b/sound/soc/amd/ps/ps-pdm-dma.c
>> @@ -391,8 +391,9 @@ static int acp63_pdm_audio_probe(struct 
>> platform_device *pdev)
>>       }
>>       pm_runtime_set_autosuspend_delay(&pdev->dev, 
>> ACP_SUSPEND_DELAY_MS);
>>       pm_runtime_use_autosuspend(&pdev->dev);
>> +    pm_runtime_mark_last_busy(&pdev->dev);
>> +    pm_runtime_set_active(&pdev->dev);
>>       pm_runtime_enable(&pdev->dev);
>> -    pm_runtime_allow(&pdev->dev);
>>       return 0;
>>   }
>>   diff --git a/sound/soc/amd/raven/acp3x-pcm-dma.c 
>> b/sound/soc/amd/raven/acp3x-pcm-dma.c
>> index 7362dd15ad30..9538f3ffc5d9 100644
>> --- a/sound/soc/amd/raven/acp3x-pcm-dma.c
>> +++ b/sound/soc/amd/raven/acp3x-pcm-dma.c
>> @@ -416,8 +416,9 @@ static int acp3x_audio_probe(struct 
>> platform_device *pdev)
>>         pm_runtime_set_autosuspend_delay(&pdev->dev, 2000);
>>       pm_runtime_use_autosuspend(&pdev->dev);
>> +    pm_runtime_mark_last_busy(&pdev->dev);
>> +    pm_runtime_set_active(&pdev->dev);
>>       pm_runtime_enable(&pdev->dev);
>> -    pm_runtime_allow(&pdev->dev);
>>       return 0;
>>   }
>>   diff --git a/sound/soc/amd/renoir/acp3x-pdm-dma.c 
>> b/sound/soc/amd/renoir/acp3x-pdm-dma.c
>> index 4e299f96521f..c3b47e9bd239 100644
>> --- a/sound/soc/amd/renoir/acp3x-pdm-dma.c
>> +++ b/sound/soc/amd/renoir/acp3x-pdm-dma.c
>> @@ -430,8 +430,9 @@ static int acp_pdm_audio_probe(struct 
>> platform_device *pdev)
>>       }
>>       pm_runtime_set_autosuspend_delay(&pdev->dev, 
>> ACP_SUSPEND_DELAY_MS);
>>       pm_runtime_use_autosuspend(&pdev->dev);
>> +    pm_runtime_mark_last_busy(&pdev->dev);
>> +    pm_runtime_set_active(&pdev->dev);
>>       pm_runtime_enable(&pdev->dev);
>> -    pm_runtime_allow(&pdev->dev);
>>       return 0;
>>   }
>>   diff --git a/sound/soc/amd/vangogh/acp5x-pcm-dma.c 
>> b/sound/soc/amd/vangogh/acp5x-pcm-dma.c
>> index 29901ee4bfe3..587dec5bb33d 100644
>> --- a/sound/soc/amd/vangogh/acp5x-pcm-dma.c
>> +++ b/sound/soc/amd/vangogh/acp5x-pcm-dma.c
>> @@ -409,9 +409,9 @@ static int acp5x_audio_probe(struct 
>> platform_device *pdev)
>>       }
>>       pm_runtime_set_autosuspend_delay(&pdev->dev, 2000);
>>       pm_runtime_use_autosuspend(&pdev->dev);
>> +    pm_runtime_mark_last_busy(&pdev->dev);
>> +    pm_runtime_set_active(&pdev->dev);
>>       pm_runtime_enable(&pdev->dev);
>> -    pm_runtime_allow(&pdev->dev);
>> -
>>       return 0;
>>   }
>>   diff --git a/sound/soc/amd/yc/acp6x-pdm-dma.c 
>> b/sound/soc/amd/yc/acp6x-pdm-dma.c
>> index d818eba48546..72c4591e451b 100644
>> --- a/sound/soc/amd/yc/acp6x-pdm-dma.c
>> +++ b/sound/soc/amd/yc/acp6x-pdm-dma.c
>> @@ -383,8 +383,9 @@ static int acp6x_pdm_audio_probe(struct 
>> platform_device *pdev)
>>       }
>>       pm_runtime_set_autosuspend_delay(&pdev->dev, 
>> ACP_SUSPEND_DELAY_MS);
>>       pm_runtime_use_autosuspend(&pdev->dev);
>> +    pm_runtime_mark_last_busy(&pdev->dev);
>> +    pm_runtime_set_active(&pdev->dev);
>>       pm_runtime_enable(&pdev->dev);
>> -    pm_runtime_allow(&pdev->dev);
>>       return 0;
>>   }