| Message ID | 1598505643-30347-1-git-send-email-harshapriya.n@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ALSA: Realtek: Early Forbid of runtime PM | expand |
On Thu, 27 Aug 2020 07:20:43 +0200, Harsha Priya wrote: > > For Realtek codecs, pm_runtime_forbid() is called in the probe function > after the build_controls(). In a stress test, occasionally the runtime > PM calls are invoked before controls are built. This causes the codec to be > runtime suspended before probe completes. Because of this, not all > controls are enumerated correctly and audio does not work until > system is rebooted. > > This patch calls pm_runtime_forbid() early to fix the issue. > Multiple stress tests of 2000+ cycles has been done to test the fix. > > Signed-off-by: Harsha Priya <harshapriya.n@intel.com> > Signed-off-by: Emmanuel Jillela <emmanuel.jillela@intel.com> > Reviewed-by: Kailang Yang <kailang@realtek.com> The behavior shouldn't be specific to that model, also not to codec vendors, but it's rather a generic problem, so it's no right place to correct, I suppose. Can we simply call pm_runtime_forbid() at creating a codec object like below? thanks, Takashi --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -1000,6 +1000,9 @@ int snd_hda_codec_device_new(struct hda_bus *bus, struct snd_card *card, if (err < 0) goto error; + /* PM runtime needs to be enabled later after binding codec */ + pm_runtime_forbid(&codec->core.dev); + return 0; error:
> > > For Realtek codecs, pm_runtime_forbid() is called in the probe > > function after the build_controls(). In a stress test, occasionally > > the runtime PM calls are invoked before controls are built. This > > causes the codec to be runtime suspended before probe completes. > > Because of this, not all controls are enumerated correctly and audio > > does not work until system is rebooted. > > > > This patch calls pm_runtime_forbid() early to fix the issue. > > Multiple stress tests of 2000+ cycles has been done to test the fix. > > > > Signed-off-by: Harsha Priya <harshapriya.n@intel.com> > > Signed-off-by: Emmanuel Jillela <emmanuel.jillela@intel.com> > > Reviewed-by: Kailang Yang <kailang@realtek.com> > > The behavior shouldn't be specific to that model, also not to codec vendors, but > it's rather a generic problem, so it's no right place to correct, I suppose. > > Can we simply call pm_runtime_forbid() at creating a codec object like below? We tried this change and its working as expected. I believe this will address similar issues across all codecs. Will send a patch with this fix. > > > thanks, > > Takashi > > --- a/sound/pci/hda/hda_codec.c > +++ b/sound/pci/hda/hda_codec.c > @@ -1000,6 +1000,9 @@ int snd_hda_codec_device_new(struct hda_bus *bus, > struct snd_card *card, > if (err < 0) > goto error; > > + /* PM runtime needs to be enabled later after binding codec */ > + pm_runtime_forbid(&codec->core.dev); > + > return 0; > > error:
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 98789691a479..30ea07fd6735 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -8418,6 +8418,7 @@ static int patch_alc269(struct hda_codec *codec) { struct alc_spec *spec; int err; + struct device *dev = hda_codec_dev(codec); err = alc_alloc_spec(codec, 0x0b); if (err < 0) @@ -8430,6 +8431,8 @@ static int patch_alc269(struct hda_codec *codec) #ifdef CONFIG_PM codec->patch_ops.suspend = alc269_suspend; codec->patch_ops.resume = alc269_resume; + pm_runtime_dont_use_autosuspend(dev); + pm_runtime_forbid(dev); #endif spec->shutup = alc_default_shutup; spec->init_hook = alc_default_init;