On Tue, 22 Dec 2015 12:45:25 +0100, Yang, Libin wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, December 22, 2015 5:00 PM To: Libin Yang Cc: Yang, Libin; mengdong.lin@linux.intel.com; alsa-devel@alsa- project.org Subject: Re: [alsa-devel] [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@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
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:
- Add struct snd_jack *jack[] member in hdmi_spec.
- Create the jack based on PCM
- 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.
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.)
thanks,
Takashi