Skip to content
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

mic_privacy: initial implementation #9788

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+582 −47
Open

mic_privacy: initial implementation #9788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bcdc190
copier: gain: interface changes for setting gain params
mbukowsk Oct 8, 2024
7eb50fc
mic_privacy: initial implementation
mbukowsk Jul 5, 2024
aa73744
ipc: added IPC SET_MIC_PRIVACY_FW_MANAGED_POLICY_MAS
mbukowsk Jul 8, 2024
37336ba
mic_privacy: implement new IPC - SNDW_MIC_PRIVACY_HW_MANAGED_STATE_CH…
mbukowsk Jan 20, 2025
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
1 change: 1 addition & 0 deletions app/boards/intel_adsp_ace30_ptl.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ CONFIG_INTEL_ADSP_IPC=y
CONFIG_INTEL_ADSP_TIMER=y
CONFIG_MM_DRV_INTEL_ADSP_TLB_REMAP_UNUSED_RAM=y
CONFIG_SYS_CLOCK_TICKS_PER_SEC=12000
CONFIG_MICROPHONE_PRIVACY=y
abonislawski marked this conversation as resolved.
Show resolved Hide resolved

# Zephyr / power settings
CONFIG_ADSP_IMR_CONTEXT_SAVE=y
Expand Down
5 changes: 4 additions & 1 deletion posix/include/sof/lib/dma.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <rtos/atomic.h>
#include <rtos/bit.h>
#include <rtos/alloc.h>
#include <sof/audio/component.h>
#include <sof/lib/io.h>
#include <sof/lib/memory.h>
#include <rtos/sof.h>
Expand All @@ -34,6 +35,7 @@
#endif

struct comp_buffer;
struct comp_dev;

/** \addtogroup sof_dma_drivers DMA Drivers
* DMA Drivers API specification.
Expand Down Expand Up @@ -550,7 +552,8 @@ int dma_buffer_copy_to(struct comp_buffer __sparse_cache *source,
* conversion function. DMA buffer consume should be performed after the data has been copied
* to all sinks.
*/
int stream_copy_from_no_consume(struct comp_buffer __sparse_cache *source,
int stream_copy_from_no_consume(struct comp_dev *dev,
struct comp_buffer __sparse_cache *source,
struct comp_buffer __sparse_cache *sink,
dma_process_func process,
uint32_t source_bytes, uint32_t chmap);
Expand Down
3 changes: 3 additions & 0 deletions src/audio/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ if(NOT CONFIG_COMP_MODULE_SHARED_LIBRARY_BUILD)
if(CONFIG_COMP_VOLUME)
add_subdirectory(volume)
endif()
if(CONFIG_MICROPHONE_PRIVACY)
add_subdirectory(mic_privacy_manager)
endif()
subdirs(pipeline)
add_subdirectory(google)
if(CONFIG_COMP_CHAIN_DMA)
Expand Down
10 changes: 10 additions & 0 deletions src/audio/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ config IPC4_GATEWAY
host and DSP without using DMA: via memory window (audio payload) and
IPC4 messages (set/get/flush commands).

config MICROPHONE_PRIVACY
bool "Microphone privacy"
default n
depends on IPC_MAJOR_4
help
Audio privacy feature allows end user to directly control if user space
applications receive actual data from input devices (microphones).
The control is bypassing application level settings or operating system
controls (like audio endpoint volume).

rsource "up_down_mixer/Kconfig"

config COMP_BLOB
Expand Down
47 changes: 47 additions & 0 deletions src/audio/base_fw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
#include <sof/debug/telemetry/performance_monitor.h>
#include <sof/debug/telemetry/telemetry.h>
#include <sof/debug/telemetry/performance_monitor.h>
#if CONFIG_MICROPHONE_PRIVACY
#include <sof/audio/mic_privacy_manager.h>
#endif
/* 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
Expand Down Expand Up @@ -120,6 +123,18 @@ static int basefw_config(uint32_t *data_offset, char *data)
tlv_value_uint32_set(tuple, IPC4_FW_CONTEXT_SAVE,
IS_ENABLED(CONFIG_ADSP_IMR_CONTEXT_SAVE));

#if CONFIG_MICROPHONE_PRIVACY
struct privacy_capabilities priv_caps;

tuple = tlv_next(tuple);

priv_caps.privacy_version = 1;
priv_caps.capabilities_length = 1;
priv_caps.capabilities[0] = mic_privacy_get_policy_register();
abonislawski marked this conversation as resolved.
Show resolved Hide resolved

tlv_value_set(tuple, IPC4_PRIVACY_CAPS_HW_CFG, sizeof(priv_caps), &priv_caps);
Copy link
Contributor

@ujfalusi ujfalusi Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The 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.
IPC4_PRIVACY_CAPS_HW_CFG == IPC4_MAX_ASTATE_COUNT_FW_CFG

Copy link
Contributor

Choose a reason for hiding this comment

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

This still sets the mic privacy into the wrong config..

Copy link
Member

Choose a reason for hiding this comment

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

@ujfalusi this looks correct now.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, this has to be within basefw_hw_config() to be in correct place.

Copy link
Contributor

Choose a reason for hiding this comment

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

with this change you add two IPC4_MAX_ASTATE_COUNT_FW_CFG to the fw_config data.
Or two IPC4_PRIVACY_CAPS_HW_CFG.

In any case this is incorrect and will cause issues for the host.

#endif

tuple = tlv_next(tuple);

/* add platform specific tuples */
Expand Down Expand Up @@ -262,6 +277,34 @@ 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)
{
#if CONFIG_MICROPHONE_PRIVACY
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;
#else
return IPC4_UNAVAILABLE;
#endif
}

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
Expand Down Expand Up @@ -643,6 +686,10 @@ 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;
}
Expand Down
86 changes: 85 additions & 1 deletion src/audio/copier/copier.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@
#include "host_copier.h"
#include "dai_copier.h"
#include "ipcgtw_copier.h"
#if CONFIG_MICROPHONE_PRIVACY
#include <zephyr/drivers/mic_privacy.h>
#endif

#if CONFIG_ZEPHYR_NATIVE_DRIVERS
#include <zephyr/drivers/dai.h>
Expand All @@ -51,6 +54,10 @@ SOF_DEFINE_REG_UUID(copier);

DECLARE_TR_CTX(copier_comp_tr, SOF_UUID(copier_uuid), LOG_LEVEL_INFO);

#if CONFIG_MICROPHONE_PRIVACY
static int mic_privacy_configure(struct comp_dev *dev, struct copier_data *cd);
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

why don't you add the mic_privacy_configure() so that forward declaration is not needed?
Plus:

#else
static inline int mic_privacy_configure(struct comp_dev *dev, struct copier_data *cd) { return 0 }
#endif /* CONFIG_MICROPHONE_PRIVACY */

and you can save on the ifdefs in code


static int copier_init(struct processing_module *mod)
{
union ipc4_connector_node_id node_id;
Expand Down Expand Up @@ -131,6 +138,15 @@ static int copier_init(struct processing_module *mod)
comp_err(dev, "unable to create host");
goto error;
}
#if CONFIG_MICROPHONE_PRIVACY
if (cd->direction == SOF_IPC_STREAM_CAPTURE && node_id.f.dma_type == ipc4_hda_host_output_class) {
Copy link
Contributor

Choose a reason for hiding this comment

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

123 chars long..

ret = mic_privacy_configure(dev, cd);
if (ret < 0) {
comp_err(dev, "unable to configure mic privacy");
goto error;
}
}
#endif
break;
case ipc4_hda_link_output_class:
case ipc4_hda_link_input_class:
Expand All @@ -144,6 +160,15 @@ static int copier_init(struct processing_module *mod)
comp_err(dev, "unable to create dai");
goto error;
}
#if CONFIG_MICROPHONE_PRIVACY
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;
}
}
#endif
break;
#if CONFIG_IPC4_GATEWAY
case ipc4_ipc_output_class:
Expand Down Expand Up @@ -198,7 +223,10 @@ static int copier_free(struct processing_module *mod)
default:
break;
}

#if CONFIG_MICROPHONE_PRIVACY
if (cd->mic_priv)
rfree(cd->mic_priv);
Copy link
Collaborator

Choose a reason for hiding this comment

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

the if isn't needed, rfree(NULL) is fine

#endif
if (cd)
rfree(cd->gtw_cfg);
rfree(cd);
Expand Down Expand Up @@ -1098,6 +1126,62 @@ static int copier_unbind(struct processing_module *mod, void *data)
return 0;
}

#if CONFIG_MICROPHONE_PRIVACY
static void mic_privacy_event(void *arg, enum notify_id type, void *data)
{
struct mic_privacy_data *mic_priv_data = arg;
struct mic_privacy_settings *mic_privacy_settings = data;

if (type == NOTIFIER_ID_MIC_PRIVACY_STATE_CHANGE) {
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 == MIC_PRIV_UNMUTED) {
if (mic_priv_data->mic_privacy_state == MIC_PRIV_MUTED) {
mic_priv_data->mic_privacy_state = MIC_PRIV_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 != MIC_PRIV_MUTED) {
mic_priv_data->mic_privacy_state = MIC_PRIV_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;
}
}

static int mic_privacy_configure(struct comp_dev *dev, struct copier_data *cd)
{
struct mic_privacy_data *mic_priv_data;
int ret;

mic_priv_data = rzalloc(SOF_MEM_ZONE_RUNTIME, 0, SOF_MEM_CAPS_RAM, sizeof(struct mic_privacy_data));
if (!mic_priv_data) {
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) {
rfree(mic_priv_data);
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);
if (ret != 0) {
rfree(mic_priv_data);

return ret;
}
#endif

static struct module_endpoint_ops copier_endpoint_ops = {
.get_total_data_processed = copier_get_processed_data,
.position = copier_position,
Expand Down
11 changes: 11 additions & 0 deletions src/audio/copier/copier.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
#include <sof/compiler_attributes.h>
#include <sof/audio/buffer.h>
#include <sof/audio/pcm_converter.h>
#if CONFIG_MICROPHONE_PRIVACY
#include <sof/lib/notifier.h>
#include <sof/audio/mic_privacy_manager.h>
#endif

static const uint32_t INVALID_QUEUE_ID = 0xFFFFFFFF;

Expand Down Expand Up @@ -270,6 +274,9 @@ struct copier_data {
uint32_t channels[IPC4_ALH_MAX_NUMBER_OF_GTW];
uint32_t chan_map[IPC4_ALH_MAX_NUMBER_OF_GTW];
struct ipcgtw_data *ipcgtw_data;
#if CONFIG_MICROPHONE_PRIVACY
struct mic_privacy_data *mic_priv;
#endif
};

int apply_attenuation(struct comp_dev *dev, struct copier_data *cd,
Expand All @@ -291,4 +298,8 @@ enum sof_ipc_stream_direction

void copier_update_params(struct copier_data *cd, struct comp_dev *dev,
struct sof_ipc_stream_params *params);

#if CONFIG_MICROPHONE_PRIVACY
void mic_privacy_event(void *arg, enum notify_id type, void *data);
#endif
#endif
3 changes: 2 additions & 1 deletion src/audio/copier/copier_dai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
if (ret < 0) {
comp_err(dev, "Failed to set gain params!");
goto gain_free;
Expand Down
30 changes: 16 additions & 14 deletions src/audio/copier/copier_gain.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,22 @@

LOG_MODULE_DECLARE(copier, CONFIG_SOF_LOG_LEVEL);

int copier_gain_set_params(struct comp_dev *dev, struct dai_data *dd)
int copier_gain_set_params(struct comp_dev *dev,
struct copier_gain_params *gain_params,
uint32_t fade_period,
enum sof_ipc_dai_type dai_type)
{
struct processing_module *mod = comp_mod(dev);
struct copier_data *cd = module_get_private_data(mod);
struct ipc4_base_module_cfg *ipc4_cfg = &cd->config.base;
uint32_t sampling_freq = ipc4_cfg->audio_fmt.sampling_frequency;
uint32_t frames = sampling_freq / dev->pipeline->period;
uint32_t fade_period = GAIN_DEFAULT_FADE_PERIOD;
int ret;

/* Set basic gain parameters */
copier_gain_set_basic_params(dev, dd, ipc4_cfg);
copier_gain_set_basic_params(dev, gain_params, ipc4_cfg);

switch (dd->dai->type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

how did this compiled before this patch?
I cannot see dd anywhere in the change,

Copy link
Contributor

Choose a reason for hiding this comment

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

and if there were dd to begin with, then why we don't we take the pointers out from it locally?

switch (dai_type) {
case SOF_DAI_INTEL_DMIC:
{
struct dmic_config_data *dmic_cfg = cd->gtw_cfg;
Expand All @@ -43,18 +45,18 @@ int copier_gain_set_params(struct comp_dev *dev, struct dai_data *dd)
/* Get fade period from DMIC blob */
fade_period = dmic_glb_cfg->ext_global_cfg.fade_in_period;
/* Convert and assign silence and fade length values */
dd->gain_data->silence_sg_length =
gain_params->silence_sg_length =
frames * dmic_glb_cfg->ext_global_cfg.silence_period;
dd->gain_data->fade_sg_length = frames * fade_period;
gain_params->fade_sg_length = frames * fade_period;
}
break;
default:
comp_info(dev, "Apply default fade period for dai type %d", dd->dai->type);
comp_info(dev, "Apply default fade period for dai type %d", dai_type);
break;
}

/* Set fade parameters */
ret = copier_gain_set_fade_params(dev, dd, ipc4_cfg, fade_period, frames);
ret = copier_gain_set_fade_params(dev, gain_params, ipc4_cfg, fade_period, frames);
if (ret)
comp_err(dev, "Failed to set fade params");

Expand Down Expand Up @@ -150,7 +152,10 @@ int copier_gain_dma_control(union ipc4_connector_node_id node, const char *confi
break;
}

ret = copier_set_gain(dev, cd->dd[0], gain_data);
struct ipc4_copier_module_cfg *copier_cfg = cd->dd[0]->dai_spec_config;
const int channels = copier_cfg->base.audio_fmt.channels_count;

ret = copier_set_gain(dev, cd->dd[0]->gain_data, gain_data, channels);
Copy link
Contributor

Choose a reason for hiding this comment

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

two gain_data is a bit confusing here.

if (ret)
comp_err(dev, "Gain DMA control: failed to set gain");
return ret;
Expand All @@ -159,12 +164,9 @@ int copier_gain_dma_control(union ipc4_connector_node_id node, const char *confi
return -ENODEV;
}

int copier_set_gain(struct comp_dev *dev, struct dai_data *dd,
struct gain_dma_control_data *gain_data)
int copier_set_gain(struct comp_dev *dev, struct copier_gain_params *gain_params,
struct gain_dma_control_data *gain_data, int channels)
{
struct copier_gain_params *gain_params = dd->gain_data;
struct ipc4_copier_module_cfg *copier_cfg = dd->dai_spec_config;
const int channels = copier_cfg->base.audio_fmt.channels_count;
uint16_t static_gain[MAX_GAIN_COEFFS_CNT];
int ret;

Expand Down
Loading
Loading