All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kai Vehmanen <kai.vehmanen@linux.intel.com>
To: Hui Wang <hui.wang@canonical.com>
Cc: Takashi Iwai <tiwai@suse.de>,
	alsa-devel@alsa-project.org, kai.vehmanen@linux.intel.com
Subject: Re: [PATCH] ALSA: hda/hdmi: let new platforms assign the pcm slot dynamically
Date: Tue, 23 Feb 2021 16:14:00 +0200 (EET)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2102231545050.864696@eliteleevi.tm.intel.com> (raw)
In-Reply-To: <20210223122205.233701-1-hui.wang@canonical.com>

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.

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?

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.

Br, Kai

  parent reply	other threads:[~2021-02-23 14:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-23 12:22 [PATCH] ALSA: hda/hdmi: let new platforms assign the pcm slot dynamically Hui Wang
2021-02-23 13:09 ` Takashi Iwai
2021-02-24  2:24   ` Hui Wang
2021-02-23 14:14 ` Kai Vehmanen [this message]
2021-02-23 16:25   ` Jaroslav Kysela
2021-02-23 16:33     ` Takashi Iwai
2021-02-23 17:51       ` Kai Vehmanen
2021-02-24  2:22         ` Hui Wang
2021-02-24 10:17           ` Kai Vehmanen
2021-02-25  8:35             ` Hui Wang
2021-02-24  2:32   ` Hui Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.22.394.2102231545050.864696@eliteleevi.tm.intel.com \
    --to=kai.vehmanen@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=hui.wang@canonical.com \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.