Re: [PATCH] drm/amdgpu: don't read registers if gfxoff is enabled (v2)
I can send a patch for that. Thanks! Alex From: Quan, Evan Sent: Tuesday, November 12, 2019 9:09 PM To: Alex Deucher ; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander Subject: RE: [PATCH] drm/amdgpu: don't read registers if gfxoff is enabled (v2) This patch is reviewed-by: Evan Quan 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 On Behalf Of Alex > Deucher > Sent: Tuesday, November 12, 2019 11:13 PM > To: amd-gfx@lists.freedesktop.org > Cc: Deucher, Alexander > 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 > --- > 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
RE: [PATCH] drm/amdgpu: don't read registers if gfxoff is enabled (v2)
This patch is reviewed-by: Evan Quan 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 On Behalf Of Alex > Deucher > Sent: Tuesday, November 12, 2019 11:13 PM > To: amd-gfx@lists.freedesktop.org > Cc: Deucher, Alexander > 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 > --- > 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_
[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 --- 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, -