| Message ID | 35eb4ce2bb8f3bb78a616c071a012f1f49d8b593.1666127246.git.quic_gokukris@quicinc.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v1,1/1] drivers: remoteproc: Add bus scaling capability during bootup | expand |
On Oct 18 2022 14:10, Gokul krishna Krishnakumar wrote: > During bootup since remote processors cannot request for > additional bus bandwidth from the interconect framework, > platform driver should provide the proxy resources. Make > a proxy vote for maximizing the bus bandwidth during bootup > for a remote processor and remove it once processor is up. > > Change-Id: I798b3b000aef4761a7ff8cb6390b7ecd98f925b7 Please remove Change-Ids - they are not for upstream. Thank you. Guru Das.
On 18/10/2022 17:10, Gokul krishna Krishnakumar wrote: > During bootup since remote processors cannot request for > additional bus bandwidth from the interconect framework, > platform driver should provide the proxy resources. Make > a proxy vote for maximizing the bus bandwidth during bootup > for a remote processor and remove it once processor is up. > (...) > > @@ -265,6 +340,7 @@ static void qcom_pas_handover(struct qcom_q6v5 *q6v5) > clk_disable_unprepare(adsp->aggre2_clk); > clk_disable_unprepare(adsp->xo); > adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); > + do_bus_scaling(adsp, false); > } > > static int adsp_stop(struct rproc *rproc) > @@ -383,6 +459,22 @@ static int adsp_init_regulator(struct qcom_adsp *adsp) > return 0; > } > > +static void adsp_init_bus_scaling(struct qcom_adsp *adsp) > +{ > + if (scm_perf_client) > + goto get_rproc_client; > + > + scm_perf_client = of_icc_get(adsp->dev, "crypto_ddr"); Aren't you adding here new DT properties to the device? If so, this requires updating bindings. > + if (IS_ERR(scm_perf_client)) > + dev_warn(adsp->dev, "Crypto scaling not setup\n"); > + > +get_rproc_client: > + adsp->bus_client = of_icc_get(adsp->dev, "rproc_ddr"); > + if (IS_ERR(adsp->bus_client)) > + dev_warn(adsp->dev, "%s: No bus client\n", __func__); > + > +} Best regards, Krzysztof
Hi Gokul, On 18/10/2022 23:10, Gokul krishna Krishnakumar wrote: > During bootup since remote processors cannot request for > additional bus bandwidth from the interconect framework, > platform driver should provide the proxy resources. Make > a proxy vote for maximizing the bus bandwidth during bootup > for a remote processor and remove it once processor is up. A similar change has been upstreamed in - [0] 65b7ebda5028 ("firmware: qcom_scm: Add bw voting support to the SCM interface") - [1] 8d9be5c6bdcd ("remoteproc: qcom: q6v5: Add interconnect path proxy vote") The main difference with this patch are: - [0] only votes for each scm calls, not across multiple calls, is that a problem ? - [0] & [1] votes (0, UINT_MAX) but you change votes (UINT_MAX, UINT_MAX), does this make a difference ? [0] https://github.com/torvalds/linux/commit/65b7ebda5028 [1] https://github.com/torvalds/linux/commit/8d9be5c6bdcd > > Change-Id: I798b3b000aef4761a7ff8cb6390b7ecd98f925b7 > Signed-off-by: Gokul krishna Krishnakumar <quic_gokukris@quicinc.com> > --- > drivers/remoteproc/qcom_q6v5_pas.c | 106 ++++++++++++++++++++++++++++++++++--- > 1 file changed, 100 insertions(+), 6 deletions(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > index 6afd094..b02a1dc 100644 > --- a/drivers/remoteproc/qcom_q6v5_pas.c > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > @@ -21,6 +21,7 @@ > #include <linux/qcom_scm.h> > #include <linux/regulator/consumer.h> > #include <linux/remoteproc.h> > +#include <linux/interconnect.h> > #include <linux/soc/qcom/mdt_loader.h> > #include <linux/soc/qcom/smem.h> > #include <linux/soc/qcom/smem_state.h> > @@ -30,8 +31,14 @@ > #include "qcom_q6v5.h" > #include "remoteproc_internal.h" > > +#define PIL_TZ_AVG_BW UINT_MAX > +#define PIL_TZ_PEAK_BW UINT_MAX > #define ADSP_DECRYPT_SHUTDOWN_DELAY_MS 100 > > +static struct icc_path *scm_perf_client; > +static int scm_pas_bw_count; > +static DEFINE_MUTEX(scm_pas_bw_mutex); > + > struct adsp_data { > int crash_reason_smem; > const char *firmware_name; > @@ -67,6 +74,7 @@ struct qcom_adsp { > > int pas_id; > unsigned int minidump_id; > + struct icc_path *bus_client; > int crash_reason_smem; > bool has_aggre2_clk; > bool decrypt_shutdown; > @@ -136,6 +144,44 @@ static void adsp_pds_disable(struct qcom_adsp *adsp, struct device **pds, > } > } > > +static int scm_pas_enable_bw(void) > +{ > + int ret = 0; > + > + if (IS_ERR(scm_perf_client)) > + return -EINVAL; > + > + mutex_lock(&scm_pas_bw_mutex); > + if (!scm_pas_bw_count) { > + ret = icc_set_bw(scm_perf_client, PIL_TZ_AVG_BW, > + PIL_TZ_PEAK_BW); > + if (ret) > + goto err_bus; > + } > + > + scm_pas_bw_count++; > + mutex_unlock(&scm_pas_bw_mutex); > + return ret; > + > +err_bus: > + pr_err("scm-pas: Bandwidth request failed (%d)\n", ret); > + icc_set_bw(scm_perf_client, 0, 0); > + > + mutex_unlock(&scm_pas_bw_mutex); > + return ret; > +} > + > +static void scm_pas_disable_bw(void) > +{ > + if (IS_ERR(scm_perf_client)) > + return; > + > + mutex_lock(&scm_pas_bw_mutex); > + if (scm_pas_bw_count-- == 1) > + icc_set_bw(scm_perf_client, 0, 0); > + mutex_unlock(&scm_pas_bw_mutex); > +} > + > static int adsp_shutdown_poll_decrypt(struct qcom_adsp *adsp) > { > unsigned int retry_num = 50; > @@ -174,15 +220,35 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw) > if (ret) > return ret; > > + scm_pas_enable_bw(); > ret = qcom_mdt_load_no_init(adsp->dev, fw, rproc->firmware, adsp->pas_id, > adsp->mem_region, adsp->mem_phys, adsp->mem_size, > &adsp->mem_reloc); > if (ret) > - return ret; > + goto exit; > > qcom_pil_info_store(adsp->info_name, adsp->mem_phys, adsp->mem_size); > +exit: > + scm_pas_disable_bw(); > + return ret; > +} > > - return 0; > +static int do_bus_scaling(struct qcom_adsp *adsp, bool enable) > +{ > + int rc = 0; > + u32 avg_bw = enable ? PIL_TZ_AVG_BW : 0; > + u32 peak_bw = enable ? PIL_TZ_PEAK_BW : 0; > + > + if (IS_ERR(adsp->bus_client)) > + dev_err(adsp->dev, "Bus scaling not setup for %s\n", > + adsp->rproc->name); > + else > + rc = icc_set_bw(adsp->bus_client, avg_bw, peak_bw); > + > + if (rc) > + dev_err(adsp->dev, "bandwidth request failed(rc:%d)\n", rc); > + > + return rc; > } > > static int adsp_start(struct rproc *rproc) > @@ -194,10 +260,14 @@ static int adsp_start(struct rproc *rproc) > if (ret) > return ret; > > - ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); > + ret = do_bus_scaling(adsp, true); > if (ret < 0) > goto disable_irqs; > > + ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); > + if (ret < 0) > + goto unscale_bus; > + > ret = clk_prepare_enable(adsp->xo); > if (ret) > goto disable_proxy_pds; > @@ -218,6 +288,8 @@ static int adsp_start(struct rproc *rproc) > goto disable_cx_supply; > } > > + scm_pas_enable_bw(); > + > ret = qcom_scm_pas_auth_and_reset(adsp->pas_id); > if (ret) { > dev_err(adsp->dev, > @@ -234,9 +306,10 @@ static int adsp_start(struct rproc *rproc) > > qcom_scm_pas_metadata_release(&adsp->pas_metadata); > > - return 0; > - > disable_px_supply: > + scm_pas_disable_bw(); > + if(!ret) > + goto exit; > if (adsp->px_supply) > regulator_disable(adsp->px_supply); > disable_cx_supply: > @@ -248,9 +321,11 @@ static int adsp_start(struct rproc *rproc) > clk_disable_unprepare(adsp->xo); > disable_proxy_pds: > adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); > +unscale_bus: > + do_bus_scaling(adsp, false); > disable_irqs: > qcom_q6v5_unprepare(&adsp->q6v5); > - > +exit: > return ret; > } > > @@ -265,6 +340,7 @@ static void qcom_pas_handover(struct qcom_q6v5 *q6v5) > clk_disable_unprepare(adsp->aggre2_clk); > clk_disable_unprepare(adsp->xo); > adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); > + do_bus_scaling(adsp, false); > } > > static int adsp_stop(struct rproc *rproc) > @@ -383,6 +459,22 @@ static int adsp_init_regulator(struct qcom_adsp *adsp) > return 0; > } > > +static void adsp_init_bus_scaling(struct qcom_adsp *adsp) > +{ > + if (scm_perf_client) > + goto get_rproc_client; > + > + scm_perf_client = of_icc_get(adsp->dev, "crypto_ddr"); > + if (IS_ERR(scm_perf_client)) > + dev_warn(adsp->dev, "Crypto scaling not setup\n"); > + > +get_rproc_client: > + adsp->bus_client = of_icc_get(adsp->dev, "rproc_ddr"); > + if (IS_ERR(adsp->bus_client)) > + dev_warn(adsp->dev, "%s: No bus client\n", __func__); > + > +} > + > static int adsp_pds_attach(struct device *dev, struct device **devs, > char **pd_names) > { > @@ -525,6 +617,8 @@ static int adsp_probe(struct platform_device *pdev) > if (ret) > goto free_rproc; > > + adsp_init_bus_scaling(adsp); > + > ret = adsp_pds_attach(&pdev->dev, adsp->proxy_pds, > desc->proxy_pd_names); > if (ret < 0)
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c index 6afd094..b02a1dc 100644 --- a/drivers/remoteproc/qcom_q6v5_pas.c +++ b/drivers/remoteproc/qcom_q6v5_pas.c @@ -21,6 +21,7 @@ #include <linux/qcom_scm.h> #include <linux/regulator/consumer.h> #include <linux/remoteproc.h> +#include <linux/interconnect.h> #include <linux/soc/qcom/mdt_loader.h> #include <linux/soc/qcom/smem.h> #include <linux/soc/qcom/smem_state.h> @@ -30,8 +31,14 @@ #include "qcom_q6v5.h" #include "remoteproc_internal.h" +#define PIL_TZ_AVG_BW UINT_MAX +#define PIL_TZ_PEAK_BW UINT_MAX #define ADSP_DECRYPT_SHUTDOWN_DELAY_MS 100 +static struct icc_path *scm_perf_client; +static int scm_pas_bw_count; +static DEFINE_MUTEX(scm_pas_bw_mutex); + struct adsp_data { int crash_reason_smem; const char *firmware_name; @@ -67,6 +74,7 @@ struct qcom_adsp { int pas_id; unsigned int minidump_id; + struct icc_path *bus_client; int crash_reason_smem; bool has_aggre2_clk; bool decrypt_shutdown; @@ -136,6 +144,44 @@ static void adsp_pds_disable(struct qcom_adsp *adsp, struct device **pds, } } +static int scm_pas_enable_bw(void) +{ + int ret = 0; + + if (IS_ERR(scm_perf_client)) + return -EINVAL; + + mutex_lock(&scm_pas_bw_mutex); + if (!scm_pas_bw_count) { + ret = icc_set_bw(scm_perf_client, PIL_TZ_AVG_BW, + PIL_TZ_PEAK_BW); + if (ret) + goto err_bus; + } + + scm_pas_bw_count++; + mutex_unlock(&scm_pas_bw_mutex); + return ret; + +err_bus: + pr_err("scm-pas: Bandwidth request failed (%d)\n", ret); + icc_set_bw(scm_perf_client, 0, 0); + + mutex_unlock(&scm_pas_bw_mutex); + return ret; +} + +static void scm_pas_disable_bw(void) +{ + if (IS_ERR(scm_perf_client)) + return; + + mutex_lock(&scm_pas_bw_mutex); + if (scm_pas_bw_count-- == 1) + icc_set_bw(scm_perf_client, 0, 0); + mutex_unlock(&scm_pas_bw_mutex); +} + static int adsp_shutdown_poll_decrypt(struct qcom_adsp *adsp) { unsigned int retry_num = 50; @@ -174,15 +220,35 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw) if (ret) return ret; + scm_pas_enable_bw(); ret = qcom_mdt_load_no_init(adsp->dev, fw, rproc->firmware, adsp->pas_id, adsp->mem_region, adsp->mem_phys, adsp->mem_size, &adsp->mem_reloc); if (ret) - return ret; + goto exit; qcom_pil_info_store(adsp->info_name, adsp->mem_phys, adsp->mem_size); +exit: + scm_pas_disable_bw(); + return ret; +} - return 0; +static int do_bus_scaling(struct qcom_adsp *adsp, bool enable) +{ + int rc = 0; + u32 avg_bw = enable ? PIL_TZ_AVG_BW : 0; + u32 peak_bw = enable ? PIL_TZ_PEAK_BW : 0; + + if (IS_ERR(adsp->bus_client)) + dev_err(adsp->dev, "Bus scaling not setup for %s\n", + adsp->rproc->name); + else + rc = icc_set_bw(adsp->bus_client, avg_bw, peak_bw); + + if (rc) + dev_err(adsp->dev, "bandwidth request failed(rc:%d)\n", rc); + + return rc; } static int adsp_start(struct rproc *rproc) @@ -194,10 +260,14 @@ static int adsp_start(struct rproc *rproc) if (ret) return ret; - ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); + ret = do_bus_scaling(adsp, true); if (ret < 0) goto disable_irqs; + ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); + if (ret < 0) + goto unscale_bus; + ret = clk_prepare_enable(adsp->xo); if (ret) goto disable_proxy_pds; @@ -218,6 +288,8 @@ static int adsp_start(struct rproc *rproc) goto disable_cx_supply; } + scm_pas_enable_bw(); + ret = qcom_scm_pas_auth_and_reset(adsp->pas_id); if (ret) { dev_err(adsp->dev, @@ -234,9 +306,10 @@ static int adsp_start(struct rproc *rproc) qcom_scm_pas_metadata_release(&adsp->pas_metadata); - return 0; - disable_px_supply: + scm_pas_disable_bw(); + if(!ret) + goto exit; if (adsp->px_supply) regulator_disable(adsp->px_supply); disable_cx_supply: @@ -248,9 +321,11 @@ static int adsp_start(struct rproc *rproc) clk_disable_unprepare(adsp->xo); disable_proxy_pds: adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); +unscale_bus: + do_bus_scaling(adsp, false); disable_irqs: qcom_q6v5_unprepare(&adsp->q6v5); - +exit: return ret; } @@ -265,6 +340,7 @@ static void qcom_pas_handover(struct qcom_q6v5 *q6v5) clk_disable_unprepare(adsp->aggre2_clk); clk_disable_unprepare(adsp->xo); adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); + do_bus_scaling(adsp, false); } static int adsp_stop(struct rproc *rproc) @@ -383,6 +459,22 @@ static int adsp_init_regulator(struct qcom_adsp *adsp) return 0; } +static void adsp_init_bus_scaling(struct qcom_adsp *adsp) +{ + if (scm_perf_client) + goto get_rproc_client; + + scm_perf_client = of_icc_get(adsp->dev, "crypto_ddr"); + if (IS_ERR(scm_perf_client)) + dev_warn(adsp->dev, "Crypto scaling not setup\n"); + +get_rproc_client: + adsp->bus_client = of_icc_get(adsp->dev, "rproc_ddr"); + if (IS_ERR(adsp->bus_client)) + dev_warn(adsp->dev, "%s: No bus client\n", __func__); + +} + static int adsp_pds_attach(struct device *dev, struct device **devs, char **pd_names) { @@ -525,6 +617,8 @@ static int adsp_probe(struct platform_device *pdev) if (ret) goto free_rproc; + adsp_init_bus_scaling(adsp); + ret = adsp_pds_attach(&pdev->dev, adsp->proxy_pds, desc->proxy_pd_names); if (ret < 0)
During bootup since remote processors cannot request for additional bus bandwidth from the interconect framework, platform driver should provide the proxy resources. Make a proxy vote for maximizing the bus bandwidth during bootup for a remote processor and remove it once processor is up. Change-Id: I798b3b000aef4761a7ff8cb6390b7ecd98f925b7 Signed-off-by: Gokul krishna Krishnakumar <quic_gokukris@quicinc.com> --- drivers/remoteproc/qcom_q6v5_pas.c | 106 ++++++++++++++++++++++++++++++++++--- 1 file changed, 100 insertions(+), 6 deletions(-)