TSC Meeting, July 2021 - Minutes

[lgirdwood]

Agenda: https://github.com/orgs/thesofproject/teams/steering-committee/discussions/24

Present

@lgirdwood @dbaluta @plbossart @cujomalainey @bzhg @beatabaranowska @iuliana-prodan @mwasko @harshapriya-n

Please update and correct if necessary.

v1.8 Release review

Release went well, the three month cadence seem to work well for everyone. Linux distros updated quickly after v1.8 was released. Quick user feedback is essential.

v1.9 Release planning

Three month cadence, stable branch created and feature freeze at end of August with stabilization and release before end of Q3 in September.

Multicore
Full pipeline and component support in v1.9. Today support is pipeline only.

Dynamic pipelines
FW is ready for full dynamic pipelines in that pipelines can be torn down and recreated with different topologies. Driver and userspace need some work here to allow modifying / unloading / reloading topologies. Multi client will help with this as it separates driver logic per function allowing different topologies to be loaded per function. AR @dbaluta will look into kernel for NXP use cases.

Codec adapter and module API
Windows support is being upstreamed into SOF with a lot of new features. This will be done incrementally over this and next year. Codec adapter needs new ABI to better target the usage, AR for @dbaluta to propose new IPC for review. Module API is based on Microsoft APO (audio processing object) and will be the direction of travel for module interface (both C and C++ version.). See https://docs.microsoft.com/en-us/windows-hardware/drivers/audio/audio-processing-object-architecture

IPC4 Windows Support
IPC4 is the IPC version used by Windows so alignment work continues to abstract IPC interfaces and associated logic to enable integration of IPC4 support.

Topology 2
Topology 2 roll out beginning, topology switch in progress. M4 macros needed for platform variants, dangerous but convent, but M4 should be used to enable variation only. e.g. M4 related to HW non reconfigurable inclusion like max clock audio rates
Topology is hard to write today, little GUI needed, needs to start with templates and inclusion, list growing. Reuse is key, mistake is copy/paste and fork. UI should be able to reload. Suggestion to use https://qjackctl.sourceforge.io/ for GUI and add support for topology 2.

PM some low hanging fruit.
IMR changes coming, reduces loading latency and can be traced withe kernel.
Deep buffering patches coming from @ranj063, adds separate pipeline for low power playback so that deep CPU C states can be maximized. CRAS uses the same buffer today, but would need some extra logic to check for low power PCMs probably alongside hints in UCM files.

Maths library next features
No new math algorithms planned for v1.9 except for SIMD optimization of existing code using HiFi3/4.

Audio processing components
Plan for v1.9 is to increase Cadence codec support and to further optimize DRC.

RTOS Support

Everyone aligned to have Zephyr as the primary RTOS using native APIs within common infrastructure.

Requirement to keep RTOS abstraction so that options exist for users who would like FreeRTOS or XTOS. This would be done with a wrapper for each OS and would be limited to subset features offered by that RTOS. NXP would own work on wrapper logic and CI and maintenance for FreeRTOS and XTOS. Intel will focus it's CI on Zephyr RTOS.

EDIT Question on Zephyr context switch on xtensa, answer is ~200 cycles, so ~500ns on Intel DSP at 400MHz.

Emulation

renode. https://renode.io/ "Qemu on steroids", take same FW builds from field into test infra, mock registers HW etc. Full granularity on end to end integration tests. Needs xtena HiFi support and DSP IP DMA etc. Needs to be functional. AR for TSC to discuss more at next TSC (with inputs on effort for xtensa support and IP support).

Coding style.

No agreement on changing so status quo remains. We dont even know if code will build with C99. AR @marc-hb to test C99 builds.

SOF Loadable Modules

Aligns with module API, @dbaluta has some integration steps, but Cadence API needs aligned first with module API. Timeline is API alignment, then
loading mechanism, target Q4, address space is dedicated today. AR for @dbaluta @mwasko @lgirdwood for followup call.

[marc-hb]

On 9 Jul 2021, at 06:41, Liam Girdwood ***@***.***> wrote: Coding style. No agreement on changing so status quo remains. We dont even know if code will build with C99. AR @marc-hb to test C99 builds.

Of course C99 comments and C99 declarations build, in fact we _already_ have some C99 declarations in the code base! Everywhere they escaped code review: https://github.com/thesofproject/sof/pull/3995/files We're in 2021; some of our toolchains are obsolete but that obsolete. What was the disagreement about? cc: - #7841 - #5750 (comment) - #8723

[plbossart]

What was the disagreement about?

what's wrong with the kernel style?

[cujomalainey]

Just wanted to validate everything was forward compatible as is without any migration. Assuming yes, I am all for allowing C99 if we put in appropriate measures to prevent discouraged practices like variable shadowing. Cppcheck will cover this but it's still not running as part of CI on SOF yet. I have been hitting some bugs in the tool which generates a lot of false positives.

…

On Fri, Jul 9, 2021, 12:10 PM Pierre-Louis Bossart ***@***.***> wrote: What was the disagreement about? what's wrong with the kernel style? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/orgs/thesofproject/teams/steering-committee/discussions/25/comments/2>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALXI7VTJB7LYRQQMK6AKEDTW5CTLANCNFSM5ACZPJBA> .

[marc-hb]

what's wrong with the kernel style?

Or rather: why is the kernel stuck with pre-C99 declarations? Not to support obsolete toolchains.

Separating declarations and initializations makes it easier to use uninitialized variables by mistake and makes it impossible to const (CERT-C DCL00-C) The compiler tries hard to catch unintialized use but as shown in the example linked below it's not an exact science and has both false negatives and false positives, as seen in the wording "may be used initialized". Then comes the temptation to "pre-"initialize variables to NULL or zero which effectively turns off the compiler warnings, see #4448, #4408, #3944 and https://stackoverflow.com/a/4105334/317623

Every language (including C99) supports or even recommends combining declarations and initializations, outside the kernel that debate is over. About 2/3 of security issues are caused by corrupted memory in C, so anything helps.

[plbossart]

answering to a question without another question doesn't help @marc-hb. this could go on for a while, e.g. why are we not using Rust?

please clarify which changes you want and what checkpatch.pl changes you'd like added.

[marc-hb]

answering to a question without another question doesn't help

That was 15% of my answer and 100% of yours...

e.g. why are we not using Rust?

We use C99 declarations in SOF already: everywhere it escaped code review. The request is just to make it officially allowed. Comparing something we use already with Rust does not help.

please clarify which changes you want

I misunderstood "what's wrong with the kernel style" as an understanding of the request; the kernel code style is notorious for forbidding C99 declarations. This request is NOT about the kernel.

Long story short: C99 declarations don't need to be at the top of a { } block:

{ /* any block */
   /* some statements and computation */
   int x = f() ; /* in-block, combined declaration and initialization. Makes "const" possible */
   ...
}

For more see examples and reference above.

what checkpatch.pl changes you'd like added.

There is no checkpatch change required for C99 declarations (probably why some escaped code review). My compiler warning above to enforce C89 declarations was not merged in SOF.

Reverting my recent #3994 would be enough to allow C99 comments // comment in checkpatch again.

[marc-hb]

Assuming yes, I am all for allowing C99 if we put in appropriate measures to prevent discouraged practices like variable shadowing.

What difference do C99 declarations make with respect to variable shadowing?

[cujomalainey]

On Fri, Jul 9, 2021, 6:56 PM Marc Herbert ***@***.***> wrote: Assuming yes, I am all for allowing C99 if we put in appropriate measures to prevent discouraged practices like variable shadowing. What difference do C99 declarations make with respect to variable shadowing?

Makes it easier to do now that we can declare variables at a block level rather than a function level. Much easier to avoid when globals are rare and all you have to watch for are arguments.

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/orgs/thesofproject/teams/steering-committee/discussions/25/comments/7>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALXI7WXGMSOAIF4L6DFVTLTW6SFDANCNFSM5ACZPJBA> .

[marc-hb]

Makes it easier to do now that we can declare variables at a block level rather than a function level.

The following is valid C89 code, maybe even older. It allows shadowing and does not require C99 declarations at all:

f()
{
   ...
   while (something) {
          int i = 42;  /* top of inner block */
    ...
   }
}

So C99 declarations and shadowing are two different problems. The Linux kernel has the anti-C99 -Wdeclaration-after-statement and that's all, so its coding style does not prevent this potential shadowing example.

I guess we have even more of these top of inner block declarations already, I mean even more than C99 declarations. I doubt any compiler flag or checker can find these declarations, they're really part of the most basic C language. Are they not compliant with the SOF code style either? Is the SOF coding style even stricter than the kernel coding style? One way or the other the code style should be written down somewhere, otherwise developers don't even know what the rules are (#3944)

[marc-hb]

discouraged practices like variable shadowing. Cppcheck will cover this but it's still not running as part of CI on SOF yet.

clang and gcc have a -Wshadow that seems to Just Work. Right now it finds 19 instances of shadowing global variables and 2 instances (in the same function) of shadowing locals.

[cujomalainey]

That sounds like something I would recommend we fix and enable. Thanks for looking into this, clearly my knowledge of the C standards vs C compile switches is a bit murky.

[marc-hb]

So, besides (in)consistency with the Linux kernel's -Wdeclaration-after-statement, what technical reason is there for the SOF unwritten coding style to forbid C99's declaration-after-statement? (While letting some escape)

[cujomalainey]

All of my concerns have been addressed, I have no issue moving forward with this.

[marc-hb]

A question about --std=c99 here: #4733

[marc-hb]

Example where C89 declarations double the number of #ifdef:

• audio: dai-zephyr: Fix unused variable warning #8723