| Message ID | 20250530-add-venus-for-qcs615-v8-0-c0092ac616d0@quicinc.com |
|---|---|
| Headers | show |
| Series | media: venus: enable venus on qcs615 | expand |
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. > If getting data from the opp table fails, fall back to using the frequency > table. > > Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com> > 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 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 >
On Fri, May 30, 2025 at 09:32:12AM +0530, Renjiang Han wrote: > QCS615 uses the same video core as SC7180, so reuse the same resource > data of SC7180 for QCS615 to enable video functionality. > > There are no resources for the video-decoder and video-encoder nodes > in the device tree, so remove these two nodes from the device tree. In > addition, to ensure that the video codec functions properly, use [3] > to add encoder and decoder node entries in the venus driver. > > Validated this series on QCS615 and SC7180. > > Note: > This series consist of DT patches and a venus driver patch. The patch > 1/3, which is venus driver patch, can be picked independently without > having any functional dependency. But patch 2/3 & patch 3/3, which are > DT patches, still depend on [1]. I'd say 2/3 and 3/3 still depend on 1/3, otherwise we can get video core on QCS615 over(?)clocked. > > [1] https://lore.kernel.org/all/20250119-qcs615-mm-v2-dt-nodes-v2-0-c46ab4080989@quicinc.com > > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> > --- > Changes in v8: > - 1. Add missing tags. > - 2. Fix the dependency to point to videoCC series alone. > - 3. Fix review comments from Konrad. > - Link to v7: https://lore.kernel.org/r/20250527-add-venus-for-qcs615-v7-0-cca26e2768e3@quicinc.com > > Changes in v7: > - 1. Update devicetree patch to fix the cherry-pick patch conflict issue. > - 2. Remove dt-bindings patch from this patch series due to it has been > picked. > - Link to v6: https://lore.kernel.org/r/20241219-add-venus-for-qcs615-v6-0-e9a74d3b003d@quicinc.com > > Changes in v6: > - 1. Remove video-decoder and video-encoder nodes from the device tree > - 2. Add a new dependency. > - 3. Fix missing tag. > - 4. Update commit message. > - Link to v5: https://lore.kernel.org/r/20241217-add-venus-for-qcs615-v5-0-747395d9e630@quicinc.com > > Changes in v5: > - 1. Remove extra blank lines in yaml files. > - 2. Add new variables in the driver while keeping the order of the > original variables. And remove unnecessary variable initialization. > - 3. Update commit message. > - 4. Update the order of nodes in the device tree. > - Link to v4: https://lore.kernel.org/r/20241213-add-venus-for-qcs615-v4-0-7e2c9a72d309@quicinc.com > > Changes in v4: > - 1. Remove qcom,qcs615-venus.yaml and use qcom,sc7180-venus.yaml for > qcs615 dt-bindings. > - 2. Add "qcom,qcs615-venus" compatible into qcom,sc7180-venus.yaml. > - 3. Remove qcs615 resource from the driver and use sc7180 resource for > the qcs615. > - 4. Use the frequency in the opp-table in devicetree for the driver. > For compatibility, if getting data from the opp table fails, the data > in the frequency table will be used. > - 5. Keep the reverse Christmas tree order coding style. > - 6. Add "qcom,sc7180-venus" compatible in devicetree. > - 7. Update cover letter message. > - Link to v3: https://lore.kernel.org/r/20241125-add-venus-for-qcs615-v3-0-5a376b97a68e@quicinc.com > > Changes in v3: > - 1. Remove the ‘|’ after 'description' in the qcom,qcs615-venus.yaml. > - 2. Add a blank line before 'opp-table' in the qcom,qcs615-venus.yaml. > - 3. Put ‘additionalProperties’ before ‘properties’ in the > qcom,qcs615-venus.yaml. > - 4. Update the subject of qcom,qcs615-venus.yaml patch. > - Link to v2: https://lore.kernel.org/r/20241112-add-venus-for-qcs615-v2-0-e67947f957af@quicinc.com > > Changes in v2: > - 1. The change-id of DT and driver are removed. > - 2. Add qcom,qcs615-venus.yaml files to explain DT. > - 3. The order of driver's commit and DT's commit is adjusted. Place the > driver's commit before the DT's commit. > - 4. Extends driver's commit message. > - 5. Split DT's commit into two commits. Add the venus node to the > qcs615.dtsi file. Then in the qcs615-ride.dts file enable the venus node. > - 6. Modify alignment, sort, upper and lower case letters issue. > - 7. Update cover letter message description. > - Link to v1: https://lore.kernel.org/r/20241008-add_qcs615_video-v1-0-436ce07bfc63@quicinc.com > > --- > Renjiang Han (3): > media: venus: pm_helpers: use opp-table for the frequency > arm64: dts: qcom: qcs615: add venus node to devicetree > arm64: dts: qcom: qcs615-ride: enable venus node to initialize video codec > > arch/arm64/boot/dts/qcom/qcs615-ride.dts | 4 ++ > arch/arm64/boot/dts/qcom/qcs615.dtsi | 78 ++++++++++++++++++++++++++ > drivers/media/platform/qcom/venus/pm_helpers.c | 53 ++++++++++++----- > 3 files changed, 121 insertions(+), 14 deletions(-) > --- > base-commit: 176e917e010cb7dcc605f11d2bc33f304292482b > change-id: 20250526-add-venus-for-qcs615-a547540656d1 > prerequisite-message-id: <20250119-qcs615-mm-v2-dt-nodes-v2-0-c46ab4080989@quicinc.com> > prerequisite-patch-id: afd2dce9e6066b1f6ce0b41ceafe0dd47ad97c40 > prerequisite-patch-id: f8d64c8cf6cd883dc7bbb2a4ed6d5a4db85c536d > > Best regards, > -- > Renjiang Han <quic_renjiang@quicinc.com> >
On 5/31/2025 4:27 AM, Dmitry Baryshkov wrote: > On Fri, May 30, 2025 at 09:32:12AM +0530, Renjiang Han wrote: >> QCS615 uses the same video core as SC7180, so reuse the same resource >> data of SC7180 for QCS615 to enable video functionality. >> >> There are no resources for the video-decoder and video-encoder nodes >> in the device tree, so remove these two nodes from the device tree. In >> addition, to ensure that the video codec functions properly, use [3] >> to add encoder and decoder node entries in the venus driver. >> >> Validated this series on QCS615 and SC7180. >> >> Note: >> This series consist of DT patches and a venus driver patch. The patch >> 1/3, which is venus driver patch, can be picked independently without >> having any functional dependency. But patch 2/3 & patch 3/3, which are >> DT patches, still depend on [1]. > I'd say 2/3 and 3/3 still depend on 1/3, otherwise we can get video core > on QCS615 over(?)clocked. Agree, so we need to make sure that the driver patch is not picked after the DT patch. > >> [1] https://lore.kernel.org/all/20250119-qcs615-mm-v2-dt-nodes-v2-0-c46ab4080989@quicinc.com >> >> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> >> --- >> Changes in v8: >> - 1. Add missing tags. >> - 2. Fix the dependency to point to videoCC series alone. >> - 3. Fix review comments from Konrad. >> - Link to v7: https://lore.kernel.org/r/20250527-add-venus-for-qcs615-v7-0-cca26e2768e3@quicinc.com >> >> Changes in v7: >> - 1. Update devicetree patch to fix the cherry-pick patch conflict issue. >> - 2. Remove dt-bindings patch from this patch series due to it has been >> picked. >> - Link to v6: https://lore.kernel.org/r/20241219-add-venus-for-qcs615-v6-0-e9a74d3b003d@quicinc.com >> >> Changes in v6: >> - 1. Remove video-decoder and video-encoder nodes from the device tree >> - 2. Add a new dependency. >> - 3. Fix missing tag. >> - 4. Update commit message. >> - Link to v5: https://lore.kernel.org/r/20241217-add-venus-for-qcs615-v5-0-747395d9e630@quicinc.com >> >> Changes in v5: >> - 1. Remove extra blank lines in yaml files. >> - 2. Add new variables in the driver while keeping the order of the >> original variables. And remove unnecessary variable initialization. >> - 3. Update commit message. >> - 4. Update the order of nodes in the device tree. >> - Link to v4: https://lore.kernel.org/r/20241213-add-venus-for-qcs615-v4-0-7e2c9a72d309@quicinc.com >> >> Changes in v4: >> - 1. Remove qcom,qcs615-venus.yaml and use qcom,sc7180-venus.yaml for >> qcs615 dt-bindings. >> - 2. Add "qcom,qcs615-venus" compatible into qcom,sc7180-venus.yaml. >> - 3. Remove qcs615 resource from the driver and use sc7180 resource for >> the qcs615. >> - 4. Use the frequency in the opp-table in devicetree for the driver. >> For compatibility, if getting data from the opp table fails, the data >> in the frequency table will be used. >> - 5. Keep the reverse Christmas tree order coding style. >> - 6. Add "qcom,sc7180-venus" compatible in devicetree. >> - 7. Update cover letter message. >> - Link to v3: https://lore.kernel.org/r/20241125-add-venus-for-qcs615-v3-0-5a376b97a68e@quicinc.com >> >> Changes in v3: >> - 1. Remove the ‘|’ after 'description' in the qcom,qcs615-venus.yaml. >> - 2. Add a blank line before 'opp-table' in the qcom,qcs615-venus.yaml. >> - 3. Put ‘additionalProperties’ before ‘properties’ in the >> qcom,qcs615-venus.yaml. >> - 4. Update the subject of qcom,qcs615-venus.yaml patch. >> - Link to v2: https://lore.kernel.org/r/20241112-add-venus-for-qcs615-v2-0-e67947f957af@quicinc.com >> >> Changes in v2: >> - 1. The change-id of DT and driver are removed. >> - 2. Add qcom,qcs615-venus.yaml files to explain DT. >> - 3. The order of driver's commit and DT's commit is adjusted. Place the >> driver's commit before the DT's commit. >> - 4. Extends driver's commit message. >> - 5. Split DT's commit into two commits. Add the venus node to the >> qcs615.dtsi file. Then in the qcs615-ride.dts file enable the venus node. >> - 6. Modify alignment, sort, upper and lower case letters issue. >> - 7. Update cover letter message description. >> - Link to v1: https://lore.kernel.org/r/20241008-add_qcs615_video-v1-0-436ce07bfc63@quicinc.com >> >> --- >> Renjiang Han (3): >> media: venus: pm_helpers: use opp-table for the frequency >> arm64: dts: qcom: qcs615: add venus node to devicetree >> arm64: dts: qcom: qcs615-ride: enable venus node to initialize video codec >> >> arch/arm64/boot/dts/qcom/qcs615-ride.dts | 4 ++ >> arch/arm64/boot/dts/qcom/qcs615.dtsi | 78 ++++++++++++++++++++++++++ >> drivers/media/platform/qcom/venus/pm_helpers.c | 53 ++++++++++++----- >> 3 files changed, 121 insertions(+), 14 deletions(-) >> --- >> base-commit: 176e917e010cb7dcc605f11d2bc33f304292482b >> change-id: 20250526-add-venus-for-qcs615-a547540656d1 >> prerequisite-message-id: <20250119-qcs615-mm-v2-dt-nodes-v2-0-c46ab4080989@quicinc.com> >> prerequisite-patch-id: afd2dce9e6066b1f6ce0b41ceafe0dd47ad97c40 >> prerequisite-patch-id: f8d64c8cf6cd883dc7bbb2a4ed6d5a4db85c536d >> >> Best regards, >> -- >> Renjiang Han <quic_renjiang@quicinc.com> >>
On 5/30/25 6:02 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. > > Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com> > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> > --- You can save yourself some error-checking pain in this patch if you do something like ret = devm_pm_opp_of_add_table(dev); if (ret == -ENODEV) { for (i = 0; i < freq_tbl_len; i++) dev_pm_opp_add(dev, freq_tbl[i], 0); } to programmatically migrate everyone to OPP.. But - tangent - I'd say efforts to preserve compatibility with DTs that don't even contain frequency data for a given target are rather futile.. Such descriptions where we only know the frequency (be it the tables that we currently have, or the constructed OPP tables that only contain frequency data and no voltage corners) are incomplete, and if the system manages to not crash if the driver requests a TURBO/max freq, it's all because we got lucky, as this consumer is not voting on (MM)CX. That said, it's probably to keep the breakage to minimum, especially given this is an old driver for old hardware.. I'll add the missing OPPs across platforms with an intention to drop the hardcoding somewhere in the future Konrad
QCS615 uses the same video core as SC7180, so reuse the same resource data of SC7180 for QCS615 to enable video functionality. There are no resources for the video-decoder and video-encoder nodes in the device tree, so remove these two nodes from the device tree. In addition, to ensure that the video codec functions properly, use [3] to add encoder and decoder node entries in the venus driver. Validated this series on QCS615 and SC7180. Note: This series consist of DT patches and a venus driver patch. The patch 1/3, which is venus driver patch, can be picked independently without having any functional dependency. But patch 2/3 & patch 3/3, which are DT patches, still depend on [1]. [1] https://lore.kernel.org/all/20250119-qcs615-mm-v2-dt-nodes-v2-0-c46ab4080989@quicinc.com Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> --- Changes in v8: - 1. Add missing tags. - 2. Fix the dependency to point to videoCC series alone. - 3. Fix review comments from Konrad. - Link to v7: https://lore.kernel.org/r/20250527-add-venus-for-qcs615-v7-0-cca26e2768e3@quicinc.com Changes in v7: - 1. Update devicetree patch to fix the cherry-pick patch conflict issue. - 2. Remove dt-bindings patch from this patch series due to it has been picked. - Link to v6: https://lore.kernel.org/r/20241219-add-venus-for-qcs615-v6-0-e9a74d3b003d@quicinc.com Changes in v6: - 1. Remove video-decoder and video-encoder nodes from the device tree - 2. Add a new dependency. - 3. Fix missing tag. - 4. Update commit message. - Link to v5: https://lore.kernel.org/r/20241217-add-venus-for-qcs615-v5-0-747395d9e630@quicinc.com Changes in v5: - 1. Remove extra blank lines in yaml files. - 2. Add new variables in the driver while keeping the order of the original variables. And remove unnecessary variable initialization. - 3. Update commit message. - 4. Update the order of nodes in the device tree. - Link to v4: https://lore.kernel.org/r/20241213-add-venus-for-qcs615-v4-0-7e2c9a72d309@quicinc.com Changes in v4: - 1. Remove qcom,qcs615-venus.yaml and use qcom,sc7180-venus.yaml for qcs615 dt-bindings. - 2. Add "qcom,qcs615-venus" compatible into qcom,sc7180-venus.yaml. - 3. Remove qcs615 resource from the driver and use sc7180 resource for the qcs615. - 4. Use the frequency in the opp-table in devicetree for the driver. For compatibility, if getting data from the opp table fails, the data in the frequency table will be used. - 5. Keep the reverse Christmas tree order coding style. - 6. Add "qcom,sc7180-venus" compatible in devicetree. - 7. Update cover letter message. - Link to v3: https://lore.kernel.org/r/20241125-add-venus-for-qcs615-v3-0-5a376b97a68e@quicinc.com Changes in v3: - 1. Remove the ‘|’ after 'description' in the qcom,qcs615-venus.yaml. - 2. Add a blank line before 'opp-table' in the qcom,qcs615-venus.yaml. - 3. Put ‘additionalProperties’ before ‘properties’ in the qcom,qcs615-venus.yaml. - 4. Update the subject of qcom,qcs615-venus.yaml patch. - Link to v2: https://lore.kernel.org/r/20241112-add-venus-for-qcs615-v2-0-e67947f957af@quicinc.com Changes in v2: - 1. The change-id of DT and driver are removed. - 2. Add qcom,qcs615-venus.yaml files to explain DT. - 3. The order of driver's commit and DT's commit is adjusted. Place the driver's commit before the DT's commit. - 4. Extends driver's commit message. - 5. Split DT's commit into two commits. Add the venus node to the qcs615.dtsi file. Then in the qcs615-ride.dts file enable the venus node. - 6. Modify alignment, sort, upper and lower case letters issue. - 7. Update cover letter message description. - Link to v1: https://lore.kernel.org/r/20241008-add_qcs615_video-v1-0-436ce07bfc63@quicinc.com --- Renjiang Han (3): media: venus: pm_helpers: use opp-table for the frequency arm64: dts: qcom: qcs615: add venus node to devicetree arm64: dts: qcom: qcs615-ride: enable venus node to initialize video codec arch/arm64/boot/dts/qcom/qcs615-ride.dts | 4 ++ arch/arm64/boot/dts/qcom/qcs615.dtsi | 78 ++++++++++++++++++++++++++ drivers/media/platform/qcom/venus/pm_helpers.c | 53 ++++++++++++----- 3 files changed, 121 insertions(+), 14 deletions(-) --- base-commit: 176e917e010cb7dcc605f11d2bc33f304292482b change-id: 20250526-add-venus-for-qcs615-a547540656d1 prerequisite-message-id: <20250119-qcs615-mm-v2-dt-nodes-v2-0-c46ab4080989@quicinc.com> prerequisite-patch-id: afd2dce9e6066b1f6ce0b41ceafe0dd47ad97c40 prerequisite-patch-id: f8d64c8cf6cd883dc7bbb2a4ed6d5a4db85c536d Best regards,