Message-ID: <95b61d3f-f6e8-4746-9195-cc38d8815d66@kernel.org>
Date: Wed, 12 Nov 2025 08:31:11 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Aaron Kling <webgeek1234@...il.com>, Jon Hunter <jonathanh@...dia.com>
Cc: Rob Herring <robh@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Thierry Reding <thierry.reding@...il.com>, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, linux-tegra@...r.kernel.org
Subject: Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

On 12/11/2025 08:21, Aaron Kling wrote:
> On Wed, Nov 12, 2025 at 12:18 AM Jon Hunter <jonathanh@...dia.com> wrote:
>>
>>
>> On 11/11/2025 23:17, Aaron Kling wrote:
>>
>> ...
>>
>>> Alright, I think I've got the picture of what's going on now. The
>>> standard arm64 defconfig enables the t194 pcie driver as a module. And
>>> my simple busybox ramdisk that I use for mainline regression testing
>>> isn't loading any modules. If I set the pcie driver to built-in, I
>>> replicate the issue. And I don't see the issue on my normal use case,
>>> because I have the dt changes as well.
>>>
>>> So it appears that the pcie driver submits icc bandwidth. And without
>>> cpufreq submitting bandwidth as well, the emc driver gets a very low
>>> number and thus sets a very low emc freq. The question becomes... what
>>> to do about it? If the related dt changes were submitted to
>>> linux-next, everything should fall into place. And I'm not sure where
>>> this falls on the severity scale since it doesn't full out break boot
>>> or prevent operation.
>>
>> Where are the related DT changes? If we can get these into -next and
>> lined up to be merged for v6.19, then that is fine. However, we should
>> not merge this for v6.19 without the DT changes.
> 
> The dt changes are here [0].
> 
> This was all part of the same series, keeping everything logically
> related together. But on v2, Krzysztof said that none of this should

I asked you about dependencies between the patches and you said there
are none, so collecting different subsystems into one is wrong. That's
nothing new, standard Linux kernel process.

What is non-standard here is keeping secret that there is impact on users.

> have ever been together and that each subsystem should get a separate
> series, even if the changes are related. Which I did, and now this is
> split across three series. The actmon series for tegra210 is in a
> similar state. Split across four series and only one has been pulled
> to linux-next.
> 
>> I will also talk with Thierry to see if he has any concerns about users
>> seeing slow performance if they don't have an up-to-date DTB.
>>
>> Is there any easy way to detect if the DTB has he necessary properties
>> to enable ICC scaling?
> 
> I'm not sure there is any simple way, given how I set up tegra186 and
> tegra194. The new dt properties are on the cpu nodes, there's nothing
> new for the emc node. So the emc driver just unconditionally declares
> itself to icc. It was doing this before too, but wouldn't do anything
> on tegra186 or tegra194 because the set_bw function was just a stub
> and the real logic happened in the bpmp bw mgr, which only exists on
> tegra234+. Now the set_bw function will directly calculate and set the
> emc clock as long as the bpmp bw mgr is not supported. Offhand, I
> can't think of anything existing to check to skip this, because
> nothing new in the dt has been added in the scope of emc.
If your ICC triggers without users, I think it is usual case - you
should not enable the sync_state but instead keep it disabled till you
have all the consumers in place.

Best regards,
Krzysztof

Powered by blists - more mailing lists