Prep for adding multiple clients in the SOF driver #1478

ranj063 · 2019-11-13T16:18:31Z

This PR includes the set of patches that prepare the SOF driver for the addition of multiple clients by partitioning the audio-specific code out of the top-level device core and also in the process perform some random cleanups that are otherwise necessary.

ranj063 · 2019-11-13T16:19:49Z

@dbaluta FYI this PR includes your changes from #1448

ranj063 · 2019-11-13T18:40:31Z

@plbossart @dbaluta please hold off on reviewing this series. I see some failure on some devices that I need to fix. I will notify when ive fixed the errors

ranj063 · 2019-11-13T20:43:54Z

@dbaluta @plbossart ready for review now!

plbossart

Looks pretty good, I have a couple of comments that would need to be addressed.
Also we'd need to verify how this impacts the SoundWire machine driver stuff, clearly we are going to see a set of conflicts, so wondering if this should go first.

plbossart · 2019-11-13T22:31:21Z

sound/soc/sof/core.c

commit message: typo on interating.

interact or iterate, pick one :-)

sound/soc/sof/core.c

sound/soc/sof/pm.c

sound/soc/sof/sof-priv.h

sound/soc/sof/core.c

ranj063 · 2019-11-15T00:30:48Z

@plbossart @dbaluta I've updated this PR now based on comments Thanks!

kv2019i

First part of review, some comments but mostly to the commit messages. Will continue review later.

include/sound/sof.h

sound/soc/sof/Makefile

kv2019i

@ranj063 Looks good, only a few minor nits left (see inline)

include/sound/sof.h

kv2019i · 2019-11-19T15:01:07Z

include/sound/sof.h

@ranj063 Again a nit with the commit message: "Set the drv_name and tplg_filename.... in the sof_machine_check op" However this patch does not touch sof_machine_check, making this confusing.

Did you mean that "As the drv_name and tplg_filename for nocodec machine driver is set in the sof_machine_check, sof_nocodec_setup() does not need..." ?

@kv2019i I changed the commit message slightly but the patch does modify sof_machine_check() to set the drv_name for the nocodec case no?

@ranj063 Ah, you are right, I misread. Having rationale why the change is done would be good as well. So I guess here the idea is to decouple sof_nocodec_setup() from acpi mach structure, right? But yeah, this is apparent from the series, so good enough for me.

dbaluta · 2019-11-19T15:45:25Z

@ranj063 one more thing: For patch "ASoC: SOF: Make creation of machine device from SOF core optional" we need to also update sof-of-dev.c file.

I have sent you a patch via email takes care of this. We just need you to apply it on your tree and then squash it under the patch "ASoC: SOF: Make creation of machine device from SOF core optional"

Remove snd_sof_init_topology() as it is never used. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Modify the signature for snd_sof_create_page_table to take struct device pointer as an argument instead of struct snd_sof_dev as this will be used by both the SOF core device and its clients. Also, move the definition out of core.c to utils.c. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

For some platforms, the refcount is explicitly incremented to prevent it from entering runtime suspend. This should be be done during probe in the core instead of being done in the PCM driver. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Currently the FW filename is obtained from the ACPI matching table when determining which machine driver to use. In preparation for making the machine driver ACPI match optional for Device Tree platforms and moving the machine driver selection out of the SOF core, this patch introduces the default_fw_filename member in struct sof_dev_desc. Once the machine driver selection is moved out of SOF core, the nocodec_fw_filename will become obsolete and will be removed. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

Move all the audio-specific code in the core, audio-specific logic in the top-level PM callbacks and the core header files into a separate file (sof-audio.*) in preparation for adding an audio client device. In the process of moving all structure definitions for widget, routes, pcm's etc, the snd_sof_dev member in all these structs is replaced with the snd_soc_component member. Also, use the component device instead of the snd_sof_dev device wherever possible in the PCM component driver, control IO functions and the topology parser as the component device will be moved over to the client device later on. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

The machine driver selection for HDA platforms will be consolidated and moved out of the SOF DSP probe callback. In preparation for that, modify the signature for hda_codec_probe_bus() to pass the hda_codec_use_common_hdmi as a variable while probing the HDA codecs. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 · 2019-11-19T17:26:54Z

@ranj063 one more thing: For patch "ASoC: SOF: Make creation of machine device from SOF core optional" we need to also update sof-of-dev.c file.

I have sent you a patch via email takes care of this. We just need you to apply it on your tree and then squash it under the patch "ASoC: SOF: Make creation of machine device from SOF core optional"

@dbaluta done!

dbaluta · 2019-11-19T21:26:28Z

@ranj063 thanks! Care to also have a look at checkpatch.pl warnings. At least this one can be fixed

CHECK: Please don't use multiple blank lines
#898: FILE: sound/soc/sof/sof-priv.h:523:
 
+

ranj063 · 2019-11-19T21:30:16Z

@ranj063 thanks! Care to also have a look at checkpatch.pl warnings. At least this one can be fixed
CHECK: Please don't use multiple blank lines
#898: FILE: sound/soc/sof/sof-priv.h:523:
 
+

@dbaluta thanks for pointing out. fixed now.

plbossart

LGTM

plbossart · 2019-11-19T23:37:05Z

@ranj063 can you look into build errors?

Currently, SOF probes machine drivers by creating a platform device and passing the machine description as private data. This is driven by the ACPI restrictions. Ideally, ACPI tables should contain the description for the machine driver. This is not possible because ACPI tables are frozen and used on multiple OS-es (e.g Windows). In the case of Device Tree we don't have this restriction, so we choose to probe the machine drivers by creating a DT node as is the standard ALSA way. This patch makes the probing of machine drivers from SOF core optional allowing for Device Tree platforms to decouple the SOF core from machine driver probing. Along with this, it also consolidates the machine driver selection for Intel platforms by defining optional ops for selecting the machine driver based on the ACPI match for HDA and non-HDA platforms and setting the mach params. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Remove nocodec_fw_filename from struct sof_dev_desc as it is not longer needed. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

This field is only set but never used. Let's remove it to make code cleaner. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Set the drv_name and tplg_filename for nocodec machine driver in sof_machine_check(). This means the sof_nocodec_setup() does not need the mach, plat_data or desc arguments any longer. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 · 2019-11-20T00:17:56Z

@ranj063 can you look into build errors?

@plbossart yes fixed now

plbossart

I'd like to merge this, @dbaluta @kv2019i @bardliao can you double-check and approve if you like this PR?

dbaluta · 2019-11-20T06:58:58Z

@plbossart looks good to me! @ranj063 thanks for the patches!

bardliao

LGTM Thanks @ranj063

kv2019i · 2019-11-20T11:40:25Z

include/sound/sof.h

@ranj063 Ah, you are right, I misread. Having rationale why the change is done would be good as well. So I guess here the idea is to decouple sof_nocodec_setup() from acpi mach structure, right? But yeah, this is apparent from the series, so good enough for me.

dbaluta · 2019-11-20T13:18:03Z

@kv2019i yes, the idea is to decouple sof_nocodec_setup() from ACPI.

plbossart · 2019-11-20T15:50:43Z

wow, a ton of reviewers this time, thanks everyone, merging.

xiulipan · 2020-01-15T02:10:49Z

sound/soc/sof/sof-pci-dev.c

 	.chip_info = &jsl_chip_info,
 	.default_fw_path = "intel/sof",
 	.default_tplg_path = "intel/sof-tplg",
-	.nocodec_fw_filename = "sof-jsl.ri",


missed .default_fw_filename = "sof-jsl.ri", here.

ranj063 requested review from RanderWang, bardliao, dbaluta, juimonen, keyonjie, kv2019i, lgirdwood, libinyang, lyakh, plbossart and singalsu as code owners November 13, 2019 16:18

ranj063 force-pushed the topic/multi-client-prep branch 2 times, most recently from ef33314 to e4bbc40 Compare November 13, 2019 18:29

ranj063 force-pushed the topic/multi-client-prep branch 2 times, most recently from 4fa835e to 483039f Compare November 13, 2019 19:55

plbossart requested changes Nov 13, 2019

View reviewed changes

plbossart mentioned this pull request Nov 13, 2019

suspend/resume fix #1479

Closed

ranj063 force-pushed the topic/multi-client-prep branch from 483039f to 551682b Compare November 15, 2019 00:15

This was referenced Nov 15, 2019

[RFC PATCH] Unifiy ACPI & DT handling inside SOF #1315

Closed

Enhancements to prepare supporting machine driver probing via device tree #1448

Closed

kv2019i reviewed Nov 18, 2019

View reviewed changes

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

sound/soc/sof/Makefile Outdated Show resolved Hide resolved

ranj063 force-pushed the topic/multi-client-prep branch from 551682b to 330a400 Compare November 18, 2019 17:48

kv2019i reviewed Nov 19, 2019

View reviewed changes

ranj063 added 2 commits November 19, 2019 09:22

ASoC: SOF: topology: remove snd_sof_init_topology()

81a856e

Remove snd_sof_init_topology() as it is never used. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 added 4 commits November 19, 2019 09:22

ranj063 force-pushed the topic/multi-client-prep branch from 330a400 to 6e08397 Compare November 19, 2019 17:26

ranj063 force-pushed the topic/multi-client-prep branch from 6e08397 to 82ec854 Compare November 19, 2019 21:30

ranj063 force-pushed the topic/multi-client-prep branch from 82ec854 to 150de37 Compare November 19, 2019 22:35

plbossart previously approved these changes Nov 19, 2019

View reviewed changes

ranj063 dismissed plbossart’s stale review via 497d0da November 19, 2019 23:41

ranj063 force-pushed the topic/multi-client-prep branch from 150de37 to 497d0da Compare November 19, 2019 23:41

dbaluta and others added 4 commits November 19, 2019 15:42

ASoC: SOF: remove nocodec_fw_filename

d7ba982

Remove nocodec_fw_filename from struct sof_dev_desc as it is not longer needed. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ASoC: SOF: Remove unused drv_name in sof_pdata

a4e6f10

This field is only set but never used. Let's remove it to make code cleaner. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 force-pushed the topic/multi-client-prep branch from 497d0da to b585b24 Compare November 19, 2019 23:43

plbossart approved these changes Nov 20, 2019

View reviewed changes

dbaluta approved these changes Nov 20, 2019

View reviewed changes

bardliao approved these changes Nov 20, 2019

View reviewed changes

kv2019i approved these changes Nov 20, 2019

View reviewed changes

plbossart merged commit 2824abb into thesofproject:topic/sof-dev Nov 20, 2019

xiulipan reviewed Jan 15, 2020

View reviewed changes

ranj063 deleted the topic/multi-client-prep branch February 13, 2022 05:09

Prep for adding multiple clients in the SOF driver #1478

Prep for adding multiple clients in the SOF driver #1478

Uh oh!

Conversation

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

Uh oh!

Uh oh!

ranj063 commented Nov 13, 2019

Uh oh!

ranj063 commented Nov 13, 2019

Uh oh!

ranj063 commented Nov 13, 2019

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart Nov 13, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ranj063 commented Nov 15, 2019

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kv2019i Nov 19, 2019

Choose a reason for hiding this comment

Uh oh!

ranj063 Nov 19, 2019

Choose a reason for hiding this comment

Uh oh!

kv2019i Nov 20, 2019

Choose a reason for hiding this comment

Uh oh!

dbaluta commented Nov 19, 2019

Uh oh!

ranj063 commented Nov 19, 2019

Uh oh!

dbaluta commented Nov 19, 2019

Uh oh!

ranj063 commented Nov 19, 2019

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart commented Nov 19, 2019

Uh oh!

ranj063 commented Nov 20, 2019

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

dbaluta commented Nov 20, 2019

Uh oh!

bardliao left a comment

Choose a reason for hiding this comment

Uh oh!

kv2019i Nov 20, 2019

Choose a reason for hiding this comment

Uh oh!

dbaluta commented Nov 20, 2019

Uh oh!

plbossart commented Nov 20, 2019

Uh oh!

xiulipan Jan 15, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

ranj063 commented Nov 13, 2019 •

edited

Loading