| Message ID | 20230731-topic-8280_venus-v1-4-8c8bbe1983a5@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | SM8350 and SC8280XP venus support | expand |
On Fri, Aug 04, 2023 at 10:09:11PM +0200, Konrad Dybcio wrote: > @@ -479,12 +488,18 @@ static __maybe_unused int venus_runtime_suspend(struct device *dev) > if (ret) > goto err_cpucfg_path; > > + ret = icc_set_bw(core->llcc_path, 0, 0); > + if (ret) > + goto err_llcc_path; > + > ret = icc_set_bw(core->video_path, 0, 0); > if (ret) > goto err_video_path; > > return ret; > > +err_llcc_path: > + icc_set_bw(core->video_path, kbps_to_icc(20000), 0); This looks wrong; you should not try to restore the video path bw which you have not yet updated here. Also error labels should be named after what they do, not after where you jump from (e.g. to avoid mistakes like the above). Perhaps you can clean up the existing labels before adding the new one. > err_video_path: > icc_set_bw(core->cpucfg_path, kbps_to_icc(1000), 0); > err_cpucfg_path: Johan
On 4.08.2023 23:04, Bryan O'Donoghue wrote: > On 04/08/2023 21:09, Konrad Dybcio wrote: >> Some newer SoCs (such as SM8350) have a third interconnect path. Add >> it and make it optional. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> --- [...] > I would scream if someone left me this comment but... > > In probe we have > > video_path = > cpu_cfgpath = > > llc_path = > > suspend > > icc_set_bw(cpu_cfgpath,); > icc_set_bw(llc_path,); > icc_set_bw(video_path,); > > resume > icc_set_bw(video_path,); > icc_set_bw(llc_path,); > icc_set_bw(cpu_cfgpath,); suspend == resume[::-1] is totally the right thing, but I'll reorder things in probe for your viewing pleasure Konrad
On 4.08.2023 23:06, Bryan O'Donoghue wrote: > On 04/08/2023 22:04, Bryan O'Donoghue wrote: >> you can get for llc_path == NULL > > [sic] You can test. Even better, I can just throw it into icc APIs as-is, as they nullcheck internally Konrad
diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index 0af45faec247..db363061748f 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -302,6 +302,15 @@ static int venus_probe(struct platform_device *pdev) if (IS_ERR(core->cpucfg_path)) return PTR_ERR(core->cpucfg_path); + core->llcc_path = devm_of_icc_get(dev, "video-llcc"); + if (IS_ERR(core->llcc_path)) { + /* LLCC path is optional */ + if (PTR_ERR(core->llcc_path) == -ENODATA) + core->llcc_path = NULL; + else + return PTR_ERR(core->llcc_path); + } + core->irq = platform_get_irq(pdev, 0); if (core->irq < 0) return core->irq; @@ -479,12 +488,18 @@ static __maybe_unused int venus_runtime_suspend(struct device *dev) if (ret) goto err_cpucfg_path; + ret = icc_set_bw(core->llcc_path, 0, 0); + if (ret) + goto err_llcc_path; + ret = icc_set_bw(core->video_path, 0, 0); if (ret) goto err_video_path; return ret; +err_llcc_path: + icc_set_bw(core->video_path, kbps_to_icc(20000), 0); err_video_path: icc_set_bw(core->cpucfg_path, kbps_to_icc(1000), 0); err_cpucfg_path: @@ -504,6 +519,10 @@ static __maybe_unused int venus_runtime_resume(struct device *dev) if (ret) return ret; + ret = icc_set_bw(core->llcc_path, kbps_to_icc(20000), 0); + if (ret) + return ret; + ret = icc_set_bw(core->cpucfg_path, kbps_to_icc(1000), 0); if (ret) return ret; diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h index 2999c24392f5..45ed1551c2db 100644 --- a/drivers/media/platform/qcom/venus/core.h +++ b/drivers/media/platform/qcom/venus/core.h @@ -65,6 +65,7 @@ struct venus_resources { unsigned int bw_tbl_enc_size; const struct bw_tbl *bw_tbl_dec; unsigned int bw_tbl_dec_size; + bool has_llcc_path; const struct reg_val *reg_tbl; unsigned int reg_tbl_size; const struct hfi_ubwc_config *ubwc_conf; @@ -134,6 +135,7 @@ struct venus_format { * @vcodec1_clks: an array of vcodec1 struct clk pointers * @video_path: an interconnect handle to video to/from memory path * @cpucfg_path: an interconnect handle to cpu configuration path + * @llcc_path: an interconnect handle to video to/from llcc path * @has_opp_table: does OPP table exist * @pmdomains: an array of pmdomains struct device pointers * @opp_dl_venus: an device-link for device OPP @@ -186,6 +188,7 @@ struct venus_core { struct clk *vcodec1_clks[VIDC_VCODEC_CLKS_NUM_MAX]; struct icc_path *video_path; struct icc_path *cpucfg_path; + struct icc_path *llcc_path; bool has_opp_table; struct device *pmdomains[VIDC_PMDOMAINS_NUM_MAX]; struct device_link *opp_dl_venus; diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c index 48c9084bb4db..79392ff8fa06 100644 --- a/drivers/media/platform/qcom/venus/pm_helpers.c +++ b/drivers/media/platform/qcom/venus/pm_helpers.c @@ -237,6 +237,9 @@ static int load_scale_bw(struct venus_core *core) dev_dbg(core->dev, VDBGL "total: avg_bw: %u, peak_bw: %u\n", total_avg, total_peak); + if (core->res->has_llcc_path) + icc_set_bw(core->llcc_path, total_avg, total_peak); + return icc_set_bw(core->video_path, total_avg, total_peak); }
Some newer SoCs (such as SM8350) have a third interconnect path. Add it and make it optional. Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- drivers/media/platform/qcom/venus/core.c | 19 +++++++++++++++++++ drivers/media/platform/qcom/venus/core.h | 3 +++ drivers/media/platform/qcom/venus/pm_helpers.c | 3 +++ 3 files changed, 25 insertions(+)