| Message ID | 20231101-gdsc-hwctrl-v3-5-0740ae6b2b04@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | PM: domains: Add control for switching back and forth to HW control | expand |
On 01/11/2023 09:04, Abel Vesa wrote: > From: Jagadeesh Kona <quic_jkona@quicinc.com> > > This change demonstrates the use of dev_pm_genpd_set_hwmode API from > video driver to switch the video mvs0 gdsc to SW/HW modes at runtime > based on requirement. > > This change adds a new boolean array member vcodec_pmdomains_hwctrl in > venus_resources structure to indicate if GDSC's have HW control support > or not. This data is used in vcodec_control_v4() to check if GDSC has > support to switch to HW control mode and then call dev_pm_genpd_set_hwmode > to switch the GDSC mode. > > Before the GDSC HWCTL was available to the consumer, the venus driver > needed to somehow keep the power from collapsing while under the driver > control. The only way to do that was to clear the CORE_PWR_DISABLE bit > (in wrapper POWER_CONTROL register) and, respectively, set it back after > the driver control was completed. > > Now, that there is a way to switch the GDSC HW/SW control back and > forth, the CORE_PWR_DISABLE toggling can be dropped. > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> So I plan to give this a test on rb5 and db410c My q here though is - has anybody on the submission list tested this through suspend/resume and vdd min ? --- bod
On 11/3/23 11:44, Bryan O'Donoghue wrote: > On 01/11/2023 09:04, Abel Vesa wrote: >> From: Jagadeesh Kona <quic_jkona@quicinc.com> >> >> This change demonstrates the use of dev_pm_genpd_set_hwmode API from >> video driver to switch the video mvs0 gdsc to SW/HW modes at runtime >> based on requirement. >> >> This change adds a new boolean array member vcodec_pmdomains_hwctrl in >> venus_resources structure to indicate if GDSC's have HW control support >> or not. This data is used in vcodec_control_v4() to check if GDSC has >> support to switch to HW control mode and then call dev_pm_genpd_set_hwmode >> to switch the GDSC mode. >> >> Before the GDSC HWCTL was available to the consumer, the venus driver >> needed to somehow keep the power from collapsing while under the driver >> control. The only way to do that was to clear the CORE_PWR_DISABLE bit >> (in wrapper POWER_CONTROL register) and, respectively, set it back after >> the driver control was completed. >> >> Now, that there is a way to switch the GDSC HW/SW control back and >> forth, the CORE_PWR_DISABLE toggling can be dropped. >> >> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> >> Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > > So I plan to give this a test on rb5 and db410c > > My q here though is - has anybody on the submission list tested this through suspend/resume and vdd min ? I think that only chromebooks (sc7[12]80) and sc7180-acer-aspire1.dts are able to hit vddmin upstream as of today Konrad
diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index 054b8e74ba4f..8145062ab6f7 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -706,6 +706,7 @@ static const struct venus_resources sdm845_res_v2 = { .vcodec1_clks = { "vcodec1_core", "vcodec1_bus" }, .vcodec_clks_num = 2, .vcodec_pmdomains = { "venus", "vcodec0", "vcodec1" }, + .vcodec_pmdomains_hwctrl = { false, true, true }, .vcodec_pmdomains_num = 3, .opp_pmdomain = (const char *[]) { "cx", NULL }, .vcodec_num = 2, @@ -755,6 +756,7 @@ static const struct venus_resources sc7180_res = { .vcodec0_clks = { "vcodec0_core", "vcodec0_bus" }, .vcodec_clks_num = 2, .vcodec_pmdomains = { "venus", "vcodec0" }, + .vcodec_pmdomains_hwctrl = { false, true }, .vcodec_pmdomains_num = 2, .opp_pmdomain = (const char *[]) { "cx", NULL }, .vcodec_num = 1, @@ -812,6 +814,7 @@ static const struct venus_resources sm8250_res = { .vcodec0_clks = { "vcodec0_core" }, .vcodec_clks_num = 1, .vcodec_pmdomains = { "venus", "vcodec0" }, + .vcodec_pmdomains_hwctrl = { false, true }, .vcodec_pmdomains_num = 2, .opp_pmdomain = (const char *[]) { "mx", NULL }, .vcodec_num = 1, @@ -871,6 +874,7 @@ static const struct venus_resources sc7280_res = { .vcodec0_clks = {"vcodec_core", "vcodec_bus"}, .vcodec_clks_num = 2, .vcodec_pmdomains = { "venus", "vcodec0" }, + .vcodec_pmdomains_hwctrl = { false, true }, .vcodec_pmdomains_num = 2, .opp_pmdomain = (const char *[]) { "cx", NULL }, .vcodec_num = 1, diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h index 4a633261ece4..6d591ecad482 100644 --- a/drivers/media/platform/qcom/venus/core.h +++ b/drivers/media/platform/qcom/venus/core.h @@ -73,6 +73,7 @@ struct venus_resources { const char * const vcodec1_clks[VIDC_VCODEC_CLKS_NUM_MAX]; unsigned int vcodec_clks_num; const char * const vcodec_pmdomains[VIDC_PMDOMAINS_NUM_MAX]; + bool vcodec_pmdomains_hwctrl[VIDC_PMDOMAINS_NUM_MAX]; unsigned int vcodec_pmdomains_num; const char **opp_pmdomain; unsigned int vcodec_num; diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c index a1b127caa90a..9d1dc8366ab0 100644 --- a/drivers/media/platform/qcom/venus/pm_helpers.c +++ b/drivers/media/platform/qcom/venus/pm_helpers.c @@ -408,35 +408,28 @@ static const struct venus_pm_ops pm_ops_v3 = { static int vcodec_control_v4(struct venus_core *core, u32 coreid, bool enable) { - void __iomem *ctrl, *stat; - u32 val; - int ret; - - if (IS_V6(core)) { - ctrl = core->wrapper_base + WRAPPER_CORE_POWER_CONTROL_V6; - stat = core->wrapper_base + WRAPPER_CORE_POWER_STATUS_V6; - } else if (coreid == VIDC_CORE_ID_1) { - ctrl = core->wrapper_base + WRAPPER_VCODEC0_MMCC_POWER_CONTROL; - stat = core->wrapper_base + WRAPPER_VCODEC0_MMCC_POWER_STATUS; - } else { - ctrl = core->wrapper_base + WRAPPER_VCODEC1_MMCC_POWER_CONTROL; - stat = core->wrapper_base + WRAPPER_VCODEC1_MMCC_POWER_STATUS; - } - - if (enable) { - writel(0, ctrl); - - ret = readl_poll_timeout(stat, val, val & BIT(1), 1, 100); - if (ret) - return ret; - } else { - writel(1, ctrl); + int i, ret = 0; + struct device *dev = core->dev; + const struct venus_resources *res = core->res; - ret = readl_poll_timeout(stat, val, !(val & BIT(1)), 1, 100); - if (ret) - return ret; + for (i = 0; i < res->vcodec_pmdomains_num; i++) { + if (res->vcodec_pmdomains_hwctrl[i]) { + + if (!core->pmdomains[i]) + return -ENODEV; + + /* + * enable( true ), switch the gdsc to SW mode + * enable( false), switch the gdsc to HW mode + */ + ret = dev_pm_genpd_set_hwmode(core->pmdomains[i], !enable); + if (ret) { + dev_err(dev, "Failed to switch power-domain:%d to %s mode\n", + i, enable ? "SW" : "HW"); + return ret; + } + } } - return 0; }