Audio: Multiband-DRC: Add switch control for IPC4 #8140
Conversation
singalsu
force-pushed
the
multiband_drc_add_ipc4_switch_control
branch
from
8f1d2b3 to
6ed5db4
Compare
singalsu
requested review from
a team,
RanderWang,
marcinszkudlinski,
pblaszko,
plbossart,
mmaka1,
lbetlej and
dbaluta
as code owners
singalsu
force-pushed
the
multiband_drc_add_ipc4_switch_control
branch
2 times, most recently
from
1ea9a49 to
95aa269
Compare
| comp_info(dev, "process_enabled = %d", cd->process_enabled); | ||
| } else { | ||
| comp_err(dev, "Illegal control id = %d, num_elems = %d", | ||
| ctl->id, ctl->num_elems); |
There was a problem hiding this comment.
but still returning 0 - ignoring and continuing?
There was a problem hiding this comment.
should not, need return a negative value.
There was a problem hiding this comment.
I'll test what happens if I return error, I didn't want to stop streaming so I left it out, but maybe it doesn't. It's good if only amixer errors.
There was a problem hiding this comment.
The control handler now errors as requested. The playback doesn't start at all since the control is sent to FW after init. I guess it's good to error incorrect topologies instead of just logging the errors, so proposing this now.
| #if CONFIG_IPC_MAJOR_3 | ||
| /* Other control types are possible only with IPC3 */ | ||
| #if CONFIG_IPC_MAJOR_4 | ||
| struct sof_ipc4_control_msg_payload *ctl; |
There was a problem hiding this comment.
line 336 only used under line 339, can move to line 339
There was a problem hiding this comment.
I combined the pointer declare and set. OK now?
| comp_info(dev, "process_enabled = %d", cd->process_enabled); | ||
| } else { | ||
| comp_err(dev, "Illegal control id = %d, num_elems = %d", | ||
| ctl->id, ctl->num_elems); |
There was a problem hiding this comment.
should not, need return a negative value.
The structs sof_ipc4_control_msg_payload and sof_ipc4_ctrl_value_chan match the similar structures in Linux kernel SOF IPC4 driver. Signed-off-by: Seppo Ingalsuo <[email protected]>
This patch adds switch control handling into multiband_drc_set_config() operation. The switch control is a set large config with param_id set to SOF_IPC4_SWITCH_CONTROL_PARAM_ID. The patch changes config_id to param_id in this function. The config_id name was confusing. Signed-off-by: Seppo Ingalsuo <[email protected]>
The workaround is preserved but the comment is updated to describe why. Signed-off-by: Seppo Ingalsuo <[email protected]>
singalsu
force-pushed
the
multiband_drc_add_ipc4_switch_control
branch
from
95aa269 to
1926420
Compare
|
"45 successful and 2 failing checks" The sparse issue is not related. Also the not tested in cavs2.5 jenkins can't be related. |
|
is cavs2.5 CI down? |
|
SOFCI TEST |
|
@cujomalainey The CI results publishing has an issue (fix under way). I'm looking up the results manually during this time. cAVS2.5 test results ok, ACE results still in progress (but all good so far). Once that completes, we can merge this. |
|
@cujomalainey Some good news, we have now successfully tested also enum control. Switch and enum controls update notification from firmware to user space works too. I'll update the example later to TDFB component. |
No description provided.