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

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

[Vinod Koul]

This series fixes the device open pm handling and adds the runtime pm
support. Also we fix lock usage while changing the driver state

Changes in v2:
 - fix style in comments
 - fix error prints and remove noisy debug

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               |   65 +++++++++++++++++++++++++++++++
 sound/soc/intel/sst/sst_drv_interface.c |   12 ++----
 sound/soc/intel/sst/sst_ipc.c           |    2 +-
 3 files changed, 70 insertions(+), 9 deletions(-)

[PATCH v2 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 v2 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 runtime 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 |   67 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 67 insertions(+), 0 deletions(-)

diff --git a/sound/soc/intel/sst/sst.c b/sound/soc/intel/sst/sst.c
index fa34217..7b8a110 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!!, Err: %d\n", ret);
+		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,6 +436,50 @@ static void intel_sst_remove(struct pci_dev *pci)
 	pci_set_drvdata(pci, NULL);
 }
 
+static int intel_sst_runtime_suspend(struct device *dev)
+{
+	int ret = 0;
+	struct intel_sst_drv *ctx = dev_get_drvdata(dev);
+
+	if (ctx->sst_state == SST_RESET) {
+		dev_dbg(dev, "LPE is already in RESET state, No action\n");
+		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);
+
+	mutex_lock(&ctx->sst_lock);
+	if (ctx->sst_state == SST_RESET) {
+		ret = sst_load_fw(ctx);
+		if (ret) {
+			dev_err(dev, "FW download fail %d\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},
@@ -429,6 +491,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 v2 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 7b8a110..04af246 100644
--- a/sound/soc/intel/sst/sst.c
+++ b/sound/soc/intel/sst/sst.c
@@ -463,15 +463,13 @@ static int intel_sst_runtime_resume(struct device *dev)
 	int ret = 0;
 	struct intel_sst_drv *ctx = dev_get_drvdata(dev);
 
-	mutex_lock(&ctx->sst_lock);
 	if (ctx->sst_state == SST_RESET) {
 		ret = sst_load_fw(ctx);
 		if (ret) {
 			dev_err(dev, "FW download fail %d\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 v2 0/3] ASoC: Intel: add runtime power management support

[Vinod Koul]

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

On Fri, Oct 31, 2014 at 10:34:51AM +0000, Mark Brown wrote:
> On Fri, Oct 31, 2014 at 12:38:18PM +0530, Vinod Koul wrote:
> > This series fixes the device open pm handling and adds the runtime pm
> > support. Also we fix lock usage while changing the driver state
> 
> Can you please stop sending so many different patch serieses for the
> same code simultaneously?  I'm assuming that these are mostly orthogonal
> but it's making it hard to keep track of what's going on and I expect at
> some point they'll end up conflicting with each other.
The cover mentions the dependency for a series. This one is the first one,
thats why I resent after your comments, you cant apply the other two without
this one.

The compressed is second series, still applies on top of this. ACPI is third
one.

If it helps, pls process this one, and I can resend the remaining ones one at
a time.

Thanks
-- 
~Vinod

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

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

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

[Mark Brown]

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

On Fri, Oct 31, 2014 at 12:38:18PM +0530, Vinod Koul wrote:
> This series fixes the device open pm handling and adds the runtime pm
> support. Also we fix lock usage while changing the driver state

Can you please stop sending so many different patch serieses for the
same code simultaneously?  I'm assuming that these are mostly orthogonal
but it's making it hard to keep track of what's going on and I expect at
some point they'll end up conflicting with each other.

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

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

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

[Mark Brown]

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

On Fri, Oct 31, 2014 at 03:30:05PM +0530, Vinod Koul wrote:

> If it helps, pls process this one, and I can resend the remaining ones one at
> a time.

Please do that in future, having information in the cover letter isn't
that helpful since I still need to actually open the cover letter and
read it.

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

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

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

[Mark Brown]

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

On Fri, Oct 31, 2014 at 12:38:20PM +0530, Vinod Koul wrote:
> This patch adds the runtime pm handlers, the driver already has code for
> get/put for runtime pm and only these handlers being missing.

Applied, thanks.

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

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

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

[Mark Brown]

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

On Fri, Oct 31, 2014 at 12:38:21PM +0530, Vinod Koul wrote:
> From: Mythri P K <mythri.p.k@intel.com>
> 
> SST state change should be done under sst_lock

Applied, but *please* try to use subject lines reflecting the style for
the subsystem.

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

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