[alsa-devel] [PATCH 0/3] ASoC: Intel: add runtime power management support
This series fixes the device opne pm handlinga dn adds the runtime pm support. Also we fix lock usage while changing teh driver state
Mythri P K (1): Audio: SST: use lock when changing SST state.
Subhransu S. Prusty (1): ASoC: Intel: mrfld: Fix runtime pm calls in sst_open_pcm_stream
Vinod Koul (1): ASoC: Intel: sst: add runtime power management handling
sound/soc/intel/sst/sst.c | 71 ++++++++++++++++++++++++++++++- sound/soc/intel/sst/sst_drv_interface.c | 12 ++---- sound/soc/intel/sst/sst_ipc.c | 2 +- 3 files changed, 75 insertions(+), 10 deletions(-)
From: Subhransu S. Prusty subhransu.s.prusty@intel.com
It's already done in open/close.
Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com --- sound/soc/intel/sst/sst_drv_interface.c | 12 ++++-------- 1 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/sound/soc/intel/sst/sst_drv_interface.c b/sound/soc/intel/sst/sst_drv_interface.c index 183b1eb..4187057 100644 --- a/sound/soc/intel/sst/sst_drv_interface.c +++ b/sound/soc/intel/sst/sst_drv_interface.c @@ -163,16 +163,11 @@ static int sst_open_pcm_stream(struct device *dev, if (!str_param) return -EINVAL;
- retval = pm_runtime_get_sync(ctx->dev); - if (retval < 0) - return retval; retval = sst_get_stream(ctx, str_param); - if (retval > 0) { + if (retval > 0) ctx->stream_cnt++; - } else { + else dev_err(ctx->dev, "sst_get_stream returned err %d\n", retval); - sst_pm_runtime_put(ctx); - }
return retval; } @@ -212,7 +207,8 @@ put: stream->period_elapsed = NULL; ctx->stream_cnt--;
- sst_pm_runtime_put(ctx); + if (retval) + dev_err(ctx->dev, "free stream returned err %d\n", retval);
dev_dbg(ctx->dev, "Exit\n"); return 0;
On Thu, Oct 30, 2014 at 10:38:54AM +0530, Vinod Koul wrote:
From: Subhransu S. Prusty subhransu.s.prusty@intel.com
It's already done in open/close.
Applied, thanks.
This patch adds the runtime pm handlers, the driver already has code for get/put for runtiem pm and only these handlers being missing.
Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com --- sound/soc/intel/sst/sst.c | 73 ++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 72 insertions(+), 1 deletions(-)
diff --git a/sound/soc/intel/sst/sst.c b/sound/soc/intel/sst/sst.c index fa34217..8eb7148 100644 --- a/sound/soc/intel/sst/sst.c +++ b/sound/soc/intel/sst/sst.c @@ -152,6 +152,23 @@ static irqreturn_t intel_sst_irq_thread_mrfld(int irq, void *context) return IRQ_HANDLED; }
+static int sst_save_dsp_context_v2(struct intel_sst_drv *sst) +{ + int ret = 0; + + ret = sst_prepare_and_post_msg(sst, SST_TASK_ID_MEDIA, IPC_CMD, + IPC_PREP_D3, PIPE_RSVD, 0, NULL, NULL, + true, true, false, true); + + if (ret < 0) { + dev_err(sst->dev, "not suspending FW!!!"); + return -EIO; + } + + return 0; +} + + static struct intel_sst_ops mrfld_ops = { .interrupt = intel_sst_interrupt_mrfld, .irq_thread = intel_sst_irq_thread_mrfld, @@ -160,6 +177,7 @@ static struct intel_sst_ops mrfld_ops = { .reset = intel_sst_reset_dsp_mrfld, .post_message = sst_post_message_mrfld, .process_reply = sst_process_reply_mrfld, + .save_dsp_context = sst_save_dsp_context_v2, .alloc_stream = sst_alloc_stream_mrfld, .post_download = sst_post_download_mrfld, }; @@ -418,7 +436,55 @@ static void intel_sst_remove(struct pci_dev *pci) pci_set_drvdata(pci, NULL); }
-/* PCI Routines */ +static int intel_sst_runtime_suspend(struct device *dev) +{ + int ret = 0; + struct intel_sst_drv *ctx = dev_get_drvdata(dev); + + dev_info(dev, "runtime_suspend called\n"); + if (ctx->sst_state == SST_RESET) { + dev_dbg(dev, "LPE is already in RESET state, No action"); + return 0; + } + /*save fw context*/ + if (ctx->ops->save_dsp_context(ctx)) + return -EBUSY; + + /* Move the SST state to Reset */ + sst_set_fw_state_locked(ctx, SST_RESET); + + synchronize_irq(ctx->irq_num); + flush_workqueue(ctx->post_msg_wq); + + return ret; +} + +static int intel_sst_runtime_resume(struct device *dev) +{ + int ret = 0; + struct intel_sst_drv *ctx = dev_get_drvdata(dev); + + dev_info(dev, "runtime_resume called\n"); + + mutex_lock(&ctx->sst_lock); + if (ctx->sst_state == SST_RESET) { + dev_dbg(dev, "DSP Downloading FW now...\n"); + ret = sst_load_fw(ctx); + if (ret) { + dev_err(dev, "FW download fail %x\n", ret); + ctx->sst_state = SST_RESET; + } + } + mutex_unlock(&ctx->sst_lock); + return ret; +} + +static const struct dev_pm_ops intel_sst_pm = { + .runtime_suspend = intel_sst_runtime_suspend, + .runtime_resume = intel_sst_runtime_resume, +}; + +/*PCI Routines*/ static struct pci_device_id intel_sst_ids[] = { { PCI_VDEVICE(INTEL, SST_MRFLD_PCI_ID), 0}, { 0, } @@ -429,6 +495,11 @@ static struct pci_driver sst_driver = { .id_table = intel_sst_ids, .probe = intel_sst_probe, .remove = intel_sst_remove, +#ifdef CONFIG_PM + .driver = { + .pm = &intel_sst_pm, + }, +#endif };
module_pci_driver(sst_driver);
On Thu, Oct 30, 2014 at 10:38:55AM +0530, Vinod Koul wrote:
- ret = sst_prepare_and_post_msg(sst, SST_TASK_ID_MEDIA, IPC_CMD,
IPC_PREP_D3, PIPE_RSVD, 0, NULL, NULL,true, true, false, true);- if (ret < 0) {
dev_err(sst->dev, "not suspending FW!!!");return -EIO;
Better to print and pass back the actual error code we got.
- dev_info(dev, "runtime_suspend called\n");
- if (ctx->sst_state == SST_RESET) {
Drop this down to a dev_dbg() or remove it please (and some others later on).
dev_dbg(dev, "LPE is already in RESET state, No action");
Missing \n.
return 0;- }
- /*save fw context*/
/* Upstream comment style please */
On Thu, Oct 30, 2014 at 06:06:09PM +0000, Mark Brown wrote:
On Thu, Oct 30, 2014 at 10:38:55AM +0530, Vinod Koul wrote:
- ret = sst_prepare_and_post_msg(sst, SST_TASK_ID_MEDIA, IPC_CMD,
IPC_PREP_D3, PIPE_RSVD, 0, NULL, NULL,true, true, false, true);- if (ret < 0) {
dev_err(sst->dev, "not suspending FW!!!");return -EIO;Better to print and pass back the actual error code we got.
OK
- dev_info(dev, "runtime_suspend called\n");
- if (ctx->sst_state == SST_RESET) {
Drop this down to a dev_dbg() or remove it please (and some others later on).
Oh yes, did remove some will celanup others
dev_dbg(dev, "LPE is already in RESET state, No action");Missing \n.
return 0;- }
- /*save fw context*/
/* Upstream comment style please */
Sure will check these
Thanks
From: Mythri P K mythri.p.k@intel.com
SST state change should be done under sst_lock
Signed-off-by: Mythri P K mythri.p.k@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com --- sound/soc/intel/sst/sst.c | 4 +--- sound/soc/intel/sst/sst_ipc.c | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/soc/intel/sst/sst.c b/sound/soc/intel/sst/sst.c index 8eb7148..05aba24 100644 --- a/sound/soc/intel/sst/sst.c +++ b/sound/soc/intel/sst/sst.c @@ -466,16 +466,14 @@ static int intel_sst_runtime_resume(struct device *dev)
dev_info(dev, "runtime_resume called\n");
- mutex_lock(&ctx->sst_lock); if (ctx->sst_state == SST_RESET) { dev_dbg(dev, "DSP Downloading FW now...\n"); ret = sst_load_fw(ctx); if (ret) { dev_err(dev, "FW download fail %x\n", ret); - ctx->sst_state = SST_RESET; + sst_set_fw_state_locked(ctx, SST_RESET); } } - mutex_unlock(&ctx->sst_lock); return ret; }
diff --git a/sound/soc/intel/sst/sst_ipc.c b/sound/soc/intel/sst/sst_ipc.c index 2126f5b..484e609 100644 --- a/sound/soc/intel/sst/sst_ipc.c +++ b/sound/soc/intel/sst/sst_ipc.c @@ -230,7 +230,7 @@ static void process_fw_init(struct intel_sst_drv *sst_drv_ctx,
dev_dbg(sst_drv_ctx->dev, "*** FW Init msg came***\n"); if (init->result) { - sst_drv_ctx->sst_state = SST_RESET; + sst_set_fw_state_locked(sst_drv_ctx, SST_RESET); dev_err(sst_drv_ctx->dev, "FW Init failed, Error %x\n", init->result); retval = init->result;
participants (2)
-
Mark Brown -
Vinod Koul