On Wed, 20 Jun 2018 11:34:52 +0200,
Lukas Wunner wrote:
>
> Hi Takashi,
>
> Henning Kühn reports that due to my commit 07f4f97d7b4b ("vga_switcheroo:
> Use device link for HDA controller"), the discrete GPU on his hybrid
> graphics laptop no longer runtime suspends.
>
> The root cause is that the single codec of the GPU's HDA controller
> doesn't support CLKSTOP and EPSS. (The "Supported Power States" are
> 0x00000009, i.e. CLKSTOP and EPSS bits are not set, cf. page 209 of
> the HDA spec.)
>
> This means that in hda_codec_runtime_suspend() we do not call
> snd_hdac_codec_link_down():
>
> if (codec_has_clkstop(codec) && codec_has_epss(codec) &&
> (state & AC_PWRST_CLK_STOP_OK))
> snd_hdac_codec_link_down(&codec->core);
>
> If snd_hdac_codec_link_down() isn't called, the bit in the codec_powered
> bitmask isn't cleared, which in turn prevents the controller from going
> to PCI_D3hot in azx_runtime_idle():
>
> if (!power_save_controller || !azx_has_pm_runtime(chip) ||
> azx_bus(chip)->codec_powered || !chip->running)
> return -EBUSY;
>
> The codec does runtime suspend to D3, but the PS-ClkStopOk bit in the
> response to "Get Power State" is not set. (Response is 0x00000033,
> cf. page 151 of the HD Audio spec.) Hence the check above
> "state & AC_PWRST_CLK_STOP_OK" also results in "false".
>
> I'm not familiar enough with the intricacies of the HD Audio spec to
> fully comprehend the implications of missing EPSS and CLKSTOP support
> and to come up with a fix. We could quirk any HDA controller in a
> vga_switcheroo setup to ignore missing EPSS and CLKSTOP support, but
> would that be safe? E.g. the spec says that if the bus clock does stop,
> "a full reset shall be performed" when the clock is reenabled. Are we
> handling this correctly?
I guess it would work with a quirk. The EPSS and CLKSTOP checks are
just to assure the modern codec PM, and GPU is always exceptional.
Supposing that it's AMD GPU, does a fix like below work?
thanks,
Takashi
-- 8< --
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -2899,8 +2899,9 @@ static int hda_codec_runtime_suspend(struct device *dev)
list_for_each_entry(pcm, &codec->pcm_list_head, list)
snd_pcm_suspend_all(pcm->pcm);
state = hda_call_codec_suspend(codec);
- if (codec_has_clkstop(codec) && codec_has_epss(codec) &&
- (state & AC_PWRST_CLK_STOP_OK))
+ if (codec->link_down_at_suspend ||
+ (codec_has_clkstop(codec) && codec_has_epss(codec) &&
+ (state & AC_PWRST_CLK_STOP_OK)))
snd_hdac_codec_link_down(&codec->core);
snd_hdac_link_power(&codec->core, false);
return 0;
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 681c360f29f9..5b00c1eb857e 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -258,6 +258,7 @@ struct hda_codec {
unsigned int power_save_node:1; /* advanced PM for each widget */
unsigned int auto_runtime_pm:1; /* enable automatic codec runtime pm */
unsigned int force_pin_prefix:1; /* Add location prefix */
+ unsigned int link_down_at_suspend:1; /* force to link down at suspend */
#ifdef CONFIG_PM
unsigned long power_on_acct;
unsigned long power_off_acct;
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 8840daf9c6a3..98e1c411c56a 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -3741,6 +3741,11 @@ static int patch_atihdmi(struct hda_codec *codec)
spec->chmap.channels_max = max(spec->chmap.channels_max, 8u);
+ /* AMD GPUs have neither EPSS nor CLKSTOP bits, hence preventing
+ * the link-down as is. Tell the core to allow it.
+ */
+ codec->link_down_at_suspend = 1;
+
return 0;
}
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
