| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <76e40130-897e-4547-8d5a-a0054f123fbe@linaro.org>
Date: Tue, 10 Jun 2025 20:01:27 +0100
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Renjiang Han <quic_renjiang@...cinc.com>,
Vikash Garodia <quic_vgarodia@...cinc.com>,
Dikshita Agarwal <quic_dikshita@...cinc.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: venus: pm_helpers: use opp-table for the frequency
On 10/06/2025 10:13, Renjiang Han wrote:
> The frequency values in the opp-table of the device tree and the freq_tbl
> in the driver are the same. So the driver can choose either table for freq
> values.
I'd completely drop this sentence as you basically contradict it in the
next line.
> However,
Drop
some platforms (such as qcs615 and sc7180) use the same core but
> have different frequency tables. Using the opp-table allows us to separate
> the core description from the frequency data and supports the use of
> fallback compatibles.
This is a bit better.
Basically you can have identical bindings, even identical compats but
for reasons such as binning of silicon parts you might run at higher or
lower frequencies so static tables in the driver are not appropriate and
are better represented in the DT.
> Reviewed-by: Vikash Garodia <quic_vgarodia@...cinc.com>
> Signed-off-by: Renjiang Han <quic_renjiang@...cinc.com>
> ---
> The frequency values in the opp-table of the device tree and the freq_tbl
> in the driver are the same. So the driver can choose either table for freq
> values.
>
> However, some platforms (such as qcs615 and sc7180) use the same core but
> have different frequency tables. Using the opp-table allows us to separate
> the core description from the frequency data and supports the use of
> fallback compatibles.
>
> Therefore, it is necessary to update pm_helpers.c to use the frequency
> values from the opp-table for the v4 core.
>
> Note:
> Earlier discussion was concluded in [1] where it was agreed to rely on
> opp-table for QCS615.
>
> [1] https://lore.kernel.org/linux-arm-msm/c9b83c8b-68d1-43bc-99d6-d2d2b9e445f4@oss.qualcomm.com/
> ---
> drivers/media/platform/qcom/venus/pm_helpers.c | 58 +++++++++++++++-----------
> 1 file changed, 34 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
> index 409aa9bd0b5d099c993eedb03177ec5ed918b4a0..8dd5a9b0d060cddfeafd4da477ade0c7aeb6c390 100644
> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
> @@ -41,16 +41,14 @@ static int core_clks_get(struct venus_core *core)
> static int core_clks_enable(struct venus_core *core)
> {
> const struct venus_resources *res = core->res;
> - const struct freq_tbl *freq_tbl = core->res->freq_tbl;
> - unsigned int freq_tbl_size = core->res->freq_tbl_size;
> - unsigned long freq;
> + struct device *dev = core->dev;
> + unsigned long freq = 0;
> + struct dev_pm_opp *opp;
> unsigned int i;
> int ret;
>
> - if (!freq_tbl)
> - return -EINVAL;
> -
> - freq = freq_tbl[freq_tbl_size - 1].freq;
> + opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> + dev_pm_opp_put(opp);
>
> for (i = 0; i < res->clks_num; i++) {
> if (IS_V6(core)) {
> @@ -636,7 +634,9 @@ static int decide_core(struct venus_inst *inst)
> u32 min_coreid, min_load, cur_inst_load;
> u32 min_lp_coreid, min_lp_load, cur_inst_lp_load;
> struct hfi_videocores_usage_type cu;
> - unsigned long max_freq;
> + unsigned long max_freq = ULONG_MAX;
> + struct device *dev = core->dev;
> + struct dev_pm_opp *opp;
> int ret = 0;
>
> if (legacy_binding) {
> @@ -659,7 +659,8 @@ static int decide_core(struct venus_inst *inst)
> cur_inst_lp_load *= inst->clk_data.low_power_freq;
> /*TODO : divide this inst->load by work_route */
>
> - max_freq = core->res->freq_tbl[0].freq;
> + opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
> + dev_pm_opp_put(opp);
>
> min_loaded_core(inst, &min_coreid, &min_load, false);
> min_loaded_core(inst, &min_lp_coreid, &min_lp_load, true);
> @@ -949,7 +950,10 @@ static int core_resets_get(struct venus_core *core)
> static int core_get_v4(struct venus_core *core)
> {
> struct device *dev = core->dev;
> + const struct freq_tbl *freq_tbl = core->res->freq_tbl;
> + unsigned int num_rows = core->res->freq_tbl_size;
> const struct venus_resources *res = core->res;
> + unsigned int i;
> int ret;
>
> ret = core_clks_get(core);
> @@ -986,9 +990,17 @@ static int core_get_v4(struct venus_core *core)
>
> if (core->res->opp_pmdomain) {
> ret = devm_pm_opp_of_add_table(dev);
> - if (ret && ret != -ENODEV) {
> - dev_err(dev, "invalid OPP table in device tree\n");
> - return ret;
> + if (ret) {
> + if (ret == -ENODEV) {
> + for (i = 0; i < num_rows; i++) {
> + ret = dev_pm_opp_add(dev, freq_tbl[i].freq, 0);
> + if (ret)
> + return ret;
> + }
> + } else {
> + dev_err(dev, "invalid OPP table in device tree\n");
> + return ret;
> + }
> }
> }
>
> @@ -1078,11 +1090,11 @@ static unsigned long calculate_inst_freq(struct venus_inst *inst,
> static int load_scale_v4(struct venus_inst *inst)
> {
> struct venus_core *core = inst->core;
> - const struct freq_tbl *table = core->res->freq_tbl;
> - unsigned int num_rows = core->res->freq_tbl_size;
> struct device *dev = core->dev;
> unsigned long freq = 0, freq_core1 = 0, freq_core2 = 0;
> + unsigned long max_freq = ULONG_MAX;
> unsigned long filled_len = 0;
> + struct dev_pm_opp *opp;
> int i, ret = 0;
>
> for (i = 0; i < inst->num_input_bufs; i++)
> @@ -1108,20 +1120,18 @@ static int load_scale_v4(struct venus_inst *inst)
>
> freq = max(freq_core1, freq_core2);
>
> - if (freq > table[0].freq) {
> - dev_dbg(dev, VDBGL "requested clock rate: %lu scaling clock rate : %lu\n",
> - freq, table[0].freq);
> + opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
> + dev_pm_opp_put(opp);
>
> - freq = table[0].freq;
> + if (freq > max_freq) {
> + dev_dbg(dev, VDBGL "requested clock rate: %lu scaling clock rate : %lu\n",
> + freq, max_freq);
> + freq = max_freq;
> goto set_freq;
> }
>
> - for (i = num_rows - 1 ; i >= 0; i--) {
> - if (freq <= table[i].freq) {
> - freq = table[i].freq;
> - break;
> - }
> - }
> + opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> + dev_pm_opp_put(opp);
>
> set_freq:
>
>
> ---
> base-commit: b27cc623e01be9de1580eaa913508b237a7a9673
> change-id: 20250610-use_freq_with_opp_table-b81162cfecba
>
> Best regards,
Please make your commit log more succinct and precise then add.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
---
bod
Powered by blists - more mailing lists