lkml.org 
[lkml]   [2024]   [Feb]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRE: [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2
    Date
    [Public]

    > -----Original Message-----
    > From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
    > Sent: Thursday, February 15, 2024 8:32 AM
    > To: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-
    > gfx@lists.freedesktop.org; Daniel Vetter <daniel@ffwll.ch>; David Airlie
    > <airlied@gmail.com>; Dennis Dalessandro
    > <dennis.dalessandro@cornelisnetworks.com>; dri-
    > devel@lists.freedesktop.org; Jason Gunthorpe <jgg@ziepe.ca>; Leon
    > Romanovsky <leon@kernel.org>; linux-kernel@vger.kernel.org; linux-
    > rdma@vger.kernel.org; Pan, Xinhui <Xinhui.Pan@amd.com>; Koenig, Christian
    > <Christian.Koenig@amd.com>
    > Cc: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>; Lukas Wunner
    > <lukas@wunner.de>
    > Subject: [PATCH 1/3] drm/radeon: Use RMW accessors for changing LNKCTL2
    >
    > Convert open coded RMW accesses for LNKCTL2 to use
    > pcie_capability_clear_and_set_word() which makes its easier to understand
    > what the code tries to do.
    >
    > LNKCTL2 is not really owned by any driver because it is a collection of control
    > bits that PCI core might need to touch. RMW accessors already have support
    > for proper locking for a selected set of registers
    > (LNKCTL2 is not yet among them but likely will be in the future) to avoid losing
    > concurrent updates.
    >
    > Suggested-by: Lukas Wunner <lukas@wunner.de>
    > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

    The radeon and amdgpu patches are:
    Acked-by: Alex Deucher <alexander.deucher@amd.com>

    Are you looking for me to pick them up or do you want to land them as part of some larger change? Either way is fine with me.

    Alex

    > ---
    > drivers/gpu/drm/radeon/cik.c | 40 ++++++++++++++----------------------
    > drivers/gpu/drm/radeon/si.c | 40 ++++++++++++++----------------------
    > 2 files changed, 30 insertions(+), 50 deletions(-)
    >
    > diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
    > index 10be30366c2b..b5e96a8fc2c1 100644
    > --- a/drivers/gpu/drm/radeon/cik.c
    > +++ b/drivers/gpu/drm/radeon/cik.c
    > @@ -9592,28 +9592,18 @@ static void cik_pcie_gen3_enable(struct
    > radeon_device *rdev)
    >
    > PCI_EXP_LNKCTL_HAWD);
    >
    > /* linkctl2 */
    > - pcie_capability_read_word(root,
    > PCI_EXP_LNKCTL2,
    > - &tmp16);
    > - tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP
    > |
    > - PCI_EXP_LNKCTL2_TX_MARGIN);
    > - tmp16 |= (bridge_cfg2 &
    > - (PCI_EXP_LNKCTL2_ENTER_COMP |
    > - PCI_EXP_LNKCTL2_TX_MARGIN));
    > - pcie_capability_write_word(root,
    > - PCI_EXP_LNKCTL2,
    > - tmp16);
    > -
    > - pcie_capability_read_word(rdev->pdev,
    > - PCI_EXP_LNKCTL2,
    > - &tmp16);
    > - tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP
    > |
    > - PCI_EXP_LNKCTL2_TX_MARGIN);
    > - tmp16 |= (gpu_cfg2 &
    > - (PCI_EXP_LNKCTL2_ENTER_COMP |
    > - PCI_EXP_LNKCTL2_TX_MARGIN));
    > - pcie_capability_write_word(rdev->pdev,
    > - PCI_EXP_LNKCTL2,
    > - tmp16);
    > + pcie_capability_clear_and_set_word(root,
    > PCI_EXP_LNKCTL2,
    > +
    > PCI_EXP_LNKCTL2_ENTER_COMP |
    > +
    > PCI_EXP_LNKCTL2_TX_MARGIN,
    > + bridge_cfg2
    > |
    > +
    > (PCI_EXP_LNKCTL2_ENTER_COMP |
    > +
    > PCI_EXP_LNKCTL2_TX_MARGIN));
    > + pcie_capability_clear_and_set_word(rdev-
    > >pdev, PCI_EXP_LNKCTL2,
    > +
    > PCI_EXP_LNKCTL2_ENTER_COMP |
    > +
    > PCI_EXP_LNKCTL2_TX_MARGIN,
    > + gpu_cfg2 |
    > +
    > (PCI_EXP_LNKCTL2_ENTER_COMP |
    > +
    > PCI_EXP_LNKCTL2_TX_MARGIN));
    >
    > tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4);
    > tmp &= ~LC_SET_QUIESCE;
    > @@ -9627,15 +9617,15 @@ static void cik_pcie_gen3_enable(struct
    > radeon_device *rdev)
    > speed_cntl &= ~LC_FORCE_DIS_SW_SPEED_CHANGE;
    > WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl);
    >
    > - pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL2, &tmp16);
    > - tmp16 &= ~PCI_EXP_LNKCTL2_TLS;
    > + tmp16 = 0;
    > if (speed_cap == PCIE_SPEED_8_0GT)
    > tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */
    > else if (speed_cap == PCIE_SPEED_5_0GT)
    > tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */
    > else
    > tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */
    > - pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL2, tmp16);
    > + pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL2,
    > + PCI_EXP_LNKCTL2_TLS, tmp16);
    >
    > speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL);
    > speed_cntl |= LC_INITIATE_LINK_SPEED_CHANGE; diff --git
    > a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index
    > 85e9cba49cec..8eeaea64c75d 100644
    > --- a/drivers/gpu/drm/radeon/si.c
    > +++ b/drivers/gpu/drm/radeon/si.c
    > @@ -7193,28 +7193,18 @@ static void si_pcie_gen3_enable(struct
    > radeon_device *rdev)
    >
    > PCI_EXP_LNKCTL_HAWD);
    >
    > /* linkctl2 */
    > - pcie_capability_read_word(root,
    > PCI_EXP_LNKCTL2,
    > - &tmp16);
    > - tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP
    > |
    > - PCI_EXP_LNKCTL2_TX_MARGIN);
    > - tmp16 |= (bridge_cfg2 &
    > - (PCI_EXP_LNKCTL2_ENTER_COMP |
    > - PCI_EXP_LNKCTL2_TX_MARGIN));
    > - pcie_capability_write_word(root,
    > - PCI_EXP_LNKCTL2,
    > - tmp16);
    > -
    > - pcie_capability_read_word(rdev->pdev,
    > - PCI_EXP_LNKCTL2,
    > - &tmp16);
    > - tmp16 &= ~(PCI_EXP_LNKCTL2_ENTER_COMP
    > |
    > - PCI_EXP_LNKCTL2_TX_MARGIN);
    > - tmp16 |= (gpu_cfg2 &
    > - (PCI_EXP_LNKCTL2_ENTER_COMP |
    > - PCI_EXP_LNKCTL2_TX_MARGIN));
    > - pcie_capability_write_word(rdev->pdev,
    > - PCI_EXP_LNKCTL2,
    > - tmp16);
    > + pcie_capability_clear_and_set_word(root,
    > PCI_EXP_LNKCTL2,
    > +
    > PCI_EXP_LNKCTL2_ENTER_COMP |
    > +
    > PCI_EXP_LNKCTL2_TX_MARGIN,
    > + bridge_cfg2
    > &
    > +
    > (PCI_EXP_LNKCTL2_ENTER_COMP |
    > +
    > PCI_EXP_LNKCTL2_TX_MARGIN));
    > + pcie_capability_clear_and_set_word(rdev-
    > >pdev, PCI_EXP_LNKCTL2,
    > +
    > PCI_EXP_LNKCTL2_ENTER_COMP |
    > +
    > PCI_EXP_LNKCTL2_TX_MARGIN,
    > + gpu_cfg2 &
    > +
    > (PCI_EXP_LNKCTL2_ENTER_COMP |
    > +
    > PCI_EXP_LNKCTL2_TX_MARGIN));
    >
    > tmp = RREG32_PCIE_PORT(PCIE_LC_CNTL4);
    > tmp &= ~LC_SET_QUIESCE;
    > @@ -7228,15 +7218,15 @@ static void si_pcie_gen3_enable(struct
    > radeon_device *rdev)
    > speed_cntl &= ~LC_FORCE_DIS_SW_SPEED_CHANGE;
    > WREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL, speed_cntl);
    >
    > - pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL2, &tmp16);
    > - tmp16 &= ~PCI_EXP_LNKCTL2_TLS;
    > + tmp16 = 0;
    > if (speed_cap == PCIE_SPEED_8_0GT)
    > tmp16 |= PCI_EXP_LNKCTL2_TLS_8_0GT; /* gen3 */
    > else if (speed_cap == PCIE_SPEED_5_0GT)
    > tmp16 |= PCI_EXP_LNKCTL2_TLS_5_0GT; /* gen2 */
    > else
    > tmp16 |= PCI_EXP_LNKCTL2_TLS_2_5GT; /* gen1 */
    > - pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL2, tmp16);
    > + pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL2,
    > + PCI_EXP_LNKCTL2_TLS, tmp16);
    >
    > speed_cntl = RREG32_PCIE_PORT(PCIE_LC_SPEED_CNTL);
    > speed_cntl |= LC_INITIATE_LINK_SPEED_CHANGE;
    > --
    > 2.39.2

    \
     
     \ /
      Last update: 2024-05-27 15:05    [W:2.213 / U:0.184 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site