linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] Platform driver update to support playback recover after resume
@ 2020-11-28  4:59 Srinivasa Rao Mandadapu
  2020-11-28  4:59 ` [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state Srinivasa Rao Mandadapu
  2020-11-28  4:59 ` [PATCH v4 2/2] ASoC: qcom: Add support for playback recover after resume Srinivasa Rao Mandadapu
  0 siblings, 2 replies; 13+ messages in thread
From: Srinivasa Rao Mandadapu @ 2020-11-28  4:59 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, rohitkr,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel
  Cc: Srinivasa Rao Mandadapu

This patch set is to add support for playback recover after hard suspend and resume.
It includes:
1. Reverting part of previous commit, which is for handling registers invalid state
after hard suspend.
2. Adding pm ops in component driver and do regcache sync.

Srinivasa Rao Mandadapu (2):
  Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF
    invalid state
  ASoC: qcom: Add support for playback recover after resume

 sound/soc/qcom/lpass-cpu.c      | 20 ++----------------
 sound/soc/qcom/lpass-platform.c | 46 +++++++++++++++++++++++++++++++----------
 2 files changed, 37 insertions(+), 29 deletions(-)

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


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

* [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state
  2020-11-28  4:59 [PATCH v4 0/2] Platform driver update to support playback recover after resume Srinivasa Rao Mandadapu
@ 2020-11-28  4:59 ` Srinivasa Rao Mandadapu
  2020-11-30 11:09   ` Srinivas Kandagatla
  2020-11-30 12:46   ` Mark Brown
  2020-11-28  4:59 ` [PATCH v4 2/2] ASoC: qcom: Add support for playback recover after resume Srinivasa Rao Mandadapu
  1 sibling, 2 replies; 13+ messages in thread
From: Srinivasa Rao Mandadapu @ 2020-11-28  4:59 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, rohitkr,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel
  Cc: Srinivasa Rao Mandadapu, V Sujith Kumar Reddy

This reverts part of commit b1824968221c
("ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state")

To identify LPAIF invalid state after device suspend and resume,
made I2S and DMA control registers not volatile, which is not necessary.
Instead invalid reg state can be handled with regcache APIs.
The BCLK ref count is necessary to enable clock only it's in disable state.

Signed-off-by: V Sujith Kumar Reddy <vsujithk@codeaurora.org>
Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
---
 sound/soc/qcom/lpass-cpu.c      | 20 ++------------------
 sound/soc/qcom/lpass-platform.c | 11 -----------
 2 files changed, 2 insertions(+), 29 deletions(-)

diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index af684fd..c5e99c2 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -270,18 +270,6 @@ static int lpass_cpu_daiops_trigger(struct snd_pcm_substream *substream,
 	struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
 	unsigned int id = dai->driver->id;
 	int ret = -EINVAL;
-	unsigned int val = 0;
-
-	ret = regmap_read(drvdata->lpaif_map,
-				LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id), &val);
-	if (ret) {
-		dev_err(dai->dev, "error reading from i2sctl reg: %d\n", ret);
-		return ret;
-	}
-	if (val == LPAIF_I2SCTL_RESET_STATE) {
-		dev_err(dai->dev, "error in i2sctl register state\n");
-		return -ENOTRECOVERABLE;
-	}
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
@@ -454,20 +442,16 @@ static bool lpass_cpu_regmap_volatile(struct device *dev, unsigned int reg)
 	struct lpass_variant *v = drvdata->variant;
 	int i;
 
-	for (i = 0; i < v->i2s_ports; ++i)
-		if (reg == LPAIF_I2SCTL_REG(v, i))
-			return true;
 	for (i = 0; i < v->irq_ports; ++i)
 		if (reg == LPAIF_IRQSTAT_REG(v, i))
 			return true;
 
 	for (i = 0; i < v->rdma_channels; ++i)
-		if (reg == LPAIF_RDMACURR_REG(v, i) || reg == LPAIF_RDMACTL_REG(v, i))
+		if (reg == LPAIF_RDMACURR_REG(v, i))
 			return true;
 
 	for (i = 0; i < v->wrdma_channels; ++i)
-		if (reg == LPAIF_WRDMACURR_REG(v, i + v->wrdma_channel_start) ||
-			reg == LPAIF_WRDMACTL_REG(v, i + v->wrdma_channel_start))
+		if (reg == LPAIF_WRDMACURR_REG(v, i + v->wrdma_channel_start))
 			return true;
 
 	return false;
diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
index 80b09de..0e71899 100644
--- a/sound/soc/qcom/lpass-platform.c
+++ b/sound/soc/qcom/lpass-platform.c
@@ -452,7 +452,6 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
 	unsigned int reg_irqclr = 0, val_irqclr = 0;
 	unsigned int  reg_irqen = 0, val_irqen = 0, val_mask = 0;
 	unsigned int dai_id = cpu_dai->driver->id;
-	unsigned int dma_ctrl_reg = 0;
 
 	ch = pcm_data->dma_ch;
 	if (dir ==  SNDRV_PCM_STREAM_PLAYBACK) {
@@ -469,17 +468,7 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
 		id = pcm_data->dma_ch - v->wrdma_channel_start;
 		map = drvdata->lpaif_map;
 	}
-	ret = regmap_read(map, LPAIF_DMACTL_REG(v, ch, dir, dai_id), &dma_ctrl_reg);
-	if (ret) {
-		dev_err(soc_runtime->dev, "error reading from rdmactl reg: %d\n", ret);
-		return ret;
-	}
 
-	if (dma_ctrl_reg == LPAIF_DMACTL_RESET_STATE ||
-		dma_ctrl_reg == LPAIF_DMACTL_RESET_STATE + 1) {
-		dev_err(soc_runtime->dev, "error in rdmactl register state\n");
-		return -ENOTRECOVERABLE;
-	}
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


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

* [PATCH v4 2/2] ASoC: qcom: Add support for playback recover after resume
  2020-11-28  4:59 [PATCH v4 0/2] Platform driver update to support playback recover after resume Srinivasa Rao Mandadapu
  2020-11-28  4:59 ` [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state Srinivasa Rao Mandadapu
@ 2020-11-28  4:59 ` Srinivasa Rao Mandadapu
  2020-11-30 11:19   ` Srinivas Kandagatla
  2020-11-30 16:26   ` Steev Klimaszewski
  1 sibling, 2 replies; 13+ messages in thread
From: Srinivasa Rao Mandadapu @ 2020-11-28  4:59 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, rohitkr,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel
  Cc: Srinivasa Rao Mandadapu, V Sujith Kumar Reddy

To support playback continuation after hard suspend(bypass powerd)
and resume add component driver ops and do regcache sync.

Signed-off-by: V Sujith Kumar Reddy <vsujithk@codeaurora.org>
Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
---
 sound/soc/qcom/lpass-platform.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
index 0e71899..12764a8 100644
--- a/sound/soc/qcom/lpass-platform.c
+++ b/sound/soc/qcom/lpass-platform.c
@@ -827,6 +827,39 @@ static void lpass_platform_pcm_free(struct snd_soc_component *component,
 	}
 }
 
+static int lpass_platform_pcmops_suspend(struct snd_soc_component *component)
+{
+	struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
+	struct regmap *map;
+	unsigned int dai_id = component->id;
+
+	if (dai_id == LPASS_DP_RX)
+		map = drvdata->hdmiif_map;
+	else
+		map = drvdata->lpaif_map;
+
+	regcache_cache_only(map, true);
+	regcache_mark_dirty(map);
+
+	return 0;
+}
+
+static int lpass_platform_pcmops_resume(struct snd_soc_component *component)
+{
+	struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
+	struct regmap *map;
+	unsigned int dai_id = component->id;
+
+	if (dai_id == LPASS_DP_RX)
+		map = drvdata->hdmiif_map;
+	else
+		map = drvdata->lpaif_map;
+
+	regcache_cache_only(map, false);
+	return regcache_sync(map);
+}
+
+
 static const struct snd_soc_component_driver lpass_component_driver = {
 	.name		= DRV_NAME,
 	.open		= lpass_platform_pcmops_open,
@@ -839,6 +872,8 @@ static const struct snd_soc_component_driver lpass_component_driver = {
 	.mmap		= lpass_platform_pcmops_mmap,
 	.pcm_construct	= lpass_platform_pcm_new,
 	.pcm_destruct	= lpass_platform_pcm_free,
+	.suspend		= lpass_platform_pcmops_suspend,
+	.resume			= lpass_platform_pcmops_resume,
 
 };
 
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


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

* Re: [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state
  2020-11-28  4:59 ` [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state Srinivasa Rao Mandadapu
@ 2020-11-30 11:09   ` Srinivas Kandagatla
  2020-11-30 12:46   ` Mark Brown
  1 sibling, 0 replies; 13+ messages in thread
From: Srinivas Kandagatla @ 2020-11-30 11:09 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, lgirdwood,
	broonie, robh+dt, plai, bgoswami, perex, tiwai, rohitkr,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel
  Cc: V Sujith Kumar Reddy



On 28/11/2020 04:59, Srinivasa Rao Mandadapu wrote:
> This reverts part of commit b1824968221c
> ("ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state")

This should probably go to Fixes tag!

> 

> To identify LPAIF invalid state after device suspend and resume,
> made I2S and DMA control registers not volatile, which is not necessary.
This comment is bit confusing!

Basically it should be something like
"DMA control registers are not volatile, so remove these from volatile 
registers list"

--srini


> Instead invalid reg state can be handled with regcache APIs.
> The BCLK ref count is necessary to enable clock only it's in disable state.
> 
> Signed-off-by: V Sujith Kumar Reddy <vsujithk@codeaurora.org>
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> ---
>   sound/soc/qcom/lpass-cpu.c      | 20 ++------------------
>   sound/soc/qcom/lpass-platform.c | 11 -----------
>   2 files changed, 2 insertions(+), 29 deletions(-)
> 
> diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
> index af684fd..c5e99c2 100644
> --- a/sound/soc/qcom/lpass-cpu.c
> +++ b/sound/soc/qcom/lpass-cpu.c
> @@ -270,18 +270,6 @@ static int lpass_cpu_daiops_trigger(struct snd_pcm_substream *substream,
>   	struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
>   	unsigned int id = dai->driver->id;
>   	int ret = -EINVAL;
> -	unsigned int val = 0;
> -
> -	ret = regmap_read(drvdata->lpaif_map,
> -				LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id), &val);
> -	if (ret) {
> -		dev_err(dai->dev, "error reading from i2sctl reg: %d\n", ret);
> -		return ret;
> -	}
> -	if (val == LPAIF_I2SCTL_RESET_STATE) {
> -		dev_err(dai->dev, "error in i2sctl register state\n");
> -		return -ENOTRECOVERABLE;
> -	}
>   
>   	switch (cmd) {
>   	case SNDRV_PCM_TRIGGER_START:
> @@ -454,20 +442,16 @@ static bool lpass_cpu_regmap_volatile(struct device *dev, unsigned int reg)
>   	struct lpass_variant *v = drvdata->variant;
>   	int i;
>   
> -	for (i = 0; i < v->i2s_ports; ++i)
> -		if (reg == LPAIF_I2SCTL_REG(v, i))
> -			return true;
>   	for (i = 0; i < v->irq_ports; ++i)
>   		if (reg == LPAIF_IRQSTAT_REG(v, i))
>   			return true;
>   
>   	for (i = 0; i < v->rdma_channels; ++i)
> -		if (reg == LPAIF_RDMACURR_REG(v, i) || reg == LPAIF_RDMACTL_REG(v, i))
> +		if (reg == LPAIF_RDMACURR_REG(v, i))
>   			return true;
>   
>   	for (i = 0; i < v->wrdma_channels; ++i)
> -		if (reg == LPAIF_WRDMACURR_REG(v, i + v->wrdma_channel_start) ||
> -			reg == LPAIF_WRDMACTL_REG(v, i + v->wrdma_channel_start))
> +		if (reg == LPAIF_WRDMACURR_REG(v, i + v->wrdma_channel_start))
>   			return true;
>   
>   	return false;
> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
> index 80b09de..0e71899 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -452,7 +452,6 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
>   	unsigned int reg_irqclr = 0, val_irqclr = 0;
>   	unsigned int  reg_irqen = 0, val_irqen = 0, val_mask = 0;
>   	unsigned int dai_id = cpu_dai->driver->id;
> -	unsigned int dma_ctrl_reg = 0;
>   
>   	ch = pcm_data->dma_ch;
>   	if (dir ==  SNDRV_PCM_STREAM_PLAYBACK) {
> @@ -469,17 +468,7 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
>   		id = pcm_data->dma_ch - v->wrdma_channel_start;
>   		map = drvdata->lpaif_map;
>   	}
> -	ret = regmap_read(map, LPAIF_DMACTL_REG(v, ch, dir, dai_id), &dma_ctrl_reg);
> -	if (ret) {
> -		dev_err(soc_runtime->dev, "error reading from rdmactl reg: %d\n", ret);
> -		return ret;
> -	}
>   
> -	if (dma_ctrl_reg == LPAIF_DMACTL_RESET_STATE ||
> -		dma_ctrl_reg == LPAIF_DMACTL_RESET_STATE + 1) {
> -		dev_err(soc_runtime->dev, "error in rdmactl register state\n");
> -		return -ENOTRECOVERABLE;
> -	}
>   	switch (cmd) {
>   	case SNDRV_PCM_TRIGGER_START:
>   	case SNDRV_PCM_TRIGGER_RESUME:
> 

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

* Re: [PATCH v4 2/2] ASoC: qcom: Add support for playback recover after resume
  2020-11-28  4:59 ` [PATCH v4 2/2] ASoC: qcom: Add support for playback recover after resume Srinivasa Rao Mandadapu
@ 2020-11-30 11:19   ` Srinivas Kandagatla
  2020-11-30 16:26   ` Steev Klimaszewski
  1 sibling, 0 replies; 13+ messages in thread
From: Srinivas Kandagatla @ 2020-11-30 11:19 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, lgirdwood,
	broonie, robh+dt, plai, bgoswami, perex, tiwai, rohitkr,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel
  Cc: V Sujith Kumar Reddy



On 28/11/2020 04:59, Srinivasa Rao Mandadapu wrote:
> To support playback continuation after hard suspend(bypass powerd)
> and resume add component driver ops and do regcache sync.
> 
> Signed-off-by: V Sujith Kumar Reddy <vsujithk@codeaurora.org>
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>

LGTM,

Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>


> ---
>   sound/soc/qcom/lpass-platform.c | 35 +++++++++++++++++++++++++++++++++++
>   1 file changed, 35 insertions(+)
> 
> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
> index 0e71899..12764a8 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -827,6 +827,39 @@ static void lpass_platform_pcm_free(struct snd_soc_component *component,
>   	}
>   }
>   
> +static int lpass_platform_pcmops_suspend(struct snd_soc_component *component)
> +{
> +	struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
> +	struct regmap *map;
> +	unsigned int dai_id = component->id;
> +
> +	if (dai_id == LPASS_DP_RX)
> +		map = drvdata->hdmiif_map;
> +	else
> +		map = drvdata->lpaif_map;
> +
> +	regcache_cache_only(map, true);
> +	regcache_mark_dirty(map);
> +
> +	return 0;
> +}
> +
> +static int lpass_platform_pcmops_resume(struct snd_soc_component *component)
> +{
> +	struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
> +	struct regmap *map;
> +	unsigned int dai_id = component->id;
> +
> +	if (dai_id == LPASS_DP_RX)
> +		map = drvdata->hdmiif_map;
> +	else
> +		map = drvdata->lpaif_map;
> +
> +	regcache_cache_only(map, false);
> +	return regcache_sync(map);
> +}
> +
> +
>   static const struct snd_soc_component_driver lpass_component_driver = {
>   	.name		= DRV_NAME,
>   	.open		= lpass_platform_pcmops_open,
> @@ -839,6 +872,8 @@ static const struct snd_soc_component_driver lpass_component_driver = {
>   	.mmap		= lpass_platform_pcmops_mmap,
>   	.pcm_construct	= lpass_platform_pcm_new,
>   	.pcm_destruct	= lpass_platform_pcm_free,
> +	.suspend		= lpass_platform_pcmops_suspend,
> +	.resume			= lpass_platform_pcmops_resume,
>   
>   };
>   
> 

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

* Re: [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state
  2020-11-28  4:59 ` [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state Srinivasa Rao Mandadapu
  2020-11-30 11:09   ` Srinivas Kandagatla
@ 2020-11-30 12:46   ` Mark Brown
  2020-12-01 17:31     ` Srinivasa Rao Mandadapu
  1 sibling, 1 reply; 13+ messages in thread
From: Mark Brown @ 2020-11-30 12:46 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu
  Cc: agross, bjorn.andersson, lgirdwood, robh+dt, plai, bgoswami,
	perex, tiwai, srinivas.kandagatla, rohitkr, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel, V Sujith Kumar Reddy

[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]

On Sat, Nov 28, 2020 at 10:29:18AM +0530, Srinivasa Rao Mandadapu wrote:
> This reverts part of commit b1824968221c
> ("ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state")
> 
> To identify LPAIF invalid state after device suspend and resume,
> made I2S and DMA control registers not volatile, which is not necessary.
> Instead invalid reg state can be handled with regcache APIs.
> The BCLK ref count is necessary to enable clock only it's in disable state.

Part of this commit message says that the problem was making the registers
non-volatile but both the change and the rest of the commit message say
that the issue was that the registers were made volatile.  I'm also
still unclear as to what the issue is either way - how does reading the
state of the registers from the hardware instead of the cache affect
things?

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v4 2/2] ASoC: qcom: Add support for playback recover after resume
  2020-11-28  4:59 ` [PATCH v4 2/2] ASoC: qcom: Add support for playback recover after resume Srinivasa Rao Mandadapu
  2020-11-30 11:19   ` Srinivas Kandagatla
@ 2020-11-30 16:26   ` Steev Klimaszewski
  1 sibling, 0 replies; 13+ messages in thread
From: Steev Klimaszewski @ 2020-11-30 16:26 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, lgirdwood,
	broonie, robh+dt, plai, bgoswami, perex, tiwai,
	srinivas.kandagatla, rohitkr, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel
  Cc: V Sujith Kumar Reddy


On 11/27/20 10:59 PM, Srinivasa Rao Mandadapu wrote:
> To support playback continuation after hard suspend(bypass powerd)
> and resume add component driver ops and do regcache sync.
>
> Signed-off-by: V Sujith Kumar Reddy <vsujithk@codeaurora.org>
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> ---
>  sound/soc/qcom/lpass-platform.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>
> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
> index 0e71899..12764a8 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -827,6 +827,39 @@ static void lpass_platform_pcm_free(struct snd_soc_component *component,
>  	}
>  }
>  
> +static int lpass_platform_pcmops_suspend(struct snd_soc_component *component)
> +{
> +	struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
> +	struct regmap *map;
> +	unsigned int dai_id = component->id;
> +
> +	if (dai_id == LPASS_DP_RX)
> +		map = drvdata->hdmiif_map;
> +	else
> +		map = drvdata->lpaif_map;
> +
> +	regcache_cache_only(map, true);
> +	regcache_mark_dirty(map);
> +
> +	return 0;
> +}
> +
> +static int lpass_platform_pcmops_resume(struct snd_soc_component *component)
> +{
> +	struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
> +	struct regmap *map;
> +	unsigned int dai_id = component->id;
> +
> +	if (dai_id == LPASS_DP_RX)
> +		map = drvdata->hdmiif_map;
> +	else
> +		map = drvdata->lpaif_map;
> +
> +	regcache_cache_only(map, false);
> +	return regcache_sync(map);
> +}
> +
> +
>  static const struct snd_soc_component_driver lpass_component_driver = {
>  	.name		= DRV_NAME,
>  	.open		= lpass_platform_pcmops_open,
> @@ -839,6 +872,8 @@ static const struct snd_soc_component_driver lpass_component_driver = {
>  	.mmap		= lpass_platform_pcmops_mmap,
>  	.pcm_construct	= lpass_platform_pcm_new,
>  	.pcm_destruct	= lpass_platform_pcm_free,
> +	.suspend		= lpass_platform_pcmops_suspend,
> +	.resume			= lpass_platform_pcmops_resume,
>  
>  };
>  

Tested this series on a Lenovo Yoga C630

Tested-by: Steev Klimaszewski <steev@kali.org>



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

* Re: [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state
  2020-11-30 12:46   ` Mark Brown
@ 2020-12-01 17:31     ` Srinivasa Rao Mandadapu
  2020-12-01 17:51       ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: Srinivasa Rao Mandadapu @ 2020-12-01 17:31 UTC (permalink / raw)
  To: Mark Brown
  Cc: agross, bjorn.andersson, lgirdwood, robh+dt, plai, bgoswami,
	perex, tiwai, srinivas.kandagatla, rohitkr, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel, V Sujith Kumar Reddy

Thanks Mark for your time!!!

On 11/30/2020 6:16 PM, Mark Brown wrote:
> On Sat, Nov 28, 2020 at 10:29:18AM +0530, Srinivasa Rao Mandadapu wrote:
>> This reverts part of commit b1824968221c
>> ("ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state")
>>
>> To identify LPAIF invalid state after device suspend and resume,
>> made I2S and DMA control registers not volatile, which is not necessary.
>> Instead invalid reg state can be handled with regcache APIs.
>> The BCLK ref count is necessary to enable clock only it's in disable state.
> Part of this commit message says that the problem was making the registers
> non-volatile but both the change and the rest of the commit message say
> that the issue was that the registers were made volatile.  I'm also
> still unclear as to what the issue is either way - how does reading the
> state of the registers from the hardware instead of the cache affect
> things?

Initial problem was, during playback if device suspended, I2S and DMA 
control registers

are getting reset and unable to recover playback after resume.

As these registers were non volatile registers, driver is not getting 
actual register value

and unable to report error state to application. Due to this application

keeps on polling for HW current pointer state and not exited from PCM 
running state.

To handle this scenario I made registers volatile and if they are in 
reset state, reported error

to application(commit b1824968221c).

Later from review comments by Srinivas kandagatla, I got to know

about regcache sync APIs, which can be used  to sync cache after resume and

HW registers can be updated with  original values. With that playback 
can be continued.

So is the reason, I am reverting partial changes in the commit b1824968221c.


> Please submit patches using subject lines reflecting the style for the
> subsystem, this makes it easier for people to identify relevant patches.
> Look at what existing commits in the area you're changing are doing and
> make sure your subject lines visually resemble what they're doing.
> There's no need to resubmit to fix this alone.

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


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

* Re: [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state
  2020-12-01 17:31     ` Srinivasa Rao Mandadapu
@ 2020-12-01 17:51       ` Mark Brown
  2020-12-14 12:43         ` Srinivasa Rao Mandadapu
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2020-12-01 17:51 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu
  Cc: agross, bjorn.andersson, lgirdwood, robh+dt, plai, bgoswami,
	perex, tiwai, srinivas.kandagatla, rohitkr, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel, V Sujith Kumar Reddy

[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]

On Tue, Dec 01, 2020 at 11:01:21PM +0530, Srinivasa Rao Mandadapu wrote:
> On 11/30/2020 6:16 PM, Mark Brown wrote:

> > Part of this commit message says that the problem was making the registers
> > non-volatile but both the change and the rest of the commit message say
> > that the issue was that the registers were made volatile.  I'm also
> > still unclear as to what the issue is either way - how does reading the
> > state of the registers from the hardware instead of the cache affect
> > things?

> Initial problem was, during playback if device suspended, I2S and DMA
> control registers

> are getting reset and unable to recover playback after resume.

> As these registers were non volatile registers, driver is not getting actual
> register value

> and unable to report error state to application. Due to this application

> keeps on polling for HW current pointer state and not exited from PCM
> running state.

> Later from review comments by Srinivas kandagatla, I got to know
> 
> about regcache sync APIs, which can be used  to sync cache after resume and
> 
> HW registers can be updated with  original values. With that playback can be
> continued.
> 
> So is the reason, I am reverting partial changes in the commit b1824968221c.

I don't understand why a fix for the register cache not being in sync
with the hardware doesn't involve syncing the register cache with the
hardware.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state
  2020-12-01 17:51       ` Mark Brown
@ 2020-12-14 12:43         ` Srinivasa Rao Mandadapu
  2020-12-14 17:50           ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: Srinivasa Rao Mandadapu @ 2020-12-14 12:43 UTC (permalink / raw)
  To: Mark Brown
  Cc: agross, bjorn.andersson, lgirdwood, robh+dt, plai, bgoswami,
	perex, tiwai, srinivas.kandagatla, rohitkr, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel, V Sujith Kumar Reddy

Thanks Mark for Your time!!!

On 12/1/2020 11:21 PM, Mark Brown wrote:
> On Tue, Dec 01, 2020 at 11:01:21PM +0530, Srinivasa Rao Mandadapu wrote:
>> On 11/30/2020 6:16 PM, Mark Brown wrote:
>>> Part of this commit message says that the problem was making the registers
>>> non-volatile but both the change and the rest of the commit message say
>>> that the issue was that the registers were made volatile.  I'm also
>>> still unclear as to what the issue is either way - how does reading the
>>> state of the registers from the hardware instead of the cache affect
>>> things?
>> Initial problem was, during playback if device suspended, I2S and DMA
>> control registers
>> are getting reset and unable to recover playback after resume.
>> As these registers were non volatile registers, driver is not getting actual
>> register value
>> and unable to report error state to application. Due to this application
>> keeps on polling for HW current pointer state and not exited from PCM
>> running state.
>> Later from review comments by Srinivas kandagatla, I got to know
>>
>> about regcache sync APIs, which can be used  to sync cache after resume and
>>
>> HW registers can be updated with  original values. With that playback can be
>> continued.
>>
>> So is the reason, I am reverting partial changes in the commit b1824968221c.
> I don't understand why a fix for the register cache not being in sync
> with the hardware doesn't involve syncing the register cache with the
> hardware.

I am sorry I couldn't understand your point. Could you please elaborate 
your query?

Actually I posted V5 version based on review comments.

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


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

* Re: [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state
  2020-12-14 12:43         ` Srinivasa Rao Mandadapu
@ 2020-12-14 17:50           ` Mark Brown
  2020-12-17  8:14             ` Srinivasa Rao Mandadapu
  2020-12-17  8:18             ` Srinivasa Rao Mandadapu
  0 siblings, 2 replies; 13+ messages in thread
From: Mark Brown @ 2020-12-14 17:50 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu
  Cc: agross, bjorn.andersson, lgirdwood, robh+dt, plai, bgoswami,
	perex, tiwai, srinivas.kandagatla, rohitkr, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel, V Sujith Kumar Reddy

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

On Mon, Dec 14, 2020 at 06:13:22PM +0530, Srinivasa Rao Mandadapu wrote:
> On 12/1/2020 11:21 PM, Mark Brown wrote:

> > > Later from review comments by Srinivas kandagatla, I got to know
> > > 
> > > about regcache sync APIs, which can be used  to sync cache after resume and
> > > 
> > > HW registers can be updated with  original values. With that playback can be
> > > continued.

> > > So is the reason, I am reverting partial changes in the commit b1824968221c.

> > I don't understand why a fix for the register cache not being in sync
> > with the hardware doesn't involve syncing the register cache with the
> > hardware.

> I am sorry I couldn't understand your point. Could you please elaborate your
> query?

Your changelog talks about syncing the cache but neither the driver nor
your change actually does that.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state
  2020-12-14 17:50           ` Mark Brown
@ 2020-12-17  8:14             ` Srinivasa Rao Mandadapu
  2020-12-17  8:18             ` Srinivasa Rao Mandadapu
  1 sibling, 0 replies; 13+ messages in thread
From: Srinivasa Rao Mandadapu @ 2020-12-17  8:14 UTC (permalink / raw)
  To: Mark Brown
  Cc: agross, bjorn.andersson, lgirdwood, robh+dt, plai, bgoswami,
	perex, tiwai, srinivas.kandagatla, rohitkr, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel, V Sujith Kumar Reddy

Thanks Mark for your Time!!!

On 12/14/2020 11:20 PM, Mark Brown wrote:
> On Mon, Dec 14, 2020 at 06:13:22PM +0530, Srinivasa Rao Mandadapu wrote:
>> On 12/1/2020 11:21 PM, Mark Brown wrote:
>>>> Later from review comments by Srinivas kandagatla, I got to know
>>>>
>>>> about regcache sync APIs, which can be used  to sync cache after resume and
>>>>
>>>> HW registers can be updated with  original values. With that playback can be
>>>> continued.
>>>> So is the reason, I am reverting partial changes in the commit b1824968221c.
>>> I don't understand why a fix for the register cache not being in sync
>>> with the hardware doesn't involve syncing the register cache with the
>>> hardware.
>> I am sorry I couldn't understand your point. Could you please elaborate your
>> query?
> Your changelog talks about syncing the cache but neither the driver nor
> your change actually does that.

Yeah.. Now I posted v6 patch 
(https://lore.kernel.org/patchwork/patch/1354638/)

with subject lines explaining actual changes.

could you please check  the same.

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


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

* Re: [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state
  2020-12-14 17:50           ` Mark Brown
  2020-12-17  8:14             ` Srinivasa Rao Mandadapu
@ 2020-12-17  8:18             ` Srinivasa Rao Mandadapu
  1 sibling, 0 replies; 13+ messages in thread
From: Srinivasa Rao Mandadapu @ 2020-12-17  8:18 UTC (permalink / raw)
  To: Mark Brown
  Cc: agross, bjorn.andersson, lgirdwood, robh+dt, plai, bgoswami,
	perex, tiwai, srinivas.kandagatla, rohitkr, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel, V Sujith Kumar Reddy

Thanks Mark for your Time!!!

On 12/14/2020 11:20 PM, Mark Brown wrote:
> On Mon, Dec 14, 2020 at 06:13:22PM +0530, Srinivasa Rao Mandadapu wrote:
>> On 12/1/2020 11:21 PM, Mark Brown wrote:
>>>> Later from review comments by Srinivas kandagatla, I got to know
>>>>
>>>> about regcache sync APIs, which can be used  to sync cache after resume and
>>>>
>>>> HW registers can be updated with  original values. With that playback can be
>>>> continued.
>>>> So is the reason, I am reverting partial changes in the commit b1824968221c.
>>> I don't understand why a fix for the register cache not being in sync
>>> with the hardware doesn't involve syncing the register cache with the
>>> hardware.
>> I am sorry I couldn't understand your point. Could you please elaborate your
>> query?
> Your changelog talks about syncing the cache but neither the driver nor
> your change actually does that.

Okay. Now I posted v6 patch 
(https://lore.kernel.org/patchwork/patch/1354638/)

with subject lines explaining actual changes.

could you please check  the same.

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


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

end of thread, other threads:[~2020-12-17  8:19 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-28  4:59 [PATCH v4 0/2] Platform driver update to support playback recover after resume Srinivasa Rao Mandadapu
2020-11-28  4:59 ` [PATCH v4 1/2] Partially revert ASoC: qcom: Fix enabling BCLK and LRCLK in LPAIF invalid state Srinivasa Rao Mandadapu
2020-11-30 11:09   ` Srinivas Kandagatla
2020-11-30 12:46   ` Mark Brown
2020-12-01 17:31     ` Srinivasa Rao Mandadapu
2020-12-01 17:51       ` Mark Brown
2020-12-14 12:43         ` Srinivasa Rao Mandadapu
2020-12-14 17:50           ` Mark Brown
2020-12-17  8:14             ` Srinivasa Rao Mandadapu
2020-12-17  8:18             ` Srinivasa Rao Mandadapu
2020-11-28  4:59 ` [PATCH v4 2/2] ASoC: qcom: Add support for playback recover after resume Srinivasa Rao Mandadapu
2020-11-30 11:19   ` Srinivas Kandagatla
2020-11-30 16:26   ` Steev Klimaszewski

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