Skip to content

chain_dma: remove bogus use of a spin-lock #10129

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

Closed
lyakh wants to merge 2 commits into from
Closed

chain_dma: remove bogus use of a spin-lock #10129

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4d5c687
audio: chain_dma: (cosmetic) remove function names from logs
lyakh Jul 15, 2025
c4aea0b
audio: chain_dma: don't abuse driver list lock
lyakh Jul 15, 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
72 changes: 29 additions & 43 deletions src/audio/chain_dma.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ static int chain_link_start(struct comp_dev *dev)
if (err < 0)
return err;

comp_info(dev, "chain_link_start(): dma_start() link chan_index = %u",
comp_info(dev, "dma_start() link chan_index = %u",
cd->chan_link->index);
return 0;
}
Expand All @@ -112,7 +112,7 @@ static int chain_link_stop(struct comp_dev *dev)
if (err < 0)
return err;

comp_info(dev, "chain_link_stop(): dma_stop() link chan_index = %u",
comp_info(dev, "dma_stop() link chan_index = %u",
cd->chan_link->index);

return 0;
Expand All @@ -127,7 +127,7 @@ static int chain_host_stop(struct comp_dev *dev)
if (err < 0)
return err;

comp_info(dev, "chain_host_stop(): dma_stop() host chan_index = %u",
comp_info(dev, "dma_stop() host chan_index = %u",
cd->chan_host->index);

return 0;
Expand Down Expand Up @@ -187,11 +187,11 @@ static enum task_state chain_task_run(void *data)
case 0:
break;
case -EPIPE:
tr_warn(&chain_dma_tr, "chain_task_run(): dma_get_status() link xrun occurred,"
tr_warn(&chain_dma_tr, "dma_get_status() link xrun occurred,"
" ret = %d", ret);
break;
default:
tr_err(&chain_dma_tr, "chain_task_run(): dma_get_status() error, ret = %d", ret);
tr_err(&chain_dma_tr, "dma_get_status() error, ret = %d", ret);
return SOF_TASK_STATE_COMPLETED;
}

Expand All @@ -202,7 +202,7 @@ static enum task_state chain_task_run(void *data)
/* Host DMA does not report xruns. All error values will be treated as critical. */
ret = chain_get_dma_status(cd, cd->chan_host, &stat);
if (ret < 0) {
tr_err(&chain_dma_tr, "chain_task_run(): dma_get_status() error, ret = %d", ret);
tr_err(&chain_dma_tr, "dma_get_status() error, ret = %d", ret);
return SOF_TASK_STATE_COMPLETED;
}

Expand All @@ -221,14 +221,14 @@ static enum task_state chain_task_run(void *data)
ret = dma_reload(cd->chan_host->dma->z_dev, cd->chan_host->index, 0, 0, increment);
if (ret < 0) {
tr_err(&chain_dma_tr,
"chain_task_run(): dma_reload() host error, ret = %d", ret);
"dma_reload() host error, ret = %d", ret);
return SOF_TASK_STATE_COMPLETED;
}

ret = dma_reload(cd->chan_link->dma->z_dev, cd->chan_link->index, 0, 0, increment);
if (ret < 0) {
tr_err(&chain_dma_tr,
"chain_task_run(): dma_reload() link error, ret = %d", ret);
"dma_reload() link error, ret = %d", ret);
return SOF_TASK_STATE_COMPLETED;
}
} else {
Expand All @@ -246,8 +246,7 @@ static enum task_state chain_task_run(void *data)
half_buff_size);
if (ret < 0) {
tr_err(&chain_dma_tr,
"chain_task_run(): dma_reload() link error, ret = %d",
ret);
"dma_reload() link error, ret = %d", ret);
return SOF_TASK_STATE_COMPLETED;
}
cd->first_data_received = true;
Expand All @@ -262,7 +261,7 @@ static enum task_state chain_task_run(void *data)
0, 0, transferred);
if (ret < 0) {
tr_err(&chain_dma_tr,
"chain_task_run(): dma_reload() host error, ret = %d", ret);
"dma_reload() host error, ret = %d", ret);
return SOF_TASK_STATE_COMPLETED;
}

Expand All @@ -271,8 +270,8 @@ static enum task_state chain_task_run(void *data)
ret = dma_reload(cd->chan_link->dma->z_dev, cd->chan_link->index,
0, 0, half_buff_size);
if (ret < 0) {
tr_err(&chain_dma_tr, "chain_task_run(): dma_reload() "
"link error, ret = %d", ret);
tr_err(&chain_dma_tr,
"dma_reload() link error, ret = %d", ret);
return SOF_TASK_STATE_COMPLETED;
}
}
Expand All @@ -283,17 +282,13 @@ static enum task_state chain_task_run(void *data)

static int chain_task_start(struct comp_dev *dev)
{
struct comp_driver_list *drivers = comp_drivers_get();
struct chain_dma_data *cd = comp_get_drvdata(dev);
k_spinlock_key_t key;
int ret;

comp_info(dev, "chain_task_start(), host_dma_id = 0x%08x", cd->host_connector_node_id.dw);
comp_info(dev, "host_dma_id = 0x%08x", cd->host_connector_node_id.dw);

key = k_spin_lock(&drivers->lock);
switch (cd->chain_task.state) {
case SOF_TASK_STATE_QUEUED:
k_spin_unlock(&drivers->lock, key);
return 0;
case SOF_TASK_STATE_COMPLETED:
break;
Expand All @@ -302,70 +297,64 @@ static int chain_task_start(struct comp_dev *dev)
case SOF_TASK_STATE_FREE:
break;
default:
comp_err(dev, "chain_task_start(), bad state transition");
ret = -EINVAL;
goto error;
comp_err(dev, "bad state transition");
return -EINVAL;
}

if (cd->stream_direction == SOF_IPC_STREAM_PLAYBACK) {
ret = chain_host_start(dev);
if (ret)
goto error;
return ret;
ret = chain_link_start(dev);
if (ret) {
chain_host_stop(dev);
goto error;
return ret;
}
} else {
ret = chain_link_start(dev);
if (ret)
goto error;
return ret;
ret = chain_host_start(dev);
if (ret) {
chain_link_stop(dev);
goto error;
return ret;
}
}

ret = schedule_task_init_ll(&cd->chain_task, SOF_UUID(chain_dma_uuid),
SOF_SCHEDULE_LL_TIMER, SOF_TASK_PRI_HIGH,
chain_task_run, cd, 0, 0);
if (ret < 0) {
comp_err(dev, "chain_task_start(), ll task initialization failed");
comp_err(dev, "ll task initialization failed");
goto error_task;
}

ret = schedule_task(&cd->chain_task, 0, 0);
if (ret < 0) {
comp_err(dev, "chain_task_start(), ll schedule task failed");
comp_err(dev, "ll schedule task failed");
schedule_task_free(&cd->chain_task);
goto error_task;
}

pm_policy_state_lock_get(PM_STATE_RUNTIME_IDLE, PM_ALL_SUBSTATES);
k_spin_unlock(&drivers->lock, key);

return 0;

error_task:
chain_host_stop(dev);
chain_link_stop(dev);
error:
k_spin_unlock(&drivers->lock, key);

return ret;
}

static int chain_task_pause(struct comp_dev *dev)
{
struct comp_driver_list *drivers = comp_drivers_get();
struct chain_dma_data *cd = comp_get_drvdata(dev);
k_spinlock_key_t key;
int ret, ret2;

if (cd->chain_task.state == SOF_TASK_STATE_FREE)
return 0;

key = k_spin_lock(&drivers->lock);
cd->first_data_received = false;
if (cd->stream_direction == SOF_IPC_STREAM_PLAYBACK) {
ret = chain_host_stop(dev);
Expand All @@ -377,8 +366,6 @@ static int chain_task_pause(struct comp_dev *dev)
if (!ret)
ret = ret2;

k_spin_unlock(&drivers->lock, key);

schedule_task_free(&cd->chain_task);
pm_policy_state_lock_put(PM_STATE_RUNTIME_IDLE, PM_ALL_SUBSTATES);

Expand Down Expand Up @@ -491,23 +478,23 @@ __cold static int chain_init(struct comp_dev *dev, void *addr, size_t length)

err = dma_config(cd->dma_host->z_dev, cd->chan_host->index, dma_cfg_host);
if (err < 0) {
comp_err(dev, "chain_init(): dma_config() failed");
comp_err(dev, "dma_config() failed");
goto error_host;
}

/* get link DMA channel */
channel = cd->link_connector_node_id.f.v_index;
channel = dma_request_channel(cd->dma_link->z_dev, &channel);
if (channel < 0) {
comp_err(dev, "chain_init(): dma_request_channel() failed");
comp_err(dev, "dma_request_channel() failed");
goto error_host;
}

cd->chan_link = &cd->dma_link->chan[channel];

err = dma_config(cd->dma_link->z_dev, cd->chan_link->index, dma_cfg_link);
if (err < 0) {
comp_err(dev, "chain_init(): dma_config() failed");
comp_err(dev, "dma_config() failed");
goto error_link;
}
return 0;
Expand Down Expand Up @@ -559,7 +546,7 @@ __cold static int chain_task_init(struct comp_dev *dev, uint8_t host_dma_id, uin

cd->dma_host = sof_dma_get(dir, 0, SOF_DMA_DEV_HOST, SOF_DMA_ACCESS_SHARED);
if (!cd->dma_host) {
comp_err(dev, "chain_task_init(): dma_get() returned NULL");
comp_err(dev, "dma_get() returned NULL");
return -EINVAL;
}

Expand All @@ -569,7 +556,7 @@ __cold static int chain_task_init(struct comp_dev *dev, uint8_t host_dma_id, uin
cd->dma_link = sof_dma_get(dir, SOF_DMA_CAP_HDA, SOF_DMA_DEV_HDA, SOF_DMA_ACCESS_SHARED);
if (!cd->dma_link) {
sof_dma_put(cd->dma_host);
comp_err(dev, "chain_task_init(): dma_get() returned NULL");
comp_err(dev, "dma_get() returned NULL");
return -EINVAL;
}

Expand All @@ -578,8 +565,7 @@ __cold static int chain_task_init(struct comp_dev *dev, uint8_t host_dma_id, uin
&addr_align);
if (ret < 0) {
comp_err(dev,
"chain_task_init(): could not get dma buffer address alignment, err = %d",
ret);
"could not get dma buffer address alignment, err = %d", ret);
goto error;
}

Expand Down Expand Up @@ -609,7 +595,7 @@ __cold static int chain_task_init(struct comp_dev *dev, uint8_t host_dma_id, uin
addr_align, false);

if (!cd->dma_buffer) {
comp_err(dev, "chain_task_init(): failed to alloc dma buffer");
comp_err(dev, "failed to alloc dma buffer");
ret = -EINVAL;
goto error;
}
Expand Down
Loading