[alsa-devel] [PATCH 0/7] ALSA: hda - hdmi: Various channel mapping fixes
Hi!
Apparently there were several more bugs in the HDMI channel mapping code, so here you go.
I haven't marked any for stable since these don't really affect the regular layouts that much and the patches aren't that trivial. If you want to pick some for stable, my suggestions are 1, 5, 6, maybe 3 (those I initially marked for stable until I changed my mind).
Anssi Hannula (7): ALSA: hda - hdmi: Fix reported channel map on common default layouts ALSA: hda - hdmi: Fix incorrect default channel mapping for unusual CAs ALSA: hda - hdmi: Fix programmed active channel count ALSA: hda - hdmi: Fix unused slots being enabled in manual and non-PCM mappings ALSA: hda - hdmi: Fix channel maps with less common speakers ALSA: hda - hdmi: Fix available channel maps missing from TLV ALSA: hda - hdmi: Tweak debug messages to be more useful
sound/pci/hda/patch_hdmi.c | 146 +++++++++++++++++++++-------------- 1 file changed, 90 insertions(+), 56 deletions(-)
-- Anssi Hannula
hdmi_setup_fake_chmap() is supposed to set the reported channel map when the channel map is not specified by the user.
However, the function indexes channel_allocations[] with a wrong value and extracts the wrong nibble from hdmi_channel_mapping[], causing wrong channel maps to be shown.
Fix those issues.
Tested on Intel HDMI to correctly generate various channel maps, for example 3,4,14,15,7,8,5,6 (instead of incorrect 3,4,8,7,5,6,14,0) for standard 7.1 channel audio. (Note that the side and rear channels are reported as RL/RR and RLC/RRC, respectively, as per the CEA-861 standard, instead of the more traditional SL/SR and RL/RR.)
Note that this only fixes the layouts that only contain traditional 7.1 speakers (2.0, 2.1, 4.0, 5.1, 7.1, etc.). E.g. the rear center of 6.1 is still being shown wrongly due to an issue with from_cea_slot() which will be fixed in a later patch.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi ---
I'm assuming the usage of RL/RR and RLC/RRC instead of SL/SR and RL/RR is intentional. If not, everything needs to be rethinked.
sound/pci/hda/patch_hdmi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 09f38a1..6ed34df 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -742,9 +742,10 @@ static int hdmi_manual_setup_channel_mapping(struct hda_codec *codec, static void hdmi_setup_fake_chmap(unsigned char *map, int ca) { int i; + int ordered_ca = get_channel_allocation_order(ca); for (i = 0; i < 8; i++) { - if (i < channel_allocations[ca].channels) - map[i] = from_cea_slot((hdmi_channel_mapping[ca][i] >> 4) & 0x0f); + if (i < channel_allocations[ordered_ca].channels) + map[i] = from_cea_slot(hdmi_channel_mapping[ca][i] & 0x0f); else map[i] = 0; }
hdmi_std_setup_channel_mapping() selects a Channel Allocation according to the sink reported speaker mask, preferring the ALSA standard layouts.
If the channel allocation is not one of the ALSA standard layouts, the ALSA channels are mapped directly to HDMI channels in order. However, the function does not take into account that there a holes in the HDMI channel map.
Additionally, the function tries to disable a slot by using AC_VERB_SET_CHAN_SLOT with parameter ((alsa_ch << 8) | 0xf), while the correct parameter is ((0xf << 8) | hdmi_slot), i.e. the slot should be unassigned, not the ALSA channel.
Fix both of the issues for non-ALSA-default layouts.
Tested on Intel HDMI with a speaker mask of FL | FR | FC | RC, which causes CA 0x06 to be selected for 4-channel audio, which causes incorrect output (sound destined to RC goes to FC and FC goes nowhere) without the patch.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi --- sound/pci/hda/patch_hdmi.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 6ed34df..5244802 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -597,22 +597,32 @@ static void hdmi_std_setup_channel_mapping(struct hda_codec *codec, bool non_pcm, int ca) { + struct cea_channel_speaker_allocation *ch_alloc; int i; int err; int order; int non_pcm_mapping[8];
order = get_channel_allocation_order(ca); + ch_alloc = &channel_allocations[order];
if (hdmi_channel_mapping[ca][1] == 0) { - for (i = 0; i < channel_allocations[order].channels; i++) - hdmi_channel_mapping[ca][i] = i | (i << 4); - for (; i < 8; i++) - hdmi_channel_mapping[ca][i] = 0xf | (i << 4); + int hdmi_slot = 0; + /* fill actual channel mappings in ALSA channel (i) order */ + for (i = 0; i < ch_alloc->channels; i++) { + while (!ch_alloc->speakers[7 - hdmi_slot] && !WARN_ON(hdmi_slot >= 8)) + hdmi_slot++; /* skip zero slots */ + + hdmi_channel_mapping[ca][i] = (i << 4) | hdmi_slot++; + } + /* fill the rest of the slots with ALSA channel 0xf */ + for (hdmi_slot = 0; hdmi_slot < 8; hdmi_slot++) + if (!ch_alloc->speakers[7 - hdmi_slot]) + hdmi_channel_mapping[ca][i++] = (0xf << 4) | hdmi_slot; }
if (non_pcm) { - for (i = 0; i < channel_allocations[order].channels; i++) + for (i = 0; i < ch_alloc->channels; i++) non_pcm_mapping[i] = i | (i << 4); for (; i < 8; i++) non_pcm_mapping[i] = 0xf | (i << 4);
Currently the converter channel count is set to the number of actual input channels. The audio infoframe channel count field is set similarly.
However, sometimes the used channel map does not map all input channels to outputs. Notably, 3 channel modes (e.g. 2.1) require a dummy input channel so there are 4 input channels. According to the HDA specification, converter channel count should be programmed according to the number of _active_ channels.
On Intel HDMI codecs (but not on NVIDIA), setting the converter channel to a higher value than there are actually mapped channels to HDMI slots will cause no audio to be output at all.
Note that the effects of this issue are currently partially masked by other bugs that prevent the driver from actually unmapping channels in certain cases. For example, if a 4 channel stream is first created and prepared, it gets a FL,FR,RL,RR mapping (ALSA->HDMI slot mapping 0->0, 1->1, 2->4, 3->5). If one thereafter assigns a FR,FL,FC mapping to it, the driver will remap 2->3 but fail to unmap 2->4 and 3->5, so there are still 4 active channels and the issue will not trigger in this case. These bugs will be fixed separately.
Fix the channel counts in the converter channel count field and in the audio infoframe channel count field to match the actual number of active channels.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi --- sound/pci/hda/patch_hdmi.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 5244802..ac818cb 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -63,6 +63,7 @@ struct hdmi_spec_per_pin { hda_nid_t pin_nid; int num_mux_nids; hda_nid_t mux_nids[HDA_MAX_CONNECTIONS]; + hda_nid_t cvt_nid;
struct hda_codec *codec; struct hdmi_eld sink_eld; @@ -902,8 +903,9 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, { hda_nid_t pin_nid = per_pin->pin_nid; int channels = per_pin->channels; + int active_channels; struct hdmi_eld *eld; - int ca; + int ca, ordered_ca; union audio_infoframe ai;
if (!channels) @@ -925,6 +927,11 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, if (ca < 0) ca = 0;
+ ordered_ca = get_channel_allocation_order(ca); + active_channels = channel_allocations[ordered_ca].channels; + + hdmi_set_channel_count(codec, per_pin->cvt_nid, active_channels); + memset(&ai, 0, sizeof(ai)); if (eld->info.conn_type == 0) { /* HDMI */ struct hdmi_audio_infoframe *hdmi_ai = &ai.hdmi; @@ -932,7 +939,7 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, hdmi_ai->type = 0x84; hdmi_ai->ver = 0x01; hdmi_ai->len = 0x0a; - hdmi_ai->CC02_CT47 = channels - 1; + hdmi_ai->CC02_CT47 = active_channels - 1; hdmi_ai->CA = ca; hdmi_checksum_audio_infoframe(hdmi_ai); } else if (eld->info.conn_type == 1) { /* DisplayPort */ @@ -941,7 +948,7 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, dp_ai->type = 0x84; dp_ai->len = 0x1b; dp_ai->ver = 0x11 << 2; - dp_ai->CC02_CT47 = channels - 1; + dp_ai->CC02_CT47 = active_channels - 1; dp_ai->CA = ca; } else { snd_printd("HDMI: unknown connection type at pin %d\n", @@ -959,7 +966,7 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, snd_printdd("hdmi_setup_audio_infoframe: " "pin=%d channels=%d\n", pin_nid, - channels); + active_channels); hdmi_setup_channel_mapping(codec, pin_nid, non_pcm, ca, channels, per_pin->chmap, per_pin->chmap_set); @@ -1238,6 +1245,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo, per_cvt = get_cvt(spec, cvt_idx); /* Claim converter */ per_cvt->assigned = 1; + per_pin->cvt_nid = per_cvt->cvt_nid; hinfo->nid = per_cvt->cvt_nid;
snd_hda_codec_write_cache(codec, per_pin->pin_nid, 0, @@ -1560,8 +1568,6 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo, per_pin->channels = substream->runtime->channels; per_pin->setup = true;
- hdmi_set_channel_count(codec, cvt_nid, substream->runtime->channels); - hdmi_setup_audio_infoframe(codec, per_pin, non_pcm);
return hdmi_setup_stream(codec, cvt_nid, pin_nid, stream_tag, format);
hdmi_manual_setup_channel_mapping() and hdmi_std_setup_channel_mapping try to assign ALSA channels to HDMI channel slots and disable (i.e. silence) other slots.
However, they try to disable a slot by using AC_VERB_SET_CHAN_SLOT with parameter ((alsa_ch << 8) | 0xf), while the correct parameter is ((0xf << 8) | hdmi_slot), i.e. the slot should be unassigned, not the ALSA channel.
Fix that by actually disabling the unused slots.
Note that this bug did not cause any (reported) issues because slots incorrectly having audio are normally ignored by a receiver if the CEA channel allocation used does not map that slot to any speaker. Additionally, the converter channel count configuration limits the number of actually active channels in any case.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi --- sound/pci/hda/patch_hdmi.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index ac818cb..30cf9a9 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -624,9 +624,9 @@ static void hdmi_std_setup_channel_mapping(struct hda_codec *codec,
if (non_pcm) { for (i = 0; i < ch_alloc->channels; i++) - non_pcm_mapping[i] = i | (i << 4); + non_pcm_mapping[i] = (i << 4) | i; for (; i < 8; i++) - non_pcm_mapping[i] = 0xf | (i << 4); + non_pcm_mapping[i] = (0xf << 4) | i; }
for (i = 0; i < 8; i++) { @@ -680,7 +680,7 @@ static int to_cea_slot(unsigned char c) if (t->map == c) return t->cea_slot; } - return 0x0f; + return -1; }
/* from CEA slot to ALSA API channel position */ @@ -733,14 +733,23 @@ static int hdmi_manual_setup_channel_mapping(struct hda_codec *codec, hda_nid_t pin_nid, int chs, unsigned char *map) { - int i; - for (i = 0; i < 8; i++) { + int alsa_pos, hdmi_slot; + int assignments[8] = {[0 ... 7] = 0xf}; + + for (alsa_pos = 0; alsa_pos < chs; alsa_pos++) { + + hdmi_slot = to_cea_slot(map[alsa_pos]); + + if (hdmi_slot < 0) + continue; /* unassigned channel */ + + assignments[hdmi_slot] = alsa_pos; + } + + for (hdmi_slot = 0; hdmi_slot < 8; hdmi_slot++) { int val, err; - if (i < chs) - val = to_cea_slot(map[i]); - else - val = 0xf; - val |= (i << 4); + + val = (assignments[hdmi_slot] << 4) | hdmi_slot; err = snd_hda_codec_write(codec, pin_nid, 0, AC_VERB_SET_HDMI_CHAN_SLOT, val); if (err)
For some speakers and slots the CEA slot <-> speaker assignment depends on the used CEA Channel Allocation value.
Therefore the from_cea_slot() and to_cea_slot() helpers currently only work correctly for the regular 7.1 speakers.
Fix them to work with all speakers, taking the re-ordered CA index as input and adapting use sites accordingly.
This change allows manual channel mapping to actually work for all CEA allocated speakers. Additionally, this fixes incorrect channel map reporting in automatic channel mapping mode when an affected speaker position is used (e.g. 6.1 map which contains an RC speaker).
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi --- sound/pci/hda/patch_hdmi.c | 70 +++++++++++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 29 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 30cf9a9..3c89e48 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -645,19 +645,27 @@ static void hdmi_std_setup_channel_mapping(struct hda_codec *codec,
struct channel_map_table { unsigned char map; /* ALSA API channel map position */ - unsigned char cea_slot; /* CEA slot value */ int spk_mask; /* speaker position bit mask */ };
static struct channel_map_table map_tables[] = { - { SNDRV_CHMAP_FL, 0x00, FL }, - { SNDRV_CHMAP_FR, 0x01, FR }, - { SNDRV_CHMAP_RL, 0x04, RL }, - { SNDRV_CHMAP_RR, 0x05, RR }, - { SNDRV_CHMAP_LFE, 0x02, LFE }, - { SNDRV_CHMAP_FC, 0x03, FC }, - { SNDRV_CHMAP_RLC, 0x06, RLC }, - { SNDRV_CHMAP_RRC, 0x07, RRC }, + { SNDRV_CHMAP_FL, FL }, + { SNDRV_CHMAP_FR, FR }, + { SNDRV_CHMAP_RL, RL }, + { SNDRV_CHMAP_RR, RR }, + { SNDRV_CHMAP_LFE, LFE }, + { SNDRV_CHMAP_FC, FC }, + { SNDRV_CHMAP_RLC, RLC }, + { SNDRV_CHMAP_RRC, RRC }, + { SNDRV_CHMAP_RC, RC }, + { SNDRV_CHMAP_FLC, FLC }, + { SNDRV_CHMAP_FRC, FRC }, + { SNDRV_CHMAP_FLH, FLH }, + { SNDRV_CHMAP_FRH, FRH }, + { SNDRV_CHMAP_FLW, FLW }, + { SNDRV_CHMAP_FRW, FRW }, + { SNDRV_CHMAP_TC, TC }, + { SNDRV_CHMAP_FCH, FCH }, {} /* terminator */ };
@@ -673,25 +681,19 @@ static int to_spk_mask(unsigned char c) }
/* from ALSA API channel position to CEA slot */ -static int to_cea_slot(unsigned char c) +static int to_cea_slot(int ordered_ca, unsigned char pos) { - struct channel_map_table *t = map_tables; - for (; t->map; t++) { - if (t->map == c) - return t->cea_slot; - } - return -1; -} + int mask = to_spk_mask(pos); + int i;
-/* from CEA slot to ALSA API channel position */ -static int from_cea_slot(unsigned char c) -{ - struct channel_map_table *t = map_tables; - for (; t->map; t++) { - if (t->cea_slot == c) - return t->map; + if (mask) { + for (i = 0; i < 8; i++) { + if (channel_allocations[ordered_ca].speakers[7 - i] == mask) + return i; + } } - return 0; + + return -1; }
/* from speaker bit mask to ALSA API channel position */ @@ -705,6 +707,14 @@ static int spk_to_chmap(int spk) return 0; }
+/* from CEA slot to ALSA API channel position */ +static int from_cea_slot(int ordered_ca, unsigned char slot) +{ + int mask = channel_allocations[ordered_ca].speakers[7 - slot]; + + return spk_to_chmap(mask); +} + /* get the CA index corresponding to the given ALSA API channel map */ static int hdmi_manual_channel_allocation(int chs, unsigned char *map) { @@ -731,14 +741,16 @@ static int hdmi_manual_channel_allocation(int chs, unsigned char *map) /* set up the channel slots for the given ALSA API channel map */ static int hdmi_manual_setup_channel_mapping(struct hda_codec *codec, hda_nid_t pin_nid, - int chs, unsigned char *map) + int chs, unsigned char *map, + int ca) { + int ordered_ca = get_channel_allocation_order(ca); int alsa_pos, hdmi_slot; int assignments[8] = {[0 ... 7] = 0xf};
for (alsa_pos = 0; alsa_pos < chs; alsa_pos++) {
- hdmi_slot = to_cea_slot(map[alsa_pos]); + hdmi_slot = to_cea_slot(ordered_ca, map[alsa_pos]);
if (hdmi_slot < 0) continue; /* unassigned channel */ @@ -765,7 +777,7 @@ static void hdmi_setup_fake_chmap(unsigned char *map, int ca) int ordered_ca = get_channel_allocation_order(ca); for (i = 0; i < 8; i++) { if (i < channel_allocations[ordered_ca].channels) - map[i] = from_cea_slot(hdmi_channel_mapping[ca][i] & 0x0f); + map[i] = from_cea_slot(ordered_ca, hdmi_channel_mapping[ca][i] & 0x0f); else map[i] = 0; } @@ -778,7 +790,7 @@ static void hdmi_setup_channel_mapping(struct hda_codec *codec, { if (!non_pcm && chmap_set) { hdmi_manual_setup_channel_mapping(codec, pin_nid, - channels, map); + channels, map, ca); } else { hdmi_std_setup_channel_mapping(codec, pin_nid, non_pcm, ca); hdmi_setup_fake_chmap(map, ca);
Currently the available channel maps TLV only contains channel maps that are limited to the traditional 7.1 speakers.
Since the other HDMI channel mapping functions have been fixed to properly handle all CEA-861-E specified speakers, allow them to be listed.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi ---
Unless there was another reason for this?
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 3c89e48..9f2c396 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1666,8 +1666,6 @@ static int hdmi_chmap_ctl_tlv(struct snd_kcontrol *kcontrol, int op_flag, struct snd_pcm_chmap *info = snd_kcontrol_chip(kcontrol); struct hda_codec *codec = info->private_data; struct hdmi_spec *spec = codec->spec; - const unsigned int valid_mask = - FL | FR | RL | RR | LFE | FC | RLC | RRC; unsigned int __user *dst; int chs, count = 0;
@@ -1685,8 +1683,6 @@ static int hdmi_chmap_ctl_tlv(struct snd_kcontrol *kcontrol, int op_flag, int chs_bytes = chs * 4; if (cap->channels != chs) continue; - if (cap->spk_mask & ~valid_mask) - continue; if (size < 8) return -ENOMEM; if (put_user(SNDRV_CTL_TLVT_CHMAP_VAR, dst) ||
Allow channel map debugging for both automatic and manual channel maps, and print CA always when updating infoframe.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi --- sound/pci/hda/patch_hdmi.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 9f2c396..7dba20c 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -639,8 +639,6 @@ static void hdmi_std_setup_channel_mapping(struct hda_codec *codec, break; } } - - hdmi_debug_channel_mapping(codec, pin_nid); }
struct channel_map_table { @@ -795,6 +793,8 @@ static void hdmi_setup_channel_mapping(struct hda_codec *codec, hdmi_std_setup_channel_mapping(codec, pin_nid, non_pcm, ca); hdmi_setup_fake_chmap(map, ca); } + + hdmi_debug_channel_mapping(codec, pin_nid); }
/* @@ -985,9 +985,9 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, if (!hdmi_infoframe_uptodate(codec, pin_nid, ai.bytes, sizeof(ai))) { snd_printdd("hdmi_setup_audio_infoframe: " - "pin=%d channels=%d\n", + "pin=%d channels=%d ca=0x%02x\n", pin_nid, - active_channels); + active_channels, ca); hdmi_setup_channel_mapping(codec, pin_nid, non_pcm, ca, channels, per_pin->chmap, per_pin->chmap_set);
At Sat, 5 Oct 2013 02:25:37 +0300, Anssi Hannula wrote:
Hi!
Apparently there were several more bugs in the HDMI channel mapping code, so here you go.
I haven't marked any for stable since these don't really affect the regular layouts that much and the patches aren't that trivial. If you want to pick some for stable, my suggestions are 1, 5, 6, maybe 3 (those I initially marked for stable until I changed my mind).
Thanks, applied all patches to for-next branch now.
Takashi
Anssi Hannula (7): ALSA: hda - hdmi: Fix reported channel map on common default layouts ALSA: hda - hdmi: Fix incorrect default channel mapping for unusual CAs ALSA: hda - hdmi: Fix programmed active channel count ALSA: hda - hdmi: Fix unused slots being enabled in manual and non-PCM mappings ALSA: hda - hdmi: Fix channel maps with less common speakers ALSA: hda - hdmi: Fix available channel maps missing from TLV ALSA: hda - hdmi: Tweak debug messages to be more useful
sound/pci/hda/patch_hdmi.c | 146 +++++++++++++++++++++-------------- 1 file changed, 90 insertions(+), 56 deletions(-)
-- Anssi Hannula
participants (2)
-
Anssi Hannula -
Takashi Iwai