Skip to content

[RFC] Demux blob #2594

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 8 commits into from
Apr 7, 2020
Merged

[RFC] Demux blob #2594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
36692b3
topology: add bit, byte and abi_version macros to utils
Mar 19, 2020
2d57fa8
topology: add binary blob macro for muxdemux
Mar 19, 2020
aa0bf5d
topology: add mux config to sof-apl-demux-pcm512x.m4
Mar 24, 2020
064d985
topology: add mux config to sof-cml-demux-rt5682.m4
Mar 24, 2020
b28d96f
topology: add mux config to sof-icl-rt711-rt1308-rt715-hdmi.m4
Mar 24, 2020
31058e2
topology: remove default demux config from pipe-volume-demux-playback
Mar 19, 2020
a26efeb
topology: remove unused demux default coeffs
Mar 24, 2020
ede497d
topology: remove unused demux pipeline
Mar 24, 2020
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
11 changes: 0 additions & 11 deletions tools/topology/m4/demux_coef_default.m4
View file
Open in desktop

This file was deleted.

29 changes: 29 additions & 0 deletions tools/topology/m4/muxdemux.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,35 @@ divert(-1)

dnl Define macro for demux widget

dnl Hard coded values for mux/demux config blob
define(mux_sof_magic, 0x00464F53)
define(mux_stream_struct_size, 16)
define(mux_config_struct_size, 8)

dnl Fill bytes of struct mux_stream_config (mux.h)
dnl reserved fields in the struct are set to 0
define(`ROUTE_MATRIX',
`PRINT_BYTES_4($1),PRINT_BYTE(0),PRINT_BYTE($2),PRINT_BYTE($3),'dnl
`PRINT_BYTE($4),'
`PRINT_BYTE($5),PRINT_BYTE($6),PRINT_BYTE($7),PRINT_BYTE($8),'dnl
`PRINT_BYTE($9),PRINT_BYTE(0),PRINT_BYTE(0),PRINT_BYTE(0)')

dnl Fill bytes of mux/demux config binary blob
dnl blob is made of sof_abi_hdr (header.h), sof_mux_config (mux.h) and
dnl list of ROUTE_MATRIXes.
dnl reserved fields in the struct are set to 0
define(`MUXDEMUX_CONFIG',
`SectionData.STR($1) {'
`bytes "'`PRINT_BYTES_4(mux_sof_magic),PRINT_BYTES_4(0),'
`PRINT_BYTES_4(eval(mux_config_struct_size + (mux_stream_struct_size * $2))),'dnl
`PRINT_BYTES_4(SOF_ABI_VERSION),'
`PRINT_BYTES_4(0),PRINT_BYTES_4(0),'
`PRINT_BYTES_4(0),PRINT_BYTES_4(0),'
`PRINT_BYTES_2(0),PRINT_BYTES_2(0),PRINT_BYTES_2($2),PRINT_BYTES_2(0),'
`$3'`"'
`}'
)

dnl Mux name)
define(`N_MUXDEMUX', `MUXDEMUX'PIPELINE_ID`.'$1)

Expand Down
41 changes: 41 additions & 0 deletions tools/topology/m4/utils.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,5 +114,46 @@ define(`VIRTUAL_WIDGET',
` no_pm "true"'
`}', `fatal_error(`Invalid parameters ($#) to VIRTUAL_WIDGET')')')

dnl create SOF_ABI_VERSION if not defined
dnl you can give the abi.h with -DSOF_ABI_FILE=(full path to abi.h)
dnl otherwise this will be empty macro
ifdef(`SOF_ABI_VERSION', `',
ifdef(`SOF_ABI_FILE',
`define(`SOF_MAJOR','
`esyscmd(`grep "#define SOF_ABI_MAJOR "' SOF_ABI_FILE `| grep -E ".[[:digit:]]$" -o'))dnl'
`define(`SOF_MINOR','
`esyscmd(`grep "#define SOF_ABI_MINOR "' SOF_ABI_FILE `| grep -E ".[[:digit:]]$" -o'))dnl'
`define(`SOF_PATCH','
`esyscmd(`grep "#define SOF_ABI_PATCH "' SOF_ABI_FILE `| grep -E ".[[:digit:]]$" -o'))dnl'
`define(`SOF_MAJOR_SHIFT','
`esyscmd(`grep "#define SOF_ABI_MAJOR_SHIFT"' SOF_ABI_FILE `| grep -E ".[[:digit:]]$" -o'))dnl'
`define(`SOF_MINOR_SHIFT','
`esyscmd(`grep "#define SOF_ABI_MINOR_SHIFT"' SOF_ABI_FILE `| grep -E ".[[:digit:]]$" -o'))dnl'
`define(`SOF_ABI_VERSION','
`eval(eval((SOF_MAJOR) << (SOF_MAJOR_SHIFT))dnl'
`| eval((SOF_MINOR) << (SOF_MINOR_SHIFT))))dnl'
`'))

dnl print number's 4 bytes from right to left as hex values
define(`PRINT_BYTES_4',
`format(`0x%02x', eval(($1)&0xFF)),'dnl
`format(`0x%02x', eval(($1>>8)&0xFF)),'dnl
`format(`0x%02x', eval(($1>>16)&0xFF)),'dnl
`format(`0x%02x', eval(($1>>24)&0xFF))')dnl

dnl print number's 2 bytes from right to left as hex values
define(`PRINT_BYTES_2',
`format(`0x%02x', eval(($1)&0xFF)),'dnl
`format(`0x%02x', eval(($1>>8)&0xFF))')dnl

dnl print a number's right most byte as hex values
define(`PRINT_BYTE',
`format(`0x%02x', eval(($1)&0xFF))')dnl

dnl make a byte from 8 binary values, right to left in increasing argument order
define(`BITS_TO_BYTE',
`eval(eval($1 << 0) | eval($2 << 1) | eval($3 << 2) | eval($4 << 3)dnl
| eval($5 << 4) | eval($6 << 5) | eval($7 << 6) | eval($8 << 7))')dnl

divert(0) dnl

33 changes: 33 additions & 0 deletions tools/topology/sof-apl-demux-pcm512x.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ include(`utils.m4')
include(`dai.m4')
include(`pipeline.m4')
include(`ssp.m4')
include(`muxdemux.m4')

# Include TLV library
include(`common/tlv.m4')
Expand All @@ -19,6 +20,36 @@ include(`platform/intel/bxt.m4')

DEBUG_START

dnl Configure demux
dnl name, pipeline_id, routing_matrix_rows
dnl Diagonal 1's in routing matrix mean that every input channel is
dnl copied to corresponding output channels in all output streams.
dnl I.e. row index is the input channel, 1 means it is copied to
dnl corresponding output channel (column index), 0 means it is discarded.
dnl There's a separate matrix for all outputs.
define(matrix1, `ROUTE_MATRIX(1,
`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)')')

Copy link
Contributor

@keyonjie keyonjie Apr 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @juimonen since we are aligned that we don't plan to support channel mix features with mux/demux component, how about simplify this like below:

define(matrix1, `ROUTE_MATRIX(1, 0, 1, 2, 3, 4, 5, 6, 7)')

And change the ROUTE_MATRIX macro definition(something like BIT($i) will be good enough)) to be with bit indices inputs(we can use '-1' if it doesn't exist for the corresponding channel).

This will simplify the top level topology files a lot.

Copy link
Author

@juimonen juimonen Apr 1, 2020

Choose a reason for hiding this comment

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

@keyonjie are we aligned on that matter ? :)

I really would not want to castrate this component... I'm 99% sure we will need the mix feature very soon in some occasion.

Copy link
Contributor

@keyonjie keyonjie Apr 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@juimonen per my understanding yes. :)

If we are inheriting the MUX/DEMUX concepts from ALSA, which means N->1 or 1->N only, no mixing concept there. But if we want to innovate a more powerful component here (e.g. named shuffler? I actually proposed this to @plbossart and @lgirdwood in 2017), that is another topic.

The pros for the more powerful one is that it is more generic and we don't need to create many components like mixer/mux/channel_selector, just use this unified one with different configure blob.

The cons with it is that the component may run with lower efficiency(we need to check more bit masks and flags) and not so straightforward to be understood, especially the higher MCPS consuming make me nervous and that's why I voted to remove the mixing fancy feature from it.

@lgirdwood please help chime in this please, if mixing is here, then using mapping mechanism to simplify/optimize the mux component may become impossible since it is not 1chan->1chan mapping anymore, there could be channel accumulation(e.g. chan a = chan b + chan c).

Copy link
Author

@juimonen juimonen Apr 1, 2020

Choose a reason for hiding this comment

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

@Keyon yes I know the alsa mux concept and as you said this component is much more.

This should then be converted as official ASoC mux component, which also forces enum controls to user space.

I actually tried to convert this to ASoC mux at some point but stopped on the way, because this was much more versatile. I also remember talking for example with @singalsuo cases that would need more complicated functioning than simple mux.

  • We could also do following: use same component as "process" component with full API, as here now.
  • Make Asoc mux interface for the same component (restricted usage)

Copy link
Author

@juimonen juimonen Apr 1, 2020

Choose a reason for hiding this comment

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

@Keyon and agree with your observations about performance, the optimizations you mention should be done. However, if you don't know what you are doing, you can do very nasty things with all our components, build really strange pipelines that will consume the whole dsp resources.

Thinking of the interface, I really think the visual "routing matrix" is really the most informative with this kind of component. With enum we would have 2^64 different matrices and it is just an example of a component for which the configuration through ALSA controls is just awkward.

Copy link
Contributor

@keyonjie keyonjie Apr 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@Keyon and agree with your observations about performance, the optimizations you mention should be done. However, if you don't know what you are doing, you can do very nasty things with all our components, build really strange pipelines that will consume the whole dsp resources.

Thinking of the interface, I really think the visual "routing matrix" is really the most informative with this kind of component. With enum we would have 2^64 different matrices and it is just an example of a component for which the configuration through ALSA controls is just awkward.

Agree with that. If we do keep the current mux component design, it is unrealistic to use enum as @lgirdwood mentioned, and your "routing matrix" depicted here is more informative and topology user friendly, thank you for that.

Copy link
Member

@lgirdwood lgirdwood Apr 1, 2020

Choose a reason for hiding this comment

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

@juimonen @keyonjie we should be aiming for enums (once mixing is removed). The max numbers of enums/options we are looking at are

  1. demux - 8 source channels on 1 stream to map of 8 sink channels on 8 streams i.e. 8 enums of 64 options
  2. mux - 8 source channels on 8 streams to 8 sink channels on 1 stream i.e. 8 enums of 64 options max.
    In reality we should have less enums and options as 8x8 topologies will be rare.

define(matrix2, `ROUTE_MATRIX(5,
`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)')')

dnl name, num_streams, route_matrix list
MUXDEMUX_CONFIG(demux_priv, 2, LIST(` ', `matrix1,', `matrix2'))

#
# Define the pipelines
#
Expand All @@ -29,6 +60,8 @@ DEBUG_START
# PCM4 <---- demux
#



dnl PIPELINE_PCM_ADD(pipeline,
dnl pipe id, pcm, max channels, format,
dnl period, priority, core,
Expand Down
30 changes: 30 additions & 0 deletions tools/topology/sof-cml-demux-rt5682.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ include(`utils.m4')
include(`dai.m4')
include(`pipeline.m4')
include(`ssp.m4')
include(`muxdemux.m4')

# Include TLV library
include(`common/tlv.m4')
Expand All @@ -19,6 +20,35 @@ include(`platform/intel/'PLATFORM`.m4')

DEBUG_START

dnl Configure demux
dnl name, pipeline_id, routing_matrix_rows
dnl Diagonal 1's in routing matrix mean that every input channel is
dnl copied to corresponding output channels in all output streams.
dnl I.e. row index is the input channel, 1 means it is copied to
dnl corresponding output channel (column index), 0 means it is discarded.
dnl There's a separate matrix for all outputs.
define(matrix1, `ROUTE_MATRIX(1,
`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)')')

define(matrix2, `ROUTE_MATRIX(5,
`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)')')

dnl name, num_streams, route_matrix list
MUXDEMUX_CONFIG(demux_priv, 2, LIST(` ', `matrix1,', `matrix2'))

#
# Define the pipelines
Expand Down
34 changes: 34 additions & 0 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 @@ -7,6 +7,7 @@ include(`utils.m4')
include(`dai.m4')
include(`pipeline.m4')
include(`alh.m4')
include(`muxdemux.m4')

# Include TLV library
include(`common/tlv.m4')
Expand All @@ -19,6 +20,39 @@ include(`platform/intel/'PLATFORM`.m4')

DEBUG_START

dnl Configure demux
dnl name, pipeline_id, routing_matrix_rows
dnl Diagonal 1's in routing matrix mean that every input channel is
dnl copied to corresponding output channels in all output streams.
dnl I.e. row index is the input channel, 1 means it is copied to
dnl corresponding output channel (column index), 0 means it is discarded.
dnl There's a separate matrix for all outputs.
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)')')

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)')')

dnl name, num_streams, route_matrix list
MUXDEMUX_CONFIG(demux_priv, 2, LIST(` ', `matrix1,', `matrix2'))
')

#
# Define the pipelines
#
Expand Down
80 changes: 0 additions & 80 deletions tools/topology/sof/pipe-demux-playback.m4
View file
Open in desktop

This file was deleted.

5 changes: 1 addition & 4 deletions tools/topology/sof/pipe-volume-demux-playback.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,14 @@ include(`muxdemux.m4')
include(`mixercontrol.m4')
include(`bytecontrol.m4')

# Use default parameters
include(`demux_coef_default.m4')

# demux Bytes control with max value of 255
C_CONTROLBYTES(DEMUX, PIPELINE_ID,
CONTROLBYTES_OPS(bytes, 258 binds the mixer control to bytes get/put handlers, 258, 258),
CONTROLBYTES_EXTOPS(258 binds the mixer control to bytes get/put handlers, 258, 258),
, , ,
CONTROLBYTES_MAX(, 304),
,
DEMUX_priv)
demux_priv)

# Volume Mixer control with max value of 32
C_CONTROLMIXER(Master Playback Volume, PIPELINE_ID,
Expand Down