| Message ID | 20241125-add-venus-for-qcs615-v3-1-5a376b97a68e@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: venus: enable venus on qcs615 | expand |
On Mon, 25 Nov 2024 11:04:49 +0530, Renjiang Han wrote: > Add support for Qualcomm video acceleration hardware used for video > stream decoding and encoding on QCOM QCS615. > > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> > --- > .../bindings/media/qcom,qcs615-venus.yaml | 182 +++++++++++++++++++++ > 1 file changed, 182 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/media/qcom,qcs615-venus.example.dts:25:18: fatal error: dt-bindings/clock/qcom,qcs615-videocc.h: No such file or directory 25 | #include <dt-bindings/clock/qcom,qcs615-videocc.h> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. make[2]: *** [scripts/Makefile.dtbs:129: Documentation/devicetree/bindings/media/qcom,qcs615-venus.example.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1442: dt_binding_check] Error 2 make: *** [Makefile:224: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241125-add-venus-for-qcs615-v3-1-5a376b97a68e@quicinc.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On Mon, Nov 25, 2024 at 11:04:49AM +0530, Renjiang Han wrote: > Add support for Qualcomm video acceleration hardware used for video > stream decoding and encoding on QCOM QCS615. > > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> > --- > .../bindings/media/qcom,qcs615-venus.yaml | 182 +++++++++++++++++++++ > 1 file changed, 182 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml Dependency for this patch must be mentioned here. Amount of dependencies make it unmergeable and untesteable. I suggest decoupling dependencies by removing clock constants. > new file mode 100644 > index 0000000000000000000000000000000000000000..7a3a01ff06d8b62bc2424a0a24857c86c6865f89 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml > @@ -0,0 +1,182 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/qcom,qcs615-venus.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm QCS615 Venus video encode and decode accelerators > + > +maintainers: > + - Stanimir Varbanov <stanimir.k.varbanov@gmail.com> > + - Vikash Garodia <quic_vgarodia@quicinc.com> > + > +description: > + The Venus IP is a video encode and decode accelerator present > + on Qualcomm platforms > + > +allOf: > + - $ref: qcom,venus-common.yaml# > + > +properties: > + compatible: > + const: qcom,qcs615-venus > + > + power-domains: > + minItems: 2 > + maxItems: 3 > + > + power-domain-names: > + minItems: 2 > + items: > + - const: venus > + - const: vcodec0 > + - const: cx > + > + clocks: > + maxItems: 5 > + > + clock-names: > + items: > + - const: core > + - const: iface > + - const: bus > + - const: vcodec0_core > + - const: vcodec0_bus > + > + iommus: > + maxItems: 1 > + > + memory-region: > + maxItems: 1 > + > + interconnects: > + maxItems: 2 > + > + interconnect-names: > + items: > + - const: video-mem > + - const: cpu-cfg > + > + operating-points-v2: true > + > + opp-table: > + type: object > + > + video-decoder: > + type: object > + > + additionalProperties: false > + > + properties: > + compatible: > + const: venus-decoder > + > + required: > + - compatible > + > + video-encoder: > + type: object Both nodes are useless - no resources here, nothing to control. Do not add nodes just to instantiate Linux drivers. Drop them. Best regards, Krzysztof
On Monday, November 25, 2024 2:38 PM, Rob Herring (Arm) wrote: > On Mon, 25 Nov 2024 11:04:49 +0530, Renjiang Han wrote: > > Add support for Qualcomm video acceleration hardware used for video > > stream decoding and encoding on QCOM QCS615. > > > > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> > > --- > > .../bindings/media/qcom,qcs615-venus.yaml | 182 +++++++++++++++++++++ > > 1 file changed, 182 insertions(+) > > > My bot found errors running 'make dt_binding_check' on your patch: > yamllint warnings/errors: > dtschema/dtc warnings/errors: > Documentation/devicetree/bindings/media/qcom,qcs615-venus.example.dts:25:18: fatal error: dt-bindings/clock/qcom,qcs615-videocc.h: No such file or directory > 25 | #include <dt-bindings/clock/qcom,qcs615-videocc.h> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > compilation terminated. > make[2]: *** [scripts/Makefile.dtbs:129: Documentation/devicetree/bindings/media/qcom,qcs615-venus.example.dtb] Error 1 > make[2]: *** Waiting for unfinished jobs.... > make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1442: dt_binding_check] Error 2 > make: *** [Makefile:224: __sub-make] Error 2 Thanks for your reply. "dt-bindings/clock/qcom,qcs615-videocc.h" has been added in https://lore.kernel.org/all/20241108-qcs615-mm-clockcontroller-v3-0-7d3b2d235fdf@quicinc.com/. > doc reference errors (make refcheckdocs): > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241125-add-venus-for-qcs615-v3-1-5a376b97a68e@quicinc.com > The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. > If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > pip3 install dtschema --upgrade > Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 25/11/2024 09:06, Renjiang Han (QUIC) wrote: > On Monday, November 25, 2024 2:38 PM, Rob Herring (Arm) wrote: >> On Mon, 25 Nov 2024 11:04:49 +0530, Renjiang Han wrote: >>> Add support for Qualcomm video acceleration hardware used for video >>> stream decoding and encoding on QCOM QCS615. >>> >>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> >>> --- >>> .../bindings/media/qcom,qcs615-venus.yaml | 182 +++++++++++++++++++++ >>> 1 file changed, 182 insertions(+) >>> > >> My bot found errors running 'make dt_binding_check' on your patch: > >> yamllint warnings/errors: > >> dtschema/dtc warnings/errors: >> Documentation/devicetree/bindings/media/qcom,qcs615-venus.example.dts:25:18: fatal error: dt-bindings/clock/qcom,qcs615-videocc.h: No such file or directory >> 25 | #include <dt-bindings/clock/qcom,qcs615-videocc.h> >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> compilation terminated. >> make[2]: *** [scripts/Makefile.dtbs:129: Documentation/devicetree/bindings/media/qcom,qcs615-venus.example.dtb] Error 1 >> make[2]: *** Waiting for unfinished jobs.... >> make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1442: dt_binding_check] Error 2 >> make: *** [Makefile:224: __sub-make] Error 2 > > Thanks for your reply. "dt-bindings/clock/qcom,qcs615-videocc.h" has been added in https://lore.kernel.org/all/20241108-qcs615-mm-clockcontroller-v3-0-7d3b2d235fdf@quicinc.com/. Why do you ignore rest of the email? > >> doc reference errors (make refcheckdocs): > >> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241125-add-venus-for-qcs615-v3-1-5a376b97a68e@quicinc.com > >> The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. Read, here. Best regards, Krzysztof
On Mon 11/25/2024 3:50 PM, Krzysztof Kozlowski wrote: > On Mon, Nov 25, 2024 at 11:04:49AM +0530, Renjiang Han wrote: > > Add support for Qualcomm video acceleration hardware used for video > > stream decoding and encoding on QCOM QCS615. > > > > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> > > > --- > > .../bindings/media/qcom,qcs615-venus.yaml | 182 +++++++++++++++++++++ > > 1 file changed, 182 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml > > b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml > Dependency for this patch must be mentioned here. > Amount of dependencies make it unmergeable and untesteable. > I suggest decoupling dependencies by removing clock constants. Thanks for your comment, I will try to remove the clock constants and use clock id instead. > > new file mode 100644 > > index > > 0000000000000000000000000000000000000000..7a3a01ff06d8b62bc2424a0a2485 > > 7c86c6865f89 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml > > @@ -0,0 +1,182 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/media/qcom,qcs615-venus.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Qualcomm QCS615 Venus video encode and decode accelerators > > + > > +maintainers: > > + - Stanimir Varbanov <stanimir.k.varbanov@gmail.com> > > > + - Vikash Garodia <quic_vgarodia@quicinc.com> > > > + > > +description: > > + The Venus IP is a video encode and decode accelerator present > > + on Qualcomm platforms > > + > > +allOf: > > + - $ref: qcom,venus-common.yaml# > > + > > +properties: > > + compatible: > > + const: qcom,qcs615-venus > > + > > + power-domains: > > + minItems: 2 > > + maxItems: 3 > > + > > + power-domain-names: > > + minItems: 2 > > + items: > > + - const: venus > > + - const: vcodec0 > > + - const: cx > > + > > + clocks: > > + maxItems: 5 > > + > > + clock-names: > > + items: > > + - const: core > > + - const: iface > > + - const: bus > > + - const: vcodec0_core > > + - const: vcodec0_bus > > + > > + iommus: > > + maxItems: 1 > > + > > + memory-region: > > + maxItems: 1 > > + > > + interconnects: > > + maxItems: 2 > > + > > + interconnect-names: > > + items: > > + - const: video-mem > > + - const: cpu-cfg > > + > > + operating-points-v2: true > > + > > + opp-table: > > + type: object > > + > > + video-decoder: > > + type: object > > + > > + additionalProperties: false > > + > > + properties: > > + compatible: > > + const: venus-decoder > > + > > + required: > > + - compatible > > + > > + video-encoder: > > + type: object > Both nodes are useless - no resources here, nothing to control. > Do not add nodes just to instantiate Linux drivers. Drop them. Do you mean I should remove video-decoder and video-encoder from here? If so, do I also need to remove these two nodes from the dtsi file and add them in the qcs615-ride.dts file? > Best regards, > Krzysztof
On 25/11/2024 16:49, Renjiang Han (QUIC) wrote: >>> + video-decoder: >>> + type: object >>> + >>> + additionalProperties: false >>> + >>> + properties: >>> + compatible: >>> + const: venus-decoder >>> + >>> + required: >>> + - compatible >>> + >>> + video-encoder: >>> + type: object > >> Both nodes are useless - no resources here, nothing to control. >> Do not add nodes just to instantiate Linux drivers. Drop them. > Do you mean I should remove video-decoder and video-encoder from here? Yes, that's my suggestion. > If so, do I also need to remove these two nodes from the dtsi file and add Yes > them in the qcs615-ride.dts file? Well, no, how would it pass dtbs_check? Don't add nodes purely for Linux driver instantiation. Best regards, Krzysztof
On Mon, Nov 25, 2024 at 11:04:49AM +0530, Renjiang Han wrote: > Add support for Qualcomm video acceleration hardware used for video > stream decoding and encoding on QCOM QCS615. > > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> > --- > .../bindings/media/qcom,qcs615-venus.yaml | 182 +++++++++++++++++++++ > 1 file changed, 182 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..7a3a01ff06d8b62bc2424a0a24857c86c6865f89 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml > @@ -0,0 +1,182 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/qcom,qcs615-venus.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm QCS615 Venus video encode and decode accelerators > + > +maintainers: > + - Stanimir Varbanov <stanimir.k.varbanov@gmail.com> > + - Vikash Garodia <quic_vgarodia@quicinc.com> > + > +description: > + The Venus IP is a video encode and decode accelerator present > + on Qualcomm platforms > + > +allOf: > + - $ref: qcom,venus-common.yaml# > + > +properties: > + compatible: > + const: qcom,qcs615-venus Please extend sc7180-venus.yaml instead. No need to duplicate unnecessary things. > + > + power-domains: > + minItems: 2 > + maxItems: 3 So, is it 2 or 3? You don't have legacy here, so you should know an exact number. > + > + power-domain-names: > + minItems: 2 And this one also can go away. > + items: > + - const: venus > + - const: vcodec0 > + - const: cx > + > + clocks: > + maxItems: 5 > + > + clock-names: > + items: > + - const: core > + - const: iface > + - const: bus > + - const: vcodec0_core > + - const: vcodec0_bus > + > + iommus: > + maxItems: 1 > + > + memory-region: > + maxItems: 1 > + > + interconnects: > + maxItems: 2 > + > + interconnect-names: > + items: > + - const: video-mem > + - const: cpu-cfg > + > + operating-points-v2: true > + > + opp-table: > + type: object > + > + video-decoder: > + type: object > + > + additionalProperties: false > + > + properties: > + compatible: > + const: venus-decoder > + > + required: > + - compatible > + > + video-encoder: > + type: object > + > + additionalProperties: false > + > + properties: > + compatible: > + const: venus-encoder > + > + required: > + - compatible > + > +required: > + - compatible > + - power-domain-names > + - iommus > + - video-decoder > + - video-encoder > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/clock/qcom,qcs615-videocc.h> > + #include <dt-bindings/interconnect/qcom,qcs615-rpmh.h> > + #include <dt-bindings/power/qcom,rpmhpd.h> > + > + venus: video-codec@aa00000 { > + compatible = "qcom,qcs615-venus"; > + reg = <0xaa00000 0x100000>; > + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; > + > + clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>, > + <&videocc VIDEO_CC_VENUS_AHB_CLK>, > + <&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>, > + <&videocc VIDEO_CC_VCODEC0_CORE_CLK>, > + <&videocc VIDEO_CC_VCODEC0_AXI_CLK>; > + clock-names = "core", > + "iface", > + "bus", > + "vcodec0_core", > + "vcodec0_bus"; > + > + power-domains = <&videocc VENUS_GDSC>, > + <&videocc VCODEC0_GDSC>, > + <&rpmhpd RPMHPD_CX>; > + power-domain-names = "venus", > + "vcodec0", > + "cx"; > + > + operating-points-v2 = <&venus_opp_table>; > + > + interconnects = <&mmss_noc MASTER_VIDEO_P0 0 > + &mc_virt SLAVE_EBI1 0>, > + <&gem_noc MASTER_APPSS_PROC 0 > + &config_noc SLAVE_VENUS_CFG 0>; > + interconnect-names = "video-mem", > + "cpu-cfg"; > + > + iommus = <&apps_smmu 0xe40 0x20>; > + > + memory-region = <&pil_video_mem>; > + > + video-decoder { > + compatible = "venus-decoder"; > + }; > + > + video-encoder { > + compatible = "venus-encoder"; > + }; > + > + venus_opp_table: opp-table { > + compatible = "operating-points-v2"; > + > + opp-133330000 { > + opp-hz = /bits/ 64 <133330000>; > + required-opps = <&rpmhpd_opp_low_svs>; > + }; > + > + opp-240000000 { > + opp-hz = /bits/ 64 <240000000>; > + required-opps = <&rpmhpd_opp_svs>; > + }; > + > + opp-300000000 { > + opp-hz = /bits/ 64 <300000000>; > + required-opps = <&rpmhpd_opp_svs_l1>; > + }; > + > + opp-380000000 { > + opp-hz = /bits/ 64 <380000000>; > + required-opps = <&rpmhpd_opp_nom>; > + }; > + > + opp-410000000 { > + opp-hz = /bits/ 64 <410000000>; > + required-opps = <&rpmhpd_opp_turbo>; > + }; > + > + opp-460000000 { > + opp-hz = /bits/ 64 <460000000>; > + required-opps = <&rpmhpd_opp_turbo_l1>; > + }; > + }; > + }; > > -- > 2.34.1 >
On 11/25/2024 11:55 PM, Krzysztof Kozlowski wrote: > On 25/11/2024 16:49, Renjiang Han (QUIC) wrote: >>>> + video-decoder: >>>> + type: object >>>> + >>>> + additionalProperties: false >>>> + >>>> + properties: >>>> + compatible: >>>> + const: venus-decoder >>>> + >>>> + required: >>>> + - compatible >>>> + >>>> + video-encoder: >>>> + type: object >>> Both nodes are useless - no resources here, nothing to control. >>> Do not add nodes just to instantiate Linux drivers. Drop them. >> Do you mean I should remove video-decoder and video-encoder from here? > Yes, that's my suggestion. > >> If so, do I also need to remove these two nodes from the dtsi file and add > Yes > >> them in the qcs615-ride.dts file? > Well, no, how would it pass dtbs_check? > > Don't add nodes purely for Linux driver instantiation. OK, I got it. I'll update like this. If video-decoder and video-encoder are removed from dtsi file and not added to qcs615-ride.dts file, then the video decoder and encoder functions will not be available on the qcs615 platform. So I think these two nodes should be added to the qcs615-ride.dts file to ensure that the qcs615 platform can enable the video decoder and encoder functions. > Best regards, > Krzysztof
On 26/11/2024 07:07, Renjiang Han wrote: > > On 11/25/2024 11:55 PM, Krzysztof Kozlowski wrote: >> On 25/11/2024 16:49, Renjiang Han (QUIC) wrote: >>>>> + video-decoder: >>>>> + type: object >>>>> + >>>>> + additionalProperties: false >>>>> + >>>>> + properties: >>>>> + compatible: >>>>> + const: venus-decoder >>>>> + >>>>> + required: >>>>> + - compatible >>>>> + >>>>> + video-encoder: >>>>> + type: object >>>> Both nodes are useless - no resources here, nothing to control. >>>> Do not add nodes just to instantiate Linux drivers. Drop them. >>> Do you mean I should remove video-decoder and video-encoder from here? >> Yes, that's my suggestion. >> >>> If so, do I also need to remove these two nodes from the dtsi file and add >> Yes >> >>> them in the qcs615-ride.dts file? >> Well, no, how would it pass dtbs_check? >> >> Don't add nodes purely for Linux driver instantiation. > OK, I got it. I'll update like this. If video-decoder and video-encoder are > > removed from dtsi file and not added to qcs615-ride.dts file, then the > > video decoder and encoder functions will not be available on the qcs615 > > platform. So I think these two nodes should be added to the > > qcs615-ride.dts file to ensure that the qcs615 platform can enable the > > video decoder and encoder functions. You just repeated the same sentences. Address my comment instead - empty device nodes should not be used just to instantiate Linux device drivers. Best regards, Krzysztof
On 11/26/2024 12:12 AM, Dmitry Baryshkov wrote: > On Mon, Nov 25, 2024 at 11:04:49AM +0530, Renjiang Han wrote: >> Add support for Qualcomm video acceleration hardware used for video >> stream decoding and encoding on QCOM QCS615. >> >> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> >> --- >> .../bindings/media/qcom,qcs615-venus.yaml | 182 +++++++++++++++++++++ >> 1 file changed, 182 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml >> new file mode 100644 >> index 0000000000000000000000000000000000000000..7a3a01ff06d8b62bc2424a0a24857c86c6865f89 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml >> @@ -0,0 +1,182 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/media/qcom,qcs615-venus.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm QCS615 Venus video encode and decode accelerators >> + >> +maintainers: >> + - Stanimir Varbanov <stanimir.k.varbanov@gmail.com> >> + - Vikash Garodia <quic_vgarodia@quicinc.com> >> + >> +description: >> + The Venus IP is a video encode and decode accelerator present >> + on Qualcomm platforms >> + >> +allOf: >> + - $ref: qcom,venus-common.yaml# >> + >> +properties: >> + compatible: >> + const: qcom,qcs615-venus > Please extend sc7180-venus.yaml instead. No need to duplicate > unnecessary things. Thanks for your review. But I'm sorry I can't get it. The devicetree for qcs615-venus is in qcs615.dtsi. I'm not sure how to use sc7180-venus.yaml instead. >> + >> + power-domains: >> + minItems: 2 >> + maxItems: 3 > So, is it 2 or 3? You don't have legacy here, so you should know an > exact number. Got it. Both minItems and maxItems should be 3. I'll update with next version. > >> + >> + power-domain-names: >> + minItems: 2 > And this one also can go away. OK. Thanks for pointing it out. I'll remove this minItems with next version. >> + items: >> + - const: venus >> + - const: vcodec0 >> + - const: cx >> + >> + clocks: >> + maxItems: 5 >> + >> + clock-names: >> + items: >> + - const: core >> + - const: iface >> + - const: bus >> + - const: vcodec0_core >> + - const: vcodec0_bus >> + >> + iommus: >> + maxItems: 1 >> + >> + memory-region: >> + maxItems: 1 >> + >> + interconnects: >> + maxItems: 2 >> + >> + interconnect-names: >> + items: >> + - const: video-mem >> + - const: cpu-cfg >> + >> + operating-points-v2: true >> + >> + opp-table: >> + type: object >> + >> + video-decoder: >> + type: object >> + >> + additionalProperties: false >> + >> + properties: >> + compatible: >> + const: venus-decoder >> + >> + required: >> + - compatible >> + >> + video-encoder: >> + type: object >> + >> + additionalProperties: false >> + >> + properties: >> + compatible: >> + const: venus-encoder >> + >> + required: >> + - compatible >> + >> +required: >> + - compatible >> + - power-domain-names >> + - iommus >> + - video-decoder >> + - video-encoder >> + >> +unevaluatedProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + #include <dt-bindings/clock/qcom,qcs615-videocc.h> >> + #include <dt-bindings/interconnect/qcom,qcs615-rpmh.h> >> + #include <dt-bindings/power/qcom,rpmhpd.h> >> + >> + venus: video-codec@aa00000 { >> + compatible = "qcom,qcs615-venus"; >> + reg = <0xaa00000 0x100000>; >> + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; >> + >> + clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>, >> + <&videocc VIDEO_CC_VENUS_AHB_CLK>, >> + <&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>, >> + <&videocc VIDEO_CC_VCODEC0_CORE_CLK>, >> + <&videocc VIDEO_CC_VCODEC0_AXI_CLK>; >> + clock-names = "core", >> + "iface", >> + "bus", >> + "vcodec0_core", >> + "vcodec0_bus"; >> + >> + power-domains = <&videocc VENUS_GDSC>, >> + <&videocc VCODEC0_GDSC>, >> + <&rpmhpd RPMHPD_CX>; >> + power-domain-names = "venus", >> + "vcodec0", >> + "cx"; >> + >> + operating-points-v2 = <&venus_opp_table>; >> + >> + interconnects = <&mmss_noc MASTER_VIDEO_P0 0 >> + &mc_virt SLAVE_EBI1 0>, >> + <&gem_noc MASTER_APPSS_PROC 0 >> + &config_noc SLAVE_VENUS_CFG 0>; >> + interconnect-names = "video-mem", >> + "cpu-cfg"; >> + >> + iommus = <&apps_smmu 0xe40 0x20>; >> + >> + memory-region = <&pil_video_mem>; >> + >> + video-decoder { >> + compatible = "venus-decoder"; >> + }; >> + >> + video-encoder { >> + compatible = "venus-encoder"; >> + }; >> + >> + venus_opp_table: opp-table { >> + compatible = "operating-points-v2"; >> + >> + opp-133330000 { >> + opp-hz = /bits/ 64 <133330000>; >> + required-opps = <&rpmhpd_opp_low_svs>; >> + }; >> + >> + opp-240000000 { >> + opp-hz = /bits/ 64 <240000000>; >> + required-opps = <&rpmhpd_opp_svs>; >> + }; >> + >> + opp-300000000 { >> + opp-hz = /bits/ 64 <300000000>; >> + required-opps = <&rpmhpd_opp_svs_l1>; >> + }; >> + >> + opp-380000000 { >> + opp-hz = /bits/ 64 <380000000>; >> + required-opps = <&rpmhpd_opp_nom>; >> + }; >> + >> + opp-410000000 { >> + opp-hz = /bits/ 64 <410000000>; >> + required-opps = <&rpmhpd_opp_turbo>; >> + }; >> + >> + opp-460000000 { >> + opp-hz = /bits/ 64 <460000000>; >> + required-opps = <&rpmhpd_opp_turbo_l1>; >> + }; >> + }; >> + }; >> >> -- >> 2.34.1 >>
On 26/11/2024 09:57, Renjiang Han wrote: >>> +description: >>> + The Venus IP is a video encode and decode accelerator present >>> + on Qualcomm platforms >>> + >>> +allOf: >>> + - $ref: qcom,venus-common.yaml# >>> + >>> +properties: >>> + compatible: >>> + const: qcom,qcs615-venus >> Please extend sc7180-venus.yaml instead. No need to duplicate >> unnecessary things. > > Thanks for your review. But I'm sorry I can't get it. The devicetree for > > qcs615-venus is in qcs615.dtsi. I'm not sure how to use sc7180-venus.yaml > > instead. DTSI is not relevant here to the bindings. I don't understand the problem, so not sure what you are asking here about. Best regards, Krzysztof
On 11/26/2024 2:42 PM, Krzysztof Kozlowski wrote: > On 26/11/2024 07:07, Renjiang Han wrote: >> On 11/25/2024 11:55 PM, Krzysztof Kozlowski wrote: >>> On 25/11/2024 16:49, Renjiang Han (QUIC) wrote: >>>>>> + video-decoder: >>>>>> + type: object >>>>>> + >>>>>> + additionalProperties: false >>>>>> + >>>>>> + properties: >>>>>> + compatible: >>>>>> + const: venus-decoder >>>>>> + >>>>>> + required: >>>>>> + - compatible >>>>>> + >>>>>> + video-encoder: >>>>>> + type: object >>>>> Both nodes are useless - no resources here, nothing to control. >>>>> Do not add nodes just to instantiate Linux drivers. Drop them. >>>> Do you mean I should remove video-decoder and video-encoder from here? >>> Yes, that's my suggestion. >>> >>>> If so, do I also need to remove these two nodes from the dtsi file and add >>> Yes >>> >>>> them in the qcs615-ride.dts file? >>> Well, no, how would it pass dtbs_check? >>> >>> Don't add nodes purely for Linux driver instantiation. >> OK, I got it. I'll update like this. If video-decoder and video-encoder are >> >> removed from dtsi file and not added to qcs615-ride.dts file, then the >> >> video decoder and encoder functions will not be available on the qcs615 >> >> platform. So I think these two nodes should be added to the >> >> qcs615-ride.dts file to ensure that the qcs615 platform can enable the >> >> video decoder and encoder functions. > You just repeated the same sentences. Address my comment instead - empty > device nodes should not be used just to instantiate Linux device drivers. Thanks for your reply. I agree with your comment. The two nodes video-decoder and video-encoder should not be placed in the devicetree. But this is affected by the venus driver. On the old platform, some only need to enable the video-decoder function or only enable the video-encoder function. So these two nodes were added to the devicetree at that time. For new platforms, the iris driver will be used in the future, and this situation will not occur. > Best regards, > Krzysztof
On 26/11/2024 10:39, Renjiang Han wrote: >>>>> If so, do I also need to remove these two nodes from the dtsi file and add >>>> Yes >>>> >>>>> them in the qcs615-ride.dts file? >>>> Well, no, how would it pass dtbs_check? >>>> >>>> Don't add nodes purely for Linux driver instantiation. >>> OK, I got it. I'll update like this. If video-decoder and video-encoder are >>> >>> removed from dtsi file and not added to qcs615-ride.dts file, then the >>> >>> video decoder and encoder functions will not be available on the qcs615 >>> >>> platform. So I think these two nodes should be added to the >>> >>> qcs615-ride.dts file to ensure that the qcs615 platform can enable the >>> >>> video decoder and encoder functions. >> You just repeated the same sentences. Address my comment instead - empty >> device nodes should not be used just to instantiate Linux device drivers. > > Thanks for your reply. I agree with your comment. The two nodes > video-decoder and > > video-encoder should not be placed in the devicetree. But this is > affected by the venus > > driver. On the old platform, some only need to enable the video-decoder > function or > > only enable the video-encoder function. So these two nodes were added to the > > devicetree at that time. For new platforms, the iris driver will be used > in the future, > > and this situation will not occur. These are new bindings, for new device, so please fix your driver. We had similar talk long time ago and answer was that it's a legacy driver which won't be developed. This means also no new devices. If you bring new devices to old driver, instead of to new iris, then it means you still develop old driver. Fix the old driver. Best regards, Krzysztof
On 11/26/2024 5:34 PM, Krzysztof Kozlowski wrote: > On 26/11/2024 09:57, Renjiang Han wrote: >>>> +description: >>>> + The Venus IP is a video encode and decode accelerator present >>>> + on Qualcomm platforms >>>> + >>>> +allOf: >>>> + - $ref: qcom,venus-common.yaml# >>>> + >>>> +properties: >>>> + compatible: >>>> + const: qcom,qcs615-venus >>> Please extend sc7180-venus.yaml instead. No need to duplicate >>> unnecessary things. >> Thanks for your review. But I'm sorry I can't get it. The devicetree for >> >> qcs615-venus is in qcs615.dtsi. I'm not sure how to use sc7180-venus.yaml >> >> instead. > DTSI is not relevant here to the bindings. I don't understand the > problem, so not sure what you are asking here about. The opp-table parameters are different in devicetree. Can we also use the same yaml file? > Best regards, > Krzysztof
On 26/11/2024 10:58, Renjiang Han wrote: > > On 11/26/2024 5:34 PM, Krzysztof Kozlowski wrote: >> On 26/11/2024 09:57, Renjiang Han wrote: >>>>> +description: >>>>> + The Venus IP is a video encode and decode accelerator present >>>>> + on Qualcomm platforms >>>>> + >>>>> +allOf: >>>>> + - $ref: qcom,venus-common.yaml# >>>>> + >>>>> +properties: >>>>> + compatible: >>>>> + const: qcom,qcs615-venus >>>> Please extend sc7180-venus.yaml instead. No need to duplicate >>>> unnecessary things. >>> Thanks for your review. But I'm sorry I can't get it. The devicetree for >>> >>> qcs615-venus is in qcs615.dtsi. I'm not sure how to use sc7180-venus.yaml >>> >>> instead. >> DTSI is not relevant here to the bindings. I don't understand the >> problem, so not sure what you are asking here about. > The opp-table parameters are different in devicetree. Can we also use > the same yaml file? Please look at existing bindings for other devices. Best regards, Krzysztof
On Tue, Nov 26, 2024 at 05:58:50PM +0800, Renjiang Han wrote: > > On 11/26/2024 5:34 PM, Krzysztof Kozlowski wrote: > > On 26/11/2024 09:57, Renjiang Han wrote: > > > > > +description: > > > > > + The Venus IP is a video encode and decode accelerator present > > > > > + on Qualcomm platforms > > > > > + > > > > > +allOf: > > > > > + - $ref: qcom,venus-common.yaml# > > > > > + > > > > > +properties: > > > > > + compatible: > > > > > + const: qcom,qcs615-venus > > > > Please extend sc7180-venus.yaml instead. No need to duplicate > > > > unnecessary things. > > > Thanks for your review. But I'm sorry I can't get it. The devicetree for > > > > > > qcs615-venus is in qcs615.dtsi. I'm not sure how to use sc7180-venus.yaml > > > > > > instead. > > DTSI is not relevant here to the bindings. I don't understand the > > problem, so not sure what you are asking here about. > The opp-table parameters are different in devicetree. Can we also use the > same yaml file? Is the contents of the OPP table a part of the bindings? > > Best regards, > > Krzysztof > > -- > Best Regards, > Renjiang >
On 11/25/2024 9:42 PM, Dmitry Baryshkov wrote: > On Mon, Nov 25, 2024 at 11:04:49AM +0530, Renjiang Han wrote: >> Add support for Qualcomm video acceleration hardware used for video >> stream decoding and encoding on QCOM QCS615. >> >> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> >> --- >> .../bindings/media/qcom,qcs615-venus.yaml | 182 +++++++++++++++++++++ >> 1 file changed, 182 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml >> new file mode 100644 >> index 0000000000000000000000000000000000000000..7a3a01ff06d8b62bc2424a0a24857c86c6865f89 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml >> @@ -0,0 +1,182 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/media/qcom,qcs615-venus.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm QCS615 Venus video encode and decode accelerators >> + >> +maintainers: >> + - Stanimir Varbanov <stanimir.k.varbanov@gmail.com> >> + - Vikash Garodia <quic_vgarodia@quicinc.com> >> + >> +description: >> + The Venus IP is a video encode and decode accelerator present >> + on Qualcomm platforms >> + >> +allOf: >> + - $ref: qcom,venus-common.yaml# >> + >> +properties: >> + compatible: >> + const: qcom,qcs615-venus > > Please extend sc7180-venus.yaml instead. No need to duplicate > unnecessary things. Exactly, we should reuse the sc7180 bindings for this. Regards, Vikash > >> + >> + power-domains: >> + minItems: 2 >> + maxItems: 3 > > So, is it 2 or 3? You don't have legacy here, so you should know an > exact number. > >> + >> + power-domain-names: >> + minItems: 2 > > And this one also can go away. > >> + items: >> + - const: venus >> + - const: vcodec0 >> + - const: cx >> + >> + clocks: >> + maxItems: 5 >> + >> + clock-names: >> + items: >> + - const: core >> + - const: iface >> + - const: bus >> + - const: vcodec0_core >> + - const: vcodec0_bus >> + >> + iommus: >> + maxItems: 1 >> + >> + memory-region: >> + maxItems: 1 >> + >> + interconnects: >> + maxItems: 2 >> + >> + interconnect-names: >> + items: >> + - const: video-mem >> + - const: cpu-cfg >> + >> + operating-points-v2: true >> + >> + opp-table: >> + type: object >> + >> + video-decoder: >> + type: object >> + >> + additionalProperties: false >> + >> + properties: >> + compatible: >> + const: venus-decoder >> + >> + required: >> + - compatible >> + >> + video-encoder: >> + type: object >> + >> + additionalProperties: false >> + >> + properties: >> + compatible: >> + const: venus-encoder >> + >> + required: >> + - compatible >> + >> +required: >> + - compatible >> + - power-domain-names >> + - iommus >> + - video-decoder >> + - video-encoder >> + >> +unevaluatedProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + #include <dt-bindings/clock/qcom,qcs615-videocc.h> >> + #include <dt-bindings/interconnect/qcom,qcs615-rpmh.h> >> + #include <dt-bindings/power/qcom,rpmhpd.h> >> + >> + venus: video-codec@aa00000 { >> + compatible = "qcom,qcs615-venus"; >> + reg = <0xaa00000 0x100000>; >> + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; >> + >> + clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>, >> + <&videocc VIDEO_CC_VENUS_AHB_CLK>, >> + <&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>, >> + <&videocc VIDEO_CC_VCODEC0_CORE_CLK>, >> + <&videocc VIDEO_CC_VCODEC0_AXI_CLK>; >> + clock-names = "core", >> + "iface", >> + "bus", >> + "vcodec0_core", >> + "vcodec0_bus"; >> + >> + power-domains = <&videocc VENUS_GDSC>, >> + <&videocc VCODEC0_GDSC>, >> + <&rpmhpd RPMHPD_CX>; >> + power-domain-names = "venus", >> + "vcodec0", >> + "cx"; >> + >> + operating-points-v2 = <&venus_opp_table>; >> + >> + interconnects = <&mmss_noc MASTER_VIDEO_P0 0 >> + &mc_virt SLAVE_EBI1 0>, >> + <&gem_noc MASTER_APPSS_PROC 0 >> + &config_noc SLAVE_VENUS_CFG 0>; >> + interconnect-names = "video-mem", >> + "cpu-cfg"; >> + >> + iommus = <&apps_smmu 0xe40 0x20>; >> + >> + memory-region = <&pil_video_mem>; >> + >> + video-decoder { >> + compatible = "venus-decoder"; >> + }; >> + >> + video-encoder { >> + compatible = "venus-encoder"; >> + }; >> + >> + venus_opp_table: opp-table { >> + compatible = "operating-points-v2"; >> + >> + opp-133330000 { >> + opp-hz = /bits/ 64 <133330000>; >> + required-opps = <&rpmhpd_opp_low_svs>; >> + }; >> + >> + opp-240000000 { >> + opp-hz = /bits/ 64 <240000000>; >> + required-opps = <&rpmhpd_opp_svs>; >> + }; >> + >> + opp-300000000 { >> + opp-hz = /bits/ 64 <300000000>; >> + required-opps = <&rpmhpd_opp_svs_l1>; >> + }; >> + >> + opp-380000000 { >> + opp-hz = /bits/ 64 <380000000>; >> + required-opps = <&rpmhpd_opp_nom>; >> + }; >> + >> + opp-410000000 { >> + opp-hz = /bits/ 64 <410000000>; >> + required-opps = <&rpmhpd_opp_turbo>; >> + }; >> + >> + opp-460000000 { >> + opp-hz = /bits/ 64 <460000000>; >> + required-opps = <&rpmhpd_opp_turbo_l1>; >> + }; >> + }; >> + }; >> >> -- >> 2.34.1 >> >
diff --git a/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml new file mode 100644 index 0000000000000000000000000000000000000000..7a3a01ff06d8b62bc2424a0a24857c86c6865f89 --- /dev/null +++ b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml @@ -0,0 +1,182 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/qcom,qcs615-venus.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm QCS615 Venus video encode and decode accelerators + +maintainers: + - Stanimir Varbanov <stanimir.k.varbanov@gmail.com> + - Vikash Garodia <quic_vgarodia@quicinc.com> + +description: + The Venus IP is a video encode and decode accelerator present + on Qualcomm platforms + +allOf: + - $ref: qcom,venus-common.yaml# + +properties: + compatible: + const: qcom,qcs615-venus + + power-domains: + minItems: 2 + maxItems: 3 + + power-domain-names: + minItems: 2 + items: + - const: venus + - const: vcodec0 + - const: cx + + clocks: + maxItems: 5 + + clock-names: + items: + - const: core + - const: iface + - const: bus + - const: vcodec0_core + - const: vcodec0_bus + + iommus: + maxItems: 1 + + memory-region: + maxItems: 1 + + interconnects: + maxItems: 2 + + interconnect-names: + items: + - const: video-mem + - const: cpu-cfg + + operating-points-v2: true + + opp-table: + type: object + + video-decoder: + type: object + + additionalProperties: false + + properties: + compatible: + const: venus-decoder + + required: + - compatible + + video-encoder: + type: object + + additionalProperties: false + + properties: + compatible: + const: venus-encoder + + required: + - compatible + +required: + - compatible + - power-domain-names + - iommus + - video-decoder + - video-encoder + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/qcom,qcs615-videocc.h> + #include <dt-bindings/interconnect/qcom,qcs615-rpmh.h> + #include <dt-bindings/power/qcom,rpmhpd.h> + + venus: video-codec@aa00000 { + compatible = "qcom,qcs615-venus"; + reg = <0xaa00000 0x100000>; + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; + + clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>, + <&videocc VIDEO_CC_VENUS_AHB_CLK>, + <&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>, + <&videocc VIDEO_CC_VCODEC0_CORE_CLK>, + <&videocc VIDEO_CC_VCODEC0_AXI_CLK>; + clock-names = "core", + "iface", + "bus", + "vcodec0_core", + "vcodec0_bus"; + + power-domains = <&videocc VENUS_GDSC>, + <&videocc VCODEC0_GDSC>, + <&rpmhpd RPMHPD_CX>; + power-domain-names = "venus", + "vcodec0", + "cx"; + + operating-points-v2 = <&venus_opp_table>; + + interconnects = <&mmss_noc MASTER_VIDEO_P0 0 + &mc_virt SLAVE_EBI1 0>, + <&gem_noc MASTER_APPSS_PROC 0 + &config_noc SLAVE_VENUS_CFG 0>; + interconnect-names = "video-mem", + "cpu-cfg"; + + iommus = <&apps_smmu 0xe40 0x20>; + + memory-region = <&pil_video_mem>; + + video-decoder { + compatible = "venus-decoder"; + }; + + video-encoder { + compatible = "venus-encoder"; + }; + + venus_opp_table: opp-table { + compatible = "operating-points-v2"; + + opp-133330000 { + opp-hz = /bits/ 64 <133330000>; + required-opps = <&rpmhpd_opp_low_svs>; + }; + + opp-240000000 { + opp-hz = /bits/ 64 <240000000>; + required-opps = <&rpmhpd_opp_svs>; + }; + + opp-300000000 { + opp-hz = /bits/ 64 <300000000>; + required-opps = <&rpmhpd_opp_svs_l1>; + }; + + opp-380000000 { + opp-hz = /bits/ 64 <380000000>; + required-opps = <&rpmhpd_opp_nom>; + }; + + opp-410000000 { + opp-hz = /bits/ 64 <410000000>; + required-opps = <&rpmhpd_opp_turbo>; + }; + + opp-460000000 { + opp-hz = /bits/ 64 <460000000>; + required-opps = <&rpmhpd_opp_turbo_l1>; + }; + }; + };
Add support for Qualcomm video acceleration hardware used for video stream decoding and encoding on QCOM QCS615. Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> --- .../bindings/media/qcom,qcs615-venus.yaml | 182 +++++++++++++++++++++ 1 file changed, 182 insertions(+)