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]

 



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?


-- 
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