* [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init
@ 2022-08-25 8:58 Lijo Lazar
2022-08-25 8:58 ` [PATCH 2/2] drm/amdgpu: Init VF's HDP flush reg offset early Lijo Lazar
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Lijo Lazar @ 2022-08-25 8:58 UTC (permalink / raw)
To: amd-gfx
Cc: Felix.Kuehling, tseewald, helgaas, Alexander.Deucher, sr,
Christian.Koenig, Hawking.Zhang
HDP flush is used early in the init sequence as part of memory controller
block initialization. Hence remapping of HDP registers needed for flush
needs to happen earlier.
This also fixes the AER error reported as Unsupported Request during
driver load.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216373
Reported-by: Tom Seewald <tseewald@gmail.com>
Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 9 +++++++++
drivers/gpu/drm/amd/amdgpu/nv.c | 6 ------
drivers/gpu/drm/amd/amdgpu/soc15.c | 6 ------
drivers/gpu/drm/amd/amdgpu/soc21.c | 6 ------
4 files changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index ce7d117efdb5..53d753e94a71 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2376,6 +2376,15 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
DRM_ERROR("amdgpu_vram_scratch_init failed %d\n", r);
goto init_failed;
}
+
+ /* remap HDP registers to a hole in mmio space,
+ * for the purpose of expose those registers
+ * to process space. This is needed for any early HDP
+ * flush operation during gmc initialization.
+ */
+ if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
+ adev->nbio.funcs->remap_hdp_registers(adev);
+
r = adev->ip_blocks[i].version->funcs->hw_init((void *)adev);
if (r) {
DRM_ERROR("hw_init %d failed %d\n", i, r);
diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index b3fba8dea63c..3ac7fef74277 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -1032,12 +1032,6 @@ static int nv_common_hw_init(void *handle)
nv_program_aspm(adev);
/* setup nbio registers */
adev->nbio.funcs->init_registers(adev);
- /* remap HDP registers to a hole in mmio space,
- * for the purpose of expose those registers
- * to process space
- */
- if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
- adev->nbio.funcs->remap_hdp_registers(adev);
/* enable the doorbell aperture */
nv_enable_doorbell_aperture(adev, true);
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
index fde6154f2009..a0481e37d7cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -1240,12 +1240,6 @@ static int soc15_common_hw_init(void *handle)
soc15_program_aspm(adev);
/* setup nbio registers */
adev->nbio.funcs->init_registers(adev);
- /* remap HDP registers to a hole in mmio space,
- * for the purpose of expose those registers
- * to process space
- */
- if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
- adev->nbio.funcs->remap_hdp_registers(adev);
/* enable the doorbell aperture */
soc15_enable_doorbell_aperture(adev, true);
diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
index 55284b24f113..16b447055102 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc21.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
@@ -660,12 +660,6 @@ static int soc21_common_hw_init(void *handle)
soc21_program_aspm(adev);
/* setup nbio registers */
adev->nbio.funcs->init_registers(adev);
- /* remap HDP registers to a hole in mmio space,
- * for the purpose of expose those registers
- * to process space
- */
- if (adev->nbio.funcs->remap_hdp_registers)
- adev->nbio.funcs->remap_hdp_registers(adev);
/* enable the doorbell aperture */
soc21_enable_doorbell_aperture(adev, true);
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] drm/amdgpu: Init VF's HDP flush reg offset early
2022-08-25 8:58 [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init Lijo Lazar
@ 2022-08-25 8:58 ` Lijo Lazar
2022-08-25 13:22 ` Alex Deucher
2022-08-25 14:07 ` [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init Alex Deucher
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Lijo Lazar @ 2022-08-25 8:58 UTC (permalink / raw)
To: amd-gfx
Cc: Felix.Kuehling, tseewald, helgaas, Alexander.Deucher, sr,
Christian.Koenig, Hawking.Zhang
Make sure the register offsets used for HDP flush in VF is
initialized early so that it works fine during any early HDP flush
sequence. For that, move the offset initialization to *_remap_hdp.
Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c | 23 +++++++++++++--------
drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c | 12 +++++++----
drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c | 23 +++++++++++++--------
drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c | 21 ++++++++++++-------
drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c | 24 ++++++++++++++--------
drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c | 23 +++++++++++++--------
7 files changed, 84 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 53d753e94a71..c0bb2e9616c5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2382,7 +2382,7 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
* to process space. This is needed for any early HDP
* flush operation during gmc initialization.
*/
- if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
+ if (adev->nbio.funcs->remap_hdp_registers)
adev->nbio.funcs->remap_hdp_registers(adev);
r = adev->ip_blocks[i].version->funcs->hw_init((void *)adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
index b465baa26762..20fa2c5ad510 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
@@ -65,10 +65,21 @@
static void nbio_v2_3_remap_hdp_registers(struct amdgpu_device *adev)
{
- WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
- adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
- WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
- adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
+ if (amdgpu_sriov_vf(adev))
+ adev->rmmio_remap.reg_offset =
+ SOC15_REG_OFFSET(
+ NBIO, 0,
+ mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL)
+ << 2;
+
+ if (!amdgpu_sriov_vf(adev)) {
+ WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
+ adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
+ WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
+ adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
+ }
}
static u32 nbio_v2_3_get_rev_id(struct amdgpu_device *adev)
@@ -338,10 +349,6 @@ static void nbio_v2_3_init_registers(struct amdgpu_device *adev)
if (def != data)
WREG32_PCIE(smnPCIE_CONFIG_CNTL, data);
-
- if (amdgpu_sriov_vf(adev))
- adev->rmmio_remap.reg_offset = SOC15_REG_OFFSET(NBIO, 0,
- mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
}
#define NAVI10_PCIE__LC_L0S_INACTIVITY_DEFAULT 0x00000000 // off by default, no gains over L1
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
index 982a89f841d5..e011d9856794 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
@@ -30,10 +30,14 @@
static void nbio_v4_3_remap_hdp_registers(struct amdgpu_device *adev)
{
- WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_MEM_FLUSH_CNTL,
- adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
- WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_REG_FLUSH_CNTL,
- adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
+ if (!amdgpu_sriov_vf(adev)) {
+ WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_MEM_FLUSH_CNTL,
+ adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
+ WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_REG_FLUSH_CNTL,
+ adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
+ }
}
static u32 nbio_v4_3_get_rev_id(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..7536ca3fcd69 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
@@ -55,10 +55,21 @@
static void nbio_v6_1_remap_hdp_registers(struct amdgpu_device *adev)
{
- WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
- adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
- WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
- adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
+ if (amdgpu_sriov_vf(adev))
+ adev->rmmio_remap.reg_offset =
+ SOC15_REG_OFFSET(
+ NBIO, 0,
+ mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL)
+ << 2;
+
+ if (!amdgpu_sriov_vf(adev)) {
+ WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
+ adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
+ WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
+ adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
+ }
}
static u32 nbio_v6_1_get_rev_id(struct amdgpu_device *adev)
@@ -276,10 +287,6 @@ static void nbio_v6_1_init_registers(struct amdgpu_device *adev)
if (def != data)
WREG32_PCIE(smnPCIE_CI_CNTL, data);
-
- if (amdgpu_sriov_vf(adev))
- adev->rmmio_remap.reg_offset = SOC15_REG_OFFSET(NBIO, 0,
- mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
}
static void nbio_v6_1_program_ltr(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c
index aa0326d00c72..6b4ac16a8466 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c
@@ -35,10 +35,20 @@
static void nbio_v7_0_remap_hdp_registers(struct amdgpu_device *adev)
{
- WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
- adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
- WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
- adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
+ if (amdgpu_sriov_vf(adev))
+ adev->rmmio_remap.reg_offset =
+ SOC15_REG_OFFSET(NBIO, 0,
+ mmHDP_MEM_COHERENCY_FLUSH_CNTL)
+ << 2;
+
+ if (!amdgpu_sriov_vf(adev)) {
+ WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
+ adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
+ WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
+ adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
+ }
}
static u32 nbio_v7_0_get_rev_id(struct amdgpu_device *adev)
@@ -273,9 +283,6 @@ const struct nbio_hdp_flush_reg nbio_v7_0_hdp_flush_reg = {
static void nbio_v7_0_init_registers(struct amdgpu_device *adev)
{
- if (amdgpu_sriov_vf(adev))
- adev->rmmio_remap.reg_offset =
- SOC15_REG_OFFSET(NBIO, 0, mmHDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
}
const struct amdgpu_nbio_funcs nbio_v7_0_funcs = {
diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
index 31776b12e4c4..fb4be72eade7 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
@@ -49,10 +49,21 @@
static void nbio_v7_2_remap_hdp_registers(struct amdgpu_device *adev)
{
- WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_MEM_FLUSH_CNTL,
- adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
- WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_REG_FLUSH_CNTL,
- adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
+ if (amdgpu_sriov_vf(adev))
+ adev->rmmio_remap.reg_offset =
+ SOC15_REG_OFFSET(
+ NBIO, 0,
+ regBIF_BX_PF0_HDP_MEM_COHERENCY_FLUSH_CNTL)
+ << 2;
+
+ if (!amdgpu_sriov_vf(adev)) {
+ WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_MEM_FLUSH_CNTL,
+ adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
+ WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_REG_FLUSH_CNTL,
+ adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
+ }
}
static u32 nbio_v7_2_get_rev_id(struct amdgpu_device *adev)
@@ -369,6 +380,7 @@ const struct nbio_hdp_flush_reg nbio_v7_2_hdp_flush_reg = {
static void nbio_v7_2_init_registers(struct amdgpu_device *adev)
{
uint32_t def, data;
+
switch (adev->ip_versions[NBIO_HWIP][0]) {
case IP_VERSION(7, 2, 1):
case IP_VERSION(7, 3, 0):
@@ -393,10 +405,6 @@ static void nbio_v7_2_init_registers(struct amdgpu_device *adev)
WREG32_PCIE_PORT(SOC15_REG_OFFSET(NBIO, 0, regPCIE_CONFIG_CNTL), data);
break;
}
-
- if (amdgpu_sriov_vf(adev))
- adev->rmmio_remap.reg_offset = SOC15_REG_OFFSET(NBIO, 0,
- regBIF_BX_PF0_HDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
}
const struct amdgpu_nbio_funcs nbio_v7_2_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..3c11af99582f 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
@@ -101,10 +101,21 @@ static void nbio_v7_4_query_ras_error_count(struct amdgpu_device *adev,
static void nbio_v7_4_remap_hdp_registers(struct amdgpu_device *adev)
{
- WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
- adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
- WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
- adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
+ if (amdgpu_sriov_vf(adev))
+ adev->rmmio_remap.reg_offset =
+ SOC15_REG_OFFSET(
+ NBIO, 0,
+ mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL)
+ << 2;
+
+ if (!amdgpu_sriov_vf(adev)) {
+ WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
+ adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
+ WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
+ adev->rmmio_remap.reg_offset +
+ KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
+ }
}
static u32 nbio_v7_4_get_rev_id(struct amdgpu_device *adev)
@@ -343,10 +354,6 @@ static void nbio_v7_4_init_registers(struct amdgpu_device *adev)
{
uint32_t baco_cntl;
- if (amdgpu_sriov_vf(adev))
- adev->rmmio_remap.reg_offset = SOC15_REG_OFFSET(NBIO, 0,
- mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
-
if (adev->ip_versions[NBIO_HWIP][0] == IP_VERSION(7, 4, 4) &&
!amdgpu_sriov_vf(adev)) {
baco_cntl = RREG32_SOC15(NBIO, 0, mmBACO_CNTL);
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: Init VF's HDP flush reg offset early
2022-08-25 8:58 ` [PATCH 2/2] drm/amdgpu: Init VF's HDP flush reg offset early Lijo Lazar
@ 2022-08-25 13:22 ` Alex Deucher
2022-08-25 14:07 ` Alex Deucher
0 siblings, 1 reply; 12+ messages in thread
From: Alex Deucher @ 2022-08-25 13:22 UTC (permalink / raw)
To: Lijo Lazar
Cc: Felix.Kuehling, amd-gfx, tseewald, helgaas, Alexander.Deucher,
sr, Christian.Koenig, Hawking.Zhang
Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
On Thu, Aug 25, 2022 at 4:58 AM Lijo Lazar <lijo.lazar@amd.com> wrote:
>
> Make sure the register offsets used for HDP flush in VF is
> initialized early so that it works fine during any early HDP flush
> sequence. For that, move the offset initialization to *_remap_hdp.
>
> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c | 23 +++++++++++++--------
> drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c | 12 +++++++----
> drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c | 23 +++++++++++++--------
> drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c | 21 ++++++++++++-------
> drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c | 24 ++++++++++++++--------
> drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c | 23 +++++++++++++--------
> 7 files changed, 84 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 53d753e94a71..c0bb2e9616c5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2382,7 +2382,7 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
> * to process space. This is needed for any early HDP
> * flush operation during gmc initialization.
> */
> - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
> + if (adev->nbio.funcs->remap_hdp_registers)
> adev->nbio.funcs->remap_hdp_registers(adev);
>
> r = adev->ip_blocks[i].version->funcs->hw_init((void *)adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> index b465baa26762..20fa2c5ad510 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> @@ -65,10 +65,21 @@
>
> static void nbio_v2_3_remap_hdp_registers(struct amdgpu_device *adev)
> {
> - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> + if (amdgpu_sriov_vf(adev))
> + adev->rmmio_remap.reg_offset =
> + SOC15_REG_OFFSET(
> + NBIO, 0,
> + mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL)
> + << 2;
> +
> + if (!amdgpu_sriov_vf(adev)) {
> + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> + adev->rmmio_remap.reg_offset +
> + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> + adev->rmmio_remap.reg_offset +
> + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> + }
> }
>
> static u32 nbio_v2_3_get_rev_id(struct amdgpu_device *adev)
> @@ -338,10 +349,6 @@ static void nbio_v2_3_init_registers(struct amdgpu_device *adev)
>
> if (def != data)
> WREG32_PCIE(smnPCIE_CONFIG_CNTL, data);
> -
> - if (amdgpu_sriov_vf(adev))
> - adev->rmmio_remap.reg_offset = SOC15_REG_OFFSET(NBIO, 0,
> - mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
> }
>
> #define NAVI10_PCIE__LC_L0S_INACTIVITY_DEFAULT 0x00000000 // off by default, no gains over L1
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> index 982a89f841d5..e011d9856794 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> @@ -30,10 +30,14 @@
>
> static void nbio_v4_3_remap_hdp_registers(struct amdgpu_device *adev)
> {
> - WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_MEM_FLUSH_CNTL,
> - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> - WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_REG_FLUSH_CNTL,
> - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> + if (!amdgpu_sriov_vf(adev)) {
> + WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_MEM_FLUSH_CNTL,
> + adev->rmmio_remap.reg_offset +
> + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> + WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_REG_FLUSH_CNTL,
> + adev->rmmio_remap.reg_offset +
> + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> + }
> }
>
> static u32 nbio_v4_3_get_rev_id(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..7536ca3fcd69 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> @@ -55,10 +55,21 @@
>
> static void nbio_v6_1_remap_hdp_registers(struct amdgpu_device *adev)
> {
> - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> + if (amdgpu_sriov_vf(adev))
> + adev->rmmio_remap.reg_offset =
> + SOC15_REG_OFFSET(
> + NBIO, 0,
> + mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL)
> + << 2;
> +
> + if (!amdgpu_sriov_vf(adev)) {
> + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> + adev->rmmio_remap.reg_offset +
> + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> + adev->rmmio_remap.reg_offset +
> + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> + }
> }
>
> static u32 nbio_v6_1_get_rev_id(struct amdgpu_device *adev)
> @@ -276,10 +287,6 @@ static void nbio_v6_1_init_registers(struct amdgpu_device *adev)
>
> if (def != data)
> WREG32_PCIE(smnPCIE_CI_CNTL, data);
> -
> - if (amdgpu_sriov_vf(adev))
> - adev->rmmio_remap.reg_offset = SOC15_REG_OFFSET(NBIO, 0,
> - mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
> }
>
> static void nbio_v6_1_program_ltr(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c
> index aa0326d00c72..6b4ac16a8466 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c
> @@ -35,10 +35,20 @@
>
> static void nbio_v7_0_remap_hdp_registers(struct amdgpu_device *adev)
> {
> - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> + if (amdgpu_sriov_vf(adev))
> + adev->rmmio_remap.reg_offset =
> + SOC15_REG_OFFSET(NBIO, 0,
> + mmHDP_MEM_COHERENCY_FLUSH_CNTL)
> + << 2;
> +
> + if (!amdgpu_sriov_vf(adev)) {
> + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> + adev->rmmio_remap.reg_offset +
> + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> + adev->rmmio_remap.reg_offset +
> + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> + }
> }
>
> static u32 nbio_v7_0_get_rev_id(struct amdgpu_device *adev)
> @@ -273,9 +283,6 @@ const struct nbio_hdp_flush_reg nbio_v7_0_hdp_flush_reg = {
>
> static void nbio_v7_0_init_registers(struct amdgpu_device *adev)
> {
> - if (amdgpu_sriov_vf(adev))
> - adev->rmmio_remap.reg_offset =
> - SOC15_REG_OFFSET(NBIO, 0, mmHDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
> }
>
> const struct amdgpu_nbio_funcs nbio_v7_0_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> index 31776b12e4c4..fb4be72eade7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> @@ -49,10 +49,21 @@
>
> static void nbio_v7_2_remap_hdp_registers(struct amdgpu_device *adev)
> {
> - WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_MEM_FLUSH_CNTL,
> - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> - WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_REG_FLUSH_CNTL,
> - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> + if (amdgpu_sriov_vf(adev))
> + adev->rmmio_remap.reg_offset =
> + SOC15_REG_OFFSET(
> + NBIO, 0,
> + regBIF_BX_PF0_HDP_MEM_COHERENCY_FLUSH_CNTL)
> + << 2;
> +
> + if (!amdgpu_sriov_vf(adev)) {
> + WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_MEM_FLUSH_CNTL,
> + adev->rmmio_remap.reg_offset +
> + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> + WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_REG_FLUSH_CNTL,
> + adev->rmmio_remap.reg_offset +
> + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> + }
> }
>
> static u32 nbio_v7_2_get_rev_id(struct amdgpu_device *adev)
> @@ -369,6 +380,7 @@ const struct nbio_hdp_flush_reg nbio_v7_2_hdp_flush_reg = {
> static void nbio_v7_2_init_registers(struct amdgpu_device *adev)
> {
> uint32_t def, data;
> +
> switch (adev->ip_versions[NBIO_HWIP][0]) {
> case IP_VERSION(7, 2, 1):
> case IP_VERSION(7, 3, 0):
> @@ -393,10 +405,6 @@ static void nbio_v7_2_init_registers(struct amdgpu_device *adev)
> WREG32_PCIE_PORT(SOC15_REG_OFFSET(NBIO, 0, regPCIE_CONFIG_CNTL), data);
> break;
> }
> -
> - if (amdgpu_sriov_vf(adev))
> - adev->rmmio_remap.reg_offset = SOC15_REG_OFFSET(NBIO, 0,
> - regBIF_BX_PF0_HDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
> }
>
> const struct amdgpu_nbio_funcs nbio_v7_2_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..3c11af99582f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> @@ -101,10 +101,21 @@ static void nbio_v7_4_query_ras_error_count(struct amdgpu_device *adev,
>
> static void nbio_v7_4_remap_hdp_registers(struct amdgpu_device *adev)
> {
> - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> + if (amdgpu_sriov_vf(adev))
> + adev->rmmio_remap.reg_offset =
> + SOC15_REG_OFFSET(
> + NBIO, 0,
> + mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL)
> + << 2;
> +
> + if (!amdgpu_sriov_vf(adev)) {
> + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> + adev->rmmio_remap.reg_offset +
> + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> + adev->rmmio_remap.reg_offset +
> + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> + }
> }
>
> static u32 nbio_v7_4_get_rev_id(struct amdgpu_device *adev)
> @@ -343,10 +354,6 @@ static void nbio_v7_4_init_registers(struct amdgpu_device *adev)
> {
> uint32_t baco_cntl;
>
> - if (amdgpu_sriov_vf(adev))
> - adev->rmmio_remap.reg_offset = SOC15_REG_OFFSET(NBIO, 0,
> - mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
> -
> if (adev->ip_versions[NBIO_HWIP][0] == IP_VERSION(7, 4, 4) &&
> !amdgpu_sriov_vf(adev)) {
> baco_cntl = RREG32_SOC15(NBIO, 0, mmBACO_CNTL);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init
2022-08-25 8:58 [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init Lijo Lazar
2022-08-25 8:58 ` [PATCH 2/2] drm/amdgpu: Init VF's HDP flush reg offset early Lijo Lazar
@ 2022-08-25 14:07 ` Alex Deucher
2022-08-25 14:22 ` Lazar, Lijo
2022-08-25 15:14 ` Felix Kuehling
2022-08-25 18:01 ` Bjorn Helgaas
3 siblings, 1 reply; 12+ messages in thread
From: Alex Deucher @ 2022-08-25 14:07 UTC (permalink / raw)
To: Lijo Lazar
Cc: Felix.Kuehling, amd-gfx, tseewald, helgaas, Alexander.Deucher,
sr, Christian.Koenig, Hawking.Zhang
On Thu, Aug 25, 2022 at 4:58 AM Lijo Lazar <lijo.lazar@amd.com> wrote:
>
> HDP flush is used early in the init sequence as part of memory controller
> block initialization. Hence remapping of HDP registers needed for flush
> needs to happen earlier.
>
> This also fixes the AER error reported as Unsupported Request during
> driver load.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216373
>
> Reported-by: Tom Seewald <tseewald@gmail.com>
> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 9 +++++++++
> drivers/gpu/drm/amd/amdgpu/nv.c | 6 ------
> drivers/gpu/drm/amd/amdgpu/soc15.c | 6 ------
> drivers/gpu/drm/amd/amdgpu/soc21.c | 6 ------
> 4 files changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index ce7d117efdb5..53d753e94a71 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2376,6 +2376,15 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
> DRM_ERROR("amdgpu_vram_scratch_init failed %d\n", r);
> goto init_failed;
> }
> +
> + /* remap HDP registers to a hole in mmio space,
> + * for the purpose of expose those registers
> + * to process space. This is needed for any early HDP
> + * flush operation during gmc initialization.
> + */
> + if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
> + adev->nbio.funcs->remap_hdp_registers(adev);
> +
We probably also need this in ip_resume() as well to handle the
suspend and resume case. Thinking about this more, maybe it's easier
to just track whether the remap has happened yet and use the old or
new offset based on that.
Alex
> r = adev->ip_blocks[i].version->funcs->hw_init((void *)adev);
> if (r) {
> DRM_ERROR("hw_init %d failed %d\n", i, r);
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index b3fba8dea63c..3ac7fef74277 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -1032,12 +1032,6 @@ static int nv_common_hw_init(void *handle)
> nv_program_aspm(adev);
> /* setup nbio registers */
> adev->nbio.funcs->init_registers(adev);
> - /* remap HDP registers to a hole in mmio space,
> - * for the purpose of expose those registers
> - * to process space
> - */
> - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
> - adev->nbio.funcs->remap_hdp_registers(adev);
> /* enable the doorbell aperture */
> nv_enable_doorbell_aperture(adev, true);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index fde6154f2009..a0481e37d7cf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -1240,12 +1240,6 @@ static int soc15_common_hw_init(void *handle)
> soc15_program_aspm(adev);
> /* setup nbio registers */
> adev->nbio.funcs->init_registers(adev);
> - /* remap HDP registers to a hole in mmio space,
> - * for the purpose of expose those registers
> - * to process space
> - */
> - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
> - adev->nbio.funcs->remap_hdp_registers(adev);
>
> /* enable the doorbell aperture */
> soc15_enable_doorbell_aperture(adev, true);
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
> index 55284b24f113..16b447055102 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
> @@ -660,12 +660,6 @@ static int soc21_common_hw_init(void *handle)
> soc21_program_aspm(adev);
> /* setup nbio registers */
> adev->nbio.funcs->init_registers(adev);
> - /* remap HDP registers to a hole in mmio space,
> - * for the purpose of expose those registers
> - * to process space
> - */
> - if (adev->nbio.funcs->remap_hdp_registers)
> - adev->nbio.funcs->remap_hdp_registers(adev);
> /* enable the doorbell aperture */
> soc21_enable_doorbell_aperture(adev, true);
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: Init VF's HDP flush reg offset early
2022-08-25 13:22 ` Alex Deucher
@ 2022-08-25 14:07 ` Alex Deucher
0 siblings, 0 replies; 12+ messages in thread
From: Alex Deucher @ 2022-08-25 14:07 UTC (permalink / raw)
To: Lijo Lazar
Cc: Felix.Kuehling, amd-gfx, tseewald, helgaas, Alexander.Deucher,
sr, Christian.Koenig, Hawking.Zhang
On Thu, Aug 25, 2022 at 9:22 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> Series is:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
Actually, hold off on that, I have a comment on patch 1.
Alex
> On Thu, Aug 25, 2022 at 4:58 AM Lijo Lazar <lijo.lazar@amd.com> wrote:
> >
> > Make sure the register offsets used for HDP flush in VF is
> > initialized early so that it works fine during any early HDP flush
> > sequence. For that, move the offset initialization to *_remap_hdp.
> >
> > Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c | 23 +++++++++++++--------
> > drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c | 12 +++++++----
> > drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c | 23 +++++++++++++--------
> > drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c | 21 ++++++++++++-------
> > drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c | 24 ++++++++++++++--------
> > drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c | 23 +++++++++++++--------
> > 7 files changed, 84 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 53d753e94a71..c0bb2e9616c5 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -2382,7 +2382,7 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
> > * to process space. This is needed for any early HDP
> > * flush operation during gmc initialization.
> > */
> > - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
> > + if (adev->nbio.funcs->remap_hdp_registers)
> > adev->nbio.funcs->remap_hdp_registers(adev);
> >
> > r = adev->ip_blocks[i].version->funcs->hw_init((void *)adev);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> > index b465baa26762..20fa2c5ad510 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> > @@ -65,10 +65,21 @@
> >
> > static void nbio_v2_3_remap_hdp_registers(struct amdgpu_device *adev)
> > {
> > - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> > - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> > - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> > - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> > + if (amdgpu_sriov_vf(adev))
> > + adev->rmmio_remap.reg_offset =
> > + SOC15_REG_OFFSET(
> > + NBIO, 0,
> > + mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL)
> > + << 2;
> > +
> > + if (!amdgpu_sriov_vf(adev)) {
> > + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> > + adev->rmmio_remap.reg_offset +
> > + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> > + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> > + adev->rmmio_remap.reg_offset +
> > + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> > + }
> > }
> >
> > static u32 nbio_v2_3_get_rev_id(struct amdgpu_device *adev)
> > @@ -338,10 +349,6 @@ static void nbio_v2_3_init_registers(struct amdgpu_device *adev)
> >
> > if (def != data)
> > WREG32_PCIE(smnPCIE_CONFIG_CNTL, data);
> > -
> > - if (amdgpu_sriov_vf(adev))
> > - adev->rmmio_remap.reg_offset = SOC15_REG_OFFSET(NBIO, 0,
> > - mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
> > }
> >
> > #define NAVI10_PCIE__LC_L0S_INACTIVITY_DEFAULT 0x00000000 // off by default, no gains over L1
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> > index 982a89f841d5..e011d9856794 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> > @@ -30,10 +30,14 @@
> >
> > static void nbio_v4_3_remap_hdp_registers(struct amdgpu_device *adev)
> > {
> > - WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_MEM_FLUSH_CNTL,
> > - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> > - WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_REG_FLUSH_CNTL,
> > - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> > + if (!amdgpu_sriov_vf(adev)) {
> > + WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_MEM_FLUSH_CNTL,
> > + adev->rmmio_remap.reg_offset +
> > + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> > + WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_REG_FLUSH_CNTL,
> > + adev->rmmio_remap.reg_offset +
> > + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> > + }
> > }
> >
> > static u32 nbio_v4_3_get_rev_id(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..7536ca3fcd69 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> > @@ -55,10 +55,21 @@
> >
> > static void nbio_v6_1_remap_hdp_registers(struct amdgpu_device *adev)
> > {
> > - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> > - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> > - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> > - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> > + if (amdgpu_sriov_vf(adev))
> > + adev->rmmio_remap.reg_offset =
> > + SOC15_REG_OFFSET(
> > + NBIO, 0,
> > + mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL)
> > + << 2;
> > +
> > + if (!amdgpu_sriov_vf(adev)) {
> > + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> > + adev->rmmio_remap.reg_offset +
> > + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> > + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> > + adev->rmmio_remap.reg_offset +
> > + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> > + }
> > }
> >
> > static u32 nbio_v6_1_get_rev_id(struct amdgpu_device *adev)
> > @@ -276,10 +287,6 @@ static void nbio_v6_1_init_registers(struct amdgpu_device *adev)
> >
> > if (def != data)
> > WREG32_PCIE(smnPCIE_CI_CNTL, data);
> > -
> > - if (amdgpu_sriov_vf(adev))
> > - adev->rmmio_remap.reg_offset = SOC15_REG_OFFSET(NBIO, 0,
> > - mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
> > }
> >
> > static void nbio_v6_1_program_ltr(struct amdgpu_device *adev)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c
> > index aa0326d00c72..6b4ac16a8466 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_0.c
> > @@ -35,10 +35,20 @@
> >
> > static void nbio_v7_0_remap_hdp_registers(struct amdgpu_device *adev)
> > {
> > - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> > - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> > - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> > - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> > + if (amdgpu_sriov_vf(adev))
> > + adev->rmmio_remap.reg_offset =
> > + SOC15_REG_OFFSET(NBIO, 0,
> > + mmHDP_MEM_COHERENCY_FLUSH_CNTL)
> > + << 2;
> > +
> > + if (!amdgpu_sriov_vf(adev)) {
> > + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> > + adev->rmmio_remap.reg_offset +
> > + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> > + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> > + adev->rmmio_remap.reg_offset +
> > + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> > + }
> > }
> >
> > static u32 nbio_v7_0_get_rev_id(struct amdgpu_device *adev)
> > @@ -273,9 +283,6 @@ const struct nbio_hdp_flush_reg nbio_v7_0_hdp_flush_reg = {
> >
> > static void nbio_v7_0_init_registers(struct amdgpu_device *adev)
> > {
> > - if (amdgpu_sriov_vf(adev))
> > - adev->rmmio_remap.reg_offset =
> > - SOC15_REG_OFFSET(NBIO, 0, mmHDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
> > }
> >
> > const struct amdgpu_nbio_funcs nbio_v7_0_funcs = {
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> > index 31776b12e4c4..fb4be72eade7 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> > @@ -49,10 +49,21 @@
> >
> > static void nbio_v7_2_remap_hdp_registers(struct amdgpu_device *adev)
> > {
> > - WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_MEM_FLUSH_CNTL,
> > - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> > - WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_REG_FLUSH_CNTL,
> > - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> > + if (amdgpu_sriov_vf(adev))
> > + adev->rmmio_remap.reg_offset =
> > + SOC15_REG_OFFSET(
> > + NBIO, 0,
> > + regBIF_BX_PF0_HDP_MEM_COHERENCY_FLUSH_CNTL)
> > + << 2;
> > +
> > + if (!amdgpu_sriov_vf(adev)) {
> > + WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_MEM_FLUSH_CNTL,
> > + adev->rmmio_remap.reg_offset +
> > + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> > + WREG32_SOC15(NBIO, 0, regBIF_BX0_REMAP_HDP_REG_FLUSH_CNTL,
> > + adev->rmmio_remap.reg_offset +
> > + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> > + }
> > }
> >
> > static u32 nbio_v7_2_get_rev_id(struct amdgpu_device *adev)
> > @@ -369,6 +380,7 @@ const struct nbio_hdp_flush_reg nbio_v7_2_hdp_flush_reg = {
> > static void nbio_v7_2_init_registers(struct amdgpu_device *adev)
> > {
> > uint32_t def, data;
> > +
> > switch (adev->ip_versions[NBIO_HWIP][0]) {
> > case IP_VERSION(7, 2, 1):
> > case IP_VERSION(7, 3, 0):
> > @@ -393,10 +405,6 @@ static void nbio_v7_2_init_registers(struct amdgpu_device *adev)
> > WREG32_PCIE_PORT(SOC15_REG_OFFSET(NBIO, 0, regPCIE_CONFIG_CNTL), data);
> > break;
> > }
> > -
> > - if (amdgpu_sriov_vf(adev))
> > - adev->rmmio_remap.reg_offset = SOC15_REG_OFFSET(NBIO, 0,
> > - regBIF_BX_PF0_HDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
> > }
> >
> > const struct amdgpu_nbio_funcs nbio_v7_2_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..3c11af99582f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> > @@ -101,10 +101,21 @@ static void nbio_v7_4_query_ras_error_count(struct amdgpu_device *adev,
> >
> > static void nbio_v7_4_remap_hdp_registers(struct amdgpu_device *adev)
> > {
> > - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> > - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> > - WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> > - adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> > + if (amdgpu_sriov_vf(adev))
> > + adev->rmmio_remap.reg_offset =
> > + SOC15_REG_OFFSET(
> > + NBIO, 0,
> > + mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL)
> > + << 2;
> > +
> > + if (!amdgpu_sriov_vf(adev)) {
> > + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_MEM_FLUSH_CNTL,
> > + adev->rmmio_remap.reg_offset +
> > + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
> > + WREG32_SOC15(NBIO, 0, mmREMAP_HDP_REG_FLUSH_CNTL,
> > + adev->rmmio_remap.reg_offset +
> > + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
> > + }
> > }
> >
> > static u32 nbio_v7_4_get_rev_id(struct amdgpu_device *adev)
> > @@ -343,10 +354,6 @@ static void nbio_v7_4_init_registers(struct amdgpu_device *adev)
> > {
> > uint32_t baco_cntl;
> >
> > - if (amdgpu_sriov_vf(adev))
> > - adev->rmmio_remap.reg_offset = SOC15_REG_OFFSET(NBIO, 0,
> > - mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL) << 2;
> > -
> > if (adev->ip_versions[NBIO_HWIP][0] == IP_VERSION(7, 4, 4) &&
> > !amdgpu_sriov_vf(adev)) {
> > baco_cntl = RREG32_SOC15(NBIO, 0, mmBACO_CNTL);
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init
2022-08-25 14:07 ` [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init Alex Deucher
@ 2022-08-25 14:22 ` Lazar, Lijo
2022-08-25 14:26 ` Alex Deucher
0 siblings, 1 reply; 12+ messages in thread
From: Lazar, Lijo @ 2022-08-25 14:22 UTC (permalink / raw)
To: Alex Deucher
Cc: Felix.Kuehling, amd-gfx, tseewald, helgaas, Alexander.Deucher,
sr, Christian.Koenig, Hawking.Zhang
On 8/25/2022 7:37 PM, Alex Deucher wrote:
> On Thu, Aug 25, 2022 at 4:58 AM Lijo Lazar <lijo.lazar@amd.com> wrote:
>>
>> HDP flush is used early in the init sequence as part of memory controller
>> block initialization. Hence remapping of HDP registers needed for flush
>> needs to happen earlier.
>>
>> This also fixes the AER error reported as Unsupported Request during
>> driver load.
>>
>> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216373&data=05%7C01%7Clijo.lazar%40amd.com%7Caeec5a5e8ec7402e546708da86a31e41%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637970332414985963%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=EQuUjHTaVPSKZdCUhL6iI4EJ56UMhKTLl86uVpSL8AU%3D&reserved=0
>>
>> Reported-by: Tom Seewald <tseewald@gmail.com>
>> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 9 +++++++++
>> drivers/gpu/drm/amd/amdgpu/nv.c | 6 ------
>> drivers/gpu/drm/amd/amdgpu/soc15.c | 6 ------
>> drivers/gpu/drm/amd/amdgpu/soc21.c | 6 ------
>> 4 files changed, 9 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index ce7d117efdb5..53d753e94a71 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2376,6 +2376,15 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
>> DRM_ERROR("amdgpu_vram_scratch_init failed %d\n", r);
>> goto init_failed;
>> }
>> +
>> + /* remap HDP registers to a hole in mmio space,
>> + * for the purpose of expose those registers
>> + * to process space. This is needed for any early HDP
>> + * flush operation during gmc initialization.
>> + */
>> + if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
>> + adev->nbio.funcs->remap_hdp_registers(adev);
>> +
>
> We probably also need this in ip_resume() as well to handle the
> suspend and resume case. Thinking about this more, maybe it's easier
> to just track whether the remap has happened yet and use the old or
> new offset based on that.
If we can use the default offset without a remap, does it make sense to
remap? What about calling the same in ip_resume?
Thanks,
Lijo
>
> Alex
>
>
>> r = adev->ip_blocks[i].version->funcs->hw_init((void *)adev);
>> if (r) {
>> DRM_ERROR("hw_init %d failed %d\n", i, r);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
>> index b3fba8dea63c..3ac7fef74277 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
>> @@ -1032,12 +1032,6 @@ static int nv_common_hw_init(void *handle)
>> nv_program_aspm(adev);
>> /* setup nbio registers */
>> adev->nbio.funcs->init_registers(adev);
>> - /* remap HDP registers to a hole in mmio space,
>> - * for the purpose of expose those registers
>> - * to process space
>> - */
>> - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
>> - adev->nbio.funcs->remap_hdp_registers(adev);
>> /* enable the doorbell aperture */
>> nv_enable_doorbell_aperture(adev, true);
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
>> index fde6154f2009..a0481e37d7cf 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
>> @@ -1240,12 +1240,6 @@ static int soc15_common_hw_init(void *handle)
>> soc15_program_aspm(adev);
>> /* setup nbio registers */
>> adev->nbio.funcs->init_registers(adev);
>> - /* remap HDP registers to a hole in mmio space,
>> - * for the purpose of expose those registers
>> - * to process space
>> - */
>> - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
>> - adev->nbio.funcs->remap_hdp_registers(adev);
>>
>> /* enable the doorbell aperture */
>> soc15_enable_doorbell_aperture(adev, true);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
>> index 55284b24f113..16b447055102 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
>> @@ -660,12 +660,6 @@ static int soc21_common_hw_init(void *handle)
>> soc21_program_aspm(adev);
>> /* setup nbio registers */
>> adev->nbio.funcs->init_registers(adev);
>> - /* remap HDP registers to a hole in mmio space,
>> - * for the purpose of expose those registers
>> - * to process space
>> - */
>> - if (adev->nbio.funcs->remap_hdp_registers)
>> - adev->nbio.funcs->remap_hdp_registers(adev);
>> /* enable the doorbell aperture */
>> soc21_enable_doorbell_aperture(adev, true);
>>
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init
2022-08-25 14:22 ` Lazar, Lijo
@ 2022-08-25 14:26 ` Alex Deucher
2022-08-25 14:53 ` Christian König
0 siblings, 1 reply; 12+ messages in thread
From: Alex Deucher @ 2022-08-25 14:26 UTC (permalink / raw)
To: Lazar, Lijo
Cc: Felix.Kuehling, amd-gfx, tseewald, helgaas, Alexander.Deucher,
sr, Christian.Koenig, Hawking.Zhang
On Thu, Aug 25, 2022 at 10:22 AM Lazar, Lijo <lijo.lazar@amd.com> wrote:
>
>
>
> On 8/25/2022 7:37 PM, Alex Deucher wrote:
> > On Thu, Aug 25, 2022 at 4:58 AM Lijo Lazar <lijo.lazar@amd.com> wrote:
> >>
> >> HDP flush is used early in the init sequence as part of memory controller
> >> block initialization. Hence remapping of HDP registers needed for flush
> >> needs to happen earlier.
> >>
> >> This also fixes the AER error reported as Unsupported Request during
> >> driver load.
> >>
> >> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216373&data=05%7C01%7Clijo.lazar%40amd.com%7Caeec5a5e8ec7402e546708da86a31e41%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637970332414985963%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=EQuUjHTaVPSKZdCUhL6iI4EJ56UMhKTLl86uVpSL8AU%3D&reserved=0
> >>
> >> Reported-by: Tom Seewald <tseewald@gmail.com>
> >> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 9 +++++++++
> >> drivers/gpu/drm/amd/amdgpu/nv.c | 6 ------
> >> drivers/gpu/drm/amd/amdgpu/soc15.c | 6 ------
> >> drivers/gpu/drm/amd/amdgpu/soc21.c | 6 ------
> >> 4 files changed, 9 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> index ce7d117efdb5..53d753e94a71 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> @@ -2376,6 +2376,15 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
> >> DRM_ERROR("amdgpu_vram_scratch_init failed %d\n", r);
> >> goto init_failed;
> >> }
> >> +
> >> + /* remap HDP registers to a hole in mmio space,
> >> + * for the purpose of expose those registers
> >> + * to process space. This is needed for any early HDP
> >> + * flush operation during gmc initialization.
> >> + */
> >> + if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
> >> + adev->nbio.funcs->remap_hdp_registers(adev);
> >> +
> >
> > We probably also need this in ip_resume() as well to handle the
> > suspend and resume case. Thinking about this more, maybe it's easier
> > to just track whether the remap has happened yet and use the old or
> > new offset based on that.
>
> If we can use the default offset without a remap, does it make sense to
> remap? What about calling the same in ip_resume?
The remap is necessary so that userspace drivers can access this to
flush the HDP registers when they need to since normally it's in a
non-accessible region of the MMIO space. I'm fine with updating it in
ip_resume as well.
Alex
>
> Thanks,
> Lijo
>
> >
> > Alex
> >
> >
> >> r = adev->ip_blocks[i].version->funcs->hw_init((void *)adev);
> >> if (r) {
> >> DRM_ERROR("hw_init %d failed %d\n", i, r);
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> >> index b3fba8dea63c..3ac7fef74277 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> >> @@ -1032,12 +1032,6 @@ static int nv_common_hw_init(void *handle)
> >> nv_program_aspm(adev);
> >> /* setup nbio registers */
> >> adev->nbio.funcs->init_registers(adev);
> >> - /* remap HDP registers to a hole in mmio space,
> >> - * for the purpose of expose those registers
> >> - * to process space
> >> - */
> >> - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
> >> - adev->nbio.funcs->remap_hdp_registers(adev);
> >> /* enable the doorbell aperture */
> >> nv_enable_doorbell_aperture(adev, true);
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> >> index fde6154f2009..a0481e37d7cf 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> >> @@ -1240,12 +1240,6 @@ static int soc15_common_hw_init(void *handle)
> >> soc15_program_aspm(adev);
> >> /* setup nbio registers */
> >> adev->nbio.funcs->init_registers(adev);
> >> - /* remap HDP registers to a hole in mmio space,
> >> - * for the purpose of expose those registers
> >> - * to process space
> >> - */
> >> - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
> >> - adev->nbio.funcs->remap_hdp_registers(adev);
> >>
> >> /* enable the doorbell aperture */
> >> soc15_enable_doorbell_aperture(adev, true);
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
> >> index 55284b24f113..16b447055102 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
> >> @@ -660,12 +660,6 @@ static int soc21_common_hw_init(void *handle)
> >> soc21_program_aspm(adev);
> >> /* setup nbio registers */
> >> adev->nbio.funcs->init_registers(adev);
> >> - /* remap HDP registers to a hole in mmio space,
> >> - * for the purpose of expose those registers
> >> - * to process space
> >> - */
> >> - if (adev->nbio.funcs->remap_hdp_registers)
> >> - adev->nbio.funcs->remap_hdp_registers(adev);
> >> /* enable the doorbell aperture */
> >> soc21_enable_doorbell_aperture(adev, true);
> >>
> >> --
> >> 2.25.1
> >>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init
2022-08-25 14:26 ` Alex Deucher
@ 2022-08-25 14:53 ` Christian König
0 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2022-08-25 14:53 UTC (permalink / raw)
To: Alex Deucher, Lazar, Lijo
Cc: Felix.Kuehling, amd-gfx, tseewald, helgaas, Alexander.Deucher,
sr, Christian.Koenig, Hawking.Zhang
Am 25.08.22 um 16:26 schrieb Alex Deucher:
> On Thu, Aug 25, 2022 at 10:22 AM Lazar, Lijo <lijo.lazar@amd.com> wrote:
>>
>>
>> On 8/25/2022 7:37 PM, Alex Deucher wrote:
>>> On Thu, Aug 25, 2022 at 4:58 AM Lijo Lazar <lijo.lazar@amd.com> wrote:
>>>> HDP flush is used early in the init sequence as part of memory controller
>>>> block initialization. Hence remapping of HDP registers needed for flush
>>>> needs to happen earlier.
>>>>
>>>> This also fixes the AER error reported as Unsupported Request during
>>>> driver load.
>>>>
>>>> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216373&data=05%7C01%7Clijo.lazar%40amd.com%7Caeec5a5e8ec7402e546708da86a31e41%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637970332414985963%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=EQuUjHTaVPSKZdCUhL6iI4EJ56UMhKTLl86uVpSL8AU%3D&reserved=0
>>>>
>>>> Reported-by: Tom Seewald <tseewald@gmail.com>
>>>> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 9 +++++++++
>>>> drivers/gpu/drm/amd/amdgpu/nv.c | 6 ------
>>>> drivers/gpu/drm/amd/amdgpu/soc15.c | 6 ------
>>>> drivers/gpu/drm/amd/amdgpu/soc21.c | 6 ------
>>>> 4 files changed, 9 insertions(+), 18 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> index ce7d117efdb5..53d753e94a71 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> @@ -2376,6 +2376,15 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
>>>> DRM_ERROR("amdgpu_vram_scratch_init failed %d\n", r);
>>>> goto init_failed;
>>>> }
>>>> +
>>>> + /* remap HDP registers to a hole in mmio space,
>>>> + * for the purpose of expose those registers
>>>> + * to process space. This is needed for any early HDP
>>>> + * flush operation during gmc initialization.
>>>> + */
>>>> + if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
>>>> + adev->nbio.funcs->remap_hdp_registers(adev);
>>>> +
>>> We probably also need this in ip_resume() as well to handle the
>>> suspend and resume case. Thinking about this more, maybe it's easier
>>> to just track whether the remap has happened yet and use the old or
>>> new offset based on that.
>> If we can use the default offset without a remap, does it make sense to
>> remap? What about calling the same in ip_resume?
> The remap is necessary so that userspace drivers can access this to
> flush the HDP registers when they need to since normally it's in a
> non-accessible region of the MMIO space. I'm fine with updating it in
> ip_resume as well.
Correct me if I'm wrong but I think remap means it is available at an
additional location, the privileged region still works as well.
So Lijo suggestion is to use the privileged area in the kernel instead
of the remapped one.
Right?
Christian.
>
> Alex
>
>
>> Thanks,
>> Lijo
>>
>>> Alex
>>>
>>>
>>>> r = adev->ip_blocks[i].version->funcs->hw_init((void *)adev);
>>>> if (r) {
>>>> DRM_ERROR("hw_init %d failed %d\n", i, r);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
>>>> index b3fba8dea63c..3ac7fef74277 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
>>>> @@ -1032,12 +1032,6 @@ static int nv_common_hw_init(void *handle)
>>>> nv_program_aspm(adev);
>>>> /* setup nbio registers */
>>>> adev->nbio.funcs->init_registers(adev);
>>>> - /* remap HDP registers to a hole in mmio space,
>>>> - * for the purpose of expose those registers
>>>> - * to process space
>>>> - */
>>>> - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
>>>> - adev->nbio.funcs->remap_hdp_registers(adev);
>>>> /* enable the doorbell aperture */
>>>> nv_enable_doorbell_aperture(adev, true);
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
>>>> index fde6154f2009..a0481e37d7cf 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
>>>> @@ -1240,12 +1240,6 @@ static int soc15_common_hw_init(void *handle)
>>>> soc15_program_aspm(adev);
>>>> /* setup nbio registers */
>>>> adev->nbio.funcs->init_registers(adev);
>>>> - /* remap HDP registers to a hole in mmio space,
>>>> - * for the purpose of expose those registers
>>>> - * to process space
>>>> - */
>>>> - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
>>>> - adev->nbio.funcs->remap_hdp_registers(adev);
>>>>
>>>> /* enable the doorbell aperture */
>>>> soc15_enable_doorbell_aperture(adev, true);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
>>>> index 55284b24f113..16b447055102 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
>>>> @@ -660,12 +660,6 @@ static int soc21_common_hw_init(void *handle)
>>>> soc21_program_aspm(adev);
>>>> /* setup nbio registers */
>>>> adev->nbio.funcs->init_registers(adev);
>>>> - /* remap HDP registers to a hole in mmio space,
>>>> - * for the purpose of expose those registers
>>>> - * to process space
>>>> - */
>>>> - if (adev->nbio.funcs->remap_hdp_registers)
>>>> - adev->nbio.funcs->remap_hdp_registers(adev);
>>>> /* enable the doorbell aperture */
>>>> soc21_enable_doorbell_aperture(adev, true);
>>>>
>>>> --
>>>> 2.25.1
>>>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init
2022-08-25 8:58 [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init Lijo Lazar
2022-08-25 8:58 ` [PATCH 2/2] drm/amdgpu: Init VF's HDP flush reg offset early Lijo Lazar
2022-08-25 14:07 ` [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init Alex Deucher
@ 2022-08-25 15:14 ` Felix Kuehling
2022-08-25 18:01 ` Bjorn Helgaas
3 siblings, 0 replies; 12+ messages in thread
From: Felix Kuehling @ 2022-08-25 15:14 UTC (permalink / raw)
To: Lijo Lazar, amd-gfx
Cc: tseewald, helgaas, Alexander.Deucher, sr, Christian.Koenig,
Hawking.Zhang
Am 2022-08-25 um 04:58 schrieb Lijo Lazar:
> HDP flush is used early in the init sequence as part of memory controller
> block initialization. Hence remapping of HDP registers needed for flush
> needs to happen earlier.
>
> This also fixes the AER error reported as Unsupported Request during
> driver load.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216373
>
> Reported-by: Tom Seewald <tseewald@gmail.com>
> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 9 +++++++++
> drivers/gpu/drm/amd/amdgpu/nv.c | 6 ------
> drivers/gpu/drm/amd/amdgpu/soc15.c | 6 ------
> drivers/gpu/drm/amd/amdgpu/soc21.c | 6 ------
> 4 files changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index ce7d117efdb5..53d753e94a71 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2376,6 +2376,15 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
> DRM_ERROR("amdgpu_vram_scratch_init failed %d\n", r);
> goto init_failed;
> }
> +
> + /* remap HDP registers to a hole in mmio space,
> + * for the purpose of expose those registers
> + * to process space. This is needed for any early HDP
> + * flush operation during gmc initialization.
> + */
> + if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
Does this work on GFXv8? You may need a NULL-check for adev->nbio.funcs.
Regards,
Felix
> + adev->nbio.funcs->remap_hdp_registers(adev);
> +
> r = adev->ip_blocks[i].version->funcs->hw_init((void *)adev);
> if (r) {
> DRM_ERROR("hw_init %d failed %d\n", i, r);
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index b3fba8dea63c..3ac7fef74277 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -1032,12 +1032,6 @@ static int nv_common_hw_init(void *handle)
> nv_program_aspm(adev);
> /* setup nbio registers */
> adev->nbio.funcs->init_registers(adev);
> - /* remap HDP registers to a hole in mmio space,
> - * for the purpose of expose those registers
> - * to process space
> - */
> - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
> - adev->nbio.funcs->remap_hdp_registers(adev);
> /* enable the doorbell aperture */
> nv_enable_doorbell_aperture(adev, true);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index fde6154f2009..a0481e37d7cf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -1240,12 +1240,6 @@ static int soc15_common_hw_init(void *handle)
> soc15_program_aspm(adev);
> /* setup nbio registers */
> adev->nbio.funcs->init_registers(adev);
> - /* remap HDP registers to a hole in mmio space,
> - * for the purpose of expose those registers
> - * to process space
> - */
> - if (adev->nbio.funcs->remap_hdp_registers && !amdgpu_sriov_vf(adev))
> - adev->nbio.funcs->remap_hdp_registers(adev);
>
> /* enable the doorbell aperture */
> soc15_enable_doorbell_aperture(adev, true);
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
> index 55284b24f113..16b447055102 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
> @@ -660,12 +660,6 @@ static int soc21_common_hw_init(void *handle)
> soc21_program_aspm(adev);
> /* setup nbio registers */
> adev->nbio.funcs->init_registers(adev);
> - /* remap HDP registers to a hole in mmio space,
> - * for the purpose of expose those registers
> - * to process space
> - */
> - if (adev->nbio.funcs->remap_hdp_registers)
> - adev->nbio.funcs->remap_hdp_registers(adev);
> /* enable the doorbell aperture */
> soc21_enable_doorbell_aperture(adev, true);
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init
2022-08-25 8:58 [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init Lijo Lazar
` (2 preceding siblings ...)
2022-08-25 15:14 ` Felix Kuehling
@ 2022-08-25 18:01 ` Bjorn Helgaas
2022-09-08 14:08 ` Deucher, Alexander
3 siblings, 1 reply; 12+ messages in thread
From: Bjorn Helgaas @ 2022-08-25 18:01 UTC (permalink / raw)
To: Lijo Lazar
Cc: Greg Kroah-Hartman, Felix.Kuehling, amd-gfx, tseewald,
Alexander.Deucher, sr, Christian.Koenig, Hawking.Zhang
[+cc Greg, no action needed yet, just FYI that stable will want these]
On Thu, Aug 25, 2022 at 02:28:19PM +0530, Lijo Lazar wrote:
> HDP flush is used early in the init sequence as part of memory controller
> block initialization. Hence remapping of HDP registers needed for flush
> needs to happen earlier.
>
> This also fixes the AER error reported as Unsupported Request during
> driver load.
I would say something like:
This prevents writes to unimplemented space, which would cause
Unsupported Request errors. Prior to 8795e182b02d ("PCI/portdrv:
Don't disable AER reporting in get_port_device_capability()"), these
errors occurred but were ignored.
The write is the error; AER is just the reporting mechanism.
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216373
We need a cc: stable because 8795e182b02d ("PCI/portdrv: Don't disable
AER reporting in get_port_device_capability()") has already been
backported to at lealst these stable kernels:
5.10.137 5.15.61 5.18.18 5.19.2
and these fixes need to go there as well. So add something like this:
Fixes: 8795e182b02d ("PCI/portdrv: Don't disable AER reporting in get_port_device_capability()")
cc: stable@vger.kernel.org
It's not that there was something wrong with 8795e182b02d and these
patches fix it; it's just that 8795e182b02d *exposed* an amdgpu
problem that was there all along. But we need some way to connect
with it.
> Reported-by: Tom Seewald <tseewald@gmail.com>
> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init
2022-08-25 18:01 ` Bjorn Helgaas
@ 2022-09-08 14:08 ` Deucher, Alexander
2022-09-08 14:13 ` Alex Deucher
0 siblings, 1 reply; 12+ messages in thread
From: Deucher, Alexander @ 2022-09-08 14:08 UTC (permalink / raw)
To: Bjorn Helgaas, Lazar, Lijo, tseewald
Cc: Greg Kroah-Hartman, Kuehling, Felix, amd-gfx, sr, Koenig,
Christian, Zhang, Hawking
[Public]
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Bjorn Helgaas
> Sent: Thursday, August 25, 2022 2:02 PM
> To: Lazar, Lijo <Lijo.Lazar@amd.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Kuehling, Felix
> <Felix.Kuehling@amd.com>; amd-gfx@lists.freedesktop.org;
> tseewald@gmail.com; Deucher, Alexander
> <Alexander.Deucher@amd.com>; sr@denx.de; Koenig, Christian
> <Christian.Koenig@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during
> init
>
> [+cc Greg, no action needed yet, just FYI that stable will want these]
>
> On Thu, Aug 25, 2022 at 02:28:19PM +0530, Lijo Lazar wrote:
> > HDP flush is used early in the init sequence as part of memory
> > controller block initialization. Hence remapping of HDP registers
> > needed for flush needs to happen earlier.
> >
> > This also fixes the AER error reported as Unsupported Request during
> > driver load.
>
> I would say something like:
>
> This prevents writes to unimplemented space, which would cause
> Unsupported Request errors. Prior to 8795e182b02d ("PCI/portdrv:
> Don't disable AER reporting in get_port_device_capability()"), these
> errors occurred but were ignored.
>
> The write is the error; AER is just the reporting mechanism.
>
> > Link:
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> >
> illa.kernel.org%2Fshow_bug.cgi%3Fid%3D216373&data=05%7C01%7Cal
> exan
> >
> der.deucher%40amd.com%7C3306aa3e6a834f2d394808da86c3dfb1%7C3dd8
> 961fe48
> >
> 84e608e11a82d994e183d%7C0%7C0%7C637970473081942953%7CUnknown%
> 7CTWFpbGZ
> >
> sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6M
> n0%3
> >
> D%7C3000%7C%7C%7C&sdata=DFqggIRt4sKNjJB5bY14yi5oJ8I4szzndQD
> blr6Vcr
> > c%3D&reserved=0
>
> We need a cc: stable because 8795e182b02d ("PCI/portdrv: Don't disable AER
> reporting in get_port_device_capability()") has already been backported to
> at lealst these stable kernels:
>
> 5.10.137 5.15.61 5.18.18 5.19.2
>
> and these fixes need to go there as well. So add something like this:
>
> Fixes: 8795e182b02d ("PCI/portdrv: Don't disable AER reporting in
> get_port_device_capability()")
> cc: stable@vger.kernel.org
>
> It's not that there was something wrong with 8795e182b02d and these
> patches fix it; it's just that 8795e182b02d *exposed* an amdgpu problem
> that was there all along. But we need some way to connect with it.
Will update the patch with these comments. Also @tseewald@gmail.com can you please test this patch and confirm that it fixes things for you as well?
Alex
>
> > Reported-by: Tom Seewald <tseewald@gmail.com>
> > Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init
2022-09-08 14:08 ` Deucher, Alexander
@ 2022-09-08 14:13 ` Alex Deucher
0 siblings, 0 replies; 12+ messages in thread
From: Alex Deucher @ 2022-09-08 14:13 UTC (permalink / raw)
To: Deucher, Alexander
Cc: Greg Kroah-Hartman, Kuehling, Felix, Lazar, Lijo, amd-gfx,
tseewald, Bjorn Helgaas, sr, Koenig, Christian, Zhang, Hawking
On Thu, Sep 8, 2022 at 10:08 AM Deucher, Alexander
<Alexander.Deucher@amd.com> wrote:
>
> [Public]
>
> > -----Original Message-----
> > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> > Bjorn Helgaas
> > Sent: Thursday, August 25, 2022 2:02 PM
> > To: Lazar, Lijo <Lijo.Lazar@amd.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Kuehling, Felix
> > <Felix.Kuehling@amd.com>; amd-gfx@lists.freedesktop.org;
> > tseewald@gmail.com; Deucher, Alexander
> > <Alexander.Deucher@amd.com>; sr@denx.de; Koenig, Christian
> > <Christian.Koenig@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>
> > Subject: Re: [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during
> > init
> >
> > [+cc Greg, no action needed yet, just FYI that stable will want these]
> >
> > On Thu, Aug 25, 2022 at 02:28:19PM +0530, Lijo Lazar wrote:
> > > HDP flush is used early in the init sequence as part of memory
> > > controller block initialization. Hence remapping of HDP registers
> > > needed for flush needs to happen earlier.
> > >
> > > This also fixes the AER error reported as Unsupported Request during
> > > driver load.
> >
> > I would say something like:
> >
> > This prevents writes to unimplemented space, which would cause
> > Unsupported Request errors. Prior to 8795e182b02d ("PCI/portdrv:
> > Don't disable AER reporting in get_port_device_capability()"), these
> > errors occurred but were ignored.
> >
> > The write is the error; AER is just the reporting mechanism.
> >
> > > Link:
> > >
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> > >
> > illa.kernel.org%2Fshow_bug.cgi%3Fid%3D216373&data=05%7C01%7Cal
> > exan
> > >
> > der.deucher%40amd.com%7C3306aa3e6a834f2d394808da86c3dfb1%7C3dd8
> > 961fe48
> > >
> > 84e608e11a82d994e183d%7C0%7C0%7C637970473081942953%7CUnknown%
> > 7CTWFpbGZ
> > >
> > sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6M
> > n0%3
> > >
> > D%7C3000%7C%7C%7C&sdata=DFqggIRt4sKNjJB5bY14yi5oJ8I4szzndQD
> > blr6Vcr
> > > c%3D&reserved=0
> >
> > We need a cc: stable because 8795e182b02d ("PCI/portdrv: Don't disable AER
> > reporting in get_port_device_capability()") has already been backported to
> > at lealst these stable kernels:
> >
> > 5.10.137 5.15.61 5.18.18 5.19.2
> >
> > and these fixes need to go there as well. So add something like this:
> >
> > Fixes: 8795e182b02d ("PCI/portdrv: Don't disable AER reporting in
> > get_port_device_capability()")
> > cc: stable@vger.kernel.org
> >
> > It's not that there was something wrong with 8795e182b02d and these
> > patches fix it; it's just that 8795e182b02d *exposed* an amdgpu problem
> > that was there all along. But we need some way to connect with it.
>
> Will update the patch with these comments. Also @tseewald@gmail.com can you please test this patch and confirm that it fixes things for you as well?
Sorry, ignore this. This old email showed up at the top of my inbox
for some reason and I mixed up the threads.
Alex
>
> Alex
>
> >
> > > Reported-by: Tom Seewald <tseewald@gmail.com>
> > > Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-09-08 14:14 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-25 8:58 [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init Lijo Lazar
2022-08-25 8:58 ` [PATCH 2/2] drm/amdgpu: Init VF's HDP flush reg offset early Lijo Lazar
2022-08-25 13:22 ` Alex Deucher
2022-08-25 14:07 ` Alex Deucher
2022-08-25 14:07 ` [PATCH 1/2] drm/amdgpu: Move HDP remapping earlier during init Alex Deucher
2022-08-25 14:22 ` Lazar, Lijo
2022-08-25 14:26 ` Alex Deucher
2022-08-25 14:53 ` Christian König
2022-08-25 15:14 ` Felix Kuehling
2022-08-25 18:01 ` Bjorn Helgaas
2022-09-08 14:08 ` Deucher, Alexander
2022-09-08 14:13 ` Alex Deucher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.