[alsa-devel] [RFC PATCH 0/3] support DP MST audio

[Yang, Libin]

Hi Takashi,

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Tuesday, September 27, 2016 1:51 AM
> To: Yang, Libin <libin.yang at intel.com>
> Cc: libin.yang at linux.intel.com; alsa-devel at alsa-project.org; Lin, Mengdong
> <mengdong.lin at intel.com>
> Subject: Re: [alsa-devel] [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 at suse.de]
> > > Sent: Monday, September 26, 2016 11:11 PM
> > > To: libin.yang at linux.intel.com
> > > Cc: alsa-devel at alsa-project.org; Lin, Mengdong
> > > <mengdong.lin at intel.com>; Yang, Libin <libin.yang at intel.com>
> > > Subject: Re: [alsa-devel] [RFC PATCH 0/3] support DP MST audio
> > >
> > > On Mon, 26 Sep 2016 10:35:35 +0200,
> > > libin.yang at linux.intel.com wrote:
> > > >
> > > > From: Libin Yang <libin.yang at linux.intel.com>
> > > >
> > > > 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.

> 
> 
> > > 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.

Regards,
Libin

> 
> 
> thanks,
> 
> Takashi