[alsa-devel] [PATCH 1/2] ALSA: hda - hdmi: Set converter channel count even without sink
Since commit 1df5a06a ("ALSA: hda - hdmi: Fix programmed active channel count") channel count is no longer being set if monitor_present is 0. This is because setting the count was moved after the CA value is determined, which is only after the monitor_present check in hdmi_setup_audio_infoframe().
Unfortunately, in some cases, such as with a non-spec-compliant codec or with a problematic video driver, monitor_present is always 0. As a specific example, this seems to happen with gen1 ATV (SiI1390 codec), causing left-channel-only stereo playback (multi-channel playback has apparently never worked with this codec despite it reporting 8 channels, reason unknown).
Simply setting converter channel count without setting the pin infoframe and channel mapping as well does not theoretically make much sense as this will just mean they are out-of-sync and multichannel playback will have a wrong channel mapping.
However, adding back just setting the converter channel count even in no-monitor case is the safest change which at least fixes the stereo playback regression on SiI1390 codec. Do that.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi Reported-by: Stephan Raue stephan@openelec.tv Tested-by: Stephan Raue stephan@openelec.tv Cc: stable@vger.kernel.org # 3.12+ --- sound/pci/hda/patch_hdmi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 0cb5b89cd0c8..1edbb9c47c2d 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1127,8 +1127,10 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, AMP_OUT_UNMUTE);
eld = &per_pin->sink_eld; - if (!eld->monitor_present) + if (!eld->monitor_present) { + hdmi_set_channel_count(codec, per_pin->cvt_nid, channels); return; + }
if (!non_pcm && per_pin->chmap_set) ca = hdmi_manual_channel_allocation(channels, per_pin->chmap);
Currently infoframe contents and channel mapping are only set when a sink (monitor) is present.
However, this does not make much sense, since 1) We can make a very reasonable guess on CA after 18e391862c ("ALSA: hda - hdmi: Fallback to ALSA allocation when selecting CA") or by relying on a previously valid ELD (or we may be using a user-specified channel map). 2) Not setting infoframe contents and channel count simply means they are left at a possibly incorrect state - playback is still allowed to proceed (with missing or wrongly mapped channels).
Reasons for monitor_present being 0 include disconnected cable, video driver issues, or codec not being spec-compliant. Note that in actual disconnected-cable case it should not matter if these settings are wrong as they will be re-set after jack detection, though.
Change the behavior to allow the infoframe contents and the channel mapping to be set even without a sink/monitor, either based on the previous valid ELD contents, if any, or based on sensible defaults (standard channel layouts or provided custom map, sink type HDMI).
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi Tested-by: Stephan Raue stephan@openelec.tv ---
This simply removes the monitor_present check, which is probably more trouble than it is worth, as it may trip obscure setups even though we can tolerate !monitor_present and !eld in 99%+ of all cases by using defaults, nowadays post-18e391862c anyway. Downside is that missing monitor_present may not be noticed by the user as ~everything continues to work.
sound/pci/hda/patch_hdmi.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 1edbb9c47c2d..016f785cdf45 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1127,10 +1127,6 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, AMP_OUT_UNMUTE);
eld = &per_pin->sink_eld; - if (!eld->monitor_present) { - hdmi_set_channel_count(codec, per_pin->cvt_nid, channels); - return; - }
if (!non_pcm && per_pin->chmap_set) ca = hdmi_manual_channel_allocation(channels, per_pin->chmap);
At Mon, 5 May 2014 02:38:44 +0300, Anssi Hannula wrote:
Currently infoframe contents and channel mapping are only set when a sink (monitor) is present.
However, this does not make much sense, since
- We can make a very reasonable guess on CA after 18e391862c ("ALSA: hda - hdmi: Fallback to ALSA allocation when selecting CA") or by relying on a previously valid ELD (or we may be using a user-specified channel map).
- Not setting infoframe contents and channel count simply means they are left at a possibly incorrect state - playback is still allowed to proceed (with missing or wrongly mapped channels).
Reasons for monitor_present being 0 include disconnected cable, video driver issues, or codec not being spec-compliant. Note that in actual disconnected-cable case it should not matter if these settings are wrong as they will be re-set after jack detection, though.
Change the behavior to allow the infoframe contents and the channel mapping to be set even without a sink/monitor, either based on the previous valid ELD contents, if any, or based on sensible defaults (standard channel layouts or provided custom map, sink type HDMI).
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi Tested-by: Stephan Raue stephan@openelec.tv
This simply removes the monitor_present check, which is probably more trouble than it is worth, as it may trip obscure setups even though we can tolerate !monitor_present and !eld in 99%+ of all cases by using defaults, nowadays post-18e391862c anyway. Downside is that missing monitor_present may not be noticed by the user as ~everything continues to work.
Is this considered as a "fix" for 3.15 kernel? If so, I find it a bit distracting to queue the previous patch for stable while we're removing the whole things here later.
If this is intended for testing for 3.16 kernel, I'm fine to take as is.
thanks,
Takashi
sound/pci/hda/patch_hdmi.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 1edbb9c47c2d..016f785cdf45 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1127,10 +1127,6 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, AMP_OUT_UNMUTE);
eld = &per_pin->sink_eld;
if (!eld->monitor_present) {
hdmi_set_channel_count(codec, per_pin->cvt_nid, channels);return;}
if (!non_pcm && per_pin->chmap_set) ca = hdmi_manual_channel_allocation(channels, per_pin->chmap);
-- 1.8.4.5
Takashi Iwai kirjoitti 2014-05-05 17:34:
At Mon, 5 May 2014 02:38:44 +0300, Anssi Hannula wrote:
Currently infoframe contents and channel mapping are only set when a sink (monitor) is present.
However, this does not make much sense, since
- We can make a very reasonable guess on CA after 18e391862c ("ALSA: hda - hdmi: Fallback to ALSA allocation when selecting CA") or by relying on a previously valid ELD (or we may be using a user-specified channel map).
- Not setting infoframe contents and channel count simply means they are left at a possibly incorrect state - playback is still allowed to proceed (with missing or wrongly mapped channels).
Reasons for monitor_present being 0 include disconnected cable, video driver issues, or codec not being spec-compliant. Note that in actual disconnected-cable case it should not matter if these settings are wrong as they will be re-set after jack detection, though.
Change the behavior to allow the infoframe contents and the channel mapping to be set even without a sink/monitor, either based on the previous valid ELD contents, if any, or based on sensible defaults (standard channel layouts or provided custom map, sink type HDMI).
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi Tested-by: Stephan Raue stephan@openelec.tv
This simply removes the monitor_present check, which is probably more trouble than it is worth, as it may trip obscure setups even though we can tolerate !monitor_present and !eld in 99%+ of all cases by using defaults, nowadays post-18e391862c anyway. Downside is that missing monitor_present may not be noticed by the user as ~everything continues to work.
Is this considered as a "fix" for 3.15 kernel? If so, I find it a bit distracting to queue the previous patch for stable while we're removing the whole things here later.
If this is intended for testing for 3.16 kernel, I'm fine to take as is.
Well, I intended it for 3.16, since I don't have anything concrete that it would fix that the previous AFAICS safer patch did not (for now, at least...).
Sorry for not making it explicit.
thanks,
Takashi
sound/pci/hda/patch_hdmi.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 1edbb9c47c2d..016f785cdf45 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1127,10 +1127,6 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, AMP_OUT_UNMUTE);
eld = &per_pin->sink_eld;
if (!eld->monitor_present) {
hdmi_set_channel_count(codec, per_pin->cvt_nid, channels);return;}
if (!non_pcm && per_pin->chmap_set) ca = hdmi_manual_channel_allocation(channels, per_pin->chmap);
-- 1.8.4.5
At Mon, 05 May 2014 17:48:04 +0300, Anssi Hannula wrote:
Takashi Iwai kirjoitti 2014-05-05 17:34:
At Mon, 5 May 2014 02:38:44 +0300, Anssi Hannula wrote:
Currently infoframe contents and channel mapping are only set when a sink (monitor) is present.
However, this does not make much sense, since
- We can make a very reasonable guess on CA after 18e391862c ("ALSA: hda - hdmi: Fallback to ALSA allocation when selecting CA") or by relying on a previously valid ELD (or we may be using a user-specified channel map).
- Not setting infoframe contents and channel count simply means they are left at a possibly incorrect state - playback is still allowed to proceed (with missing or wrongly mapped channels).
Reasons for monitor_present being 0 include disconnected cable, video driver issues, or codec not being spec-compliant. Note that in actual disconnected-cable case it should not matter if these settings are wrong as they will be re-set after jack detection, though.
Change the behavior to allow the infoframe contents and the channel mapping to be set even without a sink/monitor, either based on the previous valid ELD contents, if any, or based on sensible defaults (standard channel layouts or provided custom map, sink type HDMI).
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi Tested-by: Stephan Raue stephan@openelec.tv
This simply removes the monitor_present check, which is probably more trouble than it is worth, as it may trip obscure setups even though we can tolerate !monitor_present and !eld in 99%+ of all cases by using defaults, nowadays post-18e391862c anyway. Downside is that missing monitor_present may not be noticed by the user as ~everything continues to work.
Is this considered as a "fix" for 3.15 kernel? If so, I find it a bit distracting to queue the previous patch for stable while we're removing the whole things here later.
If this is intended for testing for 3.16 kernel, I'm fine to take as is.
Well, I intended it for 3.16, since I don't have anything concrete that it would fix that the previous AFAICS safer patch did not (for now, at least...).
Sorry for not making it explicit.
Alright, I applied both, one to for-linus and another to for-next branches. Thanks.
Takashi
participants (2)
-
Anssi Hannula -
Takashi Iwai