[PATCH v4 0/6] Configure imx8mp dsp node for rproc usage

[PATCH v4 0/6] Configure imx8mp dsp node for rproc usage

[Daniel Baluta]

DSP found in i.MX8MP SOC can be used by multiple frameworks in order to
enable various applications:
        - rproc/rpmsg framework, used to load for example Zephyr samples
        - Sound Open Firmware, used to enable various audio processing
          pipelines.

Current dsp node was configured with SOF in mind but it doesn't work
well with imx8mp-evk dts. SOF controls audio IPs from firmware side
while imx8mp-evk.dts preffers to control audio IPs from Linux side.

So, configure 'dsp' node to be used with rproc scenario and later will
add a separate dts or an overlay to configure the node for SOF.

This patch series configures and enables dsp node to be used with rproc.

Changes since v3:
	- clarify controversy on the need of abstracting the AudioMix
          as a reset controlle for DSP. This is not the case! Audiomix is used only for
          Reset/Stall and it doesn't have the capability to reset the DSP. Reset
          is done via a separate interface (DAP).
	- Removed duplicate clock in patch 3/6
        - Add R-b tags.

Changes since v2:
        - Fix dts example in patch 4/6

Changes since v1:
        - document syscon compatible as pointed by Peng Fan
        - do not disable dsp_reserved node in the dtsi file as pointed
          by Ahmad Fatoum.


Daniel Baluta (6):
  arm64: dts: imx8mp: Add mu2 root clock
  arm64: dts: imx8mp: Configure dsp node for rproc usage
  arm64: dts: imx8mp: Add DSP clocks
  dt-bindings: clock: imx8mp: Add syscon compatible
  arm64: dts: imx8mp: Add fsl,dsp-ctrl property for dsp
  arm64: dts: Add dsp rproc related mem regions

 .../bindings/clock/imx8mp-audiomix.yaml       |  6 ++++--
 arch/arm64/boot/dts/freescale/imx8mp-evk.dts  | 10 +++++++++
 arch/arm64/boot/dts/freescale/imx8mp.dtsi     | 21 ++++++++++++-------
 3 files changed, 27 insertions(+), 10 deletions(-)

-- 
2.43.0

[PATCH v4 1/6] arm64: dts: imx8mp: Add mu2 root clock

[Daniel Baluta]

Enable MU2 node and add mu2 root clock.
MU2 is used to communicate with DSP core.

Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index e0d3b8cba221..00924e0836db 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -1252,7 +1252,7 @@ mu2: mailbox@30e60000 {
 				reg = <0x30e60000 0x10000>;
 				interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
 				#mbox-cells = <2>;
-				status = "disabled";
+				clocks = <&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_MU2_ROOT>;
 			};
 
 			i2c5: i2c@30ad0000 {
-- 
2.43.0

[PATCH v4 2/6] arm64: dts: imx8mp: Configure dsp node for rproc usage

[Daniel Baluta]

DSP can be used with various frameworks (e.g audio firmware, rproc).
Currently 'dsp' configuration is intended for audio firmware but it
doesn't work well with board level DTs (e.g imx8mp-evk) because
board level DT enables audio related IPs (e.g SAI) while audio firmware
needs this IPs disabled (because firmware will configure them).

So, configure 'dsp' node to be used with rproc. This way users will be
able to use board DT to use the DSP as long as they don't clash with
Audio IP configurations.

More comples usage of 'dsp' node (e.g by audio firmware) will need to
create a separate dts file (or an overlay).

This change follows the approach taken for other i.MX8 boards
in commit 391a319c81f6d7 ("arm64: dts: imx8-ss-audio: configure dsp node
for rproc usage")

Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 00924e0836db..2f94c55e4999 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -280,7 +280,7 @@ reserved-memory {
 		ranges;
 
 		dsp_reserved: dsp@92400000 {
-			reg = <0 0x92400000 0 0x2000000>;
+			reg = <0 0x92400000 0 0x1000000>;
 			no-map;
 			status = "disabled";
 		};
@@ -2414,12 +2414,12 @@ usb_dwc3_1: usb@38200000 {
 		};
 
 		dsp: dsp@3b6e8000 {
-			compatible = "fsl,imx8mp-dsp";
+			compatible = "fsl,imx8mp-hifi4";
 			reg = <0x3b6e8000 0x88000>;
-			mbox-names = "txdb0", "txdb1",
-				"rxdb0", "rxdb1";
-			mboxes = <&mu2 2 0>, <&mu2 2 1>,
-				<&mu2 3 0>, <&mu2 3 1>;
+			power-domains = <&pgc_audio>;
+			mbox-names = "tx", "rx", "rxdb";
+			mboxes = <&mu2 0 0>, <&mu2 1 0>, <&mu2 3 0>;
+			firmware-name = "imx/dsp/hifi4.bin";
 			memory-region = <&dsp_reserved>;
 			status = "disabled";
 		};
-- 
2.43.0

[PATCH v4 3/6] arm64: dts: imx8mp: Add DSP clocks

[Daniel Baluta]

DSP core needs ocram, core and debug clocks.

Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 2f94c55e4999..14cfbd228b45 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -2416,6 +2416,10 @@ usb_dwc3_1: usb@38200000 {
 		dsp: dsp@3b6e8000 {
 			compatible = "fsl,imx8mp-hifi4";
 			reg = <0x3b6e8000 0x88000>;
+			clocks = <&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_OCRAMA_IPG>,
+				 <&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_DSP_ROOT>,
+				 <&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_DSPDBG_ROOT>;
+			clock-names = "ocram", "core", "debug";
 			power-domains = <&pgc_audio>;
 			mbox-names = "tx", "rx", "rxdb";
 			mboxes = <&mu2 0 0>, <&mu2 1 0>, <&mu2 3 0>;
-- 
2.43.0

[PATCH v4 4/6] dt-bindings: clock: imx8mp: Add syscon compatible

[Daniel Baluta]

imx8mp audiomix contains a set of registers used to control
the DSP.

The dsp will use this to acquire o reference to audiomix registers
and handle the registers to control the dsp.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 .../devicetree/bindings/clock/imx8mp-audiomix.yaml          | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/imx8mp-audiomix.yaml b/Documentation/devicetree/bindings/clock/imx8mp-audiomix.yaml
index 6588a17a7d9a..0f870aaadba2 100644
--- a/Documentation/devicetree/bindings/clock/imx8mp-audiomix.yaml
+++ b/Documentation/devicetree/bindings/clock/imx8mp-audiomix.yaml
@@ -15,7 +15,9 @@ description: |
 
 properties:
   compatible:
-    const: fsl,imx8mp-audio-blk-ctrl
+    items:
+      - const: fsl,imx8mp-audio-blk-ctrl
+      - const: syscon
 
   reg:
     maxItems: 1
@@ -63,7 +65,7 @@ examples:
     #include <dt-bindings/clock/imx8mp-clock.h>
 
     clock-controller@30e20000 {
-        compatible = "fsl,imx8mp-audio-blk-ctrl";
+        compatible = "fsl,imx8mp-audio-blk-ctrl", "syscon";
         reg = <0x30e20000 0x10000>;
         #clock-cells = <1>;
         clocks = <&clk IMX8MP_CLK_AUDIO_ROOT>,
-- 
2.43.0

[PATCH v4 5/6] arm64: dts: imx8mp: Add fsl,dsp-ctrl property for dsp

[Daniel Baluta]

Audio block control contains a set of registers and some of them used for
DSP configuration.

Drivers (rproc, SOF) are using fsl,dsp-ctrl property in order to control
the DSP, particularly for Run/Stall bit.

Note that audio block control doesn't offer the functionality to reset
thte DSP. Reset is done via DAP interface.

Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 14cfbd228b45..46b33fbb9bd1 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -1609,7 +1609,7 @@ sdma2: dma-controller@30e10000 {
 			};
 
 			audio_blk_ctrl: clock-controller@30e20000 {
-				compatible = "fsl,imx8mp-audio-blk-ctrl";
+				compatible = "fsl,imx8mp-audio-blk-ctrl", "syscon";
 				reg = <0x30e20000 0x10000>;
 				#clock-cells = <1>;
 				#reset-cells = <1>;
@@ -2425,6 +2425,7 @@ dsp: dsp@3b6e8000 {
 			mboxes = <&mu2 0 0>, <&mu2 1 0>, <&mu2 3 0>;
 			firmware-name = "imx/dsp/hifi4.bin";
 			memory-region = <&dsp_reserved>;
+			fsl,dsp-ctrl = <&audio_blk_ctrl>;
 			status = "disabled";
 		};
 	};
-- 
2.43.0

Re: [PATCH v4 5/6] arm64: dts: imx8mp: Add fsl,dsp-ctrl property for dsp

[Frank Li]

On Wed, Feb 12, 2025 at 10:52:21AM +0200, Daniel Baluta wrote:
> Audio block control contains a set of registers and some of them used for
> DSP configuration.
>
> Drivers (rproc, SOF) are using fsl,dsp-ctrl property in order to control
> the DSP, particularly for Run/Stall bit.
>
> Note that audio block control doesn't offer the functionality to reset
> thte DSP. Reset is done via DAP interface.
>
> Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 14cfbd228b45..46b33fbb9bd1 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -1609,7 +1609,7 @@ sdma2: dma-controller@30e10000 {
>  			};
>
>  			audio_blk_ctrl: clock-controller@30e20000 {
> -				compatible = "fsl,imx8mp-audio-blk-ctrl";
> +				compatible = "fsl,imx8mp-audio-blk-ctrl", "syscon";
>  				reg = <0x30e20000 0x10000>;
>  				#clock-cells = <1>;
>  				#reset-cells = <1>;
> @@ -2425,6 +2425,7 @@ dsp: dsp@3b6e8000 {
>  			mboxes = <&mu2 0 0>, <&mu2 1 0>, <&mu2 3 0>;
>  			firmware-name = "imx/dsp/hifi4.bin";
>  			memory-region = <&dsp_reserved>;
> +			fsl,dsp-ctrl = <&audio_blk_ctrl>;

I think kk's comments in v3

"This should have been implemented as reset controller, not syscon. It's
really poor choice to call everything syscon. It does not scale, does
not provide you runtime PM or probe ordering (device links). Quick look
at your drivers suggest that you have exacvtly that problems."

The above is quite good suggestion.

Your reply "But for Run/Stall bits we need to use a syscon.",

Run/Stall actually is reset, Most system, release reset signal means dsp/core
RUN.

Frank

>  			status = "disabled";
>  		};
>  	};
> --
> 2.43.0
>

Re: [PATCH v4 5/6] arm64: dts: imx8mp: Add fsl,dsp-ctrl property for dsp

[Laurentiu Mihalcea]

On 2/12/2025 6:02 PM, Frank Li wrote:
> On Wed, Feb 12, 2025 at 10:52:21AM +0200, Daniel Baluta wrote:
>> Audio block control contains a set of registers and some of them used for
>> DSP configuration.
>>
>> Drivers (rproc, SOF) are using fsl,dsp-ctrl property in order to control
>> the DSP, particularly for Run/Stall bit.
>>
>> Note that audio block control doesn't offer the functionality to reset
>> thte DSP. Reset is done via DAP interface.
>>
>> Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>
>> Reviewed-by: Peng Fan <peng.fan@nxp.com>
>> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
>> ---
>>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>> index 14cfbd228b45..46b33fbb9bd1 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>> @@ -1609,7 +1609,7 @@ sdma2: dma-controller@30e10000 {
>>  			};
>>
>>  			audio_blk_ctrl: clock-controller@30e20000 {
>> -				compatible = "fsl,imx8mp-audio-blk-ctrl";
>> +				compatible = "fsl,imx8mp-audio-blk-ctrl", "syscon";
>>  				reg = <0x30e20000 0x10000>;
>>  				#clock-cells = <1>;
>>  				#reset-cells = <1>;
>> @@ -2425,6 +2425,7 @@ dsp: dsp@3b6e8000 {
>>  			mboxes = <&mu2 0 0>, <&mu2 1 0>, <&mu2 3 0>;
>>  			firmware-name = "imx/dsp/hifi4.bin";
>>  			memory-region = <&dsp_reserved>;
>> +			fsl,dsp-ctrl = <&audio_blk_ctrl>;
> I think kk's comments in v3
>
> "This should have been implemented as reset controller, not syscon. It's
> really poor choice to call everything syscon. It does not scale, does
> not provide you runtime PM or probe ordering (device links). Quick look
> at your drivers suggest that you have exacvtly that problems."
>
> The above is quite good suggestion.
>
> Your reply "But for Run/Stall bits we need to use a syscon.",
>
> Run/Stall actually is reset, Most system, release reset signal means dsp/core
> RUN.
>
> Frank

RESET and STALL are quite different signals w/ different purposes so
calling them both RESET is confusing and inaccurate.

The syscon is used here to control the STALL signal (name of the bit we're using is RunStall)
and has nothing to do with the RESET signal, which is why it's implemented as a syscon rather
than a reset controller.

While Krzysztof's comment does make sense, I still find it odd to have this implemented as a
reset controller despite it not having anything to do with the RESET signal.

Also, do note that the two nodes are in clock provider-consumer relationship
so unless I'm gravely mistaken this should at least guarantee the probe order.

Re: [PATCH v4 5/6] arm64: dts: imx8mp: Add fsl,dsp-ctrl property for dsp

[Frank Li]

On Wed, Feb 12, 2025 at 09:04:47PM +0200, Laurentiu Mihalcea wrote:
> On 2/12/2025 6:02 PM, Frank Li wrote:
> > On Wed, Feb 12, 2025 at 10:52:21AM +0200, Daniel Baluta wrote:
> >> Audio block control contains a set of registers and some of them used for
> >> DSP configuration.
> >>
> >> Drivers (rproc, SOF) are using fsl,dsp-ctrl property in order to control
> >> the DSP, particularly for Run/Stall bit.
> >>
> >> Note that audio block control doesn't offer the functionality to reset
> >> thte DSP. Reset is done via DAP interface.
> >>
> >> Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>
> >> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> >> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> >> ---
> >>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> >> index 14cfbd228b45..46b33fbb9bd1 100644
> >> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> >> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> >> @@ -1609,7 +1609,7 @@ sdma2: dma-controller@30e10000 {
> >>  			};
> >>
> >>  			audio_blk_ctrl: clock-controller@30e20000 {
> >> -				compatible = "fsl,imx8mp-audio-blk-ctrl";
> >> +				compatible = "fsl,imx8mp-audio-blk-ctrl", "syscon";
> >>  				reg = <0x30e20000 0x10000>;
> >>  				#clock-cells = <1>;
> >>  				#reset-cells = <1>;
> >> @@ -2425,6 +2425,7 @@ dsp: dsp@3b6e8000 {
> >>  			mboxes = <&mu2 0 0>, <&mu2 1 0>, <&mu2 3 0>;
> >>  			firmware-name = "imx/dsp/hifi4.bin";
> >>  			memory-region = <&dsp_reserved>;
> >> +			fsl,dsp-ctrl = <&audio_blk_ctrl>;
> > I think kk's comments in v3
> >
> > "This should have been implemented as reset controller, not syscon. It's
> > really poor choice to call everything syscon. It does not scale, does
> > not provide you runtime PM or probe ordering (device links). Quick look
> > at your drivers suggest that you have exacvtly that problems."
> >
> > The above is quite good suggestion.
> >
> > Your reply "But for Run/Stall bits we need to use a syscon.",
> >
> > Run/Stall actually is reset, Most system, release reset signal means dsp/core
> > RUN.
> >
> > Frank
>
> RESET and STALL are quite different signals w/ different purposes so
> calling them both RESET is confusing and inaccurate.
>
> The syscon is used here to control the STALL signal (name of the bit we're using is RunStall)
> and has nothing to do with the RESET signal, which is why it's implemented as a syscon rather
> than a reset controller.
>
> While Krzysztof's comment does make sense, I still find it odd to have this implemented as a
> reset controller despite it not having anything to do with the RESET signal.

Regardless hardware signal name, the logic function is that release a
signal and let DSP core running. So we still model it as reset-controllers.

Did you actaully pause/resume DSP running at your drivers?

Frank

>
> Also, do note that the two nodes are in clock provider-consumer relationship
> so unless I'm gravely mistaken this should at least guarantee the probe order.

[PATCH v4 6/6] arm64: dts: Add dsp rproc related mem regions

[Daniel Baluta]

With imx8mp-evk board we are now configuring 'dsp' node for rproc usage,
so add rproc specific memory regions.

Also, enable dsp node because it is ready to be used.

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index 68e12a752edd..468bbb84ceaf 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -291,6 +291,16 @@ &aud2htx {
 	status = "okay";
 };
 
+&dsp_reserved {
+	status = "okay";
+};
+
+&dsp {
+	memory-region = <&dsp_vdev0buffer>, <&dsp_vdev0vring0>,
+			<&dsp_vdev0vring1>, <&dsp_reserved>;
+	status = "okay";
+};
+
 &eqos {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_eqos>;
-- 
2.43.0