Skip to content

Conversation

@marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Dec 14, 2022

Partial revert of commit 687c6f3 (".github: workflow: removed legacy RTOS platforms from Zephyr build") that was a bit too "enthusiastic".

When someone breaks the Zephyr+IPC3 build we'd like to: 1. notice, 2. tell whether it's Intel-specific, IMX-specific or not specific.

This is a one-line change and unlike testing, building is "free". It can be removed in a second when/if that becomes a burden.

Signed-off-by: Marc Herbert marc.herbert@intel.com

@marc-hb
Copy link
Collaborator Author

marc-hb commented Dec 14, 2022

Funny how TGL-H IPC3 was removed before TGL-H IPC4 compiles.

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - need to catch this and dont break the build for IPC3 users.

@marc-hb marc-hb marked this pull request as ready for review December 14, 2022 15:23
The list is getting too long and the "checks" box in the Github User
Interface is small.

Fixes commit 543acc1 (".github/workflows: add tgl-h IPC4 build")

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Partial revert of commit 687c6f3 (".github: workflow: removed
legacy RTOS platforms from Zephyr build") that was a bit too
"enthousiastic".

When someone breaks the Zephyr+IPC3 build we'd like to: 1. notice, 2.
tell whether it's Intel-specific, IMX-specific or not specific.

This is a one-line change and unlike testing, building is "free".
It can be removed in a second when/if that becomes a burden.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
Copy link
Collaborator Author

marc-hb commented Dec 15, 2022

Rebased to fix conflict with TGL-H addition #6816

Now also building TGL and TGL-H in the same job: https://github.com/thesofproject/sof/actions/runs/3707071174/jobs/6283044748

Github's "checks" box is too small.

@kv2019i kv2019i merged commit d652d1c into thesofproject:main Dec 16, 2022
@aborisovich
Copy link
Contributor

aborisovich commented Dec 16, 2022

This is a one-line change and unlike testing, building is "free". It can be removed in a second when/if that becomes a burden.

TGL had been removed because "it became a burden". We do not remove platforms for our liking.
Implementation needed for MTL required more and more additional effort to be married with legacy TGL.
Everyone very happily removed it because right now it is a blocking CI build.

Good point - need to catch this and dont break the build for IPC3 users.

@lgirdwood There is no such thing as IPC3 with Zephyr build.
As far as I know there was already announcement in some SOF release that SOF stops supporting legacy non-zephyr platforms. Please correct me If I am wrong. EDIT: Ok, there was non such announcement in https://github.com/thesofproject/sof/releases, but on our meetings we had established, that SOF release 2.4 that should be done before 2022 Q4 end will drop support for non-zephyr platforms.
This means that what @marc-hb is doing right now from my perspective, is reverting a change that was introduced too late, to be even more late than it is.

@marcinszkudlinski, @mwasko, @softwarecki, @tmleman, @pjdobrowolski, @abonislawski
Please share your opinion.

@pjdobrowolski
Copy link
Contributor

According WW44 meeting, there is no such thing like legacy platforms with Zephyr....
SOF2.2 stable branch will continue to be maintained as a long-term-support branch for the old platforms (not using Zephyr).

@mwasko
Copy link
Contributor

mwasko commented Dec 16, 2022

When someone breaks the Zephyr+IPC3 build we'd like to: 1. notice, 2. tell whether it's Intel-specific, IMX-specific or not specific.

@marc-hb, @lgirdwood my understanding is that on SOF main all Intel targets (tgl, tgl-h, adl, adl-s, rpl, mtl) will be supported only in configuration: IPC4 + Zephyr. This direction was decided to simplify our testing configuration and development. I see no reason why we should continue CI for Intel targets in IPC3 + Zephyr configuration.

If someone wish to build Intel cAVS target with IPC3 then the maintenance 2.2 branch should be used.

@marc-hb marc-hb deleted the re-add-tgl-ipc3 branch December 16, 2022 15:54
@marc-hb
Copy link
Collaborator Author

marc-hb commented Dec 16, 2022

only in configuration: IPC4 + Zephyr.

TGL+Zephyr+IPC3 is NOT supported, please read the comments and the commit message.

I see no reason why we should continue CI

There's no CI, compilation only.

This is a (very) poor man's "randconfig" https://askubuntu.com/questions/270980/when-compiling-the-linux-kernel-what-is-the-purpose-of-make-randconfig
Kconfig provides a combinatorial explosion and practically infinite number of combinations. No one expects an infinite number of combinations to be validated and especially not in CI. Yet it's desirable to make sure Kconfig is sound and that all Kconfig options compile.

TGL had been removed because "it became a burden". We do not remove platforms for our liking.
Implementation needed for MTL required more and more additional effort to be married with legacy TGL.
Everyone very happily removed it because right now it is a blocking CI build.

What burden more specifically, what failed to compile? If it was a burden, then why does it still compile even when it was not being compiled for a while?

As already mentioned, if it actually stops compiling for a known reason then it can be removed with a one-line change.

@kv2019i
Copy link
Collaborator

kv2019i commented Dec 16, 2022

@mwasko @pjdobrowolski If this is causing issues, we can remove it. This does not bring back the full CI, just the build check.

@mwasko
Copy link
Contributor

mwasko commented Dec 16, 2022

TGL+Zephyr+IPC3 is NOT supported, please read the comments and the commit message.

If we agree that TGL+Zephyr+IPC3 is not supported then I don't see any reason to do a compile check as well. Otherwise there always will be a discussion and investigation when a build check fails.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Dec 16, 2022

Otherwise there always will be a discussion and investigation when a build check fails.

Not "always". Once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants