Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions src/audio/base_fw.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <ipc4/base_fw_vendor.h>
#include <ipc4/pipeline.h>
#include <ipc4/logging.h>
#include <ipc4/notification.h>
#include <ipc/topology.h>
#include <ipc/compress_params.h>
#include <sof_versions.h>
Expand Down Expand Up @@ -696,6 +697,18 @@ __cold static int basefw_get_large_config(struct comp_dev *dev, uint32_t param_i
data_offset, data);
};

__cold static int basefw_notification_mask_info(const char *data)
{
struct ipc4_notification_mask_info *mask_info;
Copy link
Collaborator

Choose a reason for hiding this comment

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

we could make the argument const void *data and just initialise struct ipc4_notification_mask_info *mask_info = data; directly here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are right. I just used a similar function as a template ;)


assert_can_be_cold();

mask_info = (struct ipc4_notification_mask_info *)data;
ipc4_update_notification_mask(mask_info->ntfy_mask, mask_info->enabled_mask);

return IPC4_SUCCESS;
}

__cold static int basefw_astate_table(void)
{
assert_can_be_cold();
Expand Down Expand Up @@ -757,6 +770,8 @@ __cold static int basefw_set_large_config(struct comp_dev *dev, uint32_t param_i
assert_can_be_cold();

switch (param_id) {
case IPC4_NOTIFICATION_MASK:
return basefw_notification_mask_info(data);
case IPC4_ASTATE_TABLE:
return basefw_astate_table();
case IPC4_DMA_CONTROL:
Expand Down
22 changes: 8 additions & 14 deletions src/audio/chain_dma.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#include <zephyr/pm/policy.h>
#include <rtos/init.h>
#if CONFIG_XRUN_NOTIFICATIONS_ENABLE
#include <sof/ipc/notification_pool.h>
#include <ipc4/notification.h>
#endif

Expand Down Expand Up @@ -150,19 +149,14 @@ static int chain_get_dma_status(struct chain_dma_data *cd, struct dma_chan_data
int ret = dma_get_status(chan->dma->z_dev, chan->index, stat);
#if CONFIG_XRUN_NOTIFICATIONS_ENABLE
if (ret == -EPIPE && !cd->xrun_notification_sent) {
struct ipc_msg *notify = ipc_notification_pool_get(IPC4_RESOURCE_EVENT_SIZE);

if (notify) {
if (cd->stream_direction == SOF_IPC_STREAM_PLAYBACK)
gateway_underrun_notif_msg_init(notify,
cd->link_connector_node_id.dw);
else
gateway_overrun_notif_msg_init(notify,
cd->link_connector_node_id.dw);

ipc_msg_send(notify, notify->tx_data, false);
cd->xrun_notification_sent = true;
}
uint32_t node_id = cd->link_connector_node_id.dw;
bool notif_sent = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

superfluous initialisation


if (cd->stream_direction == SOF_IPC_STREAM_PLAYBACK)
notif_sent = send_gateway_underrun_notif_msg(node_id);
else
notif_sent = send_gateway_overrun_notif_msg(node_id);
cd->xrun_notification_sent = notif_sent;
} else if (!ret) {
cd->xrun_notification_sent = false;
}
Expand Down
20 changes: 7 additions & 13 deletions src/audio/dai-zephyr.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
#include <stdint.h>

#if CONFIG_XRUN_NOTIFICATIONS_ENABLE
#include <sof/ipc/notification_pool.h>
#include <ipc4/notification.h>
#endif

Expand Down Expand Up @@ -1478,19 +1477,14 @@ static int dai_get_status(struct comp_dev *dev, struct dai_data *dd, struct dma_
int ret = sof_dma_get_status(dd->chan->dma, dd->chan->index, stat);
#if CONFIG_XRUN_NOTIFICATIONS_ENABLE
if (ret == -EPIPE && !dd->xrun_notification_sent) {
struct ipc_msg *notify = ipc_notification_pool_get(IPC4_RESOURCE_EVENT_SIZE);
uint32_t ppl_id = dev->pipeline->pipeline_id;
bool notif_sent = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto


if (notify) {
if (dev->direction == SOF_IPC_STREAM_PLAYBACK)
copier_gateway_underrun_notif_msg_init(notify,
dev->pipeline->pipeline_id);
else
copier_gateway_overrun_notif_msg_init(notify,
dev->pipeline->pipeline_id);

ipc_msg_send(notify, notify->tx_data, false);
dd->xrun_notification_sent = true;
}
if (dev->direction == SOF_IPC_STREAM_PLAYBACK)
notif_sent = send_copier_gateway_underrun_notif_msg(ppl_id);
else
notif_sent = send_copier_gateway_overrun_notif_msg(ppl_id);
dd->xrun_notification_sent = notif_sent;
} else if (!ret) {
dd->xrun_notification_sent = false;
}
Expand Down
22 changes: 8 additions & 14 deletions src/audio/host-zephyr.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
#include <stdint.h>

#if CONFIG_XRUN_NOTIFICATIONS_ENABLE
#include <sof/ipc/notification_pool.h>
#include <ipc4/notification.h>
#endif

Expand Down Expand Up @@ -368,19 +367,14 @@ static int host_get_status(struct comp_dev *dev, struct host_data *hd, struct dm
int ret = sof_dma_get_status(hd->chan->dma, hd->chan->index, stat);
#if CONFIG_XRUN_NOTIFICATIONS_ENABLE
if (ret == -EPIPE && !hd->xrun_notification_sent) {
struct ipc_msg *notify = ipc_notification_pool_get(IPC4_RESOURCE_EVENT_SIZE);

if (notify) {
if (dev->direction == SOF_IPC_STREAM_PLAYBACK)
copier_gateway_underrun_notif_msg_init(notify,
dev->pipeline->pipeline_id);
else
copier_gateway_overrun_notif_msg_init(notify,
dev->pipeline->pipeline_id);

ipc_msg_send(notify, notify->tx_data, false);
hd->xrun_notification_sent = true;
}
uint32_t ppl_id = dev->pipeline->pipeline_id;
bool notif_sent = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here


if (dev->direction == SOF_IPC_STREAM_PLAYBACK)
notif_sent = send_copier_gateway_underrun_notif_msg(ppl_id);
else
notif_sent = send_copier_gateway_overrun_notif_msg(ppl_id);
hd->xrun_notification_sent = notif_sent;
} else if (!ret) {
hd->xrun_notification_sent = false;
}
Expand Down
10 changes: 1 addition & 9 deletions src/audio/mixin_mixout/mixin_mixout.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include <sof/compiler_attributes.h>
#include <rtos/panic.h>
#include <sof/ipc/msg.h>
#include <sof/ipc/notification_pool.h>
#include <rtos/init.h>
#include <sof/lib/uuid.h>
#include <sof/list.h>
Expand Down Expand Up @@ -253,8 +252,6 @@ static void mixin_check_notify_underrun(struct comp_dev *dev, struct mixin_data
const bool eos_detected = state == AUDIOBUF_STATE_END_OF_STREAM_FLUSH ||
state == AUDIOBUF_STATE_END_OF_STREAM;

struct ipc_msg *notify;

mixin_data->last_reported_underrun++;

if (!source_avail || eos_detected) {
Expand All @@ -273,13 +270,8 @@ static void mixin_check_notify_underrun(struct comp_dev *dev, struct mixin_data
(eos_detected && mixin_data->eos_delay_periods == 0)) {
mixin_data->last_reported_underrun = 0;

notify = ipc_notification_pool_get(IPC4_RESOURCE_EVENT_SIZE);
if (!notify)
return;

mixer_underrun_notif_msg_init(notify, dev->ipc_config.id, eos_detected,
send_mixer_underrun_notif_msg(dev->ipc_config.id, eos_detected,
source_avail, sinks_free);
ipc_msg_send(notify, notify->tx_data, false);
}
}
}
Expand Down
10 changes: 1 addition & 9 deletions src/audio/pipeline/pipeline-stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include <rtos/kernel.h>
#include <sof/audio/module_adapter/module/generic.h>
#include <sof/lib/cpu-clk-manager.h>
#include <sof/ipc/notification_pool.h>

#ifdef CONFIG_IPC_MAJOR_4
#include <ipc4/notification.h>
Expand Down Expand Up @@ -96,14 +95,7 @@ pipeline_should_report_enodata_on_trigger(struct comp_dev *rsrc,
void pipeline_comp_copy_error_notify(const struct comp_dev *component, int err)
{
#ifdef CONFIG_IPC_MAJOR_4
struct ipc_msg *notify;

notify = ipc_notification_pool_get(IPC4_RESOURCE_EVENT_SIZE);
if (!notify)
return;

process_data_error_notif_msg_init(notify, component->ipc_config.id, err);
ipc_msg_send(notify, notify->tx_data, false);
send_process_data_error_notif_msg(component->ipc_config.id, err);
#endif
}

Expand Down
16 changes: 8 additions & 8 deletions src/include/ipc4/notification.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,15 +288,15 @@ struct ipc4_resource_event_data_notification {

#define IPC4_RESOURCE_EVENT_SIZE sizeof(struct ipc4_resource_event_data_notification)

void process_data_error_notif_msg_init(struct ipc_msg *msg, uint32_t resource_id,
uint32_t error_code);
void send_process_data_error_notif_msg(uint32_t resource_id, uint32_t error_code);

void copier_gateway_underrun_notif_msg_init(struct ipc_msg *msg, uint32_t pipeline_id);
void copier_gateway_overrun_notif_msg_init(struct ipc_msg *msg, uint32_t pipeline_id);
void gateway_underrun_notif_msg_init(struct ipc_msg *msg, uint32_t resource_id);
void gateway_overrun_notif_msg_init(struct ipc_msg *msg, uint32_t resource_id);
bool send_copier_gateway_underrun_notif_msg(uint32_t pipeline_id);
bool send_copier_gateway_overrun_notif_msg(uint32_t pipeline_id);
bool send_gateway_underrun_notif_msg(uint32_t resource_id);
bool send_gateway_overrun_notif_msg(uint32_t resource_id);

void mixer_underrun_notif_msg_init(struct ipc_msg *msg, uint32_t resource_id, uint32_t eos_flag,
uint32_t data_mixed, uint32_t expected_data_mixed);
void send_mixer_underrun_notif_msg(uint32_t resource_id, uint32_t eos_flag, uint32_t data_mixed,
uint32_t expected_data_mixed);
void ipc4_update_notification_mask(uint32_t ntfy_mask, uint32_t enabled_mask);

#endif /* __IPC4_NOTIFICATION_H__ */
131 changes: 81 additions & 50 deletions src/ipc/ipc4/notification.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,85 +10,116 @@
#include <sof/ipc/msg.h>
#include <stdbool.h>
#include <ipc4/notification.h>
#include <sof/ipc/notification_pool.h>

#include <rtos/symbol.h>

static void resource_notif_header_init(struct ipc_msg *msg)
static uint32_t notification_mask = 0xFFFFFFFF;

static bool is_notif_filtered_out(uint32_t event_type)
{
struct ipc4_resource_event_data_notification *notif_data = msg->tx_data;
uint32_t notif_idx;

switch (event_type) {
case SOF_IPC4_GATEWAY_UNDERRUN_DETECTED:
notif_idx = IPC4_UNDERRUN_AT_GATEWAY_NOTIFICATION_MASK_IDX;
break;
case SOF_IPC4_MIXER_UNDERRUN_DETECTED:
notif_idx = IPC4_UNDERRUN_AT_MIXER_NOTIFICATION_MASK_IDX;
break;
case SOF_IPC4_GATEWAY_OVERRUN_DETECTED:
notif_idx = IPC4_OVERRUN_AT_GATEWAY_NOTIFICATION_MASK_IDX;
break;
default:
return false;
}

return (notification_mask & BIT(notif_idx)) == 0;
}

static bool send_resource_notif(uint32_t resource_id, uint32_t event_type, uint32_t resource_type,
void *data, uint32_t data_size)
{
struct ipc_msg *msg;

if (is_notif_filtered_out(event_type))
return true; //silently ignore

msg = ipc_notification_pool_get(IPC4_RESOURCE_EVENT_SIZE);
if (!msg)
return false;

struct ipc4_resource_event_data_notification *notif = msg->tx_data;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume this cannot be NULL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, the payload is allocated along with the msg as a single rzalloc operation.

union ipc4_notification_header header;

header.r.notif_type = SOF_IPC4_NOTIFY_RESOURCE_EVENT;
header.r.type = SOF_IPC4_GLB_NOTIFICATION;
header.r.rsp = SOF_IPC4_MESSAGE_DIR_MSG_REQUEST;
header.r.msg_tgt = SOF_IPC4_MESSAGE_TARGET_FW_GEN_MSG;
msg->header = header.dat;
memset(&notif_data->event_data, 0, sizeof(notif_data->event_data));
}

void copier_gateway_underrun_notif_msg_init(struct ipc_msg *msg, uint32_t pipeline_id)
{
struct ipc4_resource_event_data_notification *notif_data = msg->tx_data;
notif->resource_id = resource_id;
notif->event_type = event_type;
notif->resource_type = resource_type;
memset(&notif->event_data, 0, sizeof(notif->event_data));
if (data && data_size)
memcpy_s(&notif->event_data, sizeof(notif->event_data), data, data_size);

resource_notif_header_init(msg);
notif_data->resource_id = pipeline_id;
notif_data->event_type = SOF_IPC4_GATEWAY_UNDERRUN_DETECTED;
notif_data->resource_type = SOF_IPC4_PIPELINE;
ipc_msg_send(msg, msg->tx_data, false);

return true;
}

void gateway_underrun_notif_msg_init(struct ipc_msg *msg, uint32_t resource_id)
void ipc4_update_notification_mask(uint32_t ntfy_mask, uint32_t enabled_mask)
{
struct ipc4_resource_event_data_notification *notif_data = msg->tx_data;

resource_notif_header_init(msg);
notif_data->resource_id = resource_id;
notif_data->event_type = SOF_IPC4_GATEWAY_UNDERRUN_DETECTED;
notif_data->resource_type = SOF_IPC4_GATEWAY;
notification_mask &= enabled_mask | (~ntfy_mask);
notification_mask |= enabled_mask & ntfy_mask;
}

void copier_gateway_overrun_notif_msg_init(struct ipc_msg *msg, uint32_t pipeline_id)
bool send_copier_gateway_underrun_notif_msg(uint32_t pipeline_id)
{
struct ipc4_resource_event_data_notification *notif_data = msg->tx_data;
return send_resource_notif(pipeline_id, SOF_IPC4_GATEWAY_UNDERRUN_DETECTED,
SOF_IPC4_PIPELINE, NULL, 0);
}

resource_notif_header_init(msg);
notif_data->resource_id = pipeline_id;
notif_data->event_type = SOF_IPC4_GATEWAY_OVERRUN_DETECTED;
notif_data->resource_type = SOF_IPC4_PIPELINE;
bool send_gateway_underrun_notif_msg(uint32_t resource_id)
{
return send_resource_notif(resource_id, SOF_IPC4_GATEWAY_UNDERRUN_DETECTED,
SOF_IPC4_GATEWAY, NULL, 0);
}

void gateway_overrun_notif_msg_init(struct ipc_msg *msg, uint32_t resource_id)
bool send_copier_gateway_overrun_notif_msg(uint32_t pipeline_id)
{
struct ipc4_resource_event_data_notification *notif_data = msg->tx_data;
return send_resource_notif(pipeline_id, SOF_IPC4_GATEWAY_OVERRUN_DETECTED,
SOF_IPC4_PIPELINE, NULL, 0);
}

resource_notif_header_init(msg);
notif_data->resource_id = resource_id;
notif_data->event_type = SOF_IPC4_GATEWAY_OVERRUN_DETECTED;
notif_data->resource_type = SOF_IPC4_GATEWAY;
bool send_gateway_overrun_notif_msg(uint32_t resource_id)
{
return send_resource_notif(resource_id, SOF_IPC4_GATEWAY_OVERRUN_DETECTED,
SOF_IPC4_GATEWAY, NULL, 0);
}

void mixer_underrun_notif_msg_init(struct ipc_msg *msg, uint32_t resource_id, uint32_t eos_flag,
uint32_t data_mixed, uint32_t expected_data_mixed)
void send_mixer_underrun_notif_msg(uint32_t resource_id, uint32_t eos_flag, uint32_t data_mixed,
uint32_t expected_data_mixed)
{
struct ipc4_resource_event_data_notification *notif_data = msg->tx_data;

resource_notif_header_init(msg);
notif_data->resource_id = resource_id;
notif_data->event_type = SOF_IPC4_MIXER_UNDERRUN_DETECTED;
notif_data->resource_type = SOF_IPC4_PIPELINE;
notif_data->event_data.mixer_underrun.eos_flag = eos_flag;
notif_data->event_data.mixer_underrun.data_mixed = data_mixed;
notif_data->event_data.mixer_underrun.expected_data_mixed = expected_data_mixed;
struct ipc4_mixer_underrun_event_data mixer_underrun_data;

mixer_underrun_data.eos_flag = eos_flag;
mixer_underrun_data.data_mixed = data_mixed;
mixer_underrun_data.expected_data_mixed = expected_data_mixed;

send_resource_notif(resource_id, SOF_IPC4_MIXER_UNDERRUN_DETECTED, SOF_IPC4_PIPELINE,
Copy link
Collaborator

Choose a reason for hiding this comment

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

how about we only export send_resource_notif() and move all this component-specific functions to respective components and potentially make them static there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are pros and cons. Function send_resource_notif takes numerous arguments. If we keep it static, we give the compile a chance to optimize them.

On the other hand, your idea would allow us to move the "if (cd->stream_direction)" condition into a single static function that determines the right arguments to send_resource_notif instead of having to choose the right function based on the condition.

Its a tough call and I didn't think about this. My focus was to allow efficient implantation of the notification mask without making the existing code too cumbersome.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also some middle ground approach is possible. For example we can merge send_gateway_underrun_notif_msg and send_gateway_overrun_notif_msg into a single function send_gateway_xrun_notif_msg but still keep it in the notification module.

This approach would eliminate the repetition of the following code in dai-zephyr.c and host-zephyr.c

	if (dev->direction == SOF_IPC_STREAM_PLAYBACK)
		notif_sent = send_copier_gateway_underrun_notif_msg(ppl_id);
	else
		notif_sent = send_copier_gateway_overrun_notif_msg(ppl_id);

&mixer_underrun_data, sizeof(mixer_underrun_data));
}
EXPORT_SYMBOL(mixer_underrun_notif_msg_init);
EXPORT_SYMBOL(send_mixer_underrun_notif_msg);

void process_data_error_notif_msg_init(struct ipc_msg *msg, uint32_t resource_id,
uint32_t error_code)
void send_process_data_error_notif_msg(uint32_t resource_id, uint32_t error_code)
{
struct ipc4_resource_event_data_notification *notif_data = msg->tx_data;
struct ipc4_process_data_error_event_data error_data;

error_data.error_code = error_code;

resource_notif_header_init(msg);
notif_data->resource_id = resource_id;
notif_data->event_type = SOF_IPC4_PROCESS_DATA_ERROR;
notif_data->resource_type = SOF_IPC4_MODULE_INSTANCE;
notif_data->event_data.process_data_error.error_code = error_code;
send_resource_notif(resource_id, SOF_IPC4_PROCESS_DATA_ERROR, SOF_IPC4_MODULE_INSTANCE,
&error_data, sizeof(error_data));
}
1 change: 0 additions & 1 deletion src/ipc/notification_pool.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,3 @@ struct ipc_msg *ipc_notification_pool_get(size_t size)
item->msg.tx_size = size;
return &item->msg;
}
EXPORT_SYMBOL(ipc_notification_pool_get);