[PATCH] drm/amd/amdgpu: fix Polaris10 PCIe dpm feature issue.

[Alex Deucher]

On Thu, Apr 11, 2019 at 9:44 AM Alex Deucher <alexdeucher at gmail.com> wrote:
>
> On Thu, Apr 11, 2019 at 3:48 AM Quan, Evan <Evan.Quan at amd.com> wrote:
> >
> > Hi Alex,
> >
> > > Although, it looks like pcie_bandwidth_available() needs to be fixed up to
> > > handle gen3 speeds (PCI_EXP_LNKCAP2).  Maybe something for the future
> > > once that gets fixed up.
> > Not sure whether I understood it correctly.  But the pcie_bandwidth_available() seems handling pcie gen3 correctly.
> > [  651.460629] amdgpu 0000:03:00.0: cur_speed 22, cur_width 8
>
>
> PCI_EXP_LNKSTA only handles gen1 and 2.  If you want to check for gen3
> and 4, you need to use PCI_EXP_LNKCAP2.  See pcie_get_speed_cap() for
> example.  A similar thing needs to be done to
> pcie_bandwidth_available().

Whoops, nevermind, I was mixing up STA and CAP.  We should be able to
use it today.

Alex

>
> Alex
>
> >
> >
> > Regards,
> > Evan
> > > -----Original Message-----
> > > From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Alex
> > > Deucher
> > > Sent: Friday, March 22, 2019 12:42 PM
> > > To: Gui, Jack <Jack.Gui at amd.com>
> > > Cc: Xu, Feifei <Feifei.Xu at amd.com>; amd-gfx list <amd-
> > > gfx at lists.freedesktop.org>; Zhang, Hawking <Hawking.Zhang at amd.com>
> > > Subject: Re: [PATCH] drm/amd/amdgpu: fix Polaris10 PCIe dpm feature issue.
> > >
> > > I think you can even go a step further and just replace
> > > amdgpu_device_get_min_pci_speed_width() with
> > > pcie_bandwidth_available().  It does basically the same thing.
> > > Although, it looks like pcie_bandwidth_available() needs to be fixed up to
> > > handle gen3 speeds (PCI_EXP_LNKCAP2).  Maybe something for the future
> > > once that gets fixed up.  Also, no need for tmp_speed, you can just pass in
> > > NULL if you don't need the speed.
> > >
> > > With that fixed:
> > > Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> > >
> > > Alex
> > >
> > > On Thu, Mar 21, 2019 at 10:31 PM Gui, Jack <Jack.Gui at amd.com> wrote:
> > > >
> > > > Attach correct patch again.
> > > >
> > > > Hi Alex,
> > > >
> > > > Expand the fix to all asics, please help to review.
> > > >
> > > > BR,
> > > > Jack Gui
> > > >
> > > > -----Original Message-----
> > > > From: Gui, Jack
> > > > Sent: Friday, March 22, 2019 10:28 AM
> > > > To: 'Alex Deucher' <alexdeucher at gmail.com>; Xu, Feifei
> > > > <Feifei.Xu at amd.com>; Zhang, Hawking <Hawking.Zhang at amd.com>
> > > > Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>
> > > > Subject: RE: [PATCH] drm/amd/amdgpu: fix Polaris10 PCIe dpm feature
> > > issue.
> > > >
> > > > Hi Alex,
> > > >
> > > > Expand the fix to all asics, please help to review.
> > > >
> > > > BR,
> > > > Jack Gui
> > > >
> > > > -----Original Message-----
> > > > From: Alex Deucher <alexdeucher at gmail.com>
> > > > Sent: Thursday, March 21, 2019 9:24 PM
> > > > To: Gui, Jack <Jack.Gui at amd.com>
> > > > Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>
> > > > Subject: Re: [PATCH] drm/amd/amdgpu: fix Polaris10 PCIe dpm feature
> > > issue.
> > > >
> > > > On Thu, Mar 21, 2019 at 1:35 AM Chengming Gui <Jack.Gui at amd.com>
> > > wrote:
> > > > >
> > > > > use pcie_bandwidth_available to get real link state to update pcie
> > > > > table.
> > > > >
> > > > > Signed-off-by: Chengming Gui <Jack.Gui at amd.com>
> > > > > ---
> > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++++++
> > > > >  1 file changed, 7 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > > > > index 964a4d3..df8e58b 100644
> > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > > > > @@ -3649,7 +3649,9 @@ static void
> > > > > amdgpu_device_get_min_pci_speed_width(struct amdgpu_device
> > > *adev,  {
> > > > >         struct pci_dev *pdev = adev->pdev;
> > > > >         enum pci_bus_speed cur_speed;
> > > > > +       enum pci_bus_speed tmp_speed;
> > > > >         enum pcie_link_width cur_width;
> > > > > +       u32 ret = 0;
> > > > >
> > > > >         *speed = PCI_SPEED_UNKNOWN;
> > > > >         *width = PCIE_LNK_WIDTH_UNKNOWN; @@ -3657,6 +3659,11 @@
> > > > > static void amdgpu_device_get_min_pci_speed_width(struct
> > > amdgpu_device *adev,
> > > > >         while (pdev) {
> > > > >                 cur_speed = pcie_get_speed_cap(pdev);
> > > > >                 cur_width = pcie_get_width_cap(pdev);
> > > > > +               if (adev->asic_type == CHIP_POLARIS10)
> > > > > +                       ret = pcie_bandwidth_available(adev->pdev, NULL,
> > > > > +                                                      &tmp_speed,
> > > > > + &cur_width);
> > > >
> > > > Any reason to not just do this for all asics?
> > > >
> > > > Alex
> > > >
> > > > > +               if (!ret)
> > > > > +                       cur_width = PCIE_LNK_WIDTH_RESRV;
> > > > >
> > > > >                 if (cur_speed != PCI_SPEED_UNKNOWN) {
> > > > >                         if (*speed == PCI_SPEED_UNKNOWN)
> > > > > --
> > > > > 2.7.4
> > > > >
> > > > > _______________________________________________
> > > > > amd-gfx mailing list
> > > > > amd-gfx at lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> > > _______________________________________________
> > > amd-gfx mailing list
> > > amd-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx