[alsa-devel] [Intel-gfx] [PATCH] drm/i915/dp: DP audio API changes for MST

[Pandiyan, Dhinakaran]

On Fri, 2016-08-05 at 02:31 +0000, Yang, Libin wrote:
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of
> > Ville Syrjälä
> > Sent: Friday, August 5, 2016 4:48 AM
> > To: Takashi Iwai <tiwai at suse.de>
> > Cc: libin.yang at linux.intel.com; intel-gfx at lists.freedesktop.org; alsa-
> > devel at alsa-project.org; Pandiyan, Dhinakaran
> > <dhinakaran.pandiyan at intel.com>
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp: DP audio API changes for MST
> > 
> > On Thu, Aug 04, 2016 at 07:55:09PM +0200, Takashi Iwai wrote:
> > > On Thu, 04 Aug 2016 19:35:16 +0200,
> > > Ville Syrjälä wrote:
> > > >
> > > > On Thu, Aug 04, 2016 at 10:18:52AM -0700, Jim Bride wrote:
> > > > > On Wed, Aug 03, 2016 at 10:08:12PM +0300, Ville Syrjälä wrote:
> > > > > > On Tue, Aug 02, 2016 at 07:14:30PM -0700, Dhinakaran Pandiyan wrote:
> > > > > > > DP MST provides the capability to send multiple video and
> > > > > > > audio streams via one single port. This requires the API's
> > > > > > > between i915 and audio drivers to distinguish between audio
> > > > > > > capable displays connected to a port. This patch adds this
> > > > > > > support via an additional parameter 'int dev_id'. The existing
> > parameter 'port' does not change it's meaning.
> > > > > > >
> > > > > > > dev_id =
> > > > > > > 	MST	: pipe that the stream originates from
> > > > > > > 	Non-MST	: -1
> > > > > > >
> > > > > > > Affected APIs:
> > > > > > > struct i915_audio_component_ops
> > > > > > > -       int (*sync_audio_rate)(struct device *, int port, int rate);
> > > > > > > +	int (*sync_audio_rate)(struct device *, int port, int
> > > > > > > +dev_id,
> > > > > >
> > > > > > Does the term 'dev_id' have some special meaning on the audio
> > > > > > side? On the i915 side things would be less confusing if we just
> > > > > > called it 'pipe'.
> > > > >
> > > > > Yeah, it does.  All of the documentation on the audio side is
> > > > > written in terms of device ID, so they asked for that nomenclature.
> > > >
> > > > And is the device ID always the same as the pipe? Until now we've
> > > > made due with passing the port instead of the pipe, so either the
> > > > audio side didn't use the device ID, or its meaning changes based on
> > > > how we drive things, or they dug it out from somewhere else based on the
> > port?
> > >
> > > This is my concern, too.  Currently we have a very wild assumption
> > > even for the port mapping.  In the audio side, there is neither port
> > > nor pipe.  There are only the widget node id and the device id.  The
> > > former is supposedly corresponding to the port, and the latter to the
> > > pipe.  But the audio side has absolutely no clue about how these are
> > > connected.
> > 
> > So I tried to study this a bit, and MST and device<n> are mentioned a few
> > times in the description of some audio registers in the GPU docs. Looks like a
> > bunch of bits overlap somehow with pin vs. device usage. I don't understand
> > how that's supposed to work. Eg:
> 
> For SST & HDMI, port is mapping to pin.
> 
> For MST, in audio, each pin has several device entries. Each device entry
> can transfer audio stream. And we confirmed with silicon team,
> device entry is related to pipe.
> 
> Device entry is always the right concept in audio driver. We are not sure
> the relationship will be always right between pipe and device entry.
> 
> We are worry that the relationship may be changed in the future between
> pipe and device entry. I mean maybe the pipe is not related to device
> entry in the new platforms, but some other conception, such as transcoder.
> If so, if we are using pipe now, maybe we need change the API again.
> For the stability, we are thinking device entry is suitable.
> 
> I think your concern is right. It is not good to use audio conception 
> in gfx driver, which will cause confusion. If we can confirm that
> pipe will always be the fixed mapping to device entry, it will be
> better to use pipe.
> >  AUD_PWRSTAUD_PWRST
> >  1:0 	 PinB Widget PwrSt Set
> >  PinB Widget power state that was setFor DP MST this represents Device1
> > power state
> 
> For this register, it means different in different scenario:
> In DP SST & HDMI, it is for PinB
> In DP MST, it is for device entry 1 (on whichever pin). 
> 
> > 
> > It's anyone's guees what those bits are suppoosed to reflect when you're doing
> > MST with device1/pipe A, and at the same time you're driving port B in
> > HDMI/SST mode.
> 
> I did the similar test before. I remember (but I didn't remember clearly now)
> If one pin is used as DP MST, all the pins are used as DP MST. This means if
> PinB is used as DP MST, 1:0 represents Device 1 and 5:4 represents Device 2.
> Let's say PinC is connected to HDMI audio, it is now looked as Device 2 (or
> other device id) not as PinC audio.
> 
> Regards,
> Libin
> 

Here's what I found after digging through the specs. 

1. Firstly, "device 1" is not same as "device entry at index=1".
a) Device
"Get Device Select" (source: Bspec)
------------------------------------------------------------------------
11:06 Sink Device ID: Sink Device ID in the multi stream topology of the
DP hierarchy.
Device attached to Pipe A will have ID of "00000", Pipe B will have
"00001" and Pipe C will have "00010".
------------------------------------------------------------------------

b) Device entry
Device entry is an audio device attached to a pin(port). Each entry has
an index in a per pin list of devices.
  
2. The "device(pipe)" and the device entry(index in a per pin list)
mapping can be figured out with a sequence of "Get Device List Entry"
and "Get Device Select" verbs in the audio driver.(source:HDA spec)

i915 is not aware of "device entry" or it's mapping to device(pipe). So,
we should be sending the "pin(port)" and "device(pipe)" via
pin_eld_notify(). The audio driver can use this information to get  the
right device entry for "Get Pin Sense" verb. 

3. Confirming what Ville already said. All we need to do now is, as
Ville pointed out, rename the variables

(port and pipe) or (pin and device_id)

Since dev_id is a horrible name for a new variable in i915, let's use
port and pipe.

I will send out a patch renaming dev_id to pipe.

-DK

> > 
> > Anyways, it doesn't help that this whole device widget aspect of hda seems to
> > be undocumented. No spec I've found seems to know anything about any
> > device widgets, and yet those are how MST rolls apparently.
> > 
> > In conclusion, I'd say that consistency's sake we should use either
> > dev+pin or pipe+port in the interface, not mix both. In case of pin I
> > dev+don't think we should use the NID (I'm assuming device
> > widgets have one too), but rather the index. i915 could then do the mapping
> > somethign like dev=pipe and pin=port-1, and then snd-hda can do the idx<-
> > >nid conversion however it sees fit (just use base_nid, or walk some per-type
> > widget lists, or whatever).
> > 
> > That's assuming the widgets really are somehow ordered consistently so we
> > can index them like that. I also might have understtod everyhing I read abou
> > hda.
> > 
> > --
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx