diff mbox

[v2] ALSA: hda - Fix Skylake codec timeout

Message ID 1437035964-5658-1-git-send-email-david.henningsson@canonical.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Henningsson July 16, 2015, 8:39 a.m. UTC
When the controller is powered up but the HDMI codec is powered down
on Skylake, the power well is turned off. When the codec is then
powered up again, we need to poke the codec a little extra to make
sure it wakes up. Otherwise we'll get sad "no response from codec"
messages and broken audio.

This also changes azx_runtime_resume to actually call
snd_hdac_set_codec_wakeup for Skylake (before STATETS read).
(Otherwise it would only have been called for Haswell and Broadwell,
which both do not need it, so this probably was not the author's
intention.)

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/hda/hdac_i915.c     |  5 ++++-
 sound/pci/hda/hda_intel.c | 18 ++++++++++--------
 2 files changed, 14 insertions(+), 9 deletions(-)

Comments

Yang, Libin July 16, 2015, 3:14 p.m. UTC | #1
> -----Original Message-----
> From: David Henningsson [mailto:david.henningsson@canonical.com]
> Sent: Thursday, July 16, 2015 4:39 PM
> To: tiwai@suse.de; hui.wang@canonical.com; alsa-devel@alsa-
> project.org; Yang, Libin; Lin, Mengdong
> Cc: David Henningsson
> Subject: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
> 
> When the controller is powered up but the HDMI codec is powered
> down
> on Skylake, the power well is turned off. When the codec is then
> powered up again, we need to poke the codec a little extra to make
> sure it wakes up. Otherwise we'll get sad "no response from codec"
> messages and broken audio.

Thanks for finding this issue.

Could you please give us you test case? We didn't meet such issue
before. I would like do a full test on it.

> 
> This also changes azx_runtime_resume to actually call
> snd_hdac_set_codec_wakeup for Skylake (before STATETS read).
> (Otherwise it would only have been called for Haswell and Broadwell,
> which both do not need it, so this probably was not the author's
> intention.)
> 
> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> ---
>  sound/hda/hdac_i915.c     |  5 ++++-
>  sound/pci/hda/hda_intel.c | 18 ++++++++++--------
>  2 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
> index 442500e..5676b84 100644
> --- a/sound/hda/hdac_i915.c
> +++ b/sound/hda/hdac_i915.c
> @@ -56,8 +56,11 @@ int snd_hdac_display_power(struct hdac_bus
> *bus, bool enable)
>  		enable ? "enable" : "disable");
> 
>  	if (enable) {
> -		if (!bus->i915_power_refcount++)
> +		if (!bus->i915_power_refcount++) {
>  			acomp->ops->get_power(acomp->dev);
> +			snd_hdac_set_codec_wakeup(bus, true);
> +			snd_hdac_set_codec_wakeup(bus, false);

Mostly we have called snd_hdac_set_codec_wakeup() after calling
Snd_hdac_display_power(true). It seems we missed it in the 
link_power().

I agree moving snd_hdac_set_codec_wakeup() to here is better.
I would like do a full test on this patch.

> +		}
>  	} else {
>  		WARN_ON(!bus->i915_power_refcount);
>  		if (!--bus->i915_power_refcount)
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index ca151b4..9962237 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -979,14 +979,16 @@ static int azx_runtime_resume(struct device
> *dev)
>  	if (!azx_has_pm_runtime(chip))
>  		return 0;
> 
> -	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL
> -		&& hda->need_i915_power) {
> -		bus =  azx_bus(chip);
> -		snd_hdac_display_power(bus, true);
> -		haswell_set_bclk(hda);
> -		/* toggle codec wakeup bit for STATESTS read */
> -		snd_hdac_set_codec_wakeup(bus, true);
> -		snd_hdac_set_codec_wakeup(bus, false);
> +	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
> +		bus = azx_bus(chip);
> +		if (hda->need_i915_power) {
> +			snd_hdac_display_power(bus, true);
> +			haswell_set_bclk(hda);
> +		} else {
> +			/* toggle codec wakeup bit for STATESTS read
> */
> +			snd_hdac_set_codec_wakeup(bus, true);
> +			snd_hdac_set_codec_wakeup(bus, false);
> +		}
>  	}
> 
>  	/* Read STATESTS before controller reset */
> --
> 1.9.1
David Henningsson July 16, 2015, 3:31 p.m. UTC | #2
On 2015-07-16 17:14, Yang, Libin wrote:
>> -----Original Message-----
>> From: David Henningsson [mailto:david.henningsson@canonical.com]
>> Sent: Thursday, July 16, 2015 4:39 PM
>> To: tiwai@suse.de; hui.wang@canonical.com; alsa-devel@alsa-
>> project.org; Yang, Libin; Lin, Mengdong
>> Cc: David Henningsson
>> Subject: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
>>
>> When the controller is powered up but the HDMI codec is powered
>> down
>> on Skylake, the power well is turned off. When the codec is then
>> powered up again, we need to poke the codec a little extra to make
>> sure it wakes up. Otherwise we'll get sad "no response from codec"
>> messages and broken audio.
>
> Thanks for finding this issue.
>
> Could you please give us you test case? We didn't meet such issue
> before. I would like do a full test on it.

The test case is:

  - Install Ubuntu 15.04.
  - Upgrade to kernel 4.1.
  - Boot the new kernel.

Note: I believe Hui tested with 4.1 and got this error. In my case I 
have been running a hybrid kernel: Ubuntu 15.04, kernel 3.19.0-23.24 
(which has a backported i915 driver), and then the alsa-daily DKMS on 
top of that.

I can email you off-list with details about the machine I tested it on, 
if you like.
Takashi Iwai July 16, 2015, 3:34 p.m. UTC | #3
On Thu, 16 Jul 2015 17:31:33 +0200,
David Henningsson wrote:
> 
> 
> 
> On 2015-07-16 17:14, Yang, Libin wrote:
> >> -----Original Message-----
> >> From: David Henningsson [mailto:david.henningsson@canonical.com]
> >> Sent: Thursday, July 16, 2015 4:39 PM
> >> To: tiwai@suse.de; hui.wang@canonical.com; alsa-devel@alsa-
> >> project.org; Yang, Libin; Lin, Mengdong
> >> Cc: David Henningsson
> >> Subject: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
> >>
> >> When the controller is powered up but the HDMI codec is powered
> >> down
> >> on Skylake, the power well is turned off. When the codec is then
> >> powered up again, we need to poke the codec a little extra to make
> >> sure it wakes up. Otherwise we'll get sad "no response from codec"
> >> messages and broken audio.
> >
> > Thanks for finding this issue.
> >
> > Could you please give us you test case? We didn't meet such issue
> > before. I would like do a full test on it.
> 
> The test case is:
> 
>   - Install Ubuntu 15.04.
>   - Upgrade to kernel 4.1.
>   - Boot the new kernel.
> 
> Note: I believe Hui tested with 4.1 and got this error. In my case I 
> have been running a hybrid kernel: Ubuntu 15.04, kernel 3.19.0-23.24 
> (which has a backported i915 driver), and then the alsa-daily DKMS on 
> top of that.
> 
> I can email you off-list with details about the machine I tested it on, 
> if you like.

Did you hotplug the device or is it booted while plugged?
I had an unstable communication at hotplug / unplug on SKL at the last
time, too.  But the boot with the DP plugged worked well even on an
old SKL box.


Takashi
David Henningsson July 16, 2015, 3:37 p.m. UTC | #4
On 2015-07-16 17:34, Takashi Iwai wrote:
> On Thu, 16 Jul 2015 17:31:33 +0200,
> David Henningsson wrote:
>>
>>
>>
>> On 2015-07-16 17:14, Yang, Libin wrote:
>>>> -----Original Message-----
>>>> From: David Henningsson [mailto:david.henningsson@canonical.com]
>>>> Sent: Thursday, July 16, 2015 4:39 PM
>>>> To: tiwai@suse.de; hui.wang@canonical.com; alsa-devel@alsa-
>>>> project.org; Yang, Libin; Lin, Mengdong
>>>> Cc: David Henningsson
>>>> Subject: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
>>>>
>>>> When the controller is powered up but the HDMI codec is powered
>>>> down
>>>> on Skylake, the power well is turned off. When the codec is then
>>>> powered up again, we need to poke the codec a little extra to make
>>>> sure it wakes up. Otherwise we'll get sad "no response from codec"
>>>> messages and broken audio.
>>>
>>> Thanks for finding this issue.
>>>
>>> Could you please give us you test case? We didn't meet such issue
>>> before. I would like do a full test on it.
>>
>> The test case is:
>>
>>    - Install Ubuntu 15.04.
>>    - Upgrade to kernel 4.1.
>>    - Boot the new kernel.
>>
>> Note: I believe Hui tested with 4.1 and got this error. In my case I
>> have been running a hybrid kernel: Ubuntu 15.04, kernel 3.19.0-23.24
>> (which has a backported i915 driver), and then the alsa-daily DKMS on
>> top of that.
>>
>> I can email you off-list with details about the machine I tested it on,
>> if you like.
>
> Did you hotplug the device or is it booted while plugged?
> I had an unstable communication at hotplug / unplug on SKL at the last
> time, too.  But the boot with the DP plugged worked well even on an
> old SKL box.

For this bug, there was no HDMI monitor connected at all, but I believe 
it does not matter. On power up, the power well is enabled and then 
disabled. When the power well is enabled again without this extra wakeup 
poke (probably due to PulseAudio probing), this error happens.
Takashi Iwai July 16, 2015, 3:53 p.m. UTC | #5
On Thu, 16 Jul 2015 17:37:41 +0200,
David Henningsson wrote:
> 
> 
> 
> On 2015-07-16 17:34, Takashi Iwai wrote:
> > On Thu, 16 Jul 2015 17:31:33 +0200,
> > David Henningsson wrote:
> >>
> >>
> >>
> >> On 2015-07-16 17:14, Yang, Libin wrote:
> >>>> -----Original Message-----
> >>>> From: David Henningsson [mailto:david.henningsson@canonical.com]
> >>>> Sent: Thursday, July 16, 2015 4:39 PM
> >>>> To: tiwai@suse.de; hui.wang@canonical.com; alsa-devel@alsa-
> >>>> project.org; Yang, Libin; Lin, Mengdong
> >>>> Cc: David Henningsson
> >>>> Subject: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
> >>>>
> >>>> When the controller is powered up but the HDMI codec is powered
> >>>> down
> >>>> on Skylake, the power well is turned off. When the codec is then
> >>>> powered up again, we need to poke the codec a little extra to make
> >>>> sure it wakes up. Otherwise we'll get sad "no response from codec"
> >>>> messages and broken audio.
> >>>
> >>> Thanks for finding this issue.
> >>>
> >>> Could you please give us you test case? We didn't meet such issue
> >>> before. I would like do a full test on it.
> >>
> >> The test case is:
> >>
> >>    - Install Ubuntu 15.04.
> >>    - Upgrade to kernel 4.1.
> >>    - Boot the new kernel.
> >>
> >> Note: I believe Hui tested with 4.1 and got this error. In my case I
> >> have been running a hybrid kernel: Ubuntu 15.04, kernel 3.19.0-23.24
> >> (which has a backported i915 driver), and then the alsa-daily DKMS on
> >> top of that.
> >>
> >> I can email you off-list with details about the machine I tested it on,
> >> if you like.
> >
> > Did you hotplug the device or is it booted while plugged?
> > I had an unstable communication at hotplug / unplug on SKL at the last
> > time, too.  But the boot with the DP plugged worked well even on an
> > old SKL box.
> 
> For this bug, there was no HDMI monitor connected at all, but I believe 
> it does not matter. On power up, the power well is enabled and then 
> disabled. When the power well is enabled again without this extra wakeup 
> poke (probably due to PulseAudio probing), this error happens.

Ah, that's interesting.  So this happens when no monitor is
connected.  I haven't seen this, though; maybe depends on the board or
BIOS.


Takashi
Hui Wang July 17, 2015, 12:57 a.m. UTC | #6
On 07/16/2015 11:31 PM, David Henningsson wrote:
>
>
> On 2015-07-16 17:14, Yang, Libin wrote:
>>> -----Original Message-----
>>> From: David Henningsson [mailto:david.henningsson@canonical.com]
>>> Sent: Thursday, July 16, 2015 4:39 PM
>>> To: tiwai@suse.de; hui.wang@canonical.com; alsa-devel@alsa-
>>> project.org; Yang, Libin; Lin, Mengdong
>>> Cc: David Henningsson
>>> Subject: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
>>>
>>> When the controller is powered up but the HDMI codec is powered
>>> down
>>> on Skylake, the power well is turned off. When the codec is then
>>> powered up again, we need to poke the codec a little extra to make
>>> sure it wakes up. Otherwise we'll get sad "no response from codec"
>>> messages and broken audio.
>>
>> Thanks for finding this issue.
>>
>> Could you please give us you test case? We didn't meet such issue
>> before. I would like do a full test on it.
>
> The test case is:
>
>  - Install Ubuntu 15.04.
>  - Upgrade to kernel 4.1.
>  - Boot the new kernel.
>
> Note: I believe Hui tested with 4.1 and got this error. In my case I 
> have been running a hybrid kernel: Ubuntu 15.04, kernel 3.19.0-23.24 
> (which has a backported i915 driver), and then the alsa-daily DKMS on 
> top of that.
>
> I can email you off-list with details about the machine I tested it 
> on, if you like.
>

Here is a dmesg log, it is generated on a skl laptop with 
drm-intel-nightly kernel (v4.2), searching the keyword "timeout" in the 
text, you will find lots of timeout errors.

http://paste.ubuntu.com/11890651/
Yang, Libin July 17, 2015, 2:28 a.m. UTC | #7
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Thursday, July 16, 2015 11:54 PM
> To: David Henningsson
> Cc: Yang, Libin; hui.wang@canonical.com; alsa-devel@alsa-project.org;
> Lin, Mengdong; Lu, Han
> Subject: Re: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
> 
> On Thu, 16 Jul 2015 17:37:41 +0200,
> David Henningsson wrote:
> >
> >
> >
> > On 2015-07-16 17:34, Takashi Iwai wrote:
> > > On Thu, 16 Jul 2015 17:31:33 +0200,
> > > David Henningsson wrote:
> > >>
> > >>
> > >>
> > >> On 2015-07-16 17:14, Yang, Libin wrote:
> > >>>> -----Original Message-----
> > >>>> From: David Henningsson
> [mailto:david.henningsson@canonical.com]
> > >>>> Sent: Thursday, July 16, 2015 4:39 PM
> > >>>> To: tiwai@suse.de; hui.wang@canonical.com; alsa-devel@alsa-
> > >>>> project.org; Yang, Libin; Lin, Mengdong
> > >>>> Cc: David Henningsson
> > >>>> Subject: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
> > >>>>
> > >>>> When the controller is powered up but the HDMI codec is
> powered
> > >>>> down
> > >>>> on Skylake, the power well is turned off. When the codec is
> then
> > >>>> powered up again, we need to poke the codec a little extra to
> make
> > >>>> sure it wakes up. Otherwise we'll get sad "no response from
> codec"
> > >>>> messages and broken audio.
> > >>>
> > >>> Thanks for finding this issue.
> > >>>
> > >>> Could you please give us you test case? We didn't meet such
> issue
> > >>> before. I would like do a full test on it.
> > >>
> > >> The test case is:
> > >>
> > >>    - Install Ubuntu 15.04.
> > >>    - Upgrade to kernel 4.1.
> > >>    - Boot the new kernel.
> > >>
> > >> Note: I believe Hui tested with 4.1 and got this error. In my case I
> > >> have been running a hybrid kernel: Ubuntu 15.04, kernel 3.19.0-
> 23.24
> > >> (which has a backported i915 driver), and then the alsa-daily
> DKMS on
> > >> top of that.
> > >>
> > >> I can email you off-list with details about the machine I tested it
> on,
> > >> if you like.
> > >
> > > Did you hotplug the device or is it booted while plugged?
> > > I had an unstable communication at hotplug / unplug on SKL at the
> last
> > > time, too.  But the boot with the DP plugged worked well even on
> an
> > > old SKL box.
> >
> > For this bug, there was no HDMI monitor connected at all, but I
> believe
> > it does not matter. On power up, the power well is enabled and then
> > disabled. When the power well is enabled again without this extra
> wakeup
> > poke (probably due to PulseAudio probing), this error happens.
> 
> Ah, that's interesting.  So this happens when no monitor is
> connected.  I haven't seen this, though; maybe depends on the board
> or
> BIOS.

On SKL, each time turning on power well 2, display codec need
extra wakeup poke.

If there is monitor connected, power well will not be turned
off even audio driver asks to.

If there is monitor connected, power well will be really
turned off. In this case, when hdmi audio is used, we 
must do the wakeup poke.

> 
> 
> Takashi
Takashi Iwai July 17, 2015, 1:01 p.m. UTC | #8
On Fri, 17 Jul 2015 04:28:10 +0200,
Yang, Libin wrote:
> 
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Thursday, July 16, 2015 11:54 PM
> > To: David Henningsson
> > Cc: Yang, Libin; hui.wang@canonical.com; alsa-devel@alsa-project.org;
> > Lin, Mengdong; Lu, Han
> > Subject: Re: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
> > 
> > On Thu, 16 Jul 2015 17:37:41 +0200,
> > David Henningsson wrote:
> > >
> > >
> > >
> > > On 2015-07-16 17:34, Takashi Iwai wrote:
> > > > On Thu, 16 Jul 2015 17:31:33 +0200,
> > > > David Henningsson wrote:
> > > >>
> > > >>
> > > >>
> > > >> On 2015-07-16 17:14, Yang, Libin wrote:
> > > >>>> -----Original Message-----
> > > >>>> From: David Henningsson
> > [mailto:david.henningsson@canonical.com]
> > > >>>> Sent: Thursday, July 16, 2015 4:39 PM
> > > >>>> To: tiwai@suse.de; hui.wang@canonical.com; alsa-devel@alsa-
> > > >>>> project.org; Yang, Libin; Lin, Mengdong
> > > >>>> Cc: David Henningsson
> > > >>>> Subject: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
> > > >>>>
> > > >>>> When the controller is powered up but the HDMI codec is
> > powered
> > > >>>> down
> > > >>>> on Skylake, the power well is turned off. When the codec is
> > then
> > > >>>> powered up again, we need to poke the codec a little extra to
> > make
> > > >>>> sure it wakes up. Otherwise we'll get sad "no response from
> > codec"
> > > >>>> messages and broken audio.
> > > >>>
> > > >>> Thanks for finding this issue.
> > > >>>
> > > >>> Could you please give us you test case? We didn't meet such
> > issue
> > > >>> before. I would like do a full test on it.
> > > >>
> > > >> The test case is:
> > > >>
> > > >>    - Install Ubuntu 15.04.
> > > >>    - Upgrade to kernel 4.1.
> > > >>    - Boot the new kernel.
> > > >>
> > > >> Note: I believe Hui tested with 4.1 and got this error. In my case I
> > > >> have been running a hybrid kernel: Ubuntu 15.04, kernel 3.19.0-
> > 23.24
> > > >> (which has a backported i915 driver), and then the alsa-daily
> > DKMS on
> > > >> top of that.
> > > >>
> > > >> I can email you off-list with details about the machine I tested it
> > on,
> > > >> if you like.
> > > >
> > > > Did you hotplug the device or is it booted while plugged?
> > > > I had an unstable communication at hotplug / unplug on SKL at the
> > last
> > > > time, too.  But the boot with the DP plugged worked well even on
> > an
> > > > old SKL box.
> > >
> > > For this bug, there was no HDMI monitor connected at all, but I
> > believe
> > > it does not matter. On power up, the power well is enabled and then
> > > disabled. When the power well is enabled again without this extra
> > wakeup
> > > poke (probably due to PulseAudio probing), this error happens.
> > 
> > Ah, that's interesting.  So this happens when no monitor is
> > connected.  I haven't seen this, though; maybe depends on the board
> > or
> > BIOS.
> 
> On SKL, each time turning on power well 2, display codec need
> extra wakeup poke.
> 
> If there is monitor connected, power well will not be turned
> off even audio driver asks to.
> 
> If there is monitor connected, power well will be really
> turned off. In this case, when hdmi audio is used, we 
> must do the wakeup poke.

So the patch does the right thing?  I'm currently waiting for an ack
from you Intel people as this is highly specific to the chipset
detail.  Please give reviewed-by, acked-by or whatever if the patch is
OK.


thanks,

Takashi
Yang, Libin July 20, 2015, 7:14 a.m. UTC | #9
Hi Takashi,

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Friday, July 17, 2015 9:02 PM
> To: Yang, Libin
> Cc: David Henningsson; hui.wang@canonical.com; alsa-devel@alsa-
> project.org; Lin, Mengdong; Lu, Han
> Subject: Re: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
> 
> On Fri, 17 Jul 2015 04:28:10 +0200,
> Yang, Libin wrote:
> >
> >
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Thursday, July 16, 2015 11:54 PM
> > > To: David Henningsson
> > > Cc: Yang, Libin; hui.wang@canonical.com; alsa-devel@alsa-
> project.org;
> > > Lin, Mengdong; Lu, Han
> > > Subject: Re: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
> > >
> > > On Thu, 16 Jul 2015 17:37:41 +0200,
> > > David Henningsson wrote:
> > > >
> > > >
> > > >
> > > > On 2015-07-16 17:34, Takashi Iwai wrote:
> > > > > On Thu, 16 Jul 2015 17:31:33 +0200,
> > > > > David Henningsson wrote:
> > > > >>
> > > > >>
> > > > >>
> > > > >> On 2015-07-16 17:14, Yang, Libin wrote:
> > > > >>>> -----Original Message-----
> > > > >>>> From: David Henningsson
> > > [mailto:david.henningsson@canonical.com]
> > > > >>>> Sent: Thursday, July 16, 2015 4:39 PM
> > > > >>>> To: tiwai@suse.de; hui.wang@canonical.com; alsa-
> devel@alsa-
> > > > >>>> project.org; Yang, Libin; Lin, Mengdong
> > > > >>>> Cc: David Henningsson
> > > > >>>> Subject: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
> > > > >>>>
> > > > >>>> When the controller is powered up but the HDMI codec is
> > > powered
> > > > >>>> down
> > > > >>>> on Skylake, the power well is turned off. When the codec is
> > > then
> > > > >>>> powered up again, we need to poke the codec a little extra
> to
> > > make
> > > > >>>> sure it wakes up. Otherwise we'll get sad "no response
> from
> > > codec"
> > > > >>>> messages and broken audio.
> > > > >>>
> > > > >>> Thanks for finding this issue.
> > > > >>>
> > > > >>> Could you please give us you test case? We didn't meet such
> > > issue
> > > > >>> before. I would like do a full test on it.
> > > > >>
> > > > >> The test case is:
> > > > >>
> > > > >>    - Install Ubuntu 15.04.
> > > > >>    - Upgrade to kernel 4.1.
> > > > >>    - Boot the new kernel.
> > > > >>
> > > > >> Note: I believe Hui tested with 4.1 and got this error. In my
> case I
> > > > >> have been running a hybrid kernel: Ubuntu 15.04, kernel
> 3.19.0-
> > > 23.24
> > > > >> (which has a backported i915 driver), and then the alsa-daily
> > > DKMS on
> > > > >> top of that.
> > > > >>
> > > > >> I can email you off-list with details about the machine I tested
> it
> > > on,
> > > > >> if you like.
> > > > >
> > > > > Did you hotplug the device or is it booted while plugged?
> > > > > I had an unstable communication at hotplug / unplug on SKL at
> the
> > > last
> > > > > time, too.  But the boot with the DP plugged worked well even
> on
> > > an
> > > > > old SKL box.
> > > >
> > > > For this bug, there was no HDMI monitor connected at all, but I
> > > believe
> > > > it does not matter. On power up, the power well is enabled and
> then
> > > > disabled. When the power well is enabled again without this
> extra
> > > wakeup
> > > > poke (probably due to PulseAudio probing), this error happens.
> > >
> > > Ah, that's interesting.  So this happens when no monitor is
> > > connected.  I haven't seen this, though; maybe depends on the
> board
> > > or
> > > BIOS.
> >
> > On SKL, each time turning on power well 2, display codec need
> > extra wakeup poke.
> >
> > If there is monitor connected, power well will not be turned
> > off even audio driver asks to.
> >
> > If there is monitor connected, power well will be really
> > turned off. In this case, when hdmi audio is used, we
> > must do the wakeup poke.
> 
> So the patch does the right thing?  I'm currently waiting for an ack
> from you Intel people as this is highly specific to the chipset
> detail.  Please give reviewed-by, acked-by or whatever if the patch is
> OK.

The patch is right. I have reviewed the patch and Our QA has verified it.

> 
> 
> thanks,
> 
> Takashi
Takashi Iwai July 20, 2015, 7:34 a.m. UTC | #10
On Mon, 20 Jul 2015 09:14:15 +0200,
Yang, Libin wrote:
> 
> Hi Takashi,
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Friday, July 17, 2015 9:02 PM
> > To: Yang, Libin
> > Cc: David Henningsson; hui.wang@canonical.com; alsa-devel@alsa-
> > project.org; Lin, Mengdong; Lu, Han
> > Subject: Re: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
> > 
> > On Fri, 17 Jul 2015 04:28:10 +0200,
> > Yang, Libin wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > Sent: Thursday, July 16, 2015 11:54 PM
> > > > To: David Henningsson
> > > > Cc: Yang, Libin; hui.wang@canonical.com; alsa-devel@alsa-
> > project.org;
> > > > Lin, Mengdong; Lu, Han
> > > > Subject: Re: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
> > > >
> > > > On Thu, 16 Jul 2015 17:37:41 +0200,
> > > > David Henningsson wrote:
> > > > >
> > > > >
> > > > >
> > > > > On 2015-07-16 17:34, Takashi Iwai wrote:
> > > > > > On Thu, 16 Jul 2015 17:31:33 +0200,
> > > > > > David Henningsson wrote:
> > > > > >>
> > > > > >>
> > > > > >>
> > > > > >> On 2015-07-16 17:14, Yang, Libin wrote:
> > > > > >>>> -----Original Message-----
> > > > > >>>> From: David Henningsson
> > > > [mailto:david.henningsson@canonical.com]
> > > > > >>>> Sent: Thursday, July 16, 2015 4:39 PM
> > > > > >>>> To: tiwai@suse.de; hui.wang@canonical.com; alsa-
> > devel@alsa-
> > > > > >>>> project.org; Yang, Libin; Lin, Mengdong
> > > > > >>>> Cc: David Henningsson
> > > > > >>>> Subject: [PATCH v2] ALSA: hda - Fix Skylake codec timeout
> > > > > >>>>
> > > > > >>>> When the controller is powered up but the HDMI codec is
> > > > powered
> > > > > >>>> down
> > > > > >>>> on Skylake, the power well is turned off. When the codec is
> > > > then
> > > > > >>>> powered up again, we need to poke the codec a little extra
> > to
> > > > make
> > > > > >>>> sure it wakes up. Otherwise we'll get sad "no response
> > from
> > > > codec"
> > > > > >>>> messages and broken audio.
> > > > > >>>
> > > > > >>> Thanks for finding this issue.
> > > > > >>>
> > > > > >>> Could you please give us you test case? We didn't meet such
> > > > issue
> > > > > >>> before. I would like do a full test on it.
> > > > > >>
> > > > > >> The test case is:
> > > > > >>
> > > > > >>    - Install Ubuntu 15.04.
> > > > > >>    - Upgrade to kernel 4.1.
> > > > > >>    - Boot the new kernel.
> > > > > >>
> > > > > >> Note: I believe Hui tested with 4.1 and got this error. In my
> > case I
> > > > > >> have been running a hybrid kernel: Ubuntu 15.04, kernel
> > 3.19.0-
> > > > 23.24
> > > > > >> (which has a backported i915 driver), and then the alsa-daily
> > > > DKMS on
> > > > > >> top of that.
> > > > > >>
> > > > > >> I can email you off-list with details about the machine I tested
> > it
> > > > on,
> > > > > >> if you like.
> > > > > >
> > > > > > Did you hotplug the device or is it booted while plugged?
> > > > > > I had an unstable communication at hotplug / unplug on SKL at
> > the
> > > > last
> > > > > > time, too.  But the boot with the DP plugged worked well even
> > on
> > > > an
> > > > > > old SKL box.
> > > > >
> > > > > For this bug, there was no HDMI monitor connected at all, but I
> > > > believe
> > > > > it does not matter. On power up, the power well is enabled and
> > then
> > > > > disabled. When the power well is enabled again without this
> > extra
> > > > wakeup
> > > > > poke (probably due to PulseAudio probing), this error happens.
> > > >
> > > > Ah, that's interesting.  So this happens when no monitor is
> > > > connected.  I haven't seen this, though; maybe depends on the
> > board
> > > > or
> > > > BIOS.
> > >
> > > On SKL, each time turning on power well 2, display codec need
> > > extra wakeup poke.
> > >
> > > If there is monitor connected, power well will not be turned
> > > off even audio driver asks to.
> > >
> > > If there is monitor connected, power well will be really
> > > turned off. In this case, when hdmi audio is used, we
> > > must do the wakeup poke.
> > 
> > So the patch does the right thing?  I'm currently waiting for an ack
> > from you Intel people as this is highly specific to the chipset
> > detail.  Please give reviewed-by, acked-by or whatever if the patch is
> > OK.
> 
> The patch is right. I have reviewed the patch and Our QA has verified it.

OK, I queued the patch now.  Thanks.


Takashi
diff mbox

Patch

diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index 442500e..5676b84 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -56,8 +56,11 @@  int snd_hdac_display_power(struct hdac_bus *bus, bool enable)
 		enable ? "enable" : "disable");
 
 	if (enable) {
-		if (!bus->i915_power_refcount++)
+		if (!bus->i915_power_refcount++) {
 			acomp->ops->get_power(acomp->dev);
+			snd_hdac_set_codec_wakeup(bus, true);
+			snd_hdac_set_codec_wakeup(bus, false);
+		}
 	} else {
 		WARN_ON(!bus->i915_power_refcount);
 		if (!--bus->i915_power_refcount)
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index ca151b4..9962237 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -979,14 +979,16 @@  static int azx_runtime_resume(struct device *dev)
 	if (!azx_has_pm_runtime(chip))
 		return 0;
 
-	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL
-		&& hda->need_i915_power) {
-		bus =  azx_bus(chip);
-		snd_hdac_display_power(bus, true);
-		haswell_set_bclk(hda);
-		/* toggle codec wakeup bit for STATESTS read */
-		snd_hdac_set_codec_wakeup(bus, true);
-		snd_hdac_set_codec_wakeup(bus, false);
+	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
+		bus = azx_bus(chip);
+		if (hda->need_i915_power) {
+			snd_hdac_display_power(bus, true);
+			haswell_set_bclk(hda);
+		} else {
+			/* toggle codec wakeup bit for STATESTS read */
+			snd_hdac_set_codec_wakeup(bus, true);
+			snd_hdac_set_codec_wakeup(bus, false);
+		}
 	}
 
 	/* Read STATESTS before controller reset */