[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b1293a1a-e3bf-4e05-835d-0e1908417e88@quicinc.com>
Date: Tue, 3 Jun 2025 10:31:13 +0800
From: Renjiang Han <quic_renjiang@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
CC: Vikash Garodia <quic_vgarodia@...cinc.com>,
Dikshita Agarwal
<quic_dikshita@...cinc.com>,
Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Bjorn Andersson
<andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Rob Herring
<robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>, <linux-media@...r.kernel.org>,
<linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<devicetree@...r.kernel.org>
Subject: Re: [PATCH v8 1/3] media: venus: pm_helpers: use opp-table for the
frequency
On 5/31/2025 4:26 AM, Dmitry Baryshkov wrote:
> On Fri, May 30, 2025 at 09:32:13AM +0530, Renjiang Han wrote:
>> The frequency value in the opp-table in the device tree and the freq_tbl
>> in the driver are the same.
>>
>> Therefore, update pm_helpers.c to use the opp-table for frequency values
>> for the v4 core.
> You are kind of missing the linking between the first two sentences. "The
> tables are the same, so use the second one." You need to explain that
> some of the platforms (provide examples) use the same core, but
> different frequency tables. Using OPP tables allows us to abstract core
> description from the frequency data and use fallback compatibles.
OK. Thanks for your comment. I'll update this message in next version.
>
>> If getting data from the opp table fails, fall back to using the frequency
>> table.
>>
>> Reviewed-by: Vikash Garodia <quic_vgarodia@...cinc.com>
>> Signed-off-by: Renjiang Han <quic_renjiang@...cinc.com>
>> ---
>> drivers/media/platform/qcom/venus/pm_helpers.c | 53 +++++++++++++++++++-------
>> 1 file changed, 39 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
>> index 409aa9bd0b5d099c993eedb03177ec5ed918b4a0..434dd66076e8faf7f3feac6c29152789f8d2f81b 100644
>> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
>> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
>> @@ -43,14 +43,20 @@ 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;
>> + struct device *dev = core->dev;
>> + struct dev_pm_opp *opp;
>> unsigned long freq;
>> 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);
>> + if (IS_ERR(opp)) {
>> + if (!freq_tbl)
>> + return -EINVAL;
>> + freq = freq_tbl[freq_tbl_size - 1].freq;
>> + } else {
>> + dev_pm_opp_put(opp);
>> + }
>>
>> for (i = 0; i < res->clks_num; i++) {
>> if (IS_V6(core)) {
>> @@ -631,12 +637,15 @@ min_loaded_core(struct venus_inst *inst, u32 *min_coreid, u32 *min_load, bool lo
>>
>> static int decide_core(struct venus_inst *inst)
>> {
>> + const struct freq_tbl *freq_tbl = inst->core->res->freq_tbl;
>> const u32 ptype = HFI_PROPERTY_CONFIG_VIDEOCORES_USAGE;
>> struct venus_core *core = inst->core;
>> 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 +668,11 @@ 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);
>> + if (IS_ERR(opp))
>> + max_freq = freq_tbl[0].freq;
>> + else
>> + 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);
>> @@ -1082,7 +1095,9 @@ static int load_scale_v4(struct venus_inst *inst)
>> 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,19 +1123,29 @@ 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);
>> + if (IS_ERR(opp))
>> + max_freq = table[0].freq;
>> + else
>> + 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);
>> + if (IS_ERR(opp)) {
>> + for (i = num_rows - 1 ; i >= 0; i--) {
>> + if (freq <= table[i].freq) {
>> + freq = table[i].freq;
>> + break;
>> + }
>> }
>> + } else {
>> + dev_pm_opp_put(opp);
>> }
>>
>> set_freq:
>>
>> --
>> 2.34.1
>>
--
Best Regards,
Renjiang
Powered by blists - more mailing lists