[PATCH] drm/amdgpu: Don't enable LTR if not supported
Alex Deucher
alexdeucher at gmail.com
Thu Sep 8 03:53:35 UTC 2022
On Wed, Sep 7, 2022 at 11:53 PM Alex Deucher <alexdeucher at gmail.com> wrote:
>
> On Wed, Sep 7, 2022 at 11:40 PM Lazar, Lijo <lijo.lazar at amd.com> wrote:
> >
> >
> >
> > On 9/8/2022 8:58 AM, Alex Deucher wrote:
> > > On Wed, Sep 7, 2022 at 11:24 PM Lijo Lazar <lijo.lazar at amd.com> wrote:
> > >>
> > >> As per PCIE Base Spec r4.0 Section 6.18
> > >> 'Software must not enable LTR in an Endpoint unless the Root Complex
> > >> and all intermediate Switches indicate support for LTR.'
> > >>
> > >> This fixes the Unsupported Request error reported through AER during
> > >> ASPM enablement.
> > >>
> > >> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216455&data=05%7C01%7Clijo.lazar%40amd.com%7Cc190635e13f047625b4508da914a47a5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637982045476774989%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=mNq7A7oT2VwZVc7WtYyWj0BRAXV5MzNLir0o4%2BKiWYU%3D&reserved=0
> > >>
> > >> The error was unnoticed before and got visible because of the commit
> > >> referenced below. This doesn't fix anything in the commit below, rather
> > >> fixes the issue in amdgpu exposed by the commit. The reference is only
> > >> to associate this commit with below one so that both go together.
> > >>
> > >> Fixes: 8795e182b02d ("PCI/portdrv: Don't disable AER reporting in get_port_device_capability()")
> > >>
> > >> Reported-by: Gustaw Smolarczyk <wielkiegie at gmail.com>
> > >> Signed-off-by: Lijo Lazar <lijo.lazar at amd.com>
> > >> Cc: stable at vger.kernel.org
> > >> ---
> > >
> > > Even though the ASPM code in si.c, cik.c, and vi.c doesn't mess with
> > > LTR, it still sets up ASPM so shouldn't it be protected with
> > > CONFIG_PCIEASPM as well?
> > >
> >
> > Yes, but it is only a compilation improvement and unrelated to this
> > patch. We don't access any ASPM related kernel variables in those
> > sequences. ltr_path variable used under this patch is declared under
> > ASPM config.
>
> Thanks for the information.
>
> Reviewed-by: Alex Deucher <alexander.deucher at amd.com?
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
>
> >
> > Runtime protection is already there -
> > 0ab5d711ec74 (drm/amd: Refactor `amdgpu_aspm` to be evaluated per device)
> >
> > Thanks,
> > Lijo
> >
> > > Alex
> > >
> > >> drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c | 9 ++++++++-
> > >> drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c | 9 ++++++++-
> > >> drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c | 9 ++++++++-
> > >> 3 files changed, 24 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> > >> index b465baa26762..aa761ff3a5fa 100644
> > >> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> > >> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> > >> @@ -380,6 +380,7 @@ static void nbio_v2_3_enable_aspm(struct amdgpu_device *adev,
> > >> WREG32_PCIE(smnPCIE_LC_CNTL, data);
> > >> }
> > >>
> > >> +#ifdef CONFIG_PCIEASPM
> > >> static void nbio_v2_3_program_ltr(struct amdgpu_device *adev)
> > >> {
> > >> uint32_t def, data;
> > >> @@ -401,9 +402,11 @@ static void nbio_v2_3_program_ltr(struct amdgpu_device *adev)
> > >> if (def != data)
> > >> WREG32_PCIE(smnBIF_CFG_DEV0_EPF0_DEVICE_CNTL2, data);
> > >> }
> > >> +#endif
> > >>
> > >> static void nbio_v2_3_program_aspm(struct amdgpu_device *adev)
> > >> {
> > >> +#ifdef CONFIG_PCIEASPM
> > >> uint32_t def, data;
> > >>
> > >> def = data = RREG32_PCIE(smnPCIE_LC_CNTL);
> > >> @@ -459,7 +462,10 @@ static void nbio_v2_3_program_aspm(struct amdgpu_device *adev)
> > >> if (def != data)
> > >> WREG32_PCIE(smnPCIE_LC_CNTL6, data);
> > >>
> > >> - nbio_v2_3_program_ltr(adev);
> > >> + /* Don't bother about LTR if LTR is not enabled
> > >> + * in the path */
> > >> + if (adev->pdev->ltr_path)
> > >> + nbio_v2_3_program_ltr(adev);
> > >>
> > >> def = data = RREG32_SOC15(NBIO, 0, mmRCC_BIF_STRAP3);
> > >> data |= 0x5DE0 << RCC_BIF_STRAP3__STRAP_VLINK_ASPM_IDLE_TIMER__SHIFT;
> > >> @@ -483,6 +489,7 @@ static void nbio_v2_3_program_aspm(struct amdgpu_device *adev)
> > >> data &= ~PCIE_LC_CNTL3__LC_DSC_DONT_ENTER_L23_AFTER_PME_ACK_MASK;
> > >> if (def != data)
> > >> WREG32_PCIE(smnPCIE_LC_CNTL3, data);
> > >> +#endif
> > >> }
> > >>
> > >> static void nbio_v2_3_apply_lc_spc_mode_wa(struct amdgpu_device *adev)
> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> > >> index f7f6ddebd3e4..37615a77287b 100644
> > >> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> > >> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> > >> @@ -282,6 +282,7 @@ static void nbio_v6_1_init_registers(struct amdgpu_device *adev)
> > >> mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
> > >> }
> > >>
> > >> +#ifdef CONFIG_PCIEASPM
> > >> static void nbio_v6_1_program_ltr(struct amdgpu_device *adev)
> > >> {
> > >> uint32_t def, data;
> > >> @@ -303,9 +304,11 @@ static void nbio_v6_1_program_ltr(struct amdgpu_device *adev)
> > >> if (def != data)
> > >> WREG32_PCIE(smnBIF_CFG_DEV0_EPF0_DEVICE_CNTL2, data);
> > >> }
> > >> +#endif
> > >>
> > >> static void nbio_v6_1_program_aspm(struct amdgpu_device *adev)
> > >> {
> > >> +#ifdef CONFIG_PCIEASPM
> > >> uint32_t def, data;
> > >>
> > >> def = data = RREG32_PCIE(smnPCIE_LC_CNTL);
> > >> @@ -361,7 +364,10 @@ static void nbio_v6_1_program_aspm(struct amdgpu_device *adev)
> > >> if (def != data)
> > >> WREG32_PCIE(smnPCIE_LC_CNTL6, data);
> > >>
> > >> - nbio_v6_1_program_ltr(adev);
> > >> + /* Don't bother about LTR if LTR is not enabled
> > >> + * in the path */
> > >> + if (adev->pdev->ltr_path)
> > >> + nbio_v6_1_program_ltr(adev);
> > >>
> > >> def = data = RREG32_PCIE(smnRCC_BIF_STRAP3);
> > >> data |= 0x5DE0 << RCC_BIF_STRAP3__STRAP_VLINK_ASPM_IDLE_TIMER__SHIFT;
> > >> @@ -385,6 +391,7 @@ static void nbio_v6_1_program_aspm(struct amdgpu_device *adev)
> > >> data &= ~PCIE_LC_CNTL3__LC_DSC_DONT_ENTER_L23_AFTER_PME_ACK_MASK;
> > >> if (def != data)
> > >> WREG32_PCIE(smnPCIE_LC_CNTL3, data);
> > >> +#endif
> > >> }
> > >>
> > >> const struct amdgpu_nbio_funcs nbio_v6_1_funcs = {
> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> > >> index 11848d1e238b..19455a725939 100644
> > >> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> > >> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> > >> @@ -673,6 +673,7 @@ struct amdgpu_nbio_ras nbio_v7_4_ras = {
> > >> };
> > >>
> > >>
> > >> +#ifdef CONFIG_PCIEASPM
> > >> static void nbio_v7_4_program_ltr(struct amdgpu_device *adev)
> > >> {
> > >> uint32_t def, data;
> > >> @@ -694,9 +695,11 @@ static void nbio_v7_4_program_ltr(struct amdgpu_device *adev)
> > >> if (def != data)
> > >> WREG32_PCIE(smnBIF_CFG_DEV0_EPF0_DEVICE_CNTL2, data);
> > >> }
> > >> +#endif
> > >>
> > >> static void nbio_v7_4_program_aspm(struct amdgpu_device *adev)
> > >> {
> > >> +#ifdef CONFIG_PCIEASPM
> > >> uint32_t def, data;
> > >>
> > >> if (adev->ip_versions[NBIO_HWIP][0] == IP_VERSION(7, 4, 4))
> > >> @@ -755,7 +758,10 @@ static void nbio_v7_4_program_aspm(struct amdgpu_device *adev)
> > >> if (def != data)
> > >> WREG32_PCIE(smnPCIE_LC_CNTL6, data);
> > >>
> > >> - nbio_v7_4_program_ltr(adev);
> > >> + /* Don't bother about LTR if LTR is not enabled
> > >> + * in the path */
> > >> + if (adev->pdev->ltr_path)
> > >> + nbio_v7_4_program_ltr(adev);
> > >>
> > >> def = data = RREG32_PCIE(smnRCC_BIF_STRAP3);
> > >> data |= 0x5DE0 << RCC_BIF_STRAP3__STRAP_VLINK_ASPM_IDLE_TIMER__SHIFT;
> > >> @@ -779,6 +785,7 @@ static void nbio_v7_4_program_aspm(struct amdgpu_device *adev)
> > >> data &= ~PCIE_LC_CNTL3__LC_DSC_DONT_ENTER_L23_AFTER_PME_ACK_MASK;
> > >> if (def != data)
> > >> WREG32_PCIE(smnPCIE_LC_CNTL3, data);
> > >> +#endif
> > >> }
> > >>
> > >> const struct amdgpu_nbio_funcs nbio_v7_4_funcs = {
> > >> --
> > >> 2.25.1
> > >>
More information about the amd-gfx
mailing list