[PATCH v2 1/2] ASoC: dt-bindings: fsl_rpmsg: List DAPM endpoints ignoring system suspend

[PATCH v2 1/2] ASoC: dt-bindings: fsl_rpmsg: List DAPM endpoints ignoring system suspend

[Chancel Liu]

Add a property to list DAPM endpoints which mark paths between these
endpoints should not be disabled when system enters in suspend state.

LPA means low power audio case. On asymmetric multiprocessor, there are
Cortex-A core and Cortex-M core, Linux is running on Cortex-A core,
RTOS or other OS is running on Cortex-M core. The audio hardware
devices can be controlled by Cortex-M. LPA can be explained as a
mechanism that Cortex-A allocates a large buffer and fill audio data,
then Cortex-A can enter into suspend for the purpose of power saving.
Cortex-M continues to play the sound during suspend phase of Cortex-A.
When the data in buffer is consumed, Cortex-M will trigger the Cortex-A
to wakeup to fill data. LPA requires some audio paths still enabled
when Cortex-A enters into suspend.

Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
---
 .../devicetree/bindings/sound/fsl,rpmsg.yaml      | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
index 188f38baddec..d8fd17615bf2 100644
--- a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
@@ -91,6 +91,21 @@ properties:
       - rpmsg-audio-channel
       - rpmsg-micfil-channel
 
+  fsl,lpa-widgets:
+    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+    description: |
+      A list of DAPM endpoints which mark paths between these endpoints should
+      not be disabled when system enters in suspend state. LPA means low power
+      audio case. On asymmetric multiprocessor, there are Cortex-A core and
+      Cortex-M core, Linux is running on Cortex-A core, RTOS or other OS is
+      running on Cortex-M core. The audio hardware devices can be controlled by
+      Cortex-M. LPA can be explained as a mechanism that Cortex-A allocates a
+      large buffer and fill audio data, then Cortex-A can enter into suspend
+      for the purpose of power saving. Cortex-M continues to play the sound
+      during suspend phase of Cortex-A. When the data in buffer is consumed,
+      Cortex-M will trigger the Cortex-A to wakeup to fill data. LPA requires
+      some audio paths still enabled when Cortex-A enters into suspend.
+
 required:
   - compatible
 
-- 
2.25.1

[PATCH v2 2/2] ASoC: imx-rpmsg: Force codec power on in low power audio mode

[Chancel Liu]

Low power audio mode requires binding codec still power on while Acore
enters into suspend so Mcore can continue playback music.

ASoC machine driver acquires DAPM endpoints through reading
"fsl,lpa-widgets" property from DT and then forces the path between
these endpoints ignoring suspend.

If the rpmsg sound card is in low power audio mode, the suspend/resume
callback of binding codec is overridden to disable the suspend/resume.

Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
---
 sound/soc/fsl/imx-rpmsg.c | 58 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/sound/soc/fsl/imx-rpmsg.c b/sound/soc/fsl/imx-rpmsg.c
index b578f9a32d7f..0568a3420aae 100644
--- a/sound/soc/fsl/imx-rpmsg.c
+++ b/sound/soc/fsl/imx-rpmsg.c
@@ -20,8 +20,11 @@ struct imx_rpmsg {
 	struct snd_soc_dai_link dai;
 	struct snd_soc_card card;
 	unsigned long sysclk;
+	bool lpa;
 };
 
+static struct dev_pm_ops lpa_pm;
+
 static const struct snd_soc_dapm_widget imx_rpmsg_dapm_widgets[] = {
 	SND_SOC_DAPM_HP("Headphone Jack", NULL),
 	SND_SOC_DAPM_SPK("Ext Spk", NULL),
@@ -38,6 +41,58 @@ static int imx_rpmsg_late_probe(struct snd_soc_card *card)
 	struct device *dev = card->dev;
 	int ret;
 
+	if (data->lpa) {
+		struct snd_soc_component *codec_comp;
+		struct device_node *codec_np;
+		struct device_driver *codec_drv;
+		struct device *codec_dev = NULL;
+
+		codec_np = data->dai.codecs->of_node;
+		if (codec_np) {
+			struct platform_device *codec_pdev;
+			struct i2c_client *codec_i2c;
+
+			codec_i2c = of_find_i2c_device_by_node(codec_np);
+			if (codec_i2c)
+				codec_dev = &codec_i2c->dev;
+			if (!codec_dev) {
+				codec_pdev = of_find_device_by_node(codec_np);
+				if (codec_pdev)
+					codec_dev = &codec_pdev->dev;
+			}
+		}
+		if (codec_dev) {
+			codec_comp = snd_soc_lookup_component_nolocked(codec_dev, NULL);
+			if (codec_comp) {
+				int i, num_widgets;
+				const char *widgets;
+				struct snd_soc_dapm_context *dapm;
+
+				num_widgets = of_property_count_strings(data->card.dev->of_node,
+									"fsl,lpa-widgets");
+				for (i = 0; i < num_widgets; i++) {
+					of_property_read_string_index(data->card.dev->of_node,
+								      "fsl,lpa-widgets",
+								      i, &widgets);
+					dapm = snd_soc_component_get_dapm(codec_comp);
+					snd_soc_dapm_ignore_suspend(dapm, widgets);
+				}
+			}
+			codec_drv = codec_dev->driver;
+			if (codec_drv->pm) {
+				memcpy(&lpa_pm, codec_drv->pm, sizeof(lpa_pm));
+				lpa_pm.suspend = NULL;
+				lpa_pm.resume = NULL;
+				lpa_pm.freeze = NULL;
+				lpa_pm.thaw = NULL;
+				lpa_pm.poweroff = NULL;
+				lpa_pm.restore = NULL;
+				codec_drv->pm = &lpa_pm;
+			}
+			put_device(codec_dev);
+		}
+	}
+
 	if (!data->sysclk)
 		return 0;
 
@@ -137,6 +192,9 @@ static int imx_rpmsg_probe(struct platform_device *pdev)
 		goto fail;
 	}
 
+	if (of_property_read_bool(np, "fsl,enable-lpa"))
+		data->lpa = true;
+
 	data->card.dev = &pdev->dev;
 	data->card.owner = THIS_MODULE;
 	data->card.dapm_widgets = imx_rpmsg_dapm_widgets;
-- 
2.25.1

Re: [PATCH v2 1/2] ASoC: dt-bindings: fsl_rpmsg: List DAPM endpoints ignoring system suspend

[Mark Brown]

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

On Mon, Sep 25, 2023 at 07:09:45PM +0800, Chancel Liu wrote:

> +  fsl,lpa-widgets:
> +    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> +    description: |
> +      A list of DAPM endpoints which mark paths between these endpoints should
> +      not be disabled when system enters in suspend state. LPA means low power
> +      audio case. On asymmetric multiprocessor, there are Cortex-A core and
> +      Cortex-M core, Linux is running on Cortex-A core, RTOS or other OS is
> +      running on Cortex-M core. The audio hardware devices can be controlled by
> +      Cortex-M. LPA can be explained as a mechanism that Cortex-A allocates a
> +      large buffer and fill audio data, then Cortex-A can enter into suspend
> +      for the purpose of power saving. Cortex-M continues to play the sound
> +      during suspend phase of Cortex-A. When the data in buffer is consumed,
> +      Cortex-M will trigger the Cortex-A to wakeup to fill data. LPA requires
> +      some audio paths still enabled when Cortex-A enters into suspend.

This is a fairly standard DSP playback case as far as I can see so it
should work with DAPM without needing this obviously use case specific
stuff peering into the Linux implementation.  Generally this is done by
tagging endpoint widgets and DAIs as ignore_suspend, DAPM will then
figure out the rest of the widgets in the path.

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

RE: Re: [PATCH v2 1/2] ASoC: dt-bindings: fsl_rpmsg: List DAPM endpoints ignoring system suspend

[Chancel Liu]

> > +  fsl,lpa-widgets:
> > +    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> > +    description: |
> > +      A list of DAPM endpoints which mark paths between these
> endpoints should
> > +      not be disabled when system enters in suspend state. LPA means low
power
> > +      audio case. On asymmetric multiprocessor, there are Cortex-A core
> and
> > +      Cortex-M core, Linux is running on Cortex-A core, RTOS or other OS is
> > +      running on Cortex-M core. The audio hardware devices can be
> controlled by
> > +      Cortex-M. LPA can be explained as a mechanism that Cortex-A
> allocates a
> > +      large buffer and fill audio data, then Cortex-A can enter into suspend
> > +      for the purpose of power saving. Cortex-M continues to play the
> sound
> > +      during suspend phase of Cortex-A. When the data in buffer is
> consumed,
> > +      Cortex-M will trigger the Cortex-A to wakeup to fill data. LPA
> requires
> > +      some audio paths still enabled when Cortex-A enters into suspend.
> 
> This is a fairly standard DSP playback case as far as I can see so it
> should work with DAPM without needing this obviously use case specific
> stuff peering into the Linux implementation.  Generally this is done by
> tagging endpoint widgets and DAIs as ignore_suspend, DAPM will then
> figure out the rest of the widgets in the path.

Yes, indeed I meant to let driver get DAPM endpoints from the "fsl,lpa-widgets"
property and then set these endpoints as ignore_suspend if the sound card is
running in this use case. Do you think the description for the use case can be
simplified since it's a common use case?

Regards, 
Chancel Liu

Re: Re: [PATCH v2 1/2] ASoC: dt-bindings: fsl_rpmsg: List DAPM endpoints ignoring system suspend

[Mark Brown]

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

On Wed, Sep 27, 2023 at 03:07:49AM +0000, Chancel Liu wrote:

> > This is a fairly standard DSP playback case as far as I can see so it
> > should work with DAPM without needing this obviously use case specific
> > stuff peering into the Linux implementation.  Generally this is done by
> > tagging endpoint widgets and DAIs as ignore_suspend, DAPM will then
> > figure out the rest of the widgets in the path.

> Yes, indeed I meant to let driver get DAPM endpoints from the "fsl,lpa-widgets"
> property and then set these endpoints as ignore_suspend if the sound card is
> running in this use case. Do you think the description for the use case can be
> simplified since it's a common use case?

This is a card problem - the driver for rpmsg shouldn't need to worry
about it.  Any bindings extensions should be at the card level rather
than in a specific driver, we could probably use something in the
generic properties that they're all using.

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

RE: [EXT] Re: Re: [PATCH v2 1/2] ASoC: dt-bindings: fsl_rpmsg: List DAPM endpoints ignoring system suspend

[Chancel Liu]

> > > This is a fairly standard DSP playback case as far as I can see so
> > > it should work with DAPM without needing this obviously use case
> > > specific stuff peering into the Linux implementation.  Generally
> > > this is done by tagging endpoint widgets and DAIs as ignore_suspend,
> > > DAPM will then figure out the rest of the widgets in the path.
> 
> > Yes, indeed I meant to let driver get DAPM endpoints from the
> "fsl,lpa-widgets"
> > property and then set these endpoints as ignore_suspend if the sound
> > card is running in this use case. Do you think the description for the
> > use case can be simplified since it's a common use case?
> 
> This is a card problem - the driver for rpmsg shouldn't need to worry about it.
> Any bindings extensions should be at the card level rather than in a specific
> driver, we could probably use something in the generic properties that they're
> all using.

Instead of "fsl,lpa-widgets", I would like to add a common property 
"ignore-suspend-widgets" in sound-card-common.yaml file. So not only rpmsg
sound cards but also other sound cards which have such feature can use this
property to define wanted widgets ignoring suspend.
What do you think about that?

Regards, 
Chancel Liu

Re: [EXT] Re: Re: [PATCH v2 1/2] ASoC: dt-bindings: fsl_rpmsg: List DAPM endpoints ignoring system suspend

[Mark Brown]

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

On Sat, Oct 07, 2023 at 11:13:49AM +0000, Chancel Liu wrote:

> Instead of "fsl,lpa-widgets", I would like to add a common property 
> "ignore-suspend-widgets" in sound-card-common.yaml file. So not only rpmsg
> sound cards but also other sound cards which have such feature can use this
> property to define wanted widgets ignoring suspend.
> What do you think about that?

We can perhaps bikeshed the name a bit to be more focused on the use
case but yes, that sounds reasonable.

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