Auxiliary devices in SOF #2501

ranj063 · 2020-10-13T15:41:24Z

Now added support for nocodec machine driver, ipc test client and probes client all using the auxiliary bus

include/sound/sof.h

plbossart

Not fully convinced by the code, the selection between platform device or nocodec mode seems odd.

sound/soc/sof/nocodec.c

sound/soc/sof/sof-audio.c

plbossart

minor nit-picks, also agree with @lyakh that we should create a helper and avoid putting all this code in the middle of sof_machine_register().

plbossart · 2020-10-15T21:32:36Z

sound/soc/sof/nocodec.c

why do we need to add this specifically when changing to the auxiliary_bus?

unfortunately it is needed specifically for auxiliary bus because the naming convention for aux drv uses the KBUILD_MODNAME prefix with the machine driver name provided. But the same change is valid even with the platform driver and probably the right thing to do. Let me submit a separate patch for this.

Right, sound good

sound/soc/sof/sof-audio.c

plbossart

The SOF parts LGTM @ranj063 but still a couple of warnings to fix.

CHECK: spaces preferred around that '+' (ctx:VxV)
#555: FILE: drivers/base/auxiliary.c:234:
+		dlen = strlen(auxdrv->name+1);
 		                          ^

Checking sound/soc/sof/sof-probes-client.c ...
sound/soc/sof/sof-probes-client.c:137:10: warning: %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
   ret = snprintf(buf + strlen(buf), remaining,
         ^

sound/soc/sof/sof-ipc-test-client.c:44:14: style: Variable 'cur' is assigned a value that is never used. [unreadVariable]
 ktime_t cur = ktime_get();
             ^

Checking sound/soc/sof/control.c ...
sound/soc/sof/control.c:117:82: style:inconclusive: Function 'snd_sof_volume_info' argument 2 names different: declaration 'ucontrol' definition 'uinfo'. [funcArgNamesDifferent]
int snd_sof_volume_info(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_info *uinfo)
                                                                                 ^
sound/soc/sof/sof-audio.h:128:30: note: Function 'snd_sof_volume_info' argument 2 names different: declaration 'ucontrol' definition 'uinfo'.
   struct snd_ctl_elem_info *ucontrol);
                             ^
sound/soc/sof/control.c:117:82: note: Function 'snd_sof_volume_info' argument 2 names different: declaration 'ucontrol' definition 'uinfo'.
int snd_sof_volume_info(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_info *uinfo)
                                                                                 ^

Create an SOF client driver for IPC flood test. This driver is used to set up the debugfs entries and the read/write ops for initiating the IPC flood test that would be used to measure the min/max/avg response times for sending IPCs to the DSP. The debugfs ops definitions in the driver is existing code that has been copied from the core. These will be removed from the SOF core making is less monolithic and easier to maintain. Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Co-developed-by: Fred Oh <fred.oh@linux.intel.com> Signed-off-by: Fred Oh <fred.oh@linux.intel.com> Signed-off-by: Dave Ertman <david.m.ertman@intel.com>

Add new ops for registering/unregistering clients based on DSP capabilities and/or DT information. Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Dave Ertman <david.m.ertman@intel.com>

Define client ops for Intel platforms. For now, we only add 2 IPC test clients that will be used for run tandem IPC flood tests for. For ACPI platforms, change the Kconfig to select SND_SOC_SOF_PROBE_WORK_QUEUE to allow the ancillary driver to probe when the client is registered. Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Co-developed-by: Fred Oh <fred.oh@linux.intel.com> Signed-off-by: Fred Oh <fred.oh@linux.intel.com> Signed-off-by: Dave Ertman <david.m.ertman@intel.com>

Remove the IPC flood test support in the SOF core as it is now added in the IPC flood test client. Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Fred Oh <fred.oh@linux.intel.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Dave Ertman <david.m.ertman@intel.com>

Add client APIs to invoke the platform-specific DSP probes ops. Also, add a new API to get the SOF core device pointer which will be used for DMA buffer allocation. Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Tested-by: Fred Oh <fred.oh@linux.intel.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

sof_probe_compr_ops are not platform-specific. So move it to common compress code and export the symbol. The compilation of the common compress code is already dependent on the selection of CONFIG_SND_SOC_SOF_DEBUG_PROBES, so no need to check the Kconfig section for defining sof_probe_compr_ops again. Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Tested-by: Fred Oh <fred.oh@linux.intel.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Add a new client driver for probes support and move all the probes-related code from the core to the client driver. The probes client driver registers a component driver with one CPU DAI driver for extraction and creates a new sound card with one DUMMY DAI link with a dummy codec that will be used for extracting audio data from specific points in the audio pipeline. The probes debugfs ops are based on the initial implementation by Cezary Rojewski and have been moved out of the SOF core into the client driver making it easier to maintain. This change will make it easier for the probes functionality to be added for all platforms without having the need to modify the existing(15+) machine drivers. Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Tested-by: Fred Oh <fred.oh@linux.intel.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Register the client device for probes support on the CNL platform. Creating this client device alleviates the need for modifying the sound card definitions in the existing machine drivers to add support for the new probes feature in the FW. This will result in the creation of a separate sound card that can be used for audio data extraction from user specified points in the audio pipeline. Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Tested-by: Fred Oh <fred.oh@linux.intel.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 · 2020-10-22T19:46:41Z

The SOF parts LGTM @ranj063 but still a couple of warnings to fix.

CHECK: spaces preferred around that '+' (ctx:VxV)
#555: FILE: drivers/base/auxiliary.c:234:
+		dlen = strlen(auxdrv->name+1);
 		                          ^

Checking sound/soc/sof/sof-probes-client.c ...
sound/soc/sof/sof-probes-client.c:137:10: warning: %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
   ret = snprintf(buf + strlen(buf), remaining,
         ^

sound/soc/sof/sof-ipc-test-client.c:44:14: style: Variable 'cur' is assigned a value that is never used. [unreadVariable]
 ktime_t cur = ktime_get();
             ^

Checking sound/soc/sof/control.c ...
sound/soc/sof/control.c:117:82: style:inconclusive: Function 'snd_sof_volume_info' argument 2 names different: declaration 'ucontrol' definition 'uinfo'. [funcArgNamesDifferent]
int snd_sof_volume_info(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_info *uinfo)
                                                                                 ^
sound/soc/sof/sof-audio.h:128:30: note: Function 'snd_sof_volume_info' argument 2 names different: declaration 'ucontrol' definition 'uinfo'.
   struct snd_ctl_elem_info *ucontrol);
                             ^
sound/soc/sof/control.c:117:82: note: Function 'snd_sof_volume_info' argument 2 names different: declaration 'ucontrol' definition 'uinfo'.
int snd_sof_volume_info(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_info *uinfo)
                                                                                 ^

fixed the probes abd ipc test client warnings now but control changes are not relevant to this PR. Maybe they need a separate PR

plbossart · 2020-10-22T20:43:03Z

fixed the probes abd ipc test client warnings now but control changes are not relevant to this PR. Maybe they need a separate PR

oh man, someone AGAIN changed stuff without running cppcheck. this is painful. Will send a patch on this one.

plbossart · 2020-10-22T21:19:09Z

sound/soc/sof/intel/cnl.c

+
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
+	sof_client_dev_unregister(sdev, "probes", 0);
+#endif


can we add the probes for all of APL/CNL/TGL?

I just tried on TGL and nothing. We should add this for all cAVS1.5+ devices. Thanks!

@plbossart unfortunately Dave is having conflicts with TGL on Linus' tree. Thats why I limited the series to just CNL for now

APL should not have any conflicts though?

but the probes functionality was not validated with the APL firmware.

that makes no sense to me. Let's test it then. This only depends on the HDaudio DMA...

@fredoh9 is testing it as we speak. All my APL devices gave up on me

@plbossart I was under impression that the Probes feature is supported from CNL onward.
I built with APL FW with the probes and I'm testing on UP2. Everything seems to be working fine, except crecord's output file is not growing. I set multiple extraction points and it ends up with setting all of buffer widget IDs but no good news. I need to check with FW team for some clarification. I will update again.

Probes is tested in APL UP2. I missed aplay when I tested in APL. That's why its output didn't grow.
Below the diff.txt from @plbossart, it looks great.

plbossart · 2020-10-22T21:44:10Z

@ranj063 suggested patch: apl-cnl-tgl.diff.txt

libinyang · 2020-10-26T08:22:03Z

@ranj063 I tested this patch on my sdw platform, and can't find the 'sof-probes' card in /proc/asound/cards. Do it correctly or not? PS: I'm using the kernel: https://sof-ci-storage.sh.intel.com/pr/linux/pr2501-4763/

Auxiliary devices in SOF feature is introduced in thesofproject/linux#2501, which adds a new sof-probes card in /proc/asound/cards. Old SOF card number acquisition cannot work any more due to this change. Signed-off-by: Amery Song <chao.song@intel.com>

Auxiliary devices in SOF feature is introduced in thesofproject/linux#2501, which adds a new sof-probes card in /proc/asound/cards. Old SOF card number acquisition cannot work any more due to this change. This patch excludes the sof-probes card with a grep. Signed-off-by: Amery Song <chao.song@intel.com>

plbossart · 2020-10-26T16:37:00Z

@ranj063 I tested this patch on my sdw platform, and can't find the 'sof-probes' card in /proc/asound/cards. Do it correctly or not? PS: I'm using the kernel: https://sof-ci-storage.sh.intel.com/pr/linux/pr2501-4763/

@libinyang did you test on a CML platform? If no, probes are not added, see patch above.

fredoh9 · 2020-10-26T19:56:50Z

@libinyang which platform did you try? If you keep not seeing Probe card, I can help you to figure out why Probe soundcard is not enumerated.

libinyang · 2020-10-27T02:06:40Z

@plbossart @fredoh9 I tested it on TGL, not CML. Thanks for the information. I will look for a CML SDW platform to have a test on the userspace, especially for the pulseaudio test.

Auxiliary devices in SOF feature is introduced in thesofproject/linux#2501, and it adds a new sof-probes card in /proc/asound/cards, which will break current test, because we cannot acquire the correct card ID. This patch workarounds the issue by filtering out the sof-probes card with grep. Even it gets merged, our test is safe to run. Signed-off-by: Amery Song <chao.song@intel.com>

ranj063 requested review from bardliao, dbaluta, kv2019i, lgirdwood, libinyang, lyakh and plbossart as code owners October 13, 2020 15:41

plbossart reviewed Oct 13, 2020

View reviewed changes

include/sound/sof.h Outdated Show resolved Hide resolved

thesofproject deleted a comment from ranj063 Oct 13, 2020

plbossart requested changes Oct 13, 2020

View reviewed changes

sound/soc/sof/nocodec.c Outdated Show resolved Hide resolved

sound/soc/sof/sof-audio.c Outdated Show resolved Hide resolved

sound/soc/sof/sof-audio.c Outdated Show resolved Hide resolved

ranj063 force-pushed the topic/nocodec_aux branch 5 times, most recently from 75d190d to 689d3e1 Compare October 14, 2020 05:04

ranj063 requested a review from plbossart October 14, 2020 05:08

lyakh requested changes Oct 15, 2020

View reviewed changes

sound/soc/sof/sof-audio.c Outdated Show resolved Hide resolved

plbossart requested changes Oct 15, 2020

View reviewed changes

ranj063 force-pushed the topic/nocodec_aux branch 4 times, most recently from 316f39a to a08392f Compare October 15, 2020 23:08

ranj063 requested review from lyakh and plbossart October 15, 2020 23:24

ranj063 force-pushed the topic/nocodec_aux branch from a08392f to 30443eb Compare October 16, 2020 04:35

ranj063 requested review from RanderWang, juimonen, keyonjie and xiulipan as code owners October 16, 2020 04:35

ranj063 changed the title ~~Convert nocodec machine drv to auxiliary driver~~ [DO NOT REVIEW YET] Auxiliary devices in SOF Oct 16, 2020

ranj063 force-pushed the topic/nocodec_aux branch from f3ddaf2 to ecec880 Compare October 22, 2020 18:11

plbossart requested changes Oct 22, 2020

View reviewed changes

ranj063 added 8 commits October 22, 2020 12:45

ranj063 force-pushed the topic/nocodec_aux branch from ecec880 to f6ad524 Compare October 22, 2020 19:48

plbossart requested changes Oct 22, 2020

View reviewed changes

ranj063 closed this Oct 23, 2020

This was referenced Oct 23, 2020

Auxiliary devices in SOF break SOFCARD thesofproject/sof-test#466

Closed

Auxiliary devices in SOF break check-ipc-flood.sh (IPC3) thesofproject/sof-test#467

Closed

aiChaoSONG mentioned this pull request Oct 26, 2020

lib.sh: revise card number acquisition code thesofproject/sof-test#474

Merged

xiulipan reopened this Oct 28, 2020

xiulipan closed this Oct 28, 2020

ranj063 deleted the topic/nocodec_aux branch February 13, 2022 05:08

Auxiliary devices in SOF #2501

Auxiliary devices in SOF #2501

Uh oh!

Conversation

ranj063 commented Oct 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranj063 Oct 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

ranj063 commented Oct 22, 2020

Uh oh!

plbossart commented Oct 22, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredoh9 Oct 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plbossart commented Oct 22, 2020

Uh oh!

libinyang commented Oct 26, 2020

Uh oh!

plbossart commented Oct 26, 2020

Uh oh!

fredoh9 commented Oct 26, 2020

Uh oh!

libinyang commented Oct 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ranj063 commented Oct 13, 2020 •

edited

Loading

ranj063 Oct 15, 2020 •

edited

Loading

fredoh9 Oct 26, 2020 •

edited

Loading