| Message ID | 20230731-topic-8280_venus-v1-4-8c8bbe1983a5@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | SM8350 and SC8280XP venus support | expand |
On 7.08.2023 12:43, Johan Hovold wrote: > 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. Oh whoops :D > > 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. Ack, I wouldn't mind giving this some cleanup. Konrad
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(+)