[alsa-devel] [PATCH] ALSA: hda - Avoid outputting HDMI audio before prepare() and after close()

[Takashi Iwai]

At Sun, 17 Feb 2013 19:57:57 +0200,
Anssi Hannula wrote:
> 
> On Sunday, February 17, 2013, Takashi Iwai <tiwai at suse.de> wrote:
> > At Sun, 17 Feb 2013 14:23:21 +0200,
> > Anssi Hannula wrote:
> >>
> >> 17.02.2013 10:24, Takashi Iwai kirjoitti:
> >> > At Sat, 16 Feb 2013 22:59:40 +0200,
> >> > Anssi Hannula wrote:
> >> >>
> >> >> 16.02.2013 18:40, Anssi Hannula kirjoitti:
> >> >>> Some HDMI codecs (at least NVIDIA 0x10de000b:0x10de0101:0x100100)
> start
> >> >>> transmitting an empty audio stream as soon as PIN_OUT and
> AC_DIG1_ENABLE
> >> >>> are enabled.
> >> >>>
> >> >>> Since commit 6169b673618bf0b2518ce413b54925782a603f06 ("ALSA: hda -
> >> >>> Always turn on pins for HDMI/DP") this happens at first open() time,
> and
> >> >>> will continue even after close().
> >> >>>
> >> >>> Additionally, some codecs (at least Intel PantherPoint HDMI)
> currently
> >> >>> continue transmitting HDMI audio even after close() in case some
> actual
> >> >>> audio was output after open() (this happens regardless of PIN_OUT).
> >> >>>
> >> >>> Empty HDMI audio transmission when not intended has the effect that a
> >> >>> possible HDMI audio sink/receiver may prefer the empty HDMI audio
> stream
> >> >>> over an actual audio stream on its S/PDIF inputs.
> >> >>>
> >> >>> To avoid the issue before first prepare(), set stream format to 0 on
> >> >>> codec initialization. 0 is not a valid format value for HDMI and will
> >> >>> prevent the audio stream from being output.
> >> >>>
> >> >>> Additionally, at close() time, make sure that the stream is cleaned
> up.
> >> >>> This will ensure that the format is reset to 0 at that time,
> preventing
> >> >>> audio from being output in that case.
> >> >>>
> >> >>> Thanks to OpenELEC developers and users for their help in
> investigating
> >> >>> this issue on the affected NVIDIA "ION2" hardware, and for the
> initial
> >> >>> bug report of the issue. Testing of the final version on NVIDIA ION2
> was
> >> >>> done by OpenELEC user "MrXIII". Testing on Intel PantherPoint was
> done
> >> >>> by myself.
> >> >>>
> >> >>> Signed-off-by: Anssi Hannula <anssi.hannula at iki.fi>
> >> >>> Cc: stable at vger.kernel.org
> >> >>> ---
> >> >>>
> >> >>> This also supersedes the patch I attached yesterday as it fixes both
> >> >>> cases.
> >> >>>
> >> >>> I guess the alternative to this approach would be to fiddle with
> >> >>> AC_DIG1_ENABLE when preparing and closing the device, but with a
> quick
> >> >>> look that seemed to be possibly more complex since AC_DIG1_ENABLE is
> >> >>> already meddled with at quite a few places in hda_codec.c.
> >> >>
> >> >> Hmm... actually, that would not be so much more complex, just making
> >> >> sure AC_DIG1_ENABLE is off in hdmi_add_cvt() (by e.g. zeroing
> >> >> AC_VERB_SET_DIGI_CONVERT_1) and making snd_hda_spdif_ctls_unassign()
> >> >> call "set_spdif_ctls(codec, spdif->nid, 0, -1);".
> >> >>
> >> >> However, it would still mean that just a simple snd_pcm_open() could
> >> >> cause an empty stream to be output (if format is valid), since iec958
> >> >> controls are assigned at open() time, and iec958 mute controls
> >> >> AC_DIG1_ENABLE. Or we could add additional code in hda_codec.c to
> >> >> prevent AC_DIG1_ENABLE being set if prepare() has not been called.
> >> >
> >> > Is the problem fixed if you set codec->no_sticky_stream = 1?
> >>
> >> The issue on NVIDIA reported by users, no, that flag is not used in a
> >> simple open()+close() path.
> >
> > It is.  The flag is used in hda_codec.c.  It's a flag for setting
> > FORMAT stuff, not the SPDIF status.
> 
> The only use I see there is in __snd_hda_codec_cleanup_stream(), but AFAICS
> that is not called by open/close callbacks of hdmi, since the stream is
> only allocated in prepare(). Am I missing something?

It changes the behavior of cleanup.  The cleanup is implicitly called
before close via hw_free.

When cleanup callback isn't defined, snd_hda_codec_cleanup_stream() is
invoked as default.  And no_sticky_stream flag is evaluated there.
Hence, it must influence on simple_hdmi case, too.

> 
> >> My issue on Intel, yes.
> >>
> >> > Actually, the current behavior is intentional.  There have been bug
> >> > reports that just reopening a device causes the receiver not reacting
> >> > immediately.  In the worst case, it takes a few seconds to sync. So we
> >> > introduced a mechanism to keep the PCM stream assignment.
> >>
> >> Hmm.. If the intention is to keep a silent stream playing even after
> >> device has been closed, why was PIN_OUT disabled on close until "ALSA:
> >> hda - Always turn on pins for HDMI/DP" in December?
> >
> > The intention isn't to keep the stream playing, but it keeps the
> > STREAM tag.  It's just a weird hardware implementation that it keeps
> > playing.  And, the feature was implemented far before the new dynamic
> > SPDIF status control assignment and the dynamic pin down.  So, it got
> > broken by that stuff sometime ago.
> 
> Ah, so you mean something different with „lost sync„, I used that
> interchangeably with „hdmi stream stopped„.  Can you explain the
> difference, to get us on the same page? :)

Some receivers seem to keep sync even if you stop the stream as long
as the FORMAT tag isn't changed.  That's the only point of the sticky
PCM stream.  It's not intended to keep the stream running.

I'd say it's rather a bug of hardware or driver doing that.  Maybe
it's just for avoiding the out-of-sync situation.

What makes harder in the case of HDMI/DP, it's not only about the
hardware but related also with the video driver.  I won't be surprised
if different behavior is seen between the open-source and binary-only
drivers, or between different driver versions.  And, plus, the
behavior depends on the receiver, too.

So, we always need to think of three things if we debug this kind of
problems:
- HDMI audio codec
- Video driver, version
- Receiver hardware


Takashi