From dd49c0e77015fa8cb141defdf482673c915ee6ce Mon Sep 17 00:00:00 2001 From: Omair Mohammed Abdullah Date: Mon, 21 Nov 2011 15:47:03 +0530 Subject: [PATCH] [REBASED on main r3(stable)from r2-stable] audio: sst: make sst driver mad workqueue single threaded BZ: 17975 Driver IPC handling had various issues Makes the sst driver mad workqueue single threaded. Multiple workqueue tasks in a single threaded workqueue will not run in parallel. FW state should be set as soon as possible while freeing stream so that later messages are ignored. Drop IPC uses ctrl_blk not data_blk, so setting the ctrl_blk to on. Adds locking to checking and changing of stream states in free_stream and drop_stream to prevent a race condition. Added missing block condition to firmware context save. Change-Id: I117e413b593094a629bb3e29ab9aa75c4f4fe233 old-Change-Id: Ia4e4dbd33b32882ada369ae5a9ce9cce5f6900f6 Signed-off-by: Omair Mohammed Abdullah Reviewed-on: http://android.intel.com:8080/29524 Reviewed-by: M, Arulselvan Tested-by: M, Arulselvan Reviewed-by: buildbot Tested-by: buildbot --- sound/pci/sst/intel_sst.c | 6 ++-- sound/pci/sst/intel_sst_ipc.c | 19 ++++++------ sound/pci/sst/intel_sst_stream.c | 62 +++++++++++++++++----------------------- 3 files changed, 40 insertions(+), 47 deletions(-) diff --git a/sound/pci/sst/intel_sst.c b/sound/pci/sst/intel_sst.c index 7792c78..f459be9 100644 --- a/sound/pci/sst/intel_sst.c +++ b/sound/pci/sst/intel_sst.c @@ -142,7 +142,7 @@ static irqreturn_t intel_sst_interrupt(int irq, void *context) queue_work(sst_drv_ctx->process_reply_wq, &sst_drv_ctx->ipc_process_reply.wq); } - /* mask busy inetrrupt */ + /* mask busy interrupt */ imr.full = sst_shim_read(drv->shim, SST_IMRX); imr.part.busy_interrupt = 1; sst_shim_write(sst_drv_ctx->shim, SST_IMRX, imr.full); @@ -218,7 +218,7 @@ static int __devinit intel_sst_probe(struct pci_dev *pci, INIT_WORK(&sst_drv_ctx->mad_ops.wq, sst_process_mad_ops); init_waitqueue_head(&sst_drv_ctx->wait_queue); - sst_drv_ctx->mad_wq = create_workqueue("sst_mad_wq"); + sst_drv_ctx->mad_wq = create_singlethread_workqueue("sst_mad_wq"); if (!sst_drv_ctx->mad_wq) goto do_free_drv_ctx; sst_drv_ctx->post_msg_wq = create_workqueue("sst_post_msg_wq"); @@ -464,6 +464,8 @@ static void sst_save_dsp_context(void) return; pvt_id = sst_assign_pvt_id(sst_drv_ctx); sst_drv_ctx->alloc_block[0].sst_id = pvt_id; + sst_drv_ctx->alloc_block[0].ops_block.condition = false; + sst_drv_ctx->alloc_block[0].ops_block.on = true; sst_fill_header(&msg->header, IPC_IA_GET_FW_CTXT, 1, pvt_id); msg->header.part.data = sizeof(fw_context) + sizeof(u32); fw_context.address = virt_to_phys((void *)sst_drv_ctx->fw_cntx); diff --git a/sound/pci/sst/intel_sst_ipc.c b/sound/pci/sst/intel_sst_ipc.c index 085d7ee..f42b385 100644 --- a/sound/pci/sst/intel_sst_ipc.c +++ b/sound/pci/sst/intel_sst_ipc.c @@ -310,10 +310,11 @@ void sst_process_reply(struct work_struct *work) { struct sst_ipc_msg_wq *msg = container_of(work, struct sst_ipc_msg_wq, wq); - int str_id = msg->header.part.str_id; struct stream_info *str_info; + pr_debug("sst: IPC process reply for %x\n", msg->header.full); + switch (msg->header.part.msg_id) { case IPC_IA_TARGET_DEV_SELECT: if (!msg->header.part.data) { @@ -574,17 +575,15 @@ void sst_process_reply(struct work_struct *work) pr_debug("Drop ret bytes %x\n", drop_resp->bytes); str_info->curr_bytes = drop_resp->bytes; - str_info->ctrl_blk.ret_code = 0; + if (!drop_resp->result) + pr_debug("drop sucess for %d\n", str_id); + else + pr_err("drop for %d failed with err %d\n", + str_id, drop_resp->result); } else { - pr_err(" Msg %x reply error %x\n", - msg->header.part.msg_id, msg->header.part.data); - str_info->ctrl_blk.ret_code = -msg->header.part.data; - } - if (str_info->ctrl_blk.on == true) { - str_info->ctrl_blk.on = false; - str_info->ctrl_blk.condition = true; - wake_up(&sst_drv_ctx->wait_queue); + pr_err("fw sent small IPC for drop response!!\n"); } + break; case IPC_IA_ENABLE_RX_TIME_SLOT: if (!msg->header.part.data) { diff --git a/sound/pci/sst/intel_sst_stream.c b/sound/pci/sst/intel_sst_stream.c index fc81861..a8fbc57 100644 --- a/sound/pci/sst/intel_sst_stream.c +++ b/sound/pci/sst/intel_sst_stream.c @@ -349,7 +349,7 @@ int sst_pause_stream(int str_id) } } else { retval = -EBADRQC; - pr_err("BADQRC for stream\n"); + pr_debug("SST DBG:BADRQC for stream\n "); } return retval; @@ -436,53 +436,39 @@ int sst_drop_stream(int str_id) return retval; str_info = &sst_drv_ctx->streams[str_id]; + mutex_lock(&str_info->lock); if (str_info->status != STREAM_UN_INIT && str_info->status != STREAM_DECODE) { - if (str_info->ctrl_blk.on == true) { - pr_err("SST ERR: control path in use\n"); - return -EINVAL; - } + str_info->prev = STREAM_UN_INIT; + str_info->status = STREAM_INIT; + mutex_unlock(&str_info->lock); if (sst_create_short_msg(&msg)) { pr_err("SST ERR: mem allocation failed\n"); return -ENOMEM; } sst_fill_header(&msg->header, IPC_IA_DROP_STREAM, 0, str_id); - str_info->ctrl_blk.condition = false; - str_info->ctrl_blk.ret_code = 0; - str_info->ctrl_blk.on = true; spin_lock(&sst_drv_ctx->list_spin_lock); list_add_tail(&msg->node, &sst_drv_ctx->ipc_dispatch_list); spin_unlock(&sst_drv_ctx->list_spin_lock); sst_post_message(&sst_drv_ctx->ipc_post_msg_wq); - retval = sst_wait_interruptible_timeout(sst_drv_ctx, - &str_info->ctrl_blk, SST_BLOCK_TIMEOUT); - if (!retval) { - pr_debug("SST DBG:drop success\n"); - str_info->prev = STREAM_UN_INIT; - str_info->status = STREAM_INIT; - if (str_info->src != MAD_DRV) { - mutex_lock(&str_info->lock); - list_for_each_entry_safe(bufs, _bufs, - &str_info->bufs, node) { - list_del(&bufs->node); - kfree(bufs); - } - mutex_unlock(&str_info->lock); + if (str_info->src != MAD_DRV) { + mutex_lock(&str_info->lock); + list_for_each_entry_safe(bufs, _bufs, + &str_info->bufs, node) { + list_del(&bufs->node); + kfree(bufs); } - str_info->cumm_bytes += str_info->curr_bytes; - } else if (retval == -SST_ERR_INVALID_STREAM_ID) { - retval = -EINVAL; - mutex_lock(&sst_drv_ctx->stream_lock); - sst_clean_stream(str_info); - mutex_unlock(&sst_drv_ctx->stream_lock); + mutex_unlock(&str_info->lock); } + str_info->cumm_bytes += str_info->curr_bytes; if (str_info->data_blk.on == true) { str_info->data_blk.condition = true; str_info->data_blk.ret_code = retval; wake_up(&sst_drv_ctx->wait_queue); } } else { + mutex_unlock(&str_info->lock); retval = -EBADRQC; pr_debug("BADQRC for stream, state %x\n", str_info->status); } @@ -555,7 +541,15 @@ int sst_free_stream(int str_id) return retval; str_info = &sst_drv_ctx->streams[str_id]; + mutex_lock(&str_info->lock); if (str_info->status != STREAM_UN_INIT) { + str_info->prev = str_info->status; + str_info->status = STREAM_UN_INIT; + mutex_unlock(&str_info->lock); + if (str_info->ctrl_blk.on == true) { + pr_err("SST ERR: control path in use\n"); + return -EINVAL; + } if (sst_create_short_msg(&msg)) { pr_err("SST ERR: mem allocation failed\n"); return -ENOMEM; @@ -564,25 +558,23 @@ int sst_free_stream(int str_id) spin_lock(&sst_drv_ctx->list_spin_lock); list_add_tail(&msg->node, &sst_drv_ctx->ipc_dispatch_list); spin_unlock(&sst_drv_ctx->list_spin_lock); - sst_post_message(&sst_drv_ctx->ipc_post_msg_wq); - str_info->prev = str_info->status; - str_info->status = STREAM_UN_INIT; if (str_info->data_blk.on == true) { str_info->data_blk.condition = true; str_info->data_blk.ret_code = 0; wake_up(&sst_drv_ctx->wait_queue); } - str_info->data_blk.on = true; - str_info->data_blk.condition = false; + str_info->ctrl_blk.on = true; + str_info->ctrl_blk.condition = false; + sst_post_message(&sst_drv_ctx->ipc_post_msg_wq); retval = sst_wait_interruptible_timeout(sst_drv_ctx, &str_info->ctrl_blk, SST_BLOCK_TIMEOUT); - pr_debug("wait for free returned %d\n", retval); - msleep(100); + pr_debug("sst: wait for free returned %d\n", retval); mutex_lock(&sst_drv_ctx->stream_lock); sst_clean_stream(str_info); mutex_unlock(&sst_drv_ctx->stream_lock); pr_debug("SST DBG:Stream freed\n"); } else { + mutex_unlock(&str_info->lock); retval = -EBADRQC; pr_debug("SST DBG:BADQRC for stream\n"); } -- 2.7.4