Subject: Re: [PATCH v5 5/5] PCI: qcom: Add OPP support to scale performance state of power domain


On 11/8/2023 8:02 AM, Krishna Chaitanya Chundru wrote:
>
> On 11/3/2023 10:42 AM, Viresh Kumar wrote:
>> On 02-11-23, 07:09, Bjorn Helgaas wrote:
>>> On Thu, Nov 02, 2023 at 11:00:13AM +0530, Viresh Kumar wrote:
>>>> On 01-11-23, 17:17, Bjorn Helgaas wrote:
>>>>> Can you expand "OPP" somewhere so we know what it stands for?  I'm
>>>>> sure everybody knows except me :)
>>>> It is "Operating Performance Points", defined here:
>>>>
>>>> Documentation/power/opp.rst
>>> Thanks; I meant in the subject or commit log of the next revision, of
>>> course.
>> Yeah, I understood that. Krishna shall do it in next version I believe.
>>
> Hi All,
>
> I will do this in my next patch both commit message and ICC voting
> through OPP
>
> got stuck in some other work, will try to send new series as soon as
> possible.
>
> - Krishna Chaitanya.
>
Hi Viresh,

Sorry for late response.

We calculate ICC BW voting based up on PCIe speed and PCIe width.

Right now we are adding the opp table based up on PCIe speed.

Each PCIe controller can support multiple lane configurations like x1,
x2, x4, x8, x16 based up on controller capability.

So for each GEN speed we need  up to 5 entries in OPP table. This will
make OPP table very long.

It is best to calculate the ICC BW voting in the driver itself and apply
them through ICC driver.

Let me know your opinion on this.

Thanks & Regards,

Krishna Chaitanya.



2024-01-10 06:58:17

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v5 5/5] PCI: qcom: Add OPP support to scale performance state of power domain

On 08-01-24, 18:49, Krishna Chaitanya Chundru wrote:
> We calculate ICC BW voting based up on PCIe speed and PCIe width.
>
> Right now we are adding the opp table based up on PCIe speed.
>
> Each PCIe controller can support multiple lane configurations like x1, x2,
> x4, x8, x16 based up on controller capability.
>
> So for each GEN speed we need? up to 5 entries in OPP table. This will make
> OPP table very long.
>
> It is best to calculate the ICC BW voting in the driver itself and apply
> them through ICC driver.

I see. Are the lane configurations fixed for a platform ? I mean, do you change
those configurations at runtime or is that something that never changes, but the
driver can end up getting used on a hardware that supports any one of them ?

If they are fixed (second case), then you can use dev_pm_opp_set_prop_name() to
make that easier for you. With that you will only need 5 OPP entries, but each
of them will have five values of bw:

bw-x1, bw-x2, .... and you can select one of them during initialization.

--
viresh

Subject: Re: [PATCH v5 5/5] PCI: qcom: Add OPP support to scale performance state of power domain


On 1/10/2024 12:27 PM, Viresh Kumar wrote:
> On 08-01-24, 18:49, Krishna Chaitanya Chundru wrote:
>> We calculate ICC BW voting based up on PCIe speed and PCIe width.
>>
>> Right now we are adding the opp table based up on PCIe speed.
>>
>> Each PCIe controller can support multiple lane configurations like x1, x2,
>> x4, x8, x16 based up on controller capability.
>>
>> So for each GEN speed we need  up to 5 entries in OPP table. This will make
>> OPP table very long.
>>
>> It is best to calculate the ICC BW voting in the driver itself and apply
>> them through ICC driver.
> I see. Are the lane configurations fixed for a platform ? I mean, do you change
> those configurations at runtime or is that something that never changes, but the
> driver can end up getting used on a hardware that supports any one of them ?
>
> If they are fixed (second case), then you can use dev_pm_opp_set_prop_name() to
> make that easier for you. With that you will only need 5 OPP entries, but each
> of them will have five values of bw:
>
> bw-x1, bw-x2, .... and you can select one of them during initialization.

Hi Viresh,

At present we are not changing the link width after link is initialized,
but we have plans to

add support change link width dynamically at runtime.

So, I think it is better to have ICC BW voting in the driver itself.

Thanks & Regards,

Krishna Chaitanya.


2024-01-10 07:39:08

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v5 5/5] PCI: qcom: Add OPP support to scale performance state of power domain

On 10-01-24, 12:42, Krishna Chaitanya Chundru wrote:
> At present we are not changing the link width after link is initialized, but
> we have plans to
>
> add support change link width dynamically at runtime.

Hmm okay.

> So, I think it is better to have ICC BW voting in the driver itself.

I guess it is better to have more entries in the OPP table then.. 15-20 OPPs
isn't too many to be honest.

Replicating code is the last thing I would like to do.

Maybe you can show the different layouts of the OPP table if you are concerned.
We can then see if it is getting too much or not.

--
viresh

Subject: Re: [PATCH v5 5/5] PCI: qcom: Add OPP support to scale performance state of power domain


On 1/10/2024 1:08 PM, Viresh Kumar wrote:
> On 10-01-24, 12:42, Krishna Chaitanya Chundru wrote:
>> At present we are not changing the link width after link is initialized, but
>> we have plans to
>>
>> add support change link width dynamically at runtime.
> Hmm okay.
>
>> So, I think it is better to have ICC BW voting in the driver itself.
> I guess it is better to have more entries in the OPP table then.. 15-20 OPPs
> isn't too many to be honest.
>
> Replicating code is the last thing I would like to do.
>
> Maybe you can show the different layouts of the OPP table if you are concerned.
> We can then see if it is getting too much or not.

Viresh,

it might be less only for now may be around 20 opp entries, but PCIe
spec is being updated every few years and a new gen

gen speed will release, right now PCIe GEN6 is released but I don't we
had any device in the market now and GEN7 is in process.

So in future it might become very big table. Either we need to come up
with a framework in the OPP to select the BW based up on lane width

for particular speed or use the driver way.


Thanks & Regards,

Krishna Chaitanya.


2024-01-11 03:33:14

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v5 5/5] PCI: qcom: Add OPP support to scale performance state of power domain

On 10-01-24, 18:28, Krishna Chaitanya Chundru wrote:
> it might be less only for now may be around 20 opp entries, but PCIe spec is
> being updated every few years and a new gen
>
> gen speed will release, right now PCIe GEN6 is released but I don't we had
> any device in the market now and GEN7 is in process.
>
> So in future it might become very big table. Either we need to come up with
> a framework in the OPP to select the BW based up on lane width
>
> for particular speed or use the driver way.

Lets solve the problem the right (current) way for right now and revisit the
whole thing when it gets complex ? So I would suggest configuring the bw via the
OPP framework only, since it takes care of that for all other device types too.

We can surely revisit and try to do it differently if we find some issues going
forward.

--
viresh