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
