[igt-dev] [PATCH i-g-t] tests/core_hotunplug: apply audio unload for all i915 hw

[Kai Vehmanen]

Hi,

On Tue, 2 Nov 2021, Janusz Krzysztofik wrote:

> On Monday, 1 November 2021 17:55:43 CET Kai Vehmanen wrote:
>> The HDA audio driver does not support dynamic hotplug of a HDA codec and
>> that applies also to the display HDA codec.
> 
> Before any workaround was introduced to this test, it was triggering a kernel 
> WARN / taint on i915 unbind, but only on selected platforms.  Since the test 
> was included into BAT scope, it was affecting badly significant parts of BAT 
> runs, reducing their test coverage.  To make the test mode friendly to BAT 

ack. This issue surfaced on platforms like DG1 where audio controller is 
on the GPU. Audio driver doesn't support i915 dynamically disappearing, 
and in case of these hw, failure was easy to hit. 

> advice.  Now your change extends the scope of the current workaround over all 
> platforms.  Why do you think we should unload the audio driver on all possible 
> platforms, even on those which didn't suffer from the original issue?  Can we 

Even if the symptoms are less severe, the fundamental problem remains. 
The HDA audio driver is not designed to handle individual codecs (display 
audio is one of possibly many codecs) disappearing in hotplug fashion. 
Audio stack does support hotplug for USB and Firewire, but not for 
individual HDA codecs. This makes sense as the hardware connections are 
fixed for HDA.

So this creates unnecessary fragility to the test. If any entity in the 
system uses the audio controller during the i-g-t test, the test can fail 
-- either a warning about dangling wakeref or something worse. If a 
distribution changes runtime suspend defaults for audio controller, you 
will get different results. And you need to maintain a list of PCI IDs 
where the controller is part of the display hardware (and yo must do the 
unload).

If we always unload:
   - if audio is used by some application -> test is skipped with igt_skip()
   - as audio driver us unloaded, no application can start using audio 
     while test is running -> more predictable test runs

> approach.  Can we define exact conditions under which the audio driver should 
> be unloaded before i915 unbind and how a test can detect those conditions? 

I'd say currently the reverse is easier -- if i915 has audio capability, 
audio driver should be unloaded. And given the priv.snd_unload==true is a 
no-op on systems with no display audio capability, this simple 
approach seems to do the job.

I don't say this is perfect, but this would be a more stable baseline
to build on.

Br, Kai