diff mbox series

ASoC: SOF: build compression interface into snd_sof.ko

Message ID 20211108111132.3800548-1-arnd@kernel.org (mailing list archive)
State Accepted
Commit 1218f06cb3c6e2c51699998bc17c0d9a41ab37a6
Headers show
Series ASoC: SOF: build compression interface into snd_sof.ko | expand

Commit Message

Arnd Bergmann Nov. 8, 2021, 11:11 a.m. UTC
From: Arnd Bergmann <arnd@arndb.de>

With CONFIG_SND_SOC_SOF_COMPRESS=m, the compression code is
not built into a the main SOF driver when that is built-in:

x86_64-linux-ld: sound/soc/sof/ipc.o: in function `ipc_stream_message':
ipc.c:(.text+0x5a2): undefined reference to `snd_sof_compr_fragment_elapsed'
x86_64-linux-ld: sound/soc/sof/topology.o: in function `sof_dai_load':
topology.c:(.text+0x32d1): undefined reference to `snd_sof_compr_init_elapsed_work'
x86_64-linux-ld: topology.c:(.text+0x32e1): undefined reference to `snd_sof_compr_init_elapsed_work'

Make this a 'bool' symbol so it just decides whether the
code gets built at all.

Fixes: 858f7a5c45ca ("ASoC: SOF: Introduce fragment elapsed notification API")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 sound/soc/sof/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Pierre-Louis Bossart Nov. 8, 2021, 1:39 p.m. UTC | #1
On 11/8/21 5:11 AM, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> With CONFIG_SND_SOC_SOF_COMPRESS=m, the compression code is
> not built into a the main SOF driver when that is built-in:
> 
> x86_64-linux-ld: sound/soc/sof/ipc.o: in function `ipc_stream_message':
> ipc.c:(.text+0x5a2): undefined reference to `snd_sof_compr_fragment_elapsed'
> x86_64-linux-ld: sound/soc/sof/topology.o: in function `sof_dai_load':
> topology.c:(.text+0x32d1): undefined reference to `snd_sof_compr_init_elapsed_work'
> x86_64-linux-ld: topology.c:(.text+0x32e1): undefined reference to `snd_sof_compr_init_elapsed_work'
> 
> Make this a 'bool' symbol so it just decides whether the
> code gets built at all.
> 
> Fixes: 858f7a5c45ca ("ASoC: SOF: Introduce fragment elapsed notification API")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

It's Monday morning and my memory is still foggy but I think we fixed
this problem with https://github.com/thesofproject/linux/pull/3180,
where we changed the Kconfigs for i.MX. We haven't sent this update
upstream for some reason.

Arnd, can you share the configuration that breaks with the existing
upstream code, I can check if the problem still exists.

Thanks!
Daniel Baluta Nov. 8, 2021, 2:13 p.m. UTC | #2
On 11/8/21 3:39 PM, Pierre-Louis Bossart wrote:
>
> On 11/8/21 5:11 AM, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> With CONFIG_SND_SOC_SOF_COMPRESS=m, the compression code is
>> not built into a the main SOF driver when that is built-in:
>>
>> x86_64-linux-ld: sound/soc/sof/ipc.o: in function `ipc_stream_message':
>> ipc.c:(.text+0x5a2): undefined reference to `snd_sof_compr_fragment_elapsed'
>> x86_64-linux-ld: sound/soc/sof/topology.o: in function `sof_dai_load':
>> topology.c:(.text+0x32d1): undefined reference to `snd_sof_compr_init_elapsed_work'
>> x86_64-linux-ld: topology.c:(.text+0x32e1): undefined reference to `snd_sof_compr_init_elapsed_work'
>>
>> Make this a 'bool' symbol so it just decides whether the
>> code gets built at all.
>>
>> Fixes: 858f7a5c45ca ("ASoC: SOF: Introduce fragment elapsed notification API")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> It's Monday morning and my memory is still foggy but I think we fixed
> this problem with https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fthesofproject%2Flinux%2Fpull%2F3180&amp;data=04%7C01%7Cdaniel.baluta%40nxp.com%7C25ac869cfd1040f1be1708d9a2bd3460%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637719755777370422%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=E4K2DPkpLX2SgVJ1K99Qs3uz7l7mS96gIzYlJw9akbg%3D&amp;reserved=0,
> where we changed the Kconfigs for i.MX. We haven't sent this update
> upstream for some reason.
>
> Arnd, can you share the configuration that breaks with the existing
> upstream code, I can check if the problem still exists.


Maybe someone forgot :) to send 
https://github.com/thesofproject/linux/pull/3180/commits/7122edc88d13db8ba835bdb20f7444ae535f9ffa 
upstream.

I think that's me.

Arnd can you run your scripts with 
https://github.com/thesofproject/linux/pull/3180/commits/7122edc88d13db8ba835bdb20f7444ae535f9ffa. 
I also attached the patch

if it's easier to apply.
Arnd Bergmann Nov. 8, 2021, 4:15 p.m. UTC | #3
On Mon, Nov 8, 2021 at 3:13 PM Daniel Baluta <daniel.baluta@nxp.com> wrote:
> On 11/8/21 3:39 PM, Pierre-Louis Bossart wrote:
> > On 11/8/21 5:11 AM, Arnd Bergmann wrote:
> >> From: Arnd Bergmann <arnd@arndb.de>
> >>
> >> Fixes: 858f7a5c45ca ("ASoC: SOF: Introduce fragment elapsed notification API")
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > It's Monday morning and my memory is still foggy but I think we fixed
> > this problem with https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fthesofproject%2Flinux%2Fpull%2F3180&amp;data=04%7C01%7Cdaniel.baluta%40nxp.com%7C25ac869cfd1040f1be1708d9a2bd3460%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637719755777370422%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=E4K2DPkpLX2SgVJ1K99Qs3uz7l7mS96gIzYlJw9akbg%3D&amp;reserved=0,
> > where we changed the Kconfigs for i.MX. We haven't sent this update
> > upstream for some reason.
> >
> > Arnd, can you share the configuration that breaks with the existing
> > upstream code, I can check if the problem still exists.

https://pastebin.com/6JqM6Gkr

> Maybe someone forgot :) to send
> https://github.com/thesofproject/linux/pull/3180/commits/7122edc88d13db8ba835bdb20f7444ae535f9ffa
> upstream.
>
> I think that's me.
>
> Arnd can you run your scripts with
> https://github.com/thesofproject/linux/pull/3180/commits/7122edc88d13db8ba835bdb20f7444ae535f9ffa.
> I also attached the patch

I added that to my randconfig tree now. I did a cherry-pick of that
commit, I tried a 'git merge'
first, but that pulled in a lot of other stuff that conflicts with linux-next.

Your patch does not fix the link error though, I'm fairly sure we need
my one-liner
on top still. In this case, SND_SOC_SOF=y gets selected by
SND_SOC_SOF_INTEL_COMMON=y, while SND_SOC_SOF_COMPRESS=m gets
selected by SND_SOC_SOF_IMX_COMMON=m, and then ignored because the Makefile
only cares about =y objects built into the common module.

       Arnd
Pierre-Louis Bossart Nov. 8, 2021, 5:18 p.m. UTC | #4
On 11/8/21 10:15 AM, Arnd Bergmann wrote:
> On Mon, Nov 8, 2021 at 3:13 PM Daniel Baluta <daniel.baluta@nxp.com> wrote:
>> On 11/8/21 3:39 PM, Pierre-Louis Bossart wrote:
>>> On 11/8/21 5:11 AM, Arnd Bergmann wrote:
>>>> From: Arnd Bergmann <arnd@arndb.de>
>>>>
>>>> Fixes: 858f7a5c45ca ("ASoC: SOF: Introduce fragment elapsed notification API")
>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>> It's Monday morning and my memory is still foggy but I think we fixed
>>> this problem with https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fthesofproject%2Flinux%2Fpull%2F3180&amp;data=04%7C01%7Cdaniel.baluta%40nxp.com%7C25ac869cfd1040f1be1708d9a2bd3460%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637719755777370422%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=E4K2DPkpLX2SgVJ1K99Qs3uz7l7mS96gIzYlJw9akbg%3D&amp;reserved=0,
>>> where we changed the Kconfigs for i.MX. We haven't sent this update
>>> upstream for some reason.
>>>
>>> Arnd, can you share the configuration that breaks with the existing
>>> upstream code, I can check if the problem still exists.
> 
> https://pastebin.com/6JqM6Gkr

Ack, the problem still exists and is fixed by this patch

Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>


On a related note, with this randconfig we have a separate problem on an
AMD patch on the SOF tree

ERROR: modpost: "snd_amd_acp_find_config"
[sound/soc/amd/snd-acp-config] is a static EXPORT_SYMBOL

That wasn't detected earlier, any idea what Kconfig option I need to
enable to filter this out?
Arnd Bergmann Nov. 8, 2021, 6:11 p.m. UTC | #5
On Mon, Nov 8, 2021 at 6:18 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
> On 11/8/21 10:15 AM, Arnd Bergmann wrote:
> > On Mon, Nov 8, 2021 at 3:13 PM Daniel Baluta <daniel.baluta@nxp.com> wrote:
>
> On a related note, with this randconfig we have a separate problem on an
> AMD patch on the SOF tree
>
> ERROR: modpost: "snd_amd_acp_find_config"
> [sound/soc/amd/snd-acp-config] is a static EXPORT_SYMBOL
>
> That wasn't detected earlier, any idea what Kconfig option I need to
> enable to filter this out?

I don't see that symbol on linux-next at all, so this must be a bug
that is specific to the
current SOF tree. The message tells you that snd_amd_acp_find_config should
not be exported at all since it's static, or it should not be marked
static because it
is used by other modules. IOW having a symbol that is both static and exported
means it's not usable from built-in drivers.

       Arnd
Daniel Baluta Nov. 10, 2021, 3:27 p.m. UTC | #6
On 11/8/21 1:11 PM, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> With CONFIG_SND_SOC_SOF_COMPRESS=m, the compression code is
> not built into a the main SOF driver when that is built-in:
>
> x86_64-linux-ld: sound/soc/sof/ipc.o: in function `ipc_stream_message':
> ipc.c:(.text+0x5a2): undefined reference to `snd_sof_compr_fragment_elapsed'
> x86_64-linux-ld: sound/soc/sof/topology.o: in function `sof_dai_load':
> topology.c:(.text+0x32d1): undefined reference to `snd_sof_compr_init_elapsed_work'
> x86_64-linux-ld: topology.c:(.text+0x32e1): undefined reference to `snd_sof_compr_init_elapsed_work'
>
> Make this a 'bool' symbol so it just decides whether the
> code gets built at all.
>
> Fixes: 858f7a5c45ca ("ASoC: SOF: Introduce fragment elapsed notification API")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Indeed this is a bug and must be fixed. Thanks Arnd!

Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>

> ---
>   sound/soc/sof/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
> index 6bb4db87af03..041c54639c4d 100644
> --- a/sound/soc/sof/Kconfig
> +++ b/sound/soc/sof/Kconfig
> @@ -47,7 +47,7 @@ config SND_SOC_SOF_OF
>   	  Say Y if you need this option. If unsure select "N".
>   
>   config SND_SOC_SOF_COMPRESS
> -	tristate
> +	bool
>   	select SND_SOC_COMPRESS
>   
>   config SND_SOC_SOF_DEBUG_PROBES
Mark Brown Nov. 12, 2021, 9:27 p.m. UTC | #7
On Mon, 8 Nov 2021 12:11:14 +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> With CONFIG_SND_SOC_SOF_COMPRESS=m, the compression code is
> not built into a the main SOF driver when that is built-in:
> 
> x86_64-linux-ld: sound/soc/sof/ipc.o: in function `ipc_stream_message':
> ipc.c:(.text+0x5a2): undefined reference to `snd_sof_compr_fragment_elapsed'
> x86_64-linux-ld: sound/soc/sof/topology.o: in function `sof_dai_load':
> topology.c:(.text+0x32d1): undefined reference to `snd_sof_compr_init_elapsed_work'
> x86_64-linux-ld: topology.c:(.text+0x32e1): undefined reference to `snd_sof_compr_init_elapsed_work'
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-linus

Thanks!

[1/1] ASoC: SOF: build compression interface into snd_sof.ko
      commit: 1218f06cb3c6e2c51699998bc17c0d9a41ab37a6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
diff mbox series

Patch

diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
index 6bb4db87af03..041c54639c4d 100644
--- a/sound/soc/sof/Kconfig
+++ b/sound/soc/sof/Kconfig
@@ -47,7 +47,7 @@  config SND_SOC_SOF_OF
 	  Say Y if you need this option. If unsure select "N".
 
 config SND_SOC_SOF_COMPRESS
-	tristate
+	bool
 	select SND_SOC_COMPRESS
 
 config SND_SOC_SOF_DEBUG_PROBES