> -----Original Message----- > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > Sent: Tuesday, December 22, 2015 8:08 PM > To: Yang, Libin > Cc: Libin Yang; mengdong.lin@xxxxxxxxxxxxxxx; alsa-devel@alsa- > project.org > Subject: Re: [RFC PATCH 0/2] ALSA: hda - DP MST audio for > Jack support > > On Tue, 22 Dec 2015 12:45:25 +0100, > Yang, Libin wrote: > > > > > > > -----Original Message----- > > > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > > > Sent: Tuesday, December 22, 2015 5:00 PM > > > To: Libin Yang > > > Cc: Yang, Libin; mengdong.lin@xxxxxxxxxxxxxxx; alsa-devel@alsa- > > > project.org > > > Subject: Re: [RFC PATCH 0/2] ALSA: hda - DP MST audio > for > > > Jack support > > > > > > On Tue, 22 Dec 2015 09:47:12 +0100, > > > Libin Yang wrote: > > > > > > > > Hi Takashi, > > > > > > > > On 11/09/2015 11:15 PM, Takashi Iwai wrote: > > > > > On Mon, 09 Nov 2015 16:00:01 +0100, > > > > > Yang, Libin wrote: > > > > >> > > > > >> Hi Takashi, > > > > >> > > > > >>> -----Original Message----- > > > > >>> From: Takashi Iwai [mailto:tiwai@xxxxxxx] > > > > >>> Sent: Monday, November 09, 2015 3:59 PM > > > > >>> To: Libin Yang > > > > >>> Cc: alsa-devel@xxxxxxxxxxxxxxxx; mengdong.lin@xxxxxxxxxxxxxxx; > > > Yang, > > > > >>> Libin > > > > >>> Subject: Re: [RFC PATCH 0/2] ALSA: hda - DP MST > > > audio for > > > > >>> Jack support > > > > >>> > > > > >>> On Wed, 04 Nov 2015 07:23:14 +0100, > > > > >>> Libin Yang wrote: > > > > >>>> > > > > >>>> > > > > >>>> On 11/04/2015 12:49 AM, Takashi Iwai wrote: > > > > >>>>> On Tue, 03 Nov 2015 09:42:54 +0100, > > > > >>>>> libin.yang@xxxxxxxxxxxxxxx wrote: > > > > >>>>>> > > > > >>>>>> From: Libin Yang <libin.yang@xxxxxxxxxxxxxxx> > > > > >>>>>> > > > > >>>>>> This is the patch set for Jack support for DP MST audio > > > > >>>>>> > > > > >>>>>> Libin Yang (2): > > > > >>>>>> ALSA: hda - jack support DP MST audio > > > > >>>>>> ALSA: hda - hdmi audio add dynamically jack binding to > pin > > > > >>>>> > > > > >>>>> I'm somehow confused by this patch series. > > > > >>>>> Is the unsolicited event handling itself changed for MST? I > mean, > > > you > > > > >>>>> cannot know which MST dev# is given beforehand, while you > > > seem > > > > >>> trying > > > > >>>>> to assign such a number to the jack object. > > > > >>>> > > > > >>>> Oh, sorry I forgot to mention that we only operate on device > > > entry 0 > > > > >>>> so far in the code. It's misleading. > > > > >>>> > > > > >>>> These patches don't operate on MST audio. The full MST audio > > > driver > > > > >>>> support will be implemented in the later patches. The patches > help > > > to > > > > >>>> prepare supporting MST audio. > > > > >>>> > > > > >>>> The purpose of the patches is try to dynamically attach the > Jack. > > > > >>> > > > > >>> Well, only judging from the quick glance, I don't buy this > strategy, > > > > >>> sorry. Currently for HDMI/DP, the jack is for PCM, not for pin. > > > > >>> Namely, user-space watches the jack notification to judge > whether > > > the > > > > >>> corresponding PCM is available or not. You're trying to change > this > > > > >>> scheme completely. > > > > >> > > > > >> The patch is to make sure the jack is bound to PCM statically. > > > > >> This patch is needed because in old code, it will create jack with: > > > > >> for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { > > > > >> ... > > > > >> err = generic_hdmi_build_jack(codec, pin_idx); > > > > >> ... > > > > >> } > > > > >> Which means the jack is created based on pin number and > pin_idx. > > > > >> As a pin is bound statically to the PCM, the jack is statically bound > to > > > PCM. > > > > > > > > > > OK, so far, so good. > > > > > > > > > >> Now we are using the dynamic pcm bound. If we use the old > code to > > > > >> create the jack, the jack will be bound to pin. So we need the > patch, > > > > >> which will use the new code: > > > > >> for (pcm_idx = 0; pcm_idx < spec-> pcms_used; pcm_idx++) { > > > > >> ... > > > > >> err = generic_hdmi_build_jack(codec, pcm_idx); > > > > >> ... > > > > >> } > > > > >> which makes sure that the jack is bound to PCM (jack[0] is bound > > > > >> to pcm[0], jack[1] is bound to pcm[1] and so on). > > > > >> > > > > >> If there is no monitor, no pin is bound to the PCM. > > > > >> So no pin is bound to the Jack. When there is a monitor > connected, > > > > >> the pin is bound to a proper PCM. So we should bind the PCM's > jack > > > > >> to the pin. > > > > >> > > > > >> For example, jack0 is bound to the PCM 3 (the first jack is bound > > > > >> to the first PCM statically). When a monitor is connected to pin 5, > > > > >> PCM 3 will be assigned to the pin. We need bind jack 0 to pin 5. > > > > >> When monitor is disconnected, we must unbind the jack 0 from > pin > > > 5. > > > > >> These operation are done in function > > > > >> snd_hda_jack_bind/ snd_hda_jack_unbind. > > > > > > > > > > Then we shouldn't use the hda_jack.c code at all for HDMI/DP. > > > > > For HDMI/DP with the dynamic PCM binding, the only state to > check > > > is > > > > > its PCM assignment. It doesn't have to issue the jack state check > via > > > > > HDA verb at all. This should make things a lot easier. > > > > > > > > I'm now porting the jack supporting for MST audio. The code on > > > > hdmi-jack branch will create jack differently based > codec_has_acomp() > > > > condition. Does this mean MST Jack binding will handle both > situation? > > > > > > Ideally, yes. > > > > > > > My idea is: > > > > 1. Add struct snd_jack *jack[] member in hdmi_spec. > > > > 2. Create the jack based on PCM > > > > 3. Assign spec->jack[i] to per_pin->acomp_jack when pin is > connected. > > > > > > > > This should be OK for codec_has_acomp sitation. > > > > > > Hrm, not really. As already discussed, a jack is associated to each > > > PCM, and notified plug/unplug when it's assigned / unassigned. > > > That is, you don't have to keep extra jack array. When a PCM is > > > assigned or unassigned, simply notify per_pin->acomp_jack of that > PCM. > > > That's all. So there shouldn't be too many difference from the > > > current situation. > > > > If so, the snd_jack is dynamically bound to PCM. This means > > the jack is bound to the pin, and user space must get the pcm-pin > > mapping to know the jack is bound to which PCM. Then it can decide > > how to operate on the PCM. Not sure it is compatible with current > > pulseaudio. > > Erm, I was confused. Yes, for dynamic attach/detaching PCM, we need > to keep jack object apart from per_pin. I guess your current patchset > is broken in this regard? > > Though, for having the jack pointer, I prefer avoiding to introduce > yet another array but rather extending hdmi_spec.pcm_rec[]. Currently > it contains the hda_pcm pointers. But we can define a new object, > e.g. > > struct hdmi_pcm { > struct hda_pcm *pcm; > struct snd_jack *acomp_jack; > .... > }; > > and keeping this in hdmi_spec. > > struct hdmi_spec { > .... > struct hdmi_pcm pcm_rec[16]; > .... > }; > > get_pcm_rec() macro needs to be adjusted properly, but other than > that, there aren't so many places accessing pcm_rec[] directly, so > easy to convert. > > Maybe we can start from converting this at first, moving acomp_jack to > hdmi_spec, then apply MST patchset on its top. This might be a bit > cleaner. It's clear now. It should work. I will try it. > > In the case of non-component: the question beforehand is how to handle > MST PCM assignment at all. Namely, the non-component case assumes > that there is 1:1 object matching with the actual pin to per_pin. If > this assumption isn't kept, we'd need to use the same trick like the > component case, and stop using hda_jack infrastructure. > (So acomp_jack might be better renamed to just "jack" or so.) > If MST PCM is supported, then per_pin is not 1:1 object matching with the actual pin. It seems we will stop using hda_jack infrastructure. I will refine the jack patches based on the new method. Thanks for your suggestion. Regards, Libin > > thanks, > > Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
