Skip to content

ASoC: SOF: ipc4: Add support for generic bytes control to support notifications #5531

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

Merged
bardliao merged 8 commits into from
Oct 2, 2025
Merged

ASoC: SOF: ipc4: Add support for generic bytes control to support notifications #5531

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
0354583
ASoC: SOF: ipc4-control: If there is no data do not send bytes update
ujfalusi Sep 9, 2025
91e7ba4
ASoC: SOF: ipc4-topology: Correct the allocation size for bytes controls
ujfalusi Sep 9, 2025
49dccfd
ASoC: SOF: ipc4-control: Use the correct size for scontrol->ipc_contr…
ujfalusi Sep 9, 2025
bbbde78
ASoC: SOF: ipc4-control: Keep the payload size up to date
ujfalusi Sep 9, 2025
63231d5
ASoC: SOF: ipc4-topology: Set initial param_id for bytes control type
ujfalusi Sep 10, 2025
b055a0c
ASoC: SOF: ipc4: Support for sending payload along with LARGE_CONFIG_GET
ujfalusi Sep 11, 2025
0ddf3f0
ASoC: SOF: ipc4: Add definition for generic bytes control
ujfalusi Sep 11, 2025
1b03a2d
ASoC: SOF: ipc4-control: Add support for generic bytes control
ujfalusi Sep 11, 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
197 changes: 174 additions & 23 deletions sound/soc/sof/ipc4-control.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ static int sof_ipc4_set_get_kcontrol_data(struct snd_sof_control *scontrol,
* configuration
*/
memcpy(scontrol->ipc_control_data, scontrol->old_ipc_control_data,
scontrol->max_size);
scontrol->size);
Copy link
Copy Markdown
Collaborator

@kv2019i kv2019i Oct 1, 2025

Choose a reason for hiding this comment

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

Ok this took a while to parse, but I think this is correct. So without this patch, incorrect amount was copied as:

  • too little copied as scontrol->max_size doesn't cover the internal header struct
  • too much copied as scontrol->size may be smaller than max_size

Copy link
Copy Markdown
Collaborator Author

@ujfalusi ujfalusi Oct 1, 2025

Choose a reason for hiding this comment

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

yes, around those lines, we should copy what is available and yes, max_size is the payload's maximum, not the kernel internal buffer's size.
It is really confusing and took quite a while to unravel this for fixing.

Copy link
Copy Markdown
Collaborator Author

@ujfalusi ujfalusi Oct 1, 2025

Choose a reason for hiding this comment

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

fwiw, the max_size was always bigger than the real data+header, so it was working OK, but it was incorrect at the same time.

kfree(scontrol->old_ipc_control_data);
scontrol->old_ipc_control_data = NULL;
/* Send the last known good configuration to firmware */
Expand Down Expand Up @@ -284,6 +284,105 @@ static void sof_ipc4_refresh_generic_control(struct snd_sof_control *scontrol)
kfree(data);
}

static int
sof_ipc4_set_bytes_control_data(struct snd_sof_control *scontrol, bool lock)
{
struct sof_ipc4_control_data *cdata = scontrol->ipc_control_data;
struct snd_soc_component *scomp = scontrol->scomp;
struct sof_ipc4_control_msg_payload *msg_data;
struct sof_abi_hdr *data = cdata->data;
struct sof_ipc4_msg *msg = &cdata->msg;
size_t data_size;
int ret;

data_size = struct_size(msg_data, data, data->size);
msg_data = kzalloc(data_size, GFP_KERNEL);
if (!msg_data)
return -ENOMEM;

msg_data->id = cdata->index;
msg_data->num_elems = data->size;
memcpy(msg_data->data, data->data, data->size);

msg->extension = SOF_IPC4_MOD_EXT_MSG_PARAM_ID(data->type);

msg->data_ptr = msg_data;
msg->data_size = data_size;

ret = sof_ipc4_set_get_kcontrol_data(scontrol, true, lock);
msg->data_ptr = NULL;
msg->data_size = 0;
if (ret < 0)
dev_err(scomp->dev, "%s: Failed to set control update for %s\n",
__func__, scontrol->name);

kfree(msg_data);

return ret;
}

static int
sof_ipc4_refresh_bytes_control(struct snd_sof_control *scontrol, bool lock)
{
struct sof_ipc4_control_data *cdata = scontrol->ipc_control_data;
struct snd_soc_component *scomp = scontrol->scomp;
struct sof_ipc4_control_msg_payload *msg_data;
struct sof_abi_hdr *data = cdata->data;
struct sof_ipc4_msg *msg = &cdata->msg;
size_t data_size;
int ret = 0;

if (!scontrol->comp_data_dirty)
return 0;

if (!pm_runtime_active(scomp->dev))
return 0;

data_size = scontrol->max_size - sizeof(*data);
if (data_size < sizeof(*msg_data))
data_size = sizeof(*msg_data);

msg_data = kzalloc(data_size, GFP_KERNEL);
if (!msg_data)
return -ENOMEM;

msg->extension = SOF_IPC4_MOD_EXT_MSG_PARAM_ID(data->type);

msg_data->id = cdata->index;
msg_data->num_elems = 0; /* ignored for bytes */

msg->data_ptr = msg_data;
msg->data_size = data_size;

scontrol->comp_data_dirty = false;
ret = sof_ipc4_set_get_kcontrol_data(scontrol, false, lock);
if (!ret) {
if (msg->data_size > scontrol->max_size - sizeof(*data)) {
dev_err(scomp->dev,
"%s: no space for data in %s (%zu, %zu)\n",
__func__, scontrol->name, msg->data_size,
scontrol->max_size - sizeof(*data));
goto out;
}

data->size = msg->data_size;
scontrol->size = sizeof(*cdata) + sizeof(*data) + data->size;
memcpy(data->data, msg->data_ptr, data->size);
} else {
dev_err(scomp->dev, "Failed to read control data for %s\n",
scontrol->name);
scontrol->comp_data_dirty = true;
}

out:
msg->data_ptr = NULL;
msg->data_size = 0;

kfree(msg_data);

return ret;
}

static bool sof_ipc4_switch_put(struct snd_sof_control *scontrol,
struct snd_ctl_elem_value *ucontrol)
{
Expand Down Expand Up @@ -412,19 +511,42 @@ static int sof_ipc4_set_get_bytes_data(struct snd_sof_dev *sdev,
int ret = 0;

/* Send the new data to the firmware only if it is powered up */
if (set && !pm_runtime_active(sdev->dev))
return 0;
if (set) {
if (!pm_runtime_active(sdev->dev))
return 0;

if (!data->size) {
dev_dbg(sdev->dev, "%s: No data to be sent.\n",
scontrol->name);
return 0;
}
}

if (data->type == SOF_IPC4_BYTES_CONTROL_PARAM_ID) {
if (set)
return sof_ipc4_set_bytes_control_data(scontrol, lock);
else
return sof_ipc4_refresh_bytes_control(scontrol, lock);
}

msg->extension = SOF_IPC4_MOD_EXT_MSG_PARAM_ID(data->type);

msg->data_ptr = data->data;
msg->data_size = data->size;
if (set)
msg->data_size = data->size;
else
msg->data_size = scontrol->max_size - sizeof(*data);

ret = sof_ipc4_set_get_kcontrol_data(scontrol, set, lock);
if (ret < 0)
if (ret < 0) {
dev_err(sdev->dev, "Failed to %s for %s\n",
set ? "set bytes update" : "get bytes",
scontrol->name);
} else if (!set) {
/* Update the sizes according to the received payload data */
data->size = msg->data_size;
scontrol->size = sizeof(*cdata) + sizeof(*data) + data->size;
}

msg->data_ptr = NULL;
msg->data_size = 0;
Expand All @@ -440,6 +562,7 @@ static int sof_ipc4_bytes_put(struct snd_sof_control *scontrol,
struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
struct sof_abi_hdr *data = cdata->data;
size_t size;
int ret;

if (scontrol->max_size > sizeof(ucontrol->value.bytes.data)) {
dev_err_ratelimited(scomp->dev,
Expand All @@ -461,9 +584,12 @@ static int sof_ipc4_bytes_put(struct snd_sof_control *scontrol,
/* copy from kcontrol */
memcpy(data, ucontrol->value.bytes.data, size);

sof_ipc4_set_get_bytes_data(sdev, scontrol, true, true);
ret = sof_ipc4_set_get_bytes_data(sdev, scontrol, true, true);
if (!ret)
/* Update the cdata size */
scontrol->size = sizeof(*cdata) + size;

return 0;
return ret;
}

static int sof_ipc4_bytes_get(struct snd_sof_control *scontrol,
Expand All @@ -487,6 +613,8 @@ static int sof_ipc4_bytes_get(struct snd_sof_control *scontrol,
return -EINVAL;
}

sof_ipc4_refresh_bytes_control(scontrol, true);

size = data->size + sizeof(*data);

/* copy back to kcontrol */
Expand Down Expand Up @@ -559,20 +687,23 @@ static int sof_ipc4_bytes_ext_put(struct snd_sof_control *scontrol,
if (!scontrol->old_ipc_control_data) {
/* Create a backup of the current, valid bytes control */
scontrol->old_ipc_control_data = kmemdup(scontrol->ipc_control_data,
scontrol->max_size, GFP_KERNEL);
scontrol->size, GFP_KERNEL);
if (!scontrol->old_ipc_control_data)
return -ENOMEM;
}

/* Copy the whole binary data which includes the ABI header and the payload */
if (copy_from_user(data, tlvd->tlv, header.length)) {
memcpy(scontrol->ipc_control_data, scontrol->old_ipc_control_data,
scontrol->max_size);
scontrol->size);
kfree(scontrol->old_ipc_control_data);
scontrol->old_ipc_control_data = NULL;
return -EFAULT;
}

/* Update the cdata size */
scontrol->size = sizeof(*cdata) + header.length;

return sof_ipc4_set_get_bytes_data(sdev, scontrol, true, true);
}

Expand Down Expand Up @@ -638,6 +769,8 @@ static int sof_ipc4_bytes_ext_get(struct snd_sof_control *scontrol,
const unsigned int __user *binary_data,
unsigned int size)
{
sof_ipc4_refresh_bytes_control(scontrol, true);

return _sof_ipc4_bytes_ext_get(scontrol, binary_data, size, false);
}

Expand Down Expand Up @@ -691,6 +824,9 @@ static void sof_ipc4_control_update(struct snd_sof_dev *sdev, void *ipc_message)
case SOF_IPC4_ENUM_CONTROL_PARAM_ID:
type = SND_SOC_TPLG_TYPE_ENUM;
break;
case SOF_IPC4_BYTES_CONTROL_PARAM_ID:
type = SND_SOC_TPLG_TYPE_BYTES;
break;
default:
dev_err(sdev->dev,
"%s: Invalid control type for module %u.%u: %u\n",
Expand Down Expand Up @@ -741,23 +877,38 @@ static void sof_ipc4_control_update(struct snd_sof_dev *sdev, void *ipc_message)
* The message includes the updated value/data, update the
* control's local cache using the received notification
*/
for (i = 0; i < msg_data->num_elems; i++) {
u32 channel = msg_data->chanv[i].channel;
if (type == SND_SOC_TPLG_TYPE_BYTES) {
struct sof_abi_hdr *data = cdata->data;

if (channel >= scontrol->num_channels) {
if (msg_data->num_elems > scontrol->max_size - sizeof(*data)) {
dev_warn(sdev->dev,
"Invalid channel index for %s: %u\n",
scontrol->name, i);

/*
* Mark the scontrol as dirty to force a refresh
* on next read
*/
scontrol->comp_data_dirty = true;
break;
"%s: no space for data in %s (%u, %zu)\n",
__func__, scontrol->name, msg_data->num_elems,
scontrol->max_size - sizeof(*data));
} else {
memcpy(data->data, msg_data->data, msg_data->num_elems);
data->size = msg_data->num_elems;
scontrol->size = sizeof(*cdata) + sizeof(*data) + data->size;
}
} else {
for (i = 0; i < msg_data->num_elems; i++) {
u32 channel = msg_data->chanv[i].channel;

if (channel >= scontrol->num_channels) {
dev_warn(sdev->dev,
"Invalid channel index for %s: %u\n",
scontrol->name, i);

/*
* Mark the scontrol as dirty to force a refresh
* on next read
*/
scontrol->comp_data_dirty = true;
break;
}

cdata->chanv[channel].value = msg_data->chanv[i].value;
}

cdata->chanv[channel].value = msg_data->chanv[i].value;
}
} else {
/*
Expand Down
36 changes: 28 additions & 8 deletions sound/soc/sof/ipc4-topology.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2875,22 +2875,41 @@ static int sof_ipc4_control_load_bytes(struct snd_sof_dev *sdev, struct snd_sof_
struct sof_ipc4_msg *msg;
int ret;

if (scontrol->max_size < (sizeof(*control_data) + sizeof(struct sof_abi_hdr))) {
dev_err(sdev->dev, "insufficient size for a bytes control %s: %zu.\n",
/*
* The max_size is coming from topology and indicates the maximum size
* of sof_abi_hdr plus the payload, which excludes the local only
* 'struct sof_ipc4_control_data'
*/
if (scontrol->max_size < sizeof(struct sof_abi_hdr)) {
dev_err(sdev->dev,
"insufficient maximum size for a bytes control %s: %zu.\n",
scontrol->name, scontrol->max_size);
return -EINVAL;
}

if (scontrol->priv_size > scontrol->max_size - sizeof(*control_data)) {
dev_err(sdev->dev, "scontrol %s bytes data size %zu exceeds max %zu.\n",
scontrol->name, scontrol->priv_size,
scontrol->max_size - sizeof(*control_data));
if (scontrol->priv_size > scontrol->max_size) {
dev_err(sdev->dev,
"bytes control %s initial data size %zu exceeds max %zu.\n",
scontrol->name, scontrol->priv_size, scontrol->max_size);
return -EINVAL;
}

if (scontrol->priv_size < sizeof(struct sof_abi_hdr)) {
dev_err(sdev->dev,
"bytes control %s initial data size %zu is insufficient.\n",
scontrol->name, scontrol->priv_size);
return -EINVAL;
}

scontrol->size = sizeof(struct sof_ipc4_control_data) + scontrol->priv_size;
/*
* The used size behind the cdata pointer, which can be smaller than
* the maximum size
*/
scontrol->size = sizeof(*control_data) + scontrol->priv_size;

scontrol->ipc_control_data = kzalloc(scontrol->max_size, GFP_KERNEL);
/* Allocate the cdata: local struct size + maximum payload size */
scontrol->ipc_control_data = kzalloc(sizeof(*control_data) + scontrol->max_size,
GFP_KERNEL);
if (!scontrol->ipc_control_data)
return -ENOMEM;

Expand Down Expand Up @@ -2925,6 +2944,7 @@ static int sof_ipc4_control_load_bytes(struct snd_sof_dev *sdev, struct snd_sof_
msg->primary = SOF_IPC4_MSG_TYPE_SET(SOF_IPC4_MOD_LARGE_CONFIG_SET);
msg->primary |= SOF_IPC4_MSG_DIR(SOF_IPC4_MSG_REQUEST);
msg->primary |= SOF_IPC4_MSG_TARGET(SOF_IPC4_MODULE_MSG);
msg->extension = SOF_IPC4_MOD_EXT_MSG_PARAM_ID(control_data->data->type);

return 0;

Expand Down
9 changes: 7 additions & 2 deletions sound/soc/sof/ipc4-topology.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,19 +365,24 @@ struct sof_ipc4_control_data {

#define SOF_IPC4_SWITCH_CONTROL_PARAM_ID 200
#define SOF_IPC4_ENUM_CONTROL_PARAM_ID 201
#define SOF_IPC4_BYTES_CONTROL_PARAM_ID 202

/**
* struct sof_ipc4_control_msg_payload - IPC payload for kcontrol parameters
* @id: unique id of the control
* @num_elems: Number of elements in the chanv array
* @num_elems: Number of elements in the chanv array or number of bytes in data
* @reserved: reserved for future use, must be set to 0
* @chanv: channel ID and value array
* @data: binary payload
*/
struct sof_ipc4_control_msg_payload {
uint16_t id;
uint16_t num_elems;
uint32_t reserved[4];
DECLARE_FLEX_ARRAY(struct sof_ipc4_ctrl_value_chan, chanv);
union {
DECLARE_FLEX_ARRAY(struct sof_ipc4_ctrl_value_chan, chanv);
DECLARE_FLEX_ARRAY(uint8_t, data);
};
} __packed;

/**
Expand Down
Loading
Loading