Received-SPF: pass (google.com: domain of linux-kernel+bounces-156267-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240216062642.247504-3-jhp@endlessos.org> <20240301214117.GA408641@bhelgaas>
In-Reply-To: <20240301214117.GA408641@bhelgaas>
From: Jian-Hong Pan <jhp@endlessos.org>
Date: Wed, 24 Apr 2024 12:27:57 +0800
Message-ID: <CAPpJ_eejyughPK4JXfjNdOvZegEy30tEusjPk4ESkvCQJCbPHg@mail.gmail.com>
Subject: Re: [PATCH v4 3/3] PCI/ASPM: Fix L1.2 parameters when enable link state
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Johan Hovold <johan@kernel.org>, David Box <david.e.box@linux.intel.com>, 
	=?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>, 
	Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>, 
	Mika Westerberg <mika.westerberg@linux.intel.com>, Damien Le Moal <dlemoal@kernel.org>, 
	Nirmal Patel <nirmal.patel@linux.intel.com>, 
	Jonathan Derrick <jonathan.derrick@linux.dev>, linux-pci@vger.kernel.org, 
	linux-kernel@vger.kernel.org, linux@endlessos.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Bjorn Helgaas <helgaas@kernel.org> =E6=96=BC 2024=E5=B9=B43=E6=9C=882=E6=97=
=A5 =E9=80=B1=E5=85=AD =E4=B8=8A=E5=8D=885:41=E5=AF=AB=E9=81=93=EF=BC=9A
>
> In subject:
>
>   PCI/ASPM: Fix L1.2 parameters before enabling L1.2
>
> > +     if (state & link->aspm_capable & ASPM_STATE_L1_2_MASK) {
> > +             parent_l1ss_cap =3D aspm_get_l1ss_cap(parent);
> > +             child_l1ss_cap =3D aspm_get_l1ss_cap(child);
> > +             aspm_calc_l12_info(link, parent_l1ss_cap, child_l1ss_cap)=
;
>
> Why doesn't this happen already via normal enumeration?  It looks like
> this path should do it even without this patch:
>
>   pcie_aspm_init_link_state
>     pcie_aspm_cap_init
>       aspm_l1ss_init

I watch the VMD remapped PCI devices' power states during this path.
Their power states stay at "unknown" until someone sets their power
state to "D0".
And, because their power states stay at "unknown", the L1.2 of the
link between VMD remapped PCIe Root Port and NVMe gets wrong configs
when system goes through this path.
So, system has to calculate the L1.2 information again after set PCI
devices' power state to D0.

Sorry for the late reply.  Got my laptop back recently.

Jian-Hong Pan

> > +     }
> >       pcie_config_aspm_link(link, policy_to_aspm_state(link));
> >
> >       link->clkpm_default =3D (state & PCIE_LINK_STATE_CLKPM) ? 1 : 0;
> > --
> > 2.43.2
> >