2019-11-07 22:24:08

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 0/2] drm: replace magic nuumbers

From: Bjorn Helgaas <[email protected]>

amdgpu and radeon do a bit of mucking with the PCIe Link Control 2
register, some of it using hard-coded magic numbers. The idea here is to
replace those with #defines.

I haven't signed off on these because the first one actually changes the
bits involved because the existing code looks like it might have a typo.
But I have no way to know for sure.

I don't intend the Target Link Speed patch to change anything, so it should
be straightforward to review.

Bjorn Helgaas (2):
drm: replace Compliance/Margin magic numbers with PCI_EXP_LNKCTL2
definitions
drm: replace Target Link Speed magic numbers with PCI_EXP_LNKCTL2
definitions

drivers/gpu/drm/amd/amdgpu/cik.c | 22 ++++++++++++++--------
drivers/gpu/drm/amd/amdgpu/si.c | 18 +++++++++++-------
drivers/gpu/drm/radeon/cik.c | 22 ++++++++++++++--------
drivers/gpu/drm/radeon/si.c | 22 ++++++++++++++--------
include/uapi/linux/pci_regs.h | 2 ++
5 files changed, 55 insertions(+), 31 deletions(-)

--
2.24.0.rc1.363.gb1bccd3e3d-goog


2019-11-07 22:24:22

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 2/2] drm: replace Target Link Speed magic numbers with PCI_EXP_LNKCTL2 definitions

From: Bjorn Helgaas <[email protected]>

Replace hard-coded magic numbers with the descript PCI_EXP_LNKCTL2
definitions. No functional change intended.
---
drivers/gpu/drm/amd/amdgpu/cik.c | 8 ++++----
drivers/gpu/drm/amd/amdgpu/si.c | 8 ++++----
drivers/gpu/drm/radeon/cik.c | 8 ++++----
drivers/gpu/drm/radeon/si.c | 8 ++++----
4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c
index e4a595cdd4c1..3067bb874032 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik.c
@@ -1527,13 +1527,13 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev)
WREG32_PCIE(ixPCIE_LC_SPEED_CNTL, speed_cntl);

pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16);
- tmp16 &= ~0xf;
+ tmp16 &= ~PCI_EXP_LNKCTL2_TLS;
if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3)
- tmp16 |= 3; /* gen3 */
+ tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */
else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2)
- tmp16 |= 2; /* gen2 */
+ tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */
else
- tmp16 |= 1; /* gen1 */
+ tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */
pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16);

speed_cntl = RREG32_PCIE(ixPCIE_LC_SPEED_CNTL);
diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c
index cf543410a424..d5c83d82063b 100644
--- a/drivers/gpu/drm/amd/amdgpu/si.c
+++ b/drivers/gpu/drm/amd/amdgpu/si.c
@@ -1762,13 +1762,13 @@ static void si_pcie_gen3_enable(struct amdgpu_device *adev)
WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl);

pci_read_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16);
- tmp16 &= ~0xf;
+ tmp16 &= ~PCI_EXP_LNKCTL2_TLS;
if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3)
- tmp16 |= 3;
+ tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */
else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2)
- tmp16 |= 2;
+ tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */
else
- tmp16 |= 1;
+ tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */
pci_write_config_word(adev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16);

speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL);
diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
index 95ffa0bff2d8..a280442c81aa 100644
--- a/drivers/gpu/drm/radeon/cik.c
+++ b/drivers/gpu/drm/radeon/cik.c
@@ -9647,13 +9647,13 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev)
WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl);

pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16);
- tmp16 &= ~0xf;
+ tmp16 &= ~PCI_EXP_LNKCTL2_TLS;
if (speed_cap == PCIE_SPEED_8_0GT)
- tmp16 |= 3; /* gen3 */
+ tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */
else if (speed_cap == PCIE_SPEED_5_0GT)
- tmp16 |= 2; /* gen2 */
+ tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */
else
- tmp16 |= 1; /* gen1 */
+ tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */
pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16);

speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL);
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index 69993d34d1e9..529e70a42019 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -7230,13 +7230,13 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev)
WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl);

pci_read_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, &tmp16);
- tmp16 &= ~0xf;
+ tmp16 &= ~PCI_EXP_LNKCTL2_TLS;
if (speed_cap == PCIE_SPEED_8_0GT)
- tmp16 |= 3; /* gen3 */
+ tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */
else if (speed_cap == PCIE_SPEED_5_0GT)
- tmp16 |= 2; /* gen2 */
+ tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */
else
- tmp16 |= 1; /* gen1 */
+ tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */
pci_write_config_word(rdev->pdev, gpu_pos + PCI_EXP_LNKCTL2, tmp16);

speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL);
--
2.24.0.rc1.363.gb1bccd3e3d-goog

2019-11-07 22:47:10

by Deucher, Alexander

[permalink] [raw]
Subject: RE: [PATCH 0/2] drm: replace magic nuumbers

> -----Original Message-----
> From: amd-gfx <[email protected]> On Behalf Of
> Bjorn Helgaas
> Sent: Thursday, November 7, 2019 5:21 PM
> To: Deucher, Alexander <[email protected]>; Koenig, Christian
> <[email protected]>; Zhou, David(ChunMing)
> <[email protected]>; David Airlie <[email protected]>; Daniel Vetter
> <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>; [email protected];
> [email protected]; Frederick Lawler <[email protected]>;
> [email protected]
> Subject: [PATCH 0/2] drm: replace magic nuumbers
>
> From: Bjorn Helgaas <[email protected]>
>
> amdgpu and radeon do a bit of mucking with the PCIe Link Control 2 register,
> some of it using hard-coded magic numbers. The idea here is to replace
> those with #defines.
>
> I haven't signed off on these because the first one actually changes the bits
> involved because the existing code looks like it might have a typo.
> But I have no way to know for sure.
>

It is a typo. Your patches look correct to me. The series is:
Reviewed-by: Alex Deucher <[email protected]>

> I don't intend the Target Link Speed patch to change anything, so it should be
> straightforward to review.
>
> Bjorn Helgaas (2):
> drm: replace Compliance/Margin magic numbers with PCI_EXP_LNKCTL2
> definitions
> drm: replace Target Link Speed magic numbers with PCI_EXP_LNKCTL2
> definitions
>
> drivers/gpu/drm/amd/amdgpu/cik.c | 22 ++++++++++++++--------
> drivers/gpu/drm/amd/amdgpu/si.c | 18 +++++++++++-------
> drivers/gpu/drm/radeon/cik.c | 22 ++++++++++++++--------
> drivers/gpu/drm/radeon/si.c | 22 ++++++++++++++--------
> include/uapi/linux/pci_regs.h | 2 ++
> 5 files changed, 55 insertions(+), 31 deletions(-)
>
> --
> 2.24.0.rc1.363.gb1bccd3e3d-goog
>
> _______________________________________________
> amd-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx