Re: [RFC PATCH 0/2] ALSA: hda - DP MST audio for Jack support

[Takashi Iwai <tiwai@suse.de>]

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@suse.de]
> >>> Sent: Monday, November 09, 2015 3:59 PM
> >>> To: Libin Yang
> >>> Cc: alsa-devel@alsa-project.org; mengdong.lin@linux.intel.com; Yang,
> >>> Libin
> >>> Subject: Re: [alsa-devel] [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@linux.intel.com wrote:
> >>>>>>
> >>>>>> From: Libin Yang <libin.yang@linux.intel.com>
> >>>>>>
> >>>>>> 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.

> But what about the 
> other situation? The other situation still uses the hda_jack_tbl and 
> etc. So we still need handle hda jack if we want to reuse the current 
> code.

For other, it's similar.  The jack is associated with PCM, which is
virtually bound with per_pin.  And when it's assigned / unassigned,
call it with the given per_pin->pin_nid.


Takashi