From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: sof/hda rework to share more of patch_hdmi.c logic Date: Fri, 23 Aug 2019 19:18:46 +0200 Message-ID: References: Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id E67F9F8014A for ; Fri, 23 Aug 2019 19:18:47 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" To: Kai Vehmanen Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org 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