alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [alsa-devel] [PATCH 0/5] soundwire: intel: add DAI callbacks
@ 2020-01-10 21:46 Pierre-Louis Bossart
  2020-01-10 21:46 ` [alsa-devel] [PATCH 1/5] soundwire: intel: rename res field as link_res Pierre-Louis Bossart
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Pierre-Louis Bossart @ 2020-01-10 21:46 UTC (permalink / raw)
  To: alsa-devel
  Cc: Pierre-Louis Bossart, tiwai, gregkh, linux-kernel,
	Ranjani Sridharan, vkoul, broonie, srinivas.kandagatla, jank,
	slawomir.blauciak, Bard liao, Rander Wang

The existing mainline code is missing most of the DAI callbacks needed
for a functional implementation, and the existing ones need to be
modified to provide the relevant information to SOF drivers.

As suggested by Vinod, these patches are shared first - with the risk
that they are separated from the actual DAI enablement, so reviewers
might wonder why they are needed in the first place.

For reference, the complete set of 100+ patches required for SoundWire
on Intel platforms is available here:

https://github.com/thesofproject/linux/pull/1692

Pierre-Louis Bossart (2):
  soundwire: intel: rename res field as link_res
  soundwire: intel: free all resources on hw_free()

Rander Wang (3):
  soundwire: intel: add prepare support in sdw dai driver
  soundwire: intel: add trigger support in sdw dai driver
  soundwire: intel: add sdw_stream_setup helper for .startup callback

 drivers/soundwire/intel.c | 198 ++++++++++++++++++++++++++++++++++----
 1 file changed, 178 insertions(+), 20 deletions(-)


base-commit: b637124800a157c4df3699d1137d8533394f7678
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [alsa-devel] [PATCH 1/5] soundwire: intel: rename res field as link_res
  2020-01-10 21:46 [alsa-devel] [PATCH 0/5] soundwire: intel: add DAI callbacks Pierre-Louis Bossart
@ 2020-01-10 21:46 ` Pierre-Louis Bossart
  2020-01-10 21:46 ` [alsa-devel] [PATCH 2/5] soundwire: intel: add prepare support in sdw dai driver Pierre-Louis Bossart
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Pierre-Louis Bossart @ 2020-01-10 21:46 UTC (permalink / raw)
  To: alsa-devel
  Cc: Pierre-Louis Bossart, tiwai, gregkh, linux-kernel,
	Ranjani Sridharan, vkoul, broonie, srinivas.kandagatla, jank,
	slawomir.blauciak, Sanyog Kale, Bard liao, Rander Wang

There are too many fields called 'res' so add prefix to make it easier
to track what the structures are.

Pure rename, no functionality change

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/intel.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 0371d3d5501a..64f97bb1a135 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -103,7 +103,7 @@ enum intel_pdi_type {
 struct sdw_intel {
 	struct sdw_cdns cdns;
 	int instance;
-	struct sdw_intel_link_res *res;
+	struct sdw_intel_link_res *link_res;
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debugfs;
 #endif
@@ -193,8 +193,8 @@ static ssize_t intel_sprintf(void __iomem *mem, bool l,
 static int intel_reg_show(struct seq_file *s_file, void *data)
 {
 	struct sdw_intel *sdw = s_file->private;
-	void __iomem *s = sdw->res->shim;
-	void __iomem *a = sdw->res->alh;
+	void __iomem *s = sdw->link_res->shim;
+	void __iomem *a = sdw->link_res->alh;
 	char *buf;
 	ssize_t ret;
 	int i, j;
@@ -289,7 +289,7 @@ static void intel_debugfs_exit(struct sdw_intel *sdw) {}
 static int intel_link_power_up(struct sdw_intel *sdw)
 {
 	unsigned int link_id = sdw->instance;
-	void __iomem *shim = sdw->res->shim;
+	void __iomem *shim = sdw->link_res->shim;
 	int spa_mask, cpa_mask;
 	int link_control, ret;
 
@@ -309,7 +309,7 @@ static int intel_link_power_up(struct sdw_intel *sdw)
 
 static int intel_shim_init(struct sdw_intel *sdw)
 {
-	void __iomem *shim = sdw->res->shim;
+	void __iomem *shim = sdw->link_res->shim;
 	unsigned int link_id = sdw->instance;
 	int sync_reg, ret;
 	u16 ioctl = 0, act = 0;
@@ -370,7 +370,7 @@ static int intel_shim_init(struct sdw_intel *sdw)
 static void intel_pdi_init(struct sdw_intel *sdw,
 			   struct sdw_cdns_stream_config *config)
 {
-	void __iomem *shim = sdw->res->shim;
+	void __iomem *shim = sdw->link_res->shim;
 	unsigned int link_id = sdw->instance;
 	int pcm_cap, pdm_cap;
 
@@ -404,7 +404,7 @@ static void intel_pdi_init(struct sdw_intel *sdw,
 static int
 intel_pdi_get_ch_cap(struct sdw_intel *sdw, unsigned int pdi_num, bool pcm)
 {
-	void __iomem *shim = sdw->res->shim;
+	void __iomem *shim = sdw->link_res->shim;
 	unsigned int link_id = sdw->instance;
 	int count;
 
@@ -476,7 +476,7 @@ static int intel_pdi_ch_update(struct sdw_intel *sdw)
 static void
 intel_pdi_shim_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi)
 {
-	void __iomem *shim = sdw->res->shim;
+	void __iomem *shim = sdw->link_res->shim;
 	unsigned int link_id = sdw->instance;
 	int pdi_conf = 0;
 
@@ -508,7 +508,7 @@ intel_pdi_shim_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi)
 static void
 intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi)
 {
-	void __iomem *alh = sdw->res->alh;
+	void __iomem *alh = sdw->link_res->alh;
 	unsigned int link_id = sdw->instance;
 	unsigned int conf;
 
@@ -535,7 +535,7 @@ static int intel_params_stream(struct sdw_intel *sdw,
 			       struct snd_pcm_hw_params *hw_params,
 			       int link_id, int alh_stream_id)
 {
-	struct sdw_intel_link_res *res = sdw->res;
+	struct sdw_intel_link_res *res = sdw->link_res;
 	struct sdw_intel_stream_params_data params_data;
 
 	params_data.substream = substream;
@@ -558,7 +558,7 @@ static int intel_pre_bank_switch(struct sdw_bus *bus)
 {
 	struct sdw_cdns *cdns = bus_to_cdns(bus);
 	struct sdw_intel *sdw = cdns_to_intel(cdns);
-	void __iomem *shim = sdw->res->shim;
+	void __iomem *shim = sdw->link_res->shim;
 	int sync_reg;
 
 	/* Write to register only for multi-link */
@@ -577,7 +577,7 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
 {
 	struct sdw_cdns *cdns = bus_to_cdns(bus);
 	struct sdw_intel *sdw = cdns_to_intel(cdns);
-	void __iomem *shim = sdw->res->shim;
+	void __iomem *shim = sdw->link_res->shim;
 	int sync_reg, ret;
 
 	/* Write to register only for multi-link */
@@ -934,9 +934,9 @@ static int intel_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	sdw->instance = pdev->id;
-	sdw->res = dev_get_platdata(&pdev->dev);
+	sdw->link_res = dev_get_platdata(&pdev->dev);
 	sdw->cdns.dev = &pdev->dev;
-	sdw->cdns.registers = sdw->res->registers;
+	sdw->cdns.registers = sdw->link_res->registers;
 	sdw->cdns.instance = sdw->instance;
 	sdw->cdns.msg_count = 0;
 	sdw->cdns.bus.dev = &pdev->dev;
@@ -976,11 +976,12 @@ static int intel_probe(struct platform_device *pdev)
 	intel_pdi_ch_update(sdw);
 
 	/* Acquire IRQ */
-	ret = request_threaded_irq(sdw->res->irq, sdw_cdns_irq, sdw_cdns_thread,
+	ret = request_threaded_irq(sdw->link_res->irq,
+				   sdw_cdns_irq, sdw_cdns_thread,
 				   IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns);
 	if (ret < 0) {
 		dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling device\n",
-			sdw->res->irq);
+			sdw->link_res->irq);
 		goto err_init;
 	}
 
@@ -1010,7 +1011,7 @@ static int intel_probe(struct platform_device *pdev)
 
 err_interrupt:
 	sdw_cdns_enable_interrupt(&sdw->cdns, false);
-	free_irq(sdw->res->irq, sdw);
+	free_irq(sdw->link_res->irq, sdw);
 err_init:
 	sdw_delete_bus_master(&sdw->cdns.bus);
 	return ret;
@@ -1025,7 +1026,7 @@ static int intel_remove(struct platform_device *pdev)
 	if (!sdw->cdns.bus.prop.hw_disabled) {
 		intel_debugfs_exit(sdw);
 		sdw_cdns_enable_interrupt(&sdw->cdns, false);
-		free_irq(sdw->res->irq, sdw);
+		free_irq(sdw->link_res->irq, sdw);
 		snd_soc_unregister_component(sdw->cdns.dev);
 	}
 	sdw_delete_bus_master(&sdw->cdns.bus);
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [alsa-devel] [PATCH 2/5] soundwire: intel: add prepare support in sdw dai driver
  2020-01-10 21:46 [alsa-devel] [PATCH 0/5] soundwire: intel: add DAI callbacks Pierre-Louis Bossart
  2020-01-10 21:46 ` [alsa-devel] [PATCH 1/5] soundwire: intel: rename res field as link_res Pierre-Louis Bossart
@ 2020-01-10 21:46 ` Pierre-Louis Bossart
  2020-01-10 21:46 ` [alsa-devel] [PATCH 3/5] soundwire: intel: add trigger " Pierre-Louis Bossart
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Pierre-Louis Bossart @ 2020-01-10 21:46 UTC (permalink / raw)
  To: alsa-devel
  Cc: Pierre-Louis Bossart, tiwai, gregkh, linux-kernel,
	Ranjani Sridharan, vkoul, broonie, srinivas.kandagatla, jank,
	slawomir.blauciak, Sanyog Kale, Bard liao, Rander Wang

From: Rander Wang <rander.wang@linux.intel.com>

The existing code does not expose a prepare operation, which is very
much needed to deal with underflow and resume operations.

Signed-off-by: Rander Wang <rander.wang@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/intel.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 64f97bb1a135..f7595fd7fdf9 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -699,6 +699,21 @@ static int intel_hw_params(struct snd_pcm_substream *substream,
 	return ret;
 }
 
+static int intel_prepare(struct snd_pcm_substream *substream,
+			 struct snd_soc_dai *dai)
+{
+	struct sdw_cdns_dma_data *dma;
+
+	dma = snd_soc_dai_get_dma_data(dai, substream);
+	if (!dma) {
+		dev_err(dai->dev, "failed to get dma data in %s",
+			__func__);
+		return -EIO;
+	}
+
+	return sdw_prepare_stream(dma->stream);
+}
+
 static int
 intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 {
@@ -745,6 +760,7 @@ static int intel_pdm_set_sdw_stream(struct snd_soc_dai *dai,
 
 static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
 	.hw_params = intel_hw_params,
+	.prepare = intel_prepare,
 	.hw_free = intel_hw_free,
 	.shutdown = intel_shutdown,
 	.set_sdw_stream = intel_pcm_set_sdw_stream,
@@ -752,6 +768,7 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
 
 static const struct snd_soc_dai_ops intel_pdm_dai_ops = {
 	.hw_params = intel_hw_params,
+	.prepare = intel_prepare,
 	.hw_free = intel_hw_free,
 	.shutdown = intel_shutdown,
 	.set_sdw_stream = intel_pdm_set_sdw_stream,
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [alsa-devel] [PATCH 3/5] soundwire: intel: add trigger support in sdw dai driver
  2020-01-10 21:46 [alsa-devel] [PATCH 0/5] soundwire: intel: add DAI callbacks Pierre-Louis Bossart
  2020-01-10 21:46 ` [alsa-devel] [PATCH 1/5] soundwire: intel: rename res field as link_res Pierre-Louis Bossart
  2020-01-10 21:46 ` [alsa-devel] [PATCH 2/5] soundwire: intel: add prepare support in sdw dai driver Pierre-Louis Bossart
@ 2020-01-10 21:46 ` Pierre-Louis Bossart
  2020-01-10 21:46 ` [alsa-devel] [PATCH 4/5] soundwire: intel: add sdw_stream_setup helper for .startup callback Pierre-Louis Bossart
  2020-01-10 21:46 ` [alsa-devel] [PATCH 5/5] soundwire: intel: free all resources on hw_free() Pierre-Louis Bossart
  4 siblings, 0 replies; 8+ messages in thread
From: Pierre-Louis Bossart @ 2020-01-10 21:46 UTC (permalink / raw)
  To: alsa-devel
  Cc: Pierre-Louis Bossart, tiwai, gregkh, linux-kernel,
	Ranjani Sridharan, vkoul, broonie, srinivas.kandagatla, jank,
	slawomir.blauciak, Sanyog Kale, Bard liao, Rander Wang

From: Rander Wang <rander.wang@linux.intel.com>

The existing code does not expose a trigger callback, which is very
much required for streaming.

The SoundWire stream is enabled and disabled in trigger function.

Signed-off-by: Rander Wang <rander.wang@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/intel.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index f7595fd7fdf9..aa80c46156cb 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -714,6 +714,43 @@ static int intel_prepare(struct snd_pcm_substream *substream,
 	return sdw_prepare_stream(dma->stream);
 }
 
+static int intel_trigger(struct snd_pcm_substream *substream, int cmd,
+			 struct snd_soc_dai *dai)
+{
+	struct sdw_cdns_dma_data *dma;
+	int ret;
+
+	dma = snd_soc_dai_get_dma_data(dai, substream);
+	if (!dma) {
+		dev_err(dai->dev, "failed to get dma data in %s", __func__);
+		return -EIO;
+	}
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+	case SNDRV_PCM_TRIGGER_RESUME:
+		ret = sdw_enable_stream(dma->stream);
+		break;
+
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_STOP:
+		ret = sdw_disable_stream(dma->stream);
+		break;
+
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	if (ret)
+		dev_err(dai->dev,
+			"%s trigger %d failed: %d",
+			__func__, cmd, ret);
+	return ret;
+}
+
 static int
 intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 {
@@ -761,6 +798,7 @@ static int intel_pdm_set_sdw_stream(struct snd_soc_dai *dai,
 static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
 	.hw_params = intel_hw_params,
 	.prepare = intel_prepare,
+	.trigger = intel_trigger,
 	.hw_free = intel_hw_free,
 	.shutdown = intel_shutdown,
 	.set_sdw_stream = intel_pcm_set_sdw_stream,
@@ -769,6 +807,7 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
 static const struct snd_soc_dai_ops intel_pdm_dai_ops = {
 	.hw_params = intel_hw_params,
 	.prepare = intel_prepare,
+	.trigger = intel_trigger,
 	.hw_free = intel_hw_free,
 	.shutdown = intel_shutdown,
 	.set_sdw_stream = intel_pdm_set_sdw_stream,
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [alsa-devel] [PATCH 4/5] soundwire: intel: add sdw_stream_setup helper for .startup callback
  2020-01-10 21:46 [alsa-devel] [PATCH 0/5] soundwire: intel: add DAI callbacks Pierre-Louis Bossart
                   ` (2 preceding siblings ...)
  2020-01-10 21:46 ` [alsa-devel] [PATCH 3/5] soundwire: intel: add trigger " Pierre-Louis Bossart
@ 2020-01-10 21:46 ` Pierre-Louis Bossart
  2020-01-14  6:14   ` Vinod Koul
  2020-01-10 21:46 ` [alsa-devel] [PATCH 5/5] soundwire: intel: free all resources on hw_free() Pierre-Louis Bossart
  4 siblings, 1 reply; 8+ messages in thread
From: Pierre-Louis Bossart @ 2020-01-10 21:46 UTC (permalink / raw)
  To: alsa-devel
  Cc: Pierre-Louis Bossart, tiwai, gregkh, linux-kernel,
	Ranjani Sridharan, vkoul, broonie, srinivas.kandagatla, jank,
	slawomir.blauciak, Sanyog Kale, Bard liao, Rander Wang

From: Rander Wang <rander.wang@linux.intel.com>

The sdw stream is allocated and stored in dai to share the sdw runtime
information.

Signed-off-by: Rander Wang <rander.wang@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/intel.c | 65 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index aa80c46156cb..f4554386d832 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -617,6 +617,69 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
  * DAI routines
  */
 
+static int sdw_stream_setup(struct snd_pcm_substream *substream,
+			    struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct sdw_stream_runtime *sdw_stream = NULL;
+	char *name;
+	int i, ret;
+
+	name = kzalloc(32, GFP_KERNEL);
+	if (!name)
+		return -ENOMEM;
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		snprintf(name, 32, "%s-Playback", dai->name);
+	else
+		snprintf(name, 32, "%s-Capture", dai->name);
+
+	sdw_stream = sdw_alloc_stream(name);
+	if (!sdw_stream) {
+		dev_err(dai->dev, "alloc stream failed for DAI %s", dai->name);
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	/* Set stream pointer on CPU DAI */
+	ret = snd_soc_dai_set_sdw_stream(dai, sdw_stream, substream->stream);
+	if (ret < 0) {
+		dev_err(dai->dev, "failed to set stream pointer on cpu dai %s",
+			dai->name);
+		goto release_stream;
+	}
+
+	/* Set stream pointer on all CODEC DAIs */
+	for (i = 0; i < rtd->num_codecs; i++) {
+		ret = snd_soc_dai_set_sdw_stream(rtd->codec_dais[i], sdw_stream,
+						 substream->stream);
+		if (ret < 0) {
+			dev_err(dai->dev, "failed to set stream pointer on codec dai %s",
+				rtd->codec_dais[i]->name);
+			goto release_stream;
+		}
+	}
+
+	return 0;
+
+release_stream:
+	sdw_release_stream(sdw_stream);
+error:
+	kfree(name);
+	return ret;
+}
+
+static int intel_startup(struct snd_pcm_substream *substream,
+			 struct snd_soc_dai *dai)
+{
+	/*
+	 * TODO: add pm_runtime support here, the startup callback
+	 * will make sure the IP is 'active'
+	 */
+
+	return sdw_stream_setup(substream, dai);
+}
+
 static int intel_hw_params(struct snd_pcm_substream *substream,
 			   struct snd_pcm_hw_params *params,
 			   struct snd_soc_dai *dai)
@@ -796,6 +859,7 @@ static int intel_pdm_set_sdw_stream(struct snd_soc_dai *dai,
 }
 
 static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
+	.startup = intel_startup,
 	.hw_params = intel_hw_params,
 	.prepare = intel_prepare,
 	.trigger = intel_trigger,
@@ -805,6 +869,7 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
 };
 
 static const struct snd_soc_dai_ops intel_pdm_dai_ops = {
+	.startup = intel_startup,
 	.hw_params = intel_hw_params,
 	.prepare = intel_prepare,
 	.trigger = intel_trigger,
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [alsa-devel] [PATCH 5/5] soundwire: intel: free all resources on hw_free()
  2020-01-10 21:46 [alsa-devel] [PATCH 0/5] soundwire: intel: add DAI callbacks Pierre-Louis Bossart
                   ` (3 preceding siblings ...)
  2020-01-10 21:46 ` [alsa-devel] [PATCH 4/5] soundwire: intel: add sdw_stream_setup helper for .startup callback Pierre-Louis Bossart
@ 2020-01-10 21:46 ` Pierre-Louis Bossart
  4 siblings, 0 replies; 8+ messages in thread
From: Pierre-Louis Bossart @ 2020-01-10 21:46 UTC (permalink / raw)
  To: alsa-devel
  Cc: Pierre-Louis Bossart, tiwai, gregkh, linux-kernel,
	Ranjani Sridharan, vkoul, broonie, srinivas.kandagatla, jank,
	slawomir.blauciak, Sanyog Kale, Bard liao, Rander Wang

Make sure all calls to the SoundWire stream API are done and involve
callback

Signed-off-by: Rander Wang <rander.wang@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/intel.c | 40 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index f4554386d832..4af46693393a 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -550,6 +550,25 @@ static int intel_params_stream(struct sdw_intel *sdw,
 	return -EIO;
 }
 
+static int intel_free_stream(struct sdw_intel *sdw,
+			     struct snd_pcm_substream *substream,
+			     struct snd_soc_dai *dai,
+			     int link_id)
+{
+	struct sdw_intel_link_res *res = sdw->link_res;
+	struct sdw_intel_stream_free_data free_data;
+
+	free_data.substream = substream;
+	free_data.dai = dai;
+	free_data.link_id = link_id;
+
+	if (res->ops && res->ops->free_stream && res->dev)
+		return res->ops->free_stream(res->dev,
+					     &free_data);
+
+	return 0;
+}
+
 /*
  * bank switch routines
  */
@@ -818,6 +837,7 @@ static int
 intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 {
 	struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
+	struct sdw_intel *sdw = cdns_to_intel(cdns);
 	struct sdw_cdns_dma_data *dma;
 	int ret;
 
@@ -825,12 +845,28 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 	if (!dma)
 		return -EIO;
 
+	ret = sdw_deprepare_stream(dma->stream);
+	if (ret) {
+		dev_err(dai->dev, "sdw_deprepare_stream: failed %d", ret);
+		return ret;
+	}
+
 	ret = sdw_stream_remove_master(&cdns->bus, dma->stream);
-	if (ret < 0)
+	if (ret < 0) {
 		dev_err(dai->dev, "remove master from stream %s failed: %d\n",
 			dma->stream->name, ret);
+		return ret;
+	}
 
-	return ret;
+	ret = intel_free_stream(sdw, substream, dai, sdw->instance);
+	if (ret < 0) {
+		dev_err(dai->dev, "intel_free_stream: failed %d", ret);
+		return ret;
+	}
+
+	sdw_release_stream(dma->stream);
+
+	return 0;
 }
 
 static void intel_shutdown(struct snd_pcm_substream *substream,
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [alsa-devel] [PATCH 4/5] soundwire: intel: add sdw_stream_setup helper for .startup callback
  2020-01-10 21:46 ` [alsa-devel] [PATCH 4/5] soundwire: intel: add sdw_stream_setup helper for .startup callback Pierre-Louis Bossart
@ 2020-01-14  6:14   ` Vinod Koul
  2020-01-14 18:55     ` Pierre-Louis Bossart
  0 siblings, 1 reply; 8+ messages in thread
From: Vinod Koul @ 2020-01-14  6:14 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: alsa-devel, tiwai, gregkh, linux-kernel, Ranjani Sridharan,
	broonie, srinivas.kandagatla, jank, slawomir.blauciak,
	Sanyog Kale, Bard liao, Rander Wang

On 10-01-20, 15:46, Pierre-Louis Bossart wrote:
> From: Rander Wang <rander.wang@linux.intel.com>
> 
> The sdw stream is allocated and stored in dai to share the sdw runtime
> information.
> 
> Signed-off-by: Rander Wang <rander.wang@linux.intel.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  drivers/soundwire/intel.c | 65 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> index aa80c46156cb..f4554386d832 100644
> --- a/drivers/soundwire/intel.c
> +++ b/drivers/soundwire/intel.c
> @@ -617,6 +617,69 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
>   * DAI routines
>   */
>  
> +static int sdw_stream_setup(struct snd_pcm_substream *substream,
> +			    struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct sdw_stream_runtime *sdw_stream = NULL;
> +	char *name;
> +	int i, ret;
> +
> +	name = kzalloc(32, GFP_KERNEL);
> +	if (!name)
> +		return -ENOMEM;
> +
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +		snprintf(name, 32, "%s-Playback", dai->name);
> +	else
> +		snprintf(name, 32, "%s-Capture", dai->name);

How about use DAI_SIZE instead of 32 here and above few places? Lets not
code number like this please

> +
> +	sdw_stream = sdw_alloc_stream(name);
> +	if (!sdw_stream) {
> +		dev_err(dai->dev, "alloc stream failed for DAI %s", dai->name);
> +		ret = -ENOMEM;
> +		goto error;
> +	}
> +
> +	/* Set stream pointer on CPU DAI */
> +	ret = snd_soc_dai_set_sdw_stream(dai, sdw_stream, substream->stream);
> +	if (ret < 0) {
> +		dev_err(dai->dev, "failed to set stream pointer on cpu dai %s",
> +			dai->name);
> +		goto release_stream;
> +	}
> +
> +	/* Set stream pointer on all CODEC DAIs */
> +	for (i = 0; i < rtd->num_codecs; i++) {
> +		ret = snd_soc_dai_set_sdw_stream(rtd->codec_dais[i], sdw_stream,
> +						 substream->stream);
> +		if (ret < 0) {
> +			dev_err(dai->dev, "failed to set stream pointer on codec dai %s",
> +				rtd->codec_dais[i]->name);
> +			goto release_stream;
> +		}
> +	}
> +
> +	return 0;
> +
> +release_stream:
> +	sdw_release_stream(sdw_stream);
> +error:
> +	kfree(name);
> +	return ret;
> +}
> +
> +static int intel_startup(struct snd_pcm_substream *substream,
> +			 struct snd_soc_dai *dai)
> +{
> +	/*
> +	 * TODO: add pm_runtime support here, the startup callback
> +	 * will make sure the IP is 'active'
> +	 */
> +
> +	return sdw_stream_setup(substream, dai);
> +}
> +
>  static int intel_hw_params(struct snd_pcm_substream *substream,
>  			   struct snd_pcm_hw_params *params,
>  			   struct snd_soc_dai *dai)
> @@ -796,6 +859,7 @@ static int intel_pdm_set_sdw_stream(struct snd_soc_dai *dai,
>  }
>  
>  static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
> +	.startup = intel_startup,
>  	.hw_params = intel_hw_params,
>  	.prepare = intel_prepare,
>  	.trigger = intel_trigger,
> @@ -805,6 +869,7 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
>  };
>  
>  static const struct snd_soc_dai_ops intel_pdm_dai_ops = {
> +	.startup = intel_startup,
>  	.hw_params = intel_hw_params,
>  	.prepare = intel_prepare,
>  	.trigger = intel_trigger,
> -- 
> 2.20.1

-- 
~Vinod
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [alsa-devel] [PATCH 4/5] soundwire: intel: add sdw_stream_setup helper for .startup callback
  2020-01-14  6:14   ` Vinod Koul
@ 2020-01-14 18:55     ` Pierre-Louis Bossart
  0 siblings, 0 replies; 8+ messages in thread
From: Pierre-Louis Bossart @ 2020-01-14 18:55 UTC (permalink / raw)
  To: Vinod Koul
  Cc: alsa-devel, tiwai, gregkh, linux-kernel, Ranjani Sridharan,
	broonie, srinivas.kandagatla, jank, slawomir.blauciak,
	Sanyog Kale, Bard liao, Rander Wang



>> +	name = kzalloc(32, GFP_KERNEL);
>> +	if (!name)
>> +		return -ENOMEM;
>> +
>> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>> +		snprintf(name, 32, "%s-Playback", dai->name);
>> +	else
>> +		snprintf(name, 32, "%s-Capture", dai->name);
> 
> How about use DAI_SIZE instead of 32 here and above few places? Lets not
> code number like this please

Yes, thanks for spotting this. kasprintf seems like a better solution I 
guess, will send a v2.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-01-14 19:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-10 21:46 [alsa-devel] [PATCH 0/5] soundwire: intel: add DAI callbacks Pierre-Louis Bossart
2020-01-10 21:46 ` [alsa-devel] [PATCH 1/5] soundwire: intel: rename res field as link_res Pierre-Louis Bossart
2020-01-10 21:46 ` [alsa-devel] [PATCH 2/5] soundwire: intel: add prepare support in sdw dai driver Pierre-Louis Bossart
2020-01-10 21:46 ` [alsa-devel] [PATCH 3/5] soundwire: intel: add trigger " Pierre-Louis Bossart
2020-01-10 21:46 ` [alsa-devel] [PATCH 4/5] soundwire: intel: add sdw_stream_setup helper for .startup callback Pierre-Louis Bossart
2020-01-14  6:14   ` Vinod Koul
2020-01-14 18:55     ` Pierre-Louis Bossart
2020-01-10 21:46 ` [alsa-devel] [PATCH 5/5] soundwire: intel: free all resources on hw_free() Pierre-Louis Bossart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).