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

ASoC: SOF: Intel: Add support for hardware mic privacy change reporting #5312

Open
wants to merge 8 commits into
base: topic/sof-dev
Choose a base branch
from
Open

ASoC: SOF: Intel: Add support for hardware mic privacy change reporting #5312

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
6178620
ASoC: SOF: Intel: mtl: Split up dsp_ops setup code
ujfalusi Jan 27, 2025
e3cc721
ASoC: SOF: Intel: lnl/ptl: Only set dsp_ops which differs from MTL
ujfalusi Jan 27, 2025
3d5ff5b
ASoC: SOF: Intel: mtl: Stop exporting dsp_ops callback functions
ujfalusi Jan 27, 2025
54f085a
ASoC: SOF: Intel: Create ptl.c as placeholder for Panther Lake features
ujfalusi Jan 29, 2025
4ed3e51
ASoC: SOF: ipc4: Add support for Intel HW managed mic privacy messaging
ujfalusi Jan 29, 2025
bd8c18f
ASoC: SOF: Intel: hda-mlink: Add support for mic privacy in VS SHIM r…
ujfalusi Jan 29, 2025
fb1860e
ASoC: SOF: hda/shim: Add callbacks to handle mic privacy change for sdw
ujfalusi Jan 29, 2025
cf0443d
ASoC: SOF: Intel: ptl: Add support for mic privacy
ujfalusi Jan 29, 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
25 changes: 25 additions & 0 deletions include/sound/hda-mlink.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ struct mutex *hdac_bus_eml_get_mutex(struct hdac_bus *bus, bool alt, int elid);

int hdac_bus_eml_enable_offload(struct hdac_bus *bus, bool alt, int elid, bool enable);

/* microphone privacy specific function supported by ACE3+ architecture */
void hdac_bus_eml_set_mic_privacy_mask(struct hdac_bus *bus, bool alt, int elid,
unsigned long mask);
bool hdac_bus_eml_is_mic_privacy_changed(struct hdac_bus *bus, bool alt, int elid);
bool hdac_bus_eml_get_mic_privacy_state(struct hdac_bus *bus, bool alt, int elid);

Copy link
Member

Choose a reason for hiding this comment

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

I think the commit message should be clearer that the information is from hardware to host only. This is different to the previous patch in the sense that the host can select to receive information on the privacy changes, but it is not involved in the privacy flows.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The interrupt is to the owner of the IP, if we would have DMIC handling in kernel side then the interrupt would be coming to host.
I made the API generic to avoid churn (with sdw/dmic/ssp/hda/this/that/ variants) if ever we will have to deal with this on different IPs in kernel.

Let me think how I can improve the commit message.

#else

static inline int
Expand Down Expand Up @@ -185,4 +191,23 @@ hdac_bus_eml_enable_offload(struct hdac_bus *bus, bool alt, int elid, bool enabl
{
return 0;
}

static inline void
hdac_bus_eml_set_mic_privacy_mask(struct hdac_bus *bus, bool alt, int elid,
unsigned long mask)
{
}

static inline bool
hdac_bus_eml_is_mic_privacy_changed(struct hdac_bus *bus, bool alt, int elid)
{
return false;
}

static inline bool
hdac_bus_eml_get_mic_privacy_state(struct hdac_bus *bus, bool alt, int elid)
{
return false;
}

#endif /* CONFIG_SND_SOC_SOF_HDA_MLINK */
13 changes: 13 additions & 0 deletions include/sound/sof/ipc4/header.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ enum sof_ipc4_base_fw_params {
SOF_IPC4_FW_PARAM_MODULES_INFO_GET,
SOF_IPC4_FW_PARAM_LIBRARIES_INFO_GET = 16,
SOF_IPC4_FW_PARAM_SYSTEM_TIME = 20,
SOF_IPC4_FW_PARAM_MIC_PRIVACY_STATE_CHANGE = 35,
Copy link
Member

Choose a reason for hiding this comment

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

This is very controversial.
In the case of DMICs, the mute can be completely handled by firmware since it also processes the GPIO that toggles the privacy.
In the case of SoundWire, the mute is expected to be handled in the external codec.

In other words, this IPC does not seem to have a purpose other than academic - and it's mind-blowing that such a split implementation for a 'secure' solution relies on an IPC from the host. Recompile your kernel and the privacy is crippled... That does not sound good, does it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@plbossart, this is not the codec implemented privacy (SPE), I think SDCA spec also have similar described for the Function agent (iow, DSP).
The muting is controlled by hardware completely: it follows the mic privacy GPIO state, it is muting the incoming stream regardless what the firmware or Linux does.
But this muting would be 'rude'. We can request interrupt via audio IPs to their owner: DMIC is owned by firmware, so the interrupt goes there, SDW is owned by Linux, so the interrupt goes to Linux.
To provide better user experience, the FW is expected to do a fade on MUTE/UNMUTE (but regardless of this, the input stream is going to be muted).
FW can handle the DMIC case, but it needs a notification from Linux when SDW is used to be able to perform the fade.
If we don't send the notification to firmware the only thing that happens is that the mute is going to be immediate which is not too nice.

};

enum sof_ipc4_fw_config_params {
Expand Down Expand Up @@ -450,6 +451,18 @@ struct sof_ipc4_dx_state_info {
uint32_t dx_mask;
} __packed __aligned(4);

enum sof_ipc4_hw_config_params {
SOF_IPC4_HW_CFG_INTEL_MIC_PRIVACY_CAPS = 11,
};

#define SOF_IPC_INTEL_MIC_PRIVACY_VERSION_PTL 1

struct sof_ipc4_intel_mic_privacy_cap {
uint32_t version;
uint32_t capabilities_length;
uint32_t capabilities[];
} __packed;

/* Reply messages */

/*
Expand Down
2 changes: 1 addition & 1 deletion sound/soc/sof/intel/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ snd-sof-pci-intel-icl-y := pci-icl.o icl.o
snd-sof-pci-intel-tgl-y := pci-tgl.o tgl.o
snd-sof-pci-intel-mtl-y := pci-mtl.o mtl.o
snd-sof-pci-intel-lnl-y := pci-lnl.o lnl.o
snd-sof-pci-intel-ptl-y := pci-ptl.o
snd-sof-pci-intel-ptl-y := pci-ptl.o ptl.o

obj-$(CONFIG_SND_SOC_SOF_MERRIFIELD) += snd-sof-pci-intel-tng.o
obj-$(CONFIG_SND_SOC_SOF_INTEL_SKL) += snd-sof-pci-intel-skl.o
Expand Down
115 changes: 115 additions & 0 deletions sound/soc/sof/intel/hda-mlink.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
* @shim_offset: offset to SHIM register base
* @ip_offset: offset to IP register base
* @shim_vs_offset: offset to vendor-specific (VS) SHIM base
* @mic_privacy_mask: bitmask of sublinks where mic privacy is applied
*/
struct hdac_ext2_link {
struct hdac_ext_link hext_link;
Expand All @@ -65,6 +66,8 @@ struct hdac_ext2_link {
u32 shim_offset;
u32 ip_offset;
u32 shim_vs_offset;

unsigned long mic_privacy_mask;
};

#define hdac_ext_link_to_ext2(h) container_of(h, struct hdac_ext2_link, hext_link)
Expand All @@ -90,6 +93,13 @@ struct hdac_ext2_link {
#define AZX_REG_INTEL_UAOL_IP_OFFSET 0x100
#define AZX_REG_INTEL_UAOL_VS_SHIM_OFFSET 0xC00

/* Microphone privacy */
#define AZX_REG_INTEL_VS_SHIM_PVCCS 0x10
#define AZX_REG_INTEL_VS_SHIM_PVCCS_MDSTSCHGIE BIT(0)
#define AZX_REG_INTEL_VS_SHIM_PVCCS_MDSTSCHG BIT(8)
#define AZX_REG_INTEL_VS_SHIM_PVCCS_MDSTS BIT(9)
#define AZX_REG_INTEL_VS_SHIM_PVCCS_FMDIS BIT(10)

/* HDAML section - this part follows sequences in the hardware specification,
* including naming conventions and the use of the hdaml_ prefix.
* The code is intentionally minimal with limited dependencies on frameworks or
Expand Down Expand Up @@ -696,6 +706,14 @@ static int hdac_bus_eml_power_up_base(struct hdac_bus *bus, bool alt, int elid,
}

ret = hdaml_link_init(hlink->ml_addr + AZX_REG_ML_LCTL, sublink);
if ((h2link->mic_privacy_mask & BIT(sublink)) && !ret) {
u16 __iomem *pvccs = h2link->base_ptr +
h2link->shim_vs_offset +
sublink * h2link->instance_offset +
AZX_REG_INTEL_VS_SHIM_PVCCS;

writew(readw(pvccs) | AZX_REG_INTEL_VS_SHIM_PVCCS_MDSTSCHGIE, pvccs);
}

skip_init:
if (eml_lock)
Expand Down Expand Up @@ -742,6 +760,16 @@ static int hdac_bus_eml_power_down_base(struct hdac_bus *bus, bool alt, int elid
if (--h2link->sublink_ref_count[sublink] > 0)
goto skip_shutdown;
}

if (h2link->mic_privacy_mask & BIT(sublink)) {
u16 __iomem *pvccs = h2link->base_ptr +
h2link->shim_vs_offset +
sublink * h2link->instance_offset +
AZX_REG_INTEL_VS_SHIM_PVCCS;

writew(readw(pvccs) & ~AZX_REG_INTEL_VS_SHIM_PVCCS_MDSTSCHGIE, pvccs);
}

ret = hdaml_link_shutdown(hlink->ml_addr + AZX_REG_ML_LCTL, sublink);

skip_shutdown:
Expand Down Expand Up @@ -987,6 +1015,93 @@ int hdac_bus_eml_enable_offload(struct hdac_bus *bus, bool alt, int elid, bool e
}
EXPORT_SYMBOL_NS(hdac_bus_eml_enable_offload, "SND_SOC_SOF_HDA_MLINK");

void hdac_bus_eml_set_mic_privacy_mask(struct hdac_bus *bus, bool alt, int elid,
unsigned long mask)
{
struct hdac_ext2_link *h2link;

if (!mask)
return;

h2link = find_ext2_link(bus, alt, elid);
if (!h2link)
return;

if (__fls(mask) > h2link->slcount) {
dev_warn(bus->dev,
"%s: invalid sublink mask for %d:%d, slcount %d: %#lx\n",
__func__, alt, elid, h2link->slcount, mask);
return;
}

dev_dbg(bus->dev, "sublink mask for %d:%d, slcount %d: %#lx\n", alt,
elid, h2link->slcount, mask);

h2link->mic_privacy_mask = mask;
}
EXPORT_SYMBOL_NS(hdac_bus_eml_set_mic_privacy_mask, "SND_SOC_SOF_HDA_MLINK");

bool hdac_bus_eml_is_mic_privacy_changed(struct hdac_bus *bus, bool alt, int elid)
{
struct hdac_ext2_link *h2link;
bool changed = false;
u16 __iomem *pvccs;
int i;

h2link = find_ext2_link(bus, alt, elid);
if (!h2link)
return false;

/* The change in privacy state needs to be acked for each link */
for_each_set_bit(i, &h2link->mic_privacy_mask, h2link->slcount) {
u16 val;

if (h2link->sublink_ref_count[i] == 0)
continue;

pvccs = h2link->base_ptr +
h2link->shim_vs_offset +
i * h2link->instance_offset +
AZX_REG_INTEL_VS_SHIM_PVCCS;

val = readw(pvccs);
if (val & AZX_REG_INTEL_VS_SHIM_PVCCS_MDSTSCHG) {
writew(val, pvccs);
changed = true;
}
}

return changed;
}
EXPORT_SYMBOL_NS(hdac_bus_eml_is_mic_privacy_changed, "SND_SOC_SOF_HDA_MLINK");

bool hdac_bus_eml_get_mic_privacy_state(struct hdac_bus *bus, bool alt, int elid)
{
struct hdac_ext2_link *h2link;
u16 __iomem *pvccs;
int i;

h2link = find_ext2_link(bus, alt, elid);
if (!h2link)
return false;

for_each_set_bit(i, &h2link->mic_privacy_mask, h2link->slcount) {
if (h2link->sublink_ref_count[i] == 0)
continue;

/* Return the privacy state from the first active link */
pvccs = h2link->base_ptr +
h2link->shim_vs_offset +
i * h2link->instance_offset +
AZX_REG_INTEL_VS_SHIM_PVCCS;

return readw(pvccs) & AZX_REG_INTEL_VS_SHIM_PVCCS_MDSTS;
}

return false;
}
EXPORT_SYMBOL_NS(hdac_bus_eml_get_mic_privacy_state, "SND_SOC_SOF_HDA_MLINK");

#endif

MODULE_LICENSE("Dual BSD/GPL");
Expand Down
34 changes: 34 additions & 0 deletions sound/soc/sof/intel/hda.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,27 @@ void hda_sdw_process_wakeen_common(struct snd_sof_dev *sdev)
}
EXPORT_SYMBOL_NS(hda_sdw_process_wakeen_common, "SND_SOC_SOF_INTEL_HDA_GENERIC");

static bool hda_dsp_sdw_check_mic_privacy_irq(struct snd_sof_dev *sdev)
{
const struct sof_intel_dsp_desc *chip;

chip = get_chip_info(sdev->pdata);
if (chip && chip->check_mic_privacy_irq)
return chip->check_mic_privacy_irq(sdev, true,
AZX_REG_ML_LEPTR_ID_SDW);

return false;
}

static void hda_dsp_sdw_process_mic_privacy(struct snd_sof_dev *sdev)
{
const struct sof_intel_dsp_desc *chip;

chip = get_chip_info(sdev->pdata);
if (chip && chip->process_mic_privacy)
chip->process_mic_privacy(sdev, true, AZX_REG_ML_LEPTR_ID_SDW);
}

#else /* IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE) */
static inline int hda_sdw_acpi_scan(struct snd_sof_dev *sdev)
{
Expand Down Expand Up @@ -383,6 +404,13 @@ static inline bool hda_sdw_check_wakeen_irq(struct snd_sof_dev *sdev)
return false;
}

static inline bool hda_dsp_sdw_check_mic_privacy_irq(struct snd_sof_dev *sdev)
{
return false;
}

static inline void hda_dsp_sdw_process_mic_privacy(struct snd_sof_dev *sdev) { }

#endif /* IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE) */

/* pre fw run operations */
Expand Down Expand Up @@ -683,7 +711,13 @@ static irqreturn_t hda_dsp_interrupt_thread(int irq, void *context)

if (hda_dsp_check_sdw_irq(sdev)) {
trace_sof_intel_hda_irq(sdev, "sdw");

hda_dsp_sdw_thread(irq, hdev->sdw);

if (hda_dsp_sdw_check_mic_privacy_irq(sdev)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this have to inside the if (hda_dsp_check_sdw_irq(sdev)) @ujfalusi or is it independent of the sdw_irq like the wakeen irq?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The mic privacy interrupt in PTL is piggy-backing on the SDW interrupt for host (and on DMIC interrupt in DSP), if the SDW is not powered up we will not receive interrupt about the change. Even if we have SDW active, but the link which is under HW managed mic privacy mode is not active we will not receive interrupt (we only enable the interrupt via the managed link).

trace_sof_intel_hda_irq(sdev, "mic privacy");
hda_dsp_sdw_process_mic_privacy(sdev);
}
}

if (hda_sdw_check_wakeen_irq(sdev)) {
Expand Down
4 changes: 0 additions & 4 deletions sound/soc/sof/intel/hda.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -913,10 +913,6 @@ extern struct snd_sof_dsp_ops sof_tgl_ops;
int sof_tgl_ops_init(struct snd_sof_dev *sdev);
extern struct snd_sof_dsp_ops sof_icl_ops;
int sof_icl_ops_init(struct snd_sof_dev *sdev);
extern struct snd_sof_dsp_ops sof_mtl_ops;
int sof_mtl_ops_init(struct snd_sof_dev *sdev);
extern struct snd_sof_dsp_ops sof_lnl_ops;
int sof_lnl_ops_init(struct snd_sof_dev *sdev);

extern const struct sof_intel_dsp_desc skl_chip_info;
extern const struct sof_intel_dsp_desc apl_chip_info;
Expand Down
Loading
Loading