Skip to content

topology: remove extra demux channel definitions #3586

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

Merged
lgirdwood merged 1 commit into from
Nov 9, 2020
Merged

topology: remove extra demux channel definitions #3586

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions tools/topology/sof-icl-rt711-rt1308-rt715-hdmi.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,22 @@ ifdef(`MONO', `',
define(matrix1, `ROUTE_MATRIX(3,
`BITS_TO_BYTE(1, 0, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 1, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 1 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,1 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,1 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,1 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,1 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,1)')')
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,0)')')

define(matrix2, `ROUTE_MATRIX(4,
`BITS_TO_BYTE(1, 0, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 1, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 1 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,1 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,1 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,1 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,1 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,1)')')
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,0)',
`BITS_TO_BYTE(0, 0, 0 ,0 ,0 ,0 ,0 ,0)')')

@plbossart plbossart Nov 6, 2020

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.

@juimonen can you remind me why we have an 8x8 matrix when a 2x2 seemed to do the job? Is this because we hard-coded the demux component to 8ch?

@juimonen juimonen Nov 6, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@plbossart we support max 8ch streams, so this is the configuration block for "possibly" all channels. We don't have extra parameters to specify the size or row numbers etc.

I think we had a misunderstanding in the "simplification" code, in a way that demux interpreted all 1's to be used somehow, even though that would lie in a channel count above the stream channel count. So runtime stream channel count should dictate what is used from the matrix, and that is what is now fixed in #3583

It is like you would have multiband eq, where the top band lies above 8khz, if you send there 4kHz sine this band should not have any effect on your sine. It should not however stop you defining the bands in your config.

@plbossart plbossart Nov 6, 2020

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.

@plbossart we support max 8ch streams, so this is the configuration block for "possibly" all channels. We don't have extra parameters to specify the size or row numbers etc.

I think we had a misunderstanding in the "simplification" code, in a way that demux interpreted all 1's to be used somehow, even though that would lie in a channel count above the stream channel count. So runtime stream channel count should dictate what is used from the matrix, and that is what is now fixed in #3583

It is like you would have multiband eq, where the top band lies above 8khz, if you send there 4kHz sine this band should not have any effect on your sine. It should not however stop you defining the bands in your config.

Right, but usually you can define the number of bands... I find it odd that we force the demux to deal with 8ch even though we don't have a use case for it on playback. Anyways, that will be for another day and another optimization, this is good enough for now.


dnl name, num_streams, route_matrix list
MUXDEMUX_CONFIG(demux_priv_3, 2, LIST(` ', `matrix1,', `matrix2'))
Expand Down