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
