[PATCH 0/3] TI K3 R5F remoteproc support on J7200 SoCs

[PATCH 0/3] TI K3 R5F remoteproc support on J7200 SoCs

[Suman Anna]

Hi All,

The following series enhances the K3 R5F remoteproc driver to add support
for the R5F clusters on the newer TI K3 J7200 SoC family. The J7200 SoCs
have 2 R5FSS clusters, and both clusters are capable of supporting either
the LockStep or Split-modes like on the existing AM65x and J721E SoCs.

The R5FSS IP though is revised compared to K3 AM65x and J721E SoCs and has
two new features: 
 1. TCMs are auto-initialized during module power-up, and the behavior
    is programmable through a SEC_MMR register bit.
 2. The LockStep-mode allows the Core1 TCMs to be combined with the 
    Core0 TCMs effectively doubling the amount of TCMs available.
    The LockStep-mode on previous SoCs could only use the Core0 TCMs.
    This combined TCMs appear contiguous at the respective Core0 TCM
    addresses.

The series is based on 5.10-rc1, and can apply on top of the current rproc-next
branch as well. Following is the patch summary:
 - Patch 1 updates the dt-bindings
 - Patch 2 introduces new SoC data logic and handles the TCM auto-init
   feature
 - Patch 3 handles the TCM adjustment logic in Split-mode

regards
Suman

Suman Anna (3):
  dt-bindings: remoteproc: k3-r5f: Update bindings for J7200 SoCs
  remoteproc: k3-r5: Extend support to R5F clusters on J7200 SoCs
  remoteproc: k3-r5: Adjust TCM sizes in Split-mode on J7200 SoCs

 .../bindings/remoteproc/ti,k3-r5f-rproc.yaml  |  2 +
 drivers/remoteproc/ti_k3_r5_remoteproc.c      | 95 ++++++++++++++++++-
 2 files changed, 95 insertions(+), 2 deletions(-)

-- 
2.28.0

[PATCH 1/3] dt-bindings: remoteproc: k3-r5f: Update bindings for J7200 SoCs

[Suman Anna]

The TI K3 J7200 SoCs have two dual-core Arm R5F clusters/subsystems,
with 2 R5F cores each, one in each of the MCU and MAIN voltage domains.

These clusters are a revised IP version compared to those present on
J721E SoCs. Update the K3 R5F remoteproc bindings with the compatible
info relevant to these R5F clusters/subsystems on K3 J7200 SoCs.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 .../devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml         | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
index 4069f0f5e8fa..d905d614502b 100644
--- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
@@ -32,6 +32,7 @@ properties:
     enum:
       - ti,am654-r5fss
       - ti,j721e-r5fss
+      - ti,j7200-r5fss
 
   power-domains:
     description: |
@@ -95,6 +96,7 @@ patternProperties:
         enum:
           - ti,am654-r5f
           - ti,j721e-r5f
+          - ti,j7200-r5f
 
       reg:
         items:
-- 
2.28.0

[PATCH 2/3] remoteproc: k3-r5: Extend support to R5F clusters on J7200 SoCs

[Suman Anna]

The K3 J7200 SoC family has a revised R5F sub-system and contains a
subset of the R5F clusters present on J721E SoCs. The K3 J7200 SoCs
only have two dual-core Arm R5F clusters/subsystems with 2 R5F cores
each. One cluster is present within the MCU voltage domain (MCU_R5FSS0),
while the other is present in the MAIN voltage domain (MAIN_R5FSS0).

The revised IP has the following two new features:
 1. TCMs are auto-initialized during module power-up, and the behavior
    is programmable through a MMR bit.
 2. The LockStep-mode allows the Core1 TCMs to be combined with the
    Core0 TCMs effectively doubling the amount of TCMs available.
    The LockStep-mode on previous SoCs could only use the Core0 TCMs.
    This combined TCMs appear contiguous at the respective Core0 TCM
    addresses.

Extend the support to these clusters in the K3 R5F remoteproc driver
using J7200 specific compatibles. Logic for the second feature is
added in the next patch. The integration of these clusters is very
much similar to J721E SoCs otherwise.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/ti_k3_r5_remoteproc.c | 52 +++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 40fa7a4d2ec8..66a32dcdd7d0 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -38,6 +38,8 @@
 #define PROC_BOOT_CFG_FLAG_R5_TCM_RSTBASE		0x00000800
 #define PROC_BOOT_CFG_FLAG_R5_BTCM_EN			0x00001000
 #define PROC_BOOT_CFG_FLAG_R5_ATCM_EN			0x00002000
+/* Available from J7200 SoCs onwards */
+#define PROC_BOOT_CFG_FLAG_R5_MEM_INIT_DIS		0x00004000
 
 /* R5 TI-SCI Processor Control Flags */
 #define PROC_BOOT_CTRL_FLAG_R5_CORE_HALT		0x00000001
@@ -67,16 +69,26 @@ enum cluster_mode {
 	CLUSTER_MODE_LOCKSTEP,
 };
 
+/**
+ * struct k3_r5_soc_data - match data to handle SoC variations
+ * @tcm_ecc_autoinit: flag to denote the auto-initialization of TCMs for ECC
+ */
+struct k3_r5_soc_data {
+	bool tcm_ecc_autoinit;
+};
+
 /**
  * struct k3_r5_cluster - K3 R5F Cluster structure
  * @dev: cached device pointer
  * @mode: Mode to configure the Cluster - Split or LockStep
  * @cores: list of R5 cores within the cluster
+ * @soc_data: SoC-specific feature data for a R5FSS
  */
 struct k3_r5_cluster {
 	struct device *dev;
 	enum cluster_mode mode;
 	struct list_head cores;
+	const struct k3_r5_soc_data *soc_data;
 };
 
 /**
@@ -362,8 +374,16 @@ static int k3_r5_rproc_prepare(struct rproc *rproc)
 	struct k3_r5_cluster *cluster = kproc->cluster;
 	struct k3_r5_core *core = kproc->core;
 	struct device *dev = kproc->dev;
+	u32 ctrl = 0, cfg = 0, stat = 0;
+	u64 boot_vec = 0;
+	bool mem_init_dis;
 	int ret;
 
+	ret = ti_sci_proc_get_status(core->tsp, &boot_vec, &cfg, &ctrl, &stat);
+	if (ret < 0)
+		return ret;
+	mem_init_dis = !!(cfg & PROC_BOOT_CFG_FLAG_R5_MEM_INIT_DIS);
+
 	ret = (cluster->mode == CLUSTER_MODE_LOCKSTEP) ?
 		k3_r5_lockstep_release(cluster) : k3_r5_split_release(core);
 	if (ret) {
@@ -372,6 +392,17 @@ static int k3_r5_rproc_prepare(struct rproc *rproc)
 		return ret;
 	}
 
+	/*
+	 * Newer IP revisions like on J7200 SoCs support h/w auto-initialization
+	 * of TCMs, so there is no need to perform the s/w memzero. This bit is
+	 * configurable through System Firmware, the default value does perform
+	 * auto-init, but account for it in case it is disabled
+	 */
+	if (cluster->soc_data->tcm_ecc_autoinit && !mem_init_dis) {
+		dev_dbg(dev, "leveraging h/w init for TCM memories\n");
+		return 0;
+	}
+
 	/*
 	 * Zero out both TCMs unconditionally (access from v8 Arm core is not
 	 * affected by ATCM & BTCM enable configuration values) so that ECC
@@ -1309,15 +1340,23 @@ static int k3_r5_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev_of_node(dev);
 	struct k3_r5_cluster *cluster;
+	const struct k3_r5_soc_data *data;
 	int ret;
 	int num_cores;
 
+	data = of_device_get_match_data(&pdev->dev);
+	if (!data) {
+		dev_err(dev, "SoC-specific data is not defined\n");
+		return -ENODEV;
+	}
+
 	cluster = devm_kzalloc(dev, sizeof(*cluster), GFP_KERNEL);
 	if (!cluster)
 		return -ENOMEM;
 
 	cluster->dev = dev;
 	cluster->mode = CLUSTER_MODE_LOCKSTEP;
+	cluster->soc_data = data;
 	INIT_LIST_HEAD(&cluster->cores);
 
 	ret = of_property_read_u32(np, "ti,cluster-mode", &cluster->mode);
@@ -1367,9 +1406,18 @@ static int k3_r5_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct k3_r5_soc_data am65_j721e_soc_data = {
+	.tcm_ecc_autoinit = false,
+};
+
+static const struct k3_r5_soc_data j7200_soc_data = {
+	.tcm_ecc_autoinit = true,
+};
+
 static const struct of_device_id k3_r5_of_match[] = {
-	{ .compatible = "ti,am654-r5fss", },
-	{ .compatible = "ti,j721e-r5fss", },
+	{ .compatible = "ti,am654-r5fss", .data = &am65_j721e_soc_data, },
+	{ .compatible = "ti,j721e-r5fss", .data = &am65_j721e_soc_data, },
+	{ .compatible = "ti,j7200-r5fss", .data = &j7200_soc_data, },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, k3_r5_of_match);
-- 
2.28.0

[PATCH 3/3] remoteproc: k3-r5: Adjust TCM sizes in Split-mode on J7200 SoCs

[Suman Anna]

The J7200 SoCs have a revised R5FSS IP that adds a unique feature w.r.t
TCM sizing. Each R5F core in a cluster typically has 32 KB each of ATCM
and BTCM, with only the Core0 TCMs usable in LockStep mode. This revised
IP however doubles the total available TCM in LockStep mode by making the
Core1 TCM visible immediately after the corresponding Core0 TCM.

The R5F DT nodes on the J7200 SoCs define double (64 KB) the normal TCM
size (32 KB) for R5F Core0 for each of ATCM and BTCM to represent the
above. This increased TCM memory is only usable in LockStep-mode, and
has to be adjusted to the normal 32 KB size in Split mode. Enhance the
TI K3 R5F remoteproc for this logic through a new function. The adjustment
is a no-op on prior SoCs and relies on the correct DTS node sizes in
LockStep-mode on applicable SoCs.

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

diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 66a32dcdd7d0..62b5a4c29456 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -71,9 +71,11 @@ enum cluster_mode {
 
 /**
  * struct k3_r5_soc_data - match data to handle SoC variations
+ * @tcm_is_double: flag to denote the larger unified TCMs in certain modes
  * @tcm_ecc_autoinit: flag to denote the auto-initialization of TCMs for ECC
  */
 struct k3_r5_soc_data {
+	bool tcm_is_double;
 	bool tcm_ecc_autoinit;
 };
 
@@ -886,6 +888,43 @@ static void k3_r5_reserved_mem_exit(struct k3_r5_rproc *kproc)
 	of_reserved_mem_device_release(kproc->dev);
 }
 
+/*
+ * Each R5F core within a typical R5FSS instance has a total of 64 KB of TCMs,
+ * split equally into two 32 KB banks between ATCM and BTCM. The TCMs from both
+ * cores are usable in Split-mode, but only the Core0 TCMs can be used in
+ * LockStep-mode. The newer revisions of the R5FSS IP maximizes these TCMs by
+ * leveraging the Core1 TCMs as well in certain modes where they would have
+ * otherwise been unusable (Eg: LockStep-mode on J7200 SoCs). This is done by
+ * making a Core1 TCM visible immediately after the corresponding Core0 TCM.
+ * The SoC memory map uses the larger 64 KB sizes for the Core0 TCMs, and the
+ * dts representation reflects this increased size on supported SoCs. The Core0
+ * TCM sizes therefore have to be adjusted to only half the original size in
+ * Split mode.
+ */
+static void k3_r5_adjust_tcm_sizes(struct k3_r5_rproc *kproc)
+{
+	struct k3_r5_cluster *cluster = kproc->cluster;
+	struct k3_r5_core *core = kproc->core;
+	struct device *cdev = core->dev;
+	struct k3_r5_core *core0;
+
+	if (cluster->mode == CLUSTER_MODE_LOCKSTEP ||
+	    !cluster->soc_data->tcm_is_double)
+		return;
+
+	core0 = list_first_entry(&cluster->cores, struct k3_r5_core, elem);
+	if (core == core0) {
+		WARN_ON(core->mem[0].size != SZ_64K);
+		WARN_ON(core->mem[1].size != SZ_64K);
+
+		core->mem[0].size /= 2;
+		core->mem[1].size /= 2;
+
+		dev_dbg(cdev, "adjusted TCM sizes, ATCM = 0x%zx BTCM = 0x%zx\n",
+			core->mem[0].size, core->mem[1].size);
+	}
+}
+
 static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
 {
 	struct k3_r5_cluster *cluster = platform_get_drvdata(pdev);
@@ -933,6 +972,8 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
 			goto err_config;
 		}
 
+		k3_r5_adjust_tcm_sizes(kproc);
+
 		ret = k3_r5_reserved_mem_init(kproc);
 		if (ret) {
 			dev_err(dev, "reserved memory init failed, ret = %d\n",
@@ -1407,10 +1448,12 @@ static int k3_r5_probe(struct platform_device *pdev)
 }
 
 static const struct k3_r5_soc_data am65_j721e_soc_data = {
+	.tcm_is_double = false,
 	.tcm_ecc_autoinit = false,
 };
 
 static const struct k3_r5_soc_data j7200_soc_data = {
+	.tcm_is_double = true,
 	.tcm_ecc_autoinit = true,
 };
 
-- 
2.28.0

Re: [PATCH 3/3] remoteproc: k3-r5: Adjust TCM sizes in Split-mode on J7200 SoCs

[Mathieu Poirier]

Good afternoon Suman,

On Wed, Nov 18, 2020 at 07:05:31PM -0600, Suman Anna wrote:
> The J7200 SoCs have a revised R5FSS IP that adds a unique feature w.r.t
> TCM sizing. Each R5F core in a cluster typically has 32 KB each of ATCM
> and BTCM, with only the Core0 TCMs usable in LockStep mode. This revised
> IP however doubles the total available TCM in LockStep mode by making the
> Core1 TCM visible immediately after the corresponding Core0 TCM.
> 
> The R5F DT nodes on the J7200 SoCs define double (64 KB) the normal TCM
> size (32 KB) for R5F Core0 for each of ATCM and BTCM to represent the
> above. This increased TCM memory is only usable in LockStep-mode, and
> has to be adjusted to the normal 32 KB size in Split mode. Enhance the
> TI K3 R5F remoteproc for this logic through a new function. The adjustment
> is a no-op on prior SoCs and relies on the correct DTS node sizes in
> LockStep-mode on applicable SoCs.
> 
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
>  drivers/remoteproc/ti_k3_r5_remoteproc.c | 43 ++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c
> index 66a32dcdd7d0..62b5a4c29456 100644
> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
> @@ -71,9 +71,11 @@ enum cluster_mode {
>  
>  /**
>   * struct k3_r5_soc_data - match data to handle SoC variations
> + * @tcm_is_double: flag to denote the larger unified TCMs in certain modes
>   * @tcm_ecc_autoinit: flag to denote the auto-initialization of TCMs for ECC
>   */
>  struct k3_r5_soc_data {
> +	bool tcm_is_double;
>  	bool tcm_ecc_autoinit;
>  };
>  
> @@ -886,6 +888,43 @@ static void k3_r5_reserved_mem_exit(struct k3_r5_rproc *kproc)
>  	of_reserved_mem_device_release(kproc->dev);
>  }
>  
> +/*
> + * Each R5F core within a typical R5FSS instance has a total of 64 KB of TCMs,
> + * split equally into two 32 KB banks between ATCM and BTCM. The TCMs from both
> + * cores are usable in Split-mode, but only the Core0 TCMs can be used in
> + * LockStep-mode. The newer revisions of the R5FSS IP maximizes these TCMs by
> + * leveraging the Core1 TCMs as well in certain modes where they would have
> + * otherwise been unusable (Eg: LockStep-mode on J7200 SoCs). This is done by
> + * making a Core1 TCM visible immediately after the corresponding Core0 TCM.
> + * The SoC memory map uses the larger 64 KB sizes for the Core0 TCMs, and the
> + * dts representation reflects this increased size on supported SoCs. The Core0
> + * TCM sizes therefore have to be adjusted to only half the original size in
> + * Split mode.
> + */
> +static void k3_r5_adjust_tcm_sizes(struct k3_r5_rproc *kproc)
> +{
> +	struct k3_r5_cluster *cluster = kproc->cluster;
> +	struct k3_r5_core *core = kproc->core;
> +	struct device *cdev = core->dev;
> +	struct k3_r5_core *core0;
> +
> +	if (cluster->mode == CLUSTER_MODE_LOCKSTEP ||
> +	    !cluster->soc_data->tcm_is_double)
> +		return;

Shouldn't this be:

	if (cluster->mode == CLUSTER_MODE_SPLIT ||
	    !cluster->soc_data->tcm_is_double)
		return;

If am wrong then I'm pretty sure other people will be confused and a comment is
warranted. 

> +
> +	core0 = list_first_entry(&cluster->cores, struct k3_r5_core, elem);
> +	if (core == core0) {
> +		WARN_ON(core->mem[0].size != SZ_64K);
> +		WARN_ON(core->mem[1].size != SZ_64K);
> +
> +		core->mem[0].size /= 2;
> +		core->mem[1].size /= 2;
> +
> +		dev_dbg(cdev, "adjusted TCM sizes, ATCM = 0x%zx BTCM = 0x%zx\n",
> +			core->mem[0].size, core->mem[1].size);
> +	}
> +}
> +
>  static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
>  {
>  	struct k3_r5_cluster *cluster = platform_get_drvdata(pdev);
> @@ -933,6 +972,8 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
>  			goto err_config;
>  		}
>  
> +		k3_r5_adjust_tcm_sizes(kproc);
> +
>  		ret = k3_r5_reserved_mem_init(kproc);
>  		if (ret) {
>  			dev_err(dev, "reserved memory init failed, ret = %d\n",
> @@ -1407,10 +1448,12 @@ static int k3_r5_probe(struct platform_device *pdev)
>  }
>  
>  static const struct k3_r5_soc_data am65_j721e_soc_data = {
> +	.tcm_is_double = false,
>  	.tcm_ecc_autoinit = false,
>  };
>  
>  static const struct k3_r5_soc_data j7200_soc_data = {
> +	.tcm_is_double = true,
>  	.tcm_ecc_autoinit = true,

With the above and for the set:

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>  };
>  
> -- 
> 2.28.0
> 

Re: [PATCH 3/3] remoteproc: k3-r5: Adjust TCM sizes in Split-mode on J7200 SoCs

[Mathieu Poirier]

On Mon, 23 Nov 2020 at 16:51, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
>
> Good afternoon Suman,
>
> On Wed, Nov 18, 2020 at 07:05:31PM -0600, Suman Anna wrote:
> > The J7200 SoCs have a revised R5FSS IP that adds a unique feature w.r.t
> > TCM sizing. Each R5F core in a cluster typically has 32 KB each of ATCM
> > and BTCM, with only the Core0 TCMs usable in LockStep mode. This revised
> > IP however doubles the total available TCM in LockStep mode by making the
> > Core1 TCM visible immediately after the corresponding Core0 TCM.
> >
> > The R5F DT nodes on the J7200 SoCs define double (64 KB) the normal TCM
> > size (32 KB) for R5F Core0 for each of ATCM and BTCM to represent the
> > above. This increased TCM memory is only usable in LockStep-mode, and
> > has to be adjusted to the normal 32 KB size in Split mode. Enhance the
> > TI K3 R5F remoteproc for this logic through a new function. The adjustment
> > is a no-op on prior SoCs and relies on the correct DTS node sizes in
> > LockStep-mode on applicable SoCs.
> >
> > Signed-off-by: Suman Anna <s-anna@ti.com>
> > ---
> >  drivers/remoteproc/ti_k3_r5_remoteproc.c | 43 ++++++++++++++++++++++++
> >  1 file changed, 43 insertions(+)
> >
> > diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c
> > index 66a32dcdd7d0..62b5a4c29456 100644
> > --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
> > +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
> > @@ -71,9 +71,11 @@ enum cluster_mode {
> >
> >  /**
> >   * struct k3_r5_soc_data - match data to handle SoC variations
> > + * @tcm_is_double: flag to denote the larger unified TCMs in certain modes
> >   * @tcm_ecc_autoinit: flag to denote the auto-initialization of TCMs for ECC
> >   */
> >  struct k3_r5_soc_data {
> > +     bool tcm_is_double;
> >       bool tcm_ecc_autoinit;
> >  };
> >
> > @@ -886,6 +888,43 @@ static void k3_r5_reserved_mem_exit(struct k3_r5_rproc *kproc)
> >       of_reserved_mem_device_release(kproc->dev);
> >  }
> >
> > +/*
> > + * Each R5F core within a typical R5FSS instance has a total of 64 KB of TCMs,
> > + * split equally into two 32 KB banks between ATCM and BTCM. The TCMs from both
> > + * cores are usable in Split-mode, but only the Core0 TCMs can be used in
> > + * LockStep-mode. The newer revisions of the R5FSS IP maximizes these TCMs by
> > + * leveraging the Core1 TCMs as well in certain modes where they would have
> > + * otherwise been unusable (Eg: LockStep-mode on J7200 SoCs). This is done by
> > + * making a Core1 TCM visible immediately after the corresponding Core0 TCM.
> > + * The SoC memory map uses the larger 64 KB sizes for the Core0 TCMs, and the
> > + * dts representation reflects this increased size on supported SoCs. The Core0
> > + * TCM sizes therefore have to be adjusted to only half the original size in
> > + * Split mode.
> > + */
> > +static void k3_r5_adjust_tcm_sizes(struct k3_r5_rproc *kproc)
> > +{
> > +     struct k3_r5_cluster *cluster = kproc->cluster;
> > +     struct k3_r5_core *core = kproc->core;
> > +     struct device *cdev = core->dev;
> > +     struct k3_r5_core *core0;
> > +
> > +     if (cluster->mode == CLUSTER_MODE_LOCKSTEP ||
> > +         !cluster->soc_data->tcm_is_double)
> > +             return;
>
> Shouldn't this be:
>
>         if (cluster->mode == CLUSTER_MODE_SPLIT ||
>             !cluster->soc_data->tcm_is_double)
>                 return;
>
> If am wrong then I'm pretty sure other people will be confused and a comment is
> warranted.
>

Forget the above, I misread the context.  The memories are already set
to 64KB so there is nothing to do if in lockstep mode.

> > +
> > +     core0 = list_first_entry(&cluster->cores, struct k3_r5_core, elem);
> > +     if (core == core0) {
> > +             WARN_ON(core->mem[0].size != SZ_64K);
> > +             WARN_ON(core->mem[1].size != SZ_64K);
> > +
> > +             core->mem[0].size /= 2;
> > +             core->mem[1].size /= 2;
> > +
> > +             dev_dbg(cdev, "adjusted TCM sizes, ATCM = 0x%zx BTCM = 0x%zx\n",
> > +                     core->mem[0].size, core->mem[1].size);
> > +     }
> > +}
> > +
> >  static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
> >  {
> >       struct k3_r5_cluster *cluster = platform_get_drvdata(pdev);
> > @@ -933,6 +972,8 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
> >                       goto err_config;
> >               }
> >
> > +             k3_r5_adjust_tcm_sizes(kproc);
> > +
> >               ret = k3_r5_reserved_mem_init(kproc);
> >               if (ret) {
> >                       dev_err(dev, "reserved memory init failed, ret = %d\n",
> > @@ -1407,10 +1448,12 @@ static int k3_r5_probe(struct platform_device *pdev)
> >  }
> >
> >  static const struct k3_r5_soc_data am65_j721e_soc_data = {
> > +     .tcm_is_double = false,
> >       .tcm_ecc_autoinit = false,
> >  };
> >
> >  static const struct k3_r5_soc_data j7200_soc_data = {
> > +     .tcm_is_double = true,
> >       .tcm_ecc_autoinit = true,
>
> With the above and for the set:
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
> >  };
> >
> > --
> > 2.28.0
> >

Re: [PATCH 3/3] remoteproc: k3-r5: Adjust TCM sizes in Split-mode on J7200 SoCs

[Suman Anna]

On 11/23/20 6:55 PM, Mathieu Poirier wrote:
> On Mon, 23 Nov 2020 at 16:51, Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
>>
>> Good afternoon Suman,
>>
>> On Wed, Nov 18, 2020 at 07:05:31PM -0600, Suman Anna wrote:
>>> The J7200 SoCs have a revised R5FSS IP that adds a unique feature w.r.t
>>> TCM sizing. Each R5F core in a cluster typically has 32 KB each of ATCM
>>> and BTCM, with only the Core0 TCMs usable in LockStep mode. This revised
>>> IP however doubles the total available TCM in LockStep mode by making the
>>> Core1 TCM visible immediately after the corresponding Core0 TCM.
>>>
>>> The R5F DT nodes on the J7200 SoCs define double (64 KB) the normal TCM
>>> size (32 KB) for R5F Core0 for each of ATCM and BTCM to represent the
>>> above. This increased TCM memory is only usable in LockStep-mode, and
>>> has to be adjusted to the normal 32 KB size in Split mode. Enhance the
>>> TI K3 R5F remoteproc for this logic through a new function. The adjustment
>>> is a no-op on prior SoCs and relies on the correct DTS node sizes in
>>> LockStep-mode on applicable SoCs.
>>>
>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>> ---
>>>  drivers/remoteproc/ti_k3_r5_remoteproc.c | 43 ++++++++++++++++++++++++
>>>  1 file changed, 43 insertions(+)
>>>
>>> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c
>>> index 66a32dcdd7d0..62b5a4c29456 100644
>>> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
>>> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
>>> @@ -71,9 +71,11 @@ enum cluster_mode {
>>>
>>>  /**
>>>   * struct k3_r5_soc_data - match data to handle SoC variations
>>> + * @tcm_is_double: flag to denote the larger unified TCMs in certain modes
>>>   * @tcm_ecc_autoinit: flag to denote the auto-initialization of TCMs for ECC
>>>   */
>>>  struct k3_r5_soc_data {
>>> +     bool tcm_is_double;
>>>       bool tcm_ecc_autoinit;
>>>  };
>>>
>>> @@ -886,6 +888,43 @@ static void k3_r5_reserved_mem_exit(struct k3_r5_rproc *kproc)
>>>       of_reserved_mem_device_release(kproc->dev);
>>>  }
>>>
>>> +/*
>>> + * Each R5F core within a typical R5FSS instance has a total of 64 KB of TCMs,
>>> + * split equally into two 32 KB banks between ATCM and BTCM. The TCMs from both
>>> + * cores are usable in Split-mode, but only the Core0 TCMs can be used in
>>> + * LockStep-mode. The newer revisions of the R5FSS IP maximizes these TCMs by
>>> + * leveraging the Core1 TCMs as well in certain modes where they would have
>>> + * otherwise been unusable (Eg: LockStep-mode on J7200 SoCs). This is done by
>>> + * making a Core1 TCM visible immediately after the corresponding Core0 TCM.
>>> + * The SoC memory map uses the larger 64 KB sizes for the Core0 TCMs, and the
>>> + * dts representation reflects this increased size on supported SoCs. The Core0
>>> + * TCM sizes therefore have to be adjusted to only half the original size in
>>> + * Split mode.
>>> + */
>>> +static void k3_r5_adjust_tcm_sizes(struct k3_r5_rproc *kproc)
>>> +{
>>> +     struct k3_r5_cluster *cluster = kproc->cluster;
>>> +     struct k3_r5_core *core = kproc->core;
>>> +     struct device *cdev = core->dev;
>>> +     struct k3_r5_core *core0;
>>> +
>>> +     if (cluster->mode == CLUSTER_MODE_LOCKSTEP ||
>>> +         !cluster->soc_data->tcm_is_double)
>>> +             return;
>>
>> Shouldn't this be:
>>
>>         if (cluster->mode == CLUSTER_MODE_SPLIT ||
>>             !cluster->soc_data->tcm_is_double)
>>                 return;
>>
>> If am wrong then I'm pretty sure other people will be confused and a comment is
>> warranted.
>>
> 
> Forget the above, I misread the context.  The memories are already set
> to 64KB so there is nothing to do if in lockstep mode.

Yep, thanks for the review. The comments above the function has all the details.

regards
Suman

> 
>>> +
>>> +     core0 = list_first_entry(&cluster->cores, struct k3_r5_core, elem);
>>> +     if (core == core0) {
>>> +             WARN_ON(core->mem[0].size != SZ_64K);
>>> +             WARN_ON(core->mem[1].size != SZ_64K);
>>> +
>>> +             core->mem[0].size /= 2;
>>> +             core->mem[1].size /= 2;
>>> +
>>> +             dev_dbg(cdev, "adjusted TCM sizes, ATCM = 0x%zx BTCM = 0x%zx\n",
>>> +                     core->mem[0].size, core->mem[1].size);
>>> +     }
>>> +}
>>> +
>>>  static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
>>>  {
>>>       struct k3_r5_cluster *cluster = platform_get_drvdata(pdev);
>>> @@ -933,6 +972,8 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
>>>                       goto err_config;
>>>               }
>>>
>>> +             k3_r5_adjust_tcm_sizes(kproc);
>>> +
>>>               ret = k3_r5_reserved_mem_init(kproc);
>>>               if (ret) {
>>>                       dev_err(dev, "reserved memory init failed, ret = %d\n",
>>> @@ -1407,10 +1448,12 @@ static int k3_r5_probe(struct platform_device *pdev)
>>>  }
>>>
>>>  static const struct k3_r5_soc_data am65_j721e_soc_data = {
>>> +     .tcm_is_double = false,
>>>       .tcm_ecc_autoinit = false,
>>>  };
>>>
>>>  static const struct k3_r5_soc_data j7200_soc_data = {
>>> +     .tcm_is_double = true,
>>>       .tcm_ecc_autoinit = true,
>>
>> With the above and for the set:
>>
>> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>
>>>  };
>>>
>>> --
>>> 2.28.0
>>>