Bug 102370

diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
index cb916ac..cb55d2b 100644
--- a/tests/kms_hdmi_inject.c
+++ b/tests/kms_hdmi_inject.c
@@ -246,6 +246,12 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
        free(edid);
 }
 
+static bool has_proc_asound(void)
+{
+       struct stat st;
+       return stat("/proc/asound", &st) == 0;
+}
+
 igt_main
 {
        int drm_fd;
@@ -263,8 +269,10 @@ igt_main
        igt_subtest("inject-4k")
                hdmi_inject_4k(drm_fd, connector);
 
-       igt_subtest("inject-audio")
+       igt_subtest("inject-audio") {
+               igt_require(has_proc_asound());
                hdmi_inject_audio(drm_fd, connector);
+       }
 
        igt_fixture {
                drmModeFreeConnector(connector);

(In reply to Chris Wilson from comment #1)
> diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
> index cb916ac..cb55d2b 100644
> --- a/tests/kms_hdmi_inject.c
> +++ b/tests/kms_hdmi_inject.c
> @@ -246,6 +246,12 @@ hdmi_inject_audio(int drm_fd, drmModeConnector
> [...]

I can't find this patch on the list. Are you asking me to test it before you cook a proper patch?

Yes, not even close to being sure what is intended by this test and what its requirements should be.

(In reply to Chris Wilson from comment #3)
> Yes, not even close to being sure what is intended by this test and what its
> requirements should be.

$ stat /proc/asound/
  File: /proc/asound/
  Size: 0               Blocks: 0          IO Block: 1024   directory
Device: 4h/4d   Inode: 4026532007  Links: 4
Access: (0555/dr-xr-xr-x)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2017-08-24 16:30:30.516462761 +0300
Modify: 2017-08-24 16:30:30.516462761 +0300
Change: 2017-08-24 16:30:30.516462761 +0300
 Birth: -

So, I guess this is not the issue.

Please retest with current drm-tip. Possibly fixed by:

commit ac6c35a4d8c77083525044a192cb1a8711381e94
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Mon Sep 18 21:20:03 2017 +0300

    drm: add backwards compatibility support for drm_kms_helper.edid_firmware

commit 53fd40a90f3c0bdad86ec266ee5df833f54ace39
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Tue Sep 12 11:19:26 2017 +0300

    drm: handle override and firmware EDID at drm_do_get_edid() level

No failures in the last 11 runs, when it used to be 100% reproducible. Seems fixed!

(In reply to Martin Peres from comment #6)
> No failures in the last 11 runs, when it used to be 100% reproducible. Seems
> fixed!

Thanks! For completeness, it's possible this is fixed by

commit d81fb7fd9436e81fda67e5bc8ed0713aa28d3db2
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Tue Sep 19 18:38:13 2017 +0300

    drm/i915: always update ELD connector type after get modes

instead of the ones mentioned in comment #5.

https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_3903/fi-cfl-s2/igt@kms_hdmi_inject@inject-audio.html

(kms_hdmi_inject:1543) CRITICAL: Test assertion failure function hdmi_inject_audio, file kms_hdmi_inject.c:236:
(kms_hdmi_inject:1543) CRITICAL: Failed assertion: eld_is_valid()
(kms_hdmi_inject:1543) CRITICAL: Last errno: 2, No such file or directory
Subtest inject-audio failed.

More data from running the shards testlist on BAT machines. Overview of the results is available here:
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip.html

https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1/fi-glk-1/igt@kms_hdmi_inject@inject-audio.html
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1/fi-skl-guc/igt@kms_hdmi_inject@inject-audio.html
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1/fi-skl-gvtdvm/igt@kms_hdmi_inject@inject-audio.html

https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1/fi-bsw-n3050/igt@kms_hdmi_inject@inject-audio.html

https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_1/fi-ivb-3770/igt@kms_hdmi_inject@inject-audio.html


(kms_hdmi_inject:1707) CRITICAL: Test assertion failure function hdmi_inject_audio, file ../tests/kms_hdmi_inject.c:236:
(kms_hdmi_inject:1707) CRITICAL: Failed assertion: eld_is_valid()
(kms_hdmi_inject:1707) CRITICAL: Last errno: 2, No such file or directory
Subtest inject-audio failed.

https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_3975/shard-snb7/igt@kms_hdmi_inject@inject-audio.html

(kms_hdmi_inject:2853) CRITICAL: Test assertion failure function hdmi_inject_audio, file ../tests/kms_hdmi_inject.c:236:
(kms_hdmi_inject:2853) CRITICAL: Failed assertion: eld_is_valid()
(kms_hdmi_inject:2853) CRITICAL: Last errno: 2, No such file or directory
Subtest inject-audio failed.

Also on glk: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_4066/shard-glk8/igt@kms_hdmi_inject@inject-audio.html

(kms_hdmi_inject:1272) CRITICAL: Test assertion failure function hdmi_inject_audio, file ../tests/kms_hdmi_inject.c:236:
(kms_hdmi_inject:1272) CRITICAL: Failed assertion: eld_is_valid()
(kms_hdmi_inject:1272) CRITICAL: Last errno: 2, No such file or directory
Subtest inject-audio failed.

*** Bug 105111 has been marked as a duplicate of this bug. ***

*** Bug 105595 has been marked as a duplicate of this bug. ***

Created attachment 143675 [details]
Test execution errors

This issue is still reproducible on ICL U, errors are in attachments.
(kms_hdmi_inject:5266) CRITICAL: Test assertion failure function hdmi_inject_audio, file ../tests/kms_hdmi_inject.c:236:

Arek checked the test and could not figure out what it is supposed to test. Some of the asserts are lacking any information.

Adding Abdiel and Petri who reviewed the patch that introduced the test. Please explain to us what the test is doing and what the failure means.

Once we have this information, we'll continue with the user impact assessment!

Bumping the priority to high because we don't have any other audio test right now, until the chamelium tests from Simon Ser land!

Looking forward to it though :p

(In reply to Martin Peres from comment #17)
> Arek checked the test and could not figure out what it is supposed to test.
> Some of the asserts are lacking any information.
> 
> Adding Abdiel and Petri who reviewed the patch that introduced the test.
> Please explain to us what the test is doing and what the failure means.
> 
> Once we have this information, we'll continue with the user impact
> assessment!

@Petri/Abdiel can you explain how the failure impacts the user?

Mika to explain what the test is doing and what are our guesses on why it fails.

Meanwhile, according to what we have discussed in person this is exploring one of the theories: https://patchwork.freedesktop.org/series/60319/

The EDID used advertises no audio support.

(But the kernel seems to happily expose the audio sink on some platforms since the test doesn't always fail? But maybe this is a kernel workaround for bad EDIDs?)

The test will pick the first disconnected HDMI-A connector, force an EDID on it, and check it appears as an ELD in one of the audio cards.

It would help to add more debug information to eld_entry_is_igt and eld_is_valid.

The tests tries to check if we have valid eld in sound systems /proc/asound/HDMI. Some monitors doesn't support audio so the test first reads EDID from the monitors and then forces audio support on that EDID and computes a new checksum. Finally, this EDID is forced to use. 

The display connector is forced on and the test checks afterwards if we have valid eld on /proc/asound/HDMI/eld#0.0.

This test seems to fail because we are not able to register pins such as

snd_hda_codec_hdmi hdaudioC0D2: HDMI: pin nid 5 not registered

This was tested today with ICL-Y and HDMI with monitor and sound was heard, I propose dropping priority and focus on fix the test proper.

A CI Bug Log filter associated to this bug has been updated:

{- all machines: igt@kms_hdmi_inject@inject-audio - fail - Failed assertion: eld_is_valid() -}
{+ all machines: igt@kms_hdmi_inject@inject-audio - fail - Failed assertion: eld_has_igt() +}


  No new failures caught with the new filter

*** Bug 110842 has been marked as a duplicate of this bug. ***

The CI Bug Log issue associated to this bug has been updated.

### New filters associated

* all machines: igt@kms_hdmi_inject@inject-audio - fail - Failed assertion: eld_is_valid()
  - https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_301/fi-bsw-n3050/igt@kms_hdmi_inject@inject-audio.html
  - https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_301/fi-icl-y/igt@kms_hdmi_inject@inject-audio.html
  - https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_301/fi-ivb-3770/igt@kms_hdmi_inject@inject-audio.html
  - https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_301/fi-kbl-x1275/igt@kms_hdmi_inject@inject-audio.html
  - https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_301/fi-skl-guc/igt@kms_hdmi_inject@inject-audio.html
  - https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_301/fi-skl-gvtdvm/igt@kms_hdmi_inject@inject-audio.html

https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_301/fi-bsw-n3050/igt@kms_hdmi_inject@inject-audio.html
Starting subtest: inject-audio
(kms_hdmi_inject:1429) CRITICAL: Test assertion failure function hdmi_inject_audio, file ../tests/kms_hdmi_inject.c:255:
(kms_hdmi_inject:1429) CRITICAL: Failed assertion: eld_is_valid()
(kms_hdmi_inject:1429) CRITICAL: Last errno: 2, No such file or directory
Subtest inject-audio failed.

Mika, Simon, there is now audio tests in Chamelium. Is this test anymore valid?

Yes, these tests are still useful. They don't require a Chamelium device and are faster so they can be run more often.

At the moment the test is not very useful. We can play audio, for example, with aplay but the test is unable to find a valid ELD.

(In reply to Mika Kahola from comment #30)
> At the moment the test is not very useful. We can play audio, for example,
> with aplay but the test is unable to find a valid ELD.

I'm not sure I understand.

The test checks that ALSA correctly parses EDIDs that advertise audio support. What do you mean by "We can play audio, for example, with aplay"? Playing audio during the injection test isn't very helpful, because we have no way to know it works. Playing audio on a disconnected connector just discards the signal.

I mean we can play audio with aplay if we have such a monitor connected that has speakers. With the same setup the IGT test case 'kms_hdmi_inject --run-subtest inject-audio' fails. Based on this it seems that parsing EDID is erroneous on the IGT test.

(In reply to Mika Kahola from comment #32)
> I mean we can play audio with aplay if we have such a monitor connected that
> has speakers. With the same setup the IGT test case 'kms_hdmi_inject
> --run-subtest inject-audio' fails. Based on this it seems that parsing EDID
> is erroneous on the IGT test.

I don't think so, because it just werks™ on some other hardware.

(Additionally, I've been using the exact same EDID for playing audio with the Chamelium, and it also works.)

I forgot to mention that this was discovered, when we were looking audio issues. The first thought, of course, was that there really is an audio issue. This turned out to be not true as aplay can play audio just fine. Therefore, I think this is more a test issue.

Agreed, this is a test issue.

The CI Bug Log issue associated to this bug has been updated.

### Removed filters

* all machines: igt@kms_hdmi_inject@inject-audio - fail - Failed assertion: eld_is_valid() (added on 2 months, 1 week ago)

A CI Bug Log filter associated to this bug has been updated:

{- all machines: igt@kms_hdmi_inject@inject-audio - fail - Failed assertion: eld_has_igt() -}
{+ all machines: igt@kms_hdmi_inject@inject-audio - fail - Found zero ELDs +}


  No new failures caught with the new filter

This issue is still hit daily. Updated the filters to remove the one matching old logs (not hit for 2 months).

Interesting excerpts from the (improved) logs: below. It seems that the DRM side correctly recognizes the monitor, but the ALSA side doesn't expose the ELD correctly.

(kms_hdmi_inject:3938) igt_eld-DEBUG: Found zero ELDs

<7> [1480.969399] [drm:drm_detect_monitor_audio] Monitor has basic audio support
<7> [1480.969406] [drm:drm_add_edid_modes] ELD monitor IGT

<7> [1480.982109] [drm:intel_dump_pipe_config [i915]] audio: 1, infoframes: 1, infoframes enabled: 0x19

<7> [1481.003824] [drm:intel_audio_codec_enable [i915]] ELD on [CONNECTOR:128:HDMI-A-1], [ENCODER:127:DDI B]
<7> [1481.003893] [drm:hsw_audio_codec_enable [i915]] Enable audio codec on transcoder A, 28 bytes ELD
<7> [1481.003990] [drm:audio_config_hdmi_pixel_clock [i915]] Configuring HDMI audio for pixel clock 148500 (0x00090000)
<7> [1481.004058] [drm:hsw_audio_config_update [i915]] using automatic N

The CI Bug Log issue associated to this bug has been updated.

### New filters associated

* BSW SKL: igt@kms_hdmi_inject@inject-audio - fail - Monitor not present in ELD: /proc/asound/card0/eld#2.0
  - https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_345/fi-bsw-n3050/igt@kms_hdmi_inject@inject-audio.html
  - https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_345/fi-skl-6700k2/igt@kms_hdmi_inject@inject-audio.html
  - https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_344/fi-bsw-n3050/igt@kms_hdmi_inject@inject-audio.html
  - https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_344/fi-skl-6700k2/igt@kms_hdmi_inject@inject-audio.html
  - https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_344/fi-skl-guc/igt@kms_hdmi_inject@inject-audio.html
  - https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_346/fi-bsw-n3050/igt@kms_hdmi_inject@inject-audio.html
  - https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_346/fi-skl-6700k2/igt@kms_hdmi_inject@inject-audio.html

(In reply to CI Bug Log from comment #40)
> The CI Bug Log issue associated to this bug has been updated.
> 
> ### New filters associated
> 
> * BSW SKL: igt@kms_hdmi_inject@inject-audio - fail - Monitor not present in
> ELD: /proc/asound/card0/eld#2.0
>   -
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_345/fi-bsw-n3050/
> igt@kms_hdmi_inject@inject-audio.html
>   -
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_345/fi-skl-6700k2/
> igt@kms_hdmi_inject@inject-audio.html
>   -
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_344/fi-bsw-n3050/
> igt@kms_hdmi_inject@inject-audio.html
>   -
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_344/fi-skl-6700k2/
> igt@kms_hdmi_inject@inject-audio.html
>   -
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_344/fi-skl-guc/
> igt@kms_hdmi_inject@inject-audio.html
>   -
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_346/fi-bsw-n3050/
> igt@kms_hdmi_inject@inject-audio.html
>   -
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_346/fi-skl-6700k2/
> igt@kms_hdmi_inject@inject-audio.html

@Simon, I believe these failures belongs to this bug. Let me know if this has to be reported separately.

Quick view of all failures: http://gfx-ci.fi.intel.com/cibuglog-ng/results/all?query=test_name+%3D+%27igt%40kms_hdmi_inject%40inject-audio%27+AND+status_name+NOT+IN+%5B%27skip%27%2C+%27pass%27%2C+%27notrun%27%5D

Seems like there are multiple bugs entangled here.

I don't know why, but it seems some DUTs don't have an audio card. Is this expected?

testrunner@ivb-3770:~$ ls -l /proc/asound/
total 0
-r--r--r-- 1 root root 0 syys  11 11:25 cards
-r--r--r-- 1 root root 0 syys  11 11:25 devices
-r--r--r-- 1 root root 0 syys  11 11:25 hwdep
-r--r--r-- 1 root root 0 syys  11 11:25 pcm
dr-xr-xr-x 2 root root 0 syys  11 11:25 seq
-r--r--r-- 1 root root 0 syys  11 11:25 timers
-r--r--r-- 1 root root 0 syys  11 11:25 version
testrunner@ivb-3770:~$ cat /proc/asound/cards
--- no soundcards ---
testrunner@ivb-3770:~$ lsmod                 
Module                  Size  Used by
amdgpu               4579328  0
gpu_sched              36864  1 amdgpu
ttm                   122880  1 amdgpu
vgem                   16384  0
snd_hda_codec_realtek   126976  0
snd_hda_codec_generic    90112  1 snd_hda_codec_realtek
x86_pkg_temp_thermal    20480  0
coretemp               20480  0
mei_hdcp               24576  0
crct10dif_pclmul       16384  1
crc32_pclmul           16384  0
ghash_clmulni_intel    16384  0
snd_intel_nhlt         20480  0
snd_hda_codec         135168  2 snd_hda_codec_generic,snd_hda_codec_realtek
snd_hwdep              16384  1 snd_hda_codec
r8169                  90112  0
snd_hda_core           90112  3 snd_hda_codec_generic,snd_hda_codec,snd_hda_codec_realtek
realtek                20480  1
mei_me                 53248  1
snd_pcm               122880  2 snd_hda_codec,snd_hda_core
mei                   118784  3 mei_hdcp,mei_me
lpc_ich                28672  0
prime_numbers          16384  0

Another one:

testrunner@bdw-gvtdvm:~$ ls -l /proc/asound/
total 0
-r--r--r-- 1 root root 0 Sep 11 16:24 cards
-r--r--r-- 1 root root 0 Sep 11 16:24 devices
-r--r--r-- 1 root root 0 Sep 11 16:24 hwdep
-r--r--r-- 1 root root 0 Sep 11 16:24 pcm
dr-xr-xr-x 2 root root 0 Sep 11 16:24 seq
-r--r--r-- 1 root root 0 Sep 11 16:24 timers
-r--r--r-- 1 root root 0 Sep 11 16:24 version
testrunner@bdw-gvtdvm:~$ cat /proc/asound/cards
--- no soundcards ---
testrunner@bdw-gvtdvm:~$ lsmod
Module                  Size  Used by
amdgpu               4579328  0
gpu_sched              36864  1 amdgpu
ttm                   122880  1 amdgpu
snd_intel_nhlt         20480  0
snd_hda_codec         135168  0
snd_hwdep              16384  1 snd_hda_codec
snd_hda_core           90112  1 snd_hda_codec
snd_pcm               122880  2 snd_hda_codec,snd_hda_core
vgem                   16384  0
crct10dif_pclmul       16384  1
crc32_pclmul           16384  0
ghash_clmulni_intel    16384  0
e1000                 151552  0
prime_numbers          16384  0
i2c_piix4              28672  0

https://patchwork.freedesktop.org/series/66536/

commit 3374cd0b048f9c277b2815bf80502f9f89680176
Author: Simon Ser <simon.ser@intel.com>
Date:   Wed Sep 11 15:57:55 2019 +0300

    lib/igt_eld: introduce eld_is_supported
    
    We've seen cases in which /proc/asound doesn't exist (e.g. with the new SOF
    framework). We've also seen cases in which no soundcard is exposed by ALSA (see
    bugzilla link). Last, some audio drivers din't support ELDs (non-Intel
    drivers). In all of these cases, skipping the tests depending on ELD support
    makes more sense and makes it clearer what happens.
    
    v2: also check that the driver supports ELDs entries in procfs
    
    Signed-off-by: Simon Ser <simon.ser@intel.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102370
    Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Bleh, messed up my patch. Here's a fix: https://patchwork.freedesktop.org/series/66649/

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/3.