-
Notifications
You must be signed in to change notification settings - Fork 323
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.
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
mic_privacy: initial implementation #9788
base: main
Are you sure you want to change the base?
Changes from all commits
7350f2f
f087cb5
6ae5174
ac8073f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,7 @@ | |
#include <sof/debug/telemetry/performance_monitor.h> | ||
#include <sof/debug/telemetry/telemetry.h> | ||
#include <sof/debug/telemetry/performance_monitor.h> | ||
#include <sof/audio/mic_privacy_manager.h> | ||
/* FIXME: | ||
* Builds for some platforms like tgl fail because their defines related to memory windows are | ||
* already defined somewhere else. Remove this ifdef after it's cleaned up | ||
|
@@ -49,6 +50,7 @@ static int basefw_config(uint32_t *data_offset, char *data) | |
uint16_t version[4] = {SOF_MAJOR, SOF_MINOR, SOF_MICRO, SOF_BUILD}; | ||
struct sof_tlv *tuple = (struct sof_tlv *)data; | ||
struct ipc4_scheduler_config sche_cfg; | ||
struct privacy_capabilities priv_caps; | ||
uint32_t plat_data_offset = 0; | ||
uint32_t log_bytes_size = 0; | ||
|
||
|
@@ -122,6 +124,16 @@ static int basefw_config(uint32_t *data_offset, char *data) | |
|
||
tuple = tlv_next(tuple); | ||
|
||
|
||
priv_caps.privacy_version = 1; | ||
priv_caps.capabilities_length = 1; | ||
priv_caps.capabilities[0] = mic_privacy_get_policy_register(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How can this even compile in case of CONFIG_MICROPHONE_PRIVACY=n (everything other than PTL)? |
||
|
||
tlv_value_set(tuple, IPC4_PRIVACY_CAPS_HW_CFG, sizeof(priv_caps), &priv_caps); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sets the privacy to the wrong TLV, this is the FW_CONFIG, not the HW_CONFIG. |
||
|
||
tuple = tlv_next(tuple); | ||
|
||
|
||
/* add platform specific tuples */ | ||
basefw_vendor_fw_config(&plat_data_offset, (char *)tuple); | ||
|
||
|
@@ -262,6 +274,29 @@ static int basefw_resource_allocation_request(bool first_block, | |
} | ||
} | ||
|
||
static int basefw_mic_priv_state_changed(bool first_block, | ||
bool last_block, | ||
uint32_t data_offset_or_size, | ||
const char *data) | ||
{ | ||
Comment on lines
+278
to
+281
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. indentation looks wrong |
||
tr_info(&basefw_comp_tr, "basefw_mic_priv_state_changed, status = %d", *data); | ||
|
||
uint32_t mic_disable_status = (uint32_t)(*data); | ||
struct mic_privacy_settings settings = fill_mic_priv_settings(mic_disable_status); | ||
propagate_privacy_settings(&settings); | ||
|
||
return 0; | ||
} | ||
|
||
static int basefw_set_mic_priv_policy(bool first_block, | ||
bool last_block, | ||
uint32_t data_offset_or_size, | ||
const char *data) | ||
{ | ||
tr_info(&basefw_comp_tr, "basefw_set_mic_priv_policy"); | ||
return 0; | ||
} | ||
|
||
static int basefw_power_state_info_get(uint32_t *data_offset, char *data) | ||
{ | ||
#if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL | ||
|
@@ -643,6 +678,11 @@ static int basefw_set_large_config(struct comp_dev *dev, | |
case IPC4_RESOURCE_ALLOCATION_REQUEST: | ||
return basefw_resource_allocation_request(first_block, last_block, data_offset, | ||
data); | ||
case SNDW_MIC_PRIVACY_HW_MANAGED_STATE_CHANGE: | ||
return basefw_mic_priv_state_changed(first_block, last_block, data_offset, | ||
data); | ||
case IPC4_SET_MIC_PRIVACY_FW_MANAGED_POLICY_MASK: | ||
return basefw_set_mic_priv_policy(first_block, last_block, data_offset, data); | ||
default: | ||
break; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,7 @@ | |
#include "host_copier.h" | ||
#include "dai_copier.h" | ||
#include "ipcgtw_copier.h" | ||
#include <zephyr/drivers/mic_privacy.h> | ||
|
||
#if CONFIG_ZEPHYR_NATIVE_DRIVERS | ||
#include <zephyr/drivers/dai.h> | ||
|
@@ -51,6 +52,8 @@ SOF_DEFINE_REG_UUID(copier); | |
|
||
DECLARE_TR_CTX(copier_comp_tr, SOF_UUID(copier_uuid), LOG_LEVEL_INFO); | ||
|
||
static int mic_privacy_configure(struct comp_dev *dev, struct copier_data *cd); | ||
|
||
static int copier_init(struct processing_module *mod) | ||
{ | ||
union ipc4_connector_node_id node_id; | ||
|
@@ -131,6 +134,15 @@ static int copier_init(struct processing_module *mod) | |
comp_err(dev, "unable to create host"); | ||
goto error; | ||
} | ||
|
||
if (cd->direction == SOF_IPC_STREAM_CAPTURE && node_id.f.dma_type == ipc4_hda_host_output_class) { | ||
ret = mic_privacy_configure(dev, cd); | ||
if (ret < 0) { | ||
comp_err(dev, "unable to configure mic privacy"); | ||
goto error; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. indentation, more cases below |
||
} | ||
|
||
break; | ||
case ipc4_hda_link_output_class: | ||
case ipc4_hda_link_input_class: | ||
|
@@ -144,6 +156,14 @@ static int copier_init(struct processing_module *mod) | |
comp_err(dev, "unable to create dai"); | ||
goto error; | ||
} | ||
|
||
if (cd->direction == SOF_IPC_STREAM_CAPTURE) { | ||
ret = mic_privacy_configure(dev, cd); | ||
if (ret < 0) { | ||
comp_err(dev, "unable to configure mic privacy"); | ||
goto error; | ||
} | ||
} | ||
break; | ||
#if CONFIG_IPC4_GATEWAY | ||
case ipc4_ipc_output_class: | ||
|
@@ -198,6 +218,8 @@ static int copier_free(struct processing_module *mod) | |
default: | ||
break; | ||
} | ||
if(cd->mic_priv) | ||
rfree(cd->mic_priv); | ||
|
||
if (cd) | ||
rfree(cd->gtw_cfg); | ||
|
@@ -1098,6 +1120,62 @@ static int copier_unbind(struct processing_module *mod, void *data) | |
return 0; | ||
} | ||
|
||
static void mic_privacy_event(void *arg, enum notify_id type, void *data) | ||
{ | ||
LOG_INF("mic_privacy_event"); | ||
struct mic_privacy_data *mic_priv_data = arg; | ||
struct mic_privacy_settings *mic_privacy_settings = data; | ||
|
||
LOG_INF("mic_privacy_event, arg = %p, data = %p", arg, data); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please reduce the number of logs to a minimum |
||
|
||
if (type == NOTIFIER_ID_MIC_PRIVACY_STATE_CHANGE) { | ||
|
||
LOG_INF("NOTIFIER_ID_MIC_PRIVACY_STATE_CHANGE, max ramp time = %d, ", mic_privacy_settings->max_ramp_time); | ||
LOG_INF("mic_privacy_event, state1 = %d, state2 = %d ", mic_privacy_settings->mic_privacy_state, mic_priv_data->mic_privacy_state); | ||
|
||
if (mic_privacy_settings->mic_privacy_state == UNMUTED) { | ||
if (mic_priv_data->mic_privacy_state == MUTED) { | ||
mic_priv_data->mic_privacy_state = FADE_IN; | ||
LOG_INF("mic_privacy_event switch to FADE_IN"); | ||
} | ||
} else { | ||
//In case when mute would be triggered before copier instantiation. | ||
if (mic_priv_data->mic_privacy_state != MUTED) { | ||
mic_priv_data->mic_privacy_state = FADE_OUT; | ||
LOG_INF("mic_privacy_event switch to FADE_OUT"); | ||
} | ||
} | ||
mic_priv_data->max_ramp_time_in_ms = (mic_privacy_settings->max_ramp_time * 1000) / ADSP_RTC_FREQUENCY; | ||
LOG_INF("max_ramp_time_in_ms= %d, audio_freq = %d, max_ramp_time = %d", mic_priv_data->max_ramp_time_in_ms, mic_priv_data->audio_freq, mic_privacy_settings->max_ramp_time); | ||
|
||
} | ||
} | ||
|
||
static int mic_privacy_configure(struct comp_dev *dev, struct copier_data *cd) | ||
{ | ||
int ret; | ||
struct mic_privacy_data* mic_priv_data; | ||
mic_priv_data = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, sizeof(struct mic_privacy_data)); | ||
if (!mic_priv_data) { | ||
rfree(mic_priv_data); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not needed, allocation failed |
||
return -ENOMEM; | ||
} | ||
|
||
mic_priv_data->audio_freq = cd->config.base.audio_fmt.sampling_frequency; | ||
|
||
uint32_t zeroing_wait_time = (get_dma_zeroing_wait_time() *1000) / ADSP_RTC_FREQUENCY; | ||
|
||
ret = copier_gain_set_params(dev, &mic_priv_data->mic_priv_gain_params, zeroing_wait_time, SOF_DAI_INTEL_NONE); | ||
if(ret != 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but here you do want to free memory |
||
return ret; | ||
|
||
cd->mic_priv = mic_priv_data; | ||
|
||
ret = notifier_register(cd->mic_priv, NULL, NOTIFIER_ID_MIC_PRIVACY_STATE_CHANGE, | ||
mic_privacy_event, 0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if notifier registration fails? You return an error, will the component be usable after that? If not - also free memory |
||
return ret; | ||
} | ||
|
||
static struct module_endpoint_ops copier_endpoint_ops = { | ||
.get_total_data_processed = copier_get_processed_data, | ||
.position = copier_position, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -229,7 +229,8 @@ static int copier_dai_init(struct comp_dev *dev, | |
} | ||
cd->dd[index]->gain_data = gain_data; | ||
|
||
ret = copier_gain_set_params(dev, cd->dd[index]); | ||
ret = copier_gain_set_params(dev, cd->dd[index]->gain_data, GAIN_DEFAULT_FADE_PERIOD, | ||
cd->dd[index]->dai->type); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. indentation |
||
if (ret < 0) { | ||
comp_err(dev, "Failed to set gain params!"); | ||
goto gain_free; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this belongs to the "Zephyr / device drivers" then you probably don't need an empty line above it