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

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

[Chancel Liu]

Add a property to list DAPM endpoints which mark paths between these
endpoints ignoring suspend. These DAPM paths can still be power on when
system enters into suspend.

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

diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
index 188f38baddec..ec6e09eab427 100644
--- a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
@@ -91,6 +91,12 @@ 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
+      ignoring suspend.
+
 required:
   - compatible
 
-- 
2.25.1

[PATCH 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 1/2] ASoC: dt-bindings: fsl_rpmsg: List DAPM endpoints ignoring suspend

[Krzysztof Kozlowski]

On 25/09/2023 04:48, Chancel Liu wrote:
> Add a property to list DAPM endpoints which mark paths between these
> endpoints ignoring suspend. These DAPM paths can still be power on when
> system enters into suspend.
> 
> Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
> ---
>  Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> index 188f38baddec..ec6e09eab427 100644
> --- a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> +++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> @@ -91,6 +91,12 @@ properties:
>        - rpmsg-audio-channel
>        - rpmsg-micfil-channel
>  
> +  fsl,lpa-widgets:

What is LPA? It's not explained in property description.

> +    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> +    description: |
> +      A list of DAPM endpoints which mark paths between these endpoints
> +      ignoring suspend.

And how does it differ from audio-routing? Also, you need to explain
what is "suspend" in this context. Bindings are independent of Linux.

Best regards,
Krzysztof

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

[Chancel Liu]

> > Add a property to list DAPM endpoints which mark paths between these
> > endpoints ignoring suspend. These DAPM paths can still be power on
> > when system enters into suspend.
> >
> > Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > index 188f38baddec..ec6e09eab427 100644
> > --- a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > +++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > @@ -91,6 +91,12 @@ properties:
> >        - rpmsg-audio-channel
> >        - rpmsg-micfil-channel
> >
> > +  fsl,lpa-widgets:
> 
> What is LPA? It's not explained in property description.
> 

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 means low power audio
case. The mechanism can be explained 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.

I can add above explanation to LPA in patch v2.

> > +    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> > +    description: |
> > +      A list of DAPM endpoints which mark paths between these endpoints
> > +      ignoring suspend.
> 
> And how does it differ from audio-routing? Also, you need to explain what is
> "suspend" in this context. Bindings are independent of Linux.
> 

Normally audio paths will be disabled by ASoC dynamic audio power management if
Linux enters into suspend. LPA requires some audio paths enabled when Cortex-A
enters into suspend. We can read DAPM endpoints from the "fsl,lpa-widgets"
property and keep the paths between these endpoints enabled during suspend
phase of Cortex-A. Property "audio-routing" just declares the connection
between widgets and doesn't have such feature.

I will modify the description as following:
"A list of DAPM endpoints which mark paths between these endpoints still enabled
when system enters into suspend."

> Best regards,
> Krzysztof

Regards, 
Chancel Liu

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

[Krzysztof Kozlowski]

On 25/09/2023 10:20, Chancel Liu wrote:
>>> Add a property to list DAPM endpoints which mark paths between these
>>> endpoints ignoring suspend. These DAPM paths can still be power on
>>> when system enters into suspend.
>>>
>>> Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
>>> ---
>>>  Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
>>> b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
>>> index 188f38baddec..ec6e09eab427 100644
>>> --- a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
>>> +++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
>>> @@ -91,6 +91,12 @@ properties:
>>>        - rpmsg-audio-channel
>>>        - rpmsg-micfil-channel
>>>
>>> +  fsl,lpa-widgets:
>>
>> What is LPA? It's not explained in property description.
>>
> 
> 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 means low power audio
> case. The mechanism can be explained 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.
> 
> I can add above explanation to LPA in patch v2.
> 
>>> +    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>>> +    description: |
>>> +      A list of DAPM endpoints which mark paths between these endpoints
>>> +      ignoring suspend.
>>
>> And how does it differ from audio-routing? Also, you need to explain what is
>> "suspend" in this context. Bindings are independent of Linux.
>>
> 
> Normally audio paths will be disabled by ASoC dynamic audio power management if
> Linux enters into suspend. LPA requires some audio paths enabled when Cortex-A
> enters into suspend. We can read DAPM endpoints from the "fsl,lpa-widgets"
> property and keep the paths between these endpoints enabled during suspend
> phase of Cortex-A. Property "audio-routing" just declares the connection
> between widgets and doesn't have such feature.
> 
> I will modify the description as following:
> "A list of DAPM endpoints which mark paths between these endpoints still enabled
> when system enters into suspend."

Yes, that's better, but even better would be to say not how the OS
should behave, but how the actual entire system works. Basically these
widgets remain in use by your co-processor, thus OS should not disable
them when entering in system suspend state.

Best regards,
Krzysztof