Skip to content

Remove legacy RTOS platforms from Zephyr build - #6697

Merged
lgirdwood merged 1 commit into
thesofproject:mainfrom
aborisovich:enable-tgl-native-drivers-ci-disable-cavs-with-zephyr
Dec 2, 2022
Merged

Remove legacy RTOS platforms from Zephyr build#6697
lgirdwood merged 1 commit into
thesofproject:mainfrom
aborisovich:enable-tgl-native-drivers-ci-disable-cavs-with-zephyr

Conversation

@aborisovich

@aborisovichaborisovich commented Nov 30, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Removing old CAVS SOF RTOS platforms from being built with Zephyr in CI.

Signed-off-by: Andrey Borisovich andrey.borisovich@intel.com

@aborisovich
aborisovich marked this pull request as ready for review November 30, 2022 15:05
abonislawski
abonislawski reviewed Nov 30, 2022
View reviewed changes
Comment thread.github/workflows/zephyr.yml Outdated

@abonislawskiabonislawskiNov 30, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why was imx removed?

@aborisovichaborisovichNov 30, 2022•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

is imx a zephyr platform? Will it stay?

@lgirdwoodlgirdwoodNov 30, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, IMX is a Zephyr platform.

@lyakhlyakhDec 2, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why is TGL removed?

@aborisovichaborisovichDec 2, 2022•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

tgl is build only with ipc4 being native drivers overlay - thus using zephyr drivers right?
(see whole file not only my changes)

@marc-hbmarc-hbDec 13, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

abonislawski
abonislawski reviewed Nov 30, 2022
View reviewed changes
Comment thread.github/workflows/zephyr.yml Outdated
@aborisovichaborisovich mentioned this pull request Nov 30, 2022
Merged
@aborisovich
aborisovichforce-pushed the enable-tgl-native-drivers-ci-disable-cavs-with-zephyr branch from 3062568 to 0d33f64CompareNovember 30, 2022 15:26
@lgirdwood

lgirdwood commented Nov 30, 2022

Copy link
Copy Markdown
Member

@marc-hb any comments ?

@aborisovich
aborisovichforce-pushed the enable-tgl-native-drivers-ci-disable-cavs-with-zephyr branch from 0d33f64 to f4f1191CompareNovember 30, 2022 15:42
@aborisovichaborisovich changed the title Enable tgl native drivers build with Zephyr and disable legacy RTOS platforms from Zephyr buildRemove legacy RTOS platforms from Zephyr buildNov 30, 2022
lgirdwood
lgirdwood reviewed Nov 30, 2022
View reviewed changes
Comment threadapp/overlays/tgl-h/ipc4_overlay.conf Outdated
Comment thread.github/workflows/zephyr.yml Outdated

@lgirdwoodlgirdwoodNov 30, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, IMX is a Zephyr platform.

Comment thread.github/workflows/zephyr.yml Outdated

@lgirdwoodlgirdwoodNov 30, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we need tgl-h here too for ipc4/Zephyr @kv2019i@mengdonglin do we have tgl-h dev boards ?

@aborisovichaborisovichDec 1, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@lgirdwood we can add tgl-h later maybe?
This PR is blocking some other PRs, can we integrate it?

@aborisovichaborisovich mentioned this pull request Nov 30, 2022
Merged
9 tasks
marc-hb
marc-hb reviewed Nov 30, 2022
View reviewed changes
Comment thread.github/workflows/zephyr.yml Outdated
Andrey Borisovich
.github: workflow: removed legacy RTOS platforms from Zephyr build
2a29939
Legacy platforms not meant to work with Zephyr were added
in commit ".github: compile-test multiple zephyr revisions + IPC4"
(8543f5c).
Removed them from CI as they never should be built in the first place.
Signed-off-by: Andrey Borisovich <andrey.borisovich@intel.com>
@aborisovich
aborisovichforce-pushed the enable-tgl-native-drivers-ci-disable-cavs-with-zephyr branch from f4f1191 to 2a29939CompareDecember 1, 2022 14:59
marcinszkudlinski
marcinszkudlinski approved these changes Dec 1, 2022
View reviewed changes

@marcinszkudlinskimarcinszkudlinski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's go, lately I lost 2 days fighting with legacy platforms with Zephyr. Apl, Cnl, Icl, Jsl - those have NEVER been working fine with zephyr, have they? Just luckily compiling...

@kv2019i

kv2019i commented Dec 1, 2022

Copy link
Copy Markdown
Collaborator

Are we good to move @mengdonglin@keqiaozhang@marc-hb ? There is still a check for APL + CNL in the "sof-ci/jenkins/pr-fw-build" job, like https://sof-ci.01.org/sofpr/PR6697/build2620/build/index.html

marc-hb
marc-hb approved these changes Dec 1, 2022
View reviewed changes

@marc-hbmarc-hb left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Apl, Cnl, Icl, Jsl - those have NEVER been working fine with zephyr, have they?

It's been a couple months now but I've run Zephyr on APL about a million times.

@kv2019i

kv2019i commented Dec 1, 2022

Copy link
Copy Markdown
Collaborator

@marcinszkudlinski All the early work to enable SOF on Zephyr has been done on APL, so it has actually been the best working platform. However we now have no line of sight to move APL to use native Zephyr drivers for APL (like we now have to TGL and newer Intel platforms), so we can potentially clean up the codebase a lot for Intel hardware if we drop these old platforms (they continue to be supported in the stable-v2.2 branch of SOF, including Zephyr builds).

@abonislawski

abonislawski commented Dec 1, 2022

Copy link
Copy Markdown
Member

Sad day for SOF ;( but this is the way to go

@marc-hb

marc-hb commented Dec 1, 2022

Copy link
Copy Markdown
Collaborator

All the early work to enable SOF on Zephyr has been done on APL, so it has actually been the best working platform

Not the least because it was cheap and easy to switch to the development key.

Most other products lack a developer mode; impossible to test any bug fix on them
https://thesofproject.github.io/latest/getting_started/intel_debug/introduction.html?highlight=signed#firmware-binary

@aborisovich

aborisovich commented Dec 2, 2022

Copy link
Copy Markdown
ContributorAuthor

If pushing merging button hurts your feelings to old platforms, close your eyes then then click 🌝
Rtos rest in peace ⚰️

@aborisovich

aborisovich commented Dec 2, 2022

Copy link
Copy Markdown
ContributorAuthor

@lyakh I resolved previous conversation as it was related to IMX platforms that were restored to the build.
I removed TGL because there is TGL with IPC4 overlay that has native drivers (zephyr drivers).
Correct me if I am wrong but there is no sense in building TGL with SOF drivers with Zephyr?

@aborisovich

aborisovich commented Dec 2, 2022

Copy link
Copy Markdown
ContributorAuthor

Sad day for SOF ;( but this is the way to go

Evolution & adaptation 🌾

@lgirdwood
lgirdwood merged commit 687c6f3 into thesofproject:mainDec 2, 2022
@aborisovich
aborisovich deleted the enable-tgl-native-drivers-ci-disable-cavs-with-zephyr branch June 11, 2023 15:36
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@marc-hbmarc-hbmarc-hb approved these changes
@lgirdwoodlgirdwoodlgirdwood approved these changes
@marcinszkudlinskimarcinszkudlinskimarcinszkudlinski approved these changes
@tmlemantmlemantmleman approved these changes
@softwareckisoftwareckisoftwarecki approved these changes
@dbalutadbalutaAwaiting requested review from dbalutadbaluta is a code owner
@plbossartplbossartAwaiting requested review from plbossart
@mmaka1mmaka1Awaiting requested review from mmaka1
@lbetlejlbetlejAwaiting requested review from lbetlej
@abonislawskiabonislawskiAwaiting requested review from abonislawski
@juimonenjuimonenAwaiting requested review from juimonen
@lyakhlyakhAwaiting requested review from lyakh
+2 more reviewers
@cujomalaineycujomalaineycujomalainey approved these changes
@jxstelterjxstelterjxstelter approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

11 participants

@aborisovich@lgirdwood@kv2019i@abonislawski@marc-hb@lyakh@cujomalainey@marcinszkudlinski@jxstelter@tmleman@softwarecki