Messages in this thread | | | Subject | Re: [PATCH v6 5/6] media: venus: core: Add support for opp tables/perf voting | From | Rajendra Nayak <> | Date | Mon, 29 Jun 2020 16:19:08 +0530 |
| |
On 6/18/2020 8:24 PM, Stanimir Varbanov wrote: > Hi Rajendra, > > On 6/15/20 3:02 PM, Rajendra Nayak wrote: >> Add support to add OPP tables and perf voting on the OPP powerdomain. >> This is needed so venus votes on the corresponding performance state >> for the OPP powerdomain along with setting the core clock rate. >> >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> >> Reviewed-by: Matthias Kaehlcke <mka@chromium.org> >> Cc: Stanimir Varbanov <stanimir.varbanov@linaro.org> >> Cc: linux-media@vger.kernel.org >> --- >> No functional change in v6, rebased over 5.8-rc1 >> Bindings update to add optional PD https://lore.kernel.org/patchwork/patch/1241077/ >> >> drivers/media/platform/qcom/venus/core.c | 43 +++++++++++++++++--- >> drivers/media/platform/qcom/venus/core.h | 5 +++ >> drivers/media/platform/qcom/venus/pm_helpers.c | 54 ++++++++++++++++++++++++-- >> 3 files changed, 92 insertions(+), 10 deletions(-) >> > > <cut> > >> >> @@ -740,13 +740,16 @@ static int venc_power_v4(struct device *dev, int on) >> >> static int vcodec_domains_get(struct device *dev) >> { >> + int ret; >> + struct opp_table *opp_table; >> + struct device **opp_virt_dev; >> struct venus_core *core = dev_get_drvdata(dev); >> const struct venus_resources *res = core->res; >> struct device *pd; >> unsigned int i; >> >> if (!res->vcodec_pmdomains_num) >> - return -ENODEV; >> + goto skip_pmdomains; >> >> for (i = 0; i < res->vcodec_pmdomains_num; i++) { >> pd = dev_pm_domain_attach_by_name(dev, >> @@ -763,7 +766,41 @@ static int vcodec_domains_get(struct device *dev) >> if (!core->pd_dl_venus) >> return -ENODEV; >> >> +skip_pmdomains: >> + if (!core->has_opp_table) >> + return 0; >> + >> + /* Attach the power domain for setting performance state */ >> + opp_table = dev_pm_opp_attach_genpd(dev, res->opp_pmdomain, &opp_virt_dev); >> + if (IS_ERR(opp_table)) { >> + ret = PTR_ERR(opp_table); >> + goto opp_attach_err; >> + } >> + >> + core->opp_pmdomain = *opp_virt_dev; >> + core->opp_dl_venus = device_link_add(dev, core->opp_pmdomain, >> + DL_FLAG_RPM_ACTIVE | >> + DL_FLAG_PM_RUNTIME | >> + DL_FLAG_STATELESS); >> + if (!core->opp_dl_venus) { >> + ret = -ENODEV; >> + goto opp_dl_add_err; >> + } >> + >> return 0; >> + >> +opp_dl_add_err: >> + dev_pm_domain_detach(core->opp_pmdomain, true); >> +opp_attach_err: >> + if (core->pd_dl_venus) { >> + device_link_del(core->pd_dl_venus); >> + for (i = 0; i < res->vcodec_pmdomains_num; i++) { >> + if (IS_ERR_OR_NULL(core->pmdomains[i])) >> + continue; >> + dev_pm_domain_detach(core->pmdomains[i], true); >> + } >> + } >> + return ret; >> } >> >> static void vcodec_domains_put(struct device *dev) >> @@ -773,7 +810,7 @@ static void vcodec_domains_put(struct device *dev) >> unsigned int i; >> >> if (!res->vcodec_pmdomains_num) >> - return; >> + goto skip_pmdomains; >> >> if (core->pd_dl_venus) >> device_link_del(core->pd_dl_venus); >> @@ -783,6 +820,15 @@ static void vcodec_domains_put(struct device *dev) >> continue; >> dev_pm_domain_detach(core->pmdomains[i], true); >> } >> + >> +skip_pmdomains: >> + if (!res->opp_pmdomain) >> + return; >> + >> + if (core->opp_dl_venus) >> + device_link_del(core->opp_dl_venus); >> + >> + dev_pm_domain_detach(core->opp_pmdomain, true); > > Without corresponding changes in venus DT node we call pm_domain_detach > with core->opp_pmdomain = NULL which triggers NULL pointer dereference. > > I guess you should check: > > if (core->has_opp_table) > dev_pm_domain_detach(core->opp_pmdomain, true); > > or > > if (core->opp_pmdomain) > dev_pm_domain_detach(core->opp_pmdomain, true); > > > ... not sure which one is more appropriate or both are.
Thanks, I'll fix that up when I repost. Sorry I was out for a while so haven't been able to reproduce the rpmh timeout issue you reported. Will spend some time on it this week and see if I can reproduce it.
-- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
| |