| Message ID | 20241219-add-venus-for-qcs615-v6-2-e9a74d3b003d@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | media: venus: enable venus on qcs615 | expand |
On 19.12.2024 6:41 AM, 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. > If getting data from the opp table fails, fall back to using the frequency > table. > > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.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 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..b61c0ad152878870b7223efa274e137d3636433b 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); > + } I'm not super convinced how this could have ever worked without scaling voltage levels, by the way. Perhaps this will squash some random bugs :| Konrad
On 12/23/2024 10:17 PM, Konrad Dybcio wrote: > On 19.12.2024 6:41 AM, 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. >> If getting data from the opp table fails, fall back to using the frequency >> table. >> >> Signed-off-by: Renjiang Han<quic_renjiang@quicinc.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 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..b61c0ad152878870b7223efa274e137d3636433b 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); >> + } > I'm not super convinced how this could have ever worked without > scaling voltage levels, by the way. Perhaps this will squash some > random bugs :| > > Konrad Thanks for your comment. The default value of freq is 0, and then dev_pm_opp_find_freq_ceil is used to match freq to the maximum value in opp-table that is close to 0. The frequency values in opp-table and freq_tbl are the same, and dev_pm_opp_find_freq_ceil is used to assign the minimum value in opp-table to freq. So the logic is the same as before. I'm not sure if I've answered your concern.
On 2.01.2025 6:38 AM, Renjiang Han wrote: > > On 12/23/2024 10:17 PM, Konrad Dybcio wrote: >> On 19.12.2024 6:41 AM, 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. >>> If getting data from the opp table fails, fall back to using the frequency >>> table. >>> >>> Signed-off-by: Renjiang Han<quic_renjiang@quicinc.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 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..b61c0ad152878870b7223efa274e137d3636433b 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); >>> + } >> I'm not super convinced how this could have ever worked without >> scaling voltage levels, by the way. Perhaps this will squash some >> random bugs :| >> >> Konrad > Thanks for your comment. > The default value of freq is 0, and then dev_pm_opp_find_freq_ceil is > used to match freq to the maximum value in opp-table that is close to > 0. The frequency values in opp-table and freq_tbl are the same, and > dev_pm_opp_find_freq_ceil is used to assign the minimum value in > opp-table to freq. So the logic is the same as before. I'm not sure if > I've answered your concern. We talked offline, but for the record: my concern here was about clk_set_rate() not scaling RPM/h voltage corners, which this patch fixes Konrad
diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c index 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..b61c0ad152878870b7223efa274e137d3636433b 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)) { @@ -627,12 +633,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) { @@ -655,7 +664,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); @@ -1078,7 +1091,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++) @@ -1104,19 +1119,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:
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. If getting data from the opp table fails, fall back to using the frequency table. Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> --- drivers/media/platform/qcom/venus/pm_helpers.c | 53 +++++++++++++++++++------- 1 file changed, 39 insertions(+), 14 deletions(-)