Fix topology2 builds #6592

ranj063 · 2022-11-14T19:43:15Z

No description provided.

tools/topology/topology2/sof-ace-tplg/CMakeLists.txt

marc-hb · 2022-11-14T20:27:42Z

We already have too many "magic moves" in CI that confuse everyone except a couple people,

Unsurprisingly, no IPC4 test was run in https://sof-ci.01.org/sofpr/PR6592/build2346/devicetest/index.html

tools/topology/CMakeLists.txt

ranj063 · 2022-11-14T21:11:43Z

COMMAND cat confheader{input_name}.conf > ../${output_dir}/${output_name}.conf

I really don't like this new, per-topology ../${output_dir} sorry. I know moving files tends to "break git" but in this case I think moving then one by one at build time is much worse. We already have too many "magic moves" in CI that confuse everyone except a couple people, let's please have at least in the build system a simple directory structure where the source tree and build tree are very similar to each other and please, please no per-file output directory. I'm afraid this could confuse not just people but even surprise CMake itself.

I know moving files tends to "break git" but...

EDIT: Wait, this PR moves sources files anyway. So no need for ../${output_dir}

@marc-hb yes thats correct. I should have squahsed the first 2 patches and remove the output_dir. Fixed now.

marc-hb · 2022-11-15T19:56:58Z

tools/topology/topology2/CMakeLists.txt

Please move these new targets to their corresponding subdirectories to keep them self-contained.

I would prefer each child to add itself to the parent but no big deal.

tools/topology/topology2/avs-tplg/CMakeLists.txt

The CAVS topologies need to be in the avs-tplg folder and the MTL topologies need to be in sof-ace-tplg folder. Also, change the output file names to the names expected by the kernel and the NHLT binary files to match the topology file names. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

This folder contains definitions for both CAVS and ACE. So remove the incorrect folder naming and move everything one level up. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

marc-hb

The CMake changes look simple and good to me, thanks for doing this!

Someone else (@plbossart ?) must approve the new names and we also need someone to cleanup some of the CI moves.

I guess it's unlikely but if someone in the future ever wants to make or convert some IPC3 to topology2 they'll have to make some abi.h-related tweaks correct? As long as that code is in a single place we can wait if/when we cross that bridge.

marc-hb · 2022-11-15T21:43:07Z

tools/topology/topology2/CMakeLists.txt

I would prefer each child to add itself to the parent but no big deal.

tools/topology/topology2/avs-tplg/CMakeLists.txt

tools/topology/topology2/sof-ace-tplg/CMakeLists.txt

This will be needed for MTL. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Rename it to sof-hda-generic.conf. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

lgirdwood · 2022-11-16T12:23:44Z

SOFCI TEST

lgirdwood · 2022-11-16T12:24:07Z

Rerun the CI as 3 DUTs were in use for development.

ranj063 · 2022-11-16T16:12:00Z

Rerun the CI as 3 DUTs were in use for development.

@lgirdwood this PR will need changes in CI to deploy the topology binaries as is for testing. @fredoh9 can you please let me know when the change is ready?

lgirdwood · 2022-11-18T10:29:51Z

@fredoh9 @ranj063 what's the ETA for CI updates so this can be merged ?

fredoh9 · 2022-11-28T17:12:25Z

what's the ETA for CI updates so this can be merged ?

One PR is pending in review, I think, we can get reviewed in 1-2 days.

aborisovich

I've started to request changes on fixing those variables with
set(TPLGS ...) and then noticed they are multiline strings...
So ugly but yeah... Did not find any alternative to it.
A good piece of CMake code.

marc-hb · 2022-11-30T01:36:42Z

So ugly but yeah... Did not find any alternative to it.

All my CMake dreams shattered in just one pen stroke....

Couldn't these lists be read from some (JSON?) configuration file? EDIT: I mean IN THE FUTURE, NOT IN THIS PR.

miRoox · 2022-11-30T08:01:32Z

SOFCI TEST

lgirdwood · 2022-11-30T13:11:08Z

@fredoh9 any update on ETA ?

fredoh9 · 2022-11-30T18:06:11Z

@lgirdwood @ranj063 Now ready to merge!

ranj063 · 2022-11-30T18:46:42Z

So ugly but yeah... Did not find any alternative to it.

All my CMake dreams shattered in just one pen stroke....

Couldn't these lists be read from some (JSON?) configuration file? EDIT: I mean IN THE FUTURE, NOT IN THIS PR.

json would be fine but it will just be another file to maintain though

marc-hb · 2022-12-01T06:54:36Z

Which topology file is corresponding to the cavs-mixin-mixout-hda-4ch-mtl.tplg:

What does "correspond" mean? Are we still renaming .tplg files in CI and not testing sof-bin releases? The purpose of this PR is to stop that.

json would be fine but it will just be another file to maintain though

The idea would be to separate data and code. Should not add any line. This would make filenames in git log --stat useful again.

miRoox · 2022-12-01T07:22:11Z

What does "correspond" mean?

corresponding to the cavs-mixin-mixout-hda-4ch-mtl.tplg before this PR. Just confused with the cavs-passthrough-hdmi before. Ranjani said passthrough topology files have been deleted, so no confusion.

Are we still renaming .tplg files in CI and not testing sof-bin releases? The purpose of this PR is to stop that.

It reduced a lot of renaming for IPC4 topology files, but we still need: mtl-sdw-dmic =>sof-mtl-rt711-4ch on MTLP_RVP_SDW

[  104.079178] kernel: snd_sof:snd_sof_load_topology: sof-audio-pci-intel-mtl 0000:00:1f.3: loading topology:intel/sof-ace-tplg/sof-mtl-rt711-4ch.tplg

and sof-tgl-nocodec =>sof-adl-nocodec just like what we done in topology2.

I'm not sure if they are a bug. But if these can be handled in CMake, I think it would be better.

ranj063 · 2022-12-01T16:20:19Z

I'm not sure if they are a bug. But if these can be handled in CMake, I think it would be better.

Thanks for pointing these out @miRoox. Let me submit a fix for these.

ranj063 · 2022-12-01T16:26:06Z

sof-mtl-rt711-4ch

A couple more topology2 renames #6703

ranj063 requested review from aborisovich, dbaluta, lbetlej, lgirdwood, marc-hb, mmaka1 and plbossart as code owners November 14, 2022 19:43

ranj063 mentioned this pull request Nov 14, 2022

[FEATURE] Clean-up topology2/cavs to make it user-friendly and maintainable #6576

Closed

4 tasks

plbossart reviewed Nov 14, 2022

View reviewed changes

tools/topology/topology2/sof-ace-tplg/CMakeLists.txt Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

marc-hb reviewed Nov 14, 2022

View reviewed changes

tools/topology/CMakeLists.txt Outdated Show resolved Hide resolved

ranj063 force-pushed the fix/cavs_tplgs branch from 82392c5 to 7fdf412 Compare November 14, 2022 21:11

ranj063 requested review from marc-hb and plbossart November 14, 2022 21:20

ranj063 force-pushed the fix/cavs_tplgs branch from 7fdf412 to 9cd34aa Compare November 14, 2022 21:22

marc-hb requested changes Nov 15, 2022

View reviewed changes

ranj063 added 2 commits November 15, 2022 12:38

topology2: Remove the cavs folder

925da17

This folder contains definitions for both CAVS and ACE. So remove the incorrect folder naming and move everything one level up. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 force-pushed the fix/cavs_tplgs branch from 9cd34aa to 925da17 Compare November 15, 2022 20:40

ranj063 requested a review from marc-hb November 15, 2022 20:40

marc-hb approved these changes Nov 15, 2022

View reviewed changes

tools/topology/topology2/CMakeLists.txt Outdated

Copy link

Collaborator

marc-hb Nov 15, 2022

Choose a reason for hiding this comment

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

I would prefer each child to add itself to the parent but no big deal.

plbossart reviewed Nov 15, 2022

View reviewed changes

tools/topology/topology2/avs-tplg/CMakeLists.txt Outdated Show resolved Hide resolved

tools/topology/topology2/sof-ace-tplg/CMakeLists.txt Show resolved Hide resolved

ranj063 added 2 commits November 15, 2022 15:52

topology2: sof-ace-tplg: Build sof-hda-generic-2ch topology

f8bc4e4

This will be needed for MTL. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

topology2: rename cavs-passthrough-hdmi.conf

8705e20

Rename it to sof-hda-generic.conf. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 requested a review from plbossart November 15, 2022 23:58

plbossart approved these changes Nov 16, 2022

View reviewed changes

lgirdwood approved these changes Nov 16, 2022

View reviewed changes

lgirdwood mentioned this pull request Nov 16, 2022

topology2: add deepbuffer to nocodec #6508

Merged

RanderWang approved these changes Nov 17, 2022

View reviewed changes

ranj063 mentioned this pull request Nov 17, 2022

Topology clean ups - Part 2 #6627

Merged

ranj063 mentioned this pull request Nov 25, 2022

Audio: Copier: Add missing DMIC DAI index mapping to create_dai() #6668

Merged

aborisovich approved these changes Nov 30, 2022

View reviewed changes

ranj063 merged commit 0bdf8e3 into thesofproject:main Dec 1, 2022

ranj063 deleted the fix/cavs_tplgs branch December 1, 2022 00:55

This comment was marked as resolved.

Sign in to view

Fix topology2 builds #6592

Fix topology2 builds #6592

Uh oh!

Conversation

ranj063 commented Nov 14, 2022

Uh oh!

Uh oh!

This comment was marked as outdated.

marc-hb commented Nov 14, 2022

Uh oh!

Uh oh!

ranj063 commented Nov 14, 2022

Uh oh!

marc-hb Nov 15, 2022

Choose a reason for hiding this comment

Uh oh!

ranj063 Nov 15, 2022

Choose a reason for hiding this comment

Uh oh!

marc-hb Nov 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marc-hb left a comment

Choose a reason for hiding this comment

Uh oh!

marc-hb Nov 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lgirdwood commented Nov 16, 2022

Uh oh!

lgirdwood commented Nov 16, 2022

Uh oh!

ranj063 commented Nov 16, 2022

Uh oh!

lgirdwood commented Nov 18, 2022

Uh oh!

fredoh9 commented Nov 28, 2022

Uh oh!

aborisovich left a comment

Choose a reason for hiding this comment

Uh oh!

marc-hb commented Nov 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miRoox commented Nov 30, 2022

Uh oh!

lgirdwood commented Nov 30, 2022

Uh oh!

fredoh9 commented Nov 30, 2022

Uh oh!

ranj063 commented Nov 30, 2022

Uh oh!

This comment was marked as resolved.

marc-hb commented Dec 1, 2022

Uh oh!

miRoox commented Dec 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ranj063 commented Dec 1, 2022

Uh oh!

ranj063 commented Dec 1, 2022 • edited by marc-hb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

marc-hb commented Nov 30, 2022 •

edited

Loading

miRoox commented Dec 1, 2022 •

edited

Loading

ranj063 commented Dec 1, 2022 •

edited by marc-hb

Loading