Skip to content

Conversation

@RanderWang
Copy link

The child_count of SOF device is increased in snd_hdac_ext_bus_device_init,
so SOF can't get suspended if we just return when HDMI codec doesn't
work.

snd_hdac_ext_bus_device_exit will be invoked in snd_hdac_ext_bus_device_init
if it gets a error. Now copy this behavior to decrease SOF device
child_count to release SOF device.

fixes #2274

@RanderWang RanderWang force-pushed the runtime_pm_hdmi branch 3 times, most recently from ff3069f to d89bff4 Compare July 17, 2020 09:00
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand the idea @RanderWang but I made a set of suggestions to better explain what you are trying to fix. I also don't know if @kv2019i will have a chance to review this.

kv2019i
kv2019i previously approved these changes Jul 20, 2020
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @RanderWang ! This (plus the recent regression in 5.8-rc) that we need to get these configs under CI testing. To me the patchset looks good.. if you add the additional comments, this should be good.

lyakh
lyakh previously approved these changes Jul 21, 2020
Copy link
Collaborator

@lyakh lyakh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments are not compulsory for this PR, just optional things to consider.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't something introduced by this patch, but this code means, that in case of success this function returns 1, which isn't very common for a .probe() style function. I see that this function is only called from one location and there its return code is checked for < 0 but maybe it would be good to change this to return 0 on success.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I get your idea.

In snd_hdac_device_init pm_runtime_set_active is called to
increase child_count in parent device. But when it is failed
to build connection with GPU for one case that integrated
graphic gpu is disabled, snd_hdac_ext_bus_device_exit will be
invoked to clean up a HD-audio extended codec base device. At
this time the child_count of parent is not decreased, which
makes parent device can't get suspended.

This patch calls pm_runtime_set_suspended to decrease child_count
in parent device in snd_hdac_device_exit to match with
snd_hdac_device_init. pm_runtime_set_suspended can make sure that
it will not decrease child_count if the device is already suspended.

Signed-off-by: Rander Wang <rander.wang@intel.com>
@RanderWang RanderWang dismissed stale reviews from lyakh and kv2019i via 7ba9c02 July 21, 2020 08:28
When snd_hdac_device_init is failed, the codec is released by kfree
immediately without releasing some resources. The vendor_name should
be free if the memory is allocated and the runtime pm status should
be restored, especially the runtime pm status of parent device.

Signed-off-by: Rander Wang <rander.wang@intel.com>
When hda_codec_probe() doesn't initialize audio component, we disable
the codec and keep going. However,the resources are not released. The
child_count of SOF device is increased in snd_hdac_ext_bus_device_init
but is not decrease in error case, so SOF can't get suspended.

snd_hdac_ext_bus_device_exit will be invoked in HDA framework if it
gets a error. Now copy this behavior to release resources and decrease
SOF device child_count to release SOF device.

Signed-off-by: Rander Wang <rander.wang@intel.com>
@RanderWang
Copy link
Author

update PR, thanks for review!

@plbossart plbossart requested review from bardliao and lyakh July 21, 2020 17:22
@lyakh
Copy link
Collaborator

lyakh commented Jul 22, 2020

btw, what's that CI "failure?" I don't see any on the "details" page

@RanderWang
Copy link
Author

btw, what's that CI "failure?" I don't see any on the "details" page

I only found some tests are skip. @aiChaoSONG SKIP items will lead to CI failure ?

SKIP | SKIP | SKIP | SKIP

@aiChaoSONG
Copy link

@lyakh @RanderWang BYT/BDW don't support 'runtime-pm' case, so it's skipped.

@plbossart plbossart merged commit 94b043e into thesofproject:topic/sof-dev Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug][RKL] Runtime PM keeps active when in idle

7 participants