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

[PATCH v3 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 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 v1:
        - document syscon compatible as pointed by Peng Fan
        - do not disable dsp_reserved node in the dtsi file as pointed
          by Ahmad Fatoum.
Changes since v2:
        - Fix dts example in patch 4/6
        - Frank Li commented that we should handle the reset using the reset
        interface. I have two objections on this: we are using some bits from audiomix
        for reset together some other bits from DAP region. Also, the implementation is already upstream!
        Frank Li agreed that it's up to Rob opinion on this.

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     | 22 ++++++++++++-------
 3 files changed, 28 insertions(+), 10 deletions(-)

-- 
2.25.1

[PATCH v3 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.

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

[PATCH v3 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")

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

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

[Daniel Baluta]

DSP core needs ocram, core and debug clocks.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 2f94c55e4999..b42e6430123f 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -2416,6 +2416,11 @@ 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_OCRAMA_IPG>,
+				<&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_DSP_ROOT>,
+				<&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_DSPDBG_ROOT>;
+			clock-names = "ipg", "ocram", "core", "debug";
 			power-domains = <&pgc_audio>;
 			mbox-names = "tx", "rx", "rxdb";
 			mboxes = <&mu2 0 0>, <&mu2 1 0>, <&mu2 3 0>;
-- 
2.25.1

[PATCH v3 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.

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

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

[Daniel Baluta]

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

Drivers (rproc, SOF) are using fsl,dsp-ctrl property to access those
registers in order to control the operations of the DSP (e.g for
start, reset, etc).

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 b42e6430123f..c8f9347d602c 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>;
@@ -2426,6 +2426,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.25.1

[PATCH v3 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.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 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 d26930f1a9e9..c732ee79772d 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.25.1

RE: [PATCH v3 0/6] Configure imx8mp dsp node for rproc usage

[Peng Fan]

> Subject: [PATCH v3 0/6] Configure imx8mp dsp node for rproc usage
> 
> 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 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 v1:
>         - document syscon compatible as pointed by Peng Fan
>         - do not disable dsp_reserved node in the dtsi file as pointed
>           by Ahmad Fatoum.
> Changes since v2:
>         - Fix dts example in patch 4/6
>         - Frank Li commented that we should handle the reset using the
> reset
>         interface. I have two objections on this: we are using some bits
> from audiomix
>         for reset together some other bits from DAP region. Also, the
> implementation is already upstream!
>         Frank Li agreed that it's up to Rob opinion on this.
> 
> 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     | 22 ++++++++++++------
> -
>  3 files changed, 28 insertions(+), 10 deletions(-)
> 

Patchset looks good to me:

Reviewed-by: Peng Fan <peng.fan@nxp.com>

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

[Krzysztof Kozlowski]

On Tue, Dec 10, 2024 at 02:53:37PM +0200, Daniel Baluta wrote:
> Audio block control contains a set of registers some of them used for
> DSP configuration.
> 
> Drivers (rproc, SOF) are using fsl,dsp-ctrl property to access those
> registers in order to control the operations of the DSP (e.g for
> start, reset, etc).

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.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On Tue, Dec 10, 2024 at 02:53:36PM +0200, Daniel Baluta wrote:
> 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>

Best regards,
Krzysztof

Re: [PATCH v3 0/6] Configure imx8mp dsp node for rproc usage

[Iuliana Prodan]

On 12/10/2024 2:53 PM, Daniel Baluta wrote:
> 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 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 v1:
>          - document syscon compatible as pointed by Peng Fan
>          - do not disable dsp_reserved node in the dtsi file as pointed
>            by Ahmad Fatoum.
> Changes since v2:
>          - Fix dts example in patch 4/6
>          - Frank Li commented that we should handle the reset using the reset
>          interface. I have two objections on this: we are using some bits from audiomix
>          for reset together some other bits from DAP region. Also, the implementation is already upstream!
>          Frank Li agreed that it's up to Rob opinion on this.
>
> 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
For the series:

Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>

Thanks,
Iulia

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

[Daniel Baluta]

On Wed, Dec 11, 2024 at 11:07 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Tue, Dec 10, 2024 at 02:53:37PM +0200, Daniel Baluta wrote:
> > Audio block control contains a set of registers some of them used for
> > DSP configuration.
> >
> > Drivers (rproc, SOF) are using fsl,dsp-ctrl property to access those
> > registers in order to control the operations of the DSP (e.g for
> > start, reset, etc).
>
> 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.

Hi Krzysztof,

The bits we are using from audio-blck-ctrl for controlling the DSP are
not for reset!
They are only for Run/Stall the DSP.

The reset is done via DAP interface. I will sent a separate patch to abstract
the reset interaction using a reset controller. But for Run/Stall bits we need
to use a syscon.

thanks,
Daniel.