On Tue, 03 Dec 2019 14:48:10 +0100, Kai Vehmanen wrote:
Hi Takashi and Nikhil,
On Mon, 2 Dec 2019, Kai Vehmanen wrote:
yup, -1 for non-MST, that was just plain wrong in my Friday mail. The problem seems to be pipe id is positive in some non-MST cases (like https://github.com/thesofproject/linux/issues/1536 ), which is very suprising looking at e.g. intel_audio_codec_enable(), but that seems to be happening nevertheless.
ok, got graphics driver log from this case now, and this turns out to be a system with an unusual converter setup for HDMI output. I.e. from user point of view it's HDMI, but for graphics driver it looks like a DP connection (with MST enabled which is not so common but apparently is the case). So i915 driver is working as expected and in patch_hdmi we should handle it as a normal MST case.
Takashi, considering the commit message is wrong, I think it's better to revert the "ALSA: hda: hdmi - preserve non-MST PCM routing for Intel platforms" patch. Should I send a revert?
It also doesn't fully preseve the old routing as in the case of a single DP-MST monitor (with dev_id=1), hdmi_find_pcm_slot() will not pick the preferred PCM "per_pin->pin_nid_idx", but will choose "spec->num_nids + (per_pin->dev_id - 1)". So seems better to just drop it.
Well, if we want to keep the old behavior for compatibility just to be sure, how about a patch like below?
Takashi
--- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1348,21 +1348,18 @@ static int hdmi_find_pcm_slot(struct hdmi_spec *spec, * with the legacy static per_pin-pcm assignment that existed in the * days before DP-MST. * + * Intel DP-MST prefers this legacy behavior for compatibility, too. + * * per_pin of m!=0 prefers to get pcm=(num_nids + (m - 1)). */
- if (per_pin->dev_id == 0) { + if (per_pin->dev_id == 0 || spec->intel_hsw_fixup) { if (!test_bit(per_pin->pin_nid_idx, &spec->pcm_bitmap)) return per_pin->pin_nid_idx; } else { i = spec->num_nids + (per_pin->dev_id - 1); if (i < spec->pcm_used && !(test_bit(i, &spec->pcm_bitmap))) return i; - - /* keep legacy assignment for dev_id>0 on Intel platforms */ - if (spec->intel_hsw_fixup) - if (!test_bit(per_pin->pin_nid_idx, &spec->pcm_bitmap)) - return per_pin->pin_nid_idx; }
/* have a second try; check the area over num_nids */