ASoC: SOF: Intel: fix kconfig related if statements in machine drivers by xun2z · Pull Request #502 · thesofproject/linux

xun2z · 2019-01-04T07:45:02Z

Use SND_SOC_SOF_INTEL_TOPLEVEL configuration in some if statements of
machine drivers (SND_SOC_SOF_INTEL no longer exists)

This is to fix issue #474 , which blocks SOF from loading topology file.

plbossart

The diagnosis is correct but the fix isn't.
This should be if !IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL) so that we only touch these drivers if they are indeed selected.
In addition we should remove all these SOF-dependent cases by adding virtual widgets in the topology to remove spurious error messages and add support for Baytrail CR instead. Can you please work on the virtual widgets part?

plbossart

NAK, this needs to be a topology change for the widgets, and the kernel parts should only see the removal of the IS_ENABLED stuff

plbossart · 2019-01-07T19:31:28Z

NAK? It looks like a complete misunderstanding, the ask was to create virtual widgets in the topology file e.g. https://github.com/thesofproject/sof/blob/a2e6f82ef810a6a838783467472dfceb7933afc1/tools/topology/sof-apl-da7219.m4#L174

ok, I misunderstood your request that I need to create some widgets in the kernel, while these should all be done in the sof part. I'll remove them all.

plbossart · 2019-01-07T19:32:49Z

that change is fine

plbossart · 2019-01-07T19:33:01Z

this is fine

plbossart · 2019-01-07T19:33:09Z

plbossart · 2019-01-07T19:33:30Z

this is fine

plbossart · 2019-01-07T19:33:40Z

plbossart · 2019-01-07T19:33:47Z

plbossart · 2019-01-07T19:33:54Z

plbossart · 2019-01-07T19:34:00Z

plbossart · 2019-01-07T19:34:05Z

xun2z · 2019-01-08T09:06:46Z

I've removed all if statements and the widgets I created. Now this pull request will actually not fix the error message until corresponding SOF topology is changed.

By the way, in topology, I notice that there is a small inconsistent thing between the files I'm gonna modify and those already have virtual widgets on them.

For example, in

sof-apl-da7219.m4

codec0_in

while in

bytcr_rt5651.c

codec_in0

I'm not sure about the difference and still checking.

plbossart

if you don't follow the guidance I provided then not sure how we are going to progress.
btw the topology differences between codec_in0 and codec0_in are due to the drivers themselves, don't even think about trying to unify things.

plbossart · 2019-01-08T14:51:18Z

I believe I was clear that this needed to be removed?

This version is outdated.

All SOF-dependent statement are removed, virtual widgets shall be added in SOF topology in order to avoid missing of some codec widgets. Signed-off-by: Xun Zhang <xun2.zhang@intel.com>

xun2z · 2019-01-09T09:02:28Z

@plbossart I have rebased and updated this pr, please check.

xun2z · 2019-01-09T09:21:17Z

@plbossart SOF topology part is here
thesofproject/sof#803

plbossart · 2019-01-09T17:03:42Z

 endif ## SND_SOC_SOF_INTEL_PCI

-endif ## SND_SOC_SOF_INTEL
+endif ## SND_SOC_SOF_INTEL_TOPLEVEL


good catch, I missed this one. Thanks!

plbossart

thanks @xun2z , will merge now. There should be no adverse effects for BYT-T/CHT, and BYT-CR is broken anyways so will require a follow-up patch.
Good catch on the last change, I really didn't see it.

The .BTF_ids section is pre-filled with zeroed BTF ID entries during the build and afterwards patched by resolve_btfids with correct values. Since resolve_btfids always writes in host-native endianness, it relies on libelf to do the translation when the target ELF is cross-compiled to a different endianness (this was introduced in commit 61e8aed ("bpf: Fix libelf endian handling in resolv_btfids")). Unfortunately, the translation will corrupt the flags fields of SET8 entries because these were written during vmlinux compilation and are in the correct endianness already. This will lead to numerous selftests failures such as: $ sudo ./test_verifier 502 502 thesofproject#502/p sleepable fentry accept FAIL Failed to load prog 'Invalid argument'! bpf_fentry_test1 is not sleepable verification time 34 usec stack depth 0 processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 Summary: 0 PASSED, 0 SKIPPED, 1 FAILED Since it's not possible to instruct libelf to translate just certain values, let's manually bswap the flags (both global and entry flags) in resolve_btfids when needed, so that libelf then translates everything correctly. Fixes: ef2c6f3 ("tools/resolve_btfids: Add support for 8-byte BTF sets") Signed-off-by: Viktor Malik <vmalik@redhat.com> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Link: https://lore.kernel.org/bpf/7b6bff690919555574ce0f13d2a5996cacf7bf69.1707223196.git.vmalik@redhat.com

…endianness' Viktor Malik says: ==================== tools/resolve_btfids: fix cross-compilation to non-host endianness The .BTF_ids section is pre-filled with zeroed BTF ID entries during the build and afterwards patched by resolve_btfids with correct values. Since resolve_btfids always writes in host-native endianness, it relies on libelf to do the translation when the target ELF is cross-compiled to a different endianness (this was introduced in commit 61e8aed ("bpf: Fix libelf endian handling in resolv_btfids")). Unfortunately, the translation will corrupt the flags fields of SET8 entries because these were written during vmlinux compilation and are in the correct endianness already. This will lead to numerous selftests failures such as: $ sudo ./test_verifier 502 502 thesofproject#502/p sleepable fentry accept FAIL Failed to load prog 'Invalid argument'! bpf_fentry_test1 is not sleepable verification time 34 usec stack depth 0 processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 Summary: 0 PASSED, 0 SKIPPED, 1 FAILED Since it's not possible to instruct libelf to translate just certain values, let's manually bswap the flags (both global and entry flags) in resolve_btfids when needed, so that libelf then translates everything correctly. The first patch of the series refactors resolve_btfids by using types from btf_ids.h instead of accessing the BTF ID data using magic offsets. Acked-by: Jiri Olsa <jolsa@kernel.org> --- Changes in v4: - remove unnecessary vars and pointer casts (suggested by Daniel Xu) Changes in v3: - add byte swap of global 'flags' field in btf_id_set8 (suggested by Jiri Olsa) - cleaner refactoring of sets_patch (suggested by Jiri Olsa) - add compile-time assertion that IDs are at the beginning of pairs struct in btf_id_set8 (suggested by Daniel Borkmann) Changes in v2: - use type defs from btf_ids.h (suggested by Andrii Nakryiko) ==================== Link: https://lore.kernel.org/r/cover.1707223196.git.vmalik@redhat.com Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

plbossart reviewed Jan 4, 2019

View reviewed changes

mengdonglin mentioned this pull request Jan 5, 2019

topology: widget not found for codec during loading topology on BYT with ALC5651. #474

Closed

plbossart requested changes Jan 7, 2019

View reviewed changes

plbossart requested changes Jan 8, 2019

View reviewed changes

ASoC: SOF: Intel: remove kconfig related if statements

0e0b5f6

All SOF-dependent statement are removed, virtual widgets shall be added in SOF topology in order to avoid missing of some codec widgets. Signed-off-by: Xun Zhang <xun2.zhang@intel.com>

xun2z force-pushed the issue-474 branch from 59269ad to 0e0b5f6 Compare January 9, 2019 09:00

plbossart reviewed Jan 9, 2019

View reviewed changes

plbossart approved these changes Jan 9, 2019

View reviewed changes

plbossart merged commit 7ea9493 into thesofproject:topic/sof-dev Jan 9, 2019

Conversation

xun2z commented Jan 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

plbossart left a comment • 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 left a comment

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!

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!

xun2z commented Jan 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

xun2z commented Jan 9, 2019

Uh oh!

xun2z commented Jan 9, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xun2z commented Jan 4, 2019 •

edited

Loading

plbossart left a comment •

edited

Loading

xun2z commented Jan 8, 2019 •

edited

Loading