On Tue, 27 Sep 2016 09:47:38 +0200, Yang, Libin wrote: > > Hi Takashi, > > > -----Original Message----- > > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > > Sent: Tuesday, September 27, 2016 1:51 AM > > To: Yang, Libin <libin.yang@xxxxxxxxx> > > Cc: libin.yang@xxxxxxxxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx; Lin, Mengdong > > <mengdong.lin@xxxxxxxxx> > > Subject: Re: [RFC PATCH 0/3] support DP MST audio > > > > On Mon, 26 Sep 2016 18:07:51 +0200, > > Yang, Libin wrote: > > > > > > Hi Takashi, > > > > > > > -----Original Message----- > > > > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > > > > Sent: Monday, September 26, 2016 11:11 PM > > > > To: libin.yang@xxxxxxxxxxxxxxx > > > > Cc: alsa-devel@xxxxxxxxxxxxxxxx; Lin, Mengdong > > > > <mengdong.lin@xxxxxxxxx>; Yang, Libin <libin.yang@xxxxxxxxx> > > > > Subject: Re: [RFC PATCH 0/3] support DP MST audio > > > > > > > > On Mon, 26 Sep 2016 10:35:35 +0200, > > > > libin.yang@xxxxxxxxxxxxxxx wrote: > > > > > > > > > > From: Libin Yang <libin.yang@xxxxxxxxxxxxxxx> > > > > > > > > > > This patchset starts to support DP MST audio. > > > > > > > > > > This patchset is based on drm-intel-nightly on > > > > > git://anongit.freedesktop.org/drm-intel > > > > > > > > > > Libin Yang (3): > > > > > ALSA: hda - codec add DP MST support for connection list > > > > > ALSA: hda - add DP mst verb support > > > > > ALSA: hda - add DP mst audio support > > > > > > > > I read the patchset again, and I'm still not convinced enough by this > > change. > > > > > > Yes and I'm sorry that this patchset is delayed for a long time. > > > Our gfx driver has been redesigned and some APIs have been changed. > > > The change is done in Pandiyan, Dhinakaran's patches: > > > Patchset: Prep. for DP audio MST support > > > drm/i915: Standardize port type for DVO encoders > > > drm/i915: Store port enum in intel_encoder > > > drm/i915: Switch to using port stored in intel_encoder > > > drm/i915: Move audio_connector to intel_encoder > > > drm/i915: start adding dp mst audio > > > Patch: drm/i915/dp: DP audio API changes for MST > > > > > > > > > > > The connection list is coded in the current way under the assumption > > > > that connections are more or less static. The connections are > > > > cached in snd_array, which is only for growing up, and not suitable > > > > for the entries that might be frequently removed. The removal is > > > > done only at overriding, and it happens only once at boot. > > > > > > > > OTOH, with DP-MST case, the connection list is basically dynamic. > > > > It may increase or decrease depending on the connected monitors... > > > > Is my understanding correct? Or is the DP-MST connection list is > > > > static, too? Then I do wonder how it covers the whole connections with > > arbitrary device indices. > > > > > > For the connection list, the driver will setup all the connection list > > > at the beginning for each device entry, even it is non-MST. We > > > statically allocate the connection list and will never remove them. > > > > Hrm, so how will it actually be? Are the device indices fixed through the > > whole operations, no matter which monitor is connected? How many will > > they be? > > > > It'd be helpful if you give an example of the actual typical setup. > > Then we can judge whether the proposed implementation is suitable. > > Even it is in non-MST mode, the connection list is setup with the per_pin > initialization. And as the device entries under the same pin have the > same connection list, I use the device entry 0's connection list for other > device entries. This avoids connection list is invalid for other device entries > in non-MST. After initialization, the connection list will never change even > the non-MST/MST mode is changed. The devid=0 case is fine. This is identical with the normal widget connections. But you wrote that these connections are static even for devid!=0. Is it correct? If so, now my question again: for devid!=0, which connections are given statically? In other words, how many devid would be passed? > > > > So, the connection cache management is one thing. Another thing is > > > > that the patchset doesn't consider about the pin ELD notification. > > > > Basically we switched to ELD notification instead of the pin unsol > > > > event for Intel chips. How does it fit? > > > > > > For the ELD notification, in the patch 0003, if there is monitor > > > hotplug, gfx driver will notify audio driver with acomp. Gfx driver > > > will tell which port (pin), pipe (device entry) occurs the hotplug > > > event. And then audio driver will call snd_hdac_acomp_get_eld() to get the > > eld information. > > > > OK, then drop the patch 1 and try to implement without messing around the > > connection list. The patch 1 is what I really don't like from the beginning. It > > makes things complicated. > > As the connection list is the same for the device entries under same pin, > what do you think if we use the original APIs, ignoring the device id. This > will not change the connection list code or be a very small change. > > If you agree on that, I will have a try on it. > > On the same time, I will check with our silicon if my understanding > "the connection list is the same for the device entries under same pin" > is right. > > > Is there anything else than haswell fixup that touches the connection list with > > the dev id? If it's the only place, there can be an alternative way to hack it. > > If we always use the device entry 0 as all the device entries under the same pin, > I think we can ignore the dev id now. Well, it is only about the pin/cvt connection inside HDMI/DP codec, and it should be easily covered in a different way even with devid!=0, I suppose. That said, the patchset looks way too complex than necessary. Though, it might be a good idea to start with devid=0 assumption if it makes the changes simpler. Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
