Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.117.161 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.117.161; helo=mail.nvidia.com; pr=C
Message-ID: <a8134495-c8bf-8824-5fc4-9685a5c11640@nvidia.com>
Date:   Fri, 13 Jan 2023 17:45:44 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM
 scaling in Tegra234
Content-Language: en-US
To:     Dmitry Osipenko <digetx@gmail.com>, <treding@nvidia.com>,
        <krzysztof.kozlowski@linaro.org>, <dmitry.osipenko@collabora.com>,
        <viresh.kumar@linaro.org>, <rafael@kernel.org>,
        <jonathanh@nvidia.com>, <robh+dt@kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-tegra@vger.kernel.org>,
        <linux-pm@vger.kernel.org>, <devicetree@vger.kernel.org>
References: <20221220160240.27494-1-sumitg@nvidia.com>
 <20221220160240.27494-2-sumitg@nvidia.com>
 <4d562b75-854d-1997-8969-e7ef222e4e37@gmail.com>
 <298fcaca-c708-5d68-95d6-51673f7a2174@nvidia.com>
 <d2c4c6fc-c432-d122-1e90-3e3ef96eed72@gmail.com>
CC:     Sanjay Chandrashekara <sanjayc@nvidia.com>,
        Krishna Sitaraman <ksitaraman@nvidia.com>,
        Ishan Shah <ishah@nvidia.com>, Bibek Basu <bbasu@nvidia.com>,
        Sumit Gupta <sumitg@nvidia.com>
From:   Sumit Gupta <sumitg@nvidia.com>
In-Reply-To: <d2c4c6fc-c432-d122-1e90-3e3ef96eed72@gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jan 2023 12:15:57.5393
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 6fb09693-595c-449b-9fa6-08daf55fed38
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.117.161];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource: DS1PEPF0000E645.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR12MB8133
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_NONE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 21/12/22 22:13, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
> 
> 
> 21.12.2022 12:35, Sumit Gupta пишет:
>>
>>
>> On 20/12/22 23:40, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> 20.12.2022 19:02, Sumit Gupta пишет:
>>>> +static int tegra_emc_icc_get_init_bw(struct icc_node *node, u32
>>>> *avg, u32 *peak)
>>>> +{
>>>> +     *avg = 0;
>>>> +     *peak = 0;
>>>> +
>>>> +     return 0;
>>>> +}
>>>
>>> Looks wrong, you should add ICC support to all the drivers first and
>>> only then enable ICC. I think you added this init_bw() to work around
>>> the fact that ICC isn't supported by T234 drivers.
>>
>> If get_bw hook is not added then max freq is set due to 'INT_MAX' below.
>>
>>   void icc_node_add(struct icc_node *node, struct icc_provider *provider)
>>   {
>>     ....
>>     /* get the initial bandwidth values and sync them with hardware */
>>     if (provider->get_bw) {
>>           provider->get_bw(node, &node->init_avg, &node->init_peak);
>>     } else {
>>           node->init_avg = INT_MAX;
>>           node->init_peak = INT_MAX;
>>   }
>>
>> So, will have to add the empty functions at least.
>>
>>   static int tegra_emc_icc_get_init_bw(struct icc_node *node, u32 *avg,
>> u32 *peak)
>>   {
>> -       *avg = 0;
>> -       *peak = 0;
>> -
>>          return 0;
>>   }
>>
>> Support to all the client drivers can't be added at once as there are
>> many drivers all with different requirements and handling. This patch
>> series is the beginning to add the basic interconnect support in new
>> Tegra SoC's. Support for more clients will be added later one by one or
>> in batch.
> 
> This means that bandwidth management isn't working properly. You should
> leave the freq to INT_MAX and fix the missing integer overflows in the
> code if any, or read out the BW from FW.
> 
> Once you'll enable ICC for all drivers, it will start working.
> 

Referred the below patches and now understand what you mean.
 
https://patchwork.kernel.org/project/linux-arm-msm/cover/20200825170152.6434-1-georgi.djakov@linaro.org/
  https://lore.kernel.org/all/20210912183009.6400-1-digetx@gmail.com/

There is no MRQ currently in BPMP-FW to get the bandwidth for a client.
So, can't add that in get_bw(). As explained earlier, we are in process 
adding of adding support for all clients but that may take some time. We 
can remove the get_bw() calls initializing "*bw = 0" once we have the 
support added in all clients. Noticed that below drivers also do same.

  $ grep -rB3 "*peak = 0" drivers/interconnect/*
  drivers/interconnect/imx/imx.c-static int imx_icc_get_bw(struct 
icc_node *node, u32 *avg, u32 *peak)
  drivers/interconnect/imx/imx.c-{
  drivers/interconnect/imx/imx.c- *avg = 0;
  drivers/interconnect/imx/imx.c: *peak = 0;
  --
  drivers/interconnect/qcom/msm8974.c-static int msm8974_get_bw(struct 
icc_node *node, u32 *avg, u32 *peak)
  drivers/interconnect/qcom/msm8974.c-{
  drivers/interconnect/qcom/msm8974.c-    *avg = 0;
  drivers/interconnect/qcom/msm8974.c:    *peak = 0;