[PATCH 0/4] TI K3 DSP remoteproc fixes/enhancements

[PATCH 0/4] TI K3 DSP remoteproc fixes/enhancements

[Suman Anna]

Hi All,

The following series updates the K3 DSP remoteproc driver to fix couple
of issues, and add the internal memory loading support on C66x DSPs.
Patches are on top of latest master branch.

Following is the patch summary:
 - Patch 1 fixes a minor issue with the existing driver
 - Patch 2 is a minor improvement to sanity check the boot addresses
   used with the DSPs within the U-Boot driver code itself.
 - Patches 3 and 4 enhance the driver to support internal memory loading
   support by re-ordering the reset sequences. Patch 3 updates the MMU
   tables, without which the memcpy results in a data abort, and patch 4
   adds the logic to the driver.

regards
Suman

Suman Anna (4):
  remoteproc: k3-dsp: Fix unbalanced state machine in k3_dsp_start
  remoteproc: k3-dsp: Add a sanity check for DSP boot address alignment
  armv8: K3: j721e: Add DSP internal memory regions in MMU table
  remoteproc: k3-dsp: Add support for L2RAM loading on C66x DSPs

 arch/arm/mach-k3/arm64-mmu.c         |   8 +-
 drivers/remoteproc/ti_k3_dsp_rproc.c | 128 ++++++++++++++++++++++++---
 2 files changed, 122 insertions(+), 14 deletions(-)

-- 
2.23.0

[PATCH 1/4] remoteproc: k3-dsp: Fix unbalanced state machine in k3_dsp_start

[Suman Anna]

The global module reset is deasserted through the ti_sci_power_domain_on()
call in k3_dsp_start(), but is not asserted back if the local module reset
fails. Fix this.

While at this, remove the stale comment about assigned-clock-rates that
seems to have been copied from the K3 ARM64 Remoteproc driver.

Fixes: ab827b385718 ("remoteproc: Introduce K3 C66 and C71 remoteproc driver")
Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/ti_k3_dsp_rproc.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/remoteproc/ti_k3_dsp_rproc.c b/drivers/remoteproc/ti_k3_dsp_rproc.c
index 09e050ffb2d3..ff5d7f7f464f 100644
--- a/drivers/remoteproc/ti_k3_dsp_rproc.c
+++ b/drivers/remoteproc/ti_k3_dsp_rproc.c
@@ -102,16 +102,14 @@ static int k3_dsp_start(struct udevice *dev)
 	ret = ti_sci_proc_request(&dsp->tsp);
 	if (ret)
 		return ret;
-	/*
-	 * Setting the right clock frequency would have taken care by
-	 * assigned-clock-rates during the device probe. So no need to
-	 * set the frequency again here.
-	 */
+
 	ret = ti_sci_proc_power_domain_on(&dsp->tsp);
 	if (ret)
 		goto proc_release;
 
 	ret = reset_deassert(&dsp->dsp_rst);
+	if (ret)
+		ti_sci_proc_power_domain_off(&dsp->tsp);
 
 proc_release:
 	ti_sci_proc_release(&dsp->tsp);
-- 
2.23.0

[PATCH 2/4] remoteproc: k3-dsp: Add a sanity check for DSP boot address alignment

[Suman Anna]

The DSP remote processors on K3 SoCs require a boot register to be
programmed with a boot address, and these boot addresses need to be
aligned on certain address boundaries. The current code does not have
any error checks, and relies on the System Firmware to perform the
checking. Add logic to perform this sanity check within the remoteproc
driver itself to detect these anomalies specifically, and print a
meaningful trace. This avoids the cumbersome debug of root-causing
such failures from the corresponding TI-SCI failure.

The C66x and C71x DSP cores have different alignment needs and are
as follows:
	C66x DSP = 1 KB (0x400)
	C71x DSP = 2 MB (0x200000)

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/ti_k3_dsp_rproc.c | 34 +++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/remoteproc/ti_k3_dsp_rproc.c b/drivers/remoteproc/ti_k3_dsp_rproc.c
index ff5d7f7f464f..4937fdd0a776 100644
--- a/drivers/remoteproc/ti_k3_dsp_rproc.c
+++ b/drivers/remoteproc/ti_k3_dsp_rproc.c
@@ -2,7 +2,7 @@
 /*
  * Texas Instruments' K3 DSP Remoteproc driver
  *
- * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Copyright (C) 2018-2020 Texas Instruments Incorporated - http://www.ti.com/
  *	Lokesh Vutla <lokeshvutla@ti.com>
  *
  */
@@ -18,6 +18,7 @@
 #include <power-domain.h>
 #include <dm/device_compat.h>
 #include <linux/err.h>
+#include <linux/sizes.h>
 #include <linux/soc/ti/ti_sci_protocol.h>
 #include "ti_sci_proc.h"
 
@@ -37,16 +38,26 @@ struct k3_dsp_mem {
 	size_t size;
 };
 
+/**
+ * struct k3_dsp_boot_data - internal data structure used for boot
+ * @boot_align_addr: Boot vector address alignment granularity
+ */
+struct k3_dsp_boot_data {
+	u32 boot_align_addr;
+};
+
 /**
  * struct k3_dsp_privdata - Structure representing Remote processor data.
  * @rproc_rst:		rproc reset control data
  * @tsp:		Pointer to TISCI proc contrl handle
+ * @data:		Pointer to DSP specific boot data structure
  * @mem:		Array of available memories
  * @num_mem:		Number of available memories
  */
 struct k3_dsp_privdata {
 	struct reset_ctl dsp_rst;
 	struct ti_sci_proc tsp;
+	struct k3_dsp_boot_data *data;
 	struct k3_dsp_mem *mem;
 	int num_mems;
 };
@@ -62,6 +73,7 @@ struct k3_dsp_privdata {
 static int k3_dsp_load(struct udevice *dev, ulong addr, ulong size)
 {
 	struct k3_dsp_privdata *dsp = dev_get_priv(dev);
+	struct k3_dsp_boot_data *data = dsp->data;
 	u32 boot_vector;
 	int ret;
 
@@ -77,6 +89,12 @@ static int k3_dsp_load(struct udevice *dev, ulong addr, ulong size)
 	}
 
 	boot_vector = rproc_elf_get_boot_addr(dev, addr);
+	if (boot_vector & (data->boot_align_addr - 1)) {
+		ret = -EINVAL;
+		dev_err(dev, "Boot vector 0x%x not aligned on 0x%x boundary\n",
+			boot_vector, data->boot_align_addr);
+		goto proc_release;
+	}
 
 	dev_dbg(dev, "%s: Boot vector = 0x%x\n", __func__, boot_vector);
 
@@ -300,6 +318,8 @@ static int k3_dsp_of_to_priv(struct udevice *dev, struct k3_dsp_privdata *dsp)
 	if (ret)
 		return ret;
 
+	dsp->data = (struct k3_dsp_boot_data *)dev_get_driver_data(dev);
+
 	return 0;
 }
 
@@ -338,9 +358,17 @@ static int k3_dsp_remove(struct udevice *dev)
 	return 0;
 }
 
+static const struct k3_dsp_boot_data c66_data = {
+	.boot_align_addr = SZ_1K,
+};
+
+static const struct k3_dsp_boot_data c71_data = {
+	.boot_align_addr = SZ_2M,
+};
+
 static const struct udevice_id k3_dsp_ids[] = {
-	{ .compatible = "ti,j721e-c66-dsp"},
-	{ .compatible = "ti,j721e-c71-dsp"},
+	{ .compatible = "ti,j721e-c66-dsp", .data = (ulong)&c66_data, },
+	{ .compatible = "ti,j721e-c71-dsp", .data = (ulong)&c71_data, },
 	{}
 };
 
-- 
2.23.0

[PATCH 3/4] armv8: K3: j721e: Add DSP internal memory regions in MMU table

[Suman Anna]

The A72 U-Boot code supports early load and boot of a number of
remote processors including the C66_0 and C66_1 DSPs. The current
code supports only loading into the DDR regions which were already
given the appropriate memory attributes. The C66 DSPs also have L1
and L2 internal memory regions that can behave as normal-memories.

Add a new entry to the J721E MMU table covering these regions with
the appropriate memory attributes to allow the A72 U-Boot code to
support loading directly into these memory regions.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/mach-k3/arm64-mmu.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-k3/arm64-mmu.c b/arch/arm/mach-k3/arm64-mmu.c
index 7f908eee803b..582ccf7422b8 100644
--- a/arch/arm/mach-k3/arm64-mmu.c
+++ b/arch/arm/mach-k3/arm64-mmu.c
@@ -60,7 +60,7 @@ struct mm_region *mem_map = am654_mem_map;
 
 #ifdef CONFIG_SOC_K3_J721E
 /* NR_DRAM_BANKS + 32bit IO + 64bit IO + terminator */
-#define NR_MMU_REGIONS	(CONFIG_NR_DRAM_BANKS + 5)
+#define NR_MMU_REGIONS	(CONFIG_NR_DRAM_BANKS + 6)
 
 /* ToDo: Add 64bit IO */
 struct mm_region j721e_mem_map[NR_MMU_REGIONS] = {
@@ -102,6 +102,12 @@ struct mm_region j721e_mem_map[NR_MMU_REGIONS] = {
 		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
 			 PTE_BLOCK_NON_SHARE |
 			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+	}, {
+		.virt = 0x4d80000000UL,
+		.phys = 0x4d80000000UL,
+		.size = 0x0002000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL_NC) |
+			 PTE_BLOCK_INNER_SHARE
 	}, {
 		/* List terminator */
 		0,
-- 
2.23.0

[PATCH 4/4] remoteproc: k3-dsp: Add support for L2RAM loading on C66x DSPs

[Suman Anna]

The resets for the DSP processors on K3 SoCs are managed through the
Power and Sleep Controller (PSC) module. Each DSP typically has two
resets - a global module reset for powering on the device, and a local
reset that affects only the CPU while allowing access to the other
sub-modules within the DSP processor sub-systems.

The C66x DSPs have two levels of internal RAMs that can be used to
boot from, and the firmware loading into these RAMs require the
local reset to be asserted with the device powered on/enabled using
the module reset. Enhance the K3 DSP remoteproc driver to add support
for loading into the internal RAMs. The local reset is deasserted on
SoC power-on-reset, so logic has to be added in probe in remoteproc
mode to balance the remoteproc state-machine.

Note that the local resets are a no-op on C71x cores, and the hardware
does not supporting loading into its internal RAMs.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/ti_k3_dsp_rproc.c | 90 +++++++++++++++++++++++++---
 1 file changed, 83 insertions(+), 7 deletions(-)

diff --git a/drivers/remoteproc/ti_k3_dsp_rproc.c b/drivers/remoteproc/ti_k3_dsp_rproc.c
index 4937fdd0a776..1fc8193ad93f 100644
--- a/drivers/remoteproc/ti_k3_dsp_rproc.c
+++ b/drivers/remoteproc/ti_k3_dsp_rproc.c
@@ -4,7 +4,7 @@
  *
  * Copyright (C) 2018-2020 Texas Instruments Incorporated - http://www.ti.com/
  *	Lokesh Vutla <lokeshvutla@ti.com>
- *
+ *	Suman Anna <s-anna@ti.com>
  */
 
 #include <common.h>
@@ -41,9 +41,11 @@ struct k3_dsp_mem {
 /**
  * struct k3_dsp_boot_data - internal data structure used for boot
  * @boot_align_addr: Boot vector address alignment granularity
+ * @uses_lreset: Flag to denote the need for local reset management
  */
 struct k3_dsp_boot_data {
 	u32 boot_align_addr;
+	bool uses_lreset;
 };
 
 /**
@@ -62,6 +64,54 @@ struct k3_dsp_privdata {
 	int num_mems;
 };
 
+/*
+ * The C66x DSP cores have a local reset that affects only the CPU, and a
+ * generic module reset that powers on the device and allows the DSP internal
+ * memories to be accessed while the local reset is asserted. This function is
+ * used to release the global reset on C66x DSPs to allow loading into the DSP
+ * internal RAMs. This helper function is invoked in k3_dsp_load() before any
+ * actual firmware loading and is undone only in k3_dsp_stop(). The local reset
+ * on C71x cores is a no-op and the global reset cannot be released on C71x
+ * cores until after the firmware images are loaded, so this function does
+ * nothing for C71x cores.
+ */
+static int k3_dsp_prepare(struct udevice *dev)
+{
+	struct k3_dsp_privdata *dsp = dev_get_priv(dev);
+	struct k3_dsp_boot_data *data = dsp->data;
+	int ret;
+
+	/* local reset is no-op on C71x processors */
+	if (!data->uses_lreset)
+		return 0;
+
+	ret = ti_sci_proc_power_domain_on(&dsp->tsp);
+	if (ret)
+		dev_err(dev, "cannot enable internal RAM loading, ret = %d\n",
+			ret);
+
+	return ret;
+}
+
+/*
+ * This function is the counterpart to k3_dsp_prepare() and is used to assert
+ * the global reset on C66x DSP cores (no-op for C71x DSP cores). This completes
+ * the second step of powering down the C66x DSP cores. The cores themselves
+ * are halted through the local reset in first step. This function is invoked
+ * in k3_dsp_stop() after the local reset is asserted.
+ */
+static int k3_dsp_unprepare(struct udevice *dev)
+{
+	struct k3_dsp_privdata *dsp = dev_get_priv(dev);
+	struct k3_dsp_boot_data *data = dsp->data;
+
+	/* local reset is no-op on C71x processors */
+	if (!data->uses_lreset)
+		return 0;
+
+	return ti_sci_proc_power_domain_off(&dsp->tsp);
+}
+
 /**
  * k3_dsp_load() - Load up the Remote processor image
  * @dev:	rproc device pointer
@@ -82,10 +132,17 @@ static int k3_dsp_load(struct udevice *dev, ulong addr, ulong size)
 	if (ret)
 		return ret;
 
+	ret = k3_dsp_prepare(dev);
+	if (ret) {
+		dev_err(dev, "DSP prepare failed for core %d\n",
+			dsp->tsp.proc_id);
+		goto proc_release;
+	}
+
 	ret = rproc_elf_load_image(dev, addr, size);
 	if (ret < 0) {
 		dev_err(dev, "Loading elf failed %d\n", ret);
-		goto proc_release;
+		goto unprepare;
 	}
 
 	boot_vector = rproc_elf_get_boot_addr(dev, addr);
@@ -99,6 +156,9 @@ static int k3_dsp_load(struct udevice *dev, ulong addr, ulong size)
 	dev_dbg(dev, "%s: Boot vector = 0x%x\n", __func__, boot_vector);
 
 	ret = ti_sci_proc_set_config(&dsp->tsp, boot_vector, 0, 0);
+unprepare:
+	if (ret)
+		k3_dsp_unprepare(dev);
 proc_release:
 	ti_sci_proc_release(&dsp->tsp);
 	return ret;
@@ -113,6 +173,7 @@ proc_release:
 static int k3_dsp_start(struct udevice *dev)
 {
 	struct k3_dsp_privdata *dsp = dev_get_priv(dev);
+	struct k3_dsp_boot_data *data = dsp->data;
 	int ret;
 
 	dev_dbg(dev, "%s\n", __func__);
@@ -121,13 +182,17 @@ static int k3_dsp_start(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	ret = ti_sci_proc_power_domain_on(&dsp->tsp);
-	if (ret)
-		goto proc_release;
+	if (!data->uses_lreset) {
+		ret = ti_sci_proc_power_domain_on(&dsp->tsp);
+		if (ret)
+			goto proc_release;
+	}
 
 	ret = reset_deassert(&dsp->dsp_rst);
-	if (ret)
-		ti_sci_proc_power_domain_off(&dsp->tsp);
+	if (ret) {
+		if (!data->uses_lreset)
+			ti_sci_proc_power_domain_off(&dsp->tsp);
+	}
 
 proc_release:
 	ti_sci_proc_release(&dsp->tsp);
@@ -344,6 +409,15 @@ static int k3_dsp_probe(struct udevice *dev)
 		return ret;
 	}
 
+	/*
+	 * The DSP local resets are deasserted by default on Power-On-Reset.
+	 * Assert the local resets to ensure the DSPs don't execute bogus code
+	 * in .load() callback when the module reset is released to support
+	 * internal memory loading. This is needed for C66x DSPs, and is a
+	 * no-op on C71x DSPs.
+	 */
+	reset_assert(&dsp->dsp_rst);
+
 	dev_dbg(dev, "Remoteproc successfully probed\n");
 
 	return 0;
@@ -360,10 +434,12 @@ static int k3_dsp_remove(struct udevice *dev)
 
 static const struct k3_dsp_boot_data c66_data = {
 	.boot_align_addr = SZ_1K,
+	.uses_lreset = true,
 };
 
 static const struct k3_dsp_boot_data c71_data = {
 	.boot_align_addr = SZ_2M,
+	.uses_lreset = false,
 };
 
 static const struct udevice_id k3_dsp_ids[] = {
-- 
2.23.0

[PATCH 0/4] TI K3 DSP remoteproc fixes/enhancements

[Lokesh Vutla]

On 11/03/20 2:35 AM, Suman Anna wrote:
> Hi All,
> 
> The following series updates the K3 DSP remoteproc driver to fix couple
> of issues, and add the internal memory loading support on C66x DSPs.
> Patches are on top of latest master branch.
> 
> Following is the patch summary:
>  - Patch 1 fixes a minor issue with the existing driver
>  - Patch 2 is a minor improvement to sanity check the boot addresses
>    used with the DSPs within the U-Boot driver code itself.
>  - Patches 3 and 4 enhance the driver to support internal memory loading
>    support by re-ordering the reset sequences. Patch 3 updates the MMU
>    tables, without which the memcpy results in a data abort, and patch 4
>    adds the logic to the driver.
Applied to u-boot-ti/next

Thanks and regards,
Lokesh

> 
> regards
> Suman
> 
> Suman Anna (4):
>   remoteproc: k3-dsp: Fix unbalanced state machine in k3_dsp_start
>   remoteproc: k3-dsp: Add a sanity check for DSP boot address alignment
>   armv8: K3: j721e: Add DSP internal memory regions in MMU table
>   remoteproc: k3-dsp: Add support for L2RAM loading on C66x DSPs
> 
>  arch/arm/mach-k3/arm64-mmu.c         |   8 +-
>  drivers/remoteproc/ti_k3_dsp_rproc.c | 128 ++++++++++++++++++++++++---
>  2 files changed, 122 insertions(+), 14 deletions(-)
>