Bug 94566

Adding Takashi for the SND failures and Ville who saw this on other platforms already.

Could you update the log to bugzilla?  It's unreachable.

(In reply to Takashi Iwai from comment #2)
> Could you update the log to bugzilla?  It's unreachable.

Ah, sorry. Here it is:

[  239.855741] ------------[ cut here ]------------
[  239.855750] WARNING: CPU: 1 PID: 87 at sound/hda/hdac_i915.c:91 snd_hdac_display_power+0xf1/0x110 [snd_hda_core]()
[  239.855752] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel i915 snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core mei_me mei snd_pcm i2c_hid sdhci_pci e1000e sdhci ptp mmc_core pps_core
[  239.855778] CPU: 1 PID: 87 Comm: kworker/1:2 Tainted: G     U  W       4.5.0-gfxbench+ #1
[  239.855780] Hardware name:                  /NUC6i5SYB, BIOS SYSKLi35.86A.0028.2015.1112.1822 11/12/2015
[  239.855784] Workqueue: pm pm_runtime_work
[  239.855786]  0000000000000000 ffff8802644f7be8 ffffffff813fef15 0000000000000000
[  239.855791]  ffffffffa00f877c ffff8802644f7c20 ffffffff81078a21 0000000000000000
[  239.855795]  ffff88026661d668 ffff880263ee10d8 ffff880265bc5668 0000000000000000
[  239.855799] Call Trace:
[  239.855803]  [<ffffffff813fef15>] dump_stack+0x67/0x92
[  239.855807]  [<ffffffff81078a21>] warn_slowpath_common+0x81/0xc0
[  239.855810]  [<ffffffff81078b15>] warn_slowpath_null+0x15/0x20
[  239.855815]  [<ffffffffa00f77e1>] snd_hdac_display_power+0xf1/0x110 [snd_hda_core]
[  239.855819]  [<ffffffffa013839d>] azx_intel_link_power+0xd/0x10 [snd_hda_intel]
[  239.855827]  [<ffffffffa011932a>] azx_link_power+0x1a/0x30 [snd_hda_codec]
[  239.855831]  [<ffffffffa00f21f9>] snd_hdac_link_power+0x29/0x40 [snd_hda_core]
[  239.855837]  [<ffffffffa01142a6>] hda_codec_runtime_suspend+0x76/0xa0 [snd_hda_codec]
[  239.855843]  [<ffffffffa0114230>] ? hda_codec_runtime_resume+0x50/0x50 [snd_hda_codec]
[  239.855847]  [<ffffffff8154719d>] __rpm_callback+0x2d/0x70
[  239.855850]  [<ffffffff815471ff>] rpm_callback+0x1f/0x80
[  239.855855]  [<ffffffffa0114230>] ? hda_codec_runtime_resume+0x50/0x50 [snd_hda_codec]
[  239.855858]  [<ffffffff81547764>] rpm_suspend+0x134/0x7f0
[  239.855861]  [<ffffffff815494c6>] pm_runtime_work+0x76/0xc0
[  239.855864]  [<ffffffff81093c8b>] process_one_work+0x1cb/0x680
[  239.855867]  [<ffffffff81093c06>] ? process_one_work+0x146/0x680
[  239.855869]  [<ffffffff81094189>] worker_thread+0x49/0x490
[  239.855872]  [<ffffffff81094140>] ? process_one_work+0x680/0x680
[  239.855874]  [<ffffffff81094140>] ? process_one_work+0x680/0x680
[  239.855878]  [<ffffffff8109a87a>] kthread+0xea/0x100
[  239.855881]  [<ffffffff817c53d7>] ? _raw_spin_unlock_irq+0x27/0x50
[  239.855885]  [<ffffffff8109a790>] ? kthread_create_on_node+0x1f0/0x1f0
[  239.855888]  [<ffffffff817c607f>] ret_from_fork+0x3f/0x70
[  239.855891]  [<ffffffff8109a790>] ? kthread_create_on_node+0x1f0/0x1f0
[  239.855893] ---[ end trace 74f6ab5178db725a ]---

Thanks.  It looks like the same bug Ville reported yesterday.

Is this a regression?  If yes, it'd be great to know the working and broken commit ids.

I'm checking a SKL machine with a DP audio, but unable to reproduce this, so far.
Any special setup?

I did the test on my SKL with latest nightly kernel doing the s3 stress test. But I can't reproduce this issue.

If it is the i915_power_refcount issue, is there any possibility that the below will happen:
1. runtime_resume is called (link_power_control is not set so far)
2. set the codec->core.link_power_control = 1 in hdmi audio driver
3. runtime_suspend is called

(In reply to Libin Yang from comment #6)
> I did the test on my SKL with latest nightly kernel doing the s3 stress
> test. But I can't reproduce this issue.
> 
> If it is the i915_power_refcount issue, is there any possibility that the
> below will happen:
> 1. runtime_resume is called (link_power_control is not set so far)
> 2. set the codec->core.link_power_control = 1 in hdmi audio driver
> 3. runtime_suspend is called

Well, it means that the runtime PM up/down occurs during the codec driver initialization.  This shouldn't happen normally.

Maybe the module reloading is a key to trigger?  I saw that some module reloading test was done before the failing tests.  We forgot something at removal, then leading to unbalance...?

In anyway, if Intel people can show me the exact reproducer that can run on my system, it'd be really helpful.

(In reply to Takashi Iwai from comment #7)
> (In reply to Libin Yang from comment #6)
> > I did the test on my SKL with latest nightly kernel doing the s3 stress
> > test. But I can't reproduce this issue.
> > 
> > If it is the i915_power_refcount issue, is there any possibility that the
> > below will happen:
> > 1. runtime_resume is called (link_power_control is not set so far)
> > 2. set the codec->core.link_power_control = 1 in hdmi audio driver
> > 3. runtime_suspend is called
> 
> Well, it means that the runtime PM up/down occurs during the codec driver
> initialization.  This shouldn't happen normally.
> 
> Maybe the module reloading is a key to trigger?  I saw that some module
> reloading test was done before the failing tests.  We forgot something at
> removal, then leading to unbalance...?
> 
> In anyway, if Intel people can show me the exact reproducer that can run on
> my system, it'd be really helpful.

Yes we do module reloads during our tests, it's possible that it triggers or make an issue more likely. Please check the tests/drv_module_reload_basic scripts in igt for the exact steps.

I ran tests/drv_module_reload_basic on my machine, but this itself seems working, I got no problem.

IIRC, in the Intel CI, the warning appeared later, not during this test script.  So, which test to run after this might be relevant.

Created attachment 122421 [details]
dmesg before piglit/igt tests

(In reply to Takashi Iwai from comment #9)
> I ran tests/drv_module_reload_basic on my machine, but this itself seems
> working, I got no problem.
> 
> IIRC, in the Intel CI, the warning appeared later, not during this test
> script.  So, which test to run after this might be relevant.

I should've attached the logs, since the link isn't reachable externally, will do so in the future. Please see the dmesg log before and after starting the piglit tests. There seems to be a WARN from HDA already during booting and then also right after the first subtest:
gem_ringfill: executing
gem_ringfill: starting subtest basic-default-S3

Created attachment 122422 [details]
dmesg during piglit/igt tests

Aha, that's interesting.  Now it gets clearer.

My wild guess is like below:

- The system boots with both i915 and HDA as modules, and they are loaded / initialized asynchronously.

- i915 init takes some time, while HD-audio starts initializing the bus at first.
  This results in the failure of the i915 component binding:

[    6.708025] snd_hda_intel 0000:00:1f.3: failed to add i915 component master (-19)

- Then i915 gets initialized, and HD-audio bus loads the HDMI codec driver.
  The recent HDMI codec driver tries to bind with i915 by itself.  It was for the old chipset without Powerwell.  At this time, the binding succeeds:

[    8.857780] snd_hda_intel 0000:00:1f.3: bound 0000:00:02.0 (ops i915_audio_component_bind_ops [i915])

- Then the whole probe procedure finishes, it decreases the refcount, as if the bus had also bound with i915.  So it gets unbalanced.


So, for the unbalanced PM refcount itself, we can avoid it by disabling the second on-demand binding.  But it means nothing but the i915 / HDA binding fails totally.

I wonder how this worked in the past.  Did the HDMI audio test pass even before reloading the modules?

(In reply to Takashi Iwai from comment #12)
> Aha, that's interesting.  Now it gets clearer.
> 
> My wild guess is like below:
> 
> - The system boots with both i915 and HDA as modules, and they are loaded /
> initialized asynchronously.
> 
> - i915 init takes some time, while HD-audio starts initializing the bus at
> first.
>   This results in the failure of the i915 component binding:
> 
> [    6.708025] snd_hda_intel 0000:00:1f.3: failed to add i915 component
> master (-19)
> 
> - Then i915 gets initialized, and HD-audio bus loads the HDMI codec driver.
>   The recent HDMI codec driver tries to bind with i915 by itself.  It was
> for the old chipset without Powerwell.  At this time, the binding succeeds:
> 
> [    8.857780] snd_hda_intel 0000:00:1f.3: bound 0000:00:02.0 (ops
> i915_audio_component_bind_ops [i915])
> 
> - Then the whole probe procedure finishes, it decreases the refcount, as if
> the bus had also bound with i915.  So it gets unbalanced.
> 
> 
> So, for the unbalanced PM refcount itself, we can avoid it by disabling the
> second on-demand binding.  But it means nothing but the i915 / HDA binding
> fails totally.

There is no guarantee which module gets loaded first and so no guarantee that you won't get the "failed to add i915 component master (-19)" error. The only proper way to solve this is to initialize anything needing the display driver from the component bind hook (hdac_component_master_bind).

(In reply to Imre Deak from comment #13)
> There is no guarantee which module gets loaded first and so no guarantee
> that you won't get the "failed to add i915 component master (-19)" error.
> The only proper way to solve this is to initialize anything needing the
> display driver from the component bind hook (hdac_component_master_bind).

Yes, but the reason we didn't do this is because we heard that the i915 powerwell has to be enabled *before* the codec gets probed.  So, it's rather surprising that the codec probe succeeded even without the powerwell hook.  Maybe it just worked because they were probed in parallel.  Or, the information from Intel hardware team might be wrong.  Who knows.

And, we can't block for syncing with i915 slave bind at the HD-audio bus probe time, either.  There are SKL systems without i915 graphics, and the same HD-audio bus is used for the Realtek codec for builtin audio.  So, the missing i915 binding is no fatal error on such systems.

That said, the current non-blocking behavior is on purpose.  But, *if* the codec probe would work without the powerwell hook in the controller side beforehand, we an put the blocking component binding again at the codec probe.  This might work -- but the technical information is missing whether it's correct or not.

(In reply to Takashi Iwai from comment #14)
> (In reply to Imre Deak from comment #13)
> > There is no guarantee which module gets loaded first and so no guarantee
> > that you won't get the "failed to add i915 component master (-19)" error.
> > The only proper way to solve this is to initialize anything needing the
> > display driver from the component bind hook (hdac_component_master_bind).
> 
> Yes, but the reason we didn't do this is because we heard that the i915
> powerwell has to be enabled *before* the codec gets probed.  So, it's rather
> surprising that the codec probe succeeded even without the powerwell hook. 
> Maybe it just worked because they were probed in parallel.  Or, the
> information from Intel hardware team might be wrong.  Who knows.

That would mean that the very first time you can physically probe the codec is when i915 is already loaded and hence hdac_component_master_bind() gets called. There is just no way around this.

> And, we can't block for syncing with i915 slave bind at the HD-audio bus
> probe time, either.  There are SKL systems without i915 graphics, and the
> same HD-audio bus is used for the Realtek codec for builtin audio.  So, the
> missing i915 binding is no fatal error on such systems.

Right, but then you have to split out the parts that have a hard dependency on the i915 driver, and initialize that part when the binding completes. There is again, just no way around this.

> That said, the current non-blocking behavior is on purpose.  But, *if* the
> codec probe would work without the powerwell hook in the controller side
> beforehand, we an put the blocking component binding again at the codec
> probe.  This might work -- but the technical information is missing whether
> it's correct or not.

I think you'd need to determine what parts of the HDA driver have a hard dependency on i915, split those out and init them from component bind hook. Everything else can get initialized earlier, for example the Skylake/Realtek stuff you mention above.

Well, this is a sort of chicken-and-egg problem.  For probing the HDMI codec, i915 binding is needed, yes.  But, for knowing whether it's a HDMI codec or not, we need to probe the codec at first.  Thus the driver needs the i915 binding *before* probing the codec.  Yet, i915 binding can't be done when the system has no i915 graphics...

We may look for Intel VGA PCI entries, and enable i915 binding only when found.  This would work in most cases, but not in 100%.  User may still build / use a system without graphics (e.g. booting with nomodeset option).

And, one more problem is that we gather up the all codecs as a single card.  Blocking a probe of one codec means to block the probe of the whole card.  So we can't take unlimited block behavior.

(And the unlimited block is anyway bad especially when the configuration is dynamic; you can disable KMS easily via nomodeset, while the HD-audio driver will still try to probe HDMI codec because it doesn't know that the graphics side is disabled.  Then it'll stall.)

(In reply to Takashi Iwai from comment #16)
> Well, this is a sort of chicken-and-egg problem.  For probing the HDMI
> codec, i915 binding is needed, yes.  But, for knowing whether it's a HDMI
> codec or not, we need to probe the codec at first.  Thus the driver needs
> the i915 binding *before* probing the codec.  Yet, i915 binding can't be
> done when the system has no i915 graphics...

I don't think it's a chicken-and-egg problem. There are two possibilities: 

1. Probing the codec needs a power well provided by the i915 device and as such the i915 driver. In this case there must be an i915 graphics device physically present in the system and so you can load the i915 driver which will end up doing the binding. You said that it's not clear yet whether this HW dependency really exists, I have no idea either, so someone needs to find the definite answer for this.

2. Probing the codec doesn't need an i915 provided power well (or any other functionality provided via the component interface), in which case there is no problem you just need to remove the power_well get/puts and probe the codec whenever.

> We may look for Intel VGA PCI entries, and enable i915 binding only when
> found.  This would work in most cases, but not in 100%.  User may still
> build / use a system without graphics (e.g. booting with nomodeset option).

In case 1. above booting with modeset or otherwise not having the i915 driver for some reason would also imply a lack of audio support, that is you would need to fail the probing of the audio driver.

(In reply to Imre Deak from comment #18)
> (In reply to Takashi Iwai from comment #16)

> In case 1. above booting with modeset or otherwise not having the i915
                                ^nomodeset
> driver for some reason would also imply a lack of audio support, that is you
> would need to fail the probing of the audio driver.

(In reply to Takashi Iwai from comment #17)
> And, one more problem is that we gather up the all codecs as a single card. 
> Blocking a probe of one codec means to block the probe of the whole card. 
> So we can't take unlimited block behavior.

Can you still separate interface registration from HW access? I assume the latter could be delay until the user first opens the device node, so you could do the blocking there and register the corresponding interface earlier.

> (And the unlimited block is anyway bad especially when the configuration is
> dynamic; you can disable KMS easily via nomodeset, while the HD-audio driver
> will still try to probe HDMI codec because it doesn't know that the graphics
> side is disabled.  Then it'll stall.)

Can the above help?

(In reply to Imre Deak from comment #18)
> (In reply to Takashi Iwai from comment #16)
> > Well, this is a sort of chicken-and-egg problem.  For probing the HDMI
> > codec, i915 binding is needed, yes.  But, for knowing whether it's a HDMI
> > codec or not, we need to probe the codec at first.  Thus the driver needs
> > the i915 binding *before* probing the codec.  Yet, i915 binding can't be
> > done when the system has no i915 graphics...
> 
> I don't think it's a chicken-and-egg problem. There are two possibilities: 
> 
> 1. Probing the codec needs a power well provided by the i915 device and as
> such the i915 driver. In this case there must be an i915 graphics device
> physically present in the system and so you can load the i915 driver which
> will end up doing the binding. You said that it's not clear yet whether this
> HW dependency really exists, I have no idea either, so someone needs to find
> the definite answer for this.

This is the case we're assuming, so far.  And, here we have a dependency problem.
 
> 2. Probing the codec doesn't need an i915 provided power well (or any other
> functionality provided via the component interface), in which case there is
> no problem you just need to remove the power_well get/puts and probe the
> codec whenever.

A remaining question in this case is how to bail out when no graphics driver is available even if the graphics hardware is on the machine.  In the current binding mechanism, it'll wait forever?
 
> > We may look for Intel VGA PCI entries, and enable i915 binding only when
> > found.  This would work in most cases, but not in 100%.  User may still
> > build / use a system without graphics (e.g. booting with nomodeset option).
> 
> In case 1. above booting with modeset or otherwise not having the i915
> driver for some reason would also imply a lack of audio support, that is you
> would need to fail the probing of the audio driver.

Yes, but it shouldn't fail for the Realtek audio codec, but should fail (drop) only HDMI codec.  Here comes back to the question again: for knowing what codec is on the bus, we already need i915 binding.  But how to fail i915 binding in this case?  Will i915 driver inform to component?  What if the module isn't present (e.g. in the limited initrd)?

(In reply to Imre Deak from comment #20)
> (In reply to Takashi Iwai from comment #17)
> > And, one more problem is that we gather up the all codecs as a single card. 
> > Blocking a probe of one codec means to block the probe of the whole card. 
> > So we can't take unlimited block behavior.
> 
> Can you still separate interface registration from HW access? I assume the
> latter could be delay until the user first opens the device node, so you
> could do the blocking there and register the corresponding interface earlier.

It's difficult, unfortunately.  HDMI codec may provide multiple streams (or even zero stream), and for knowing how many streams, we need to probe.  So, without probing, we don't know which device interface to provide.
 
And, on SKL architecture, the HDMI codec is on the same HDA bus that is represented as a single sound card instance.  A delayed probe means the dynamic addition / removal of PCM and other devices.  However, most of user-space stuff like PA doesn't work in that way (although it's possible in kernel API level).  They support only the card-level hotplug.

(In reply to Takashi Iwai from comment #21)
> (In reply to Imre Deak from comment #18)
> > (In reply to Takashi Iwai from comment #16)
> > > Well, this is a sort of chicken-and-egg problem.  For probing the HDMI
> > > codec, i915 binding is needed, yes.  But, for knowing whether it's a HDMI
> > > codec or not, we need to probe the codec at first.  Thus the driver needs
> > > the i915 binding *before* probing the codec.  Yet, i915 binding can't be
> > > done when the system has no i915 graphics...
> > 
> > I don't think it's a chicken-and-egg problem. There are two possibilities: 
> > 
> > 1. Probing the codec needs a power well provided by the i915 device and as
> > such the i915 driver. In this case there must be an i915 graphics device
> > physically present in the system and so you can load the i915 driver which
> > will end up doing the binding. You said that it's not clear yet whether this
> > HW dependency really exists, I have no idea either, so someone needs to find
> > the definite answer for this.
> 
> This is the case we're assuming, so far.  And, here we have a dependency
> problem.
>  
> > 2. Probing the codec doesn't need an i915 provided power well (or any other
> > functionality provided via the component interface), in which case there is
> > no problem you just need to remove the power_well get/puts and probe the
> > codec whenever.
> 
> A remaining question in this case is how to bail out when no graphics driver
> is available even if the graphics hardware is on the machine.  In the
> current binding mechanism, it'll wait forever?

1. If you mean no graphics driver because it wasn't enabled in Kconfig: that is an invalid configuration you shouldn't allow if Intel HDMI support in the audio driver is enabled in Kconfig; you need a Kconfig dependency for this.

2. If you mean the user boots with nomodeset or the i915 driver fails to probe: in the former case the user should know that audio will also stop working. In both cases it's to be expected that audio won't be available.

Other than the above scenarios I don't see why the binding wouldn't complete eventually. 

> > > We may look for Intel VGA PCI entries, and enable i915 binding only when
> > > found.  This would work in most cases, but not in 100%.  User may still
> > > build / use a system without graphics (e.g. booting with nomodeset option).
> > 
> > In case 1. above booting with modeset or otherwise not having the i915
> > driver for some reason would also imply a lack of audio support, that is you
> > would need to fail the probing of the audio driver.
> 
> Yes, but it shouldn't fail for the Realtek audio codec, but should fail
> (drop) only HDMI codec.

Assuming the limitation that the HDMI codec cannot be registered dynamically, the Realtek codec probing should also fail in this case. Yes, it sucks, but that's inherent in the audio framework and not a problem of the HDA driver. In any case this can only arise due to a kernel configuration problem, or in case of an error as discussed earlier, so in both cases it's not too unexpected to lose audio.

> Here comes back to the question again: for knowing what codec is on the bus, 
> we already need i915 binding.

To know that there is an Intel HDMI codec on the bus, you don't need the binding you can just look at PCIIDs. At this point you know you'll have to delay the probing of the HDMI codec (or any other codec due to the audio framework limitation) until the component binding completes.

> But how to fail i915 binding in this case? 

You don't need to fail it, whenever the audio driver decides to wait for the binding to complete (based on PCIIDs and Intel HDMI support being enabled in the audio Kconfig) the i915 driver must be present (because of a Kconfig dependency) and loaded eventually. The only cases when binding wouldn't complete are exceptional, the nomodeset and error cases discussed earlier.

> Will i915 driver inform to component?

In which case? The only reason i915 wouldn't initiate the binding is some error case, when it's also ok to fail audio probing (provided the audio driver decided to wait based on the previous point).

> What if the module isn't present (e.g. in the limited initrd)?

And it's in the rootfs only? Then it's ok to wait with audio probing until the i915 module gets loaded from the rootfs.

(In reply to Takashi Iwai from comment #22)
> (In reply to Imre Deak from comment #20)
> > (In reply to Takashi Iwai from comment #17)
> > > And, one more problem is that we gather up the all codecs as a single card. 
> > > Blocking a probe of one codec means to block the probe of the whole card. 
> > > So we can't take unlimited block behavior.
> > 
> > Can you still separate interface registration from HW access? I assume the
> > latter could be delay until the user first opens the device node, so you
> > could do the blocking there and register the corresponding interface earlier.
> 
> It's difficult, unfortunately.  HDMI codec may provide multiple streams (or
> even zero stream), and for knowing how many streams, we need to probe.  So,
> without probing, we don't know which device interface to provide.
>  
> And, on SKL architecture, the HDMI codec is on the same HDA bus that is
> represented as a single sound card instance. 

Why can't the HDMI codec be on a separate sound card?

> A delayed probe means the dynamic addition / removal of PCM and other 
> devices.  However, most of user-space stuff like PA doesn't work in that way 
> (although it's possible in kernel API level).  They support only the 
> card-level hotplug.

(In reply to Imre Deak from comment #24)
> (In reply to Takashi Iwai from comment #22)
> > (In reply to Imre Deak from comment #20)
> > > (In reply to Takashi Iwai from comment #17)
> > > > And, one more problem is that we gather up the all codecs as a single card. 
> > > > Blocking a probe of one codec means to block the probe of the whole card. 
> > > > So we can't take unlimited block behavior.
> > > 
> > > Can you still separate interface registration from HW access? I assume the
> > > latter could be delay until the user first opens the device node, so you
> > > could do the blocking there and register the corresponding interface earlier.
> > 
> > It's difficult, unfortunately.  HDMI codec may provide multiple streams (or
> > even zero stream), and for knowing how many streams, we need to probe.  So,
> > without probing, we don't know which device interface to provide.
> >  
> > And, on SKL architecture, the HDMI codec is on the same HDA bus that is
> > represented as a single sound card instance. 
> 
> Why can't the HDMI codec be on a separate sound card?

Because currently we build the card per PCI controller, as the PCI controller governs the stream DMA.

(In reply to Imre Deak from comment #23)
> (In reply to Takashi Iwai from comment #21)
> > (In reply to Imre Deak from comment #18)
> > > (In reply to Takashi Iwai from comment #16)
> > > > Well, this is a sort of chicken-and-egg problem.  For probing the HDMI
> > > > codec, i915 binding is needed, yes.  But, for knowing whether it's a HDMI
> > > > codec or not, we need to probe the codec at first.  Thus the driver needs
> > > > the i915 binding *before* probing the codec.  Yet, i915 binding can't be
> > > > done when the system has no i915 graphics...
> > > 
> > > I don't think it's a chicken-and-egg problem. There are two possibilities: 
> > > 
> > > 1. Probing the codec needs a power well provided by the i915 device and as
> > > such the i915 driver. In this case there must be an i915 graphics device
> > > physically present in the system and so you can load the i915 driver which
> > > will end up doing the binding. You said that it's not clear yet whether this
> > > HW dependency really exists, I have no idea either, so someone needs to find
> > > the definite answer for this.
> > 
> > This is the case we're assuming, so far.  And, here we have a dependency
> > problem.
> >  
> > > 2. Probing the codec doesn't need an i915 provided power well (or any other
> > > functionality provided via the component interface), in which case there is
> > > no problem you just need to remove the power_well get/puts and probe the
> > > codec whenever.
> > 
> > A remaining question in this case is how to bail out when no graphics driver
> > is available even if the graphics hardware is on the machine.  In the
> > current binding mechanism, it'll wait forever?
> 
> 1. If you mean no graphics driver because it wasn't enabled in Kconfig: that
> is an invalid configuration you shouldn't allow if Intel HDMI support in the
> audio driver is enabled in Kconfig; you need a Kconfig dependency for this.
> 
> 2. If you mean the user boots with nomodeset or the i915 driver fails to
> probe: in the former case the user should know that audio will also stop
> working. In both cases it's to be expected that audio won't be available.
> 
> Other than the above scenarios I don't see why the binding wouldn't complete
> eventually. 
> 
> > > > We may look for Intel VGA PCI entries, and enable i915 binding only when
> > > > found.  This would work in most cases, but not in 100%.  User may still
> > > > build / use a system without graphics (e.g. booting with nomodeset option).
> > > 
> > > In case 1. above booting with modeset or otherwise not having the i915
> > > driver for some reason would also imply a lack of audio support, that is you
> > > would need to fail the probing of the audio driver.
> > 
> > Yes, but it shouldn't fail for the Realtek audio codec, but should fail
> > (drop) only HDMI codec.
> 
> Assuming the limitation that the HDMI codec cannot be registered
> dynamically, the Realtek codec probing should also fail in this case. Yes,
> it sucks, but that's inherent in the audio framework and not a problem of
> the HDA driver.

Sorry, this no-go.  It's a clear regression from the previous kernels, and I know people expecting this works like that.  (I've got bug reports in the past for similar things already.)

> In any case this can only arise due to a kernel
> configuration problem, or in case of an error as discussed earlier, so in
> both cases it's not too unexpected to lose audio.

It's not always an "error".  nomodeset option, for example, should be possible to be handled gracefully.

The missing i915 module can be seen as a configuration error, and we have some excuse not to make things working well.  But nomodeset is just a runtime configuration, and it shouldn't affect the things that worked in previous kernels.

> > Here comes back to the question again: for knowing what codec is on the bus, 
> > we already need i915 binding.
> 
> To know that there is an Intel HDMI codec on the bus, you don't need the
> binding you can just look at PCIIDs.

No, on SKL or BSW, there is no PCI controller for HDMI.

>  At this point you know you'll have to
> delay the probing of the HDMI codec (or any other codec due to the audio
> framework limitation) until the component binding completes.
> 
> > But how to fail i915 binding in this case? 
> 
> You don't need to fail it, whenever the audio driver decides to wait for the
> binding to complete (based on PCIIDs and Intel HDMI support being enabled in
> the audio Kconfig) the i915 driver must be present (because of a Kconfig
> dependency) and loaded eventually. The only cases when binding wouldn't
> complete are exceptional, the nomodeset and error cases discussed earlier.

The question is how to decide to wait or not.
For nomodeset case, clearly we shouldn't wait.
 
> > Will i915 driver inform to component?
> 
> In which case? The only reason i915 wouldn't initiate the binding is some
> error case, when it's also ok to fail audio probing (provided the audio
> driver decided to wait based on the previous point).

Well, my question is how the audio driver knows that i915 failed to register the slave.  Otherwise the audio driver will wait forever with a hope i915 will register anytime later.
 
> > What if the module isn't present (e.g. in the limited initrd)?
> 
> And it's in the rootfs only? Then it's ok to wait with audio probing until
> the i915 module gets loaded from the rootfs.

The question is how long we may block.  I guess some blocking behavior with timeout is acceptable for this particular case, as it's a system configuration problem.  But again, nomodeset is a different matter.

(In reply to Takashi Iwai from comment #26)
> > To know that there is an Intel HDMI codec on the bus, you don't need the
> > binding you can just look at PCIIDs.
> 
> No, on SKL or BSW, there is no PCI controller for HDMI.

I mean, there is no PCI controller dedicated for HDMI.  On SKL & co, all HDMI and Realtek codecs are on the same bus that is controlled by a single PCI.

(In reply to Takashi Iwai from comment #26)
>[...]
> > > 
> > > Yes, but it shouldn't fail for the Realtek audio codec, but should fail
> > > (drop) only HDMI codec.
> > 
> > Assuming the limitation that the HDMI codec cannot be registered
> > dynamically, the Realtek codec probing should also fail in this case. Yes,
> > it sucks, but that's inherent in the audio framework and not a problem of
> > the HDA driver.
> 
> Sorry, this no-go. 

To support this you need to fix the audio framework to allow for dynamic registration of the HDMI codec.

> It's a clear regression from the previous kernels, and I
> know people expecting this works like that.  (I've got bug reports in the
> past for similar things already.)

Could you describe the exact configuration? Is Intel HDMI support enabled in Kconfig? Is there an Intel HDMI codec present in the bug reporter's system?

> > In any case this can only arise due to a kernel
> > configuration problem, or in case of an error as discussed earlier, so in
> > both cases it's not too unexpected to lose audio.
> 
> It's not always an "error".

Please specify in which case, I would like to better understand.

> nomodeset option, for example, should be possible to be handled gracefully.

You can check if nomodeset was passed on the command line.

> The missing i915 module can be seen as a configuration error, and we have
> some excuse not to make things working well.  But nomodeset is just a
> runtime configuration, and it shouldn't affect the things that worked in
> previous kernels.
> 
> > > Here comes back to the question again: for knowing what codec is on the bus, 
> > > we already need i915 binding.
> > 
> > To know that there is an Intel HDMI codec on the bus, you don't need the
> > binding you can just look at PCIIDs.
> 
> No, on SKL or BSW, there is no PCI controller for HDMI.

So why not check for the presence of SKL, BSW etc. graphics PCIIDs? That does indicate the presence of an HDMI codec.

> >  At this point you know you'll have to
> > delay the probing of the HDMI codec (or any other codec due to the audio
> > framework limitation) until the component binding completes.
> > 
> > > But how to fail i915 binding in this case? 
> > 
> > You don't need to fail it, whenever the audio driver decides to wait for the
> > binding to complete (based on PCIIDs and Intel HDMI support being enabled in
> > the audio Kconfig) the i915 driver must be present (because of a Kconfig
> > dependency) and loaded eventually. The only cases when binding wouldn't
> > complete are exceptional, the nomodeset and error cases discussed earlier.
> 
> The question is how to decide to wait or not.
> For nomodeset case, clearly we shouldn't wait.

The audio driver can check if nomodeset was passed. 

> > > Will i915 driver inform to component?
> > 
> > In which case? The only reason i915 wouldn't initiate the binding is some
> > error case, when it's also ok to fail audio probing (provided the audio
> > driver decided to wait based on the previous point).
> 
> Well, my question is how the audio driver knows that i915 failed to register
> the slave.  Otherwise the audio driver will wait forever with a hope i915
> will register anytime later.
>  
> > > What if the module isn't present (e.g. in the limited initrd)?
> > 
> > And it's in the rootfs only? Then it's ok to wait with audio probing until
> > the i915 module gets loaded from the rootfs.
> 
> The question is how long we may block.  I guess some blocking behavior with
> timeout is acceptable for this particular case, as it's a system
> configuration problem.  But again, nomodeset is a different matter.

I don't think we need some timeout, it's an exceptional case if binding doesn't complete in a timely manner.

(In reply to Takashi Iwai from comment #25)
> (In reply to Imre Deak from comment #24)
> > (In reply to Takashi Iwai from comment #22)
> > > (In reply to Imre Deak from comment #20)
> > > > (In reply to Takashi Iwai from comment #17)
> > > > > And, one more problem is that we gather up the all codecs as a single card. 
> > > > > Blocking a probe of one codec means to block the probe of the whole card. 
> > > > > So we can't take unlimited block behavior.
> > > > 
> > > > Can you still separate interface registration from HW access? I assume the
> > > > latter could be delay until the user first opens the device node, so you
> > > > could do the blocking there and register the corresponding interface earlier.
> > > 
> > > It's difficult, unfortunately.  HDMI codec may provide multiple streams (or
> > > even zero stream), and for knowing how many streams, we need to probe.  So,
> > > without probing, we don't know which device interface to provide.
> > >  
> > > And, on SKL architecture, the HDMI codec is on the same HDA bus that is
> > > represented as a single sound card instance. 
> > 
> > Why can't the HDMI codec be on a separate sound card?
> 
> Because currently we build the card per PCI controller, as the PCI
> controller governs the stream DMA.

Ok, but is it in practice possible? Sound card looks like a software abstraction to me, behind which you could implement your own way of arbitrating access to the HW among the separate sound cards.

(In reply to Imre Deak from comment #28)
> (In reply to Takashi Iwai from comment #26)
> >[...]
> > > > 
> > > > Yes, but it shouldn't fail for the Realtek audio codec, but should fail
> > > > (drop) only HDMI codec.
> > > 
> > > Assuming the limitation that the HDMI codec cannot be registered
> > > dynamically, the Realtek codec probing should also fail in this case. Yes,
> > > it sucks, but that's inherent in the audio framework and not a problem of
> > > the HDA driver.
> > 
> > Sorry, this no-go. 
> 
> To support this you need to fix the audio framework to allow for dynamic
> registration of the HDMI codec.

The point is that it just worked.
 
> > It's a clear regression from the previous kernels, and I
> > know people expecting this works like that.  (I've got bug reports in the
> > past for similar things already.)
> 
> Could you describe the exact configuration? Is Intel HDMI support enabled in
> Kconfig? Is there an Intel HDMI codec present in the bug reporter's system?
y v
> > > In any case this can only arise due to a kernel
> > > configuration problem, or in case of an error as discussed earlier, so in
> > > both cases it's not too unexpected to lose audio.
> > 
> > It's not always an "error".
> 
> Please specify in which case, I would like to better understand.

The disablement of i915 KMS is often user's choice of action, and then it is no error, per se.  We can't handle it as a fatal error in the audio side as a justification to shut off everything, especially if that action worked in the previous kernels.
 
> > nomodeset option, for example, should be possible to be handled gracefully.
> 
> You can check if nomodeset was passed on the command line.

i915.modeset=0 is another option.  And, it can be passed implicitly via /etc/modprobe.d/*.
 
> > The missing i915 module can be seen as a configuration error, and we have
> > some excuse not to make things working well.  But nomodeset is just a
> > runtime configuration, and it shouldn't affect the things that worked in
> > previous kernels.
> > 
> > > > Here comes back to the question again: for knowing what codec is on the bus, 
> > > > we already need i915 binding.
> > > 
> > > To know that there is an Intel HDMI codec on the bus, you don't need the
> > > binding you can just look at PCIIDs.
> > 
> > No, on SKL or BSW, there is no PCI controller for HDMI.
> 
> So why not check for the presence of SKL, BSW etc. graphics PCIIDs? That
> does indicate the presence of an HDMI codec.

Yes, we can check that the Intel VGA is present.  I wrote it in my early comment, too.  But, then how do you know that i915 graphics driver will really kick off the component?  The i915 driver might be disabled (nomodeset or whatever) or it might fail to initialize.  Then the audio driver still waits forever?
 
> > >  At this point you know you'll have to
> > > delay the probing of the HDMI codec (or any other codec due to the audio
> > > framework limitation) until the component binding completes.
> > > 
> > > > But how to fail i915 binding in this case? 
> > > 
> > > You don't need to fail it, whenever the audio driver decides to wait for the
> > > binding to complete (based on PCIIDs and Intel HDMI support being enabled in
> > > the audio Kconfig) the i915 driver must be present (because of a Kconfig
> > > dependency) and loaded eventually. The only cases when binding wouldn't
> > > complete are exceptional, the nomodeset and error cases discussed earlier.
> > 
> > The question is how to decide to wait or not.
> > For nomodeset case, clearly we shouldn't wait.
> 
> The audio driver can check if nomodeset was passed. 

nomodeset is just one case.  There are others the audio driver can't know without help of graphics driver side.
 
> > > > Will i915 driver inform to component?
> > > 
> > > In which case? The only reason i915 wouldn't initiate the binding is some
> > > error case, when it's also ok to fail audio probing (provided the audio
> > > driver decided to wait based on the previous point).
> > 
> > Well, my question is how the audio driver knows that i915 failed to register
> > the slave.  Otherwise the audio driver will wait forever with a hope i915
> > will register anytime later.
> >  
> > > > What if the module isn't present (e.g. in the limited initrd)?
> > > 
> > > And it's in the rootfs only? Then it's ok to wait with audio probing until
> > > the i915 module gets loaded from the rootfs.
> > 
> > The question is how long we may block.  I guess some blocking behavior with
> > timeout is acceptable for this particular case, as it's a system
> > configuration problem.  But again, nomodeset is a different matter.
> 
> I don't think we need some timeout, it's an exceptional case if binding
> doesn't complete in a timely manner.

Yes, and these are exceptions that should be handled as error, IMO.  It's safer to have a timeout, really.  You should never expect users doing sane :)

(In reply to Imre Deak from comment #29)
> (In reply to Takashi Iwai from comment #25)
> > (In reply to Imre Deak from comment #24)
> > > (In reply to Takashi Iwai from comment #22)
> > > > (In reply to Imre Deak from comment #20)
> > > > > (In reply to Takashi Iwai from comment #17)
> > > > > > And, one more problem is that we gather up the all codecs as a single card. 
> > > > > > Blocking a probe of one codec means to block the probe of the whole card. 
> > > > > > So we can't take unlimited block behavior.
> > > > > 
> > > > > Can you still separate interface registration from HW access? I assume the
> > > > > latter could be delay until the user first opens the device node, so you
> > > > > could do the blocking there and register the corresponding interface earlier.
> > > > 
> > > > It's difficult, unfortunately.  HDMI codec may provide multiple streams (or
> > > > even zero stream), and for knowing how many streams, we need to probe.  So,
> > > > without probing, we don't know which device interface to provide.
> > > >  
> > > > And, on SKL architecture, the HDMI codec is on the same HDA bus that is
> > > > represented as a single sound card instance. 
> > > 
> > > Why can't the HDMI codec be on a separate sound card?
> > 
> > Because currently we build the card per PCI controller, as the PCI
> > controller governs the stream DMA.
> 
> Ok, but is it in practice possible? Sound card looks like a software
> abstraction to me, behind which you could implement your own way of
> arbitrating access to the HW among the separate sound cards.

Possible, but really tough.

Imagine to split i915 driver to multiple individual drm drivers, each of which takes care of only one output (VGA, eDP, HDMI, DP).  Your suggestion is something like that.

(In reply to Takashi Iwai from comment #30)
> (In reply to Imre Deak from comment #28)
> > (In reply to Takashi Iwai from comment #26)
> > >[...]
> > > > > 
> > > > > Yes, but it shouldn't fail for the Realtek audio codec, but should fail
> > > > > (drop) only HDMI codec.
> > > > 
> > > > Assuming the limitation that the HDMI codec cannot be registered
> > > > dynamically, the Realtek codec probing should also fail in this case. Yes,
> > > > it sucks, but that's inherent in the audio framework and not a problem of
> > > > the HDA driver.
> > > 
> > > Sorry, this no-go. 
> > 
> > To support this you need to fix the audio framework to allow for dynamic
> > registration of the HDMI codec.
> 
> The point is that it just worked.

Well, it didn't really work in a generic way for anybody who wanted HDMI functionality as we see now. But I'm not saying we should break the reporter's machine, just trying to figure out what caused the problem and how could it be solved in a way that also provides a more robust HDMI functionality.

> > > It's a clear regression from the previous kernels, and I
> > > know people expecting this works like that.  (I've got bug reports in the
> > > past for similar things already.)
> > 
> > Could you describe the exact configuration? Is Intel HDMI support enabled in
> > Kconfig? Is there an Intel HDMI codec present in the bug reporter's system?
> y v

So the i915 driver must (or should) be configured too for this user (b/c of Kconfig dependency). So why the i915 driver didn't bind, is it because of nomodeset/i915.modeset=0? In this case I think we could do a null-op binding from the i915 side so you could bail out skipping HDMI probing.

> > > > In any case this can only arise due to a kernel
> > > > configuration problem, or in case of an error as discussed earlier, so in
> > > > both cases it's not too unexpected to lose audio.
> > > 
> > > It's not always an "error".
> > 
> > Please specify in which case, I would like to better understand.
> 
> The disablement of i915 KMS is often user's choice of action, and then it is
> no error, per se.  We can't handle it as a fatal error in the audio side as
> a justification to shut off everything, especially if that action worked in
> the previous kernels.
> > > nomodeset option, for example, should be possible to be handled gracefully.
> > 
> > You can check if nomodeset was passed on the command line.
> 
> i915.modeset=0 is another option.  And, it can be passed implicitly via
> /etc/modprobe.d/*.

Ok, so would the above null-op binding help?

> > > The missing i915 module can be seen as a configuration error, and we have
> > > some excuse not to make things working well.  But nomodeset is just a
> > > runtime configuration, and it shouldn't affect the things that worked in
> > > previous kernels.
> > > 
> > > > > Here comes back to the question again: for knowing what codec is on the bus, 
> > > > > we already need i915 binding.
> > > > 
> > > > To know that there is an Intel HDMI codec on the bus, you don't need the
> > > > binding you can just look at PCIIDs.
> > > 
> > > No, on SKL or BSW, there is no PCI controller for HDMI.
> > 
> > So why not check for the presence of SKL, BSW etc. graphics PCIIDs? That
> > does indicate the presence of an HDMI codec.
> 
> Yes, we can check that the Intel VGA is present.  I wrote it in my early
> comment, too.  But, then how do you know that i915 graphics driver will
> really kick off the component?  The i915 driver might be disabled (nomodeset
> or whatever) or it might fail to initialize.  Then the audio driver still
> waits forever?

Would the above null-op binding help?

Yes, a null bind or similar way to inform to component master was also what I had in my mind.  This should help in case i915 KMS is disabled intentionally or fails initialization.

The rest cases are basically configuration errors (e.g. missing i915 module on media), and this could be covered by timeout.  Then at least user will see the error instead of the forever silent.

The next question is whether the i915 power is really mandatory before issuing the vendor id verb for HDMI codec on HDA bus.  If this can be skipped, the things will be easier...

(In reply to Takashi Iwai from comment #33)
> Yes, a null bind or similar way to inform to component master was also what
> I had in my mind.  This should help in case i915 KMS is disabled
> intentionally or fails initialization.

Great, so I'll try to put together something that does this from the i915 side.

> The rest cases are basically configuration errors (e.g. missing i915 module
> on media), and this could be covered by timeout.  Then at least user will
> see the error instead of the forever silent.
> 
> The next question is whether the i915 power is really mandatory before
> issuing the vendor id verb for HDMI codec on HDA bus.  If this can be
> skipped, the things will be easier...

Created attachment 122433 [details] [review]
Add audio component stub

(In reply to Imre Deak from comment #34)
> (In reply to Takashi Iwai from comment #33)
> > Yes, a null bind or similar way to inform to component master was also what
> > I had in my mind.  This should help in case i915 KMS is disabled
> > intentionally or fails initialization.
> 
> Great, so I'll try to put together something that does this from the i915
> side.

Please see the attached patches, let me know if they work for you. The second patch is just for showing how to check for the disabled component functionality on the audio side.

Created attachment 122434 [details] [review]
Silently fail component ops if audio component is disabled

Thanks, the patches look good to me.  Now we'd need the changes in the audio side.  I'll poke it a bit later.

Priority and severity increased. This basic test is failing on APL.
Tested with:
Kernel drm-intel-nightly 4.6.0-rc3 dc5380b from http://cgit.freedesktop.org/drm-intel/
  commit dc5380b5263ebb0bf251bb09db542585702b528b
  Author: Chris Wilson <chris@chris-wilson.co.uk>
  Date:   Mon Apr 11 20:43:45 2016 +0100
  drm-intel-nightly: 2016y-04m-11d-19h-43m-10s UTC integration manifest

For the same test with following configuration i'm facing dmesg-warn. Sharing logs


++ Platform                            : BXT-P
 ++ Motherboard model                   : Broxton P
 ++ Motherboard type                    : NOTEBOOK Hand Held
 ++ Motherboard manufacturer            : Intel Corp.
 ++ CPU family                          : Other
 ++ CPU information                     : 06/5c
 ++ GPU Card                            : Intel Corporation Device 5a84 (rev 03) (prog-if 00 [VGA controller])
 ++ Memory ram                          : 8 GB
 ++ Maximum memory ram allowed          : 16 GB
 ++ Display resolution                  :
 ++ CPU's number                        : 4
 ++ Hard drive capacity                 : 120 GB

 ++ Kernel version                      : 4.6.0-rc3-nightly+
 ++ Linux distribution                  : Ubuntu 15.10
 ++ Architecture                        : 64-bit
 
 ++ xf86-video-intel version            : 2.99.917
 ++ Xorg-Xserver version                : 1.17.2
 ++ DRM version                         : 2.4.64
 
 ++ Cairo version                       : 1.14.2
 ++ Intel GPU Tools version             : Tag [intel-gpu-tools-1.14-133-gc89e8db] / Commit [c89e8db]
 ++ Kernel driver in use                : i915
 ++ Hardware acceleration               :
 ++ Bios revision                       : 131.10
 ++ KSC revision                        : 1.12

Created attachment 122945 [details]
dmesg log

[   72.109585] WARNING: CPU: 0 PID: 1639 at /home/shared/kernels/drm-intel-nightly/drivers/gpu/drm/i915/intel_pm.c:3647 skl_update_other_pipe_wm+0x172/0x180 [i915]
[   72.109586] WARN_ON(!wm_changed)

You must be looking at the wrong bug.  This warning has nothing to do with this bug entry, but rather an issue related with below:
  https://bugs.freedesktop.org/show_bug.cgi?id=89055

This result is not anymore fail but dmesg_warn and is tracked into bug 89055