Skip to content

[V8] Move to snd-hda-codec-hdmi for SOF HDMI/DP support#1155

Closed
kv2019i wants to merge 13 commits intothesofproject:topic/sof-devfrom
kv2019i:topic/hda-hdmi
Closed

[V8] Move to snd-hda-codec-hdmi for SOF HDMI/DP support#1155
kv2019i wants to merge 13 commits intothesofproject:topic/sof-devfrom
kv2019i:topic/hda-hdmi

Conversation

@kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Aug 23, 2019 •
edited
Loading

Here's patchset to move from hdac-hdmi to snd-hda-codec-hdmi to implement HDMI/DP support.
Two machine drivers (UP^2 with Hifiberry and generic HDA-audio) are updated as example.

Related patch to topology to align the PCM interface with legacy driver: TBD -> topology changes not needed after all

Implements #1123
Implements #1078
Fixes #1104
Fixes #1077
Fixes #1375

Event log:

  • V8 (WW44.2) v8 sent to alsa-devel -- a few more comments + sparse warnings
  • V7 (WW43.3) v7 sent to alsa-devel -- addressing Takashi's feedback on inline functions, rebased on Mark's tree
  • V6 (WW41.5) v6 sent to alsa-devel -- fixes [ICL] Resume from S3 fails with snd-hda-codec-hdmi #1263
  • V5 (WW39.3) v5 sent to alsa-devel -- fixed issue on ICL
  • V4 (WW37.4) v4 sent to alsa-devel -- addressed feedback to V3, fix to ICL support
  • V3 (WW37.2) v3 sent to alsa-devel -- addressed feedback to V2, extended series to cover all SOF/HDMI machine drivers, various smaller cleanups
  • V2 (WW35.5) v2 sent to alsa-devel -- implemented runtime selection of driver selection to support distribution kernels; various bugfies
  • V1/rc4 (WW35.4) added chmap support; patch sent upstream to alsa-devel
  • prelist-rc3 (WW35.3) dynamically parse PCM ids from topology at probe; multiple clean ups
  • prelist-rc2 (WW35.2) v2 patchset, added a mechanism to pass PCM device numbers from machine driver to HDA codec driver; lots of cleanups; tested with PA12.2; added support for skl-hda-generic-dsp
  • prelist-rc1 (WW34): initial patchset

Currently tested:

  • eld procfs is working with SOF
  • single-monitor playback is working
  • playback to different monitors is working (HDMI and DP)
  • concurrent playback to two monitors (HDMI+DP)
  • concurrent playback to three monitors (HDMI + 2xDP in MST mode)
  • jack detection works (as expected by e.g. Pulseaudio)
  • jack detection works without changing topology with Pulseaudio 12.x and newer

Open issues list: NONE, no open issues left

  • higher rate of failures on ICL in regression test -> fixed with V5 series WW39 and display driver patches WW40
  • concept of backup/virtual PCMs used by "dyn-pcm-assign" mode of hda-codec-hdmi does not work well with ASoC, SOF and topology
  • how do we pass the "SOF mode" info to hda-codec-hdmi? new hda-codec flag added
  • chmap ctl registration is broken
  • naming of some entities should be revisioted (e.g. get rid of the "hifiX" names) -> fixed in v2
  • plan needed how/which machine drivers to update (on need basis, or move over all)
  • compatibility of machine drivers with SST driver
  • test legacy driver works with the patch_hdmi.c modifications in this set

@kv2019i kv2019i added the upstream Patch has been sent to upstream (e.g. alsa-devel) label Aug 23, 2019
@kv2019i kv2019i mentioned this pull request Aug 23, 2019
Closed
@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 23, 2019

@libinyang and @xiulipan : can you check the thread on https://mailman.alsa-project.org/pipermail/alsa-devel/2019-August/154561.html about the dyn-pcm-assign mode in patch_hdmi.c.

It would seem we are going towards DP-MST without the backup PCMs. Can you recall any critical problems with this approach (i.e. reasons for originally adding the backup PCMs to DP-MST support).

plbossart
plbossart reviewed Aug 23, 2019
View reviewed changes
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty heavy-duty stuff @kv2019i, thanks for sharing.

I am not clear on what we call the PCMs, and how we will refer to HDMI 'pcms' from a topology file. Or if we assume the topology is bypassed completely.

I would also suggest using the Up2 board without any codec, that is an excellent way of testing if plain vanilla HDMI works and with the same capabilities as legacy HDaudio.

@mengdonglin mengdonglin added the reuse legacy HDMI codec Reuse legacy HDA HDMI codec driver snd-hda-codec-hdmi label Aug 26, 2019
@libinyang

This comment has been minimized.

Sign in to view
ranj063
ranj063 reviewed Aug 26, 2019
View reviewed changes
Copy link
Collaborator

@ranj063 ranj063 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kv2019i no sign off in this patch

ranj063
ranj063 reviewed Aug 26, 2019
View reviewed changes
@kv2019i

This comment has been minimized.

Sign in to view
ranj063
ranj063 reviewed Aug 26, 2019
View reviewed changes
ranj063
ranj063 reviewed Aug 26, 2019
View reviewed changes
@libinyang

This comment has been minimized.

Sign in to view
@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 27, 2019

@libinyang Thanks for quick response? Can you recall what was the specific case? I was looking at Pulseaudio code and it seems to be ok as long as we provide "HDMI/DP,pcm=X Jack" card control that links to correct PCM. Or was there was problem when connection more than 3 monitors?

@kv2019i There is no real case when we discussed. It's not related to the monitors number being more than 3. I tried to find the old emails, but failed. I remember PA said they will assume 3, 7, 8 will be assigned normally. And they hope if the there is DPMST audio, the second DPMST audio should fristly tried to another pcm number. So we created pcm 9, pcm 10. The policy is if it is the second DPMST audio device (let's assume pcm 3 is assigned to the first DPMST audio device) on the same DDI port, it should be assigned to pcm 9, instead of pcm 7.

@libinyang Thanks for the response. I found at least one related thing that has been fixed in PA. It seems that in Pulseaudio 11.x, there is no ELD autodetect support yet, so the PCM numbers have to be hardcoded in Pulseaudio paths files. This PA version is in e.g. 18.04 Ubuntu. Pulseaudio 12.x has ELD autodetect and this version is in Ubuntu 18.10 and newer. I think this is something we can live with SOF -> in practise recommend to use at least PA 12.x to make HDMI autodetect work.

As for assigning pcms 9 and 10, that will still be an issue. I'll ask about this on pulseaudio devel.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 27, 2019

v2 patch uploaded, changes: added a mechanism to pass PCM device numbers from machine driver to HDA codec driver; lots of cleanups; tested with PA12.2; added support for skl-hda-generic-dsp

@kv2019i kv2019i mentioned this pull request Aug 27, 2019
Closed
@libinyang
Copy link

libinyang commented Aug 28, 2019

@libinyang Thanks for quick response? Can you recall what was the specific case? I was looking at Pulseaudio code and it seems to be ok as long as we provide "HDMI/DP,pcm=X Jack" card control that links to correct PCM. Or was there was problem when connection more than 3 monitors?

@kv2019i There is no real case when we discussed. It's not related to the monitors number being more than 3. I tried to find the old emails, but failed. I remember PA said they will assume 3, 7, 8 will be assigned normally. And they hope if the there is DPMST audio, the second DPMST audio should fristly tried to another pcm number. So we created pcm 9, pcm 10. The policy is if it is the second DPMST audio device (let's assume pcm 3 is assigned to the first DPMST audio device) on the same DDI port, it should be assigned to pcm 9, instead of pcm 7.

@libinyang Thanks for the response. I found at least one related thing that has been fixed in PA. It seems that in Pulseaudio 11.x, there is no ELD autodetect support yet, so the PCM numbers have to be hardcoded in Pulseaudio paths files. This PA version is in e.g. 18.04 Ubuntu. Pulseaudio 12.x has ELD autodetect and this version is in Ubuntu 18.10 and newer. I think this is something we can live with SOF -> in practise recommend to use at least PA 12.x to make HDMI autodetect work.

As for assigning pcms 9 and 10, that will still be an issue. I'll ask about this on pulseaudio devel.

@kv2019i Yes, the old PA may not support pcm 9 and pcm 10 well. But at least, it will not break the normal pcm 3, pcm 7, pcm 8. :)

@kv2019i kv2019i force-pushed the topic/hda-hdmi branch 3 times, most recently from a4b8725 to a5b1988 Compare August 28, 2019 15:34
@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 28, 2019

v3 patch uploaded: dynamically parse PCM ids from topology at probe; multiple clean ups

This starts to look good. Unless any showstoppers seen, I'll send this to alsa-devel tomorrow for wider discussion. Aside testing, biggest remaining open is whether we switch over all machine drivers at once or not. Current patch basicly requires to switch all machine drivers at once, so for merging, all machine drivers (for SOF-supported devices) need to be updated in the same series.

kv2019i
kv2019i commented Aug 28, 2019
View reviewed changes
ranj063
ranj063 reviewed Aug 28, 2019
View reviewed changes
ranj063
ranj063 reviewed Aug 28, 2019
View reviewed changes
plbossart
plbossart reviewed Aug 28, 2019
View reviewed changes
Copy link
Member

@plbossart plbossart left a comment •
edited by kv2019i
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart wrote:

I would also have expected the hdac_hdmi code to be removed, and all kconfigs to be updated?

Let's discuss that on alsa-devel. That's the big question for merging. Change of codec driver will be visible to applications -- the mixer interface is different, jack detection works differently (affects e.g. UCM). I guess what this comes down to is that can we just go ahead and make the change, or do we need to add Kconfig option to allow people to opt-in for the new behavior. Latter means we need to keep hdac_hdmi around, and potentially keep the support for both options in all machine drivers as well.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 29, 2019

@kv2019i wrote:

As for assigning pcms 9 and 10, that will still be an issue. I'll ask about this on pulseaudio devel.

Ok confirmed with PA developers. This is no longer an issue -> https://lists.freedesktop.org/archives/pulseaudio-discuss/2019-August/031358.html

@kv2019i
Copy link
Collaborator Author

kv2019i commented Oct 11, 2019

https://mailman.alsa-project.org/pipermail/alsa-devel/2019-October/156636.html

@kv2019i
ALSA: hda/hdmi - implement mst_no_extra_pcms flag
63ab86c 
To support the DP-MST multiple streams via single connector feature,
the HDMI driver was extended with the concept of backup PCMs. See
commit 9152085 ("ALSA: hda - add DP MST audio support").

This implementation works fine with snd_hda_intel.c as PCM topology
is fully managed within the single driver.

When the HDA codec driver is used from ASoC components, the concept
of backup PCMs no longer fits. For ASoC topologies, the physical
HDMI converters are presented as backend DAIs and these should match
with hardware capabilities. The ASoC topology may define arbitrary
PCMs (i.e. frontend DAIs) and have processing elements before eventual
routing to the HDMI BE DAIs. With backup PCMs, the link between
FE and BE DAIs would become dynamic and change when monitors are
(un)plugged. This would lead to modifying the topology every time
hotplug events happen, which is not currently possible in ASoC and
there does not seem to be any obvious benefits from this design.

To overcome above problems and enable the HDMI driver to be used
from ASoC, this patch adds a new mode (mst_no_extra_pcms flags) to
patch_hdmi.c. In this mode, the codec driver does not assume
the backup PCMs to be created.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@kv2019i
ASoC: hdac_hda: add support for HDMI/DP as a HDA codec
aea20ac 
Handle all HDA codecs using same logic, including HDMI/DP.

Call to snd_hda_codec_build_controls() is delayed for HDMI/DP HDA
devices. This is needed to discover the PCM device numbers as
defined in topology.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
Copy link
Collaborator Author

kv2019i commented Oct 22, 2019 •
edited
Loading

Update to v7

@plbossart Do you want to take this via your pull-req, or should I sent a separate v007 mail to alsa-devel and to Mark

Update: I made this non-draft in case we decide to merge this via sof-dev.
Update2: Saw Pierre's comment on alsa-devel -> will go via mailing list directly.

@kv2019i kv2019i changed the title [RFC] Move to snd-hda-codec-hdmi for SOF HDMI/DP support Move to snd-hda-codec-hdmi for SOF HDMI/DP support Oct 22, 2019
@kv2019i kv2019i marked this pull request as ready for review October 22, 2019 17:17
@kv2019i kv2019i requested a review from lgirdwood as a code owner October 22, 2019 17:17
@kv2019i
ASoC: Intel: skl-hda-dsp-generic: use snd-hda-codec-hdmi
ce63e74 
Add support for using snd-hda-codec-hdmi driver for HDMI/DP
instead of ASoC hdac-hdmi. This is aligned with how other
HDA codecs are already handled.

When snd-hda-codec-hdmi is used, the PCM device numbers are
parsed from card topology and passed to the codec driver.
This needs to be done at runtime as topology changes may
affect PCM device allocation.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
ASoC: Intel: skl-hda-dsp-generic: fix include guard name
baef39d 
Match the include guard define to actual filename. The source
directory now has an actual hda_dsp_common.h header, so the old
include guard may cause confusion.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@kv2019i
ASoC: SOF: Intel: add support for snd-hda-codec-hdmi
d489c3e 
Add support to implement HDMI/DP audio by using the common
snd-hda-codec-hdmi driver.

Change of codec driver affects user-space as the two
drivers expose different mixer controls. A new kernel
module option "use_common_hdmi" is added to user-space
to indicate which interface should be used. The default
driver can be selected via a Kconfig option.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
ASoC: Intel: bxt-da7219-max98357a: common hdmi codec support
0929c20 
Add support for using snd-hda-codec-hdmi driver for HDMI/DP
instead of ASoC hdac-hdmi. This is aligned with how other
HDA codecs are already handled.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
ASoC: Intel: glk_rt5682_max98357a: common hdmi codec support
31a5d18 
Add support for using snd-hda-codec-hdmi driver for HDMI/DP
instead of ASoC hdac-hdmi. This is aligned with how other
HDA codecs are already handled.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
ASoC: intel: sof_rt5682: common hdmi codec support
93d6ff5 
Add support for using snd-hda-codec-hdmi driver for HDMI/DP
instead of ASoC hdac-hdmi. This is aligned with how other
HDA codecs are already handled.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
ASoC: Intel: bxt_rt298: common hdmi codec support
15861f9 
Add support for using snd-hda-codec-hdmi driver for HDMI/DP
instead of ASoC hdac-hdmi. This is aligned with how other
HDA codecs are already handled.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
ASoC: Intel: bxt_pcm512x: use snd-hda-codec-hdmi
e64932b 
Add support for using snd-hda-codec-hdmi driver for HDMI/DP
instead of ASoC hdac-hdmi. This is aligned with how other HDA
codecs are already handled.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
Copy link
Collaborator Author

kv2019i commented Oct 23, 2019

  • V7 (WW43.3) v7 sent to alsa-devel -- addressing Takashi's feedback on inline functions, rebased on Mark's tree

https://mailman.alsa-project.org/pipermail/alsa-devel/2019-October/157201.html

@kv2019i kv2019i changed the title Move to snd-hda-codec-hdmi for SOF HDMI/DP support [V7] Move to snd-hda-codec-hdmi for SOF HDMI/DP support Oct 24, 2019
This was referenced Oct 24, 2019
Closed
Closed
@kv2019i kv2019i changed the title [V7] Move to snd-hda-codec-hdmi for SOF HDMI/DP support [V8] Move to snd-hda-codec-hdmi for SOF HDMI/DP support Oct 29, 2019
@kv2019i
Copy link
Collaborator Author

kv2019i commented Oct 29, 2019

V8 sent to alsa-devel. I'll stop refreshing the series to this PR and only push incremental changes to keep in sync

@ranj063
Copy link
Collaborator

ranj063 commented Nov 1, 2019

@kv2019i we can close this one right? Its upstream and merged to sof-dev too

@macchian macchian mentioned this pull request Nov 4, 2019
Closed
@kv2019i
Copy link
Collaborator Author

kv2019i commented Nov 4, 2019

@ranj063 wrote:

@kv2019i we can close this one right? Its upstream and merged to sof-dev too

Yes! This I will gladly do! :)

@kv2019i kv2019i closed this Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@plbossart plbossart plbossart left review comments

@ranj063 ranj063 ranj063 left review comments

@keyonjie keyonjie Awaiting requested review from keyonjie

@libinyang libinyang Awaiting requested review from libinyang

@xiulipan xiulipan Awaiting requested review from xiulipan

@lgirdwood lgirdwood Awaiting requested review from lgirdwood lgirdwood is a code owner

Assignees

No one assigned

Labels

reuse legacy HDMI codec Reuse legacy HDA HDMI codec driver snd-hda-codec-hdmi upstream Patch has been sent to upstream (e.g. alsa-devel)

Projects

None yet

Milestone

No milestone

5 participants

@kv2019i @libinyang @ranj063 @plbossart @mengdonglin