Re: [alsa-devel] [PATCH 2/2] [RFC] ALSA: hda: hdmi - preserve non-MST PCM routing for Intel platforms

From: Takashi Iwai <tiwai@suse.de>
To: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Cc: alsa-devel@alsa-project.org, Nikhil Mahale <nmahale@nvidia.com>
Subject: Re: [alsa-devel] [PATCH 2/2] [RFC] ALSA: hda: hdmi - preserve non-MST PCM routing for Intel platforms
Date: Fri, 29 Nov 2019 16:08:03 +0100	[thread overview]
Message-ID: <s5hsgm6ra98.wl-tiwai@suse.de> (raw)
In-Reply-To: <alpine.DEB.2.21.1911291638430.16459@zeliteleevi>

On Fri, 29 Nov 2019 15:47:11 +0100,
Kai Vehmanen wrote:
> 
> Hi Takashi, Nikil and others,
> 
> On Fri, 29 Nov 2019, Kai Vehmanen wrote:
> > This difference leads to some subtle differences in hdmi_find_pcm_slot()
> > with regards to how non-MST monitors are assigned to PCMs.
> > This patch restores the original behaviour on Intel platforms while
> > keeping the new allocation policy on other platforms.
> 
> hmm, there seems a couple of more issues. The first patch is a clear bug 
> that leads to segfault with SOF+patch_hdmi on some platforms (pipe B used
> for single monitor HDMI case -> dev_id=1 -> non-existant pcm selected
> and eventual kernel oops).
> 
> This second patch is however trickier. Nikhil your patch changed the 
> default allocation a bit, so the routing might be difference also with 
> snd-hda-intel (i.e. not SOF) for existing platforms and this may surprise 
> users.

Well, but the allocation itself is dynamic for DP-MST, even on Intel,
so user can't expect the completely persistent index assignment.
That's the reason I took Nikhil's patch (even I suggested to simplify
in that way).

We had a trick to assign the primary index.  It still works, right?
That should be the only concern.

> Digging deeper, we seem to have a slight semantics difference in how
> intel_pin_eld_notify() and generic_acomp_pin_eld_notify() handle
> the third pipe/dev_id parameter.

This is a platform-specific part, and on Intel, the assumption has
been that pipe is equivalent with dev_id.  If this changed, of course,
we must reconsider the whole picture.

For generic_acomp_pin_eld_notify(), it's gfx-driver specific, too.
And currently dev_id = -1 in AMDGPU, so we don't think too much about
the behavior compatibility.

> Any thoughts how to solve? I first I thought making separate functions
> for hdmi_find_pcm_slot() and allow platforms to define an alternative
> implementation, but in this RFC patch I opted for a simpler quirk in the 
> function. This is becoming fairly messy I must say -- the amount of 
> code commentary needed is a good indication some simplifaction would
> be in order.

Yeah, that's a bit messy.  The only expectation is the primary slot
assignment -- i.e. the case only one monitor is connected.  As long as
this behavior is kept, I don't think any big problem with the dynamic
assignment.

> PS I did not have time to fully test the RFC patch, so this is just
>    for discussion now...

Since the assignment should work with your patch somehow, I already
applied it.  Let's do fine tune-up during 5.5 rc cycles, if any.

thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel