From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Dwivedi, Avaneesh Kumar (avani)" Subject: Re: [RESEND PATCH v2] remoteproc: qcom: Add venus rproc support on msm8996 platform. Date: Mon, 5 Dec 2016 15:16:49 +0530 Message-ID: References: <1480416647-3518-1-git-send-email-akdwived@codeaurora.org> <1480416647-3518-2-git-send-email-akdwived@codeaurora.org> <20161129192702.GA6095@codeaurora.org> <1c22ad41-2d14-be04-ecf0-f0395463978a@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]:56666 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752306AbcLEJqy (ORCPT ); Mon, 5 Dec 2016 04:46:54 -0500 In-Reply-To: Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Stanimir Varbanov , Stephen Boyd Cc: bjorn.andersson@linaro.org, agross@codeaurora.org, linux-arm-msm@vger.kernel.org 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 >>>> --- >>>> .../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: >>>> - Definition: must contain "qcom,venus-pil" >>>> + Definition: must contain "qcom,venus-pil" or >>>> + "qcom,venus-msm8996-pil" >>>> - memory-region: >>>> Usage: required >>>> Value type: >>>> Definition: a phandle to a node describing reserved memory >>>> +- clocks: >>>> + Usage: required >>>> + Value type: >>>> + 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: >>>> + 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: >>>> + 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. -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.