Messages in this thread | | | Date | Fri, 4 Aug 2023 22:04:15 +0100 | Subject | Re: [PATCH 4/6] media: platform: venus: Add optional LLCC path | From | Bryan O'Donoghue <> |
| |
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> > --- > 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(+) > > 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; > +
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,);
it would be nice to have probe match the ordering ...
> 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;
Why do you need this bool, you can get for llc_path == NULL
--- bod
| |