Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 0099260397
Subject: Re: [RFC PATCH] clk: qcom: clk-rpmh: Add IPA clock support
To:     Stephen Boyd <sboyd@kernel.org>, Alex Elder <elder@linaro.org>,
        linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     georgi.djakov@linaro.org, bjorn.andersson@linaro.org,
        evgreen@google.com, tdas@codeaurora.org
References: <1543895413-1553-1-git-send-email-daidavid1@codeaurora.org>
 <1543895413-1553-2-git-send-email-daidavid1@codeaurora.org>
 <154395145491.88331.1174781210192403998@swboyd.mtv.corp.google.com>
 <8dafe631-4b16-94cd-392e-84728f2bb382@linaro.org>
 <154396284056.88331.12279283832884556349@swboyd.mtv.corp.google.com>
 <edeebce5-24a2-b844-6bff-781017949e2d@codeaurora.org>
 <154399414865.88331.2447825064224349951@swboyd.mtv.corp.google.com>
From:   David Dai <daidavid1@codeaurora.org>
Message-ID: <6ffdf6ca-51b5-a968-bb4e-c4d6d46f63aa@codeaurora.org>
Date:   Wed, 5 Dec 2018 17:24:18 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <154399414865.88331.2447825064224349951@swboyd.mtv.corp.google.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 12/4/2018 11:15 PM, Stephen Boyd wrote:
> Quoting David Dai (2018-12-04 17:14:10)
>> On 12/4/2018 2:34 PM, Stephen Boyd wrote:
>>> Quoting Alex Elder (2018-12-04 13:41:47)
>>>> On 12/4/18 1:24 PM, Stephen Boyd wrote:
>>>>> Quoting David Dai (2018-12-03 19:50:13)
>>>>>> Add IPA clock support by extending the current clk rpmh driver to support
>>>>>> clocks that are managed by a different type of RPMh resource known as
>>>>>> Bus Clock Manager(BCM).
>>>>> Yes, but why? Does the IPA driver need to set clk rates and that somehow
>>>>> doesn't work as a bandwidth request?
>>>> The IPA core clock is a *clock*, not a bus.  Representing it as if
>>>> it were a bus, abusing the interconnect interface--pretending a bandwidth
>>>> request is really a clock rate request--is kind of kludgy.  I think Bjorn
>>>> and David (and maybe Georgi? I don't know) decided a long time ago that
>>>> exposing this as a clock is the right way to do it.  I agree with that.
>>>>
>>> But then we translate that clock rate into a bandwidth request to the
>>> BCM hardware? Seems really weird because it's doing the opposite of what
>>> you say is abusive. What does the IPA driver plan to do with this clk?
>>> Calculate a frequency by knowing that it really boils down to some
>>> bandwidth that then gets converted back into some clock frequency? Do we
>>> have the user somewhere that can be pointed to?
>> The clock rate is translated into a unitless threshold value sent as
>> part of the rpmh msg
>> that BCM takes to select a performance. In this case, the unit
>> conversion is based on
>> the unit value read from the aux data which is in Khz. I understand that
>> this wasn't
>> explicitly mentioned anywhere and I'll improve on that next patch.
> How is this different from bus bandwidth requests? In those cases the
> bandwidth is calculated in bits per second or something like that, and
> written to the hardware so it can convert that bandwidth into kHz and
> set a bus clk frequency in the clock controller? So in the IPA case
> we've skipped the bps to kHz conversion step and gone straight to the
> clk frequency setting part? Is a BCM able to aggregate units of
> bandwidth or kHz depending on how it's configured and this BCM is
> configured for kHz?

The data written to the hardware is just a 14bit scalar value that it 
takes to select a performance/frequency from a preset table. It's not 
really doing any sort of conversion in hardware in this case, instead 
the value is computed by software based on the aux data given. Think of 
it as a generic aggregator as opposed to being strictly bandwidth and 
the aggregator itself does not care what type of value it is(be it Khz 
or BW/s).

>
>> Here's a link to
>> the IPA driver implementation: https://lkml.org/lkml/2018/11/7/220
> Thanks for the link. It looks like the IPA driver hard codes a rate of
> 75 MHz.
>

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project