Skip to content

[RFC] topology: set mux/demux parameters with tuples #2345

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

Closed
juimonen wants to merge 1 commit into from
Closed

[RFC] topology: set mux/demux parameters with tuples #2345

Show file tree
Hide file tree
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
51 changes: 39 additions & 12 deletions tools/topology/m4/muxdemux.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,23 @@ dnl Define macro for demux widget
dnl Mux name)
define(`N_MUXDEMUX', `MUXDEMUX'PIPELINE_ID`.'$1)

dnl W_MUXDEMUX(name, mux/demux, format, periods_sink, periods_source, kcontrol_list)
dnl CH_MAP_COEFF(coeff)
Copy link
Collaborator

@ranj063 ranj063 Feb 20, 2020

Choose a reason for hiding this comment

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

id, coeff?

Copy link
Author

@juimonen juimonen Feb 21, 2020

Choose a reason for hiding this comment

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

yes

define(`CH_MAP_COEFF',
` tuples."word.$1" {'
` SOF_TKN_CHANNEL_MAP_COEFF' STR($2)
` }')

dnl CH_MAP(name, id, ext_id, mask)
define(`CH_MAP',
` tuples."word.$1" {'
Copy link
Collaborator

@ranj063 ranj063 Feb 20, 2020

Choose a reason for hiding this comment

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

did you mean $1 here? or $2?

Copy link
Author

@juimonen juimonen Feb 21, 2020

Choose a reason for hiding this comment

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

so as I understand the $1 is like "name". So it should be the same "name" meaning in both CH_MAP and CHMAP_COEFF as we have actual "id" token member in the other... goes quite confusing otherwise. I'm not totally sure what is actually the meaning of this "word.xxx" syntax, can we actually parse/see that in driver/kernel side? So I copied this from your dmic pdm...

Copy link
Collaborator

@ranj063 ranj063 Feb 20, 2020

Choose a reason for hiding this comment

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

missing tokens "sof_chmap_tokens"?

Copy link
Author

@juimonen juimonen Feb 21, 2020

Choose a reason for hiding this comment

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

it is included in _mux_data_tuples_w which has the tokens "sof_chmap_tokens"... should I do it somehow differently?

` SOF_TKN_CHANNEL_MAP_INDEX' STR($2)
` SOF_TKN_CHANNEL_MAP_EXT_ID' STR($3)
` SOF_TKN_CHANNEL_MAP_MASK' STR($4)
` }'
$5
)

dnl W_MUXDEMUX(name, format, periods_sink, periods_source, num_chmaps, chmap_list)
define(`W_MUXDEMUX',
`SectionVendorTuples."'N_MUXDEMUX($1)`_tuples_w" {'
` tokens "sof_comp_tokens"'
Expand All @@ -20,18 +36,30 @@ define(`W_MUXDEMUX',
`SectionVendorTuples."'N_MUXDEMUX($1)`_tuples_str" {'
` tokens "sof_comp_tokens"'
` tuples."string" {'
` SOF_TKN_COMP_FORMAT' STR($3)
` }'
`}'
`SectionVendorTuples."'N_MUXDEMUX($1)`_mux_process_tuples_str" {'
` tokens "sof_process_tokens"'
` tuples."string" {'
`ifelse(`$2', `0', ` SOF_TKN_PROCESS_TYPE' "MUX", ` SOF_TKN_PROCESS_TYPE' "DEMUX")'
` SOF_TKN_COMP_FORMAT' STR($2)
` }'
`}'
`SectionData."'N_MUXDEMUX($1)`_data_str" {'
` tuples "'N_MUXDEMUX($1)`_tuples_str"'
` tuples "'N_MUXDEMUX($1)`_mux_process_tuples_str"'
`}'
`SectionVendorTuples."'N_MUXDEMUX($1)`_mux_tuples_type" {'
` tokens "sof_process_tokens"'
` tuples."string" {'
` SOF_TKN_PROCESS_TYPE "MUX"'
` }'
`}'
`SectionData."'N_MUXDEMUX($1)`_data_type" {'
` tuples "'N_MUXDEMUX($1)`_mux_tuples_type"'
`}'
`SectionVendorTuples."'N_MUXDEMUX($1)`_mux_data_tuples_w" {'
` tokens "sof_chmap_tokens"'
` tuples."word" {'
` SOF_TKN_STREAM_MAP_NUM_CH_MAPS' STR($5)
` }'
$6
`}'
`SectionData."'N_MUXDEMUX($1)`_mux_data_w" {'
` tuples "'N_MUXDEMUX($1)`_mux_data_tuples_w"'
`}'
`SectionWidget."'N_MUXDEMUX($1)`" {'
` index "'PIPELINE_ID`"'
Expand All @@ -40,9 +68,8 @@ define(`W_MUXDEMUX',
` data ['
` "'N_MUXDEMUX($1)`_data_w"'
` "'N_MUXDEMUX($1)`_data_str"'
` ]'
` bytes ['
$6
` "'N_MUXDEMUX($1)`_data_type"'
` "'N_MUXDEMUX($1)`_mux_data_w"'
` ]'
`}')

Expand Down
13 changes: 1 addition & 12 deletions tools/topology/sof/pipe-demux-playback.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,6 @@ 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)
#
# Components and Buffers
#
Expand All @@ -42,7 +31,7 @@ C_CONTROLBYTES(DEMUX, PIPELINE_ID,
W_PCM_PLAYBACK(PCM_ID, Low Latency Playback, 2, 0)

# Mux 0 has 2 sink and source periods.
W_MUXDEMUX(0, 1, PIPELINE_FORMAT, 2, 2, LIST(` ', "DEMUX"))
W_MUXDEMUX(0, PIPELINE_FORMAT, 2, 2, 2, LIST(` ', CH_MAP(map_1, 0, 1, 5, LIST(` ', CH_MAP_COEFF(1, 34954), CH_MAP_COEFF(2, 87343))), CH_MAP(map_2, 1, 5, 5, LIST(` ', CH_MAP_COEFF(3, 12345), CH_MAP_COEFF(4, 54321)))))

# Low Latency Buffers
W_BUFFER(0, COMP_BUFFER_SIZE(2,
Expand Down
15 changes: 1 addition & 14 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 @@ -20,19 +20,6 @@ include(`pcm.m4')
include(`pga.m4')
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)

# Volume Mixer control with max value of 32
C_CONTROLMIXER(Master Playback Volume, PIPELINE_ID,
Expand Down Expand Up @@ -65,7 +52,7 @@ W_PCM_PLAYBACK(PCM_ID, Low Latency Playback, 2, 0)
W_PGA(1, PIPELINE_FORMAT, DAI_PERIODS, 2, playback_pga_conf, LIST(` ', "PIPELINE_ID Master Playback Volume"))

# Mux 0 has 2 sink and source periods.
W_MUXDEMUX(0, 1, PIPELINE_FORMAT, 2, 2, LIST(` ', "DEMUX"))
W_MUXDEMUX(0, PIPELINE_FORMAT, 2, 2, 2, LIST(` ', CH_MAP(map_1, 0, 1, 5, LIST(` ', CH_MAP_COEFF(1, 34954), CH_MAP_COEFF(2, 87343))), CH_MAP(map_2, 1, 5, 5, LIST(` ', CH_MAP_COEFF(3, 12345), CH_MAP_COEFF(4, 54321)))))

# Low Latency Buffers
W_BUFFER(0, COMP_BUFFER_SIZE(2,
Expand Down
8 changes: 8 additions & 0 deletions tools/topology/sof/tokens.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,11 @@ SectionVendorTokens."sof_led_tokens" {
SOF_TKN_MUTE_LED_USE "1300"
SOF_TKN_MUTE_LED_DIRECTION "1301"
}

SectionVendorTokens."sof_chmap_tokens" {
SOF_TKN_STREAM_MAP_NUM_CH_MAPS "1400"
Copy link
Collaborator

@ranj063 ranj063 Feb 20, 2020

Choose a reason for hiding this comment

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

this has to be split into a separate token maybe called sof_mux_demux_token

Copy link
Author

@juimonen juimonen Feb 21, 2020

Choose a reason for hiding this comment

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

I mean sure... So what is the reasoning for this? It could be that the channel maps etc are used elsewhere also than demux only. I mean maybe it would be somehow more clear, but basically we are defining the "parse unit" in the kernel side tuples array definitions, so that could easily combine tuple id's all over and no one would notice...

SOF_TKN_CHANNEL_MAP_INDEX "1401"
SOF_TKN_CHANNEL_MAP_EXT_ID "1402"
SOF_TKN_CHANNEL_MAP_MASK "1403"
SOF_TKN_CHANNEL_MAP_COEFF "1404"
Copy link
Collaborator

@ranj063 ranj063 Feb 20, 2020

Choose a reason for hiding this comment

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

I am confused by this too. This suggests that sof_ch_map_tokens contains exactly one channel_map_coeff but there could be multiple ones no?

Copy link
Author

@juimonen juimonen Feb 20, 2020

Choose a reason for hiding this comment

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

well it is just the token id for coeff. Yes there can be var array of coeffs in var array of ch_maps (which are in stream map struct). I made an example in sof side with 2 ch_maps with 2 coeffs each...

Copy link
Collaborator

@ranj063 ranj063 Feb 20, 2020

Choose a reason for hiding this comment

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

I will also split the coeff into a separate set of tokens just like you've done in your linux PR

Copy link
Author

@juimonen juimonen Feb 21, 2020

Choose a reason for hiding this comment

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

sorry I didn't get what you mean here..?

Copy link
Member

@lgirdwood lgirdwood Feb 21, 2020

Choose a reason for hiding this comment

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

Can you comment the tokens too - we should really be doing this for all tokens.

}