| Message ID | 20220922145846.114312-1-zhangqilong3@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 08fc2a7448afc1660ec2f1b5c437fcd14155a7ee |
| Headers | show |
| Series | [-next] ASoC: soc-component: using pm_runtime_resume_and_get instead of pm_runtime_get_sync | expand |
On Thu, 22 Sep 2022 22:58:46 +0800, Zhang Qilong wrote: > Using the newest pm_runtime_resume_and_get is more appropriate > for simplifing code here. > > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: soc-component: using pm_runtime_resume_and_get instead of pm_runtime_get_sync commit: 08fc2a7448afc1660ec2f1b5c437fcd14155a7ee 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
Hi, On Thu, Sep 22, 2022 at 10:58:46PM +0800, Zhang Qilong wrote: > Using the newest pm_runtime_resume_and_get is more appropriate > for simplifing code here. > > Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com> > --- > sound/soc/soc-component.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c > index e12f8244242b..659b9ade4158 100644 > --- a/sound/soc/soc-component.c > +++ b/sound/soc/soc-component.c > @@ -1213,11 +1213,9 @@ int snd_soc_pcm_component_pm_runtime_get(struct snd_soc_pcm_runtime *rtd, > int i; > > for_each_rtd_components(rtd, i, component) { > - int ret = pm_runtime_get_sync(component->dev); > - if (ret < 0 && ret != -EACCES) { > - pm_runtime_put_noidle(component->dev); > + int ret = pm_runtime_resume_and_get(component->dev); > + if (ret < 0 && ret != -EACCES) > return soc_component_ret(component, ret); > - } > /* mark stream if succeeded */ > soc_component_mark_push(component, stream, pm); This creates an issue on the RaspberryPi4 on 6.1-rc1. At boot time, we now have a bunch of: [ 35.536496] hdmi-audio-codec hdmi-audio-codec.1.auto: Runtime PM usage count underflow! They were bisected back to that commit, and reverting it makes it go away. I think this is due to the fact that the function here used to call pm_runtime_put_noidle() only if there was an error, and that error wasn't EACCES. However, pm_runtime_resume_and_get() will call pm_runtime_put_noidle() for any error. Thus, if our __pm_runtime_resume() call return EACCES, we used to keep the our reference but we will now drop it. So I guess we should just revert it, possibly with a comment? Maxime
On 10/19/22 08:56, Maxime Ripard wrote: > Hi, > > On Thu, Sep 22, 2022 at 10:58:46PM +0800, Zhang Qilong wrote: >> Using the newest pm_runtime_resume_and_get is more appropriate >> for simplifing code here. >> >> Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com> >> --- >> sound/soc/soc-component.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c >> index e12f8244242b..659b9ade4158 100644 >> --- a/sound/soc/soc-component.c >> +++ b/sound/soc/soc-component.c >> @@ -1213,11 +1213,9 @@ int snd_soc_pcm_component_pm_runtime_get(struct snd_soc_pcm_runtime *rtd, >> int i; >> >> for_each_rtd_components(rtd, i, component) { >> - int ret = pm_runtime_get_sync(component->dev); >> - if (ret < 0 && ret != -EACCES) { >> - pm_runtime_put_noidle(component->dev); >> + int ret = pm_runtime_resume_and_get(component->dev); >> + if (ret < 0 && ret != -EACCES) >> return soc_component_ret(component, ret); >> - } >> /* mark stream if succeeded */ >> soc_component_mark_push(component, stream, pm); > > This creates an issue on the RaspberryPi4 on 6.1-rc1. > > At boot time, we now have a bunch of: > [ 35.536496] hdmi-audio-codec hdmi-audio-codec.1.auto: Runtime PM usage count underflow! > > They were bisected back to that commit, and reverting it makes it go > away. > > I think this is due to the fact that the function here used to call > pm_runtime_put_noidle() only if there was an error, and that error > wasn't EACCES. However, pm_runtime_resume_and_get() will call > pm_runtime_put_noidle() for any error. > > Thus, if our __pm_runtime_resume() call return EACCES, we used to keep > the our reference but we will now drop it. So I guess we should just > revert it, possibly with a comment? This is already reverted, see patch from Peter: [PATCH] Revert "ASoC: soc-component: using pm_runtime_resume_and_get instead of pm_runtime_get_sync"
On Wed, Oct 19, 2022 at 08:59:58AM -0500, Pierre-Louis Bossart wrote: > On 10/19/22 08:56, Maxime Ripard wrote: > > On Thu, Sep 22, 2022 at 10:58:46PM +0800, Zhang Qilong wrote: > >> Using the newest pm_runtime_resume_and_get is more appropriate > >> for simplifing code here. > >> > >> Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com> > >> --- > >> sound/soc/soc-component.c | 6 ++---- > >> 1 file changed, 2 insertions(+), 4 deletions(-) > >> > >> diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c > >> index e12f8244242b..659b9ade4158 100644 > >> --- a/sound/soc/soc-component.c > >> +++ b/sound/soc/soc-component.c > >> @@ -1213,11 +1213,9 @@ int snd_soc_pcm_component_pm_runtime_get(struct snd_soc_pcm_runtime *rtd, > >> int i; > >> > >> for_each_rtd_components(rtd, i, component) { > >> - int ret = pm_runtime_get_sync(component->dev); > >> - if (ret < 0 && ret != -EACCES) { > >> - pm_runtime_put_noidle(component->dev); > >> + int ret = pm_runtime_resume_and_get(component->dev); > >> + if (ret < 0 && ret != -EACCES) > >> return soc_component_ret(component, ret); > >> - } > >> /* mark stream if succeeded */ > >> soc_component_mark_push(component, stream, pm); > > > > This creates an issue on the RaspberryPi4 on 6.1-rc1. > > > > At boot time, we now have a bunch of: > > [ 35.536496] hdmi-audio-codec hdmi-audio-codec.1.auto: Runtime PM usage count underflow! > > > > They were bisected back to that commit, and reverting it makes it go > > away. > > > > I think this is due to the fact that the function here used to call > > pm_runtime_put_noidle() only if there was an error, and that error > > wasn't EACCES. However, pm_runtime_resume_and_get() will call > > pm_runtime_put_noidle() for any error. > > > > Thus, if our __pm_runtime_resume() call return EACCES, we used to keep > > the our reference but we will now drop it. So I guess we should just > > revert it, possibly with a comment? > > This is already reverted, see patch from Peter: > > [PATCH] Revert "ASoC: soc-component: using pm_runtime_resume_and_get > instead of pm_runtime_get_sync" I missed it, thanks for the pointer Maxime
diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c index e12f8244242b..659b9ade4158 100644 --- a/sound/soc/soc-component.c +++ b/sound/soc/soc-component.c @@ -1213,11 +1213,9 @@ int snd_soc_pcm_component_pm_runtime_get(struct snd_soc_pcm_runtime *rtd, int i; for_each_rtd_components(rtd, i, component) { - int ret = pm_runtime_get_sync(component->dev); - if (ret < 0 && ret != -EACCES) { - pm_runtime_put_noidle(component->dev); + int ret = pm_runtime_resume_and_get(component->dev); + if (ret < 0 && ret != -EACCES) return soc_component_ret(component, ret); - } /* mark stream if succeeded */ soc_component_mark_push(component, stream, pm); }
Using the newest pm_runtime_resume_and_get is more appropriate for simplifing code here. Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com> --- sound/soc/soc-component.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)