From patchwork Wed May 17 10:52:30 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ilpo_J=C3=A4rvinen?= X-Patchwork-Id: 13246292 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 55D32C7EE22 for ; Thu, 18 May 2023 08:20:34 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 6BC3110E097; Thu, 18 May 2023 08:20:26 +0000 (UTC) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id EC7C010E3E3; Wed, 17 May 2023 10:53:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1684320801; x=1715856801; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=tCBTG6EshP8yDnY64rQPzjgjv41iMK/kEF3IhZM1+uU=; b=CquO31RidDLaWlFo1nEsVFJzOIvDpuKsNQsn/vXXP20qJoMqCrYaoBSl CrtC/OV/pJcq2CRPDK5QQX2qXRqUNtPn/LuH9vixl4Vm4npG645EHFlre U4eOZhdo6j2wkl+CQukGucjSY4DDskMeJ6Iz8ERFjoewL563XiY9sRnDS 3+sqOCHwuPIRr9mlU1d3acXnmXIi4qHuWLWm94lGLImF3dNLK3rE4eWj5 jiD3rxdmMNyD+ql7nLD9NS05CxPJH9MKYHjfqNCpopU4VAeWQzuChBlQ2 pufC3Xq2KyX5FyjzFzXBcFNc1EwjzmZqhHjpG8RZeQTopNWt0J8fhRNmk g==; X-IronPort-AV: E=McAfee;i="6600,9927,10712"; a="438071667" X-IronPort-AV: E=Sophos;i="5.99,281,1677571200"; d="scan'208";a="438071667" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 May 2023 03:53:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10712"; a="734650965" X-IronPort-AV: E=Sophos;i="5.99,281,1677571200"; d="scan'208";a="734650965" Received: from lnstern-mobl1.amr.corp.intel.com (HELO ijarvine-MOBL2.ger.corp.intel.com) ([10.251.221.185]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 May 2023 03:53:13 -0700 From: =?utf-8?q?Ilpo_J=C3=A4rvinen?= To: linux-pci@vger.kernel.org, Bjorn Helgaas , Lorenzo Pieralisi , Rob Herring , =?utf-8?q?Krzysztof_Wilczy=C5=84ski?= , Emmanuel Grumbach , "Rafael J . Wysocki" , Heiner Kallweit , Lukas Wunner , Alex Deucher , =?utf-8?q?Christian_K=C3=B6nig?= , "Pan, Xinhui" , David Airlie , Daniel Vetter , Jammy Zhou , Ken Wang , amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: [PATCH v2 4/9] drm/amdgpu: Use RMW accessors for changing LNKCTL Date: Wed, 17 May 2023 13:52:30 +0300 Message-Id: <20230517105235.29176-5-ilpo.jarvinen@linux.intel.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230517105235.29176-1-ilpo.jarvinen@linux.intel.com> References: <20230517105235.29176-1-ilpo.jarvinen@linux.intel.com> MIME-Version: 1.0 X-Mailman-Approved-At: Thu, 18 May 2023 08:20:23 +0000 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Dean Luick , =?utf-8?q?Ilpo_J=C3=A4rvin?= =?utf-8?q?en?= , Andy Shevchenko , stable@vger.kernel.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Don't assume that only the driver would be accessing LNKCTL. ASPM policy changes can trigger write to LNKCTL outside of driver's control. And in the case of upstream bridge, the driver does not even own the device it's changing the registers for. Use RMW capability accessors which do proper locking to avoid losing concurrent updates to the register value. Fixes: a2e73f56fa62 ("drm/amdgpu: Add support for CIK parts") Fixes: 62a37553414a ("drm/amdgpu: add si implementation v10") Suggested-by: Lukas Wunner Signed-off-by: Ilpo Järvinen Cc: stable@vger.kernel.org --- drivers/gpu/drm/amd/amdgpu/cik.c | 36 +++++++++----------------------- drivers/gpu/drm/amd/amdgpu/si.c | 36 +++++++++----------------------- 2 files changed, 20 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c index de6d10390ab2..9be6da37032a 100644 --- a/drivers/gpu/drm/amd/amdgpu/cik.c +++ b/drivers/gpu/drm/amd/amdgpu/cik.c @@ -1574,17 +1574,8 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) u16 bridge_cfg2, gpu_cfg2; u32 max_lw, current_lw, tmp; - pcie_capability_read_word(root, PCI_EXP_LNKCTL, - &bridge_cfg); - pcie_capability_read_word(adev->pdev, PCI_EXP_LNKCTL, - &gpu_cfg); - - tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD; - pcie_capability_write_word(root, PCI_EXP_LNKCTL, tmp16); - - tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD; - pcie_capability_write_word(adev->pdev, PCI_EXP_LNKCTL, - tmp16); + pcie_capability_set_word(root, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_HAWD); + pcie_capability_set_word(adev->pdev, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_HAWD); tmp = RREG32_PCIE(ixPCIE_LC_STATUS1); max_lw = (tmp & PCIE_LC_STATUS1__LC_DETECTED_LINK_WIDTH_MASK) >> @@ -1637,21 +1628,14 @@ static void cik_pcie_gen3_enable(struct amdgpu_device *adev) msleep(100); /* linkctl */ - pcie_capability_read_word(root, PCI_EXP_LNKCTL, - &tmp16); - tmp16 &= ~PCI_EXP_LNKCTL_HAWD; - tmp16 |= (bridge_cfg & PCI_EXP_LNKCTL_HAWD); - pcie_capability_write_word(root, PCI_EXP_LNKCTL, - tmp16); - - pcie_capability_read_word(adev->pdev, - PCI_EXP_LNKCTL, - &tmp16); - tmp16 &= ~PCI_EXP_LNKCTL_HAWD; - tmp16 |= (gpu_cfg & PCI_EXP_LNKCTL_HAWD); - pcie_capability_write_word(adev->pdev, - PCI_EXP_LNKCTL, - tmp16); + pcie_capability_clear_and_set_word(root, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_HAWD, + bridge_cfg & + PCI_EXP_LNKCTL_HAWD); + pcie_capability_clear_and_set_word(adev->pdev, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_HAWD, + gpu_cfg & + PCI_EXP_LNKCTL_HAWD); /* linkctl2 */ pcie_capability_read_word(root, PCI_EXP_LNKCTL2, diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c index 7f99e130acd0..fd34c2100bd9 100644 --- a/drivers/gpu/drm/amd/amdgpu/si.c +++ b/drivers/gpu/drm/amd/amdgpu/si.c @@ -2276,17 +2276,8 @@ static void si_pcie_gen3_enable(struct amdgpu_device *adev) u16 bridge_cfg2, gpu_cfg2; u32 max_lw, current_lw, tmp; - pcie_capability_read_word(root, PCI_EXP_LNKCTL, - &bridge_cfg); - pcie_capability_read_word(adev->pdev, PCI_EXP_LNKCTL, - &gpu_cfg); - - tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD; - pcie_capability_write_word(root, PCI_EXP_LNKCTL, tmp16); - - tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD; - pcie_capability_write_word(adev->pdev, PCI_EXP_LNKCTL, - tmp16); + pcie_capability_set_word(root, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_HAWD); + pcie_capability_set_word(adev->pdev, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_HAWD); tmp = RREG32_PCIE(PCIE_LC_STATUS1); max_lw = (tmp & LC_DETECTED_LINK_WIDTH_MASK) >> LC_DETECTED_LINK_WIDTH_SHIFT; @@ -2331,21 +2322,14 @@ static void si_pcie_gen3_enable(struct amdgpu_device *adev) mdelay(100); - pcie_capability_read_word(root, PCI_EXP_LNKCTL, - &tmp16); - tmp16 &= ~PCI_EXP_LNKCTL_HAWD; - tmp16 |= (bridge_cfg & PCI_EXP_LNKCTL_HAWD); - pcie_capability_write_word(root, PCI_EXP_LNKCTL, - tmp16); - - pcie_capability_read_word(adev->pdev, - PCI_EXP_LNKCTL, - &tmp16); - tmp16 &= ~PCI_EXP_LNKCTL_HAWD; - tmp16 |= (gpu_cfg & PCI_EXP_LNKCTL_HAWD); - pcie_capability_write_word(adev->pdev, - PCI_EXP_LNKCTL, - tmp16); + pcie_capability_clear_and_set_word(root, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_HAWD, + bridge_cfg & + PCI_EXP_LNKCTL_HAWD); + pcie_capability_clear_and_set_word(adev->pdev, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_HAWD, + gpu_cfg & + PCI_EXP_LNKCTL_HAWD); pcie_capability_read_word(root, PCI_EXP_LNKCTL2, &tmp16); From patchwork Wed May 17 10:52:31 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ilpo_J=C3=A4rvinen?= X-Patchwork-Id: 13246294 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8557DC7EE2C for ; Thu, 18 May 2023 08:20:47 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 06F3C10E4E8; Thu, 18 May 2023 08:20:37 +0000 (UTC) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0F79610E3E3; Wed, 17 May 2023 10:53:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1684320807; x=1715856807; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=0Dt11mJGY3uST81NgKzX3Wy33nGbeSvB9w1sz8eDsQc=; b=Tn6SDXFde9AyxZDmchANB1jLyY++oxPK+lVbqORvuFbQorR8vkSeq4TZ wmMxmvV/dpsyKr8biUxv1YwaafoGoZ4zjcnRKhaW73vzdfjyYe+N7igwi LcHrDerkq4PR9gdnWWo0OKwWBHAlT+5dAGB4L6C3EPn5FS6SvNtlFV2SO i0OKdXmpRF7QqvIHA7z9N4YvXL/BK6ypDs7xxf8bgvVw2TNEApyWHR2gi bdJ/m6bzGTLzqUjBrMWCaiVMphlHIgtldEnfYNK5FEpBuCEjsvyzKQh8A KdxWaxmb+AyGLP0yqm3lxjV9+4m3bjNgBKBrDg9jO4Z+AfQYXvpT9rz2j A==; X-IronPort-AV: E=McAfee;i="6600,9927,10712"; a="438071675" X-IronPort-AV: E=Sophos;i="5.99,281,1677571200"; d="scan'208";a="438071675" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 May 2023 03:53:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10712"; a="734650981" X-IronPort-AV: E=Sophos;i="5.99,281,1677571200"; d="scan'208";a="734650981" Received: from lnstern-mobl1.amr.corp.intel.com (HELO ijarvine-MOBL2.ger.corp.intel.com) ([10.251.221.185]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 May 2023 03:53:21 -0700 From: =?utf-8?q?Ilpo_J=C3=A4rvinen?= To: linux-pci@vger.kernel.org, Bjorn Helgaas , Lorenzo Pieralisi , Rob Herring , =?utf-8?q?Krzysztof_Wilczy=C5=84ski?= , Emmanuel Grumbach , "Rafael J . Wysocki" , Heiner Kallweit , Lukas Wunner , Alex Deucher , =?utf-8?q?Christian_K=C3=B6nig?= , "Pan, Xinhui" , David Airlie , Daniel Vetter , amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: [PATCH v2 5/9] drm/radeon: Use RMW accessors for changing LNKCTL Date: Wed, 17 May 2023 13:52:31 +0300 Message-Id: <20230517105235.29176-6-ilpo.jarvinen@linux.intel.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230517105235.29176-1-ilpo.jarvinen@linux.intel.com> References: <20230517105235.29176-1-ilpo.jarvinen@linux.intel.com> MIME-Version: 1.0 X-Mailman-Approved-At: Thu, 18 May 2023 08:20:23 +0000 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Dean Luick , =?utf-8?q?Ilpo_J=C3=A4rvin?= =?utf-8?q?en?= , Andy Shevchenko , stable@vger.kernel.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Don't assume that only the driver would be accessing LNKCTL. ASPM policy changes can trigger write to LNKCTL outside of driver's control. And in the case of upstream bridge, the driver does not even own the device it's changing the registers for. Use RMW capability accessors which do proper locking to avoid losing concurrent updates to the register value. Fixes: 8a7cd27679d0 ("drm/radeon/cik: add support for pcie gen1/2/3 switching") Fixes: b9d305dfb66c ("drm/radeon: implement pcie gen2/3 support for SI") Suggested-by: Lukas Wunner Signed-off-by: Ilpo Järvinen Cc: stable@vger.kernel.org --- drivers/gpu/drm/radeon/cik.c | 36 ++++++++++------------------------- drivers/gpu/drm/radeon/si.c | 37 ++++++++++-------------------------- 2 files changed, 20 insertions(+), 53 deletions(-) diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index 5819737c21c6..a6f3c811ceb8 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -9534,17 +9534,8 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) u16 bridge_cfg2, gpu_cfg2; u32 max_lw, current_lw, tmp; - pcie_capability_read_word(root, PCI_EXP_LNKCTL, - &bridge_cfg); - pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL, - &gpu_cfg); - - tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD; - pcie_capability_write_word(root, PCI_EXP_LNKCTL, tmp16); - - tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD; - pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL, - tmp16); + pcie_capability_set_word(root, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_HAWD); + pcie_capability_set_word(rdev->pdev, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_HAWD); tmp = RREG32_PCIE_PORT(PCIE_LC_STATUS1); max_lw = (tmp & LC_DETECTED_LINK_WIDTH_MASK) >> LC_DETECTED_LINK_WIDTH_SHIFT; @@ -9591,21 +9582,14 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) msleep(100); /* linkctl */ - pcie_capability_read_word(root, PCI_EXP_LNKCTL, - &tmp16); - tmp16 &= ~PCI_EXP_LNKCTL_HAWD; - tmp16 |= (bridge_cfg & PCI_EXP_LNKCTL_HAWD); - pcie_capability_write_word(root, PCI_EXP_LNKCTL, - tmp16); - - pcie_capability_read_word(rdev->pdev, - PCI_EXP_LNKCTL, - &tmp16); - tmp16 &= ~PCI_EXP_LNKCTL_HAWD; - tmp16 |= (gpu_cfg & PCI_EXP_LNKCTL_HAWD); - pcie_capability_write_word(rdev->pdev, - PCI_EXP_LNKCTL, - tmp16); + pcie_capability_clear_and_set_word(root, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_HAWD, + bridge_cfg & + PCI_EXP_LNKCTL_HAWD); + pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_HAWD, + gpu_cfg & + PCI_EXP_LNKCTL_HAWD); /* linkctl2 */ pcie_capability_read_word(root, PCI_EXP_LNKCTL2, diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index 8d5e4b25609d..a91012447b56 100644 --- a/drivers/gpu/drm/radeon/si.c +++ b/drivers/gpu/drm/radeon/si.c @@ -7131,17 +7131,8 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev) u16 bridge_cfg2, gpu_cfg2; u32 max_lw, current_lw, tmp; - pcie_capability_read_word(root, PCI_EXP_LNKCTL, - &bridge_cfg); - pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL, - &gpu_cfg); - - tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD; - pcie_capability_write_word(root, PCI_EXP_LNKCTL, tmp16); - - tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD; - pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL, - tmp16); + pcie_capability_set_word(root, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_HAWD); + pcie_capability_set_word(rdev->pdev, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_HAWD); tmp = RREG32_PCIE(PCIE_LC_STATUS1); max_lw = (tmp & LC_DETECTED_LINK_WIDTH_MASK) >> LC_DETECTED_LINK_WIDTH_SHIFT; @@ -7188,22 +7179,14 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev) msleep(100); /* linkctl */ - pcie_capability_read_word(root, PCI_EXP_LNKCTL, - &tmp16); - tmp16 &= ~PCI_EXP_LNKCTL_HAWD; - tmp16 |= (bridge_cfg & PCI_EXP_LNKCTL_HAWD); - pcie_capability_write_word(root, - PCI_EXP_LNKCTL, - tmp16); - - pcie_capability_read_word(rdev->pdev, - PCI_EXP_LNKCTL, - &tmp16); - tmp16 &= ~PCI_EXP_LNKCTL_HAWD; - tmp16 |= (gpu_cfg & PCI_EXP_LNKCTL_HAWD); - pcie_capability_write_word(rdev->pdev, - PCI_EXP_LNKCTL, - tmp16); + pcie_capability_clear_and_set_word(root, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_HAWD, + bridge_cfg & + PCI_EXP_LNKCTL_HAWD); + pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_HAWD, + gpu_cfg & + PCI_EXP_LNKCTL_HAWD); /* linkctl2 */ pcie_capability_read_word(root, PCI_EXP_LNKCTL2,