Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASoC: SOF: add sof audio mediator capture support #197

Conversation

libinyang
Copy link

This patch adds the capture support for SOF audio mediator.

In the vBE, it adds a BE dai_link for capture.

The dai_link name "vm#n_dai_linkP/C". n means the vm_id of the vFE.
P/C means playback or capture.

Signed-off-by: Libin Yang [email protected]

This patch adds the capture support for SOF audio mediator.

In the vBE, it adds a BE dai_link for capture.

The dai_link name "vm#n_dai_linkP/C". n means the vm_id of the vFE.
P/C means playback or capture.

Signed-off-by: Libin Yang <[email protected]>
@libinyang
Copy link
Author

@lgirdwood
@plbossart
Today github is not stable, I can't add reviewers. I will add you as reviewer later.

Regards.
Libin

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.

I don't get what you are trying to do with SSPs exposed to a virtio front-end.

@@ -206,8 +206,8 @@ static struct snd_soc_dai_link broxton_tdf8532_dais[] = {
.no_pcm = 1,
},
{
/* SSP4 for vm */
.name = "vm_dai_link",
/* SSP4 for vm1 */
Copy link
Member

Choose a reason for hiding this comment

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

What? Why would a VM be aware of which SSP is used for the amplifier? This makes no sense to me, what are you trying to do here?

Copy link
Author

Choose a reason for hiding this comment

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

This setting is not in virtual machine, but in SOS. This is the vBE BE dai_link. vBE should know the HW topology. With this HW information, vBE driver knows which codec it is using and then asoc can setup the codec.

@@ -217,6 +217,19 @@ static struct snd_soc_dai_link broxton_tdf8532_dais[] = {
.dpcm_playback = 1,
.no_pcm = 1,
},
{
/* SSP2 Dirana for vm1 */
.name = "vm1_dai_linkC",
Copy link
Member

Choose a reason for hiding this comment

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

same here, why would one add a new BE dailink? This is not quite right.

Copy link
Author

Choose a reason for hiding this comment

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

the upper dai_link is SSP4 connecting to codec for playback. This dai_link is SSP2 connecting Dirana for capture. They are different HW topology in the HW and SOS driver should know it.

Copy link
Member

Choose a reason for hiding this comment

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

No. There should be no additional DAI links. the BE should use a PCM FE and rely on firmware to route PCM samples to the DAI. what you are implementing is hardware partitioning, this is not what we want. The Virtio BE shall not deal directly with DAI links or even know about them.

Copy link
Author

Choose a reason for hiding this comment

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

"the BE should use a PCM FE and rely on firmware to route PCM samples to the DAI" works for the native driver without audio mediator support. However audio mediator is a little different.

In native driver, user opens PCM0. The audio driver knows everything, such as which codec dai it is using (codec dai is bound to the BE dai link and so it is bound to the PCM device). So ASoC will setup the codec and so on.

Please image:
User triggers playback in vFE. vFE notifies vBE we will playback on the PCM0 in vFE (PCM0 device is in vFE. vBE's PCM0 is different from vFE's PCM0. vBE's PCM0 is exposed to the user in SOS). So vBE must route PCM samples to the correct DAI. vBE get the playback trigger from vFE. It is not a trigger from vBE userspace. So ASoC will not set the HW setting automatically. Please notice, HW setting which ASoC sets is triggered by the userspace. However, playback in vFE will not trigger SOS ASoC operating. So vBE must handle the setting itself.

in native:
userspace <--> alsa <---> ASoC <---> cpu dai/codec dai

in audio mediator:
vFE userspace <---> vFE alsa <---> vFE ASoC <---> vFE audio driver <---> vBE audio driver <---> (SOF <---> cpu_dai)/codec dai

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I don't get at all why you would need the vBE to know anything about DAIs. I am not going to continue this discussion and will revert everything related to virtualization in sof-dev since it's a distraction at this point.

@@ -438,7 +445,7 @@ struct snd_sof_dsp_ops snd_sof_vfe_ops = {

/* DAI drivers */
.drv = virtio_dai,
.num_drv = 1,
.num_drv = 2,
Copy link
Member

Choose a reason for hiding this comment

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

I don't get what SSP have to do in a file called virtio-fe. What sort of design are you implementing here? This is not mediation if you let each FE get access to a peripheral.

Copy link
Author

@libinyang libinyang Oct 23, 2018

Choose a reason for hiding this comment

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

virtio-fe is the uos virtual device driver. Comparing with the SOS vBE driver, vFE driver doesn't know anything about the real HW connection. vFE will not operate directly on the HW, it will mediate with vBE with virtqueue, and vBE will operate on the real HW.

So, the design is, vFE1 <---> vBE, vFE2 <----> vBE; vBE <----> HW. With this mediator, every UOS can playback/capture. Otherwise, if vFE1 operates directly on the HW, other vFEs can't work.

Copy link
Member

Choose a reason for hiding this comment

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

again no. vBE <--> PCM device, not hardware.

Copy link
Author

Choose a reason for hiding this comment

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

I skip the PCM step here. Actually it should be:
vBE ---> find the PCM device related dai link ---> HW setting.

We didn't let vBE operate the PCM device directly because vFE we are still using the SOF architecture. So we can benefit from it and directly work with SOF to improve the performance.

vBE directly operates on PCM deivce is a VBS-U solution. The performance is worse than VBS-K. This is why we didn't operate on PCM device directly.

@libinyang
Copy link
Author

Can you move all these virtio dai to another file as this file should be generic

Sure. I will.

@libinyang
Copy link
Author

Can you move all these virtio dai to another file as this file should be generic

Sure. I will.

When I try to move them to a separate file, I found it is a little hard to do. Most of them are generic, but the HW connection topology is specific, such as which codec it is connecting with. This is HW dependent. Do you have any suggestion?

@lgirdwood
Copy link
Member

@libinyang I though all your hard coded topology was only temporary code and would be removed once your topology file loading was completed ?

I assume you mean the machine driver BE DAI links as your "HW connection topology" ? If so the BE DAI links are already defined in the SOS machine driver, they do not need any redefined for vBE and vFE should never need them..

@plbossart
Copy link
Member

This PR will not be merged, too controversial and not a priority so closing. we may reopen when we have more time but for the time being it's a distraction.

@plbossart plbossart closed this Oct 24, 2018
@libinyang
Copy link
Author

@lgirdwood Yes. The new topology is in developing. I have made about 50% progress. Currently, Jocelyn has assigned a SOF bug to me. I will mainly focus on SOF buf fixing now. As Pierre mentioned, it is not a priority and a distraction, I will pend the virtualization now and will continue on the audio mediator after I fixed some SOF bugs. Let's discuss the audio mediator later.

@plbossart OK. I will pend the audio mediator task and will focus on the SOF driver. Let's discuss it later.

Thank you, Pierre and Liam, for your review on the audio mediator patch.

bardliao pushed a commit to bardliao/linux that referenced this pull request Aug 22, 2024
Fix invalid access to pgdat during hot-remove operation:
ndctl users reported a GPF when trying to destroy a namespace:
$ ndctl destroy-namespace all -r all -f
 Segmentation fault
 dmesg:
 Oops: general protection fault, probably for
 non-canonical address 0xdffffc0000005650: 0000 [#1] PREEMPT SMP KASAN
 PTI
 KASAN: probably user-memory-access in range
 [0x000000000002b280-0x000000000002b287]
 CPU: 26 UID: 0 PID: 1868 Comm: ndctl Not tainted 6.11.0-rc1 #1
 Hardware name: Dell Inc. PowerEdge R640/08HT8T, BIOS
 2.20.1 09/13/2023
 RIP: 0010:mod_node_page_state+0x2a/0x110

cxl-test users report a GPF when trying to unload the test module:
$ modrpobe -r cxl-test
 dmesg
 BUG: unable to handle page fault for address: 0000000000004200
 #PF: supervisor read access in kernel mode
 #PF: error_code(0x0000) - not-present page
 PGD 0 P4D 0
 Oops: Oops: 0000 [#1] PREEMPT SMP PTI
 CPU: 0 UID: 0 PID: 1076 Comm: modprobe Tainted: G O N 6.11.0-rc1 thesofproject#197
 Tainted: [O]=OOT_MODULE, [N]=TEST
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/15
 RIP: 0010:mod_node_page_state+0x6/0x90

Currently, when memory is hot-plugged or hot-removed the accounting is
done based on the assumption that memmap is allocated from the same node
as the hot-plugged/hot-removed memory, which is not always the case.

In addition, there are challenges with keeping the node id of the memory
that is being remove to the time when memmap accounting is actually
performed: since this is done after remove_pfn_range_from_zone(), and
also after remove_memory_block_devices(). Meaning that we cannot use
pgdat nor walking though memblocks to get the nid.

Given all of that, account the memmap overhead system wide instead.

For this we are going to be using global atomic counters, but given that
memmap size is rarely modified, and normally is only modified either
during early boot when there is only one CPU, or under a hotplug global
mutex lock, therefore there is no need for per-cpu optimizations.

Also, while we are here rename nr_memmap to nr_memmap_pages, and
nr_memmap_boot to nr_memmap_boot_pages to be self explanatory that the
units are in page count.

[[email protected]: address a few nits from David Hildenbrand]
  Link: https://lkml.kernel.org/r/[email protected]
Link: https://lkml.kernel.org/r/[email protected]
Link: https://lkml.kernel.org/r/[email protected]
Fixes: 15995a3 ("mm: report per-page metadata information")
Signed-off-by: Pasha Tatashin <[email protected]>
Reported-by: Yi Zhang <[email protected]>
Closes: https://lore.kernel.org/linux-cxl/CAHj4cs9Ax1=CoJkgBGP_+sNu6-6=6v=_L-ZBZY0bVLD3wUWZQg@mail.gmail.com
Reported-by: Alison Schofield <[email protected]>
Closes: https://lore.kernel.org/linux-mm/Zq0tPd2h6alFz8XF@aschofie-mobl2/#t
Tested-by: Dan Williams <[email protected]>
Tested-by: Alison Schofield <[email protected]>
Acked-by: David Hildenbrand <[email protected]>
Acked-by: David Rientjes <[email protected]>
Tested-by: Yi Zhang <[email protected]>
Cc: Domenico Cerasuolo <[email protected]>
Cc: Fan Ni <[email protected]>
Cc: Joel Granados <[email protected]>
Cc: Johannes Weiner <[email protected]>
Cc: Li Zhijian <[email protected]>
Cc: Matthew Wilcox (Oracle) <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Muchun Song <[email protected]>
Cc: Nhat Pham <[email protected]>
Cc: Sourav Panda <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: Yosry Ahmed <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants