diff mbox series

ALSA: hda: Enable runtime PM when codec probe fails

Message ID 20201214060621.1102931-1-kai.heng.feng@canonical.com (mailing list archive)
State New, archived
Headers show
Series ALSA: hda: Enable runtime PM when codec probe fails | expand

Commit Message

Kai-Heng Feng Dec. 14, 2020, 6:06 a.m. UTC
When codec probe fails, it doesn't enable runtime suspend, and can
prevent graphics card from getting powered down:
[    4.280991] snd_hda_intel 0000:01:00.1: no codecs initialized

$ cat /sys/bus/pci/devices/0000:01:00.1/power/runtime_status
active

So enable runtime PM when codec probe fails, to let graphics card be
able to runtime suspend again.

Merge azx_probe_continue() into azx_probe() and just let probe fail for
this case could be a better approach. However that's a much bigger task
so let's settle with a quirk workaround.

BugLink: https://bugs.launchpad.net/bugs/1907212
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 sound/pci/hda/hda_intel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Takashi Iwai Dec. 14, 2020, 7:58 a.m. UTC | #1
On Mon, 14 Dec 2020 07:06:20 +0100,
Kai-Heng Feng wrote:
> 
> When codec probe fails, it doesn't enable runtime suspend, and can
> prevent graphics card from getting powered down:
> [    4.280991] snd_hda_intel 0000:01:00.1: no codecs initialized
> 
> $ cat /sys/bus/pci/devices/0000:01:00.1/power/runtime_status
> active
> 
> So enable runtime PM when codec probe fails, to let graphics card be
> able to runtime suspend again.

Well, the runtime status is also active if the driver isn't probed at
all.  In that sense, keeping the status active at the driver load
failure is rather consistent, IMO.  If the driver fails or unloaded,
it should restore the status as if it were beforehand.


thanks,

Takashi

> 
> Merge azx_probe_continue() into azx_probe() and just let probe fail for
> this case could be a better approach. However that's a much bigger task
> so let's settle with a quirk workaround.
> 
> BugLink: https://bugs.launchpad.net/bugs/1907212
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  sound/pci/hda/hda_intel.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index 6852668f1bcb..3fd920069268 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -2328,7 +2328,7 @@ static int azx_probe_continue(struct azx *chip)
>  	if (bus->codec_mask) {
>  		err = azx_probe_codecs(chip, azx_max_codecs[chip->driver_type]);
>  		if (err < 0)
> -			goto out_free;
> +			goto out_enable_rpm;
>  	}
>  
>  #ifdef CONFIG_SND_HDA_PATCH_LOADER
> @@ -2360,6 +2360,7 @@ static int azx_probe_continue(struct azx *chip)
>  
>  	set_default_power_save(chip);
>  
> +out_enable_rpm:
>  	if (azx_has_pm_runtime(chip)) {
>  		pm_runtime_use_autosuspend(&pci->dev);
>  		pm_runtime_allow(&pci->dev);
> -- 
> 2.29.2
>
Kai-Heng Feng Dec. 14, 2020, 12:12 p.m. UTC | #2
On Mon, Dec 14, 2020 at 08:58, Takashi Iwai <tiwai@suse.de> wrote:
> On Mon, 14 Dec 2020 07:06:20 +0100,
> Kai-Heng Feng wrote:
>> 
>>  When codec probe fails, it doesn't enable runtime suspend, and can
>>  prevent graphics card from getting powered down:
>>  [    4.280991] snd_hda_intel 0000:01:00.1: no codecs initialized
>> 
>>  $ cat /sys/bus/pci/devices/0000:01:00.1/power/runtime_status
>>  active
>> 
>>  So enable runtime PM when codec probe fails, to let graphics card be
>>  able to runtime suspend again.
> 
> Well, the runtime status is also active if the driver isn't probed at
> all.  In that sense, keeping the status active at the driver load
> failure is rather consistent, IMO.  If the driver fails or unloaded,
> it should restore the status as if it were beforehand.

Ok, let me find a better way to handle it.

Kai-Heng

> 
> 
> thanks,
> 
> Takashi
> 
>> 
>>  Merge azx_probe_continue() into azx_probe() and just let probe fail 
>> for
>>  this case could be a better approach. However that's a much bigger 
>> task
>>  so let's settle with a quirk workaround.
>> 
>>  BugLink: <https://bugs.launchpad.net/bugs/1907212>
>>  Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com 
>> <mailto:kai.heng.feng@canonical.com>>
>>  ---
>>   sound/pci/hda/hda_intel.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>>  diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
>>  index 6852668f1bcb..3fd920069268 100644
>>  --- a/sound/pci/hda/hda_intel.c
>>  +++ b/sound/pci/hda/hda_intel.c
>>  @@ -2328,7 +2328,7 @@ static int azx_probe_continue(struct azx 
>> *chip)
>>   	if (bus->codec_mask) {
>>   		err = azx_probe_codecs(chip, azx_max_codecs[chip->driver_type]);
>>   		if (err < 0)
>>  -			goto out_free;
>>  +			goto out_enable_rpm;
>>   	}
>> 
>>   #ifdef CONFIG_SND_HDA_PATCH_LOADER
>>  @@ -2360,6 +2360,7 @@ static int azx_probe_continue(struct azx 
>> *chip)
>> 
>>   	set_default_power_save(chip);
>> 
>>  +out_enable_rpm:
>>   	if (azx_has_pm_runtime(chip)) {
>>   		pm_runtime_use_autosuspend(&pci->dev);
>>   		pm_runtime_allow(&pci->dev);
>>  --
>>  2.29.2
>>
diff mbox series

Patch

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 6852668f1bcb..3fd920069268 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2328,7 +2328,7 @@  static int azx_probe_continue(struct azx *chip)
 	if (bus->codec_mask) {
 		err = azx_probe_codecs(chip, azx_max_codecs[chip->driver_type]);
 		if (err < 0)
-			goto out_free;
+			goto out_enable_rpm;
 	}
 
 #ifdef CONFIG_SND_HDA_PATCH_LOADER
@@ -2360,6 +2360,7 @@  static int azx_probe_continue(struct azx *chip)
 
 	set_default_power_save(chip);
 
+out_enable_rpm:
 	if (azx_has_pm_runtime(chip)) {
 		pm_runtime_use_autosuspend(&pci->dev);
 		pm_runtime_allow(&pci->dev);