[Public] > -----Original Message----- > From: Gong, Richard <Richard.Gong@xxxxxxx> > Sent: Friday, April 8, 2022 11:20 > To: Limonciello, Mario <Mario.Limonciello@xxxxxxx>; Deucher, Alexander > <Alexander.Deucher@xxxxxxx>; Koenig, Christian > <Christian.Koenig@xxxxxxx>; Pan, Xinhui <Xinhui.Pan@xxxxxxx>; > airlied@xxxxxxxx; daniel@xxxxxxxx > Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] drm/amdgpu: disable ASPM for legacy products that > don't support ASPM > > > On 4/8/2022 10:47 AM, Limonciello, Mario wrote: > > [Public] > > > > > > > >> -----Original Message----- > >> From: Gong, Richard <Richard.Gong@xxxxxxx> > >> Sent: Friday, April 8, 2022 10:45 > >> To: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, > Christian > >> <Christian.Koenig@xxxxxxx>; Pan, Xinhui <Xinhui.Pan@xxxxxxx>; > >> airlied@xxxxxxxx; daniel@xxxxxxxx > >> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; > linux- > >> kernel@xxxxxxxxxxxxxxx; Limonciello, Mario > <Mario.Limonciello@xxxxxxx>; > >> Gong, Richard <Richard.Gong@xxxxxxx> > >> Subject: [PATCH] drm/amdgpu: disable ASPM for legacy products that > don't > >> support ASPM > >> > >> Active State Power Management (ASPM) feature is enabled since kernel > >> 5.14. > >> However there are some legacy products (WX3200 and RX640 are > examples) > >> that > >> do not support ASPM. Use them as video/display output and system > would > >> hang > >> during suspend/resume. > >> > >> Add extra check to disable ASPM for old products that don't have > >> ASPM support. > >> > >> Signed-off-by: Richard Gong <richard.gong@xxxxxxx> > >> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1885 > >> --- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > >> index bb1c025d9001..8987107f41ee 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > >> @@ -2012,6 +2012,10 @@ static int amdgpu_pci_probe(struct pci_dev > >> *pdev, > >> if (amdgpu_aspm == -1 && !pcie_aspm_enabled(pdev)) > >> amdgpu_aspm = 0; > >> > >> + /* disable ASPM for the legacy products that don't support ASPM */ > >> + if ((flags & AMD_ASIC_MASK) == CHIP_POLARIS12) > >> + amdgpu_aspm = 0; > >> + > > I think it's problematic to disable it for the entire driver. There might be > multiple > > AMDGPUs in the system, and others may support ASPM. > > The "problem" are WX3200 and RX640, both are from the same POLARIS12 > family. > Right but what if some other "working" cards are included in the system too? Then ASPM gets disabled for them too. It should only be disabled for the pci_dev corresponding to problematic GPUs in problematic situations. > > Can it be done just as part of probe for Polaris? > > > >> if (amdgpu_virtual_display || > >> amdgpu_device_asic_has_dc_support(flags & AMD_ASIC_MASK)) > >> supports_atomic = true; > >> -- > >> 2.25.1
