Re: sof/hda rework to share more of patch_hdmi.c logic

[Takashi Iwai <tiwai@suse.de>]

On Fri, 23 Aug 2019 17:55:05 +0200,
Kai Vehmanen wrote:
> 
> Hey,
> 
> 
> On Fri, 23 Aug 2019, Takashi Iwai wrote:
> > On Fri, 23 Aug 2019, Takashi Iwai wrote:
> > > The current patch_hdmi.c implementation is based on the theoretical
> > > possibility, and limitation to the reduced PCM streams would work, I
> > > suppose.
> [...]
> > 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.
> 
> ack. This does feel very unlikely to be a problem. And one could implement 
> a wait list -> delay PCM attach if all audio converters are taken when the 
> new monitor ELD update happens. Upon next disconnect of an already 
> attached audio enabled monitor, one could check the wait list and hook up 
> monitors to the freed converters. But yeah, at least on i915 I don't see 
> the need for this.
> 
> This does raise the question whether we should change the behaviour 
> for the non-DSP HD driver as well...? Tempting, but the risk for breaking 
> apps may be too high.

It's a good question.  On one hand, it'd be good to clean up a bit,
but we never know the use of backup PCM ever happened.  No news is a
good news, after all.

> > 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.
> 
> Yes, I didn't have to actually change this code at all. I just limit the 
> PCM count to number of converters, and the above code works without 
> modifications (and skips (2)). Only problem left is how to change to 
> toggle the different rules for calculing pcm count. Unless we change the 
> behaviour for non-DSP HD driver as well, there needs to be some dynamic 
> configuration of patch_hdmi. Kconfig/module-param won't do, I assume 
> distros want to enable both options with same kernel.

We can introduce some new flag in struct hda_codec indicating this
restriction, and set it in hdac_hda_codec_probe() beforehand.

Or, we may introduce a new Kconfig and disable statically.  The new
Kconfig may have "depends on SOF=n" or whatever to be sure.
Although we have no 100% guarantee as mentioned in the above, this
shouldn't be a big problem.  Worth for try.

Honestly speaking, I'm not entirely sure which is a better way to go.
Both would be fairly small changes.

You can try them out and see which looks saner.  I really don't mind
either way.


thanks,

Takashi