| Message ID | 0843621b-386b-4173-9e3c-9538cdb4641d@freebox.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,WIP] venus: add qcom,no-low-power property | expand |
On 19.02.2024 18:18, Marc Gonzalez wrote: > From: Pierre-Hugues Husson <phhusson@freebox.fr> > > On our msm8998-based device, calling venus_sys_set_power_control() > breaks playback. Since the vendor kernel never calls it, we assume > it should not be called for this device/FW combo. > > Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr> > Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> > --- FWIW, this is also broken on other SoCs.. 8280/8350 and 6115 to name a couple. Konrad
On Mon, 19 Feb 2024 at 19:29, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > On 19.02.2024 18:18, Marc Gonzalez wrote: > > From: Pierre-Hugues Husson <phhusson@freebox.fr> > > > > On our msm8998-based device, calling venus_sys_set_power_control() > > breaks playback. Since the vendor kernel never calls it, we assume > > it should not be called for this device/FW combo. > > > > Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr> > > Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> > > --- > > FWIW, this is also broken on other SoCs.. 8280/8350 and 6115 > to name a couple. Then let's just disable it until it gets unbroken?
On Mon, 19 Feb 2024 18:18:55 +0100, Marc Gonzalez wrote: > From: Pierre-Hugues Husson <phhusson@freebox.fr> > > On our msm8998-based device, calling venus_sys_set_power_control() > breaks playback. Since the vendor kernel never calls it, we assume > it should not be called for this device/FW combo. > > Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr> > Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> > --- > TODO in v2: split the patch in 2 > Is "qcom,no-low-power" a proper name for the property? > Is a boolean property the right approach? > --- > .../devicetree/bindings/media/qcom,venus-common.yaml | 3 +++ > drivers/media/platform/qcom/venus/hfi_venus.c | 9 +++++---- > 2 files changed, 8 insertions(+), 4 deletions(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,venus-common.yaml: properties:qcom,no-low-power: 'description' is a dependency of 'type' hint: Vendor specific properties must have a type and description unless they have a defined, common suffix. from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/0843621b-386b-4173-9e3c-9538cdb4641d@freebox.fr 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 19/02/2024 5:44 p.m., Dmitry Baryshkov wrote: > On Mon, 19 Feb 2024 at 19:29, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> >> On 19.02.2024 18:18, Marc Gonzalez wrote: >>> From: Pierre-Hugues Husson <phhusson@freebox.fr> >>> >>> On our msm8998-based device, calling venus_sys_set_power_control() >>> breaks playback. Since the vendor kernel never calls it, we assume >>> it should not be called for this device/FW combo. >>> >>> Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr> >>> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> >>> --- >> >> FWIW, this is also broken on other SoCs.. 8280/8350 and 6115 >> to name a couple. > > Then let's just disable it until it gets unbroken? Its functional on most of our upstream stuff though, why switch if off unless necessary ? Maybe it should be an opt-in instead of an opt-out, TBH my own feeling is its better to minimize the amount of work and opt as per the proposed patch. Perhaps the qcom vidc team can give insights on 8280xp and 8350 when we come to tackling new HFI6XX and later SoCs ... --- bod
On 19/02/2024 20:24, Bryan O'Donoghue wrote: > On 19/02/2024 5:44 p.m., Dmitry Baryshkov wrote: > >> On Mon, 19 Feb 2024 at 19:29, Konrad Dybcio wrote: >>> >>> On 19.02.2024 18:18, Marc Gonzalez wrote: >>> >>>> On our msm8998-based device, calling venus_sys_set_power_control() >>>> breaks playback. Since the vendor kernel never calls it, we assume >>>> it should not be called for this device/FW combo. >>> >>> FWIW, this is also broken on other SoCs.. 8280/8350 and 6115 >>> to name a couple. >> >> Then let's just disable it until it gets unbroken? > > Its functional on most of our upstream stuff though, why switch if off > unless necessary ? > > Maybe it should be an opt-in instead of an opt-out, TBH my own feeling > is its better to minimize the amount of work and opt as per the proposed > patch. > > Perhaps the qcom vidc team can give insights on 8280xp and 8350 when we > come to tackling new HFI6XX and later SoCs ... I was wondering if the chosen property name might cause issues later... Thinking "qcom,no-low-power" might be a bit too general? Perhaps would need to mention venus somewhere in the name, to limit this to the video decoder? Regards
On 20/02/2024 10:56 a.m., Marc Gonzalez wrote: > On 19/02/2024 20:24, Bryan O'Donoghue wrote: > >> On 19/02/2024 5:44 p.m., Dmitry Baryshkov wrote: >> >>> On Mon, 19 Feb 2024 at 19:29, Konrad Dybcio wrote: >>>> >>>> On 19.02.2024 18:18, Marc Gonzalez wrote: >>>> >>>>> On our msm8998-based device, calling venus_sys_set_power_control() >>>>> breaks playback. Since the vendor kernel never calls it, we assume >>>>> it should not be called for this device/FW combo. >>>> >>>> FWIW, this is also broken on other SoCs.. 8280/8350 and 6115 >>>> to name a couple. >>> >>> Then let's just disable it until it gets unbroken? >> >> Its functional on most of our upstream stuff though, why switch if off >> unless necessary ? >> >> Maybe it should be an opt-in instead of an opt-out, TBH my own feeling >> is its better to minimize the amount of work and opt as per the proposed >> patch. >> >> Perhaps the qcom vidc team can give insights on 8280xp and 8350 when we >> come to tackling new HFI6XX and later SoCs ... > > I was wondering if the chosen property name might cause issues later... > > Thinking "qcom,no-low-power" might be a bit too general? > Perhaps would need to mention venus somewhere in the name, > to limit this to the video decoder? > > Regards > Yep, the word venus should probably appear in the property name. --- bod
On 20/02/2024 12:21, Bryan O'Donoghue wrote: > On 20/02/2024 10:56 a.m., Marc Gonzalez wrote: >> On 19/02/2024 20:24, Bryan O'Donoghue wrote: >> >>> On 19/02/2024 5:44 p.m., Dmitry Baryshkov wrote: >>> >>>> On Mon, 19 Feb 2024 at 19:29, Konrad Dybcio wrote: >>>>> >>>>> On 19.02.2024 18:18, Marc Gonzalez wrote: >>>>> >>>>>> On our msm8998-based device, calling venus_sys_set_power_control() >>>>>> breaks playback. Since the vendor kernel never calls it, we assume >>>>>> it should not be called for this device/FW combo. >>>>> >>>>> FWIW, this is also broken on other SoCs.. 8280/8350 and 6115 >>>>> to name a couple. >>>> >>>> Then let's just disable it until it gets unbroken? >>> >>> Its functional on most of our upstream stuff though, why switch if off >>> unless necessary ? >>> >>> Maybe it should be an opt-in instead of an opt-out, TBH my own feeling >>> is its better to minimize the amount of work and opt as per the proposed >>> patch. >>> >>> Perhaps the qcom vidc team can give insights on 8280xp and 8350 when we >>> come to tackling new HFI6XX and later SoCs ... >> >> I was wondering if the chosen property name might cause issues later... >> >> Thinking "qcom,no-low-power" might be a bit too general? >> Perhaps would need to mention venus somewhere in the name, >> to limit this to the video decoder? >> >> Regards >> > > Yep, the word venus should probably appear in the property name. This is RFC, so I am ignoring it, but just in case before you send v2 with the same: You described the desired Linux feature or behavior, not the actual hardware. The bindings are about the latter, so instead you need to rephrase the property and its description to match actual hardware capabilities/features/configuration etc. Best regards, Krzysztof
On 20/02/2024 12:37, Krzysztof Kozlowski wrote: > On 20/02/2024 12:21, Bryan O'Donoghue wrote: > >> On 20/02/2024 10:56 a.m., Marc Gonzalez wrote: >> >>> On 19/02/2024 20:24, Bryan O'Donoghue wrote: >>> >>>> On 19/02/2024 5:44 p.m., Dmitry Baryshkov wrote: >>>> >>>>> On Mon, 19 Feb 2024 at 19:29, Konrad Dybcio wrote: >>>>>> >>>>>> On 19.02.2024 18:18, Marc Gonzalez wrote: >>>>>> >>>>>>> On our msm8998-based device, calling venus_sys_set_power_control() >>>>>>> breaks playback. Since the vendor kernel never calls it, we assume >>>>>>> it should not be called for this device/FW combo. >>>>>> >>>>>> FWIW, this is also broken on other SoCs.. 8280/8350 and 6115 >>>>>> to name a couple. >>>>> >>>>> Then let's just disable it until it gets unbroken? >>>> >>>> Its functional on most of our upstream stuff though, why switch if off >>>> unless necessary ? >>>> >>>> Maybe it should be an opt-in instead of an opt-out, TBH my own feeling >>>> is its better to minimize the amount of work and opt as per the proposed >>>> patch. >>>> >>>> Perhaps the qcom vidc team can give insights on 8280xp and 8350 when we >>>> come to tackling new HFI6XX and later SoCs ... >>> >>> I was wondering if the chosen property name might cause issues later... >>> >>> Thinking "qcom,no-low-power" might be a bit too general? >>> Perhaps would need to mention venus somewhere in the name, >>> to limit this to the video decoder? >> >> Yep, the word venus should probably appear in the property name. > > This is RFC, so I am ignoring it, but just in case before you send v2 > with the same: > > You described the desired Linux feature or behavior, not the actual > hardware. The bindings are about the latter, so instead you need to > rephrase the property and its description to match actual hardware > capabilities/features/configuration etc. I added the RFC tag explicitly because I was hoping for the DT folks and msm maintainers to comment on the property name ;) Thanks for your comment! Here's the proposal for v2: qcom,venus-broken-low-power-mode Description: This property is defined for devices where playback does not work when the video decoder is in low power mode. Regards
On 20/02/2024 13:34, Marc Gonzalez wrote: > On 20/02/2024 12:37, Krzysztof Kozlowski wrote: > >> On 20/02/2024 12:21, Bryan O'Donoghue wrote: >> >>> On 20/02/2024 10:56 a.m., Marc Gonzalez wrote: >>> >>>> On 19/02/2024 20:24, Bryan O'Donoghue wrote: >>>> >>>>> On 19/02/2024 5:44 p.m., Dmitry Baryshkov wrote: >>>>> >>>>>> On Mon, 19 Feb 2024 at 19:29, Konrad Dybcio wrote: >>>>>>> >>>>>>> On 19.02.2024 18:18, Marc Gonzalez wrote: >>>>>>> >>>>>>>> On our msm8998-based device, calling venus_sys_set_power_control() >>>>>>>> breaks playback. Since the vendor kernel never calls it, we assume >>>>>>>> it should not be called for this device/FW combo. >>>>>>> >>>>>>> FWIW, this is also broken on other SoCs.. 8280/8350 and 6115 >>>>>>> to name a couple. >>>>>> >>>>>> Then let's just disable it until it gets unbroken? >>>>> >>>>> Its functional on most of our upstream stuff though, why switch if off >>>>> unless necessary ? >>>>> >>>>> Maybe it should be an opt-in instead of an opt-out, TBH my own feeling >>>>> is its better to minimize the amount of work and opt as per the proposed >>>>> patch. >>>>> >>>>> Perhaps the qcom vidc team can give insights on 8280xp and 8350 when we >>>>> come to tackling new HFI6XX and later SoCs ... >>>> >>>> I was wondering if the chosen property name might cause issues later... >>>> >>>> Thinking "qcom,no-low-power" might be a bit too general? >>>> Perhaps would need to mention venus somewhere in the name, >>>> to limit this to the video decoder? >>> >>> Yep, the word venus should probably appear in the property name. >> >> This is RFC, so I am ignoring it, but just in case before you send v2 >> with the same: >> >> You described the desired Linux feature or behavior, not the actual >> hardware. The bindings are about the latter, so instead you need to >> rephrase the property and its description to match actual hardware >> capabilities/features/configuration etc. > > I added the RFC tag explicitly because I was hoping for the DT folks > and msm maintainers to comment on the property name ;) And for the PATCH we would not comment? RFC means not ready and you gather opinion before doing more work. Some maintainers ignore entirely RFC patches. > > Thanks for your comment! > > Here's the proposal for v2: > > qcom,venus-broken-low-power-mode > > Description: > This property is defined for devices where playback does not work > when the video decoder is in low power mode. Would be nice to know what's broken but if that's tricky to get, then sounds fine. Best regards, Krzysztof
Hi, On 2/20/2024 6:57 PM, Krzysztof Kozlowski wrote: > On 20/02/2024 13:34, Marc Gonzalez wrote: >> On 20/02/2024 12:37, Krzysztof Kozlowski wrote: >> >>> On 20/02/2024 12:21, Bryan O'Donoghue wrote: >>> >>>> On 20/02/2024 10:56 a.m., Marc Gonzalez wrote: >>>> >>>>> On 19/02/2024 20:24, Bryan O'Donoghue wrote: >>>>> >>>>>> On 19/02/2024 5:44 p.m., Dmitry Baryshkov wrote: >>>>>> >>>>>>> On Mon, 19 Feb 2024 at 19:29, Konrad Dybcio wrote: >>>>>>>> >>>>>>>> On 19.02.2024 18:18, Marc Gonzalez wrote: >>>>>>>> >>>>>>>>> On our msm8998-based device, calling venus_sys_set_power_control() >>>>>>>>> breaks playback. Since the vendor kernel never calls it, we assume >>>>>>>>> it should not be called for this device/FW combo. >>>>>>>> >>>>>>>> FWIW, this is also broken on other SoCs.. 8280/8350 and 6115 >>>>>>>> to name a couple. >>>>>>> >>>>>>> Then let's just disable it until it gets unbroken? >>>>>> >>>>>> Its functional on most of our upstream stuff though, why switch if off >>>>>> unless necessary ? >>>>>> >>>>>> Maybe it should be an opt-in instead of an opt-out, TBH my own feeling >>>>>> is its better to minimize the amount of work and opt as per the proposed >>>>>> patch. >>>>>> >>>>>> Perhaps the qcom vidc team can give insights on 8280xp and 8350 when we >>>>>> come to tackling new HFI6XX and later SoCs ... >>>>> >>>>> I was wondering if the chosen property name might cause issues later... >>>>> >>>>> Thinking "qcom,no-low-power" might be a bit too general? >>>>> Perhaps would need to mention venus somewhere in the name, >>>>> to limit this to the video decoder? >>>> >>>> Yep, the word venus should probably appear in the property name. >>> >>> This is RFC, so I am ignoring it, but just in case before you send v2 >>> with the same: >>> >>> You described the desired Linux feature or behavior, not the actual >>> hardware. The bindings are about the latter, so instead you need to >>> rephrase the property and its description to match actual hardware >>> capabilities/features/configuration etc. >> >> I added the RFC tag explicitly because I was hoping for the DT folks >> and msm maintainers to comment on the property name ;) > > And for the PATCH we would not comment? RFC means not ready and you > gather opinion before doing more work. Some maintainers ignore entirely > RFC patches. > >> >> Thanks for your comment! >> >> Here's the proposal for v2: >> >> qcom,venus-broken-low-power-mode >> >> Description: >> This property is defined for devices where playback does not work >> when the video decoder is in low power mode. > > Would be nice to know what's broken but if that's tricky to get, then > sounds fine. msm8998 supports configuring the VCodec (venus core0) GDSC in HW power control mode. Could you please check and confirm if the driver is configuring only the VCodec GDSC and not the venus GDSC. Look for the attribute "qcom,support-hw-trigger" in vendor dt file. Regards, Vikash
On 20/02/2024 14:53, Vikash Garodia wrote: > On 2/20/2024 6:57 PM, Krzysztof Kozlowski wrote: > >> On 20/02/2024 13:34, Marc Gonzalez wrote: >> >>> Here's the proposal for v2: >>> >>> qcom,venus-broken-low-power-mode >>> >>> Description: >>> This property is defined for devices where playback does not work >>> when the video decoder is in low power mode. >> >> Would be nice to know what's broken but if that's tricky to get, then >> sounds fine. > > msm8998 supports configuring the VCodec (venus core0) GDSC in HW power control > mode. Could you please check and confirm if the driver is configuring only the > VCodec GDSC and not the venus GDSC. Look for the attribute > "qcom,support-hw-trigger" in vendor dt file. [ Vendor DTS for easy reference: ] [ https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998.dtsi ] In the queue, we have a patch enabling the Venus Decoder (VDEC) in mainline. (It is using the previously proposed "qcom,no-low-power" mechanism, but that might not be necessary, if I understand correctly?) diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi index 2793cc22d381a..5084191be1446 100644 --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi @@ -3000,6 +3000,56 @@ mdss_dsi1_phy: phy@c996400 { }; }; + venus: video-codec@cc00000 { + compatible = "qcom,msm8998-venus"; + reg = <0x0cc00000 0xff000>; + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; + power-domains = <&mmcc VIDEO_TOP_GDSC>; + clocks = <&mmcc VIDEO_CORE_CLK>, + <&mmcc VIDEO_AHB_CLK>, + <&mmcc VIDEO_AXI_CLK>, + <&mmcc VIDEO_MAXI_CLK>; + clock-names = "core", "iface", "bus", "mbus"; + iommus = <&mmss_smmu 0x400>, + <&mmss_smmu 0x401>, + <&mmss_smmu 0x40a>, + <&mmss_smmu 0x407>, + <&mmss_smmu 0x40e>, + <&mmss_smmu 0x40f>, + <&mmss_smmu 0x408>, + <&mmss_smmu 0x409>, + <&mmss_smmu 0x40b>, + <&mmss_smmu 0x40c>, + <&mmss_smmu 0x40d>, + <&mmss_smmu 0x410>, + <&mmss_smmu 0x411>, + <&mmss_smmu 0x421>, + <&mmss_smmu 0x428>, + <&mmss_smmu 0x429>, + <&mmss_smmu 0x42b>, + <&mmss_smmu 0x42c>, + <&mmss_smmu 0x42d>, + <&mmss_smmu 0x411>, + <&mmss_smmu 0x431>; + memory-region = <&venus_mem>; + status = "disabled"; + qcom,no-low-power; /*** WORK AROUND LOW-POWER ISSUE ***/ + + video-decoder { + compatible = "venus-decoder"; + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; + clock-names = "core"; + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; + }; + + video-encoder { + compatible = "venus-encoder"; + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; + clock-names = "core"; + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; + }; + }; + mmss_smmu: iommu@cd00000 { compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; reg = <0x0cd00000 0x40000>; diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index a712dd4f02a5b..ad1705e510312 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -585,6 +585,43 @@ static const struct venus_resources msm8996_res = { .fwname = "qcom/venus-4.2/venus.mbn", }; +static const struct freq_tbl msm8998_freq_table[] = { + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ + { 972000, 520000000 }, /* 4k UHD @ 30 */ + { 489600, 346666667 }, /* 1080p @ 60 */ + { 244800, 150000000 }, /* 1080p @ 30 */ + { 108000, 75000000 }, /* 720p @ 30 */ +}; + +static const struct reg_val msm8998_reg_preset[] = { + { 0x80124, 0x00000003 }, + { 0x80550, 0x01111111 }, + { 0x80560, 0x01111111 }, + { 0x80568, 0x01111111 }, + { 0x80570, 0x01111111 }, + { 0x80580, 0x01111111 }, + { 0xe2010, 0x00000000 }, +}; + +static const struct venus_resources msm8998_res = { + .freq_tbl = msm8998_freq_table, + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), + .reg_tbl = msm8998_reg_preset, + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), + .clks = {"core", "iface", "bus", "mbus"}, + .clks_num = 4, + .vcodec0_clks = { "core" }, + .vcodec1_clks = { "core" }, + .vcodec_clks_num = 1, + .max_load = 2563200, + .hfi_version = HFI_VERSION_3XX, + .vmem_id = VIDC_RESOURCE_NONE, + .vmem_size = 0, + .vmem_addr = 0, + .dma_mask = 0xddc00000 - 1, + .fwname = "qcom/venus-4.4/venus.mbn", +}; + static const struct freq_tbl sdm660_freq_table[] = { { 979200, 518400000 }, { 489600, 441600000 }, @@ -891,6 +928,7 @@ static const struct venus_resources sc7280_res = { static const struct of_device_id venus_dt_match[] = { { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, @Vikash, are you saying that perhaps the DTS for video-codec@cc00000 needs to be written slightly differently? Regards
On 2/20/2024 8:15 PM, Marc Gonzalez wrote: > On 20/02/2024 14:53, Vikash Garodia wrote: > >> On 2/20/2024 6:57 PM, Krzysztof Kozlowski wrote: >> >>> On 20/02/2024 13:34, Marc Gonzalez wrote: >>> >>>> Here's the proposal for v2: >>>> >>>> qcom,venus-broken-low-power-mode >>>> >>>> Description: >>>> This property is defined for devices where playback does not work >>>> when the video decoder is in low power mode. >>> >>> Would be nice to know what's broken but if that's tricky to get, then >>> sounds fine. >> >> msm8998 supports configuring the VCodec (venus core0) GDSC in HW power control >> mode. Could you please check and confirm if the driver is configuring only the >> VCodec GDSC and not the venus GDSC. Look for the attribute >> "qcom,support-hw-trigger" in vendor dt file. > > [ Vendor DTS for easy reference: ] > [ https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998.dtsi ] > > In the queue, we have a patch enabling the Venus Decoder (VDEC) in mainline. > (It is using the previously proposed "qcom,no-low-power" mechanism, but that > might not be necessary, if I understand correctly?) > > > diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi > index 2793cc22d381a..5084191be1446 100644 > --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi > @@ -3000,6 +3000,56 @@ mdss_dsi1_phy: phy@c996400 { > }; > }; > > + venus: video-codec@cc00000 { > + compatible = "qcom,msm8998-venus"; > + reg = <0x0cc00000 0xff000>; > + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; > + power-domains = <&mmcc VIDEO_TOP_GDSC>; > + clocks = <&mmcc VIDEO_CORE_CLK>, > + <&mmcc VIDEO_AHB_CLK>, > + <&mmcc VIDEO_AXI_CLK>, > + <&mmcc VIDEO_MAXI_CLK>; > + clock-names = "core", "iface", "bus", "mbus"; > + iommus = <&mmss_smmu 0x400>, > + <&mmss_smmu 0x401>, > + <&mmss_smmu 0x40a>, > + <&mmss_smmu 0x407>, > + <&mmss_smmu 0x40e>, > + <&mmss_smmu 0x40f>, > + <&mmss_smmu 0x408>, > + <&mmss_smmu 0x409>, > + <&mmss_smmu 0x40b>, > + <&mmss_smmu 0x40c>, > + <&mmss_smmu 0x40d>, > + <&mmss_smmu 0x410>, > + <&mmss_smmu 0x411>, > + <&mmss_smmu 0x421>, > + <&mmss_smmu 0x428>, > + <&mmss_smmu 0x429>, > + <&mmss_smmu 0x42b>, > + <&mmss_smmu 0x42c>, > + <&mmss_smmu 0x42d>, > + <&mmss_smmu 0x411>, > + <&mmss_smmu 0x431>; > + memory-region = <&venus_mem>; > + status = "disabled"; > + qcom,no-low-power; /*** WORK AROUND LOW-POWER ISSUE ***/ > + > + video-decoder { > + compatible = "venus-decoder"; > + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; > + clock-names = "core"; > + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; > + }; > + > + video-encoder { > + compatible = "venus-encoder"; > + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; > + clock-names = "core"; > + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; > + }; > + }; > + > mmss_smmu: iommu@cd00000 { > compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; > reg = <0x0cd00000 0x40000>; > diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c > index a712dd4f02a5b..ad1705e510312 100644 > --- a/drivers/media/platform/qcom/venus/core.c > +++ b/drivers/media/platform/qcom/venus/core.c > @@ -585,6 +585,43 @@ static const struct venus_resources msm8996_res = { > .fwname = "qcom/venus-4.2/venus.mbn", > }; > > +static const struct freq_tbl msm8998_freq_table[] = { > + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ > + { 972000, 520000000 }, /* 4k UHD @ 30 */ > + { 489600, 346666667 }, /* 1080p @ 60 */ > + { 244800, 150000000 }, /* 1080p @ 30 */ > + { 108000, 75000000 }, /* 720p @ 30 */ > +}; > + > +static const struct reg_val msm8998_reg_preset[] = { > + { 0x80124, 0x00000003 }, > + { 0x80550, 0x01111111 }, > + { 0x80560, 0x01111111 }, > + { 0x80568, 0x01111111 }, > + { 0x80570, 0x01111111 }, > + { 0x80580, 0x01111111 }, > + { 0xe2010, 0x00000000 }, > +}; > + > +static const struct venus_resources msm8998_res = { > + .freq_tbl = msm8998_freq_table, > + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), > + .reg_tbl = msm8998_reg_preset, > + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), > + .clks = {"core", "iface", "bus", "mbus"}, > + .clks_num = 4, > + .vcodec0_clks = { "core" }, > + .vcodec1_clks = { "core" }, > + .vcodec_clks_num = 1, > + .max_load = 2563200, > + .hfi_version = HFI_VERSION_3XX, > + .vmem_id = VIDC_RESOURCE_NONE, > + .vmem_size = 0, > + .vmem_addr = 0, > + .dma_mask = 0xddc00000 - 1, > + .fwname = "qcom/venus-4.4/venus.mbn", > +}; > + > static const struct freq_tbl sdm660_freq_table[] = { > { 979200, 518400000 }, > { 489600, 441600000 }, > @@ -891,6 +928,7 @@ static const struct venus_resources sc7280_res = { > static const struct of_device_id venus_dt_match[] = { > { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, > { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, > + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, > { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, > { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, > { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, > > > > @Vikash, are you saying that perhaps the DTS for video-codec@cc00000 > needs to be written slightly differently? Certainly yes. For ex, in the clock list, i do not see the core clocks listed i.e clk_mmss_video_subcore0_clk and clk_mmss_video_subcore1_clk. You can refer the downstream video DT node [1] and then align it as per venus driver [1] https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-vidc.dtsi Regards, Vikash
On 23/02/2024 14:48, Vikash Garodia wrote: > On 2/20/2024 8:15 PM, Marc Gonzalez wrote: > >> On 20/02/2024 14:53, Vikash Garodia wrote: >> >>> msm8998 supports configuring the VCodec (venus core0) GDSC in HW power control >>> mode. Could you please check and confirm if the driver is configuring only the >>> VCodec GDSC and not the venus GDSC. Look for the attribute >>> "qcom,support-hw-trigger" in vendor dt file. >> >> [ Vendor DTS for easy reference: ] >> [ https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998.dtsi ] >> >> In the queue, we have a patch enabling the Venus Decoder (VDEC) in mainline. >> (It is using the previously proposed "qcom,no-low-power" mechanism, but that >> might not be necessary, if I understand correctly?) >> >> >> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi >> index 2793cc22d381a..5084191be1446 100644 >> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi >> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi >> @@ -3000,6 +3000,56 @@ mdss_dsi1_phy: phy@c996400 { >> }; >> }; >> >> + venus: video-codec@cc00000 { >> + compatible = "qcom,msm8998-venus"; >> + reg = <0x0cc00000 0xff000>; >> + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; >> + power-domains = <&mmcc VIDEO_TOP_GDSC>; >> + clocks = <&mmcc VIDEO_CORE_CLK>, >> + <&mmcc VIDEO_AHB_CLK>, >> + <&mmcc VIDEO_AXI_CLK>, >> + <&mmcc VIDEO_MAXI_CLK>; >> + clock-names = "core", "iface", "bus", "mbus"; >> + iommus = <&mmss_smmu 0x400>, >> + <&mmss_smmu 0x401>, >> + <&mmss_smmu 0x40a>, >> + <&mmss_smmu 0x407>, >> + <&mmss_smmu 0x40e>, >> + <&mmss_smmu 0x40f>, >> + <&mmss_smmu 0x408>, >> + <&mmss_smmu 0x409>, >> + <&mmss_smmu 0x40b>, >> + <&mmss_smmu 0x40c>, >> + <&mmss_smmu 0x40d>, >> + <&mmss_smmu 0x410>, >> + <&mmss_smmu 0x411>, >> + <&mmss_smmu 0x421>, >> + <&mmss_smmu 0x428>, >> + <&mmss_smmu 0x429>, >> + <&mmss_smmu 0x42b>, >> + <&mmss_smmu 0x42c>, >> + <&mmss_smmu 0x42d>, >> + <&mmss_smmu 0x411>, >> + <&mmss_smmu 0x431>; >> + memory-region = <&venus_mem>; >> + status = "disabled"; >> + qcom,no-low-power; /*** WORK AROUND LOW-POWER ISSUE ***/ >> + >> + video-decoder { >> + compatible = "venus-decoder"; >> + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; >> + clock-names = "core"; >> + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; >> + }; >> + >> + video-encoder { >> + compatible = "venus-encoder"; >> + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; >> + clock-names = "core"; >> + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; >> + }; >> + }; >> + >> mmss_smmu: iommu@cd00000 { >> compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; >> reg = <0x0cd00000 0x40000>; >> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c >> index a712dd4f02a5b..ad1705e510312 100644 >> --- a/drivers/media/platform/qcom/venus/core.c >> +++ b/drivers/media/platform/qcom/venus/core.c >> @@ -585,6 +585,43 @@ static const struct venus_resources msm8996_res = { >> .fwname = "qcom/venus-4.2/venus.mbn", >> }; >> >> +static const struct freq_tbl msm8998_freq_table[] = { >> + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ >> + { 972000, 520000000 }, /* 4k UHD @ 30 */ >> + { 489600, 346666667 }, /* 1080p @ 60 */ >> + { 244800, 150000000 }, /* 1080p @ 30 */ >> + { 108000, 75000000 }, /* 720p @ 30 */ >> +}; >> + >> +static const struct reg_val msm8998_reg_preset[] = { >> + { 0x80124, 0x00000003 }, >> + { 0x80550, 0x01111111 }, >> + { 0x80560, 0x01111111 }, >> + { 0x80568, 0x01111111 }, >> + { 0x80570, 0x01111111 }, >> + { 0x80580, 0x01111111 }, >> + { 0xe2010, 0x00000000 }, >> +}; >> + >> +static const struct venus_resources msm8998_res = { >> + .freq_tbl = msm8998_freq_table, >> + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), >> + .reg_tbl = msm8998_reg_preset, >> + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), >> + .clks = {"core", "iface", "bus", "mbus"}, >> + .clks_num = 4, >> + .vcodec0_clks = { "core" }, >> + .vcodec1_clks = { "core" }, >> + .vcodec_clks_num = 1, >> + .max_load = 2563200, >> + .hfi_version = HFI_VERSION_3XX, >> + .vmem_id = VIDC_RESOURCE_NONE, >> + .vmem_size = 0, >> + .vmem_addr = 0, >> + .dma_mask = 0xddc00000 - 1, >> + .fwname = "qcom/venus-4.4/venus.mbn", >> +}; >> + >> static const struct freq_tbl sdm660_freq_table[] = { >> { 979200, 518400000 }, >> { 489600, 441600000 }, >> @@ -891,6 +928,7 @@ static const struct venus_resources sc7280_res = { >> static const struct of_device_id venus_dt_match[] = { >> { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, >> { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, >> + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, >> { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, >> { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, >> { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, >> >> >> >> @Vikash, are you saying that perhaps the DTS for video-codec@cc00000 >> needs to be written slightly differently? > > > Certainly yes. For ex, in the clock list, i do not see the core clocks listed > i.e clk_mmss_video_subcore0_clk and clk_mmss_video_subcore1_clk. You can refer > the downstream video DT node [1] and then align it as per venus driver > [1] > https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-vidc.dtsi If I understand correctly (which is far from certain), we should base the "qcom,msm8998-venus" DT node on "qcom,sdm845-venus-v2" rather than "qcom,msm8996-venus" ? Regards
On 2/26/2024 6:39 PM, Marc Gonzalez wrote: > On 23/02/2024 14:48, Vikash Garodia wrote: > >> On 2/20/2024 8:15 PM, Marc Gonzalez wrote: >> >>> On 20/02/2024 14:53, Vikash Garodia wrote: >>> >>>> msm8998 supports configuring the VCodec (venus core0) GDSC in HW power control >>>> mode. Could you please check and confirm if the driver is configuring only the >>>> VCodec GDSC and not the venus GDSC. Look for the attribute >>>> "qcom,support-hw-trigger" in vendor dt file. >>> >>> [ Vendor DTS for easy reference: ] >>> [ https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998.dtsi ] >>> >>> In the queue, we have a patch enabling the Venus Decoder (VDEC) in mainline. >>> (It is using the previously proposed "qcom,no-low-power" mechanism, but that >>> might not be necessary, if I understand correctly?) >>> >>> >>> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi >>> index 2793cc22d381a..5084191be1446 100644 >>> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi >>> @@ -3000,6 +3000,56 @@ mdss_dsi1_phy: phy@c996400 { >>> }; >>> }; >>> >>> + venus: video-codec@cc00000 { >>> + compatible = "qcom,msm8998-venus"; >>> + reg = <0x0cc00000 0xff000>; >>> + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; >>> + power-domains = <&mmcc VIDEO_TOP_GDSC>; >>> + clocks = <&mmcc VIDEO_CORE_CLK>, >>> + <&mmcc VIDEO_AHB_CLK>, >>> + <&mmcc VIDEO_AXI_CLK>, >>> + <&mmcc VIDEO_MAXI_CLK>; >>> + clock-names = "core", "iface", "bus", "mbus"; >>> + iommus = <&mmss_smmu 0x400>, >>> + <&mmss_smmu 0x401>, >>> + <&mmss_smmu 0x40a>, >>> + <&mmss_smmu 0x407>, >>> + <&mmss_smmu 0x40e>, >>> + <&mmss_smmu 0x40f>, >>> + <&mmss_smmu 0x408>, >>> + <&mmss_smmu 0x409>, >>> + <&mmss_smmu 0x40b>, >>> + <&mmss_smmu 0x40c>, >>> + <&mmss_smmu 0x40d>, >>> + <&mmss_smmu 0x410>, >>> + <&mmss_smmu 0x411>, >>> + <&mmss_smmu 0x421>, >>> + <&mmss_smmu 0x428>, >>> + <&mmss_smmu 0x429>, >>> + <&mmss_smmu 0x42b>, >>> + <&mmss_smmu 0x42c>, >>> + <&mmss_smmu 0x42d>, >>> + <&mmss_smmu 0x411>, >>> + <&mmss_smmu 0x431>; >>> + memory-region = <&venus_mem>; >>> + status = "disabled"; >>> + qcom,no-low-power; /*** WORK AROUND LOW-POWER ISSUE ***/ >>> + >>> + video-decoder { >>> + compatible = "venus-decoder"; >>> + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; >>> + clock-names = "core"; >>> + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; >>> + }; >>> + >>> + video-encoder { >>> + compatible = "venus-encoder"; >>> + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; >>> + clock-names = "core"; >>> + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; >>> + }; >>> + }; >>> + >>> mmss_smmu: iommu@cd00000 { >>> compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; >>> reg = <0x0cd00000 0x40000>; >>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c >>> index a712dd4f02a5b..ad1705e510312 100644 >>> --- a/drivers/media/platform/qcom/venus/core.c >>> +++ b/drivers/media/platform/qcom/venus/core.c >>> @@ -585,6 +585,43 @@ static const struct venus_resources msm8996_res = { >>> .fwname = "qcom/venus-4.2/venus.mbn", >>> }; >>> >>> +static const struct freq_tbl msm8998_freq_table[] = { >>> + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ >>> + { 972000, 520000000 }, /* 4k UHD @ 30 */ >>> + { 489600, 346666667 }, /* 1080p @ 60 */ >>> + { 244800, 150000000 }, /* 1080p @ 30 */ >>> + { 108000, 75000000 }, /* 720p @ 30 */ >>> +}; >>> + >>> +static const struct reg_val msm8998_reg_preset[] = { >>> + { 0x80124, 0x00000003 }, >>> + { 0x80550, 0x01111111 }, >>> + { 0x80560, 0x01111111 }, >>> + { 0x80568, 0x01111111 }, >>> + { 0x80570, 0x01111111 }, >>> + { 0x80580, 0x01111111 }, >>> + { 0xe2010, 0x00000000 }, >>> +}; >>> + >>> +static const struct venus_resources msm8998_res = { >>> + .freq_tbl = msm8998_freq_table, >>> + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), >>> + .reg_tbl = msm8998_reg_preset, >>> + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), >>> + .clks = {"core", "iface", "bus", "mbus"}, >>> + .clks_num = 4, >>> + .vcodec0_clks = { "core" }, >>> + .vcodec1_clks = { "core" }, >>> + .vcodec_clks_num = 1, >>> + .max_load = 2563200, >>> + .hfi_version = HFI_VERSION_3XX, >>> + .vmem_id = VIDC_RESOURCE_NONE, >>> + .vmem_size = 0, >>> + .vmem_addr = 0, >>> + .dma_mask = 0xddc00000 - 1, >>> + .fwname = "qcom/venus-4.4/venus.mbn", >>> +}; >>> + >>> static const struct freq_tbl sdm660_freq_table[] = { >>> { 979200, 518400000 }, >>> { 489600, 441600000 }, >>> @@ -891,6 +928,7 @@ static const struct venus_resources sc7280_res = { >>> static const struct of_device_id venus_dt_match[] = { >>> { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, >>> { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, >>> + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, >>> { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, >>> { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, >>> { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, >>> >>> >>> >>> @Vikash, are you saying that perhaps the DTS for video-codec@cc00000 >>> needs to be written slightly differently? >> >> >> Certainly yes. For ex, in the clock list, i do not see the core clocks listed >> i.e clk_mmss_video_subcore0_clk and clk_mmss_video_subcore1_clk. You can refer >> the downstream video DT node [1] and then align it as per venus driver >> [1] >> https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-vidc.dtsi > > > If I understand correctly (which is far from certain), > we should base the "qcom,msm8998-venus" DT node on > "qcom,sdm845-venus-v2" rather than "qcom,msm8996-venus" ? Thats correct, but thats just another way to do the configuration. With the existing node, is video decode as well as encode working ? Regards, Vikash
On 26/02/2024 15:30, Vikash Garodia wrote: > On 2/26/2024 6:39 PM, Marc Gonzalez wrote: > >> On 23/02/2024 14:48, Vikash Garodia wrote: >> >>> On 2/20/2024 8:15 PM, Marc Gonzalez wrote: >>> >>>> On 20/02/2024 14:53, Vikash Garodia wrote: >>>> >>>>> msm8998 supports configuring the VCodec (venus core0) GDSC in HW power control >>>>> mode. Could you please check and confirm if the driver is configuring only the >>>>> VCodec GDSC and not the venus GDSC. Look for the attribute >>>>> "qcom,support-hw-trigger" in vendor dt file. >>>> >>>> [ Vendor DTS for easy reference: ] >>>> [ https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998.dtsi ] >>>> >>>> In the queue, we have a patch enabling the Venus Decoder (VDEC) in mainline. >>>> (It is using the previously proposed "qcom,no-low-power" mechanism, but that >>>> might not be necessary, if I understand correctly?) >>>> >>>> >>>> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi >>>> index 2793cc22d381a..5084191be1446 100644 >>>> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi >>>> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi >>>> @@ -3000,6 +3000,56 @@ mdss_dsi1_phy: phy@c996400 { >>>> }; >>>> }; >>>> >>>> + venus: video-codec@cc00000 { >>>> + compatible = "qcom,msm8998-venus"; >>>> + reg = <0x0cc00000 0xff000>; >>>> + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; >>>> + power-domains = <&mmcc VIDEO_TOP_GDSC>; >>>> + clocks = <&mmcc VIDEO_CORE_CLK>, >>>> + <&mmcc VIDEO_AHB_CLK>, >>>> + <&mmcc VIDEO_AXI_CLK>, >>>> + <&mmcc VIDEO_MAXI_CLK>; >>>> + clock-names = "core", "iface", "bus", "mbus"; >>>> + iommus = <&mmss_smmu 0x400>, >>>> + <&mmss_smmu 0x401>, >>>> + <&mmss_smmu 0x40a>, >>>> + <&mmss_smmu 0x407>, >>>> + <&mmss_smmu 0x40e>, >>>> + <&mmss_smmu 0x40f>, >>>> + <&mmss_smmu 0x408>, >>>> + <&mmss_smmu 0x409>, >>>> + <&mmss_smmu 0x40b>, >>>> + <&mmss_smmu 0x40c>, >>>> + <&mmss_smmu 0x40d>, >>>> + <&mmss_smmu 0x410>, >>>> + <&mmss_smmu 0x411>, >>>> + <&mmss_smmu 0x421>, >>>> + <&mmss_smmu 0x428>, >>>> + <&mmss_smmu 0x429>, >>>> + <&mmss_smmu 0x42b>, >>>> + <&mmss_smmu 0x42c>, >>>> + <&mmss_smmu 0x42d>, >>>> + <&mmss_smmu 0x411>, >>>> + <&mmss_smmu 0x431>; >>>> + memory-region = <&venus_mem>; >>>> + status = "disabled"; >>>> + qcom,no-low-power; /*** WORK AROUND LOW-POWER ISSUE ***/ >>>> + >>>> + video-decoder { >>>> + compatible = "venus-decoder"; >>>> + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; >>>> + clock-names = "core"; >>>> + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; >>>> + }; >>>> + >>>> + video-encoder { >>>> + compatible = "venus-encoder"; >>>> + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; >>>> + clock-names = "core"; >>>> + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; >>>> + }; >>>> + }; >>>> + >>>> mmss_smmu: iommu@cd00000 { >>>> compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; >>>> reg = <0x0cd00000 0x40000>; >>>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c >>>> index a712dd4f02a5b..ad1705e510312 100644 >>>> --- a/drivers/media/platform/qcom/venus/core.c >>>> +++ b/drivers/media/platform/qcom/venus/core.c >>>> @@ -585,6 +585,43 @@ static const struct venus_resources msm8996_res = { >>>> .fwname = "qcom/venus-4.2/venus.mbn", >>>> }; >>>> >>>> +static const struct freq_tbl msm8998_freq_table[] = { >>>> + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ >>>> + { 972000, 520000000 }, /* 4k UHD @ 30 */ >>>> + { 489600, 346666667 }, /* 1080p @ 60 */ >>>> + { 244800, 150000000 }, /* 1080p @ 30 */ >>>> + { 108000, 75000000 }, /* 720p @ 30 */ >>>> +}; >>>> + >>>> +static const struct reg_val msm8998_reg_preset[] = { >>>> + { 0x80124, 0x00000003 }, >>>> + { 0x80550, 0x01111111 }, >>>> + { 0x80560, 0x01111111 }, >>>> + { 0x80568, 0x01111111 }, >>>> + { 0x80570, 0x01111111 }, >>>> + { 0x80580, 0x01111111 }, >>>> + { 0xe2010, 0x00000000 }, >>>> +}; >>>> + >>>> +static const struct venus_resources msm8998_res = { >>>> + .freq_tbl = msm8998_freq_table, >>>> + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), >>>> + .reg_tbl = msm8998_reg_preset, >>>> + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), >>>> + .clks = {"core", "iface", "bus", "mbus"}, >>>> + .clks_num = 4, >>>> + .vcodec0_clks = { "core" }, >>>> + .vcodec1_clks = { "core" }, >>>> + .vcodec_clks_num = 1, >>>> + .max_load = 2563200, >>>> + .hfi_version = HFI_VERSION_3XX, >>>> + .vmem_id = VIDC_RESOURCE_NONE, >>>> + .vmem_size = 0, >>>> + .vmem_addr = 0, >>>> + .dma_mask = 0xddc00000 - 1, >>>> + .fwname = "qcom/venus-4.4/venus.mbn", >>>> +}; >>>> + >>>> static const struct freq_tbl sdm660_freq_table[] = { >>>> { 979200, 518400000 }, >>>> { 489600, 441600000 }, >>>> @@ -891,6 +928,7 @@ static const struct venus_resources sc7280_res = { >>>> static const struct of_device_id venus_dt_match[] = { >>>> { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, >>>> { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, >>>> + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, >>>> { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, >>>> { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, >>>> { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, >>>> >>>> >>>> >>>> @Vikash, are you saying that perhaps the DTS for video-codec@cc00000 >>>> needs to be written slightly differently? >>> >>> >>> Certainly yes. For ex, in the clock list, i do not see the core clocks listed >>> i.e clk_mmss_video_subcore0_clk and clk_mmss_video_subcore1_clk. You can refer >>> the downstream video DT node [1] and then align it as per venus driver >>> [1] >>> https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-vidc.dtsi >> >> If I understand correctly (which is far from certain), >> we should base the "qcom,msm8998-venus" DT node on >> "qcom,sdm845-venus-v2" rather than "qcom,msm8996-venus" ? > > That's correct, but that's just another way to do the configuration. With the > existing node, is video decode as well as encode working ? Errr, there is currently no existing node for msm8998-venus? With the proposed node above (based on msm8996-venus) AND the proposed work-around disabling low-power mode, decoding works correctly. Encoding does not work, but it has never been used/tested on our device. [h264_v4l2m2m @ 0xaaaaec9c44a0] Using device /dev/video1 [h264_v4l2m2m @ 0xaaaaec9c44a0] driver 'qcom-venus' on card 'Qualcomm Venus video encoder' in mplane mode [h264_v4l2m2m @ 0xaaaaec9c44a0] requesting formats: output=NV12/yuv420p capture=H264/none [h264_v4l2m2m @ 0xaaaaec9c44a0] output VIDIOC_REQBUFS failed: Invalid argument [h264_v4l2m2m @ 0xaaaaec9c44a0] no v4l2 output context's buffers [h264_v4l2m2m @ 0xaaaaec9c44a0] can't configure encoder [vost#0:0/h264_v4l2m2m @ 0xaaaaec9c4160] Error while opening encoder - maybe incorrect parameters such as bit_rate, rate, width or height. Regards
On 2/26/2024 9:25 PM, Marc Gonzalez wrote: > On 26/02/2024 15:30, Vikash Garodia wrote: > >> On 2/26/2024 6:39 PM, Marc Gonzalez wrote: >> >>> On 23/02/2024 14:48, Vikash Garodia wrote: >>> >>>> On 2/20/2024 8:15 PM, Marc Gonzalez wrote: >>>> >>>>> On 20/02/2024 14:53, Vikash Garodia wrote: >>>>> >>>>>> msm8998 supports configuring the VCodec (venus core0) GDSC in HW power control >>>>>> mode. Could you please check and confirm if the driver is configuring only the >>>>>> VCodec GDSC and not the venus GDSC. Look for the attribute >>>>>> "qcom,support-hw-trigger" in vendor dt file. >>>>> >>>>> [ Vendor DTS for easy reference: ] >>>>> [ https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998.dtsi ] >>>>> >>>>> In the queue, we have a patch enabling the Venus Decoder (VDEC) in mainline. >>>>> (It is using the previously proposed "qcom,no-low-power" mechanism, but that >>>>> might not be necessary, if I understand correctly?) >>>>> >>>>> >>>>> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi >>>>> index 2793cc22d381a..5084191be1446 100644 >>>>> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi >>>>> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi >>>>> @@ -3000,6 +3000,56 @@ mdss_dsi1_phy: phy@c996400 { >>>>> }; >>>>> }; >>>>> >>>>> + venus: video-codec@cc00000 { >>>>> + compatible = "qcom,msm8998-venus"; >>>>> + reg = <0x0cc00000 0xff000>; >>>>> + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; >>>>> + power-domains = <&mmcc VIDEO_TOP_GDSC>; >>>>> + clocks = <&mmcc VIDEO_CORE_CLK>, >>>>> + <&mmcc VIDEO_AHB_CLK>, >>>>> + <&mmcc VIDEO_AXI_CLK>, >>>>> + <&mmcc VIDEO_MAXI_CLK>; >>>>> + clock-names = "core", "iface", "bus", "mbus"; >>>>> + iommus = <&mmss_smmu 0x400>, >>>>> + <&mmss_smmu 0x401>, >>>>> + <&mmss_smmu 0x40a>, >>>>> + <&mmss_smmu 0x407>, >>>>> + <&mmss_smmu 0x40e>, >>>>> + <&mmss_smmu 0x40f>, >>>>> + <&mmss_smmu 0x408>, >>>>> + <&mmss_smmu 0x409>, >>>>> + <&mmss_smmu 0x40b>, >>>>> + <&mmss_smmu 0x40c>, >>>>> + <&mmss_smmu 0x40d>, >>>>> + <&mmss_smmu 0x410>, >>>>> + <&mmss_smmu 0x411>, >>>>> + <&mmss_smmu 0x421>, >>>>> + <&mmss_smmu 0x428>, >>>>> + <&mmss_smmu 0x429>, >>>>> + <&mmss_smmu 0x42b>, >>>>> + <&mmss_smmu 0x42c>, >>>>> + <&mmss_smmu 0x42d>, >>>>> + <&mmss_smmu 0x411>, >>>>> + <&mmss_smmu 0x431>; >>>>> + memory-region = <&venus_mem>; >>>>> + status = "disabled"; >>>>> + qcom,no-low-power; /*** WORK AROUND LOW-POWER ISSUE ***/ >>>>> + >>>>> + video-decoder { >>>>> + compatible = "venus-decoder"; >>>>> + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; >>>>> + clock-names = "core"; >>>>> + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; >>>>> + }; >>>>> + >>>>> + video-encoder { >>>>> + compatible = "venus-encoder"; >>>>> + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; >>>>> + clock-names = "core"; >>>>> + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; >>>>> + }; >>>>> + }; >>>>> + >>>>> mmss_smmu: iommu@cd00000 { >>>>> compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; >>>>> reg = <0x0cd00000 0x40000>; >>>>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c >>>>> index a712dd4f02a5b..ad1705e510312 100644 >>>>> --- a/drivers/media/platform/qcom/venus/core.c >>>>> +++ b/drivers/media/platform/qcom/venus/core.c >>>>> @@ -585,6 +585,43 @@ static const struct venus_resources msm8996_res = { >>>>> .fwname = "qcom/venus-4.2/venus.mbn", >>>>> }; >>>>> >>>>> +static const struct freq_tbl msm8998_freq_table[] = { >>>>> + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ >>>>> + { 972000, 520000000 }, /* 4k UHD @ 30 */ >>>>> + { 489600, 346666667 }, /* 1080p @ 60 */ >>>>> + { 244800, 150000000 }, /* 1080p @ 30 */ >>>>> + { 108000, 75000000 }, /* 720p @ 30 */ >>>>> +}; >>>>> + >>>>> +static const struct reg_val msm8998_reg_preset[] = { >>>>> + { 0x80124, 0x00000003 }, >>>>> + { 0x80550, 0x01111111 }, >>>>> + { 0x80560, 0x01111111 }, >>>>> + { 0x80568, 0x01111111 }, >>>>> + { 0x80570, 0x01111111 }, >>>>> + { 0x80580, 0x01111111 }, >>>>> + { 0xe2010, 0x00000000 }, >>>>> +}; >>>>> + >>>>> +static const struct venus_resources msm8998_res = { >>>>> + .freq_tbl = msm8998_freq_table, >>>>> + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), >>>>> + .reg_tbl = msm8998_reg_preset, >>>>> + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), >>>>> + .clks = {"core", "iface", "bus", "mbus"}, >>>>> + .clks_num = 4, >>>>> + .vcodec0_clks = { "core" }, >>>>> + .vcodec1_clks = { "core" }, >>>>> + .vcodec_clks_num = 1, >>>>> + .max_load = 2563200, >>>>> + .hfi_version = HFI_VERSION_3XX, >>>>> + .vmem_id = VIDC_RESOURCE_NONE, >>>>> + .vmem_size = 0, >>>>> + .vmem_addr = 0, >>>>> + .dma_mask = 0xddc00000 - 1, >>>>> + .fwname = "qcom/venus-4.4/venus.mbn", >>>>> +}; >>>>> + >>>>> static const struct freq_tbl sdm660_freq_table[] = { >>>>> { 979200, 518400000 }, >>>>> { 489600, 441600000 }, >>>>> @@ -891,6 +928,7 @@ static const struct venus_resources sc7280_res = { >>>>> static const struct of_device_id venus_dt_match[] = { >>>>> { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, >>>>> { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, >>>>> + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, >>>>> { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, >>>>> { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, >>>>> { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, >>>>> >>>>> >>>>> >>>>> @Vikash, are you saying that perhaps the DTS for video-codec@cc00000 >>>>> needs to be written slightly differently? >>>> >>>> >>>> Certainly yes. For ex, in the clock list, i do not see the core clocks listed >>>> i.e clk_mmss_video_subcore0_clk and clk_mmss_video_subcore1_clk. You can refer >>>> the downstream video DT node [1] and then align it as per venus driver >>>> [1] >>>> https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-vidc.dtsi >>> >>> If I understand correctly (which is far from certain), >>> we should base the "qcom,msm8998-venus" DT node on >>> "qcom,sdm845-venus-v2" rather than "qcom,msm8996-venus" ? >> >> That's correct, but that's just another way to do the configuration. With the >> existing node, is video decode as well as encode working ? > > Errr, there is currently no existing node for msm8998-venus? My bad, i meant your initial node msm8998-venus, without migrating to v2. > > With the proposed node above (based on msm8996-venus) > AND the proposed work-around disabling low-power mode, > decoding works correctly. Nice, lets fix the work-around part before migrating to v2. Could you share the configurations for VIDEO_SUBCORE0_GDSC and VIDEO_SUBCORE1_GDSC ? If we see vendor code[1], sys power plane control is very much supported, so ideally we should get it working without the workaround [1] https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/drivers/media/platform/msm/vidc/venus_hfi.c#L2223 > Encoding does not work, but it has never been used/tested on our device. > > [h264_v4l2m2m @ 0xaaaaec9c44a0] Using device /dev/video1 > [h264_v4l2m2m @ 0xaaaaec9c44a0] driver 'qcom-venus' on card 'Qualcomm Venus video encoder' in mplane mode > [h264_v4l2m2m @ 0xaaaaec9c44a0] requesting formats: output=NV12/yuv420p capture=H264/none > [h264_v4l2m2m @ 0xaaaaec9c44a0] output VIDIOC_REQBUFS failed: Invalid argument > [h264_v4l2m2m @ 0xaaaaec9c44a0] no v4l2 output context's buffers > [h264_v4l2m2m @ 0xaaaaec9c44a0] can't configure encoder > [vost#0:0/h264_v4l2m2m @ 0xaaaaec9c4160] Error while opening encoder - maybe incorrect parameters such as bit_rate, rate, width or height. We can revisit the encoder failure once we have decode fixed without any workaround. Regards, Vikash
On 27/02/2024 07:55, Vikash Garodia wrote: > On 2/26/2024 9:25 PM, Marc Gonzalez wrote: > >> Errr, there is currently no existing node for msm8998-venus? > > My bad, i meant your initial node msm8998-venus, without migrating to v2. > >> With the proposed node above (based on msm8996-venus) >> AND the proposed work-around disabling low-power mode, >> decoding works correctly. > > Nice, lets fix the work-around part before migrating to v2. Could you share the > configurations for VIDEO_SUBCORE0_GDSC and VIDEO_SUBCORE1_GDSC ? > > If we see vendor code[1], sys power plane control is very much supported, so > ideally we should get it working without the workaround > [1] https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/drivers/media/platform/msm/vidc/venus_hfi.c#L2223 OK, for easy reference, here are the proposed changes again: diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi index 2793cc22d381a..5084191be1446 100644 --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi @@ -3000,6 +3000,56 @@ mdss_dsi1_phy: phy@c996400 { }; }; + venus: video-codec@cc00000 { + compatible = "qcom,msm8998-venus"; + reg = <0x0cc00000 0xff000>; + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; + power-domains = <&mmcc VIDEO_TOP_GDSC>; + clocks = <&mmcc VIDEO_CORE_CLK>, + <&mmcc VIDEO_AHB_CLK>, + <&mmcc VIDEO_AXI_CLK>, + <&mmcc VIDEO_MAXI_CLK>; + clock-names = "core", "iface", "bus", "mbus"; + iommus = <&mmss_smmu 0x400>, + <&mmss_smmu 0x401>, + <&mmss_smmu 0x40a>, + <&mmss_smmu 0x407>, + <&mmss_smmu 0x40e>, + <&mmss_smmu 0x40f>, + <&mmss_smmu 0x408>, + <&mmss_smmu 0x409>, + <&mmss_smmu 0x40b>, + <&mmss_smmu 0x40c>, + <&mmss_smmu 0x40d>, + <&mmss_smmu 0x410>, + <&mmss_smmu 0x411>, + <&mmss_smmu 0x421>, + <&mmss_smmu 0x428>, + <&mmss_smmu 0x429>, + <&mmss_smmu 0x42b>, + <&mmss_smmu 0x42c>, + <&mmss_smmu 0x42d>, + <&mmss_smmu 0x411>, + <&mmss_smmu 0x431>; + memory-region = <&venus_mem>; + status = "disabled"; + qcom,venus-broken-low-power-mode; + + video-decoder { + compatible = "venus-decoder"; + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; + clock-names = "core"; + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; + }; + + video-encoder { + compatible = "venus-encoder"; + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; + clock-names = "core"; + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; + }; + }; + mmss_smmu: iommu@cd00000 { compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; reg = <0x0cd00000 0x40000>; diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index a712dd4f02a5b..ad1705e510312 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -585,6 +585,43 @@ static const struct venus_resources msm8996_res = { .fwname = "qcom/venus-4.2/venus.mbn", }; +static const struct freq_tbl msm8998_freq_table[] = { + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ + { 972000, 520000000 }, /* 4k UHD @ 30 */ + { 489600, 346666667 }, /* 1080p @ 60 */ + { 244800, 150000000 }, /* 1080p @ 30 */ + { 108000, 75000000 }, /* 720p @ 30 */ +}; + +static const struct reg_val msm8998_reg_preset[] = { + { 0x80124, 0x00000003 }, + { 0x80550, 0x01111111 }, + { 0x80560, 0x01111111 }, + { 0x80568, 0x01111111 }, + { 0x80570, 0x01111111 }, + { 0x80580, 0x01111111 }, + { 0xe2010, 0x00000000 }, +}; + +static const struct venus_resources msm8998_res = { + .freq_tbl = msm8998_freq_table, + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), + .reg_tbl = msm8998_reg_preset, + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), + .clks = {"core", "iface", "bus", "mbus"}, + .clks_num = 4, + .vcodec0_clks = { "core" }, + .vcodec1_clks = { "core" }, + .vcodec_clks_num = 1, + .max_load = 2563200, + .hfi_version = HFI_VERSION_3XX, + .vmem_id = VIDC_RESOURCE_NONE, + .vmem_size = 0, + .vmem_addr = 0, + .dma_mask = 0xddc00000 - 1, + .fwname = "qcom/venus-4.4/venus.mbn", +}; + static const struct freq_tbl sdm660_freq_table[] = { { 979200, 518400000 }, { 489600, 441600000 }, @@ -891,6 +928,7 @@ static const struct venus_resources sc7280_res = { static const struct of_device_id venus_dt_match[] = { { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, This patch is on top of v6.8-rc1 so the configurations for VIDEO_SUBCOREx_GDSC are as defined in mainline. #define VIDEO_SUBCORE0_CLK_SRC 51 #define VIDEO_SUBCORE1_CLK_SRC 52 #define VIDEO_TOP_GDSC 1 #define VIDEO_SUBCORE0_GDSC 2 #define VIDEO_SUBCORE1_GDSC 3 https://github.com/torvalds/linux/blob/master/drivers/clk/qcom/mmcc-msm8998.c#L2536-L2561 static struct gdsc video_top_gdsc = { .gdscr = 0x1024, .pd = { .name = "video_top", }, .pwrsts = PWRSTS_OFF_ON, }; static struct gdsc video_subcore0_gdsc = { .gdscr = 0x1040, .pd = { .name = "video_subcore0", }, .parent = &video_top_gdsc.pd, .pwrsts = PWRSTS_OFF_ON, }; static struct gdsc video_subcore1_gdsc = { .gdscr = 0x1044, .pd = { .name = "video_subcore1", }, .parent = &video_top_gdsc.pd, .pwrsts = PWRSTS_OFF_ON, }; const u8 pwrsts; /* Powerdomain allowable state bitfields */ #define PWRSTS_OFF BIT(0) /* * There is no SW control to transition a GDSC into * PWRSTS_RET. This happens in HW when the parent * domain goes down to a low power state */ #define PWRSTS_RET BIT(1) #define PWRSTS_ON BIT(2) #define PWRSTS_OFF_ON (PWRSTS_OFF | PWRSTS_ON) #define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON) const u16 flags; #define VOTABLE BIT(0) #define CLAMP_IO BIT(1) #define HW_CTRL BIT(2) #define SW_RESET BIT(3) #define AON_RESET BIT(4) #define POLL_CFG_GDSCR BIT(5) #define ALWAYS_ON BIT(6) #define RETAIN_FF_ENABLE BIT(7) #define NO_RET_PERIPH BIT(8) Should .pwrsts be PWRSTS_RET_ON instead of PWRSTS_OFF_ON? Should .flags be HW_CTRL instead of 0? Regards
On 2/27/2024 9:41 PM, Marc Gonzalez wrote: > On 27/02/2024 07:55, Vikash Garodia wrote: > >> On 2/26/2024 9:25 PM, Marc Gonzalez wrote: >> >>> Errr, there is currently no existing node for msm8998-venus? >> >> My bad, i meant your initial node msm8998-venus, without migrating to v2. >> >>> With the proposed node above (based on msm8996-venus) >>> AND the proposed work-around disabling low-power mode, >>> decoding works correctly. >> >> Nice, lets fix the work-around part before migrating to v2. Could you share the >> configurations for VIDEO_SUBCORE0_GDSC and VIDEO_SUBCORE1_GDSC ? >> >> If we see vendor code[1], sys power plane control is very much supported, so >> ideally we should get it working without the workaround >> [1] https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/drivers/media/platform/msm/vidc/venus_hfi.c#L2223 > > OK, for easy reference, here are the proposed changes again: > > diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi > index 2793cc22d381a..5084191be1446 100644 > --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi > @@ -3000,6 +3000,56 @@ mdss_dsi1_phy: phy@c996400 { > }; > }; > > + venus: video-codec@cc00000 { > + compatible = "qcom,msm8998-venus"; > + reg = <0x0cc00000 0xff000>; > + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; > + power-domains = <&mmcc VIDEO_TOP_GDSC>; > + clocks = <&mmcc VIDEO_CORE_CLK>, > + <&mmcc VIDEO_AHB_CLK>, > + <&mmcc VIDEO_AXI_CLK>, > + <&mmcc VIDEO_MAXI_CLK>; > + clock-names = "core", "iface", "bus", "mbus"; > + iommus = <&mmss_smmu 0x400>, > + <&mmss_smmu 0x401>, > + <&mmss_smmu 0x40a>, > + <&mmss_smmu 0x407>, > + <&mmss_smmu 0x40e>, > + <&mmss_smmu 0x40f>, > + <&mmss_smmu 0x408>, > + <&mmss_smmu 0x409>, > + <&mmss_smmu 0x40b>, > + <&mmss_smmu 0x40c>, > + <&mmss_smmu 0x40d>, > + <&mmss_smmu 0x410>, > + <&mmss_smmu 0x411>, > + <&mmss_smmu 0x421>, > + <&mmss_smmu 0x428>, > + <&mmss_smmu 0x429>, > + <&mmss_smmu 0x42b>, > + <&mmss_smmu 0x42c>, > + <&mmss_smmu 0x42d>, > + <&mmss_smmu 0x411>, > + <&mmss_smmu 0x431>; > + memory-region = <&venus_mem>; > + status = "disabled"; > + qcom,venus-broken-low-power-mode; > + > + video-decoder { > + compatible = "venus-decoder"; > + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; > + clock-names = "core"; > + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; > + }; > + > + video-encoder { > + compatible = "venus-encoder"; > + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; > + clock-names = "core"; > + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; > + }; > + }; > + > mmss_smmu: iommu@cd00000 { > compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; > reg = <0x0cd00000 0x40000>; > diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c > index a712dd4f02a5b..ad1705e510312 100644 > --- a/drivers/media/platform/qcom/venus/core.c > +++ b/drivers/media/platform/qcom/venus/core.c > @@ -585,6 +585,43 @@ static const struct venus_resources msm8996_res = { > .fwname = "qcom/venus-4.2/venus.mbn", > }; > > +static const struct freq_tbl msm8998_freq_table[] = { > + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ > + { 972000, 520000000 }, /* 4k UHD @ 30 */ > + { 489600, 346666667 }, /* 1080p @ 60 */ > + { 244800, 150000000 }, /* 1080p @ 30 */ > + { 108000, 75000000 }, /* 720p @ 30 */ > +}; > + > +static const struct reg_val msm8998_reg_preset[] = { > + { 0x80124, 0x00000003 }, > + { 0x80550, 0x01111111 }, > + { 0x80560, 0x01111111 }, > + { 0x80568, 0x01111111 }, > + { 0x80570, 0x01111111 }, > + { 0x80580, 0x01111111 }, > + { 0xe2010, 0x00000000 }, > +}; > + > +static const struct venus_resources msm8998_res = { > + .freq_tbl = msm8998_freq_table, > + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), > + .reg_tbl = msm8998_reg_preset, > + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), > + .clks = {"core", "iface", "bus", "mbus"}, > + .clks_num = 4, > + .vcodec0_clks = { "core" }, > + .vcodec1_clks = { "core" }, > + .vcodec_clks_num = 1, > + .max_load = 2563200, > + .hfi_version = HFI_VERSION_3XX, > + .vmem_id = VIDC_RESOURCE_NONE, > + .vmem_size = 0, > + .vmem_addr = 0, > + .dma_mask = 0xddc00000 - 1, > + .fwname = "qcom/venus-4.4/venus.mbn", > +}; > + > static const struct freq_tbl sdm660_freq_table[] = { > { 979200, 518400000 }, > { 489600, 441600000 }, > @@ -891,6 +928,7 @@ static const struct venus_resources sc7280_res = { > static const struct of_device_id venus_dt_match[] = { > { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, > { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, > + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, > { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, > { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, > { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, > > > > This patch is on top of v6.8-rc1 > so the configurations for VIDEO_SUBCOREx_GDSC > are as defined in mainline. > > #define VIDEO_SUBCORE0_CLK_SRC 51 > #define VIDEO_SUBCORE1_CLK_SRC 52 > > #define VIDEO_TOP_GDSC 1 > #define VIDEO_SUBCORE0_GDSC 2 > #define VIDEO_SUBCORE1_GDSC 3 > > https://github.com/torvalds/linux/blob/master/drivers/clk/qcom/mmcc-msm8998.c#L2536-L2561 > > static struct gdsc video_top_gdsc = { > .gdscr = 0x1024, > .pd = { > .name = "video_top", > }, > .pwrsts = PWRSTS_OFF_ON, > }; > > static struct gdsc video_subcore0_gdsc = { > .gdscr = 0x1040, > .pd = { > .name = "video_subcore0", > }, > .parent = &video_top_gdsc.pd, > .pwrsts = PWRSTS_OFF_ON, > }; > > static struct gdsc video_subcore1_gdsc = { > .gdscr = 0x1044, > .pd = { > .name = "video_subcore1", > }, > .parent = &video_top_gdsc.pd, > .pwrsts = PWRSTS_OFF_ON, > }; > > > const u8 pwrsts; > /* Powerdomain allowable state bitfields */ > #define PWRSTS_OFF BIT(0) > /* > * There is no SW control to transition a GDSC into > * PWRSTS_RET. This happens in HW when the parent > * domain goes down to a low power state > */ > #define PWRSTS_RET BIT(1) > #define PWRSTS_ON BIT(2) > #define PWRSTS_OFF_ON (PWRSTS_OFF | PWRSTS_ON) > #define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON) > const u16 flags; > #define VOTABLE BIT(0) > #define CLAMP_IO BIT(1) > #define HW_CTRL BIT(2) > #define SW_RESET BIT(3) > #define AON_RESET BIT(4) > #define POLL_CFG_GDSCR BIT(5) > #define ALWAYS_ON BIT(6) > #define RETAIN_FF_ENABLE BIT(7) > #define NO_RET_PERIPH BIT(8) > > > Should .pwrsts be PWRSTS_RET_ON instead of PWRSTS_OFF_ON? > > Should .flags be HW_CTRL instead of 0? Not completely sure on these configurations, but certainly both the video_subcore0_gdsc and video_subcore1_gdsc should be configured in hardware control mode in the gdsc configuration. Regards, Vikash
On 29/02/2024 16:32, Vikash Garodia wrote: > On 2/27/2024 9:41 PM, Marc Gonzalez wrote: > >> On 27/02/2024 07:55, Vikash Garodia wrote: >> >>> On 2/26/2024 9:25 PM, Marc Gonzalez wrote: >>> >>>> Errr, there is currently no existing node for msm8998-venus? >>> >>> My bad, i meant your initial node msm8998-venus, without migrating to v2. >>> >>>> With the proposed node above (based on msm8996-venus) >>>> AND the proposed work-around disabling low-power mode, >>>> decoding works correctly. >>> >>> Nice, lets fix the work-around part before migrating to v2. Could you share the >>> configurations for VIDEO_SUBCORE0_GDSC and VIDEO_SUBCORE1_GDSC ? >>> >>> If we see vendor code[1], sys power plane control is very much supported, so >>> ideally we should get it working without the workaround >>> [1] https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/drivers/media/platform/msm/vidc/venus_hfi.c#L2223 >> >> OK, for easy reference, here are the proposed changes again: >> >> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi >> index 2793cc22d381a..5084191be1446 100644 >> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi >> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi >> @@ -3000,6 +3000,56 @@ mdss_dsi1_phy: phy@c996400 { >> }; >> }; >> >> + venus: video-codec@cc00000 { >> + compatible = "qcom,msm8998-venus"; >> + reg = <0x0cc00000 0xff000>; >> + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; >> + power-domains = <&mmcc VIDEO_TOP_GDSC>; >> + clocks = <&mmcc VIDEO_CORE_CLK>, >> + <&mmcc VIDEO_AHB_CLK>, >> + <&mmcc VIDEO_AXI_CLK>, >> + <&mmcc VIDEO_MAXI_CLK>; >> + clock-names = "core", "iface", "bus", "mbus"; >> + iommus = <&mmss_smmu 0x400>, >> + <&mmss_smmu 0x401>, >> + <&mmss_smmu 0x40a>, >> + <&mmss_smmu 0x407>, >> + <&mmss_smmu 0x40e>, >> + <&mmss_smmu 0x40f>, >> + <&mmss_smmu 0x408>, >> + <&mmss_smmu 0x409>, >> + <&mmss_smmu 0x40b>, >> + <&mmss_smmu 0x40c>, >> + <&mmss_smmu 0x40d>, >> + <&mmss_smmu 0x410>, >> + <&mmss_smmu 0x411>, >> + <&mmss_smmu 0x421>, >> + <&mmss_smmu 0x428>, >> + <&mmss_smmu 0x429>, >> + <&mmss_smmu 0x42b>, >> + <&mmss_smmu 0x42c>, >> + <&mmss_smmu 0x42d>, >> + <&mmss_smmu 0x411>, >> + <&mmss_smmu 0x431>; >> + memory-region = <&venus_mem>; >> + status = "disabled"; >> + qcom,venus-broken-low-power-mode; >> + >> + video-decoder { >> + compatible = "venus-decoder"; >> + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; >> + clock-names = "core"; >> + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; >> + }; >> + >> + video-encoder { >> + compatible = "venus-encoder"; >> + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; >> + clock-names = "core"; >> + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; >> + }; >> + }; >> + >> mmss_smmu: iommu@cd00000 { >> compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; >> reg = <0x0cd00000 0x40000>; >> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c >> index a712dd4f02a5b..ad1705e510312 100644 >> --- a/drivers/media/platform/qcom/venus/core.c >> +++ b/drivers/media/platform/qcom/venus/core.c >> @@ -585,6 +585,43 @@ static const struct venus_resources msm8996_res = { >> .fwname = "qcom/venus-4.2/venus.mbn", >> }; >> >> +static const struct freq_tbl msm8998_freq_table[] = { >> + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ >> + { 972000, 520000000 }, /* 4k UHD @ 30 */ >> + { 489600, 346666667 }, /* 1080p @ 60 */ >> + { 244800, 150000000 }, /* 1080p @ 30 */ >> + { 108000, 75000000 }, /* 720p @ 30 */ >> +}; >> + >> +static const struct reg_val msm8998_reg_preset[] = { >> + { 0x80124, 0x00000003 }, >> + { 0x80550, 0x01111111 }, >> + { 0x80560, 0x01111111 }, >> + { 0x80568, 0x01111111 }, >> + { 0x80570, 0x01111111 }, >> + { 0x80580, 0x01111111 }, >> + { 0xe2010, 0x00000000 }, >> +}; >> + >> +static const struct venus_resources msm8998_res = { >> + .freq_tbl = msm8998_freq_table, >> + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), >> + .reg_tbl = msm8998_reg_preset, >> + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), >> + .clks = {"core", "iface", "bus", "mbus"}, >> + .clks_num = 4, >> + .vcodec0_clks = { "core" }, >> + .vcodec1_clks = { "core" }, >> + .vcodec_clks_num = 1, >> + .max_load = 2563200, >> + .hfi_version = HFI_VERSION_3XX, >> + .vmem_id = VIDC_RESOURCE_NONE, >> + .vmem_size = 0, >> + .vmem_addr = 0, >> + .dma_mask = 0xddc00000 - 1, >> + .fwname = "qcom/venus-4.4/venus.mbn", >> +}; >> + >> static const struct freq_tbl sdm660_freq_table[] = { >> { 979200, 518400000 }, >> { 489600, 441600000 }, >> @@ -891,6 +928,7 @@ static const struct venus_resources sc7280_res = { >> static const struct of_device_id venus_dt_match[] = { >> { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, >> { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, >> + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, >> { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, >> { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, >> { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, >> >> >> >> This patch is on top of v6.8-rc1 >> so the configurations for VIDEO_SUBCOREx_GDSC >> are as defined in mainline. >> >> #define VIDEO_SUBCORE0_CLK_SRC 51 >> #define VIDEO_SUBCORE1_CLK_SRC 52 >> >> #define VIDEO_TOP_GDSC 1 >> #define VIDEO_SUBCORE0_GDSC 2 >> #define VIDEO_SUBCORE1_GDSC 3 >> >> https://github.com/torvalds/linux/blob/master/drivers/clk/qcom/mmcc-msm8998.c#L2536-L2561 >> >> static struct gdsc video_top_gdsc = { >> .gdscr = 0x1024, >> .pd = { >> .name = "video_top", >> }, >> .pwrsts = PWRSTS_OFF_ON, >> }; >> >> static struct gdsc video_subcore0_gdsc = { >> .gdscr = 0x1040, >> .pd = { >> .name = "video_subcore0", >> }, >> .parent = &video_top_gdsc.pd, >> .pwrsts = PWRSTS_OFF_ON, >> }; >> >> static struct gdsc video_subcore1_gdsc = { >> .gdscr = 0x1044, >> .pd = { >> .name = "video_subcore1", >> }, >> .parent = &video_top_gdsc.pd, >> .pwrsts = PWRSTS_OFF_ON, >> }; >> >> >> const u8 pwrsts; >> /* Powerdomain allowable state bitfields */ >> #define PWRSTS_OFF BIT(0) >> /* >> * There is no SW control to transition a GDSC into >> * PWRSTS_RET. This happens in HW when the parent >> * domain goes down to a low power state >> */ >> #define PWRSTS_RET BIT(1) >> #define PWRSTS_ON BIT(2) >> #define PWRSTS_OFF_ON (PWRSTS_OFF | PWRSTS_ON) >> #define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON) >> const u16 flags; >> #define VOTABLE BIT(0) >> #define CLAMP_IO BIT(1) >> #define HW_CTRL BIT(2) >> #define SW_RESET BIT(3) >> #define AON_RESET BIT(4) >> #define POLL_CFG_GDSCR BIT(5) >> #define ALWAYS_ON BIT(6) >> #define RETAIN_FF_ENABLE BIT(7) >> #define NO_RET_PERIPH BIT(8) >> >> >> Should .pwrsts be PWRSTS_RET_ON instead of PWRSTS_OFF_ON? >> >> Should .flags be HW_CTRL instead of 0? > > Not completely sure on these configurations, but certainly both the > video_subcore0_gdsc and video_subcore1_gdsc should be configured in hardware > control mode in the gdsc configuration. Jeffrey, Bjorn, I'm trying to get mainline support for the msm8998 video decoder (venus). Apparently, there appears to be an issue with the multimedia clocks. Do you remember why you chose PWRSTS_OFF_ON instead of PWRSTS_RET_ON? And why the HW_CTRL bit is not set? In 96893e101eb294bced8358fbd48cbac175977aa4 "clk: qcom: Put venus core0/1 gdscs to hw control mode" Sricharan set the HW_CTRL bit in venus_core0_gdsc & venus_core1_gdsc for msm8996. A priori, it is required for msm8998 as well. Same deal with 196eb928525579 "clk: qcom: mmcc-sdm660: Add hw_ctrl flag to venus_core0_gdsc" Regards
On 2/29/2024 9:24 AM, Marc Gonzalez wrote: > On 29/02/2024 16:32, Vikash Garodia wrote: > >> On 2/27/2024 9:41 PM, Marc Gonzalez wrote: >> >>> On 27/02/2024 07:55, Vikash Garodia wrote: >>> >>>> On 2/26/2024 9:25 PM, Marc Gonzalez wrote: >>>> >>>>> Errr, there is currently no existing node for msm8998-venus? >>>> >>>> My bad, i meant your initial node msm8998-venus, without migrating to v2. >>>> >>>>> With the proposed node above (based on msm8996-venus) >>>>> AND the proposed work-around disabling low-power mode, >>>>> decoding works correctly. >>>> >>>> Nice, lets fix the work-around part before migrating to v2. Could you share the >>>> configurations for VIDEO_SUBCORE0_GDSC and VIDEO_SUBCORE1_GDSC ? >>>> >>>> If we see vendor code[1], sys power plane control is very much supported, so >>>> ideally we should get it working without the workaround >>>> [1] https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/drivers/media/platform/msm/vidc/venus_hfi.c#L2223 >>> >>> OK, for easy reference, here are the proposed changes again: >>> >>> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi >>> index 2793cc22d381a..5084191be1446 100644 >>> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi >>> @@ -3000,6 +3000,56 @@ mdss_dsi1_phy: phy@c996400 { >>> }; >>> }; >>> >>> + venus: video-codec@cc00000 { >>> + compatible = "qcom,msm8998-venus"; >>> + reg = <0x0cc00000 0xff000>; >>> + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; >>> + power-domains = <&mmcc VIDEO_TOP_GDSC>; >>> + clocks = <&mmcc VIDEO_CORE_CLK>, >>> + <&mmcc VIDEO_AHB_CLK>, >>> + <&mmcc VIDEO_AXI_CLK>, >>> + <&mmcc VIDEO_MAXI_CLK>; >>> + clock-names = "core", "iface", "bus", "mbus"; >>> + iommus = <&mmss_smmu 0x400>, >>> + <&mmss_smmu 0x401>, >>> + <&mmss_smmu 0x40a>, >>> + <&mmss_smmu 0x407>, >>> + <&mmss_smmu 0x40e>, >>> + <&mmss_smmu 0x40f>, >>> + <&mmss_smmu 0x408>, >>> + <&mmss_smmu 0x409>, >>> + <&mmss_smmu 0x40b>, >>> + <&mmss_smmu 0x40c>, >>> + <&mmss_smmu 0x40d>, >>> + <&mmss_smmu 0x410>, >>> + <&mmss_smmu 0x411>, >>> + <&mmss_smmu 0x421>, >>> + <&mmss_smmu 0x428>, >>> + <&mmss_smmu 0x429>, >>> + <&mmss_smmu 0x42b>, >>> + <&mmss_smmu 0x42c>, >>> + <&mmss_smmu 0x42d>, >>> + <&mmss_smmu 0x411>, >>> + <&mmss_smmu 0x431>; >>> + memory-region = <&venus_mem>; >>> + status = "disabled"; >>> + qcom,venus-broken-low-power-mode; >>> + >>> + video-decoder { >>> + compatible = "venus-decoder"; >>> + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; >>> + clock-names = "core"; >>> + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; >>> + }; >>> + >>> + video-encoder { >>> + compatible = "venus-encoder"; >>> + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; >>> + clock-names = "core"; >>> + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; >>> + }; >>> + }; >>> + >>> mmss_smmu: iommu@cd00000 { >>> compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; >>> reg = <0x0cd00000 0x40000>; >>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c >>> index a712dd4f02a5b..ad1705e510312 100644 >>> --- a/drivers/media/platform/qcom/venus/core.c >>> +++ b/drivers/media/platform/qcom/venus/core.c >>> @@ -585,6 +585,43 @@ static const struct venus_resources msm8996_res = { >>> .fwname = "qcom/venus-4.2/venus.mbn", >>> }; >>> >>> +static const struct freq_tbl msm8998_freq_table[] = { >>> + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ >>> + { 972000, 520000000 }, /* 4k UHD @ 30 */ >>> + { 489600, 346666667 }, /* 1080p @ 60 */ >>> + { 244800, 150000000 }, /* 1080p @ 30 */ >>> + { 108000, 75000000 }, /* 720p @ 30 */ >>> +}; >>> + >>> +static const struct reg_val msm8998_reg_preset[] = { >>> + { 0x80124, 0x00000003 }, >>> + { 0x80550, 0x01111111 }, >>> + { 0x80560, 0x01111111 }, >>> + { 0x80568, 0x01111111 }, >>> + { 0x80570, 0x01111111 }, >>> + { 0x80580, 0x01111111 }, >>> + { 0xe2010, 0x00000000 }, >>> +}; >>> + >>> +static const struct venus_resources msm8998_res = { >>> + .freq_tbl = msm8998_freq_table, >>> + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), >>> + .reg_tbl = msm8998_reg_preset, >>> + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), >>> + .clks = {"core", "iface", "bus", "mbus"}, >>> + .clks_num = 4, >>> + .vcodec0_clks = { "core" }, >>> + .vcodec1_clks = { "core" }, >>> + .vcodec_clks_num = 1, >>> + .max_load = 2563200, >>> + .hfi_version = HFI_VERSION_3XX, >>> + .vmem_id = VIDC_RESOURCE_NONE, >>> + .vmem_size = 0, >>> + .vmem_addr = 0, >>> + .dma_mask = 0xddc00000 - 1, >>> + .fwname = "qcom/venus-4.4/venus.mbn", >>> +}; >>> + >>> static const struct freq_tbl sdm660_freq_table[] = { >>> { 979200, 518400000 }, >>> { 489600, 441600000 }, >>> @@ -891,6 +928,7 @@ static const struct venus_resources sc7280_res = { >>> static const struct of_device_id venus_dt_match[] = { >>> { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, >>> { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, >>> + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, >>> { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, >>> { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, >>> { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, >>> >>> >>> >>> This patch is on top of v6.8-rc1 >>> so the configurations for VIDEO_SUBCOREx_GDSC >>> are as defined in mainline. >>> >>> #define VIDEO_SUBCORE0_CLK_SRC 51 >>> #define VIDEO_SUBCORE1_CLK_SRC 52 >>> >>> #define VIDEO_TOP_GDSC 1 >>> #define VIDEO_SUBCORE0_GDSC 2 >>> #define VIDEO_SUBCORE1_GDSC 3 >>> >>> https://github.com/torvalds/linux/blob/master/drivers/clk/qcom/mmcc-msm8998.c#L2536-L2561 >>> >>> static struct gdsc video_top_gdsc = { >>> .gdscr = 0x1024, >>> .pd = { >>> .name = "video_top", >>> }, >>> .pwrsts = PWRSTS_OFF_ON, >>> }; >>> >>> static struct gdsc video_subcore0_gdsc = { >>> .gdscr = 0x1040, >>> .pd = { >>> .name = "video_subcore0", >>> }, >>> .parent = &video_top_gdsc.pd, >>> .pwrsts = PWRSTS_OFF_ON, >>> }; >>> >>> static struct gdsc video_subcore1_gdsc = { >>> .gdscr = 0x1044, >>> .pd = { >>> .name = "video_subcore1", >>> }, >>> .parent = &video_top_gdsc.pd, >>> .pwrsts = PWRSTS_OFF_ON, >>> }; >>> >>> >>> const u8 pwrsts; >>> /* Powerdomain allowable state bitfields */ >>> #define PWRSTS_OFF BIT(0) >>> /* >>> * There is no SW control to transition a GDSC into >>> * PWRSTS_RET. This happens in HW when the parent >>> * domain goes down to a low power state >>> */ >>> #define PWRSTS_RET BIT(1) >>> #define PWRSTS_ON BIT(2) >>> #define PWRSTS_OFF_ON (PWRSTS_OFF | PWRSTS_ON) >>> #define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON) >>> const u16 flags; >>> #define VOTABLE BIT(0) >>> #define CLAMP_IO BIT(1) >>> #define HW_CTRL BIT(2) >>> #define SW_RESET BIT(3) >>> #define AON_RESET BIT(4) >>> #define POLL_CFG_GDSCR BIT(5) >>> #define ALWAYS_ON BIT(6) >>> #define RETAIN_FF_ENABLE BIT(7) >>> #define NO_RET_PERIPH BIT(8) >>> >>> >>> Should .pwrsts be PWRSTS_RET_ON instead of PWRSTS_OFF_ON? >>> >>> Should .flags be HW_CTRL instead of 0? >> >> Not completely sure on these configurations, but certainly both the >> video_subcore0_gdsc and video_subcore1_gdsc should be configured in hardware >> control mode in the gdsc configuration. > > Jeffrey, Bjorn, > > I'm trying to get mainline support for the msm8998 video decoder (venus). > Apparently, there appears to be an issue with the multimedia clocks. > > Do you remember why you chose PWRSTS_OFF_ON instead of PWRSTS_RET_ON? > > And why the HW_CTRL bit is not set? Not really. Looks like this was 4-5 years ago. I'm guessing I looked at either 845 or 8996 to figure out which one was closer to 8998, and then copied that implementation with a quick sanity check against downstream and any hardware documentation I could find. I think the GDSC implementation has drastically improved since then, so OFF_ON might have been correct then, but is not correct now. Sorry, probably not much help. -Jeff
On 2/29/24 17:24, Marc Gonzalez wrote: > On 29/02/2024 16:32, Vikash Garodia wrote: > >> On 2/27/2024 9:41 PM, Marc Gonzalez wrote: >> >>> On 27/02/2024 07:55, Vikash Garodia wrote: >>> >>>> On 2/26/2024 9:25 PM, Marc Gonzalez wrote: >>>> >>>>> Errr, there is currently no existing node for msm8998-venus? >>>> >>>> My bad, i meant your initial node msm8998-venus, without migrating to v2. >>>> >>>>> With the proposed node above (based on msm8996-venus) >>>>> AND the proposed work-around disabling low-power mode, >>>>> decoding works correctly. >>>> >>>> Nice, lets fix the work-around part before migrating to v2. Could you share the >>>> configurations for VIDEO_SUBCORE0_GDSC and VIDEO_SUBCORE1_GDSC ? >>>> >>>> If we see vendor code[1], sys power plane control is very much supported, so >>>> ideally we should get it working without the workaround >>>> [1] https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/drivers/media/platform/msm/vidc/venus_hfi.c#L2223 >>> >>> OK, for easy reference, here are the proposed changes again: >>> >>> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi >>> index 2793cc22d381a..5084191be1446 100644 >>> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi >>> @@ -3000,6 +3000,56 @@ mdss_dsi1_phy: phy@c996400 { >>> }; >>> }; >>> >>> + venus: video-codec@cc00000 { >>> + compatible = "qcom,msm8998-venus"; >>> + reg = <0x0cc00000 0xff000>; >>> + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; >>> + power-domains = <&mmcc VIDEO_TOP_GDSC>; >>> + clocks = <&mmcc VIDEO_CORE_CLK>, >>> + <&mmcc VIDEO_AHB_CLK>, >>> + <&mmcc VIDEO_AXI_CLK>, >>> + <&mmcc VIDEO_MAXI_CLK>; >>> + clock-names = "core", "iface", "bus", "mbus"; >>> + iommus = <&mmss_smmu 0x400>, >>> + <&mmss_smmu 0x401>, >>> + <&mmss_smmu 0x40a>, >>> + <&mmss_smmu 0x407>, >>> + <&mmss_smmu 0x40e>, >>> + <&mmss_smmu 0x40f>, >>> + <&mmss_smmu 0x408>, >>> + <&mmss_smmu 0x409>, >>> + <&mmss_smmu 0x40b>, >>> + <&mmss_smmu 0x40c>, >>> + <&mmss_smmu 0x40d>, >>> + <&mmss_smmu 0x410>, >>> + <&mmss_smmu 0x411>, >>> + <&mmss_smmu 0x421>, >>> + <&mmss_smmu 0x428>, >>> + <&mmss_smmu 0x429>, >>> + <&mmss_smmu 0x42b>, >>> + <&mmss_smmu 0x42c>, >>> + <&mmss_smmu 0x42d>, >>> + <&mmss_smmu 0x411>, >>> + <&mmss_smmu 0x431>; >>> + memory-region = <&venus_mem>; >>> + status = "disabled"; >>> + qcom,venus-broken-low-power-mode; >>> + >>> + video-decoder { >>> + compatible = "venus-decoder"; >>> + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; >>> + clock-names = "core"; >>> + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; >>> + }; >>> + >>> + video-encoder { >>> + compatible = "venus-encoder"; >>> + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; >>> + clock-names = "core"; >>> + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; >>> + }; >>> + }; >>> + >>> mmss_smmu: iommu@cd00000 { >>> compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; >>> reg = <0x0cd00000 0x40000>; >>> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c >>> index a712dd4f02a5b..ad1705e510312 100644 >>> --- a/drivers/media/platform/qcom/venus/core.c >>> +++ b/drivers/media/platform/qcom/venus/core.c >>> @@ -585,6 +585,43 @@ static const struct venus_resources msm8996_res = { >>> .fwname = "qcom/venus-4.2/venus.mbn", >>> }; >>> >>> +static const struct freq_tbl msm8998_freq_table[] = { >>> + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ >>> + { 972000, 520000000 }, /* 4k UHD @ 30 */ >>> + { 489600, 346666667 }, /* 1080p @ 60 */ >>> + { 244800, 150000000 }, /* 1080p @ 30 */ >>> + { 108000, 75000000 }, /* 720p @ 30 */ >>> +}; >>> + >>> +static const struct reg_val msm8998_reg_preset[] = { >>> + { 0x80124, 0x00000003 }, >>> + { 0x80550, 0x01111111 }, >>> + { 0x80560, 0x01111111 }, >>> + { 0x80568, 0x01111111 }, >>> + { 0x80570, 0x01111111 }, >>> + { 0x80580, 0x01111111 }, >>> + { 0xe2010, 0x00000000 }, >>> +}; >>> + >>> +static const struct venus_resources msm8998_res = { >>> + .freq_tbl = msm8998_freq_table, >>> + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), >>> + .reg_tbl = msm8998_reg_preset, >>> + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), >>> + .clks = {"core", "iface", "bus", "mbus"}, >>> + .clks_num = 4, >>> + .vcodec0_clks = { "core" }, >>> + .vcodec1_clks = { "core" }, >>> + .vcodec_clks_num = 1, >>> + .max_load = 2563200, >>> + .hfi_version = HFI_VERSION_3XX, >>> + .vmem_id = VIDC_RESOURCE_NONE, >>> + .vmem_size = 0, >>> + .vmem_addr = 0, >>> + .dma_mask = 0xddc00000 - 1, >>> + .fwname = "qcom/venus-4.4/venus.mbn", >>> +}; >>> + >>> static const struct freq_tbl sdm660_freq_table[] = { >>> { 979200, 518400000 }, >>> { 489600, 441600000 }, >>> @@ -891,6 +928,7 @@ static const struct venus_resources sc7280_res = { >>> static const struct of_device_id venus_dt_match[] = { >>> { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, >>> { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, >>> + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, >>> { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, >>> { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, >>> { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, >>> >>> >>> >>> This patch is on top of v6.8-rc1 >>> so the configurations for VIDEO_SUBCOREx_GDSC >>> are as defined in mainline. >>> >>> #define VIDEO_SUBCORE0_CLK_SRC 51 >>> #define VIDEO_SUBCORE1_CLK_SRC 52 >>> >>> #define VIDEO_TOP_GDSC 1 >>> #define VIDEO_SUBCORE0_GDSC 2 >>> #define VIDEO_SUBCORE1_GDSC 3 >>> >>> https://github.com/torvalds/linux/blob/master/drivers/clk/qcom/mmcc-msm8998.c#L2536-L2561 >>> >>> static struct gdsc video_top_gdsc = { >>> .gdscr = 0x1024, >>> .pd = { >>> .name = "video_top", >>> }, >>> .pwrsts = PWRSTS_OFF_ON, >>> }; >>> >>> static struct gdsc video_subcore0_gdsc = { >>> .gdscr = 0x1040, >>> .pd = { >>> .name = "video_subcore0", >>> }, >>> .parent = &video_top_gdsc.pd, >>> .pwrsts = PWRSTS_OFF_ON, >>> }; >>> >>> static struct gdsc video_subcore1_gdsc = { >>> .gdscr = 0x1044, >>> .pd = { >>> .name = "video_subcore1", >>> }, >>> .parent = &video_top_gdsc.pd, >>> .pwrsts = PWRSTS_OFF_ON, >>> }; >>> >>> >>> const u8 pwrsts; >>> /* Powerdomain allowable state bitfields */ >>> #define PWRSTS_OFF BIT(0) >>> /* >>> * There is no SW control to transition a GDSC into >>> * PWRSTS_RET. This happens in HW when the parent >>> * domain goes down to a low power state >>> */ >>> #define PWRSTS_RET BIT(1) >>> #define PWRSTS_ON BIT(2) >>> #define PWRSTS_OFF_ON (PWRSTS_OFF | PWRSTS_ON) >>> #define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON) >>> const u16 flags; >>> #define VOTABLE BIT(0) >>> #define CLAMP_IO BIT(1) >>> #define HW_CTRL BIT(2) >>> #define SW_RESET BIT(3) >>> #define AON_RESET BIT(4) >>> #define POLL_CFG_GDSCR BIT(5) >>> #define ALWAYS_ON BIT(6) >>> #define RETAIN_FF_ENABLE BIT(7) >>> #define NO_RET_PERIPH BIT(8) >>> >>> >>> Should .pwrsts be PWRSTS_RET_ON instead of PWRSTS_OFF_ON? >>> >>> Should .flags be HW_CTRL instead of 0? >> >> Not completely sure on these configurations, but certainly both the >> video_subcore0_gdsc and video_subcore1_gdsc should be configured in hardware >> control mode in the gdsc configuration. > > Jeffrey, Bjorn, > > I'm trying to get mainline support for the msm8998 video decoder (venus). > Apparently, there appears to be an issue with the multimedia clocks. > > Do you remember why you chose PWRSTS_OFF_ON instead of PWRSTS_RET_ON? Doing a quick reconnaissance against msm-4.4, PWRSTS_OFF_ON looks very sane. Moreover, PWRSTS_RET_ON very much only looks useful as a hack for non-fully-implemented drivers (and that would indeed match the state of our usb and pcie driver today) - it prevents GDSC shutdown, but tells the PM framework that the registered power domain has collapsed > > And why the HW_CTRL bit is not set? HW_CTRL means "totally hand over the control of this GDSC to the hardware" where "hardware" is very loosely defined.. Reading msm-4.4 again, it seems like we want HW_CTRL mode to be enabled if and only if the low power property has been set through the venus firmware interface. More particularly, we don't want it to be there once we wanna shut down Venus for good. This is being worked on by folks over at [1]. https://lore.kernel.org/linux-arm-msm/20240122-gdsc-hwctrl-v4-0-9061e8a7aa07@linaro.org/ Konrad
On 29/02/2024 16:32, Vikash Garodia wrote: > Not completely sure on these configurations, but certainly both the > video_subcore0_gdsc and video_subcore1_gdsc should be configured in hardware > control mode in the gdsc configuration. Hello, Still trying to land support for venus decoder on msm8998 in mainline. This is the patch I have at the moment: diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi index 4dfe2d09ac285..67b8374ddf02f 100644 --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi @@ -3010,6 +3010,55 @@ mdss_dsi1_phy: phy@c996400 { }; }; + venus: video-codec@cc00000 { + compatible = "qcom,msm8998-venus"; + reg = <0x0cc00000 0xff000>; + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; + power-domains = <&mmcc VIDEO_TOP_GDSC>; + clocks = <&mmcc VIDEO_CORE_CLK>, + <&mmcc VIDEO_AHB_CLK>, + <&mmcc VIDEO_AXI_CLK>, + <&mmcc VIDEO_MAXI_CLK>; + clock-names = "core", "iface", "bus", "mbus"; + iommus = <&mmss_smmu 0x400>, + <&mmss_smmu 0x401>, + <&mmss_smmu 0x40a>, + <&mmss_smmu 0x407>, + <&mmss_smmu 0x40e>, + <&mmss_smmu 0x40f>, + <&mmss_smmu 0x408>, + <&mmss_smmu 0x409>, + <&mmss_smmu 0x40b>, + <&mmss_smmu 0x40c>, + <&mmss_smmu 0x40d>, + <&mmss_smmu 0x410>, + <&mmss_smmu 0x411>, + <&mmss_smmu 0x421>, + <&mmss_smmu 0x428>, + <&mmss_smmu 0x429>, + <&mmss_smmu 0x42b>, + <&mmss_smmu 0x42c>, + <&mmss_smmu 0x42d>, + <&mmss_smmu 0x411>, + <&mmss_smmu 0x431>; + memory-region = <&venus_mem>; + status = "disabled"; + + video-decoder { + compatible = "venus-decoder"; + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; + clock-names = "core"; + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; + }; + + video-encoder { + compatible = "venus-encoder"; + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; + clock-names = "core"; + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; + }; + }; + mmss_smmu: iommu@cd00000 { compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; reg = <0x0cd00000 0x40000>; diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index ce206b7097541..42e0c580e093d 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -587,6 +587,47 @@ static const struct venus_resources msm8996_res = { .fwname = "qcom/venus-4.2/venus.mbn", }; +static const struct freq_tbl msm8998_freq_table[] = { + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ + { 972000, 520000000 }, /* 4k UHD @ 30 */ + { 489600, 346666667 }, /* 1080p @ 60 */ + { 244800, 150000000 }, /* 1080p @ 30 */ + { 108000, 75000000 }, /* 720p @ 30 */ +}; + +/* + * https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-vidc.dtsi + */ +static const struct reg_val msm8998_reg_preset[] = { + { 0x80124, 0x00000003 }, + { 0x80550, 0x01111111 }, + { 0x80560, 0x01111111 }, + { 0x80568, 0x01111111 }, + { 0x80570, 0x01111111 }, + { 0x80580, 0x01111111 }, + { 0x80588, 0x01111111 }, + { 0xe2010, 0x00000000 }, +}; + +static const struct venus_resources msm8998_res = { + .freq_tbl = msm8998_freq_table, + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), + .reg_tbl = msm8998_reg_preset, + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), + .clks = { "core", "iface", "bus", "mbus" }, + .clks_num = 4, + .vcodec0_clks = { "core" }, + .vcodec1_clks = { "core" }, + .vcodec_clks_num = 1, + .max_load = 2563200, + .hfi_version = HFI_VERSION_3XX, + .vmem_id = VIDC_RESOURCE_NONE, + .vmem_size = 0, + .vmem_addr = 0, + .dma_mask = 0xddc00000 - 1, + .fwname = "qcom/venus-4.4/venus.mbn", +}; + static const struct freq_tbl sdm660_freq_table[] = { { 979200, 518400000 }, { 489600, 441600000 }, @@ -893,6 +934,7 @@ static const struct venus_resources sc7280_res = { static const struct of_device_id venus_dt_match[] = { { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c index f9437b6412b91..abdc578ce988e 100644 --- a/drivers/media/platform/qcom/venus/hfi_venus.c +++ b/drivers/media/platform/qcom/venus/hfi_venus.c @@ -945,6 +945,7 @@ static int venus_sys_set_default_properties(struct venus_hfi_device *hdev) dev_warn(dev, "setting idle response ON failed (%d)\n", ret); } + venus_fw_low_power_mode = false; ret = venus_sys_set_power_control(hdev, venus_fw_low_power_mode); if (ret) dev_warn(dev, "setting hw power collapse ON failed (%d)\n", With the quick and dirty hack in hfi_venus.c I am able to correctly decode using venus with: # mpv --hwdec=v4l2m2m-copy --vo=tct --quiet demo-480.webm (+) Video --vid=1 (*) (vp9 854x480 29.970fps) (+) Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl Without the hack, HW decoding fails (and falls back to SW decode) # cd /home && mpv --hwdec=v4l2m2m-copy --vo=tct --quiet demo-480.webm (+) Video --vid=1 (*) (vp9 854x480 29.970fps) (+) Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) [ffmpeg/video] vp9_v4l2m2m: output VIDIOC_REQBUFS failed: Connection timed out [ffmpeg/video] vp9_v4l2m2m: no v4l2 output context's buffers [ffmpeg/video] vp9_v4l2m2m: can't configure decoder Could not open codec. Not sure where to go from here. Vikash, do you have any guidance? (I think you were not a fan of the DT-based work-around?) Regards
On 08/04/2024 16:39, Marc Gonzalez wrote: > On 29/02/2024 16:32, Vikash Garodia wrote: > >> Not completely sure on these configurations, but certainly both the >> video_subcore0_gdsc and video_subcore1_gdsc should be configured in hardware >> control mode in the gdsc configuration. > > Hello, > > Still trying to land support for venus decoder on msm8998 in mainline. > > This is the patch I have at the moment: > > diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi > index 4dfe2d09ac285..67b8374ddf02f 100644 > --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi > @@ -3010,6 +3010,55 @@ mdss_dsi1_phy: phy@c996400 { > }; > }; > > + venus: video-codec@cc00000 { > + compatible = "qcom,msm8998-venus"; > + reg = <0x0cc00000 0xff000>; > + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; > + power-domains = <&mmcc VIDEO_TOP_GDSC>; > + clocks = <&mmcc VIDEO_CORE_CLK>, > + <&mmcc VIDEO_AHB_CLK>, > + <&mmcc VIDEO_AXI_CLK>, > + <&mmcc VIDEO_MAXI_CLK>; > + clock-names = "core", "iface", "bus", "mbus"; > + iommus = <&mmss_smmu 0x400>, > + <&mmss_smmu 0x401>, > + <&mmss_smmu 0x40a>, > + <&mmss_smmu 0x407>, > + <&mmss_smmu 0x40e>, > + <&mmss_smmu 0x40f>, > + <&mmss_smmu 0x408>, > + <&mmss_smmu 0x409>, > + <&mmss_smmu 0x40b>, > + <&mmss_smmu 0x40c>, > + <&mmss_smmu 0x40d>, > + <&mmss_smmu 0x410>, > + <&mmss_smmu 0x411>, > + <&mmss_smmu 0x421>, > + <&mmss_smmu 0x428>, > + <&mmss_smmu 0x429>, > + <&mmss_smmu 0x42b>, > + <&mmss_smmu 0x42c>, > + <&mmss_smmu 0x42d>, > + <&mmss_smmu 0x411>, > + <&mmss_smmu 0x431>; > + memory-region = <&venus_mem>; > + status = "disabled"; > + > + video-decoder { > + compatible = "venus-decoder"; > + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; > + clock-names = "core"; > + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; > + }; > + > + video-encoder { > + compatible = "venus-encoder"; > + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; > + clock-names = "core"; > + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; > + }; > + }; > + > mmss_smmu: iommu@cd00000 { > compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; > reg = <0x0cd00000 0x40000>; > diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c > index ce206b7097541..42e0c580e093d 100644 > --- a/drivers/media/platform/qcom/venus/core.c > +++ b/drivers/media/platform/qcom/venus/core.c > @@ -587,6 +587,47 @@ static const struct venus_resources msm8996_res = { > .fwname = "qcom/venus-4.2/venus.mbn", > }; > > +static const struct freq_tbl msm8998_freq_table[] = { > + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ > + { 972000, 520000000 }, /* 4k UHD @ 30 */ > + { 489600, 346666667 }, /* 1080p @ 60 */ > + { 244800, 150000000 }, /* 1080p @ 30 */ > + { 108000, 75000000 }, /* 720p @ 30 */ > +}; > + > +/* > + * https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-vidc.dtsi > + */ > +static const struct reg_val msm8998_reg_preset[] = { > + { 0x80124, 0x00000003 }, > + { 0x80550, 0x01111111 }, > + { 0x80560, 0x01111111 }, > + { 0x80568, 0x01111111 }, > + { 0x80570, 0x01111111 }, > + { 0x80580, 0x01111111 }, > + { 0x80588, 0x01111111 }, > + { 0xe2010, 0x00000000 }, > +}; > + > +static const struct venus_resources msm8998_res = { > + .freq_tbl = msm8998_freq_table, > + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), > + .reg_tbl = msm8998_reg_preset, > + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), > + .clks = { "core", "iface", "bus", "mbus" }, > + .clks_num = 4, > + .vcodec0_clks = { "core" }, > + .vcodec1_clks = { "core" }, > + .vcodec_clks_num = 1, > + .max_load = 2563200, > + .hfi_version = HFI_VERSION_3XX, > + .vmem_id = VIDC_RESOURCE_NONE, > + .vmem_size = 0, > + .vmem_addr = 0, > + .dma_mask = 0xddc00000 - 1, > + .fwname = "qcom/venus-4.4/venus.mbn", > +}; > + > static const struct freq_tbl sdm660_freq_table[] = { > { 979200, 518400000 }, > { 489600, 441600000 }, > @@ -893,6 +934,7 @@ static const struct venus_resources sc7280_res = { > static const struct of_device_id venus_dt_match[] = { > { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, > { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, > + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, > { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, > { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, > { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, > diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c > index f9437b6412b91..abdc578ce988e 100644 > --- a/drivers/media/platform/qcom/venus/hfi_venus.c > +++ b/drivers/media/platform/qcom/venus/hfi_venus.c > @@ -945,6 +945,7 @@ static int venus_sys_set_default_properties(struct venus_hfi_device *hdev) > dev_warn(dev, "setting idle response ON failed (%d)\n", ret); > } > > + venus_fw_low_power_mode = false; So instead of this workaround, @Vikash is asking for HW_CTRL similar to what we have in 8996 8996 has a top-level "magic" GDSC which 8998 doesn't appear to have. I think the CXC stuff would still be valid. diff --git a/drivers/clk/qcom/mmcc-msm8998.c b/drivers/clk/qcom/mmcc-msm8998.c index 1180e48c687ac..275fb3b71ede4 100644 --- a/drivers/clk/qcom/mmcc-msm8998.c +++ b/drivers/clk/qcom/mmcc-msm8998.c @@ -2535,6 +2535,8 @@ static struct clk_branch vmem_ahb_clk = { static struct gdsc video_top_gdsc = { .gdscr = 0x1024, + .cxcs = (unsigned int []){ 0x1028, 0x1034, 0x1038 }, + .cxc_count = 3, .pd = { .name = "video_top", }, @@ -2543,20 +2545,26 @@ static struct gdsc video_top_gdsc = { static struct gdsc video_subcore0_gdsc = { .gdscr = 0x1040, + .cxcs = (unsigned int []){ 0x1048 }, + .cxc_count = 1, .pd = { .name = "video_subcore0", }, .parent = &video_top_gdsc.pd, .pwrsts = PWRSTS_OFF_ON, + .flags = HW_CTRL, }; static struct gdsc video_subcore1_gdsc = { .gdscr = 0x1044, + .cxcs = (unsigned int []){ 0x104c }, + .cxc_count = 1, .pd = { .name = "video_subcore1", }, .parent = &video_top_gdsc.pd, .pwrsts = PWRSTS_OFF_ON, + .flags = HW_CTRL, }; Can you give it a try ? --- bod
On 09/04/2024 13:27, Bryan O'Donoghue wrote: > On 08/04/2024 16:39, Marc Gonzalez wrote: > >> On 29/02/2024 16:32, Vikash Garodia wrote: >> >>> Not completely sure on these configurations, but certainly both the >>> video_subcore0_gdsc and video_subcore1_gdsc should be configured in >>> hardware control mode in the gdsc configuration. >> >> Still trying to land support for venus decoder on msm8998 in mainline. >> >> This is the patch I have at the moment: >> >> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi >> index 4dfe2d09ac285..67b8374ddf02f 100644 >> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi >> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi >> @@ -3010,6 +3010,55 @@ mdss_dsi1_phy: phy@c996400 { >> }; >> }; >> >> + venus: video-codec@cc00000 { >> + compatible = "qcom,msm8998-venus"; >> + reg = <0x0cc00000 0xff000>; >> + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; >> + power-domains = <&mmcc VIDEO_TOP_GDSC>; >> + clocks = <&mmcc VIDEO_CORE_CLK>, >> + <&mmcc VIDEO_AHB_CLK>, >> + <&mmcc VIDEO_AXI_CLK>, >> + <&mmcc VIDEO_MAXI_CLK>; >> + clock-names = "core", "iface", "bus", "mbus"; >> + iommus = <&mmss_smmu 0x400>, >> + <&mmss_smmu 0x401>, >> + <&mmss_smmu 0x40a>, >> + <&mmss_smmu 0x407>, >> + <&mmss_smmu 0x40e>, >> + <&mmss_smmu 0x40f>, >> + <&mmss_smmu 0x408>, >> + <&mmss_smmu 0x409>, >> + <&mmss_smmu 0x40b>, >> + <&mmss_smmu 0x40c>, >> + <&mmss_smmu 0x40d>, >> + <&mmss_smmu 0x410>, >> + <&mmss_smmu 0x411>, >> + <&mmss_smmu 0x421>, >> + <&mmss_smmu 0x428>, >> + <&mmss_smmu 0x429>, >> + <&mmss_smmu 0x42b>, >> + <&mmss_smmu 0x42c>, >> + <&mmss_smmu 0x42d>, >> + <&mmss_smmu 0x411>, >> + <&mmss_smmu 0x431>; >> + memory-region = <&venus_mem>; >> + status = "disabled"; >> + >> + video-decoder { >> + compatible = "venus-decoder"; >> + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; >> + clock-names = "core"; >> + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; >> + }; >> + >> + video-encoder { >> + compatible = "venus-encoder"; >> + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; >> + clock-names = "core"; >> + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; >> + }; >> + }; >> + >> mmss_smmu: iommu@cd00000 { >> compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; >> reg = <0x0cd00000 0x40000>; >> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c >> index ce206b7097541..42e0c580e093d 100644 >> --- a/drivers/media/platform/qcom/venus/core.c >> +++ b/drivers/media/platform/qcom/venus/core.c >> @@ -587,6 +587,47 @@ static const struct venus_resources msm8996_res = { >> .fwname = "qcom/venus-4.2/venus.mbn", >> }; >> >> +static const struct freq_tbl msm8998_freq_table[] = { >> + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ >> + { 972000, 520000000 }, /* 4k UHD @ 30 */ >> + { 489600, 346666667 }, /* 1080p @ 60 */ >> + { 244800, 150000000 }, /* 1080p @ 30 */ >> + { 108000, 75000000 }, /* 720p @ 30 */ >> +}; >> + >> +/* >> + * https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-vidc.dtsi >> + */ >> +static const struct reg_val msm8998_reg_preset[] = { >> + { 0x80124, 0x00000003 }, >> + { 0x80550, 0x01111111 }, >> + { 0x80560, 0x01111111 }, >> + { 0x80568, 0x01111111 }, >> + { 0x80570, 0x01111111 }, >> + { 0x80580, 0x01111111 }, >> + { 0x80588, 0x01111111 }, >> + { 0xe2010, 0x00000000 }, >> +}; >> + >> +static const struct venus_resources msm8998_res = { >> + .freq_tbl = msm8998_freq_table, >> + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), >> + .reg_tbl = msm8998_reg_preset, >> + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), >> + .clks = { "core", "iface", "bus", "mbus" }, >> + .clks_num = 4, >> + .vcodec0_clks = { "core" }, >> + .vcodec1_clks = { "core" }, >> + .vcodec_clks_num = 1, >> + .max_load = 2563200, >> + .hfi_version = HFI_VERSION_3XX, >> + .vmem_id = VIDC_RESOURCE_NONE, >> + .vmem_size = 0, >> + .vmem_addr = 0, >> + .dma_mask = 0xddc00000 - 1, >> + .fwname = "qcom/venus-4.4/venus.mbn", >> +}; >> + >> static const struct freq_tbl sdm660_freq_table[] = { >> { 979200, 518400000 }, >> { 489600, 441600000 }, >> @@ -893,6 +934,7 @@ static const struct venus_resources sc7280_res = { >> static const struct of_device_id venus_dt_match[] = { >> { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, >> { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, >> + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, >> { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, >> { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, >> { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, >> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c >> index f9437b6412b91..abdc578ce988e 100644 >> --- a/drivers/media/platform/qcom/venus/hfi_venus.c >> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c >> @@ -945,6 +945,7 @@ static int venus_sys_set_default_properties(struct venus_hfi_device *hdev) >> dev_warn(dev, "setting idle response ON failed (%d)\n", ret); >> } >> >> + venus_fw_low_power_mode = false; > > So instead of this workaround, @Vikash is asking for HW_CTRL similar to > what we have in 8996 > > 8996 has a top-level "magic" GDSC which 8998 doesn't appear to have. > > I think the CXC stuff would still be valid. > > diff --git a/drivers/clk/qcom/mmcc-msm8998.c > b/drivers/clk/qcom/mmcc-msm8998.c > index 1180e48c687ac..275fb3b71ede4 100644 > --- a/drivers/clk/qcom/mmcc-msm8998.c > +++ b/drivers/clk/qcom/mmcc-msm8998.c > @@ -2535,6 +2535,8 @@ static struct clk_branch vmem_ahb_clk = { > > static struct gdsc video_top_gdsc = { > .gdscr = 0x1024, > + .cxcs = (unsigned int []){ 0x1028, 0x1034, 0x1038 }, > + .cxc_count = 3, > .pd = { > .name = "video_top", > }, > @@ -2543,20 +2545,26 @@ static struct gdsc video_top_gdsc = { > > static struct gdsc video_subcore0_gdsc = { > .gdscr = 0x1040, > + .cxcs = (unsigned int []){ 0x1048 }, > + .cxc_count = 1, > .pd = { > .name = "video_subcore0", > }, > .parent = &video_top_gdsc.pd, > .pwrsts = PWRSTS_OFF_ON, > + .flags = HW_CTRL, > }; > > static struct gdsc video_subcore1_gdsc = { > .gdscr = 0x1044, > + .cxcs = (unsigned int []){ 0x104c }, > + .cxc_count = 1, > .pd = { > .name = "video_subcore1", > }, > .parent = &video_top_gdsc.pd, > .pwrsts = PWRSTS_OFF_ON, > + .flags = HW_CTRL, > }; > > Can you give it a try ? Looks like your patch DOES fix the issue! References (mostly) for myself static struct gdsc video_top_gdsc = { .gdscr = 0x1024, .cxcs = (unsigned int []){ 0x1028, 0x1034, 0x1038 }, static struct gdsc video_subcore0_gdsc = { .gdscr = 0x1040, .cxcs = (unsigned int []){ 0x1048 }, static struct gdsc video_subcore1_gdsc = { .gdscr = 0x1044, .cxcs = (unsigned int []){ 0x104c }, GDSCR = Globally Distributed Switch Controller Register https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/include/dt-bindings/clock/msm-clocks-hwio-8998.h 0x1024 = undocumented = MMSS_VIDEO GDSCR 0x1028 = MMSS_VIDEO_CORE_CBCR 0x1030 = MMSS_VIDEO_AHB_CBCR 0x1034 = MMSS_VIDEO_AXI_CBCR 0x1038 = MMSS_VIDEO_MAXI_CBCR 0x1040 = undocumented = MMSS_VIDEO_SUBCORE0 GDSCR 0x1044 = undocumented = MMSS_VIDEO_SUBCORE1 GDSCR 0x1048 = MMSS_VIDEO_SUBCORE0_CBCR 0x104c = MMSS_VIDEO_SUBCORE1_CBCR On msm8996 static struct gdsc venus_gdsc = { .gdscr = 0x1024, .cxcs = (unsigned int []){ 0x1028, 0x1034, 0x1038 }, static struct gdsc venus_core0_gdsc = { .gdscr = 0x1040, .cxcs = (unsigned int []){ 0x1048 }, static struct gdsc venus_core1_gdsc = { .gdscr = 0x1044, .cxcs = (unsigned int []){ 0x104c }, https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/include/dt-bindings/clock/msm-clocks-hwio-8996.h 0x1028 = MMSS_VIDEO_CORE_CBCR = MMSS_VIDEO_CXO_CBCR 0x1030 = MMSS_VIDEO_AHB_CBCR 0x1034 = MMSS_VIDEO_AXI_CBCR 0x1038 = MMSS_VIDEO_MAXI_CBCR 0x1048 = MMSS_VIDEO_SUBCORE0_CBCR 0x104c = MMSS_VIDEO_SUBCORE1_CBCR Registers of interest are mapped identically in msm8996 & msm8998. Therefore, it makes sense for the code to be identical. Regards
On 09/04/2024 13:27, Bryan O'Donoghue wrote:
> Can you give it a try ?
Random notes
For easy reference, I've used this command to test:
$ mpv --hwdec=v4l2m2m-copy --vo=tct --quiet --no-audio demo-480.webm
And it displays the video directly in the terminal :)
(Rendering speed depends on terminal size)
I'd never played the video to the end.
I notice I get:
[ 397.410006] qcom-venus cc00000.video-codec: session error: event id:1001 (deadb000), session id:79d7c000
[ 397.410114] qcom-venus-decoder cc00000.video-codec:video-decoder: dec: event session error 1001
How bad is that?
Sometimes, decoding simply fails immediately.
Must quit & restart.
Will have to script a 100 starts and check frequency of failures.
Will test with higher-resolution video.
# time mpv --hwdec=v4l2m2m-copy --vo=null --quiet --no-audio --untimed demo-480.webm
(+) Video --vid=1 (*) (vp9 854x480 29.970fps)
Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz)
[ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl
[ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl
[ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl
[ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl
[ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl
[ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl
[ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl
[ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl
[ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl
[ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl
[ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl
Using hardware decoding (v4l2m2m-copy).
VO: [null] 854x480 nv12
[ffmpeg/video] vp9_v4l2m2m: capture POLLERR
Exiting... (Quit)
/*** HANGS UNTIL CTRL-C ***/
real 0m21.467s
user 0m3.795s
sys 0m1.914s
# time mpv --hwdec=v4l2m2m-copy --vo=null --quiet --no-audio --untimed --length=30 demo-1440.webm
(+) Video --vid=1 (*) (vp9 2560x1440 59.940fps)
Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz)
[ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl
Using hardware decoding (v4l2m2m-copy).
VO: [null] 2560x1440 nv12
Exiting... (End of file)
real 0m16.433s
user 0m1.764s
sys 0m1.118s
Regards
On 09/04/2024 17:53, Marc Gonzalez wrote: > On 09/04/2024 13:27, Bryan O'Donoghue wrote: > >> Can you give it a try ? > > Random notes > > For easy reference, I've used this command to test: > > $ mpv --hwdec=v4l2m2m-copy --vo=tct --quiet --no-audio demo-480.webm > > And it displays the video directly in the terminal :) > (Rendering speed depends on terminal size) > > I'd never played the video to the end. > I notice I get: > > [ 397.410006] qcom-venus cc00000.video-codec: session error: event id:1001 (deadb000), session id:79d7c000 > [ 397.410114] qcom-venus-decoder cc00000.video-codec:video-decoder: dec: event session error 1001 > > How bad is that? > > > Sometimes, decoding simply fails immediately. > Must quit & restart. > Will have to script a 100 starts and check frequency of failures. > > > Will test with higher-resolution video. > > # time mpv --hwdec=v4l2m2m-copy --vo=null --quiet --no-audio --untimed demo-480.webm > (+) Video --vid=1 (*) (vp9 854x480 29.970fps) > Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > Using hardware decoding (v4l2m2m-copy). > VO: [null] 854x480 nv12 > [ffmpeg/video] vp9_v4l2m2m: capture POLLERR > Exiting... (Quit) > /*** HANGS UNTIL CTRL-C ***/ I think there are a number of different resolutions across SoCs that will exhibit this behavior, I've seen it with lower resolutions on 8250. Its a bug that we need to drill into in Venus but I don't think is a bug that is specific to your setup. > > real 0m21.467s > user 0m3.795s > sys 0m1.914s > > > # time mpv --hwdec=v4l2m2m-copy --vo=null --quiet --no-audio --untimed --length=30 demo-1440.webm > (+) Video --vid=1 (*) (vp9 2560x1440 59.940fps) > Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > Using hardware decoding (v4l2m2m-copy). > VO: [null] 2560x1440 nv12 > Exiting... (End of file) > > real 0m16.433s > user 0m1.764s > sys 0m1.118s If this higher resolution is stable for you, I'd say this is about baseline. 1. The GDSC change should make no impact on playback or available resolution 2. Higher more "normal" use cases like 1080p should be fine. If so, then file "low resolution is broken" under a "known unknown" and scrub your patches for submission. If not, we need to do more 8998 specific debug. --- bod
On 3/13/2024 12:09 AM, Konrad Dybcio wrote: > > > On 2/29/24 17:24, Marc Gonzalez wrote: >> On 29/02/2024 16:32, Vikash Garodia wrote: >> >>> On 2/27/2024 9:41 PM, Marc Gonzalez wrote: >>> >>>> On 27/02/2024 07:55, Vikash Garodia wrote: >>>> >>>>> On 2/26/2024 9:25 PM, Marc Gonzalez wrote: >>>>> >>>>>> Errr, there is currently no existing node for msm8998-venus? >>>>> >>>>> My bad, i meant your initial node msm8998-venus, without migrating to v2. >>>>> >>>>>> With the proposed node above (based on msm8996-venus) >>>>>> AND the proposed work-around disabling low-power mode, >>>>>> decoding works correctly. >>>>> >>>>> Nice, lets fix the work-around part before migrating to v2. Could you share >>>>> the >>>>> configurations for VIDEO_SUBCORE0_GDSC and VIDEO_SUBCORE1_GDSC ? >>>>> >>>>> If we see vendor code[1], sys power plane control is very much supported, so >>>>> ideally we should get it working without the workaround >>>>> [1] >>>>> https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/drivers/media/platform/msm/vidc/venus_hfi.c#L2223 >>>> >>>> OK, for easy reference, here are the proposed changes again: >>>> >>>> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi >>>> b/arch/arm64/boot/dts/qcom/msm8998.dtsi >>>> index 2793cc22d381a..5084191be1446 100644 >>>> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi >>>> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi >>>> @@ -3000,6 +3000,56 @@ mdss_dsi1_phy: phy@c996400 { >>>> }; >>>> }; >>>> + venus: video-codec@cc00000 { >>>> + compatible = "qcom,msm8998-venus"; >>>> + reg = <0x0cc00000 0xff000>; >>>> + interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; >>>> + power-domains = <&mmcc VIDEO_TOP_GDSC>; >>>> + clocks = <&mmcc VIDEO_CORE_CLK>, >>>> + <&mmcc VIDEO_AHB_CLK>, >>>> + <&mmcc VIDEO_AXI_CLK>, >>>> + <&mmcc VIDEO_MAXI_CLK>; >>>> + clock-names = "core", "iface", "bus", "mbus"; >>>> + iommus = <&mmss_smmu 0x400>, >>>> + <&mmss_smmu 0x401>, >>>> + <&mmss_smmu 0x40a>, >>>> + <&mmss_smmu 0x407>, >>>> + <&mmss_smmu 0x40e>, >>>> + <&mmss_smmu 0x40f>, >>>> + <&mmss_smmu 0x408>, >>>> + <&mmss_smmu 0x409>, >>>> + <&mmss_smmu 0x40b>, >>>> + <&mmss_smmu 0x40c>, >>>> + <&mmss_smmu 0x40d>, >>>> + <&mmss_smmu 0x410>, >>>> + <&mmss_smmu 0x411>, >>>> + <&mmss_smmu 0x421>, >>>> + <&mmss_smmu 0x428>, >>>> + <&mmss_smmu 0x429>, >>>> + <&mmss_smmu 0x42b>, >>>> + <&mmss_smmu 0x42c>, >>>> + <&mmss_smmu 0x42d>, >>>> + <&mmss_smmu 0x411>, >>>> + <&mmss_smmu 0x431>; >>>> + memory-region = <&venus_mem>; >>>> + status = "disabled"; >>>> + qcom,venus-broken-low-power-mode; >>>> + >>>> + video-decoder { >>>> + compatible = "venus-decoder"; >>>> + clocks = <&mmcc VIDEO_SUBCORE0_CLK>; >>>> + clock-names = "core"; >>>> + power-domains = <&mmcc VIDEO_SUBCORE0_GDSC>; >>>> + }; >>>> + >>>> + video-encoder { >>>> + compatible = "venus-encoder"; >>>> + clocks = <&mmcc VIDEO_SUBCORE1_CLK>; >>>> + clock-names = "core"; >>>> + power-domains = <&mmcc VIDEO_SUBCORE1_GDSC>; >>>> + }; >>>> + }; >>>> + >>>> mmss_smmu: iommu@cd00000 { >>>> compatible = "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; >>>> reg = <0x0cd00000 0x40000>; >>>> diff --git a/drivers/media/platform/qcom/venus/core.c >>>> b/drivers/media/platform/qcom/venus/core.c >>>> index a712dd4f02a5b..ad1705e510312 100644 >>>> --- a/drivers/media/platform/qcom/venus/core.c >>>> +++ b/drivers/media/platform/qcom/venus/core.c >>>> @@ -585,6 +585,43 @@ static const struct venus_resources msm8996_res = { >>>> .fwname = "qcom/venus-4.2/venus.mbn", >>>> }; >>>> +static const struct freq_tbl msm8998_freq_table[] = { >>>> + { 1944000, 520000000 }, /* 4k UHD @ 60 (decode only) */ >>>> + { 972000, 520000000 }, /* 4k UHD @ 30 */ >>>> + { 489600, 346666667 }, /* 1080p @ 60 */ >>>> + { 244800, 150000000 }, /* 1080p @ 30 */ >>>> + { 108000, 75000000 }, /* 720p @ 30 */ >>>> +}; >>>> + >>>> +static const struct reg_val msm8998_reg_preset[] = { >>>> + { 0x80124, 0x00000003 }, >>>> + { 0x80550, 0x01111111 }, >>>> + { 0x80560, 0x01111111 }, >>>> + { 0x80568, 0x01111111 }, >>>> + { 0x80570, 0x01111111 }, >>>> + { 0x80580, 0x01111111 }, >>>> + { 0xe2010, 0x00000000 }, >>>> +}; >>>> + >>>> +static const struct venus_resources msm8998_res = { >>>> + .freq_tbl = msm8998_freq_table, >>>> + .freq_tbl_size = ARRAY_SIZE(msm8998_freq_table), >>>> + .reg_tbl = msm8998_reg_preset, >>>> + .reg_tbl_size = ARRAY_SIZE(msm8998_reg_preset), >>>> + .clks = {"core", "iface", "bus", "mbus"}, >>>> + .clks_num = 4, >>>> + .vcodec0_clks = { "core" }, >>>> + .vcodec1_clks = { "core" }, >>>> + .vcodec_clks_num = 1, >>>> + .max_load = 2563200, >>>> + .hfi_version = HFI_VERSION_3XX, >>>> + .vmem_id = VIDC_RESOURCE_NONE, >>>> + .vmem_size = 0, >>>> + .vmem_addr = 0, >>>> + .dma_mask = 0xddc00000 - 1, >>>> + .fwname = "qcom/venus-4.4/venus.mbn", >>>> +}; >>>> + >>>> static const struct freq_tbl sdm660_freq_table[] = { >>>> { 979200, 518400000 }, >>>> { 489600, 441600000 }, >>>> @@ -891,6 +928,7 @@ static const struct venus_resources sc7280_res = { >>>> static const struct of_device_id venus_dt_match[] = { >>>> { .compatible = "qcom,msm8916-venus", .data = &msm8916_res, }, >>>> { .compatible = "qcom,msm8996-venus", .data = &msm8996_res, }, >>>> + { .compatible = "qcom,msm8998-venus", .data = &msm8998_res, }, >>>> { .compatible = "qcom,sdm660-venus", .data = &sdm660_res, }, >>>> { .compatible = "qcom,sdm845-venus", .data = &sdm845_res, }, >>>> { .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, }, >>>> >>>> >>>> >>>> This patch is on top of v6.8-rc1 >>>> so the configurations for VIDEO_SUBCOREx_GDSC >>>> are as defined in mainline. >>>> >>>> #define VIDEO_SUBCORE0_CLK_SRC 51 >>>> #define VIDEO_SUBCORE1_CLK_SRC 52 >>>> >>>> #define VIDEO_TOP_GDSC 1 >>>> #define VIDEO_SUBCORE0_GDSC 2 >>>> #define VIDEO_SUBCORE1_GDSC 3 >>>> >>>> https://github.com/torvalds/linux/blob/master/drivers/clk/qcom/mmcc-msm8998.c#L2536-L2561 >>>> >>>> static struct gdsc video_top_gdsc = { >>>> .gdscr = 0x1024, >>>> .pd = { >>>> .name = "video_top", >>>> }, >>>> .pwrsts = PWRSTS_OFF_ON, >>>> }; >>>> >>>> static struct gdsc video_subcore0_gdsc = { >>>> .gdscr = 0x1040, >>>> .pd = { >>>> .name = "video_subcore0", >>>> }, >>>> .parent = &video_top_gdsc.pd, >>>> .pwrsts = PWRSTS_OFF_ON, >>>> }; >>>> >>>> static struct gdsc video_subcore1_gdsc = { >>>> .gdscr = 0x1044, >>>> .pd = { >>>> .name = "video_subcore1", >>>> }, >>>> .parent = &video_top_gdsc.pd, >>>> .pwrsts = PWRSTS_OFF_ON, >>>> }; >>>> >>>> >>>> const u8 pwrsts; >>>> /* Powerdomain allowable state bitfields */ >>>> #define PWRSTS_OFF BIT(0) >>>> /* >>>> * There is no SW control to transition a GDSC into >>>> * PWRSTS_RET. This happens in HW when the parent >>>> * domain goes down to a low power state >>>> */ >>>> #define PWRSTS_RET BIT(1) >>>> #define PWRSTS_ON BIT(2) >>>> #define PWRSTS_OFF_ON (PWRSTS_OFF | PWRSTS_ON) >>>> #define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON) >>>> const u16 flags; >>>> #define VOTABLE BIT(0) >>>> #define CLAMP_IO BIT(1) >>>> #define HW_CTRL BIT(2) >>>> #define SW_RESET BIT(3) >>>> #define AON_RESET BIT(4) >>>> #define POLL_CFG_GDSCR BIT(5) >>>> #define ALWAYS_ON BIT(6) >>>> #define RETAIN_FF_ENABLE BIT(7) >>>> #define NO_RET_PERIPH BIT(8) >>>> >>>> >>>> Should .pwrsts be PWRSTS_RET_ON instead of PWRSTS_OFF_ON? >>>> >>>> Should .flags be HW_CTRL instead of 0? >>> >>> Not completely sure on these configurations, but certainly both the >>> video_subcore0_gdsc and video_subcore1_gdsc should be configured in hardware >>> control mode in the gdsc configuration. >> >> Jeffrey, Bjorn, >> >> I'm trying to get mainline support for the msm8998 video decoder (venus). >> Apparently, there appears to be an issue with the multimedia clocks. >> >> Do you remember why you chose PWRSTS_OFF_ON instead of PWRSTS_RET_ON? > > Doing a quick reconnaissance against msm-4.4, PWRSTS_OFF_ON looks > very sane. > > Moreover, PWRSTS_RET_ON very much only looks useful as a hack for > non-fully-implemented drivers (and that would indeed match the state > of our usb and pcie driver today) - it prevents GDSC shutdown, but > tells the PM framework that the registered power domain has collapsed > >> >> And why the HW_CTRL bit is not set? > > HW_CTRL means "totally hand over the control of this GDSC to the > hardware" where "hardware" is very loosely defined.. > > Reading msm-4.4 again, it seems like we want HW_CTRL mode to be > enabled if and only if the low power property has been set through > the venus firmware interface. For video hardware, there are 2 steps for this: 1. Inform video firmware that the GDSC is in HW control. This is done via HFI 2. Switch GDSC mode runtime during the usecase as SW or HW mode. This is currently done via programming few power control video hardware registers directly. AFAIU, if the GDSC flag configuration has HW_CTRL (in file mmcc-msm8998.c) and the HFI in #1 above indicates the same, then all should be good. Regards, Vikash > > More particularly, we don't want it to be there once we wanna shut > down Venus for good. This is being worked on by folks over at [1]. > > https://lore.kernel.org/linux-arm-msm/20240122-gdsc-hwctrl-v4-0-9061e8a7aa07@linaro.org/ > > Konrad
On 4/10/2024 1:47 PM, Bryan O'Donoghue wrote: > On 09/04/2024 17:53, Marc Gonzalez wrote: >> On 09/04/2024 13:27, Bryan O'Donoghue wrote: >> >>> Can you give it a try ? >> >> Random notes >> >> For easy reference, I've used this command to test: >> >> $ mpv --hwdec=v4l2m2m-copy --vo=tct --quiet --no-audio demo-480.webm >> >> And it displays the video directly in the terminal :) >> (Rendering speed depends on terminal size) >> >> I'd never played the video to the end. >> I notice I get: >> >> [ 397.410006] qcom-venus cc00000.video-codec: session error: event id:1001 >> (deadb000), session id:79d7c000 >> [ 397.410114] qcom-venus-decoder cc00000.video-codec:video-decoder: dec: >> event session error 1001 >> >> How bad is that? >> >> >> Sometimes, decoding simply fails immediately. >> Must quit & restart. >> Will have to script a 100 starts and check frequency of failures. >> >> >> Will test with higher-resolution video. We were writing sample test application [1] to test video usecase and more specifically with user driven inputs. Incase this help in your trial. I haven't yet tried this on venus driver and used it mainly for the some other v4l video driver, but i can expect it to work given it is v4l interface based. It has few sample config which can dump the decoded output to validate. Ignore if this adds to your effort to validate from this app, will publish once we try on venus driver. Regards, Vikash [1] https://github.com/quic/v4l-video-test-app >> >> # time mpv --hwdec=v4l2m2m-copy --vo=null --quiet --no-audio --untimed >> demo-480.webm >> (+) Video --vid=1 (*) (vp9 854x480 29.970fps) >> Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> Using hardware decoding (v4l2m2m-copy). >> VO: [null] 854x480 nv12 >> [ffmpeg/video] vp9_v4l2m2m: capture POLLERR >> Exiting... (Quit) >> /*** HANGS UNTIL CTRL-C ***/ > > I think there are a number of different resolutions across SoCs that will > exhibit this behavior, I've seen it with lower resolutions on 8250. > > Its a bug that we need to drill into in Venus but I don't think is a bug that is > specific to your setup. > >> >> real 0m21.467s >> user 0m3.795s >> sys 0m1.914s >> >> >> # time mpv --hwdec=v4l2m2m-copy --vo=null --quiet --no-audio --untimed >> --length=30 demo-1440.webm >> (+) Video --vid=1 (*) (vp9 2560x1440 59.940fps) >> Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> Using hardware decoding (v4l2m2m-copy). >> VO: [null] 2560x1440 nv12 >> Exiting... (End of file) >> >> real 0m16.433s >> user 0m1.764s >> sys 0m1.118s > > If this higher resolution is stable for you, I'd say this is about baseline. > > 1. The GDSC change should make no impact on playback or available resolution > > 2. Higher more "normal" use cases like 1080p should be fine. > > If so, then file "low resolution is broken" under a "known unknown" and scrub > your patches for submission. > > If not, we need to do more 8998 specific debug. > > --- > bod
On 10/04/2024 10:17, Bryan O'Donoghue wrote: > On 09/04/2024 17:53, Marc Gonzalez wrote: > >> On 09/04/2024 13:27, Bryan O'Donoghue wrote: >> >>> Can you give it a try ? >> >> Random notes >> >> For easy reference, I've used this command to test: >> >> $ mpv --hwdec=v4l2m2m-copy --vo=tct --quiet --no-audio demo-480.webm >> >> And it displays the video directly in the terminal :) >> (Rendering speed depends on terminal size) >> >> I'd never played the video to the end. >> I notice I get: >> >> [ 397.410006] qcom-venus cc00000.video-codec: session error: event id:1001 (deadb000), session id:79d7c000 >> [ 397.410114] qcom-venus-decoder cc00000.video-codec:video-decoder: dec: event session error 1001 >> >> How bad is that? >> >> >> Sometimes, decoding simply fails immediately. >> Must quit & restart. >> Will have to script a 100 starts and check frequency of failures. >> >> >> Will test with higher-resolution video. >> >> # time mpv --hwdec=v4l2m2m-copy --vo=null --quiet --no-audio --untimed demo-480.webm >> (+) Video --vid=1 (*) (vp9 854x480 29.970fps) >> Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> Using hardware decoding (v4l2m2m-copy). >> VO: [null] 854x480 nv12 >> [ffmpeg/video] vp9_v4l2m2m: capture POLLERR >> Exiting... (Quit) >> /*** HANGS UNTIL CTRL-C ***/ > > I think there are a number of different resolutions across SoCs that > will exhibit this behavior, I've seen it with lower resolutions on 8250. > > Its a bug that we need to drill into in Venus but I don't think is a bug > that is specific to your setup. > >> >> real 0m21.467s >> user 0m3.795s >> sys 0m1.914s >> >> >> # time mpv --hwdec=v4l2m2m-copy --vo=null --quiet --no-audio --untimed --length=30 demo-1440.webm >> (+) Video --vid=1 (*) (vp9 2560x1440 59.940fps) >> Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) >> [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl >> Using hardware decoding (v4l2m2m-copy). >> VO: [null] 2560x1440 nv12 >> Exiting... (End of file) >> >> real 0m16.433s >> user 0m1.764s >> sys 0m1.118s > > If this higher resolution is stable for you, I'd say this is about baseline. > > 1. The GDSC change should make no impact on playback or available resolution > > 2. Higher more "normal" use cases like 1080p should be fine. > > If so, then file "low resolution is broken" under a "known unknown" and > scrub your patches for submission. > > If not, we need to do more 8998 specific debug. FWIW, I get the same behavior with 854x480 and 2560x1440: 1) If mpv runs with '--length=N' (play only part of the file) then mpv exits cleanly, with no kernel messages. 2) If mpv plays the entire file, then mpv hangs at the end (needs CTRL-C to exit) and driver prints to kernel: [68643.935888] qcom-venus cc00000.video-codec: session error: event id:1001 (deadb000), session id:79d42000 [68643.935995] qcom-venus-decoder cc00000.video-codec:video-decoder: dec: event session error 1001 # time mpv --hwdec=v4l2m2m-copy --vo=null --quiet --no-audio --untimed demo-1440.webm (+) Video --vid=1 (*) (vp9 2560x1440 59.940fps) Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl Using hardware decoding (v4l2m2m-copy). VO: [null] 2560x1440 nv12 [ffmpeg/video] vp9_v4l2m2m: capture POLLERR Exiting... (Quit) real 2m43.292s user 0m9.204s sys 0m4.591s # time mpv --hwdec=v4l2m2m-copy --vo=null --quiet --no-audio --untimed --length=300 demo-1440.webm (+) Video --vid=1 (*) (vp9 2560x1440 59.940fps) Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl Using hardware decoding (v4l2m2m-copy). VO: [null] 2560x1440 nv12 Exiting... (End of file) real 2m32.412s user 0m8.401s sys 0m4.917s 300 seconds decoded in 152 seconds = 2x for 1440p. Will send the patch series in a few hours. Regards
On 10/04/2024 13:23, Marc Gonzalez wrote: > FWIW, I get the same behavior with 854x480 and 2560x1440: > > 1) If mpv runs with '--length=N' (play only part of the file) > then mpv exits cleanly, with no kernel messages. On -next ? I think you mentioned before you were doing your work on an older kernel and forward porting patches ? > 2) If mpv plays the entire file, then mpv hangs at the end > (needs CTRL-C to exit) and driver prints to kernel: > [68643.935888] qcom-venus cc00000.video-codec: session error: event id:1001 (deadb000), session id:79d42000 > [68643.935995] qcom-venus-decoder cc00000.video-codec:video-decoder: dec: event : 1001 Hmm #define HFI_ERR_SESSION_FATAL 0x1001 Something is causing the firmware to return this packet to you. Probably worth tracing the last five messages sent by the firmware prior to that to see if we can root-cause. --- bod
On 10/04/2024 15:14, Bryan O'Donoghue wrote: > On 10/04/2024 13:23, Marc Gonzalez wrote: > >> FWIW, I get the same behavior with 854x480 and 2560x1440: >> >> 1) If mpv runs with '--length=N' (play only part of the file) >> then mpv exits cleanly, with no kernel messages. > > On -next ? > > I think you mentioned before you were doing your work on an older kernel > and forward porting patches ? I work on v6.9-rc1 Is -next that much different in that area? >> 2) If mpv plays the entire file, then mpv hangs at the end >> (needs CTRL-C to exit) and driver prints to kernel: >> [68643.935888] qcom-venus cc00000.video-codec: session error: event id:1001 (deadb000), session id:79d42000 >> [68643.935995] qcom-venus-decoder cc00000.video-codec:video-decoder: dec: event : 1001 > > Hmm > > #define HFI_ERR_SESSION_FATAL 0x1001 > > Something is causing the firmware to return this packet to you. > > Probably worth tracing the last five messages sent by the firmware prior > to that to see if we can root-cause. I'm not seeing anything from the FW in dmesg. I suppose I need to enable DEBUG in various places? Regards
On 10/04/2024 14:18, Marc Gonzalez wrote: > On 10/04/2024 15:14, Bryan O'Donoghue wrote: > >> On 10/04/2024 13:23, Marc Gonzalez wrote: >> >>> FWIW, I get the same behavior with 854x480 and 2560x1440: >>> >>> 1) If mpv runs with '--length=N' (play only part of the file) >>> then mpv exits cleanly, with no kernel messages. >> >> On -next ? >> >> I think you mentioned before you were doing your work on an older kernel >> and forward porting patches ? > > I work on v6.9-rc1 > Is -next that much different in that area? No, I thought you were on a 4.x kernel for some reason. 6.9.x is fine > >>> 2) If mpv plays the entire file, then mpv hangs at the end >>> (needs CTRL-C to exit) and driver prints to kernel: >>> [68643.935888] qcom-venus cc00000.video-codec: session error: event id:1001 (deadb000), session id:79d42000 >>> [68643.935995] qcom-venus-decoder cc00000.video-codec:video-decoder: dec: event : 1001 >> >> Hmm >> >> #define HFI_ERR_SESSION_FATAL 0x1001 >> >> Something is causing the firmware to return this packet to you. >> >> Probably worth tracing the last five messages sent by the firmware prior >> to that to see if we can root-cause. > > I'm not seeing anything from the FW in dmesg. > I suppose I need to enable DEBUG in various places? --- a/drivers/media/platform/qcom/venus/hfi_venus.c +++ b/drivers/media/platform/qcom/venus/hfi_venus.c @@ -129,7 +129,8 @@ struct venus_hfi_device { u8 dbg_buf[IFACEQ_VAR_HUGE_PKT_SIZE]; }; -static bool venus_pkt_debug; +#define DEBUG +static bool venus_pkt_debug = true; We can add additional flags - it'd be nice if these could be controlled by a module prameter or debugfs trigger - to this venus_fw_debug = 0x2f; But start with the default mask = int venus_fw_debug = HFI_DEBUG_MSG_ERROR | HFI_DEBUG_MSG_FATAL --- bod
On 10/04/2024 14:29, Bryan O'Donoghue wrote:
> venus_fw_debug = 0x2f;
* 0x3f;
---
bod
On 10/04/2024 14:14, Bryan O'Donoghue wrote: > On 10/04/2024 13:23, Marc Gonzalez wrote: >> FWIW, I get the same behavior with 854x480 and 2560x1440: >> >> 1) If mpv runs with '--length=N' (play only part of the file) >> then mpv exits cleanly, with no kernel messages. > > On -next ? > > I think you mentioned before you were doing your work on an older kernel > and forward porting patches ? > >> 2) If mpv plays the entire file, then mpv hangs at the end >> (needs CTRL-C to exit) and driver prints to kernel: >> [68643.935888] qcom-venus cc00000.video-codec: session error: event >> id:1001 (deadb000), session id:79d42000 >> [68643.935995] qcom-venus-decoder cc00000.video-codec:video-decoder: >> dec: event : 1001 > > Hmm > > #define HFI_ERR_SESSION_FATAL 0x1001 > > Something is causing the firmware to return this packet to you. > > Probably worth tracing the last five messages sent by the firmware prior > to that to see if we can root-cause. > > --- > bod BTW I think you've found two bugs here 1. When we receive a fatal error from firmware we don't "bug out" properly. So we hang forever waiting for more events which won't come. We should fix the handling of SESSION_FATAL to => termination. Clearly after HFI_ERR_SESSION_FATAL we should be completely done not hanging around waiting for additional inputs. 2. Why do we get a fatal error for the session ? Are we continuing to send commands to the firmware after termination maybe - so is there a incongruous context between firmware and Linux ? I don't think either is a blocker specifically for your DTS submission so I think you should go ahead with your DTS stuff. Also though, I think 1) we don't exit properly on HFI_ERR_SESSION_FATAL and 2) we should root-cause why HFI_ERR_SESSION_FATAL is generated at all. --- bod
On 10/04/2024 15:14, Bryan O'Donoghue wrote: > On 10/04/2024 13:23, Marc Gonzalez wrote: > >> FWIW, I get the same behavior with 854x480 and 2560x1440: >> >> 1) If mpv runs with '--length=N' (play only part of the file) >> then mpv exits cleanly, with no kernel messages. > > On -next ? > > I think you mentioned before you were doing your work on an older kernel > and forward porting patches ? > >> 2) If mpv plays the entire file, then mpv hangs at the end >> (needs CTRL-C to exit) and driver prints to kernel: >> [68643.935888] qcom-venus cc00000.video-codec: session error: event id:1001 (deadb000), session id:79d42000 >> [68643.935995] qcom-venus-decoder cc00000.video-codec:video-decoder: dec: event : 1001 > > Hmm > > #define HFI_ERR_SESSION_FATAL 0x1001 > > Something is causing the firmware to return this packet to you. > > Probably worth tracing the last five messages sent by the firmware prior > to that to see if we can root-cause. On kernel command line: log_buf_len=16777216 Before decode: dmesg -n 1 # time mpv --hwdec=v4l2m2m-copy --vd-lavc-software-fallback=no --vo=null --no-audio --untimed --quiet demo-480.webm (+) Video --vid=1 (*) (vp9 854x480 29.970fps) Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl Using hardware decoding (v4l2m2m-copy). VO: [null] 854x480 nv12 [ffmpeg/video] vp9_v4l2m2m: capture POLLERR Exiting... (Quit) real 0m26.810s user 0m4.168s sys 0m3.690s Do you see anything relevant? [ 107.135902] tutu: 00000000: 24 00 00 00 01 10 02 00 00 b0 0c 7a 06 00 00 01 $..........z.... [ 107.135908] tutu: 00000010: 00 00 00 00 00 00 00 00 00 00 70 d8 00 00 00 00 ..........p..... [ 107.135913] tutu: 00000020: 08 00 00 00 .... [ 107.135933] tutu: 00000000: 68 00 00 00 08 10 22 00 00 b0 0c 7a 00 00 00 00 h....."....z.... [ 107.135939] tutu: 00000010: 4d 04 00 00 00 00 00 00 00 00 00 00 60 bf aa 12 M...........`... [ 107.135943] tutu: 00000020: 00 02 00 00 00 00 00 00 00 00 00 00 7e 1b 43 75 ............~.Cu [ 107.135948] tutu: 00000030: 00 10 0a 00 00 d8 09 00 00 00 00 00 56 03 00 00 ............V... [ 107.135953] tutu: 00000040: e0 01 00 00 00 00 00 00 00 00 00 00 0a 00 00 00 ................ [ 107.135958] tutu: 00000050: 00 00 00 00 05 00 00 00 02 00 00 00 00 00 a0 d8 ................ [ 107.135963] tutu: 00000060: 00 00 00 00 00 00 00 00 ........ [ 107.136433] tutu: 00000000: 2c 00 00 00 05 10 21 00 00 b0 0c 7a 00 00 00 00 ,.....!....z.... [ 107.136464] tutu: 00000010: 00 00 00 00 00 10 0a 00 00 00 00 00 07 00 00 00 ................ [ 107.136469] tutu: 00000020: 00 00 80 d8 00 00 00 00 00 00 00 00 ............ [ 107.136614] tutu: 00000000: 3c 00 00 00 04 10 21 00 00 b0 0c 7a 00 00 00 00 <.....!....z.... [ 107.136661] tutu: 00000010: 50 e5 b2 12 00 00 00 00 00 00 00 00 00 00 00 00 P............... [ 107.136667] tutu: 00000020: 00 00 00 00 00 00 26 00 9f 00 00 00 0a 00 00 00 ......&......... [ 107.136672] tutu: 00000030: 00 00 c0 da 00 00 00 00 28 3d fa 85 ........(=.. [ 107.137791] tutu: 00000000: 28 00 00 00 07 10 22 00 00 b0 0c 7a 00 00 00 00 (....."....z.... [ 107.137799] tutu: 00000010: 9c 00 10 00 18 17 11 00 0b 00 00 00 00 00 80 da ................ [ 107.137804] tutu: 00000020: 00 00 00 00 00 00 00 00 ........ [ 107.137815] tutu: 00000000: 88 00 00 00 04 00 02 00 04 00 00 00 6c 00 00 00 ............l... [ 107.137820] tutu: 00000010: 00 00 00 00 c8 11 01 00 0a 3c 56 46 57 5f 48 3a .........<VFW_H: [ 107.137826] tutu: 00000020: 57 6f 72 6b 65 72 3a 76 70 39 64 3a 37 61 30 63 Worker:vp9d:7a0c [ 107.137832] tutu: 00000030: 62 30 30 30 3a 30 3e 20 56 50 39 20 48 65 61 64 b000:0> VP9 Head [ 107.137837] tutu: 00000040: 65 72 20 28 49 4e 54 45 52 20 46 52 41 4d 45 20 er (INTER FRAME [ 107.137842] tutu: 00000050: 2d 20 66 72 6f 6d 20 61 63 74 69 76 65 5f 72 65 - from active_re [ 107.137847] tutu: 00000060: 66 5f 69 64 78 5f 65 6e 61 62 6c 65 64 29 3a 57 f_idx_enabled):W [ 107.137851] tutu: 00000070: 69 64 74 68 3d 38 35 34 20 48 65 69 67 68 74 3d idth=854 Height= [ 107.137857] tutu: 00000080: 34 38 30 00 20 4b 65 79 480. Key [ 107.137863] tutu: 00000000: 76 00 00 00 04 00 02 00 08 00 00 00 5a 00 00 00 v...........Z... [ 107.137868] tutu: 00000010: 00 00 00 00 c9 11 01 00 0a 3c 56 46 57 5f 45 3a .........<VFW_E: [ 107.137874] tutu: 00000020: 57 6f 72 6b 65 72 3a 76 70 39 64 3a 37 61 30 63 Worker:vp9d:7a0c [ 107.137878] tutu: 00000030: 62 30 30 30 3a 30 3e 20 76 70 78 44 65 63 5f 50 b000:0> vpxDec_P [ 107.137883] tutu: 00000040: 61 72 73 65 46 6f 72 53 75 70 65 72 46 72 61 6d arseForSuperFram [ 107.137888] tutu: 00000050: 65 28 38 33 32 35 29 3a 20 6d 61 72 6b 65 72 20 e(8325): marker [ 107.137893] tutu: 00000060: 76 61 6c 75 65 20 62 65 66 6f 72 65 20 3d 20 30 value before = 0 [ 107.137898] tutu: 00000070: 0a 00 46 52 3d 38 ..FR=8 [ 107.137905] tutu: 00000000: 75 00 00 00 04 00 02 00 08 00 00 00 59 00 00 00 u...........Y... [ 107.137909] tutu: 00000010: 00 00 00 00 ca 11 01 00 0a 3c 56 46 57 5f 45 3a .........<VFW_E: [ 107.137915] tutu: 00000020: 57 6f 72 6b 65 72 3a 76 70 39 64 3a 37 61 30 63 Worker:vp9d:7a0c [ 107.137919] tutu: 00000030: 62 30 30 30 3a 30 3e 20 76 70 78 44 65 63 5f 50 b000:0> vpxDec_P [ 107.137924] tutu: 00000040: 61 72 73 65 46 6f 72 53 75 70 65 72 46 72 61 6d arseForSuperFram [ 107.137929] tutu: 00000050: 65 28 38 33 33 30 29 3a 20 6d 61 72 6b 65 72 20 e(8330): marker [ 107.137934] tutu: 00000060: 76 61 6c 75 65 20 61 66 74 65 72 20 3d 20 30 0a value after = 0. [ 107.137940] tutu: 00000070: 00 00 46 52 3d ..FR= [ 107.137946] tutu: 00000000: 7c 00 00 00 04 00 02 00 04 00 00 00 60 00 00 00 |...........`... [ 107.137952] tutu: 00000010: 00 00 00 00 cb 11 01 00 0a 3c 56 46 57 5f 48 3a .........<VFW_H: [ 107.137957] tutu: 00000020: 56 70 70 44 65 63 3a 76 70 39 64 3a 37 61 30 63 VppDec:vp9d:7a0c [ 107.137962] tutu: 00000030: 62 30 30 30 3a 30 3e 20 56 50 78 20 44 65 63 6f b000:0> VPx Deco [ 107.137967] tutu: 00000040: 64 65 72 3a 20 28 44 69 72 65 63 74 20 4d 6f 64 der: (Direct Mod [ 107.137971] tutu: 00000050: 65 29 20 53 50 44 2d 41 50 53 50 20 46 72 61 6d e) SPD-APSP Fram [ 107.137976] tutu: 00000060: 65 20 31 30 30 36 37 20 44 6f 6e 65 20 53 65 73 e 10067 Done Ses [ 107.137981] tutu: 00000070: 73 69 6f 6e 20 30 0a 00 6e 20 30 0a sion 0..n 0. [ 107.137987] tutu: 00000000: 53 00 00 00 04 00 02 00 04 00 00 00 37 00 00 00 S...........7... [ 107.137992] tutu: 00000010: 00 00 00 00 cc 11 01 00 0a 3c 56 46 57 5f 48 3a .........<VFW_H: [ 107.137997] tutu: 00000020: 56 70 70 44 65 63 3a 76 70 39 64 3a 37 61 30 63 VppDec:vp9d:7a0c [ 107.138002] tutu: 00000030: 62 30 30 30 3a 30 3e 20 50 72 6f 63 65 73 73 69 b000:0> Processi [ 107.138007] tutu: 00000040: 6e 67 20 44 69 72 65 63 74 20 4d 6f 64 65 00 64 ng Direct Mode.d [ 107.138012] tutu: 00000050: 65 29 20 e) [ 107.138018] tutu: 00000000: 68 00 00 00 04 00 02 00 04 00 00 00 4c 00 00 00 h...........L... [ 107.138024] tutu: 00000010: 00 00 00 00 cd 11 01 00 0a 3c 56 46 57 5f 48 3a .........<VFW_H: [ 107.138028] tutu: 00000020: 56 70 70 44 65 63 3a 76 70 39 64 3a 37 61 30 63 VppDec:vp9d:7a0c [ 107.138033] tutu: 00000030: 62 30 30 30 3a 30 3e 20 56 50 78 20 44 65 63 6f b000:0> VPx Deco [ 107.138038] tutu: 00000040: 64 65 72 3a 56 50 50 2d 53 44 45 20 50 72 6f 63 der:VPP-SDE Proc [ 107.138043] tutu: 00000050: 65 73 73 69 6e 67 20 46 72 61 6d 65 20 31 30 30 essing Frame 100 [ 107.138048] tutu: 00000060: 36 38 0a 00 30 36 37 20 68..067 [ 107.138054] tutu: 00000000: 81 00 00 00 04 00 02 00 04 00 00 00 65 00 00 00 ............e... [ 107.138059] tutu: 00000010: 00 00 00 00 ce 11 01 00 0a 3c 56 46 57 5f 48 3a .........<VFW_H: [ 107.138064] tutu: 00000020: 56 70 70 44 65 63 3a 76 70 39 64 3a 37 61 30 63 VppDec:vp9d:7a0c [ 107.138069] tutu: 00000030: 62 30 30 30 3a 30 3e 20 56 50 78 20 44 65 63 6f b000:0> VPx Deco [ 107.138074] tutu: 00000040: 64 65 72 3a 20 28 44 69 72 65 63 74 20 4d 6f 64 der: (Direct Mod [ 107.138079] tutu: 00000050: 65 29 20 53 65 6e 64 69 6e 67 20 46 72 61 6d 65 e) Sending Frame [ 107.138083] tutu: 00000060: 20 31 30 30 36 38 20 74 6f 20 56 50 50 2d 53 44 10068 to VPP-SD [ 107.138089] tutu: 00000070: 45 20 53 65 73 73 69 6f 6e 20 30 0a 00 00 00 00 E Session 0..... [ 107.138093] tutu: 00000080: 34 4 [ 107.138111] tutu: 00000000: 24 00 00 00 01 10 02 00 00 b0 0c 7a 06 00 00 01 $..........z.... [ 107.138117] tutu: 00000010: 00 00 00 00 00 00 00 00 00 00 a0 d8 00 00 00 00 ................ [ 107.138121] tutu: 00000020: 05 00 00 00 .... [ 107.138137] tutu: 00000000: 68 00 00 00 08 10 22 00 00 b0 0c 7a 00 00 00 00 h....."....z.... [ 107.138142] tutu: 00000010: 4d 04 00 00 00 00 00 00 00 00 00 00 48 40 ab 12 M...........H@.. [ 107.138147] tutu: 00000020: 00 02 00 00 00 00 00 00 00 00 00 00 7e 1b 43 75 ............~.Cu [ 107.138152] tutu: 00000030: 00 10 0a 00 00 d8 09 00 00 00 00 00 56 03 00 00 ............V... [ 107.138157] tutu: 00000040: e0 01 00 00 00 00 00 00 00 00 00 00 0b 00 00 00 ................ [ 107.138162] tutu: 00000050: 00 00 00 00 01 00 00 00 02 00 00 00 00 00 e0 d8 ................ [ 107.138167] tutu: 00000060: 00 00 00 00 00 00 00 00 ........ [ 107.138173] tutu: 00000000: 3c 00 00 00 04 10 21 00 00 b0 0c 7a 00 00 00 00 <.....!....z.... [ 107.138180] tutu: 00000010: 38 66 b3 12 00 00 00 00 00 00 00 00 00 00 00 00 8f.............. [ 107.138186] tutu: 00000020: 00 00 00 00 00 00 26 00 32 00 00 00 0b 00 00 00 ......&.2....... [ 107.138191] tutu: 00000030: 00 00 80 da 00 00 00 00 28 3d fa 85 ........(=.. [ 107.138699] tutu: 00000000: 2c 00 00 00 05 10 21 00 00 b0 0c 7a 00 00 00 00 ,.....!....z.... [ 107.138712] tutu: 00000010: 00 00 00 00 00 10 0a 00 00 00 00 00 06 00 00 00 ................ [ 107.138717] tutu: 00000020: 00 00 90 d8 00 00 00 00 00 00 00 00 ............ [ 107.138947] tutu: 00000000: 3c 00 00 00 04 10 21 00 00 b0 0c 7a 00 00 00 00 <.....!....z.... [ 107.138955] tutu: 00000010: 00 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 ................ [ 107.138961] tutu: 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ [ 107.138966] tutu: 00000030: 00 b0 ad de 00 00 00 00 00 00 00 00 ............ [ 107.139622] tutu: 00000000: 1c 00 00 00 01 10 02 00 00 b0 0c 7a 02 00 00 00 ...........z.... [ 107.139661] tutu: 00000010: 01 10 00 00 00 b0 ad de 03 00 01 00 ............ [ 107.139670] qcom-venus cc00000.video-codec: session error: event id:1001 (deadb000), session id:7a0cb000 [ 107.139695] qcom-venus-decoder cc00000.video-codec:video-decoder: dec: event session error 1001 [ 107.139703] tutu: 00000000: 28 00 00 00 07 10 22 00 00 b0 0c 7a 03 10 00 00 (....."....z.... [ 107.139708] tutu: 00000010: 10 00 00 00 0c 00 00 00 00 00 00 00 00 b0 ad de ................ [ 107.139713] tutu: 00000020: 20 00 00 00 c0 46 02 00 ....F.. [ 107.139726] tutu: 00000000: 88 00 00 00 04 00 02 00 04 00 00 00 6c 00 00 00 ............l... [ 107.139732] tutu: 00000010: 00 00 00 00 cf 11 01 00 0a 3c 56 46 57 5f 48 3a .........<VFW_H: [ 107.139737] tutu: 00000020: 57 6f 72 6b 65 72 3a 76 70 39 64 3a 37 61 30 63 Worker:vp9d:7a0c [ 107.139742] tutu: 00000030: 62 30 30 30 3a 30 3e 20 56 50 39 20 48 65 61 64 b000:0> VP9 Head [ 107.139747] tutu: 00000040: 65 72 20 28 49 4e 54 45 52 20 46 52 41 4d 45 20 er (INTER FRAME [ 107.139752] tutu: 00000050: 2d 20 66 72 6f 6d 20 61 63 74 69 76 65 5f 72 65 - from active_re [ 107.139757] tutu: 00000060: 66 5f 69 64 78 5f 65 6e 61 62 6c 65 64 29 3a 57 f_idx_enabled):W [ 107.139762] tutu: 00000070: 69 64 74 68 3d 38 35 34 20 48 65 69 67 68 74 3d idth=854 Height= [ 107.139772] tutu: 00000080: 34 38 30 00 20 4b 65 79 480. Key [ 107.139778] tutu: 00000000: 76 00 00 00 04 00 02 00 08 00 00 00 5a 00 00 00 v...........Z... [ 107.139783] tutu: 00000010: 00 00 00 00 d0 11 01 00 0a 3c 56 46 57 5f 45 3a .........<VFW_E: [ 107.139788] tutu: 00000020: 57 6f 72 6b 65 72 3a 76 70 39 64 3a 37 61 30 63 Worker:vp9d:7a0c [ 107.139793] tutu: 00000030: 62 30 30 30 3a 30 3e 20 76 70 78 44 65 63 5f 50 b000:0> vpxDec_P [ 107.139798] tutu: 00000040: 61 72 73 65 46 6f 72 53 75 70 65 72 46 72 61 6d arseForSuperFram [ 107.139803] tutu: 00000050: 65 28 38 33 32 35 29 3a 20 6d 61 72 6b 65 72 20 e(8325): marker [ 107.139807] tutu: 00000060: 76 61 6c 75 65 20 62 65 66 6f 72 65 20 3d 20 30 value before = 0 [ 107.139813] tutu: 00000070: 0a 00 46 52 3d 38 ..FR=8 [ 107.139819] tutu: 00000000: 75 00 00 00 04 00 02 00 08 00 00 00 59 00 00 00 u...........Y... [ 107.139824] tutu: 00000010: 00 00 00 00 d1 11 01 00 0a 3c 56 46 57 5f 45 3a .........<VFW_E: [ 107.139829] tutu: 00000020: 57 6f 72 6b 65 72 3a 76 70 39 64 3a 37 61 30 63 Worker:vp9d:7a0c [ 107.139834] tutu: 00000030: 62 30 30 30 3a 30 3e 20 76 70 78 44 65 63 5f 50 b000:0> vpxDec_P [ 107.139839] tutu: 00000040: 61 72 73 65 46 6f 72 53 75 70 65 72 46 72 61 6d arseForSuperFram [ 107.139843] tutu: 00000050: 65 28 38 33 33 30 29 3a 20 6d 61 72 6b 65 72 20 e(8330): marker [ 107.139848] tutu: 00000060: 76 61 6c 75 65 20 61 66 74 65 72 20 3d 20 30 0a value after = 0. [ 107.139853] tutu: 00000070: 00 00 46 52 3d ..FR= Regards
On 25/04/2024 18:43, Marc Gonzalez wrote: > On 10/04/2024 15:14, Bryan O'Donoghue wrote: > >> On 10/04/2024 13:23, Marc Gonzalez wrote: >> >>> FWIW, I get the same behavior with 854x480 and 2560x1440: >>> >>> 2) If mpv plays the entire file, then mpv hangs at the end >>> (needs CTRL-C to exit) and driver prints to kernel: >>> [68643.935888] qcom-venus cc00000.video-codec: session error: event id:1001 (deadb000), session id:79d42000 >>> [68643.935995] qcom-venus-decoder cc00000.video-codec:video-decoder: dec: event : 1001 >> >> Hmm >> >> #define HFI_ERR_SESSION_FATAL 0x1001 >> >> Something is causing the firmware to return this packet to you. >> >> Probably worth tracing the last five messages sent by the firmware prior >> to that to see if we can root-cause. > > On kernel command line: log_buf_len=16777216 > Before decode: dmesg -n 1 > > # time mpv --hwdec=v4l2m2m-copy --vd-lavc-software-fallback=no --vo=null --no-audio --untimed --quiet demo-480.webm > (+) Video --vid=1 (*) (vp9 854x480 29.970fps) > Audio --aid=1 --alang=eng (*) (opus 2ch 48000Hz) > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > [ffmpeg/video] vp9_v4l2m2m: VIDIOC_G_FMT ioctl > Using hardware decoding (v4l2m2m-copy). > VO: [null] 854x480 nv12 > [ffmpeg/video] vp9_v4l2m2m: capture POLLERR > Exiting... (Quit) > real 0m26.810s > user 0m4.168s > sys 0m3.690s > > > > Do you see anything relevant? > > > [ 107.135902] tutu: 00000000: 24 00 00 00 01 10 02 00 00 b0 0c 7a 06 00 00 01 $..........z.... > [ 107.135908] tutu: 00000010: 00 00 00 00 00 00 00 00 00 00 70 d8 00 00 00 00 ..........p..... > [ 107.135913] tutu: 00000020: 08 00 00 00 .... > [ 107.135933] tutu: 00000000: 68 00 00 00 08 10 22 00 00 b0 0c 7a 00 00 00 00 h....."....z.... > [ 107.135939] tutu: 00000010: 4d 04 00 00 00 00 00 00 00 00 00 00 60 bf aa 12 M...........`... > [ 107.135943] tutu: 00000020: 00 02 00 00 00 00 00 00 00 00 00 00 7e 1b 43 75 ............~.Cu > [ 107.135948] tutu: 00000030: 00 10 0a 00 00 d8 09 00 00 00 00 00 56 03 00 00 ............V... > [ 107.135953] tutu: 00000040: e0 01 00 00 00 00 00 00 00 00 00 00 0a 00 00 00 ................ > [ 107.135958] tutu: 00000050: 00 00 00 00 05 00 00 00 02 00 00 00 00 00 a0 d8 ................ > [ 107.135963] tutu: 00000060: 00 00 00 00 00 00 00 00 ........ > [ 107.136433] tutu: 00000000: 2c 00 00 00 05 10 21 00 00 b0 0c 7a 00 00 00 00 ,.....!....z.... > [ 107.136464] tutu: 00000010: 00 00 00 00 00 10 0a 00 00 00 00 00 07 00 00 00 ................ > [ 107.136469] tutu: 00000020: 00 00 80 d8 00 00 00 00 00 00 00 00 ............ > [ 107.136614] tutu: 00000000: 3c 00 00 00 04 10 21 00 00 b0 0c 7a 00 00 00 00 <.....!....z.... > [ 107.136661] tutu: 00000010: 50 e5 b2 12 00 00 00 00 00 00 00 00 00 00 00 00 P............... > [ 107.136667] tutu: 00000020: 00 00 00 00 00 00 26 00 9f 00 00 00 0a 00 00 00 ......&......... > [ 107.136672] tutu: 00000030: 00 00 c0 da 00 00 00 00 28 3d fa 85 ........(=.. > [ 107.137791] tutu: 00000000: 28 00 00 00 07 10 22 00 00 b0 0c 7a 00 00 00 00 (....."....z.... > [ 107.137799] tutu: 00000010: 9c 00 10 00 18 17 11 00 0b 00 00 00 00 00 80 da ................ > [ 107.137804] tutu: 00000020: 00 00 00 00 00 00 00 00 ........ > [ 107.137815] tutu: 00000000: 88 00 00 00 04 00 02 00 04 00 00 00 6c 00 00 00 ............l... > [ 107.137820] tutu: 00000010: 00 00 00 00 c8 11 01 00 0a 3c 56 46 57 5f 48 3a .........<VFW_H: > [ 107.137826] tutu: 00000020: 57 6f 72 6b 65 72 3a 76 70 39 64 3a 37 61 30 63 Worker:vp9d:7a0c > [ 107.137832] tutu: 00000030: 62 30 30 30 3a 30 3e 20 56 50 39 20 48 65 61 64 b000:0> VP9 Head > [ 107.137837] tutu: 00000040: 65 72 20 28 49 4e 54 45 52 20 46 52 41 4d 45 20 er (INTER FRAME > [ 107.137842] tutu: 00000050: 2d 20 66 72 6f 6d 20 61 63 74 69 76 65 5f 72 65 - from active_re > [ 107.137847] tutu: 00000060: 66 5f 69 64 78 5f 65 6e 61 62 6c 65 64 29 3a 57 f_idx_enabled):W > [ 107.137851] tutu: 00000070: 69 64 74 68 3d 38 35 34 20 48 65 69 67 68 74 3d idth=854 Height= > [ 107.137857] tutu: 00000080: 34 38 30 00 20 4b 65 79 480. Key > [ 107.137863] tutu: 00000000: 76 00 00 00 04 00 02 00 08 00 00 00 5a 00 00 00 v...........Z... > [ 107.137868] tutu: 00000010: 00 00 00 00 c9 11 01 00 0a 3c 56 46 57 5f 45 3a .........<VFW_E: > [ 107.137874] tutu: 00000020: 57 6f 72 6b 65 72 3a 76 70 39 64 3a 37 61 30 63 Worker:vp9d:7a0c > [ 107.137878] tutu: 00000030: 62 30 30 30 3a 30 3e 20 76 70 78 44 65 63 5f 50 b000:0> vpxDec_P > [ 107.137883] tutu: 00000040: 61 72 73 65 46 6f 72 53 75 70 65 72 46 72 61 6d arseForSuperFram > [ 107.137888] tutu: 00000050: 65 28 38 33 32 35 29 3a 20 6d 61 72 6b 65 72 20 e(8325): marker > [ 107.137893] tutu: 00000060: 76 61 6c 75 65 20 62 65 66 6f 72 65 20 3d 20 30 value before = 0 > [ 107.137898] tutu: 00000070: 0a 00 46 52 3d 38 ..FR=8 > [ 107.137905] tutu: 00000000: 75 00 00 00 04 00 02 00 08 00 00 00 59 00 00 00 u...........Y... > [ 107.137909] tutu: 00000010: 00 00 00 00 ca 11 01 00 0a 3c 56 46 57 5f 45 3a .........<VFW_E: > [ 107.137915] tutu: 00000020: 57 6f 72 6b 65 72 3a 76 70 39 64 3a 37 61 30 63 Worker:vp9d:7a0c > [ 107.137919] tutu: 00000030: 62 30 30 30 3a 30 3e 20 76 70 78 44 65 63 5f 50 b000:0> vpxDec_P > [ 107.137924] tutu: 00000040: 61 72 73 65 46 6f 72 53 75 70 65 72 46 72 61 6d arseForSuperFram > [ 107.137929] tutu: 00000050: 65 28 38 33 33 30 29 3a 20 6d 61 72 6b 65 72 20 e(8330): marker > [ 107.137934] tutu: 00000060: 76 61 6c 75 65 20 61 66 74 65 72 20 3d 20 30 0a value after = 0. > [ 107.137940] tutu: 00000070: 00 00 46 52 3d ..FR= > [ 107.137946] tutu: 00000000: 7c 00 00 00 04 00 02 00 04 00 00 00 60 00 00 00 |...........`... > [ 107.137952] tutu: 00000010: 00 00 00 00 cb 11 01 00 0a 3c 56 46 57 5f 48 3a .........<VFW_H: > [ 107.137957] tutu: 00000020: 56 70 70 44 65 63 3a 76 70 39 64 3a 37 61 30 63 VppDec:vp9d:7a0c > [ 107.137962] tutu: 00000030: 62 30 30 30 3a 30 3e 20 56 50 78 20 44 65 63 6f b000:0> VPx Deco > [ 107.137967] tutu: 00000040: 64 65 72 3a 20 28 44 69 72 65 63 74 20 4d 6f 64 der: (Direct Mod > [ 107.137971] tutu: 00000050: 65 29 20 53 50 44 2d 41 50 53 50 20 46 72 61 6d e) SPD-APSP Fram > [ 107.137976] tutu: 00000060: 65 20 31 30 30 36 37 20 44 6f 6e 65 20 53 65 73 e 10067 Done Ses > [ 107.137981] tutu: 00000070: 73 69 6f 6e 20 30 0a 00 6e 20 30 0a sion 0..n 0. > [ 107.137987] tutu: 00000000: 53 00 00 00 04 00 02 00 04 00 00 00 37 00 00 00 S...........7... > [ 107.137992] tutu: 00000010: 00 00 00 00 cc 11 01 00 0a 3c 56 46 57 5f 48 3a .........<VFW_H: > [ 107.137997] tutu: 00000020: 56 70 70 44 65 63 3a 76 70 39 64 3a 37 61 30 63 VppDec:vp9d:7a0c > [ 107.138002] tutu: 00000030: 62 30 30 30 3a 30 3e 20 50 72 6f 63 65 73 73 69 b000:0> Processi > [ 107.138007] tutu: 00000040: 6e 67 20 44 69 72 65 63 74 20 4d 6f 64 65 00 64 ng Direct Mode.d > [ 107.138012] tutu: 00000050: 65 29 20 e) > [ 107.138018] tutu: 00000000: 68 00 00 00 04 00 02 00 04 00 00 00 4c 00 00 00 h...........L... > [ 107.138024] tutu: 00000010: 00 00 00 00 cd 11 01 00 0a 3c 56 46 57 5f 48 3a .........<VFW_H: > [ 107.138028] tutu: 00000020: 56 70 70 44 65 63 3a 76 70 39 64 3a 37 61 30 63 VppDec:vp9d:7a0c > [ 107.138033] tutu: 00000030: 62 30 30 30 3a 30 3e 20 56 50 78 20 44 65 63 6f b000:0> VPx Deco > [ 107.138038] tutu: 00000040: 64 65 72 3a 56 50 50 2d 53 44 45 20 50 72 6f 63 der:VPP-SDE Proc > [ 107.138043] tutu: 00000050: 65 73 73 69 6e 67 20 46 72 61 6d 65 20 31 30 30 essing Frame 100 > [ 107.138048] tutu: 00000060: 36 38 0a 00 30 36 37 20 68..067 > [ 107.138054] tutu: 00000000: 81 00 00 00 04 00 02 00 04 00 00 00 65 00 00 00 ............e... > [ 107.138059] tutu: 00000010: 00 00 00 00 ce 11 01 00 0a 3c 56 46 57 5f 48 3a .........<VFW_H: > [ 107.138064] tutu: 00000020: 56 70 70 44 65 63 3a 76 70 39 64 3a 37 61 30 63 VppDec:vp9d:7a0c > [ 107.138069] tutu: 00000030: 62 30 30 30 3a 30 3e 20 56 50 78 20 44 65 63 6f b000:0> VPx Deco > [ 107.138074] tutu: 00000040: 64 65 72 3a 20 28 44 69 72 65 63 74 20 4d 6f 64 der: (Direct Mod > [ 107.138079] tutu: 00000050: 65 29 20 53 65 6e 64 69 6e 67 20 46 72 61 6d 65 e) Sending Frame > [ 107.138083] tutu: 00000060: 20 31 30 30 36 38 20 74 6f 20 56 50 50 2d 53 44 10068 to VPP-SD > [ 107.138089] tutu: 00000070: 45 20 53 65 73 73 69 6f 6e 20 30 0a 00 00 00 00 E Session 0..... > [ 107.138093] tutu: 00000080: 34 4 > [ 107.138111] tutu: 00000000: 24 00 00 00 01 10 02 00 00 b0 0c 7a 06 00 00 01 $..........z.... > [ 107.138117] tutu: 00000010: 00 00 00 00 00 00 00 00 00 00 a0 d8 00 00 00 00 ................ > [ 107.138121] tutu: 00000020: 05 00 00 00 .... > [ 107.138137] tutu: 00000000: 68 00 00 00 08 10 22 00 00 b0 0c 7a 00 00 00 00 h....."....z.... > [ 107.138142] tutu: 00000010: 4d 04 00 00 00 00 00 00 00 00 00 00 48 40 ab 12 M...........H@.. > [ 107.138147] tutu: 00000020: 00 02 00 00 00 00 00 00 00 00 00 00 7e 1b 43 75 ............~.Cu > [ 107.138152] tutu: 00000030: 00 10 0a 00 00 d8 09 00 00 00 00 00 56 03 00 00 ............V... > [ 107.138157] tutu: 00000040: e0 01 00 00 00 00 00 00 00 00 00 00 0b 00 00 00 ................ > [ 107.138162] tutu: 00000050: 00 00 00 00 01 00 00 00 02 00 00 00 00 00 e0 d8 ................ > [ 107.138167] tutu: 00000060: 00 00 00 00 00 00 00 00 ........ > [ 107.138173] tutu: 00000000: 3c 00 00 00 04 10 21 00 00 b0 0c 7a 00 00 00 00 <.....!....z.... > [ 107.138180] tutu: 00000010: 38 66 b3 12 00 00 00 00 00 00 00 00 00 00 00 00 8f.............. > [ 107.138186] tutu: 00000020: 00 00 00 00 00 00 26 00 32 00 00 00 0b 00 00 00 ......&.2....... > [ 107.138191] tutu: 00000030: 00 00 80 da 00 00 00 00 28 3d fa 85 ........(=.. > [ 107.138699] tutu: 00000000: 2c 00 00 00 05 10 21 00 00 b0 0c 7a 00 00 00 00 ,.....!....z.... > [ 107.138712] tutu: 00000010: 00 00 00 00 00 10 0a 00 00 00 00 00 06 00 00 00 ................ > [ 107.138717] tutu: 00000020: 00 00 90 d8 00 00 00 00 00 00 00 00 ............ > [ 107.138947] tutu: 00000000: 3c 00 00 00 04 10 21 00 00 b0 0c 7a 00 00 00 00 <.....!....z.... > [ 107.138955] tutu: 00000010: 00 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 ................ > [ 107.138961] tutu: 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > [ 107.138966] tutu: 00000030: 00 b0 ad de 00 00 00 00 00 00 00 00 ............ > [ 107.139622] tutu: 00000000: 1c 00 00 00 01 10 02 00 00 b0 0c 7a 02 00 00 00 ...........z.... > [ 107.139661] tutu: 00000010: 01 10 00 00 00 b0 ad de 03 00 01 00 ............ > > [ 107.139670] qcom-venus cc00000.video-codec: session error: event id:1001 (deadb000), session id:7a0cb000 > [ 107.139695] qcom-venus-decoder cc00000.video-codec:video-decoder: dec: event session error 1001 > > [ 107.139703] tutu: 00000000: 28 00 00 00 07 10 22 00 00 b0 0c 7a 03 10 00 00 (....."....z.... > [ 107.139708] tutu: 00000010: 10 00 00 00 0c 00 00 00 00 00 00 00 00 b0 ad de ................ > [ 107.139713] tutu: 00000020: 20 00 00 00 c0 46 02 00 ....F.. > [ 107.139726] tutu: 00000000: 88 00 00 00 04 00 02 00 04 00 00 00 6c 00 00 00 ............l... > [ 107.139732] tutu: 00000010: 00 00 00 00 cf 11 01 00 0a 3c 56 46 57 5f 48 3a .........<VFW_H: > [ 107.139737] tutu: 00000020: 57 6f 72 6b 65 72 3a 76 70 39 64 3a 37 61 30 63 Worker:vp9d:7a0c > [ 107.139742] tutu: 00000030: 62 30 30 30 3a 30 3e 20 56 50 39 20 48 65 61 64 b000:0> VP9 Head > [ 107.139747] tutu: 00000040: 65 72 20 28 49 4e 54 45 52 20 46 52 41 4d 45 20 er (INTER FRAME > [ 107.139752] tutu: 00000050: 2d 20 66 72 6f 6d 20 61 63 74 69 76 65 5f 72 65 - from active_re > [ 107.139757] tutu: 00000060: 66 5f 69 64 78 5f 65 6e 61 62 6c 65 64 29 3a 57 f_idx_enabled):W > [ 107.139762] tutu: 00000070: 69 64 74 68 3d 38 35 34 20 48 65 69 67 68 74 3d idth=854 Height= > [ 107.139772] tutu: 00000080: 34 38 30 00 20 4b 65 79 480. Key > [ 107.139778] tutu: 00000000: 76 00 00 00 04 00 02 00 08 00 00 00 5a 00 00 00 v...........Z... > [ 107.139783] tutu: 00000010: 00 00 00 00 d0 11 01 00 0a 3c 56 46 57 5f 45 3a .........<VFW_E: > [ 107.139788] tutu: 00000020: 57 6f 72 6b 65 72 3a 76 70 39 64 3a 37 61 30 63 Worker:vp9d:7a0c > [ 107.139793] tutu: 00000030: 62 30 30 30 3a 30 3e 20 76 70 78 44 65 63 5f 50 b000:0> vpxDec_P > [ 107.139798] tutu: 00000040: 61 72 73 65 46 6f 72 53 75 70 65 72 46 72 61 6d arseForSuperFram > [ 107.139803] tutu: 00000050: 65 28 38 33 32 35 29 3a 20 6d 61 72 6b 65 72 20 e(8325): marker > [ 107.139807] tutu: 00000060: 76 61 6c 75 65 20 62 65 66 6f 72 65 20 3d 20 30 value before = 0 > [ 107.139813] tutu: 00000070: 0a 00 46 52 3d 38 ..FR=8 > [ 107.139819] tutu: 00000000: 75 00 00 00 04 00 02 00 08 00 00 00 59 00 00 00 u...........Y... > [ 107.139824] tutu: 00000010: 00 00 00 00 d1 11 01 00 0a 3c 56 46 57 5f 45 3a .........<VFW_E: > [ 107.139829] tutu: 00000020: 57 6f 72 6b 65 72 3a 76 70 39 64 3a 37 61 30 63 Worker:vp9d:7a0c > [ 107.139834] tutu: 00000030: 62 30 30 30 3a 30 3e 20 76 70 78 44 65 63 5f 50 b000:0> vpxDec_P > [ 107.139839] tutu: 00000040: 61 72 73 65 46 6f 72 53 75 70 65 72 46 72 61 6d arseForSuperFram > [ 107.139843] tutu: 00000050: 65 28 38 33 33 30 29 3a 20 6d 61 72 6b 65 72 20 e(8330): marker > [ 107.139848] tutu: 00000060: 76 61 6c 75 65 20 61 66 74 65 72 20 3d 20 30 0a value after = 0. > [ 107.139853] tutu: 00000070: 00 00 46 52 3d ..FR= Bryan suggested adding Dikshita. Is there any useful information in the debug output above? Regards
diff --git a/Documentation/devicetree/bindings/media/qcom,venus-common.yaml b/Documentation/devicetree/bindings/media/qcom,venus-common.yaml index 3153d91f9d18a..69cb16dc4852c 100644 --- a/Documentation/devicetree/bindings/media/qcom,venus-common.yaml +++ b/Documentation/devicetree/bindings/media/qcom,venus-common.yaml @@ -62,6 +62,9 @@ properties: required: - iommus + qcom,no-low-power: + type: boolean + required: - reg - clocks diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c index f9437b6412b91..2cd85a8cd837e 100644 --- a/drivers/media/platform/qcom/venus/hfi_venus.c +++ b/drivers/media/platform/qcom/venus/hfi_venus.c @@ -945,10 +945,11 @@ static int venus_sys_set_default_properties(struct venus_hfi_device *hdev) dev_warn(dev, "setting idle response ON failed (%d)\n", ret); } - ret = venus_sys_set_power_control(hdev, venus_fw_low_power_mode); - if (ret) - dev_warn(dev, "setting hw power collapse ON failed (%d)\n", - ret); + if (!of_property_read_bool(dev->of_node, "qcom,no-low-power")) { + ret = venus_sys_set_power_control(hdev, venus_fw_low_power_mode); + if (ret) + dev_warn(dev, "setting hw power collapse ON failed (%d)\n", ret); + } /* For specific venus core, it is mandatory to set the UBWC configuration */ if (res->ubwc_conf) {