[PATCH 0/3] ASoC: Intel: add runtime power management support

[PATCH 0/3] ASoC: Intel: add runtime power management support

[Vinod Koul]

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(-)

[PATCH 1/3] ASoC: Intel: mrfld: Fix runtime pm calls in sst_open_pcm_stream

[Vinod Koul]

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;
-- 
1.7.0.4

[PATCH 2/3] ASoC: Intel: sst: add runtime power management handling

[Vinod Koul]

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);
-- 
1.7.0.4

[PATCH 3/3] Audio: SST: use lock when changing SST state.

[Vinod Koul]

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;
-- 
1.7.0.4

Re: [PATCH 2/3] ASoC: Intel: sst: add runtime power management handling

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 682 bytes --]

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 */

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/3] ASoC: Intel: sst: add runtime power management handling

[Vinod Koul]

[-- Attachment #1.1: Type: text/plain, Size: 921 bytes --]

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
-- 
~Vinod

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/3] ASoC: Intel: mrfld: Fix runtime pm calls in sst_open_pcm_stream

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 181 bytes --]

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.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]