Skip to content

topology: graph: Add PIPELINE_ID to graph name #2207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
lbetlej merged 1 commit into from
Feb 4, 2020
Merged

topology: graph: Add PIPELINE_ID to graph name #2207

lbetlej merged 1 commit into from
Feb 4, 2020

Conversation

@ranj063
Copy link
Collaborator

@ranj063 ranj063 commented Dec 13, 2019 •
edited
Loading

Add the PIPELINE_ID to the graph section name to
make them unique for the platback and capture pipelines
that are associated with the same SSP.

Fixes #2201

@ranj063 ranj063 mentioned this pull request Dec 13, 2019
Closed
@ranj063 ranj063 requested review from juimonen and kv2019i December 13, 2019 16:16
xiulipan
xiulipan suggested changes Dec 16, 2019
View reviewed changes
Copy link
Contributor

@xiulipan xiulipan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Fix is good, but let's update the inline comments.

tools/topology/m4/utils.m4 Outdated
Copy link
Contributor

@xiulipan xiulipan Dec 16, 2019

Choose a reason for hiding this comment

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

I think we can also make the function comment changed too.

Suggested change
dnl P_GRAPH(name, CONNECTIONS)
dnl P_GRAPH(name, pipeline id, CONNECTIONS)

Copy link
Collaborator

@kv2019i kv2019i Dec 16, 2019

Choose a reason for hiding this comment

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

This seems to be wrong before this patch already, but anyways an important correction.

Copy link
Collaborator Author

@ranj063 ranj063 Dec 16, 2019

Choose a reason for hiding this comment

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

@xiulipan @kv2019i done!

kv2019i
kv2019i reviewed Dec 16, 2019
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Maybe you can correct the comment as well, looks good otherwise.

@lgirdwood
Copy link
Member

lgirdwood commented Dec 17, 2019

@ranj063 can you check the CI. @kv2019i can you re-review.

kv2019i
kv2019i approved these changes Dec 17, 2019
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Thanks, looks good.

@ranj063
Copy link
Collaborator Author

ranj063 commented Dec 17, 2019

@ranj063 can you check the CI. @kv2019i can you re-review.

@zrombel can you please help me with the codecheck error? I am not able to decipher it

xiulipan
xiulipan approved these changes Dec 18, 2019
View reviewed changes
Copy link
Contributor

@xiulipan xiulipan left a comment

Choose a reason for hiding this comment

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

LGTM! good catch

@lgirdwood
Copy link
Member

lgirdwood commented Dec 18, 2019

SOFCI TEST

jajanusz
jajanusz reviewed Jan 2, 2020
View reviewed changes
@ranj063
topology: graph: Add PIPELINE_ID to graph name
92dcada 
Add the PIPELINE_ID to the graph section name to
make them unique for the playback and capture pipelines
that are associated with the same SSP. Also fix the
comment while we're at it.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063 ranj063 force-pushed the fix/pipeline_graph branch from 2d72741 to 92dcada Compare January 7, 2020 19:01
@ranj063 ranj063 requested review from dbaluta and tlauda as code owners January 7, 2020 19:01
@lgirdwood
Copy link
Member

lgirdwood commented Jan 7, 2020

SOFCI TEST

@lgirdwood
Copy link
Member

lgirdwood commented Jan 7, 2020 •
edited
Loading

@fredoh9 @xiulipan fwiw, I'm seeing some 404 results on some CI links to dmesg, logger etc for test failures.

@plbossart
Copy link
Member

plbossart commented Jan 7, 2020

@fredoh9 @xiulipan fwiw, I'm seeing some 404 results on some CI links to dmesg, logger etc for test failures.

same with kernel PR thesofproject/linux#1663, 404 for all reports

@xiulipan
Copy link
Contributor

xiulipan commented Jan 8, 2020

@lgirdwood @plbossart seeing the same issue now, working on this.

@lgirdwood
Copy link
Member

lgirdwood commented Jan 8, 2020

@zrombel can you comment on the codecheck failure, I cant see it. Thanks

@zrombel
Copy link

zrombel commented Jan 8, 2020

KW server didn't respond. Rerun and code check pass.

@lgirdwood
Copy link
Member

lgirdwood commented Jan 8, 2020 •
edited
Loading

@zrombel thanks ! @xiulipan I have a 404 on the ICL test failure. We are ready to merge once that is clarified.

@xiulipan
Copy link
Contributor

xiulipan commented Jan 13, 2020

@lgirdwood The ICL fail is due to the DSP reset retry failed.

Caught error: Jan  7 12:57:09 jf-icl-rvp-hda-4 kernel: [  395.214758] sof-audio-pci 0000:00:1f.3: error: cl_dsp_init: timeout HDA_DSP_SRAM_REG_ROM_STATUS read
Jan  7 12:57:09 jf-icl-rvp-hda-4 kernel: [  395.214806] sof-audio-pci 0000:00:1f.3: error: status = 0x00000000 panic = 0x00000000
Jan  7 12:57:09 jf-icl-rvp-hda-4 kernel: [  395.214858] sof-audio-pci 0000:00:1f.3: error: Error code=0xffffffff: FW status=0xffffffff
Jan  7 12:57:09 jf-icl-rvp-hda-4 kernel: [  395.214862] sof-audio-pci 0000:00:1f.3: error: iteration 0 of Core En/ROM load failed: -110
kernel log has error(s)

And the 404 issue is some URL path issue, the dmesg can be found in https://sof-ci.01.org/sofpr/PR2207/build4440/devicetest/ICL_RVP_HDA/check-pause-resume-playback-10.sh/check-pause-resume-playback-10/dmesg.txt

@lgirdwood
Copy link
Member

lgirdwood commented Jan 20, 2020

SOFCI TEST

@lgirdwood
Copy link
Member

lgirdwood commented Jan 20, 2020

@ranj063 rerunning CI since the same CI issue has cropped up on various PRs.

@ranj063
Copy link
Collaborator Author

ranj063 commented Feb 3, 2020

@lgirdwood is this PR ready for merging now?

@lgirdwood
Copy link
Member

lgirdwood commented Feb 4, 2020

SOFCI TEST

@lgirdwood
Copy link
Member

lgirdwood commented Feb 4, 2020

@ranj063 Hope so, TGL build was showing a fail so rerunning. Jenkins was good, usual suspect for module reload. Will merge once TGL build is resolved (could be CI issue).

@lbetlej lbetlej merged commit 55afcf5 into thesofproject:master Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@paulstelian97 paulstelian97 paulstelian97 left review comments

@lgirdwood lgirdwood lgirdwood approved these changes

@kv2019i kv2019i kv2019i approved these changes

@lbetlej lbetlej Awaiting requested review from lbetlej

@mmaka1 mmaka1 Awaiting requested review from mmaka1

@plbossart plbossart Awaiting requested review from plbossart

@juimonen juimonen Awaiting requested review from juimonen

@dbaluta dbaluta Awaiting requested review from dbaluta

@tlauda tlauda Awaiting requested review from tlauda

+2 more reviewers

@jajanusz jajanusz jajanusz left review comments

@xiulipan xiulipan xiulipan 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.

[BUG] topology sof-bdw-codec.m4 (sof-bdw-rt5640.conf)

9 participants

@ranj063 @lgirdwood @plbossart @xiulipan @zrombel @paulstelian97 @jajanusz @kv2019i @lbetlej