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@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
