* RE: [PATCH v3] remoteproc: imx_dsp_rproc: add module parameter to ignore ready flag from remote processor
2023-01-17 11:03 [PATCH v3] remoteproc: imx_dsp_rproc: add module parameter to ignore ready flag from remote processor Iuliana Prodan (OSS)
@ 2023-01-18 5:25 ` S.J. Wang
2023-01-18 7:53 ` Daniel Baluta
2023-01-18 17:24 ` Mathieu Poirier
2 siblings, 0 replies; 5+ messages in thread
From: S.J. Wang @ 2023-01-18 5:25 UTC (permalink / raw)
To: Iuliana Prodan (OSS), Bjorn Andersson, Mathieu Poirier, Shawn Guo,
Sascha Hauer, Fabio Estevam, Daniel Baluta, Iuliana Prodan
Cc: dl-linux-imx, linux-remoteproc@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Pengutronix Kernel Team
>
> There are cases when we want to test a simple "hello world"
> application on the DSP and we don't have IPC between the cores.
> Therefore, skip the wait for remote processor to start.
>
> Added "ignore_dsp_ready" flag while inserting the module to ignore remote
> processor reply after start.
> By default, this is off - do not ignore reply from rproc.
>
> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Acked-by: Shengjiu Wang <shengjiu.wang@gmail.com>
Best regards
Wang Shengjiu
>
> ---
> Changes since v2
> - s/ignoreready/ignore_dsp_ready
>
> Changes since v1
> - change BIT(31) to BIT(1) for REMOTE_SKIP_WAIT
>
> ---
> drivers/remoteproc/imx_dsp_rproc.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/remoteproc/imx_dsp_rproc.c
> b/drivers/remoteproc/imx_dsp_rproc.c
> index 95da1cbefacf..22e2ef068c67 100644
> --- a/drivers/remoteproc/imx_dsp_rproc.c
> +++ b/drivers/remoteproc/imx_dsp_rproc.c
> @@ -26,9 +26,20 @@
> #include "remoteproc_elf_helpers.h"
> #include "remoteproc_internal.h"
>
> +#define IMX_DSP_IGNORE_REMOTE_READY 0
> +
> +/*
> + * Module parameters
> + */
> +static unsigned int imx_dsp_rproc_ignore_ready =
> +IMX_DSP_IGNORE_REMOTE_READY;
> module_param_named(ignore_dsp_ready,
> +imx_dsp_rproc_ignore_ready, int, 0644);
> MODULE_PARM_DESC(ignore_dsp_ready,
> + "Ignore remote proc reply after start, default is 0 (off).");
> +
> #define DSP_RPROC_CLK_MAX 5
>
> #define REMOTE_IS_READY BIT(0)
> +#define REMOTE_SKIP_WAIT BIT(1)
> #define REMOTE_READY_WAIT_MAX_RETRIES 500
>
> /* att flags */
> @@ -285,6 +296,9 @@ static int imx_dsp_rproc_ready(struct rproc *rproc)
> if (!priv->rxdb_ch)
> return 0;
>
> + if (priv->flags & REMOTE_SKIP_WAIT)
> + return 0;
> +
> for (i = 0; i < REMOTE_READY_WAIT_MAX_RETRIES; i++) {
> if (priv->flags & REMOTE_IS_READY)
> return 0;
> @@ -903,6 +917,9 @@ static int imx_dsp_rproc_probe(struct
> platform_device *pdev)
> priv->rproc = rproc;
> priv->dsp_dcfg = dsp_dcfg;
>
> + if (imx_dsp_rproc_ignore_ready)
> + priv->flags |= REMOTE_SKIP_WAIT;
> +
> dev_set_drvdata(dev, rproc);
>
> INIT_WORK(&priv->rproc_work, imx_dsp_rproc_vq_work);
> --
> 2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3] remoteproc: imx_dsp_rproc: add module parameter to ignore ready flag from remote processor
2023-01-17 11:03 [PATCH v3] remoteproc: imx_dsp_rproc: add module parameter to ignore ready flag from remote processor Iuliana Prodan (OSS)
2023-01-18 5:25 ` S.J. Wang
@ 2023-01-18 7:53 ` Daniel Baluta
2023-01-18 17:24 ` Mathieu Poirier
2 siblings, 0 replies; 5+ messages in thread
From: Daniel Baluta @ 2023-01-18 7:53 UTC (permalink / raw)
To: Iuliana Prodan (OSS)
Cc: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
S.J. Wang, Fabio Estevam, Daniel Baluta, Iuliana Prodan,
linux-imx, linux-remoteproc, linux-arm-kernel, linux-kernel,
Pengutronix Kernel Team
On Tue, Jan 17, 2023 at 1:10 PM Iuliana Prodan (OSS)
<iuliana.prodan@oss.nxp.com> wrote:
>
> From: Iuliana Prodan <iuliana.prodan@nxp.com>
>
> There are cases when we want to test a simple "hello world"
> application on the DSP and we don't have IPC between the cores.
> Therefore, skip the wait for remote processor to start.
>
> Added "ignore_dsp_ready" flag while inserting the module to ignore
> remote processor reply after start.
> By default, this is off - do not ignore reply from rproc.
>
> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] remoteproc: imx_dsp_rproc: add module parameter to ignore ready flag from remote processor
2023-01-17 11:03 [PATCH v3] remoteproc: imx_dsp_rproc: add module parameter to ignore ready flag from remote processor Iuliana Prodan (OSS)
2023-01-18 5:25 ` S.J. Wang
2023-01-18 7:53 ` Daniel Baluta
@ 2023-01-18 17:24 ` Mathieu Poirier
2023-01-18 23:55 ` Iuliana Prodan
2 siblings, 1 reply; 5+ messages in thread
From: Mathieu Poirier @ 2023-01-18 17:24 UTC (permalink / raw)
To: Iuliana Prodan (OSS)
Cc: Bjorn Andersson, Shawn Guo, Sascha Hauer, S.J. Wang,
Fabio Estevam, Daniel Baluta, Iuliana Prodan, linux-imx,
linux-remoteproc, linux-arm-kernel, linux-kernel,
Pengutronix Kernel Team
Hi Iuliana,
On Tue, Jan 17, 2023 at 01:03:57PM +0200, Iuliana Prodan (OSS) wrote:
> From: Iuliana Prodan <iuliana.prodan@nxp.com>
>
> There are cases when we want to test a simple "hello world"
> application on the DSP and we don't have IPC between the cores.
> Therefore, skip the wait for remote processor to start.
>
> Added "ignore_dsp_ready" flag while inserting the module to ignore
> remote processor reply after start.
> By default, this is off - do not ignore reply from rproc.
>
> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
>
> ---
> Changes since v2
> - s/ignoreready/ignore_dsp_ready
>
> Changes since v1
> - change BIT(31) to BIT(1) for REMOTE_SKIP_WAIT
>
> ---
> drivers/remoteproc/imx_dsp_rproc.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/remoteproc/imx_dsp_rproc.c b/drivers/remoteproc/imx_dsp_rproc.c
> index 95da1cbefacf..22e2ef068c67 100644
> --- a/drivers/remoteproc/imx_dsp_rproc.c
> +++ b/drivers/remoteproc/imx_dsp_rproc.c
> @@ -26,9 +26,20 @@
> #include "remoteproc_elf_helpers.h"
> #include "remoteproc_internal.h"
>
> +#define IMX_DSP_IGNORE_REMOTE_READY 0
> +
> +/*
> + * Module parameters
> + */
> +static unsigned int imx_dsp_rproc_ignore_ready = IMX_DSP_IGNORE_REMOTE_READY;
Static variables are initialised to '0' and as such this is not needed.
> +module_param_named(ignore_dsp_ready, imx_dsp_rproc_ignore_ready, int, 0644);
> +MODULE_PARM_DESC(ignore_dsp_ready,
> + "Ignore remote proc reply after start, default is 0 (off).");
> +
> #define DSP_RPROC_CLK_MAX 5
>
> #define REMOTE_IS_READY BIT(0)
> +#define REMOTE_SKIP_WAIT BIT(1)
> #define REMOTE_READY_WAIT_MAX_RETRIES 500
>
> /* att flags */
> @@ -285,6 +296,9 @@ static int imx_dsp_rproc_ready(struct rproc *rproc)
> if (!priv->rxdb_ch)
> return 0;
>
> + if (priv->flags & REMOTE_SKIP_WAIT)
> + return 0;
> +
This looks very hackish to me...
Here priv->rxdb_ch is valid and as such the DB mailbox has been setup, which
contradicts the commit log where it is stated that "we don't have IPC between
cores". Moreover, the commit log mentions to "skip the wait for remote
processor to start". How can the remote processor executed an sample
application if it is not ready?
Lastly, is there even a need to call imx_dsp_rproc_mbox_init() if an IPC is not
needed?
I'm fine with the module parameter but would much rather see a solution that
does not configure any kind of IPC related mechanic when it is not needed.
Thanks,
Mathieu
> for (i = 0; i < REMOTE_READY_WAIT_MAX_RETRIES; i++) {
> if (priv->flags & REMOTE_IS_READY)
> return 0;
> @@ -903,6 +917,9 @@ static int imx_dsp_rproc_probe(struct platform_device *pdev)
> priv->rproc = rproc;
> priv->dsp_dcfg = dsp_dcfg;
>
> + if (imx_dsp_rproc_ignore_ready)
> + priv->flags |= REMOTE_SKIP_WAIT;
> +
> dev_set_drvdata(dev, rproc);
>
> INIT_WORK(&priv->rproc_work, imx_dsp_rproc_vq_work);
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3] remoteproc: imx_dsp_rproc: add module parameter to ignore ready flag from remote processor
2023-01-18 17:24 ` Mathieu Poirier
@ 2023-01-18 23:55 ` Iuliana Prodan
0 siblings, 0 replies; 5+ messages in thread
From: Iuliana Prodan @ 2023-01-18 23:55 UTC (permalink / raw)
To: Mathieu Poirier, Iuliana Prodan (OSS)
Cc: Bjorn Andersson, Shawn Guo, Sascha Hauer, S.J. Wang,
Fabio Estevam, Daniel Baluta, linux-imx, linux-remoteproc,
linux-arm-kernel, linux-kernel, Pengutronix Kernel Team
On 1/18/2023 7:24 PM, Mathieu Poirier wrote:
> Hi Iuliana,
>
> On Tue, Jan 17, 2023 at 01:03:57PM +0200, Iuliana Prodan (OSS) wrote:
>> From: Iuliana Prodan <iuliana.prodan@nxp.com>
>>
>> There are cases when we want to test a simple "hello world"
>> application on the DSP and we don't have IPC between the cores.
>> Therefore, skip the wait for remote processor to start.
>>
>> Added "ignore_dsp_ready" flag while inserting the module to ignore
>> remote processor reply after start.
>> By default, this is off - do not ignore reply from rproc.
>>
>> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
>>
>> ---
>> Changes since v2
>> - s/ignoreready/ignore_dsp_ready
>>
>> Changes since v1
>> - change BIT(31) to BIT(1) for REMOTE_SKIP_WAIT
>>
>> ---
>> drivers/remoteproc/imx_dsp_rproc.c | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/remoteproc/imx_dsp_rproc.c b/drivers/remoteproc/imx_dsp_rproc.c
>> index 95da1cbefacf..22e2ef068c67 100644
>> --- a/drivers/remoteproc/imx_dsp_rproc.c
>> +++ b/drivers/remoteproc/imx_dsp_rproc.c
>> @@ -26,9 +26,20 @@
>> #include "remoteproc_elf_helpers.h"
>> #include "remoteproc_internal.h"
>>
>> +#define IMX_DSP_IGNORE_REMOTE_READY 0
>> +
>> +/*
>> + * Module parameters
>> + */
>> +static unsigned int imx_dsp_rproc_ignore_ready = IMX_DSP_IGNORE_REMOTE_READY;
> Static variables are initialised to '0' and as such this is not needed.
>
>> +module_param_named(ignore_dsp_ready, imx_dsp_rproc_ignore_ready, int, 0644);
>> +MODULE_PARM_DESC(ignore_dsp_ready,
>> + "Ignore remote proc reply after start, default is 0 (off).");
>> +
>> #define DSP_RPROC_CLK_MAX 5
>>
>> #define REMOTE_IS_READY BIT(0)
>> +#define REMOTE_SKIP_WAIT BIT(1)
>> #define REMOTE_READY_WAIT_MAX_RETRIES 500
>>
>> /* att flags */
>> @@ -285,6 +296,9 @@ static int imx_dsp_rproc_ready(struct rproc *rproc)
>> if (!priv->rxdb_ch)
>> return 0;
>>
>> + if (priv->flags & REMOTE_SKIP_WAIT)
>> + return 0;
>> +
> This looks very hackish to me...
>
> Here priv->rxdb_ch is valid and as such the DB mailbox has been setup, which
> contradicts the commit log where it is stated that "we don't have IPC between
> cores". Moreover, the commit log mentions to "skip the wait for remote
> processor to start". How can the remote processor executed an sample
> application if it is not ready?
>
> Lastly, is there even a need to call imx_dsp_rproc_mbox_init() if an IPC is not
> needed?
>
> I'm fine with the module parameter but would much rather see a solution that
> does not configure any kind of IPC related mechanic when it is not needed.
>
> Thanks,
> Mathieu
Hi Mathieu,
I've tested this with hello_world sample from Zephyr.
This was loaded on a hifi4 core from Linux using remoteproc. And, with
this patch is working, otherwise I get "can't start rproc imx-dsp-rproc:
-110:" because the ARM core is waiting for a reply from the hifi4.
I agree, I shouldn't initialize mbox if there is no IPC between the cores.
I'll fix this in a v4.
Thanks,
Iulia
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread