| Message ID | 20230306231202.12223-7-quic_molvera@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | remoteproc: qcom_q6v5_pas: Add support for QDU1000/QRU1000 mpss | expand |
On Mon, Mar 06, 2023 at 03:12:01PM -0800, Melody Olvera wrote: > Some chipsets will have DSPs which will have begun running prior > to linux booting, so add support to late attach these DSPs by > adding support for: > - run-time checking of an offline or running DSP via rmb register > - a late attach framework to attach to the running DSP > - a handshake mechanism to ensure full and proper booting via rmb > > Signed-off-by: Melody Olvera <quic_molvera@quicinc.com> > --- > drivers/remoteproc/qcom_q6v5_pas.c | 103 ++++++++++++++++++++++++++++- > 1 file changed, 102 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > index 0871108fb4dc..e22be6a029a8 100644 > --- a/drivers/remoteproc/qcom_q6v5_pas.c > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > @@ -242,10 +242,89 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw) > return ret; > } > > +static int adsp_attach(struct rproc *rproc) > +{ > + struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; > + int i, ret; > + > + ret = qcom_q6v5_prepare(&adsp->q6v5); > + if (ret) > + return ret; > + > + ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); > + if (ret < 0) > + goto disable_irqs; > + > + ret = clk_prepare_enable(adsp->xo); > + if (ret) > + goto disable_proxy_pds; > + > + ret = clk_prepare_enable(adsp->aggre2_clk); > + if (ret) > + goto disable_xo_clk; > + > + if (adsp->cx_supply) { > + ret = regulator_enable(adsp->cx_supply); > + if (ret) > + goto disable_aggre2_clk; > + } > + > + if (adsp->px_supply) { > + ret = regulator_enable(adsp->px_supply); > + if (ret) > + goto disable_cx_supply; > + } > + > + /* if needed, signal Q6 to continute booting */ > + if (adsp->q6v5.rmb_base) { > + for (i = 0; i < RMB_POLL_MAX_TIMES; i++) { > + if (readl_relaxed(adsp->q6v5.rmb_base + RMB_BOOT_WAIT_REG)) { > + writel_relaxed(1, adsp->q6v5.rmb_base + RMB_BOOT_CONT_REG); > + break; > + } > + msleep(20); > + } > + > + if (!readl_relaxed(adsp->q6v5.rmb_base + RMB_BOOT_WAIT_REG)) { > + dev_err(adsp->dev, "Didn't get rmb signal from %s\n", rproc->name); > + goto disable_px_supply; > + } > + } > + > + ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000)); > + if (ret == -ETIMEDOUT) { > + dev_err(adsp->dev, "start timed out\n"); > + qcom_scm_pas_shutdown(adsp->pas_id); > + goto disable_px_supply; > + } > + > + return 0; > + > +disable_px_supply: > + if (adsp->px_supply) > + regulator_disable(adsp->px_supply); > +disable_cx_supply: > + if (adsp->cx_supply) > + regulator_disable(adsp->cx_supply); > +disable_aggre2_clk: > + clk_disable_unprepare(adsp->aggre2_clk); > +disable_xo_clk: > + clk_disable_unprepare(adsp->xo); > +disable_proxy_pds: > + adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); > +disable_irqs: > + qcom_q6v5_unprepare(&adsp->q6v5); > + > + /* Remove pointer to the loaded firmware, only valid in adsp_load() & adsp_start() */ > + adsp->firmware = NULL; > + > + return ret; > +} > + > static int adsp_start(struct rproc *rproc) > { > struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; > - int ret; > + int i, ret; > > ret = qcom_q6v5_prepare(&adsp->q6v5); > if (ret) > @@ -304,6 +383,22 @@ static int adsp_start(struct rproc *rproc) > goto release_pas_metadata; > } > > + /* if needed, signal Q6 to continute booting */ Why does this come before the wait_for_start()? Is the DSP actually up and running when you hit attach, or is it just loaded? > + if (adsp->q6v5.rmb_base) { Afaict this is copy-paste from attach, please move it to a helper function. > + for (i = 0; i < RMB_POLL_MAX_TIMES; i++) { > + if (readl_relaxed(adsp->q6v5.rmb_base + RMB_BOOT_WAIT_REG)) { > + writel_relaxed(1, adsp->q6v5.rmb_base + RMB_BOOT_CONT_REG); > + break; > + } > + msleep(20); > + } > + > + if (!readl_relaxed(adsp->q6v5.rmb_base + RMB_BOOT_WAIT_REG)) { If you hit the break above, there should be no reason to read this register again. Seems cleaner to write this as: ret = readl_poll_timeout(); if (ret < 0) goto release; writel(1, ...); Regards, Bjorn > + dev_err(adsp->dev, "Didn't get rmb signal from %s\n", rproc->name); > + goto release_pas_metadata; > + } > + } > + > ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000)); > if (ret == -ETIMEDOUT) { > dev_err(adsp->dev, "start timed out\n"); > @@ -413,6 +508,7 @@ static unsigned long adsp_panic(struct rproc *rproc) > static const struct rproc_ops adsp_ops = { > .unprepare = adsp_unprepare, > .start = adsp_start, > + .attach = adsp_attach, > .stop = adsp_stop, > .da_to_va = adsp_da_to_va, > .parse_fw = qcom_register_dump_segments, > @@ -423,6 +519,7 @@ static const struct rproc_ops adsp_ops = { > static const struct rproc_ops adsp_minidump_ops = { > .unprepare = adsp_unprepare, > .start = adsp_start, > + .attach = adsp_attach, > .stop = adsp_stop, > .da_to_va = adsp_da_to_va, > .load = adsp_load, > @@ -728,6 +825,10 @@ static int adsp_probe(struct platform_device *pdev) > if (ret) > goto detach_proxy_pds; > > + if (adsp->q6v5.rmb_base && > + readl_relaxed(adsp->q6v5.rmb_base + RMB_Q6_BOOT_STATUS_REG)) > + rproc->state = RPROC_DETACHED; > + > qcom_add_glink_subdev(rproc, &adsp->glink_subdev, desc->ssr_name); > qcom_add_smd_subdev(rproc, &adsp->smd_subdev); > adsp->sysmon = qcom_add_sysmon_subdev(rproc, > -- > 2.25.1 >
On 3/15/2023 7:27 PM, Bjorn Andersson wrote: > On Mon, Mar 06, 2023 at 03:12:01PM -0800, Melody Olvera wrote: >> Some chipsets will have DSPs which will have begun running prior >> to linux booting, so add support to late attach these DSPs by >> adding support for: >> - run-time checking of an offline or running DSP via rmb register >> - a late attach framework to attach to the running DSP >> - a handshake mechanism to ensure full and proper booting via rmb >> >> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com> >> --- >> drivers/remoteproc/qcom_q6v5_pas.c | 103 ++++++++++++++++++++++++++++- >> 1 file changed, 102 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c >> index 0871108fb4dc..e22be6a029a8 100644 >> --- a/drivers/remoteproc/qcom_q6v5_pas.c >> +++ b/drivers/remoteproc/qcom_q6v5_pas.c >> @@ -242,10 +242,89 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw) >> return ret; >> } >> >> +static int adsp_attach(struct rproc *rproc) >> +{ >> + struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; >> + int i, ret; >> + >> + ret = qcom_q6v5_prepare(&adsp->q6v5); >> + if (ret) >> + return ret; >> + >> + ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); >> + if (ret < 0) >> + goto disable_irqs; >> + >> + ret = clk_prepare_enable(adsp->xo); >> + if (ret) >> + goto disable_proxy_pds; >> + >> + ret = clk_prepare_enable(adsp->aggre2_clk); >> + if (ret) >> + goto disable_xo_clk; >> + >> + if (adsp->cx_supply) { >> + ret = regulator_enable(adsp->cx_supply); >> + if (ret) >> + goto disable_aggre2_clk; >> + } >> + >> + if (adsp->px_supply) { >> + ret = regulator_enable(adsp->px_supply); >> + if (ret) >> + goto disable_cx_supply; >> + } >> + >> + /* if needed, signal Q6 to continute booting */ >> + if (adsp->q6v5.rmb_base) { >> + for (i = 0; i < RMB_POLL_MAX_TIMES; i++) { >> + if (readl_relaxed(adsp->q6v5.rmb_base + RMB_BOOT_WAIT_REG)) { >> + writel_relaxed(1, adsp->q6v5.rmb_base + RMB_BOOT_CONT_REG); >> + break; >> + } >> + msleep(20); >> + } >> + >> + if (!readl_relaxed(adsp->q6v5.rmb_base + RMB_BOOT_WAIT_REG)) { >> + dev_err(adsp->dev, "Didn't get rmb signal from %s\n", rproc->name); >> + goto disable_px_supply; >> + } >> + } >> + >> + ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000)); >> + if (ret == -ETIMEDOUT) { >> + dev_err(adsp->dev, "start timed out\n"); >> + qcom_scm_pas_shutdown(adsp->pas_id); >> + goto disable_px_supply; >> + } >> + >> + return 0; >> + >> +disable_px_supply: >> + if (adsp->px_supply) >> + regulator_disable(adsp->px_supply); >> +disable_cx_supply: >> + if (adsp->cx_supply) >> + regulator_disable(adsp->cx_supply); >> +disable_aggre2_clk: >> + clk_disable_unprepare(adsp->aggre2_clk); >> +disable_xo_clk: >> + clk_disable_unprepare(adsp->xo); >> +disable_proxy_pds: >> + adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); >> +disable_irqs: >> + qcom_q6v5_unprepare(&adsp->q6v5); >> + >> + /* Remove pointer to the loaded firmware, only valid in adsp_load() & adsp_start() */ >> + adsp->firmware = NULL; >> + >> + return ret; >> +} >> + >> static int adsp_start(struct rproc *rproc) >> { >> struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; >> - int ret; >> + int i, ret; >> >> ret = qcom_q6v5_prepare(&adsp->q6v5); >> if (ret) >> @@ -304,6 +383,22 @@ static int adsp_start(struct rproc *rproc) >> goto release_pas_metadata; >> } >> >> + /* if needed, signal Q6 to continute booting */ > Why does this come before the wait_for_start()? Is the DSP actually up > and running when you hit attach, or is it just loaded? Yeah so DSP is loaded and partially booted, but it's waiting for a signal from our driver to complete the boot process through to err_ready. > >> + if (adsp->q6v5.rmb_base) { > Afaict this is copy-paste from attach, please move it to a helper > function. Sure. > >> + for (i = 0; i < RMB_POLL_MAX_TIMES; i++) { >> + if (readl_relaxed(adsp->q6v5.rmb_base + RMB_BOOT_WAIT_REG)) { >> + writel_relaxed(1, adsp->q6v5.rmb_base + RMB_BOOT_CONT_REG); >> + break; >> + } >> + msleep(20); >> + } >> + >> + if (!readl_relaxed(adsp->q6v5.rmb_base + RMB_BOOT_WAIT_REG)) { > If you hit the break above, there should be no reason to read this > register again. > > Seems cleaner to write this as: > > ret = readl_poll_timeout(); > if (ret < 0) > goto release; > > writel(1, ...); That is much cleaner; I didn't know about the poll timeout function. Will rewrite using it. Thanks, Melody > > Regards, > Bjorn > >> + dev_err(adsp->dev, "Didn't get rmb signal from %s\n", rproc->name); >> + goto release_pas_metadata; >> + } >> + } >> + >> ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000)); >> if (ret == -ETIMEDOUT) { >> dev_err(adsp->dev, "start timed out\n"); >> @@ -413,6 +508,7 @@ static unsigned long adsp_panic(struct rproc *rproc) >> static const struct rproc_ops adsp_ops = { >> .unprepare = adsp_unprepare, >> .start = adsp_start, >> + .attach = adsp_attach, >> .stop = adsp_stop, >> .da_to_va = adsp_da_to_va, >> .parse_fw = qcom_register_dump_segments, >> @@ -423,6 +519,7 @@ static const struct rproc_ops adsp_ops = { >> static const struct rproc_ops adsp_minidump_ops = { >> .unprepare = adsp_unprepare, >> .start = adsp_start, >> + .attach = adsp_attach, >> .stop = adsp_stop, >> .da_to_va = adsp_da_to_va, >> .load = adsp_load, >> @@ -728,6 +825,10 @@ static int adsp_probe(struct platform_device *pdev) >> if (ret) >> goto detach_proxy_pds; >> >> + if (adsp->q6v5.rmb_base && >> + readl_relaxed(adsp->q6v5.rmb_base + RMB_Q6_BOOT_STATUS_REG)) >> + rproc->state = RPROC_DETACHED; >> + >> qcom_add_glink_subdev(rproc, &adsp->glink_subdev, desc->ssr_name); >> qcom_add_smd_subdev(rproc, &adsp->smd_subdev); >> adsp->sysmon = qcom_add_sysmon_subdev(rproc, >> -- >> 2.25.1 >>
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c index 0871108fb4dc..e22be6a029a8 100644 --- a/drivers/remoteproc/qcom_q6v5_pas.c +++ b/drivers/remoteproc/qcom_q6v5_pas.c @@ -242,10 +242,89 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw) return ret; } +static int adsp_attach(struct rproc *rproc) +{ + struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; + int i, ret; + + ret = qcom_q6v5_prepare(&adsp->q6v5); + if (ret) + return ret; + + ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); + if (ret < 0) + goto disable_irqs; + + ret = clk_prepare_enable(adsp->xo); + if (ret) + goto disable_proxy_pds; + + ret = clk_prepare_enable(adsp->aggre2_clk); + if (ret) + goto disable_xo_clk; + + if (adsp->cx_supply) { + ret = regulator_enable(adsp->cx_supply); + if (ret) + goto disable_aggre2_clk; + } + + if (adsp->px_supply) { + ret = regulator_enable(adsp->px_supply); + if (ret) + goto disable_cx_supply; + } + + /* if needed, signal Q6 to continute booting */ + if (adsp->q6v5.rmb_base) { + for (i = 0; i < RMB_POLL_MAX_TIMES; i++) { + if (readl_relaxed(adsp->q6v5.rmb_base + RMB_BOOT_WAIT_REG)) { + writel_relaxed(1, adsp->q6v5.rmb_base + RMB_BOOT_CONT_REG); + break; + } + msleep(20); + } + + if (!readl_relaxed(adsp->q6v5.rmb_base + RMB_BOOT_WAIT_REG)) { + dev_err(adsp->dev, "Didn't get rmb signal from %s\n", rproc->name); + goto disable_px_supply; + } + } + + ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000)); + if (ret == -ETIMEDOUT) { + dev_err(adsp->dev, "start timed out\n"); + qcom_scm_pas_shutdown(adsp->pas_id); + goto disable_px_supply; + } + + return 0; + +disable_px_supply: + if (adsp->px_supply) + regulator_disable(adsp->px_supply); +disable_cx_supply: + if (adsp->cx_supply) + regulator_disable(adsp->cx_supply); +disable_aggre2_clk: + clk_disable_unprepare(adsp->aggre2_clk); +disable_xo_clk: + clk_disable_unprepare(adsp->xo); +disable_proxy_pds: + adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); +disable_irqs: + qcom_q6v5_unprepare(&adsp->q6v5); + + /* Remove pointer to the loaded firmware, only valid in adsp_load() & adsp_start() */ + adsp->firmware = NULL; + + return ret; +} + static int adsp_start(struct rproc *rproc) { struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; - int ret; + int i, ret; ret = qcom_q6v5_prepare(&adsp->q6v5); if (ret) @@ -304,6 +383,22 @@ static int adsp_start(struct rproc *rproc) goto release_pas_metadata; } + /* if needed, signal Q6 to continute booting */ + if (adsp->q6v5.rmb_base) { + for (i = 0; i < RMB_POLL_MAX_TIMES; i++) { + if (readl_relaxed(adsp->q6v5.rmb_base + RMB_BOOT_WAIT_REG)) { + writel_relaxed(1, adsp->q6v5.rmb_base + RMB_BOOT_CONT_REG); + break; + } + msleep(20); + } + + if (!readl_relaxed(adsp->q6v5.rmb_base + RMB_BOOT_WAIT_REG)) { + dev_err(adsp->dev, "Didn't get rmb signal from %s\n", rproc->name); + goto release_pas_metadata; + } + } + ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000)); if (ret == -ETIMEDOUT) { dev_err(adsp->dev, "start timed out\n"); @@ -413,6 +508,7 @@ static unsigned long adsp_panic(struct rproc *rproc) static const struct rproc_ops adsp_ops = { .unprepare = adsp_unprepare, .start = adsp_start, + .attach = adsp_attach, .stop = adsp_stop, .da_to_va = adsp_da_to_va, .parse_fw = qcom_register_dump_segments, @@ -423,6 +519,7 @@ static const struct rproc_ops adsp_ops = { static const struct rproc_ops adsp_minidump_ops = { .unprepare = adsp_unprepare, .start = adsp_start, + .attach = adsp_attach, .stop = adsp_stop, .da_to_va = adsp_da_to_va, .load = adsp_load, @@ -728,6 +825,10 @@ static int adsp_probe(struct platform_device *pdev) if (ret) goto detach_proxy_pds; + if (adsp->q6v5.rmb_base && + readl_relaxed(adsp->q6v5.rmb_base + RMB_Q6_BOOT_STATUS_REG)) + rproc->state = RPROC_DETACHED; + qcom_add_glink_subdev(rproc, &adsp->glink_subdev, desc->ssr_name); qcom_add_smd_subdev(rproc, &adsp->smd_subdev); adsp->sysmon = qcom_add_sysmon_subdev(rproc,
Some chipsets will have DSPs which will have begun running prior to linux booting, so add support to late attach these DSPs by adding support for: - run-time checking of an offline or running DSP via rmb register - a late attach framework to attach to the running DSP - a handshake mechanism to ensure full and proper booting via rmb Signed-off-by: Melody Olvera <quic_molvera@quicinc.com> --- drivers/remoteproc/qcom_q6v5_pas.c | 103 ++++++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 1 deletion(-)