| Message ID | 20210225102028.41769-1-hui.wang@canonical.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] ALSA: hda/hdmi: let new platforms assign the pcm slot dynamically | expand |
Hi, thanks! I tested this on a few machines and seems good, no functional issues. On Thu, 25 Feb 2021, Hui Wang wrote: > --- a/sound/pci/hda/patch_hdmi.c > +++ b/sound/pci/hda/patch_hdmi.c > @@ -157,6 +157,7 @@ struct hdmi_spec { > > bool dyn_pin_out; > bool dyn_pcm_assign; > + bool no_fixed_assign; hmm, I wonder if we could come up with a better name. "dyn_pcm_assign" and "no_fixed_assign" seem a bit at odds. How about: bool dyn_pcm_assign; + bool dyn_pcm_no_legacy; Then it's more clear this is still the same dynamic PCM assign policy, but we just drop the legacy exception. Takashi, do you think we can go with this without any method (a module option) to opt-out? I'm ok to go with this, given all users should be using kcontrol status to select the PCM for HDMI/DP output. Any other method is not guaranteed anyways to work in all cases as the assignment is dynamic in the end.. Br, Kai
On Fri, 26 Feb 2021 19:14:32 +0100, Kai Vehmanen wrote: > > Takashi, do you think we can go with this without any method (a module > option) to opt-out? Yes, let's try without a module option at first. I bet no one would be annoyed by this change. > I'm ok to go with this, given all users should be using kcontrol status to > select the PCM for HDMI/DP output. Any other method is not guaranteed > anyways to work in all cases as the assignment is dynamic in the end.. Right. Let's hope that all modern systems already deal with the situation better than ago. thanks, Takashi
On 2/27/21 2:14 AM, Kai Vehmanen wrote: > Hi, > > thanks! I tested this on a few machines and seems good, no functional > issues. > > On Thu, 25 Feb 2021, Hui Wang wrote: > >> --- a/sound/pci/hda/patch_hdmi.c >> +++ b/sound/pci/hda/patch_hdmi.c >> @@ -157,6 +157,7 @@ struct hdmi_spec { >> >> bool dyn_pin_out; >> bool dyn_pcm_assign; >> + bool no_fixed_assign; > hmm, I wonder if we could come up with a better name. "dyn_pcm_assign" > and "no_fixed_assign" seem a bit at odds. > > How about: > bool dyn_pcm_assign; > + bool dyn_pcm_no_legacy; > > Then it's more clear this is still the same dynamic PCM assign policy, but > we just drop the legacy exception. OK, will change to it. Thanks. > Takashi, do you think we can go with this without any method (a module > option) to opt-out? > > I'm ok to go with this, given all users should be using kcontrol status to > select the PCM for HDMI/DP output. Any other method is not guaranteed > anyways to work in all cases as the assignment is dynamic in the end.. > > Br, Kai
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 97adff0cbcab..1aca815b920d 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -157,6 +157,7 @@ struct hdmi_spec { bool dyn_pin_out; bool dyn_pcm_assign; + bool no_fixed_assign; bool intel_hsw_fixup; /* apply Intel platform-specific fixups */ /* * Non-generic VIA/NVIDIA specific @@ -1345,6 +1346,12 @@ static int hdmi_find_pcm_slot(struct hdmi_spec *spec, { int i; + /* on the new machines, try to assign the pcm slot dynamically, + * not use the preferred fixed map anymore. + */ + if (spec->no_fixed_assign) + goto last_try; + /* * generic_hdmi_build_pcms() may allocate extra PCMs on some * platforms (with maximum of 'num_nids + dev_num - 1') @@ -1374,6 +1381,7 @@ static int hdmi_find_pcm_slot(struct hdmi_spec *spec, return i; } + last_try: /* the last try; check the empty slots in pins */ for (i = 0; i < spec->num_nids; i++) { if (!test_bit(i, &spec->pcm_bitmap)) @@ -2988,8 +2996,16 @@ static int patch_i915_tgl_hdmi(struct hda_codec *codec) * the index indicate the port number. */ static const int map[] = {0x4, 0x6, 0x8, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf}; + int ret; - return intel_hsw_common_init(codec, 0x02, map, ARRAY_SIZE(map)); + ret = intel_hsw_common_init(codec, 0x02, map, ARRAY_SIZE(map)); + if (!ret) { + struct hdmi_spec *spec = codec->spec; + + spec->no_fixed_assign = true; + } + + return ret; } /* Intel Baytrail and Braswell; with eld notifier */
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. 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(). This change comes from the discussion between Takashi and Kai Vehmanen. Please refer to: https://github.com/alsa-project/alsa-lib/pull/118 Signed-off-by: Hui Wang <hui.wang@canonical.com> --- Did the test with this patch on 2 TGL machines, and with both legacy HDA driver and sof driver, plug 1 monitor into the machine, plug 2 monitors into the machine and plug 2 monitors into a dp-mst hub, they all worked as expcted, the first 2 pcm slots was assigned to monitors, and playback worked well. sound/pci/hda/patch_hdmi.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)