This patch is reviewed-by: Evan Quan <evan.quan@xxxxxxx> However, i just find we need a separate patch to clear PP_GFXOFF_MASK support from Arcturus. Can you do that or you want me to do that? > -----Original Message----- > From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Alex > Deucher > Sent: Tuesday, November 12, 2019 11:13 PM > To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx> > Subject: [PATCH] drm/amdgpu: don't read registers if gfxoff is enabled (v2) > > When gfxoff is enabled, accessing gfx registers via MMIO > can lead to a hang. > > v2: return cached registers properly. > > Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497 > Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx> > --- > drivers/gpu/drm/amd/amdgpu/nv.c | 27 ++++++++++++++++---------- > drivers/gpu/drm/amd/amdgpu/soc15.c | 31 ++++++++++++++++++------------ > 2 files changed, 36 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c > b/drivers/gpu/drm/amd/amdgpu/nv.c > index af68f9815f28..7283d6198b89 100644 > --- a/drivers/gpu/drm/amd/amdgpu/nv.c > +++ b/drivers/gpu/drm/amd/amdgpu/nv.c > @@ -201,17 +201,25 @@ static uint32_t nv_read_indexed_register(struct > amdgpu_device *adev, u32 se_num, > return val; > } > > -static uint32_t nv_get_register_value(struct amdgpu_device *adev, > +static int nv_get_register_value(struct amdgpu_device *adev, > bool indexed, u32 se_num, > - u32 sh_num, u32 reg_offset) > + u32 sh_num, u32 reg_offset, > + u32 *value) > { > if (indexed) { > - return nv_read_indexed_register(adev, se_num, sh_num, > reg_offset); > + if (adev->pm.pp_feature & PP_GFXOFF_MASK) > + return -EINVAL; > + *value = nv_read_indexed_register(adev, se_num, sh_num, > reg_offset); > } else { > - if (reg_offset == SOC15_REG_OFFSET(GC, 0, > mmGB_ADDR_CONFIG)) > - return adev->gfx.config.gb_addr_config; > - return RREG32(reg_offset); > + if (reg_offset == SOC15_REG_OFFSET(GC, 0, > mmGB_ADDR_CONFIG)) { > + *value = adev->gfx.config.gb_addr_config; > + } else { > + if (adev->pm.pp_feature & PP_GFXOFF_MASK) > + return -EINVAL; > + *value = RREG32(reg_offset); > + } > } > + return 0; > } > > static int nv_read_register(struct amdgpu_device *adev, u32 se_num, > @@ -227,10 +235,9 @@ static int nv_read_register(struct amdgpu_device > *adev, u32 se_num, > (adev->reg_offset[en->hwip][en->inst][en->seg] + en- > >reg_offset)) > continue; > > - *value = nv_get_register_value(adev, > - > nv_allowed_read_registers[i].grbm_indexed, > - se_num, sh_num, reg_offset); > - return 0; > + return nv_get_register_value(adev, > + > nv_allowed_read_registers[i].grbm_indexed, > + se_num, sh_num, reg_offset, value); > } > return -EINVAL; > } > diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c > b/drivers/gpu/drm/amd/amdgpu/soc15.c > index 305ad3eec987..2cc16e9f39fb 100644 > --- a/drivers/gpu/drm/amd/amdgpu/soc15.c > +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c > @@ -363,19 +363,27 @@ static uint32_t soc15_read_indexed_register(struct > amdgpu_device *adev, u32 se_n > return val; > } > > -static uint32_t soc15_get_register_value(struct amdgpu_device *adev, > +static int soc15_get_register_value(struct amdgpu_device *adev, > bool indexed, u32 se_num, > - u32 sh_num, u32 reg_offset) > + u32 sh_num, u32 reg_offset, > + u32 *value) > { > if (indexed) { > - return soc15_read_indexed_register(adev, se_num, sh_num, > reg_offset); > + if (adev->pm.pp_feature & PP_GFXOFF_MASK) > + return -EINVAL; > + *value = soc15_read_indexed_register(adev, se_num, sh_num, > reg_offset); > } else { > - if (reg_offset == SOC15_REG_OFFSET(GC, 0, > mmGB_ADDR_CONFIG)) > - return adev->gfx.config.gb_addr_config; > - else if (reg_offset == SOC15_REG_OFFSET(GC, 0, > mmDB_DEBUG2)) > - return adev->gfx.config.db_debug2; > - return RREG32(reg_offset); > + if (reg_offset == SOC15_REG_OFFSET(GC, 0, > mmGB_ADDR_CONFIG)) { > + *value = adev->gfx.config.gb_addr_config; > + } else if (reg_offset == SOC15_REG_OFFSET(GC, 0, > mmDB_DEBUG2)) { > + *value = adev->gfx.config.db_debug2; > + } else { > + if (adev->pm.pp_feature & PP_GFXOFF_MASK) > + return -EINVAL; > + *value = RREG32(reg_offset); > + } > } > + return 0; > } > > static int soc15_read_register(struct amdgpu_device *adev, u32 se_num, > @@ -391,10 +399,9 @@ static int soc15_read_register(struct amdgpu_device > *adev, u32 se_num, > + en->reg_offset)) > continue; > > - *value = soc15_get_register_value(adev, > - > soc15_allowed_read_registers[i].grbm_indexed, > - se_num, sh_num, reg_offset); > - return 0; > + return soc15_get_register_value(adev, > + > soc15_allowed_read_registers[i].grbm_indexed, > + se_num, sh_num, reg_offset, > value); > } > return -EINVAL; > } > -- > 2.23.0 > > _______________________________________________ > amd-gfx mailing list > amd-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
