We test it on the BIOS V73.

On the old drm-intel-nightly(2014.12), it works well.
On the latest drm-intel-nightly, it failed.

Does v4.0-rc2 work? Can you bisect?

OK. We will try to do the bisect

This bug need C1 CPU, we returned the CPU to other groups. We will apply for the new CPU and do the Bisect.

We have test on the C1 CPU with the latest drm-intel-nightly. The bug still exists.

Bisect from drm-intel-next-queued kernel tree.

The first bad commit should be:
commit 94dd5138c5ed02d26982d9704e8c1e9d72e20b40
Author:     Satheeshakrishna M <satheeshakrishna.m@intel.com>
AuthorDate: Wed Feb 4 13:57:44 2015 +0000
Commit:     Daniel Vetter <daniel.vetter@ffwll.ch>
CommitDate: Fri Feb 13 23:28:01 2015 +0100

    drm/i915/skl: Implementation of SKL display power well support

    This patch implements core logic of SKL display power well.

    v2: Addressed Imre's comments
        - Added respective DDIs under power well #1 and #2
        - Simplified repetitive code in power well programming

    v3: Implemented Imre's comments
        - Further simplified power well programming
        - Made sure that PW 1 is enabled prior to PW 2

    v4: Fix minor conflict with the the cherryview support (Damien)

    v5: Add the PLL power domain to the always on power well (Damien)

    v6: Disable BIOS power well (Imre)
        Use power well data for comparison (Imre)
        Put the PLL power domain into PW1 as its needed for CDCLK (Satheesh,
        Damien)

    v7: Addressed Imre's comments
      - Lowered the time out to 1ms
      - Added parantheses in macro
      - Moved debug message and fixed wait_for interval

    v8:
      - Add a WARN() when swiching on an unknown power well (Imre, done by Damien)
      - Whitespace fixes (spaces instead of tabs) (Damien)

    v9: (Imre, done by Damien)
      - Merge the register definitions with this patch
      - Merge the MISC IO power well in this patch

    v10: (Imre, done by Damien)

      - Define the Misc I/O power domains to be the power well 1 ones as Misc I/O
        needs to be enabled with PW1
      - Added Transcoder A and VGA domains to PW 2
      - Remove the MISC_IO power domains as well in the the always on
        domains definition
      - Move Misc I/O power well at the top of the power well list so it's turned
        on right after PW1.

    Reviewed-by: Imre Deak <imre.deak@intel.com>
    Signed-off-by: Satheeshakrishna M <satheeshakrishna.m@intel.com> (v3,v6,v7)
    Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Could you try the following patch:

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 060f7a2..a821a7f 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -298,7 +298,8 @@ enum {
 	 AZX_DCAPS_SNOOP_TYPE(SCH))
 
 #define AZX_DCAPS_INTEL_SKYLAKE \
-	(AZX_DCAPS_INTEL_PCH | AZX_DCAPS_SEPARATE_STREAM_TAG)
+	(AZX_DCAPS_INTEL_PCH | AZX_DCAPS_SEPARATE_STREAM_TAG |\
+	 AZX_DCAPS_I915_POWERWELL)
 
 /* quirks for ATI SB / AMD Hudson */
 #define AZX_DCAPS_PRESET_ATI_SB \

Have tried the patch on drm-intel-next-queued branch.

No any improvment, this bug still exist.

Please always attach dmesg with drm.debug=14.

Created attachment 114564 [details]
dmesg of drm-intel-testing-0313 boot up

Attached dmesg of drm-intel-testing-0313 kernel boot up.

(In reply to jinliangx.wang from comment #10)
> Created attachment 114564 [details]
> dmesg of drm-intel-testing-0313 boot up
> 
> Attached dmesg of drm-intel-testing-0313 kernel boot up.

This doesn't seem to include the patch from comment 7. Could you provide a dmesg log with that patch applied?

Yes, it's not include your patch.
I just think you need dmesg while this issue appear.
I will attach later.

Created attachment 114575 [details]
dmesg with patch applied

(In reply to jinliangx.wang from comment #13)
> Created attachment 114575 [details]
> dmesg with patch applied

Thanks.

I think there are multiple issues here, one is that the audio driver didn't ask for its power well, this is fixed by the patch in comment 7. The patch needs to be updated since it will also make haswell_set_bclk program the EM4/5 registers which are not existant on SKL.

Since the above fix is not enough, I assume that something needed by audio gets reset when turning off the audio power well and doesn't get reinitialized when reenabling it.

Adding Takashi for any insight.

Hi Imre,

Based on my experience, EM4/5 impacts on the playback speed, I don't think it will impact on the audio codec detection. (Actually before we detect HDMI codec, we will never touch such registers)

For the power well, yes, we need set the power well before HDMI codec detection. And the audio driver does do it already.

After power well on, we will do the HDMI codec detection. We won't power off the power well before HDMI codec detection.

BTW: I have asked the BIOS team about this issue. They told me that we should:
1. update the BIOS to latest (we have already done)
2. update the gfx driver to latest(windows). So I believe that BIOS may have changed some interfaces with gfx for this issue. Maybe we missed the change?

Thanks,
Libin

Hi Imre,

I found SKL doesn't apply the powerwell flag. I will test by adding the flag although it shouldn't add the flag here. Will update the test result later.

Thanks,
Libin

Hi,

(In reply to Libin Yang from comment #15)
> Hi Imre,
> 
> Based on my experience, EM4/5 impacts on the playback speed, I don't think
> it will impact on the audio codec detection. (Actually before we detect HDMI
> codec, we will never touch such registers)

What I meant is that the EM4/5 registers do not exist on SKL. Adding simply the AZX_DCAPS_I915_POWERWELL flag to AZX_DCAPS_INTEL_SKYLAKE would result in programming these registers also on SKL, which is incorrect, so that programming needs to be avoided.

> For the power well, yes, we need set the power well before HDMI codec
> detection. And the audio driver does do it already.

It didn't do it, you need to set AZX_DCAPS_I915_POWERWELL for that.

> After power well on, we will do the HDMI codec detection. We won't power off
> the power well before HDMI codec detection.
> 
> BTW: I have asked the BIOS team about this issue. They told me that we
> should:
> 1. update the BIOS to latest (we have already done)
> 2. update the gfx driver to latest(windows). So I believe that BIOS may have
> changed some interfaces with gfx for this issue. Maybe we missed the change?

I'm not aware of any other interaction between the gfx and sound driver besides what is done via the i915/audio component framework. For SKL in practice this means turning on and off power well 2. I still suspect that after turning power well 2 off then back on (this sequence actually happens based on the attached dmesg) some of the audio HW state gets reset and is not reinitialized properly. I would probably start enabling debugging in the sound driver and see where the detection fails.

(In reply to Libin Yang from comment #16)
> Hi Imre,
> 
> I found SKL doesn't apply the powerwell flag. I will test by adding the flag
> although it shouldn't add the flag here. Will update the test result later.

Not sure what you mean here. The sound driver needs to turn on/off its dependent power well (power well 2 in case of SKL), and it does this on platforms with the AZX_DCAPS_I915_POWERWELL flag set. So the flag needs to be set for SKL.

(In reply to Imre Deak from comment #17)
> I'm not aware of any other interaction between the gfx and sound driver
> besides what is done via the i915/audio component framework.

Basically what we do in hsw_audio_codec_enable() should trigger an irq in the audio driver to, well, whatever it needs to do. It works for HSW/BDW, I don't see why not for SKL. I didn't spot any changes in the audio programming sequence in bspec, so my conclusion is that the audio driver needs to be checked. (And indeed we've already pointed out the powerwell enabling/disabling missing for SKL.)

Hi Imre,

Thanks for reply. Please see my comments below.


> 
> What I meant is that the EM4/5 registers do not exist on SKL. Adding simply
> the AZX_DCAPS_I915_POWERWELL flag to AZX_DCAPS_INTEL_SKYLAKE would result in
> programming these registers also on SKL, which is incorrect, so that
> programming needs to be avoided.

OK, I see. This should be another issue. I will fix this issue later. 

> 
> > For the power well, yes, we need set the power well before HDMI codec
> > detection. And the audio driver does do it already.
> 
> It didn't do it, you need to set AZX_DCAPS_I915_POWERWELL for that.

I added the flag AZX_DCAPS_I915_POWERWELL and did the test yesterday. The test result is the same. Sometimes, the codec will not be recognized.

> 
> > After power well on, we will do the HDMI codec detection. We won't power off
> > the power well before HDMI codec detection.
> > 
> > BTW: I have asked the BIOS team about this issue. They told me that we
> > should:
> > 1. update the BIOS to latest (we have already done)
> > 2. update the gfx driver to latest(windows). So I believe that BIOS may have
> > changed some interfaces with gfx for this issue. Maybe we missed the change?
> 
> I'm not aware of any other interaction between the gfx and sound driver
> besides what is done via the i915/audio component framework. For SKL in
> practice this means turning on and off power well 2. I still suspect that
> after turning power well 2 off then back on (this sequence actually happens
> based on the attached dmesg) some of the audio HW state gets reset and is
> not reinitialized properly. I would probably start enabling debugging in the
> sound driver and see where the detection fails.

For " power well 2", do you mean the functions in audio driver "hda_i915_init()" and "hda_display_power()" will handle it? If so, audio driver doesn't call hda_display_power(hda, false) will not turn the power well2 off, right? Or gfx driver will turn off the power well that audio driver is not realized?

> 
> (In reply to Libin Yang from comment #16)
> > Hi Imre,
> > 
> > I found SKL doesn't apply the powerwell flag. I will test by adding the flag
> > although it shouldn't add the flag here. Will update the test result later.
> 
> Not sure what you mean here. The sound driver needs to turn on/off its
> dependent power well (power well 2 in case of SKL), and it does this on
> platforms with the AZX_DCAPS_I915_POWERWELL flag set. So the flag needs to
> be set for SKL.

Yes. I added the flag and tested it internally, it didn't work. And you are right, although it can't fix the audio codec detect issue. We really need this flag as we are using the HDMI codec. I will check with Takashi whether adding the flag is OK as you know this flag is mainly for HDA controller.

Hi Jani

(In reply to Jani Nikula from comment #18)
> (In reply to Imre Deak from comment #17)
> > I'm not aware of any other interaction between the gfx and sound driver
> > besides what is done via the i915/audio component framework.
> 
> Basically what we do in hsw_audio_codec_enable() should trigger an irq in
> the audio driver to, well, whatever it needs to do. It works for HSW/BDW, I
> don't see why not for SKL. I didn't spot any changes in the audio
> programming sequence in bspec, so my conclusion is that the audio driver
> needs to be checked. (And indeed we've already pointed out the powerwell
> enabling/disabling missing for SKL.)

Yes. The audio driver need the flag. I will add it later. But after adding the flag, the HDMI codec still sometime can't be recognized.

Thanks for the clue, I will check whether the audio driver is triggered correctly when hsw_audio_codec_enable() is called.

Hi Imre & Yani,

It suddenly occurred to me that maybe we need some delay after power on the power well. I will do the stress test by adding the delay. And will update the test result later.

Thanks,
Libin

Hi Imre & Jani,

The update:

I have tried to add the delay after powering on the powerwell but the issue still exists.

Hi

As the hdmi audio can be deteced successfully when monitor is connected, could you please check what's the difference gfx driver will do freom monitor isn't connected?

(In reply to Libin Yang from comment #23)
> As the hdmi audio can be deteced successfully when monitor is connected,
> could you please check what's the difference gfx driver will do freom
> monitor isn't connected?

For one, you can't expect to be able to read the registers (or the read values to make sense) *unless* you've called the ->get_power() function through the component API. There are plenty of audio registers that i915 never touches but need the power well from i915 to operate. And you should expect the registers to lose their values after you ->put_power(). Check bspec.

Hi Jani

(In reply to Jani Nikula from comment #24)
> (In reply to Libin Yang from comment #23)
> > As the hdmi audio can be deteced successfully when monitor is connected,
> > could you please check what's the difference gfx driver will do freom
> > monitor isn't connected?
> 
> For one, you can't expect to be able to read the registers (or the read
> values to make sense) *unless* you've called the ->get_power() function
> through the component API. There are plenty of audio registers that i915
> never touches but need the power well from i915 to operate. And you should
> expect the registers to lose their values after you ->put_power(). Check
> bspec.

For the get_power(), do you mean the power well. 

I will check whether the power well is put in audio driver side when reading the register. Thanks.

BTW: For the detect, in audio driver, Imre has submitted the power well codec before and I have applied the patch from Imre to enable power well on SKL before detecting audio codec. Suppose if the audio driver is not suspended, it will always get the power well. 

fyi, In alsa mail list, some one suggests not always powering on the power well. Only power on it when needed. When audio will playback/capture, audio driver will power on the power well. When gfx send unsol events, gfx will power on the power well.

Hi Imre & Jani,

I have debugged on the power well, it seems the power well is OK.

As Imre suggested last week, I removed the EM4/5 setting. It seems this can fix this bug. Interesting!!!

I will update the status tomorrow after the stress test.

Thanks for your help on this bug.

Regards,
Libin

It still failed :(

Hi all,

The bug is fixed.