[alsa-devel] sof/hda rework to share more of patch_hdmi.c logic

[Takashi Iwai]

On Fri, 23 Aug 2019 16:55:09 +0200,
Takashi Iwai wrote:
> 
> On Fri, 23 Aug 2019 16:29:40 +0200,
> Kai Vehmanen wrote:
> > 
> > Hi,
> > 
> > On Thu, 15 Aug 2019, Takashi Iwai wrote:
> > > Kai Vehmanen wrote:
> > >> into modifying the SOF Intel backend to use 
> > >> snd-hda-codec-hdmi/patch_hdmi.c for HDMI/DP audio support, i.e. to be able 
> > >> to share this code between snd-hda-intel and SOF Intel (and not using 
> > >> hdac-hdmi).
> > [..]
> > >> This will change how HDMI is exposed to user-space with SOF Intel 
> > >> drivers, so we need to be extra careful how this is introduced. But 
> > 
> > > Agreed.  I guess the biggest difference is the handling of the 
> > > DP-MST.  The legacy HD-audio HDMI driver takes a different approach
> > > for DP-MST, namely, it chooses dynamically the pin that is connected
> > > with a monitor.  It's for keeping the compatibility (more or less)
> > > with old behavior; the program just needs to open the device that
> > > corresponds to the notification via jack ctl without fiddling with
> > > other extra routing.
> > 
> > indeed. I've now got to a point where I have the key functionality
> > in place. And after some reworks, the changes to patch_hdmi.c 
> > are pretty minimal, which is very nice (I started with a much more 
> > evasive patch).
> > 
> > But, but. The DP-MST handling is indeed iffy. I tried a few approaches, 
> > but it is hard to reconcile concept of "backup PCMs" of patch_hdmi.c with 
> > concepts of ASoC and ALSA topology, where the PCM and DAIs are supposed to 
> > be defined in the topology file. This gets worse with SOF (and any similar 
> > usage) which allow you to have arbitrary DSP processing between a PCM and 
> > the HDMI/DP DAIs. So this seems like a dead-end.
> > 
> > What I ended up doing was to make a new mode to patch_hdmi.c that limits 
> > PCM count to actual converter count (and this is aligned with topology), 
> > and still supporting DP-MST by always mapping monitors to a free PCM. I've 
> > tested some complex DP-MST scenarios and this seems to work pretty well. 
> > The jack detection will still be able to tell which of the PCMs have a 
> > monitor detected.
> > 
> > I wonder if this would be an acceptable approach, given 
> > the reuse benefits we get. Downsides:
> > - assignment of monitors to PCMs will depend on ELD update ordering
> 
> This is true in anyway for the legacy HD-audio, too.
> 
> > - in SOF we need to align PCM numbering scheme in all topologies 
> >   we convert over to patch_hdmi.c
> 
> I don't think this can be a problem.
> 
> Practically seen, the number of PCMs could be just 3 on Skylake and
> co, even with DP MST, because i915 can drive only up to 3 monitors.
> The driver *should* receive the unplug event before the plug event to
> a new monitor, so hda_detach_hda_pcm() gets called before
> hda_attach_hda_pcm().
> 
> The current patch_hdmi.c implementation is based on the theoretical
> possibility, and limitation to the reduced PCM streams would work, I
> suppose.

For more correctness: the patch_hdmi.c implementation actually would
work even without the backup PCM streams.  The backup PCM streams are
there for assuring the compatible behavior for old applications that
access to the fixed PCM device (e.g. hw:1,1).  But, it's hardly
possible to get more than three audio-monitors active in the real
scenario, we've almost never seen this necessity.

The actual behavior can be found in the description of commit
a76056f2e57e:

    When monitor is connected, find a proper PCM for the monitor.
    When monitor is disconnected, unbind the PCM from the pin.
    
    The binding policy (use Intel platform as example) is:
    1. Try to use the legacy pin-pcm mapping for the device entry 0
       of the pin.
    2. If step 1 fails, try to bind pin to the backup PCMs. For example,
       on Intel platform, if DP MST is enabled, 5 PCMs will be created.
       PCM 3, PCM 7, PCM 8 are supposed to be used by device entry 0 of
       pin 5, pin 6 and pin 7. PCM 9 and PCM 10 are the backup PCMs.
    3. If step 2 fails, try to find any PCM to bind to the pin.
    
Removing the backup streams means the removal of step 2, but the
driver will keep working in step 3.


thanks,

Takashi