From: Peng Fan <[email protected]>
Most logic are same as i.MX8QXP, but i.MX8QM has two general purpose
M4 cores, the two cores runs independently and they has different resource
id, different start address from SCFW view.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/remoteproc/imx_rproc.c | 41 +++++++++++++++++++++++++++++++---
1 file changed, 38 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 49cce9dd55c7..8326193c13d6 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -3,6 +3,7 @@
* Copyright (c) 2017 Pengutronix, Oleksij Rempel <[email protected]>
*/
+#include <dt-bindings/firmware/imx/rsrc.h>
#include <linux/arm-smccc.h>
#include <linux/clk.h>
#include <linux/err.h>
@@ -75,10 +76,13 @@ struct imx_rproc_mem {
size_t size;
};
-/* att flags */
+/* att flags: lower 16 bits specifying core, higher 16 bits for flags */
/* M4 own area. Can be mapped at probe */
-#define ATT_OWN BIT(1)
-#define ATT_IOMEM BIT(2)
+#define ATT_OWN BIT(31)
+#define ATT_IOMEM BIT(30)
+
+#define ATT_CORE_MASK 0xffff
+#define ATT_CORE(I) BIT((I))
struct imx_rproc {
struct device *dev;
@@ -99,6 +103,7 @@ struct imx_rproc {
u32 rsrc_id; /* resource id */
u32 entry; /* cpu start address */
int num_pd;
+ u32 core_index;
struct device **pd_dev;
struct device_link **pd_dev_link;
};
@@ -129,6 +134,19 @@ static const struct imx_rproc_att imx_rproc_att_imx93[] = {
{ 0xD0000000, 0xa0000000, 0x10000000, 0 },
};
+static const struct imx_rproc_att imx_rproc_att_imx8qm[] = {
+ /* dev addr , sys addr , size , flags */
+ { 0x08000000, 0x08000000, 0x10000000, 0},
+ /* TCML */
+ { 0x1FFE0000, 0x34FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(0)},
+ { 0x1FFE0000, 0x38FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(1)},
+ /* TCMU */
+ { 0x20000000, 0x35000000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(0)},
+ { 0x20000000, 0x39000000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(1)},
+ /* DDR (Data) */
+ { 0x80000000, 0x80000000, 0x60000000, 0 },
+};
+
static const struct imx_rproc_att imx_rproc_att_imx8qxp[] = {
{ 0x08000000, 0x08000000, 0x10000000, 0 },
/* TCML/U */
@@ -279,6 +297,12 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
.method = IMX_RPROC_MMIO,
};
+static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = {
+ .att = imx_rproc_att_imx8qm,
+ .att_size = ARRAY_SIZE(imx_rproc_att_imx8qm),
+ .method = IMX_RPROC_SCU_API,
+};
+
static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qxp = {
.att = imx_rproc_att_imx8qxp,
.att_size = ARRAY_SIZE(imx_rproc_att_imx8qxp),
@@ -395,6 +419,11 @@ static int imx_rproc_da_to_sys(struct imx_rproc *priv, u64 da,
for (i = 0; i < dcfg->att_size; i++) {
const struct imx_rproc_att *att = &dcfg->att[i];
+ if (att->flags & ATT_CORE_MASK) {
+ if (!((BIT(priv->core_index)) & (att->flags & ATT_CORE_MASK)))
+ continue;
+ }
+
if (da >= att->da && da + len < att->da + att->size) {
unsigned int offset = da - att->da;
@@ -815,6 +844,11 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
return ret;
}
+ if (priv->rsrc_id == IMX_SC_R_M4_1_PID0)
+ priv->core_index = 1;
+ else
+ priv->core_index = 0;
+
/*
* If Mcore resource is not owned by Acore partition, It is kicked by ROM,
* and Linux could only do IPC with Mcore and nothing else.
@@ -1008,6 +1042,7 @@ static const struct of_device_id imx_rproc_of_match[] = {
{ .compatible = "fsl,imx8mn-cm7", .data = &imx_rproc_cfg_imx8mn },
{ .compatible = "fsl,imx8mp-cm7", .data = &imx_rproc_cfg_imx8mn },
{ .compatible = "fsl,imx8qxp-cm4", .data = &imx_rproc_cfg_imx8qxp },
+ { .compatible = "fsl,imx8qm-cm4", .data = &imx_rproc_cfg_imx8qm },
{ .compatible = "fsl,imx8ulp-cm33", .data = &imx_rproc_cfg_imx8ulp },
{ .compatible = "fsl,imx93-cm33", .data = &imx_rproc_cfg_imx93 },
{},
--
2.25.1
On Tue, May 17, 2022 at 02:49:36PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> Most logic are same as i.MX8QXP, but i.MX8QM has two general purpose
> M4 cores, the two cores runs independently and they has different resource
> id, different start address from SCFW view.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/remoteproc/imx_rproc.c | 41 +++++++++++++++++++++++++++++++---
> 1 file changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 49cce9dd55c7..8326193c13d6 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -3,6 +3,7 @@
> * Copyright (c) 2017 Pengutronix, Oleksij Rempel <[email protected]>
> */
>
> +#include <dt-bindings/firmware/imx/rsrc.h>
> #include <linux/arm-smccc.h>
> #include <linux/clk.h>
> #include <linux/err.h>
> @@ -75,10 +76,13 @@ struct imx_rproc_mem {
> size_t size;
> };
>
> -/* att flags */
> +/* att flags: lower 16 bits specifying core, higher 16 bits for flags */
> /* M4 own area. Can be mapped at probe */
> -#define ATT_OWN BIT(1)
> -#define ATT_IOMEM BIT(2)
> +#define ATT_OWN BIT(31)
> +#define ATT_IOMEM BIT(30)
> +
> +#define ATT_CORE_MASK 0xffff
> +#define ATT_CORE(I) BIT((I))
>
> struct imx_rproc {
> struct device *dev;
> @@ -99,6 +103,7 @@ struct imx_rproc {
> u32 rsrc_id; /* resource id */
> u32 entry; /* cpu start address */
> int num_pd;
> + u32 core_index;
> struct device **pd_dev;
> struct device_link **pd_dev_link;
> };
> @@ -129,6 +134,19 @@ static const struct imx_rproc_att imx_rproc_att_imx93[] = {
> { 0xD0000000, 0xa0000000, 0x10000000, 0 },
> };
>
> +static const struct imx_rproc_att imx_rproc_att_imx8qm[] = {
> + /* dev addr , sys addr , size , flags */
> + { 0x08000000, 0x08000000, 0x10000000, 0},
> + /* TCML */
> + { 0x1FFE0000, 0x34FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(0)},
> + { 0x1FFE0000, 0x38FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(1)},
> + /* TCMU */
> + { 0x20000000, 0x35000000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(0)},
> + { 0x20000000, 0x39000000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(1)},
> + /* DDR (Data) */
> + { 0x80000000, 0x80000000, 0x60000000, 0 },
> +};
> +
> static const struct imx_rproc_att imx_rproc_att_imx8qxp[] = {
> { 0x08000000, 0x08000000, 0x10000000, 0 },
> /* TCML/U */
> @@ -279,6 +297,12 @@ static const struct imx_rproc_dcfg imx_rproc_cfg_imx8mq = {
> .method = IMX_RPROC_MMIO,
> };
>
> +static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = {
> + .att = imx_rproc_att_imx8qm,
> + .att_size = ARRAY_SIZE(imx_rproc_att_imx8qm),
> + .method = IMX_RPROC_SCU_API,
> +};
> +
> static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qxp = {
> .att = imx_rproc_att_imx8qxp,
> .att_size = ARRAY_SIZE(imx_rproc_att_imx8qxp),
> @@ -395,6 +419,11 @@ static int imx_rproc_da_to_sys(struct imx_rproc *priv, u64 da,
> for (i = 0; i < dcfg->att_size; i++) {
> const struct imx_rproc_att *att = &dcfg->att[i];
>
> + if (att->flags & ATT_CORE_MASK) {
> + if (!((BIT(priv->core_index)) & (att->flags & ATT_CORE_MASK)))
> + continue;
> + }
This is very cryptic - I just spent 20 minutes looking at it and I'm still not
sure I got the full meaning. Please add enough comments to make things obvious
on first read.
I am done reviewing this patchset.
Thanks,
Mathieu
> +
> if (da >= att->da && da + len < att->da + att->size) {
> unsigned int offset = da - att->da;
>
> @@ -815,6 +844,11 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
> return ret;
> }
>
> + if (priv->rsrc_id == IMX_SC_R_M4_1_PID0)
> + priv->core_index = 1;
> + else
> + priv->core_index = 0;
> +
> /*
> * If Mcore resource is not owned by Acore partition, It is kicked by ROM,
> * and Linux could only do IPC with Mcore and nothing else.
> @@ -1008,6 +1042,7 @@ static const struct of_device_id imx_rproc_of_match[] = {
> { .compatible = "fsl,imx8mn-cm7", .data = &imx_rproc_cfg_imx8mn },
> { .compatible = "fsl,imx8mp-cm7", .data = &imx_rproc_cfg_imx8mn },
> { .compatible = "fsl,imx8qxp-cm4", .data = &imx_rproc_cfg_imx8qxp },
> + { .compatible = "fsl,imx8qm-cm4", .data = &imx_rproc_cfg_imx8qm },
> { .compatible = "fsl,imx8ulp-cm33", .data = &imx_rproc_cfg_imx8ulp },
> { .compatible = "fsl,imx93-cm33", .data = &imx_rproc_cfg_imx93 },
> {},
> --
> 2.25.1
>
> Subject: Re: [PATCH V3 5/6] remoteproc: imx_rproc: support i.MX8QM
>
> On Tue, May 17, 2022 at 02:49:36PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <[email protected]>
> >
> > Most logic are same as i.MX8QXP, but i.MX8QM has two general purpose
> > M4 cores, the two cores runs independently and they has different
> > resource id, different start address from SCFW view.
> >
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
> > drivers/remoteproc/imx_rproc.c | 41
> > +++++++++++++++++++++++++++++++---
> > 1 file changed, 38 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/remoteproc/imx_rproc.c
> > b/drivers/remoteproc/imx_rproc.c index 49cce9dd55c7..8326193c13d6
> > 100644
> > --- a/drivers/remoteproc/imx_rproc.c
> > +++ b/drivers/remoteproc/imx_rproc.c
> > @@ -3,6 +3,7 @@
> > * Copyright (c) 2017 Pengutronix, Oleksij Rempel
> <[email protected]>
> > */
> >
> > +#include <dt-bindings/firmware/imx/rsrc.h>
> > #include <linux/arm-smccc.h>
> > #include <linux/clk.h>
> > #include <linux/err.h>
> > @@ -75,10 +76,13 @@ struct imx_rproc_mem {
> > size_t size;
> > };
> >
> > -/* att flags */
> > +/* att flags: lower 16 bits specifying core, higher 16 bits for flags
> > +*/
> > /* M4 own area. Can be mapped at probe */
> > -#define ATT_OWN BIT(1)
> > -#define ATT_IOMEM BIT(2)
> > +#define ATT_OWN BIT(31)
> > +#define ATT_IOMEM BIT(30)
> > +
> > +#define ATT_CORE_MASK 0xffff
> > +#define ATT_CORE(I) BIT((I))
> >
> > struct imx_rproc {
> > struct device *dev;
> > @@ -99,6 +103,7 @@ struct imx_rproc {
> > u32 rsrc_id; /* resource id */
> > u32 entry; /* cpu start address */
> > int num_pd;
> > + u32 core_index;
> > struct device **pd_dev;
> > struct device_link **pd_dev_link;
> > };
> > @@ -129,6 +134,19 @@ static const struct imx_rproc_att
> imx_rproc_att_imx93[] = {
> > { 0xD0000000, 0xa0000000, 0x10000000, 0 }, };
> >
> > +static const struct imx_rproc_att imx_rproc_att_imx8qm[] = {
> > + /* dev addr , sys addr , size , flags */
> > + { 0x08000000, 0x08000000, 0x10000000, 0},
> > + /* TCML */
> > + { 0x1FFE0000, 0x34FE0000, 0x00020000, ATT_OWN | ATT_IOMEM |
> ATT_CORE(0)},
> > + { 0x1FFE0000, 0x38FE0000, 0x00020000, ATT_OWN | ATT_IOMEM |
> ATT_CORE(1)},
> > + /* TCMU */
> > + { 0x20000000, 0x35000000, 0x00020000, ATT_OWN | ATT_IOMEM |
> ATT_CORE(0)},
> > + { 0x20000000, 0x39000000, 0x00020000, ATT_OWN | ATT_IOMEM |
> ATT_CORE(1)},
> > + /* DDR (Data) */
> > + { 0x80000000, 0x80000000, 0x60000000, 0 }, };
> > +
> > static const struct imx_rproc_att imx_rproc_att_imx8qxp[] = {
> > { 0x08000000, 0x08000000, 0x10000000, 0 },
> > /* TCML/U */
> > @@ -279,6 +297,12 @@ static const struct imx_rproc_dcfg
> imx_rproc_cfg_imx8mq = {
> > .method = IMX_RPROC_MMIO,
> > };
> >
> > +static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qm = {
> > + .att = imx_rproc_att_imx8qm,
> > + .att_size = ARRAY_SIZE(imx_rproc_att_imx8qm),
> > + .method = IMX_RPROC_SCU_API,
> > +};
> > +
> > static const struct imx_rproc_dcfg imx_rproc_cfg_imx8qxp = {
> > .att = imx_rproc_att_imx8qxp,
> > .att_size = ARRAY_SIZE(imx_rproc_att_imx8qxp),
> > @@ -395,6 +419,11 @@ static int imx_rproc_da_to_sys(struct imx_rproc
> *priv, u64 da,
> > for (i = 0; i < dcfg->att_size; i++) {
> > const struct imx_rproc_att *att = &dcfg->att[i];
> >
> > + if (att->flags & ATT_CORE_MASK) {
> > + if (!((BIT(priv->core_index)) & (att->flags & ATT_CORE_MASK)))
> > + continue;
> > + }
>
> This is very cryptic - I just spent 20 minutes looking at it and I'm still not sure I
> got the full meaning. Please add enough comments to make things obvious
> on first read.
There are two generic M4 cores in i.MX8QM, so core_index is 0 for M4_0,
and 1 for M4_1.
In the memory mapping array:
{ 0x1FFE0000, 0x34FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(0)},
{ 0x1FFE0000, 0x38FE0000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(1)},
/* TCMU */
{ 0x20000000, 0x35000000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(0)},
{ 0x20000000, 0x39000000, 0x00020000, ATT_OWN | ATT_IOMEM | ATT_CORE(1)},
ATT_CORE(0) means it is for M4_0, ATT_CORE(1) for M4_1.
Back to this piece code:
if (att->flags & ATT_CORE_MASK) {
if (!((BIT(priv->core_index)) & (att->flags & ATT_CORE_MASK)))
continue;
}
Taking M4_1 for example, priv->core_index is 1. So when it need translate an address with
ATT_CORE(X) flag, it should ignore ATT_CORE(0) entries. Hope this is clear. For adding
comments, how do you think:
/* Bypass the entries that not belong to the current remote core */
Thanks,
Peng.
>
> I am done reviewing this patchset.
>
> Thanks,
> Mathieu
>
>
> > +
> > if (da >= att->da && da + len < att->da + att->size) {
> > unsigned int offset = da - att->da;
> >
> > @@ -815,6 +844,11 @@ static int imx_rproc_detect_mode(struct
> imx_rproc *priv)
> > return ret;
> > }
> >
> > + if (priv->rsrc_id == IMX_SC_R_M4_1_PID0)
> > + priv->core_index = 1;
> > + else
> > + priv->core_index = 0;
> > +
> > /*
> > * If Mcore resource is not owned by Acore partition, It is kicked by
> ROM,
> > * and Linux could only do IPC with Mcore and nothing else.
> > @@ -1008,6 +1042,7 @@ static const struct of_device_id
> imx_rproc_of_match[] = {
> > { .compatible = "fsl,imx8mn-cm7", .data = &imx_rproc_cfg_imx8mn },
> > { .compatible = "fsl,imx8mp-cm7", .data = &imx_rproc_cfg_imx8mn },
> > { .compatible = "fsl,imx8qxp-cm4", .data = &imx_rproc_cfg_imx8qxp },
> > + { .compatible = "fsl,imx8qm-cm4", .data = &imx_rproc_cfg_imx8qm },
> > { .compatible = "fsl,imx8ulp-cm33", .data = &imx_rproc_cfg_imx8ulp },
> > { .compatible = "fsl,imx93-cm33", .data = &imx_rproc_cfg_imx93 },
> > {},
> > --
> > 2.25.1
> >