[PATCH] conf: HdmiLpeAudio: remove the "front" pcm definition

[PATCH] conf: HdmiLpeAudio: remove the "front" pcm definition

[Tanu Kaskinen]

PulseAudio assumes that the "front" pcm device always refers to an
analog device, not HDMI. While that assumption is not really valid, the
reality is that without that assumption PulseAudio can't know whether
"front" and "hdmi" refer to a different or the same device.

The HDMI LPE driver doesn't allow audio streaming while the HDMI cable
is unplugged, so PulseAudio has to know when it's plugged in and when
it's not. If both "front" and "hdmi" devices exist, PulseAudio will
notice that HDMI is unplugged, but it doesn't know that "front" refers
to the same device, and PulseAudio will try to use the "front" device
with bad consequences. The kernel driver's refusal to stream any audio
makes PulseAudio enter an infinite loop and then the kernel kills
PulseAudio, because it consumes too much CPU time in a realtime thread.

While the looping in PulseAudio could probably be fixed, that wouldn't
change the fact that PulseAudio thinks that there is an analog device. I
believe it's best to avoid having the same device as both "front" and
"hdmi" in alsa-lib.

I removed also the surround configuration includes. I don't think they
had any effect anyway, so I wonder why they were there in the first
place.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=100488
---
 src/conf/cards/HdmiLpeAudio.conf | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/src/conf/cards/HdmiLpeAudio.conf b/src/conf/cards/HdmiLpeAudio.conf
index 9fa30da0..a1e493da 100644
--- a/src/conf/cards/HdmiLpeAudio.conf
+++ b/src/conf/cards/HdmiLpeAudio.conf
@@ -2,30 +2,6 @@
 # Configuration for the Intel HDMI/DP LPE audio
 #
 
-<confdir:pcm/front.conf>
-
-HdmiLpeAudio.pcm.front.0 {
-	@args [ CARD ]
-	@args.CARD {
-		type string
-	}
-	type softvol
-	slave.pcm {
-		type hw
-		card $CARD
-	}
-	control {
-		name "PCM Playback Volume"
-		card $CARD
-	}
-}
-
-<confdir:pcm/surround40.conf>
-<confdir:pcm/surround21.conf>
-<confdir:pcm/surround41.conf>
-<confdir:pcm/surround50.conf>
-<confdir:pcm/surround51.conf>
-
 <confdir:pcm/hdmi.conf>
 
 HdmiLpeAudio.pcm.hdmi.0 {
-- 
2.14.2

Re: [PATCH] conf: HdmiLpeAudio: remove the "front" pcm definition

[Takashi Iwai]

On Wed, 04 Oct 2017 21:44:00 +0200,
Tanu Kaskinen wrote:
> 
> PulseAudio assumes that the "front" pcm device always refers to an
> analog device, not HDMI. While that assumption is not really valid, the
> reality is that without that assumption PulseAudio can't know whether
> "front" and "hdmi" refer to a different or the same device.
> 
> The HDMI LPE driver doesn't allow audio streaming while the HDMI cable
> is unplugged, so PulseAudio has to know when it's plugged in and when
> it's not. If both "front" and "hdmi" devices exist, PulseAudio will
> notice that HDMI is unplugged, but it doesn't know that "front" refers
> to the same device, and PulseAudio will try to use the "front" device
> with bad consequences. The kernel driver's refusal to stream any audio
> makes PulseAudio enter an infinite loop and then the kernel kills
> PulseAudio, because it consumes too much CPU time in a realtime thread.
> 
> While the looping in PulseAudio could probably be fixed, that wouldn't
> change the fact that PulseAudio thinks that there is an analog device. I
> believe it's best to avoid having the same device as both "front" and
> "hdmi" in alsa-lib.
> 
> I removed also the surround configuration includes. I don't think they
> had any effect anyway, so I wonder why they were there in the first
> place.

These definitions are there for LPCM surround channels.

The biggest problem behind the scene is that we mixed up both logical
and physical configurations.  The "front" or "surround51" belong to
the former, while "hdmi" belongs to the latter.

In the past, there was only SPDIF that has only two stereo channels,
so the logical configuration was fixed with that.  We copied that
config for HDMI/DP, and now we hit the inconsistency more clearly...

I suppose the PA issue gets fixed just by this removal?  If yes, I'm
willing to apply it, including in the next release.


thanks,

Takashi

> 
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=100488
> ---
>  src/conf/cards/HdmiLpeAudio.conf | 24 ------------------------
>  1 file changed, 24 deletions(-)
> 
> diff --git a/src/conf/cards/HdmiLpeAudio.conf b/src/conf/cards/HdmiLpeAudio.conf
> index 9fa30da0..a1e493da 100644
> --- a/src/conf/cards/HdmiLpeAudio.conf
> +++ b/src/conf/cards/HdmiLpeAudio.conf
> @@ -2,30 +2,6 @@
>  # Configuration for the Intel HDMI/DP LPE audio
>  #
>  
> -<confdir:pcm/front.conf>
> -
> -HdmiLpeAudio.pcm.front.0 {
> -	@args [ CARD ]
> -	@args.CARD {
> -		type string
> -	}
> -	type softvol
> -	slave.pcm {
> -		type hw
> -		card $CARD
> -	}
> -	control {
> -		name "PCM Playback Volume"
> -		card $CARD
> -	}
> -}
> -
> -<confdir:pcm/surround40.conf>
> -<confdir:pcm/surround21.conf>
> -<confdir:pcm/surround41.conf>
> -<confdir:pcm/surround50.conf>
> -<confdir:pcm/surround51.conf>
> -
>  <confdir:pcm/hdmi.conf>
>  
>  HdmiLpeAudio.pcm.hdmi.0 {
> -- 
> 2.14.2
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH] conf: HdmiLpeAudio: remove the "front" pcm definition

[Tanu Kaskinen]

On Thu, 2017-10-05 at 08:38 +0200, Takashi Iwai wrote:
> On Wed, 04 Oct 2017 21:44:00 +0200,
> Tanu Kaskinen wrote:
> > 
> > PulseAudio assumes that the "front" pcm device always refers to an
> > analog device, not HDMI. While that assumption is not really valid, the
> > reality is that without that assumption PulseAudio can't know whether
> > "front" and "hdmi" refer to a different or the same device.
> > 
> > The HDMI LPE driver doesn't allow audio streaming while the HDMI cable
> > is unplugged, so PulseAudio has to know when it's plugged in and when
> > it's not. If both "front" and "hdmi" devices exist, PulseAudio will
> > notice that HDMI is unplugged, but it doesn't know that "front" refers
> > to the same device, and PulseAudio will try to use the "front" device
> > with bad consequences. The kernel driver's refusal to stream any audio
> > makes PulseAudio enter an infinite loop and then the kernel kills
> > PulseAudio, because it consumes too much CPU time in a realtime thread.
> > 
> > While the looping in PulseAudio could probably be fixed, that wouldn't
> > change the fact that PulseAudio thinks that there is an analog device. I
> > believe it's best to avoid having the same device as both "front" and
> > "hdmi" in alsa-lib.
> > 
> > I removed also the surround configuration includes. I don't think they
> > had any effect anyway, so I wonder why they were there in the first
> > place.
> 
> These definitions are there for LPCM surround channels.

Are they supposed to do something useful? When PulseAudio probes the
surround40:0 device, opening the device fails:

alsa-util.c: Trying surround40:0 with SND_PCM_NO_AUTO_FORMAT ...
(alsa-lib)confmisc.c: Unable to find definition 'cards.HdmiLpeAudio.pcm.surround40.0:CARD=0'
(alsa-lib)conf.c: function snd_func_refer returned error: No such file or directory
(alsa-lib)conf.c: Evaluate error: No such file or directory
(alsa-lib)pcm.c: Unknown PCM surround40:0

> The biggest problem behind the scene is that we mixed up both logical
> and physical configurations.  The "front" or "surround51" belong to
> the former, while "hdmi" belongs to the latter.
> 
> In the past, there was only SPDIF that has only two stereo channels,
> so the logical configuration was fixed with that.  We copied that
> config for HDMI/DP, and now we hit the inconsistency more clearly...
> 
> I suppose the PA issue gets fixed just by this removal?  If yes, I'm
> willing to apply it, including in the next release.

A user reported that removing the "front" definition fixes the PA
issue, together with jack detection patches for PA that haven't been
merged yet.

-- 
Tanu

https://www.patreon.com/tanuk

Re: [PATCH] conf: HdmiLpeAudio: remove the "front" pcm definition

[Takashi Iwai]

On Thu, 05 Oct 2017 17:10:11 +0200,
Tanu Kaskinen wrote:
> 
> On Thu, 2017-10-05 at 08:38 +0200, Takashi Iwai wrote:
> > On Wed, 04 Oct 2017 21:44:00 +0200,
> > Tanu Kaskinen wrote:
> > > 
> > > PulseAudio assumes that the "front" pcm device always refers to an
> > > analog device, not HDMI. While that assumption is not really valid, the
> > > reality is that without that assumption PulseAudio can't know whether
> > > "front" and "hdmi" refer to a different or the same device.
> > > 
> > > The HDMI LPE driver doesn't allow audio streaming while the HDMI cable
> > > is unplugged, so PulseAudio has to know when it's plugged in and when
> > > it's not. If both "front" and "hdmi" devices exist, PulseAudio will
> > > notice that HDMI is unplugged, but it doesn't know that "front" refers
> > > to the same device, and PulseAudio will try to use the "front" device
> > > with bad consequences. The kernel driver's refusal to stream any audio
> > > makes PulseAudio enter an infinite loop and then the kernel kills
> > > PulseAudio, because it consumes too much CPU time in a realtime thread.
> > > 
> > > While the looping in PulseAudio could probably be fixed, that wouldn't
> > > change the fact that PulseAudio thinks that there is an analog device. I
> > > believe it's best to avoid having the same device as both "front" and
> > > "hdmi" in alsa-lib.
> > > 
> > > I removed also the surround configuration includes. I don't think they
> > > had any effect anyway, so I wonder why they were there in the first
> > > place.
> > 
> > These definitions are there for LPCM surround channels.
> 
> Are they supposed to do something useful? When PulseAudio probes the
> surround40:0 device, opening the device fails:
> 
> alsa-util.c: Trying surround40:0 with SND_PCM_NO_AUTO_FORMAT ...
> (alsa-lib)confmisc.c: Unable to find definition 'cards.HdmiLpeAudio.pcm.surround40.0:CARD=0'
> (alsa-lib)conf.c: function snd_func_refer returned error: No such file or directory
> (alsa-lib)conf.c: Evaluate error: No such file or directory
> (alsa-lib)pcm.c: Unknown PCM surround40:0

Hm, it might be not well tested, then :)

> > The biggest problem behind the scene is that we mixed up both logical
> > and physical configurations.  The "front" or "surround51" belong to
> > the former, while "hdmi" belongs to the latter.
> > 
> > In the past, there was only SPDIF that has only two stereo channels,
> > so the logical configuration was fixed with that.  We copied that
> > config for HDMI/DP, and now we hit the inconsistency more clearly...
> > 
> > I suppose the PA issue gets fixed just by this removal?  If yes, I'm
> > willing to apply it, including in the next release.
> 
> A user reported that removing the "front" definition fixes the PA
> issue, together with jack detection patches for PA that haven't been
> merged yet.

OK, I applied the patch now.


thanks,

Takashi