Re: [PATCH v2 2/2] remoteproc: mediatek: Support MT8188 SCP core 1
Hi Mathieu,
Thanks for the reviews.
I will correct it in the next version.
Best regards,
Olivia
On Mon, 2024-04-22 at 09:33 -0600, Mathieu Poirier wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Hi Olivia,
>
> On Fri, Apr 19, 2024 at 04:42:11PM +0800, Olivia Wen wrote:
> > From: "olivia.wen"
> >
> > There are three primary modifications.
> >
> > 1. The struct mtk_scp_of_data usage on MT8188
> > MT8192 functions are unsuitable for the dual-core MT8188 SCP,
> > which has two RISC-V cores similar to MT8195 but without L1TCM.
> > We've added MT8188-specific functions to configure L1TCM
> > in multicore setups.
> >
> > 2. SCP_IPI_IMGSYS_CMD feature
> > This version also adds SCP_IPI_IMGSYS_CMD to facilitate
> > communication between the imgsys kernel and the backend driver.
> >
> > 3. Different code sizes and IPI share buffer sizes
> > Each SCP necessitates different code and IPI share buffer sizes.
> > Introducing a structure mtk_scp_sizes_data to handle them.
> >
>
> Please split in 3 different patches and in the changelog, concentrate
> on "why"
> you are making the changes rather than "what" changes are done.
>
> Thanks,
> Mathieu
>
> > Signed-off-by: olivia.wen
> > ---
> > drivers/remoteproc/mtk_common.h| 11 +-
> > drivers/remoteproc/mtk_scp.c | 230
> +
> > drivers/remoteproc/mtk_scp_ipi.c | 7 +-
> > include/linux/remoteproc/mtk_scp.h | 1 +
> > 4 files changed, 223 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/remoteproc/mtk_common.h
> b/drivers/remoteproc/mtk_common.h
> > index 6d7736a..fd5c539 100644
> > --- a/drivers/remoteproc/mtk_common.h
> > +++ b/drivers/remoteproc/mtk_common.h
> > @@ -78,7 +78,6 @@
> > #define MT8195_L2TCM_OFFSET0x850d0
> >
> > #define SCP_FW_VER_LEN32
> > -#define SCP_SHARE_BUFFER_SIZE288
> >
> > struct scp_run {
> > u32 signaled;
> > @@ -97,6 +96,11 @@ struct scp_ipi_desc {
> >
> > struct mtk_scp;
> >
> > +struct mtk_scp_sizes_data {
> > +size_t max_dram_size;
> > +size_t ipi_share_buffer_size;
> > +};
> > +
> > struct mtk_scp_of_data {
> > int (*scp_clk_get)(struct mtk_scp *scp);
> > int (*scp_before_load)(struct mtk_scp *scp);
> > @@ -110,6 +114,7 @@ struct mtk_scp_of_data {
> > u32 host_to_scp_int_bit;
> >
> > size_t ipi_buf_offset;
> > +const struct mtk_scp_sizes_data *scp_sizes;
> > };
> >
> > struct mtk_scp_of_cluster {
> > @@ -141,10 +146,10 @@ struct mtk_scp {
> > struct scp_ipi_desc ipi_desc[SCP_IPI_MAX];
> > bool ipi_id_ack[SCP_IPI_MAX];
> > wait_queue_head_t ack_wq;
> > +u8 *share_buf;
> >
> > void *cpu_addr;
> > dma_addr_t dma_addr;
> > -size_t dram_size;
> >
> > struct rproc_subdev *rpmsg_subdev;
> >
> > @@ -162,7 +167,7 @@ struct mtk_scp {
> > struct mtk_share_obj {
> > u32 id;
> > u32 len;
> > -u8 share_buf[SCP_SHARE_BUFFER_SIZE];
> > +u8 *share_buf;
> > };
> >
> > void scp_memcpy_aligned(void __iomem *dst, const void *src,
> unsigned int len);
> > diff --git a/drivers/remoteproc/mtk_scp.c
> b/drivers/remoteproc/mtk_scp.c
> > index 6751829..e281d28 100644
> > --- a/drivers/remoteproc/mtk_scp.c
> > +++ b/drivers/remoteproc/mtk_scp.c
> > @@ -20,7 +20,6 @@
> > #include "mtk_common.h"
> > #include "remoteproc_internal.h"
> >
> > -#define MAX_CODE_SIZE 0x50
> > #define SECTION_NAME_IPI_BUFFER ".ipi_buffer"
> >
> > /**
> > @@ -94,14 +93,15 @@ static void scp_ipi_handler(struct mtk_scp
> *scp)
> > {
> > struct mtk_share_obj __iomem *rcv_obj = scp->recv_buf;
> > struct scp_ipi_desc *ipi_desc = scp->ipi_desc;
> > -u8 tmp_data[SCP_SHARE_BUFFER_SIZE];
> > scp_ipi_handler_t handler;
> > u32 id = readl(_obj->id);
> > u32 len = readl(_obj->len);
> > +const struct mtk_scp_sizes_data *scp_sizes;
> >
> > -if (len > SCP_SHARE_BUFFER_SIZE) {
> > -dev_err(scp->dev, "ipi message too long (len %d, max %d)", len,
> > -SCP_SHARE_BUFFER_SIZE);
> > +scp_sizes = scp->data->scp_sizes;
> > +if (len > scp_sizes->ipi_share_buffer_size) {
> > +dev_err(scp->dev, "ipi message too long (len %d, max %zd)", len,
> > +scp_sizes->ipi_share_buffer_size);
> > return;
> > }
> > if (id >= SCP_IPI_MAX) {
> > @@ -117,8 +117,9 @@ static void scp_ipi_handler(struct mtk_scp
> *scp)
> > return;
> > }
> >
> > -memcpy_fromio(tmp_data, _obj->share_buf, len);
> > -handler(tmp_data, len, ipi_desc[id].priv);
> > +memset(scp->share_buf, 0, scp_sizes->ipi_share_buffer_size);
> > +memcpy_fromio(scp->share_buf, _obj->share_buf, len);
> > +handler(scp->share_buf, len, ipi_desc[id].priv);
> > scp_ipi_unlock(scp, id);
> >
> > scp->ipi_id_ack[id] = true;
> > @@ -133,6 +134,8 @@ static int scp_ipi_init(struct mtk_scp *scp,
> const struct firmware *fw)
> > {
> > int ret;
> > size_t buf_sz, offset;
> > +size_t share_buf_offset;
> > +const struct mtk_scp_sizes_data *scp_sizes;
> >
> > /* read the ipi buf addr from FW itself first */
> > ret = scp_elf_read_ipi_buf_addr(scp, fw,
Re: [PATCH v2 2/2] remoteproc: mediatek: Support MT8188 SCP core 1
Hi Olivia,
On Fri, Apr 19, 2024 at 04:42:11PM +0800, Olivia Wen wrote:
> From: "olivia.wen"
>
> There are three primary modifications.
>
> 1. The struct mtk_scp_of_data usage on MT8188
> MT8192 functions are unsuitable for the dual-core MT8188 SCP,
> which has two RISC-V cores similar to MT8195 but without L1TCM.
> We've added MT8188-specific functions to configure L1TCM
> in multicore setups.
>
> 2. SCP_IPI_IMGSYS_CMD feature
> This version also adds SCP_IPI_IMGSYS_CMD to facilitate
> communication between the imgsys kernel and the backend driver.
>
> 3. Different code sizes and IPI share buffer sizes
> Each SCP necessitates different code and IPI share buffer sizes.
> Introducing a structure mtk_scp_sizes_data to handle them.
>
Please split in 3 different patches and in the changelog, concentrate on "why"
you are making the changes rather than "what" changes are done.
Thanks,
Mathieu
> Signed-off-by: olivia.wen
> ---
> drivers/remoteproc/mtk_common.h| 11 +-
> drivers/remoteproc/mtk_scp.c | 230
> +
> drivers/remoteproc/mtk_scp_ipi.c | 7 +-
> include/linux/remoteproc/mtk_scp.h | 1 +
> 4 files changed, 223 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
> index 6d7736a..fd5c539 100644
> --- a/drivers/remoteproc/mtk_common.h
> +++ b/drivers/remoteproc/mtk_common.h
> @@ -78,7 +78,6 @@
> #define MT8195_L2TCM_OFFSET 0x850d0
>
> #define SCP_FW_VER_LEN 32
> -#define SCP_SHARE_BUFFER_SIZE288
>
> struct scp_run {
> u32 signaled;
> @@ -97,6 +96,11 @@ struct scp_ipi_desc {
>
> struct mtk_scp;
>
> +struct mtk_scp_sizes_data {
> + size_t max_dram_size;
> + size_t ipi_share_buffer_size;
> +};
> +
> struct mtk_scp_of_data {
> int (*scp_clk_get)(struct mtk_scp *scp);
> int (*scp_before_load)(struct mtk_scp *scp);
> @@ -110,6 +114,7 @@ struct mtk_scp_of_data {
> u32 host_to_scp_int_bit;
>
> size_t ipi_buf_offset;
> + const struct mtk_scp_sizes_data *scp_sizes;
> };
>
> struct mtk_scp_of_cluster {
> @@ -141,10 +146,10 @@ struct mtk_scp {
> struct scp_ipi_desc ipi_desc[SCP_IPI_MAX];
> bool ipi_id_ack[SCP_IPI_MAX];
> wait_queue_head_t ack_wq;
> + u8 *share_buf;
>
> void *cpu_addr;
> dma_addr_t dma_addr;
> - size_t dram_size;
>
> struct rproc_subdev *rpmsg_subdev;
>
> @@ -162,7 +167,7 @@ struct mtk_scp {
> struct mtk_share_obj {
> u32 id;
> u32 len;
> - u8 share_buf[SCP_SHARE_BUFFER_SIZE];
> + u8 *share_buf;
> };
>
> void scp_memcpy_aligned(void __iomem *dst, const void *src, unsigned int
> len);
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index 6751829..e281d28 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -20,7 +20,6 @@
> #include "mtk_common.h"
> #include "remoteproc_internal.h"
>
> -#define MAX_CODE_SIZE 0x50
> #define SECTION_NAME_IPI_BUFFER ".ipi_buffer"
>
> /**
> @@ -94,14 +93,15 @@ static void scp_ipi_handler(struct mtk_scp *scp)
> {
> struct mtk_share_obj __iomem *rcv_obj = scp->recv_buf;
> struct scp_ipi_desc *ipi_desc = scp->ipi_desc;
> - u8 tmp_data[SCP_SHARE_BUFFER_SIZE];
> scp_ipi_handler_t handler;
> u32 id = readl(_obj->id);
> u32 len = readl(_obj->len);
> + const struct mtk_scp_sizes_data *scp_sizes;
>
> - if (len > SCP_SHARE_BUFFER_SIZE) {
> - dev_err(scp->dev, "ipi message too long (len %d, max %d)", len,
> - SCP_SHARE_BUFFER_SIZE);
> + scp_sizes = scp->data->scp_sizes;
> + if (len > scp_sizes->ipi_share_buffer_size) {
> + dev_err(scp->dev, "ipi message too long (len %d, max %zd)", len,
> + scp_sizes->ipi_share_buffer_size);
> return;
> }
> if (id >= SCP_IPI_MAX) {
> @@ -117,8 +117,9 @@ static void scp_ipi_handler(struct mtk_scp *scp)
> return;
> }
>
> - memcpy_fromio(tmp_data, _obj->share_buf, len);
> - handler(tmp_data, len, ipi_desc[id].priv);
> + memset(scp->share_buf, 0, scp_sizes->ipi_share_buffer_size);
> + memcpy_fromio(scp->share_buf, _obj->share_buf, len);
> + handler(scp->share_buf, len, ipi_desc[id].priv);
> scp_ipi_unlock(scp, id);
>
> scp->ipi_id_ack[id] = true;
> @@ -133,6 +134,8 @@ static int scp_ipi_init(struct mtk_scp *scp, const struct
> firmware *fw)
> {
> int ret;
> size_t buf_sz, offset;
> + size_t share_buf_offset;
> + const struct mtk_scp_sizes_data *scp_sizes;
>
> /* read the ipi buf addr from FW itself first */
> ret = scp_elf_read_ipi_buf_addr(scp, fw, );
> @@ -152,12 +155,15 @@ static int scp_ipi_init(struct mtk_scp *scp, const
> struct firmware *fw)
> return -EOVERFLOW;
> }
>
> + scp_sizes = scp->data->scp_sizes;
>
Re: [PATCH v2 2/2] remoteproc: mediatek: Support MT8188 SCP core 1
Il 19/04/24 10:42, Olivia Wen ha scritto: From: "olivia.wen" There are three primary modifications. 1. The struct mtk_scp_of_data usage on MT8188 MT8192 functions are unsuitable for the dual-core MT8188 SCP, which has two RISC-V cores similar to MT8195 but without L1TCM. We've added MT8188-specific functions to configure L1TCM in multicore setups. 2. SCP_IPI_IMGSYS_CMD feature This version also adds SCP_IPI_IMGSYS_CMD to facilitate communication between the imgsys kernel and the backend driver. 3. Different code sizes and IPI share buffer sizes Each SCP necessitates different code and IPI share buffer sizes. Introducing a structure mtk_scp_sizes_data to handle them. Signed-off-by: olivia.wen Reviewed-by: AngeloGioacchino Del Regno
