Re: [PATCH] ALSA: hda/hdmi: let new platforms assign the pcm slot dynamically

[Hui Wang <hui.wang@canonical.com>]

On 2/23/21 10:14 PM, Kai Vehmanen wrote:
> Hi,
>
> thanks for the patch!
>
> On Tue, 23 Feb 2021, Hui Wang wrote:
>
>> If the platform set the dyn_pcm_assign to true, it will call
>> hdmi_find_pcm_slot() to find a pcm slot when hdmi/dp monitor is
>> connected and need to create a pcm.
> [...]
>> This change comes from the discussion between Takashi and
>> Kai Vehmanen. Please refer to:
>> https://github.com/alsa-project/alsa-lib/pull/118
> I did propose to merge the alsa-lib change to give us a bit more time to
> think about how this should be handled in kernel.
That is not merged yet, and my PM was pushing me to find a solution, 
then I wrote this patch.
>
> While this patch certainly solves the problem of kernel picking ALSA PCMs,
> which current alsa-lib cannot handle, it leaves us a bit halfway. We'd
> create many PCMs that will never be used. And this change is a bit more
> involved.
>
>> So far only intel_hsw_common_init() and patch_nvhdmi() set the
>> dyn_pcm_assign to true, here we let tgl platforms assign the pcm slot
>> dynamically first, if the driver runs for a period of time and there
>> is no regression reported, we could set no_fixed_assgin to true in
>> the intel_hsw_common_init(), and then set it to true in the
>> patch_nvhdmi().
> Staged plan sounds good here, although I'd be fairly cautious with this.
> People using Pulseaudio/Pipewire+UCM won't notice a thing, but I'm sure
> there are people out there assuming a fixed "physical connector -> ALSA
> PCM" mapping and not using UCM. Probably at least some way to opt-out
> would be needed for older platforms.
>
>> +	if (port_num > 6)
>> +		spec->no_fixed_assign = true;
> I think this is magic enough of a number to be defined separately along
> with some documentation. So basicly user-space has a max limit of 8 now
> and two PCMs are reserved for DP-MST, so that brings us to six, right?
Yes, and also before the TGL, the max number of physical pins is 6 (icl).
>
> This is somewhat arbitrary still. If we simply want to enable the mode for
> TGL only, easier and cleaned would be to set this flag in
> patch_i915_tgl_hdmi() directly.

OK, will consider it.

Thanks,

Hui.

> Br, Kai