Re: [RESEND PATCH v2] remoteproc: qcom: Add venus rproc support on msm8996 platform.

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

 




On 12/05/2016 11:46 AM, Dwivedi, Avaneesh Kumar (avani) wrote:
> 
> 
> On 11/30/2016 2:52 PM, Stanimir Varbanov wrote:
>> Hi,
>>
>> On 11/30/2016 07:24 AM, Dwivedi, Avaneesh Kumar (avani) wrote:
>>>
>>> On 11/30/2016 12:57 AM, Stephen Boyd wrote:
>>>> On 11/29, Avaneesh Kumar Dwivedi wrote:
>>>>> This patch is based on
>>>>>      https://patchwork.kernel.org/patch/9415627/
>>>>>      https://patchwork.kernel.org/patch/9415651/
>>>>>
>>>>> This patch add clock initialization, enable and disable support.
>>>>> Required resource name string and rating are differentiated based
>>>>> on compatible string. Also added documentation for venus pil on
>>>>> msm8996.
>>>>>
>>>>> Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@xxxxxxxxxxxxxx>
>>>>> ---
>>>>>    .../devicetree/bindings/remoteproc/qcom,venus.txt  |  26 ++++-
>>>>>    drivers/remoteproc/qcom_venus_pil.c                | 116
>>>>> ++++++++++++++++++++-
>>>>>    2 files changed, 140 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
>>>>> b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
>>>>> index 2d73ba1..417026b 100644
>>>>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
>>>>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
>>>>> @@ -6,13 +6,30 @@ on the Qualcomm Venus remote processor core.
>>>>>    - compatible:
>>>>>        Usage: required
>>>>>        Value type: <string>
>>>>> -    Definition: must contain "qcom,venus-pil"
>>>>> +    Definition: must contain "qcom,venus-pil" or
>>>>> +                "qcom,venus-msm8996-pil"
>>>>>      - memory-region:
>>>>>        Usage: required
>>>>>        Value type: <phandle>
>>>>>        Definition: a phandle to a node describing reserved memory
>>>>>    +- clocks:
>>>>> +    Usage: required
>>>>> +    Value type: <prop-encoded-array>
>>>>> +    Definition: reference to the core, iface and bus and maxi clocks
>>>>> to be held on
>>>>> +            behalf of the booting of the venus core
>>>>> +
>>>>> +- clock-names:
>>>>> +    Usage: required
>>>>> +    Value type: <stringlist>
>>>>> +    Definition: should be "core_clk", "iface_clk", "bus_clk",
>>>>> "maxi_clk"
>>>> Please drop _clk from all clock names.
>>> OK
>>>>> +
>>>>> +- power-domains:
>>>>> +    Usage: required
>>>>> +    Value type: <prop-encoded-array>
>>>>> +    Definition: reference to the venus gdsc to be turned on before
>>>>> booting venus core
>>>> All these new properties can't be required if the original
>>>> compatible is used, right?
>>> gdsc were earlier handled via regulator framework, but now gdsc's are
>>> being handled via power domain framework.
>>> so any driver which require gdsc to be enabled need to use this property
>>> on mainline kernel.
>>> so these properties will even be applicable for 8916 venus rproc on
>>> mainline kernel.
>> Then I'd propose to add compatible string for msm8916 too. We will need
>> to distinguish between SoCs in order to select proper Venus firmware
>> file too.
>>
>> I think using power domain in remoteproc driver will break power
>> management of the v4l2 venus driver. Presently I use runtime pm to
>> switch ON/OFF GDSC and enable/disable Venus clocks.
>>
>> When the .probe of venus remoteproc driver is called the power domain
>> will become ON and never switched OFF (except when platform driver
>> .remove method is called). One possible solution would be to add runtime
>> pm in venus remoreproc driver.
>>
>> IMO we should think more about that.
>>
>> Bjorn, Stephen what are your opinions?
>>
> Shall I go ahead and add runtime PM API calls during shutdown/start call
> along with other changes to address other comments? i was waiting any
> comment from Sboyed and Bjorn.
> 

IMO runtime should be added in remoteproc core, so you can start with an
RFC patch to see how is going.

-- 
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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