Re: [PATCH] Revert "arm64: tegra: Add interconnect properties for Tegra210"

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5871 bytes --]

On Wed, Dec 17, 2025 at 02:42:58PM -0600, Aaron Kling wrote:
> On Wed, Dec 17, 2025 at 12:20 PM Aaron Kling <webgeek1234@gmail.com> wrote:
> >
> > On Wed, Dec 17, 2025 at 4:48 AM Jon Hunter <jonathanh@nvidia.com> wrote:
> > >
> > > Commit 59a42707a094 ("arm64: tegra: Add interconnect properties for
> > > Tegra210") populated interconnect properties for Tegra210 and this is
> > > preventing the Tegra DRM driver from probing successfully. The following
> > > error is observed on boot ...
> > >
> > >  drm drm: failed to initialize 54240000.dc: -517
> > >
> > > For now revert this change, until a fix is available.
> > >
> > > Fixes: 59a42707a094 ("arm64: tegra: Add interconnect properties for Tegra210")
> > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> > > ---
> > >  arch/arm64/boot/dts/nvidia/tegra210.dtsi | 24 ------------------------
> > >  1 file changed, 24 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> > > index 709da31d5785..137aa8375257 100644
> > > --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> > > +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> > > @@ -202,19 +202,6 @@ dc@54200000 {
> > >
> > >                         nvidia,outputs = <&dsia &dsib &sor0 &sor1>;
> > >                         nvidia,head = <0>;
> > > -
> > > -                       interconnects = <&mc TEGRA210_MC_DISPLAY0A &emc>,
> > > -                                       <&mc TEGRA210_MC_DISPLAY0B &emc>,
> > > -                                       <&mc TEGRA210_MC_DISPLAY0C &emc>,
> > > -                                       <&mc TEGRA210_MC_DISPLAYHC &emc>,
> > > -                                       <&mc TEGRA210_MC_DISPLAYD &emc>,
> > > -                                       <&mc TEGRA210_MC_DISPLAYT &emc>;
> > > -                       interconnect-names = "wina",
> > > -                                            "winb",
> > > -                                            "winc",
> > > -                                            "cursor",
> > > -                                            "wind",
> > > -                                            "wint";
> > >                 };
> > >
> > >                 dc@54240000 {
> > > @@ -230,15 +217,6 @@ dc@54240000 {
> > >
> > >                         nvidia,outputs = <&dsia &dsib &sor0 &sor1>;
> > >                         nvidia,head = <1>;
> > > -
> > > -                       interconnects = <&mc TEGRA210_MC_DISPLAY0AB &emc>,
> > > -                                       <&mc TEGRA210_MC_DISPLAY0BB &emc>,
> > > -                                       <&mc TEGRA210_MC_DISPLAY0CB &emc>,
> > > -                                       <&mc TEGRA210_MC_DISPLAYHCB &emc>;
> > > -                       interconnect-names = "wina",
> > > -                                            "winb",
> > > -                                            "winc",
> > > -                                            "cursor";
> > >                 };
> > >
> > >                 dsia: dsi@54300000 {
> > > @@ -1052,7 +1030,6 @@ mc: memory-controller@70019000 {
> > >
> > >                 #iommu-cells = <1>;
> > >                 #reset-cells = <1>;
> > > -               #interconnect-cells = <1>;
> > >         };
> > >
> > >         emc: external-memory-controller@7001b000 {
> > > @@ -1066,7 +1043,6 @@ emc: external-memory-controller@7001b000 {
> > >                 nvidia,memory-controller = <&mc>;
> > >                 operating-points-v2 = <&emc_icc_dvfs_opp_table>;
> > >
> > > -               #interconnect-cells = <0>;
> > >                 #cooling-cells = <2>;
> > >         };
> > >
> > > --
> > > 2.43.0
> > >
> >
> > A little bit of documentation on this, should someone read the list to
> > see what happened. The original report of the failure is here [0] and
> > only occurred when the tegra210-emc driver fails to probe. After this
> > report, the related code change [1] to tegra210-emc which registers
> > the driver to icc was silently dropped from -next, but these dt
> > changes remained. So now these interconnect routes do cause tegra-drm
> > to universally fail on tegra210.
> >
> > Aaron
> >
> > [0] https://lore.kernel.org/all/56aed0ec-b104-4612-8901-3f6f95e0afab@nvidia.com/
> > [1] https://lore.kernel.org/all/20251027-t210-actmon-p2-v6-1-1c4bd227d676@gmail.com/
> 
> There may be another option here. I'm beginning to think there will
> not be any way to set the icc routes for the dc's while the emc driver
> can fail to probe. The next best thing looks to be just dropping the
> interconnect nodes from the dc nodes and leaving the rest of the
> original commit in place. Then reland the tegra210-emc driver change.
> This should put the no-emc case back to where it was, while allowing
> actmon to do its scaling when emc is available. And I will move to the
> dc icc routes to downstream dt's, where I tightly control that emc is
> available.
> 
> Does this sound reasonable? If so, I will go stage the changes and
> verify that it works as intended.

Let's go with the revert for now, since that clearly documents where
things go wrong and it can be easily reapplied once the root cause has
been resolved.

It's a bit unfortunate that we don't have a way of making these
interconnect properties optional. If EMC fails to probe for whatever
reason, I think the assumption should be that it doesn't do any dynamic
scaling of the EMC frequency and hence the entire ICC stuff isn't needed
and should just be no-ops.

On the other hand, other than the patches getting reverted, there's
really no good reason for the EMC driver to fail to provide them, hence
I think once that's been restored we can apply this again and then that
should be the end of it.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]