Re: [RFC WIP PATCH] venus: add qcom,no-low-power property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux