[PATCH 1/2] Revert "drm/amdgpu: don't read registers if gfxoff is enabled (v2)"

[PATCH 1/2] Revert "drm/amdgpu: don't read registers if gfxoff is enabled (v2)"

[Alex Deucher]

This reverts commit 5e49d6f654c569c2de920babbaf5cf7c4c4a353f.

Drop this workaround in favor of a better one.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/nv.c    | 27 ++++++++++----------------
 drivers/gpu/drm/amd/amdgpu/soc15.c | 31 ++++++++++++------------------
 2 files changed, 22 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 7283d6198b89..af68f9815f28 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -201,25 +201,17 @@ static uint32_t nv_read_indexed_register(struct amdgpu_device *adev, u32 se_num,
 	return val;
 }
 
-static int nv_get_register_value(struct amdgpu_device *adev,
+static uint32_t nv_get_register_value(struct amdgpu_device *adev,
 				      bool indexed, u32 se_num,
-				      u32 sh_num, u32 reg_offset,
-				      u32 *value)
+				      u32 sh_num, u32 reg_offset)
 {
 	if (indexed) {
-		if (adev->pm.pp_feature & PP_GFXOFF_MASK)
-			return -EINVAL;
-		*value = nv_read_indexed_register(adev, se_num, sh_num, reg_offset);
+		return nv_read_indexed_register(adev, se_num, sh_num, reg_offset);
 	} else {
-		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);
-		}
+		if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG))
+			return adev->gfx.config.gb_addr_config;
+		return RREG32(reg_offset);
 	}
-	return 0;
 }
 
 static int nv_read_register(struct amdgpu_device *adev, u32 se_num,
@@ -235,9 +227,10 @@ 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;
 
-		return nv_get_register_value(adev,
-					     nv_allowed_read_registers[i].grbm_indexed,
-					     se_num, sh_num, reg_offset, value);
+		*value = nv_get_register_value(adev,
+					       nv_allowed_read_registers[i].grbm_indexed,
+					       se_num, sh_num, reg_offset);
+		return 0;
 	}
 	return -EINVAL;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
index 3b55655f79c4..8e1640bc07af 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -363,27 +363,19 @@ static uint32_t soc15_read_indexed_register(struct amdgpu_device *adev, u32 se_n
 	return val;
 }
 
-static int soc15_get_register_value(struct amdgpu_device *adev,
+static uint32_t soc15_get_register_value(struct amdgpu_device *adev,
 					 bool indexed, u32 se_num,
-					 u32 sh_num, u32 reg_offset,
-					 u32 *value)
+					 u32 sh_num, u32 reg_offset)
 {
 	if (indexed) {
-		if (adev->pm.pp_feature & PP_GFXOFF_MASK)
-			return -EINVAL;
-	        *value = soc15_read_indexed_register(adev, se_num, sh_num, reg_offset);
+		return soc15_read_indexed_register(adev, se_num, sh_num, reg_offset);
 	} else {
-		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);
-		}
+		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);
 	}
-	return 0;
 }
 
 static int soc15_read_register(struct amdgpu_device *adev, u32 se_num,
@@ -399,9 +391,10 @@ static int soc15_read_register(struct amdgpu_device *adev, u32 se_num,
 					+ en->reg_offset))
 			continue;
 
-		return soc15_get_register_value(adev,
-						soc15_allowed_read_registers[i].grbm_indexed,
-						se_num, sh_num, reg_offset, value);
+		*value = soc15_get_register_value(adev,
+						  soc15_allowed_read_registers[i].grbm_indexed,
+						  se_num, sh_num, reg_offset);
+		return 0;
 	}
 	return -EINVAL;
 }
-- 
2.23.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/2] Revert "drm/amdgpu: don't read registers if gfxoff is enabled (v2)"

[Alex Deucher]

This reverts commit 5e49d6f654c569c2de920babbaf5cf7c4c4a353f.

Drop this workaround in favor of a better one.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/nv.c    | 27 ++++++++++----------------
 drivers/gpu/drm/amd/amdgpu/soc15.c | 31 ++++++++++++------------------
 2 files changed, 22 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 7283d6198b89..af68f9815f28 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -201,25 +201,17 @@ static uint32_t nv_read_indexed_register(struct amdgpu_device *adev, u32 se_num,
 	return val;
 }
 
-static int nv_get_register_value(struct amdgpu_device *adev,
+static uint32_t nv_get_register_value(struct amdgpu_device *adev,
 				      bool indexed, u32 se_num,
-				      u32 sh_num, u32 reg_offset,
-				      u32 *value)
+				      u32 sh_num, u32 reg_offset)
 {
 	if (indexed) {
-		if (adev->pm.pp_feature & PP_GFXOFF_MASK)
-			return -EINVAL;
-		*value = nv_read_indexed_register(adev, se_num, sh_num, reg_offset);
+		return nv_read_indexed_register(adev, se_num, sh_num, reg_offset);
 	} else {
-		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);
-		}
+		if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG))
+			return adev->gfx.config.gb_addr_config;
+		return RREG32(reg_offset);
 	}
-	return 0;
 }
 
 static int nv_read_register(struct amdgpu_device *adev, u32 se_num,
@@ -235,9 +227,10 @@ 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;
 
-		return nv_get_register_value(adev,
-					     nv_allowed_read_registers[i].grbm_indexed,
-					     se_num, sh_num, reg_offset, value);
+		*value = nv_get_register_value(adev,
+					       nv_allowed_read_registers[i].grbm_indexed,
+					       se_num, sh_num, reg_offset);
+		return 0;
 	}
 	return -EINVAL;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
index 3b55655f79c4..8e1640bc07af 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -363,27 +363,19 @@ static uint32_t soc15_read_indexed_register(struct amdgpu_device *adev, u32 se_n
 	return val;
 }
 
-static int soc15_get_register_value(struct amdgpu_device *adev,
+static uint32_t soc15_get_register_value(struct amdgpu_device *adev,
 					 bool indexed, u32 se_num,
-					 u32 sh_num, u32 reg_offset,
-					 u32 *value)
+					 u32 sh_num, u32 reg_offset)
 {
 	if (indexed) {
-		if (adev->pm.pp_feature & PP_GFXOFF_MASK)
-			return -EINVAL;
-	        *value = soc15_read_indexed_register(adev, se_num, sh_num, reg_offset);
+		return soc15_read_indexed_register(adev, se_num, sh_num, reg_offset);
 	} else {
-		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);
-		}
+		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);
 	}
-	return 0;
 }
 
 static int soc15_read_register(struct amdgpu_device *adev, u32 se_num,
@@ -399,9 +391,10 @@ static int soc15_read_register(struct amdgpu_device *adev, u32 se_num,
 					+ en->reg_offset))
 			continue;
 
-		return soc15_get_register_value(adev,
-						soc15_allowed_read_registers[i].grbm_indexed,
-						se_num, sh_num, reg_offset, value);
+		*value = soc15_get_register_value(adev,
+						  soc15_allowed_read_registers[i].grbm_indexed,
+						  se_num, sh_num, reg_offset);
+		return 0;
 	}
 	return -EINVAL;
 }
-- 
2.23.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Alex Deucher]

When gfxoff is enabled, accessing gfx registers via MMIO
can lead to a hang.

Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 6ddea7607ad0..5f3b3a705b29 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
 			return -ENOMEM;
 		alloc_size = info->read_mmr_reg.count * sizeof(*regs);
 
-		for (i = 0; i < info->read_mmr_reg.count; i++)
+		amdgpu_gfx_off_ctrl(adev, false);
+		for (i = 0; i < info->read_mmr_reg.count; i++) {
 			if (amdgpu_asic_read_register(adev, se_num, sh_num,
 						      info->read_mmr_reg.dword_offset + i,
 						      &regs[i])) {
 				DRM_DEBUG_KMS("unallowed offset %#x\n",
 					      info->read_mmr_reg.dword_offset + i);
 				kfree(regs);
+				amdgpu_gfx_off_ctrl(adev, true);
 				return -EFAULT;
 			}
+		}
+		amdgpu_gfx_off_ctrl(adev, true);
 		n = copy_to_user(out, regs, min(size, alloc_size));
 		kfree(regs);
 		return n ? -EFAULT : 0;
-- 
2.23.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Alex Deucher]

When gfxoff is enabled, accessing gfx registers via MMIO
can lead to a hang.

Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 6ddea7607ad0..5f3b3a705b29 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
 			return -ENOMEM;
 		alloc_size = info->read_mmr_reg.count * sizeof(*regs);
 
-		for (i = 0; i < info->read_mmr_reg.count; i++)
+		amdgpu_gfx_off_ctrl(adev, false);
+		for (i = 0; i < info->read_mmr_reg.count; i++) {
 			if (amdgpu_asic_read_register(adev, se_num, sh_num,
 						      info->read_mmr_reg.dword_offset + i,
 						      &regs[i])) {
 				DRM_DEBUG_KMS("unallowed offset %#x\n",
 					      info->read_mmr_reg.dword_offset + i);
 				kfree(regs);
+				amdgpu_gfx_off_ctrl(adev, true);
 				return -EFAULT;
 			}
+		}
+		amdgpu_gfx_off_ctrl(adev, true);
 		n = copy_to_user(out, regs, min(size, alloc_size));
 		kfree(regs);
 		return n ? -EFAULT : 0;
-- 
2.23.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Alex Deucher]

Ping?

On Thu, Nov 14, 2019 at 11:42 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> When gfxoff is enabled, accessing gfx registers via MMIO
> can lead to a hang.
>
> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 6ddea7607ad0..5f3b3a705b29 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>                         return -ENOMEM;
>                 alloc_size = info->read_mmr_reg.count * sizeof(*regs);
>
> -               for (i = 0; i < info->read_mmr_reg.count; i++)
> +               amdgpu_gfx_off_ctrl(adev, false);
> +               for (i = 0; i < info->read_mmr_reg.count; i++) {
>                         if (amdgpu_asic_read_register(adev, se_num, sh_num,
>                                                       info->read_mmr_reg.dword_offset + i,
>                                                       &regs[i])) {
>                                 DRM_DEBUG_KMS("unallowed offset %#x\n",
>                                               info->read_mmr_reg.dword_offset + i);
>                                 kfree(regs);
> +                               amdgpu_gfx_off_ctrl(adev, true);
>                                 return -EFAULT;
>                         }
> +               }
> +               amdgpu_gfx_off_ctrl(adev, true);
>                 n = copy_to_user(out, regs, min(size, alloc_size));
>                 kfree(regs);
>                 return n ? -EFAULT : 0;
> --
> 2.23.0
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Alex Deucher]

Ping?

On Thu, Nov 14, 2019 at 11:42 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> When gfxoff is enabled, accessing gfx registers via MMIO
> can lead to a hang.
>
> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 6ddea7607ad0..5f3b3a705b29 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>                         return -ENOMEM;
>                 alloc_size = info->read_mmr_reg.count * sizeof(*regs);
>
> -               for (i = 0; i < info->read_mmr_reg.count; i++)
> +               amdgpu_gfx_off_ctrl(adev, false);
> +               for (i = 0; i < info->read_mmr_reg.count; i++) {
>                         if (amdgpu_asic_read_register(adev, se_num, sh_num,
>                                                       info->read_mmr_reg.dword_offset + i,
>                                                       &regs[i])) {
>                                 DRM_DEBUG_KMS("unallowed offset %#x\n",
>                                               info->read_mmr_reg.dword_offset + i);
>                                 kfree(regs);
> +                               amdgpu_gfx_off_ctrl(adev, true);
>                                 return -EFAULT;
>                         }
> +               }
> +               amdgpu_gfx_off_ctrl(adev, true);
>                 n = copy_to_user(out, regs, min(size, alloc_size));
>                 kfree(regs);
>                 return n ? -EFAULT : 0;
> --
> 2.23.0
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Yuan, Xiaojie]

Hi Alex,

IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)

If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.

BR,
Xiaojie

> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> 
> When gfxoff is enabled, accessing gfx registers via MMIO
> can lead to a hang.
> 
> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 6ddea7607ad0..5f3b3a705b29 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>            return -ENOMEM;
>        alloc_size = info->read_mmr_reg.count * sizeof(*regs);
> 
> -        for (i = 0; i < info->read_mmr_reg.count; i++)
> +        amdgpu_gfx_off_ctrl(adev, false);
> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
>            if (amdgpu_asic_read_register(adev, se_num, sh_num,
>                              info->read_mmr_reg.dword_offset + i,
>                              &regs[i])) {
>                DRM_DEBUG_KMS("unallowed offset %#x\n",
>                          info->read_mmr_reg.dword_offset + i);
>                kfree(regs);
> +                amdgpu_gfx_off_ctrl(adev, true);
>                return -EFAULT;
>            }
> +        }
> +        amdgpu_gfx_off_ctrl(adev, true);
>        n = copy_to_user(out, regs, min(size, alloc_size));
>        kfree(regs);
>        return n ? -EFAULT : 0;
> -- 
> 2.23.0
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Yuan, Xiaojie]

Hi Alex,

IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)

If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.

BR,
Xiaojie

> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> 
> When gfxoff is enabled, accessing gfx registers via MMIO
> can lead to a hang.
> 
> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 6ddea7607ad0..5f3b3a705b29 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>            return -ENOMEM;
>        alloc_size = info->read_mmr_reg.count * sizeof(*regs);
> 
> -        for (i = 0; i < info->read_mmr_reg.count; i++)
> +        amdgpu_gfx_off_ctrl(adev, false);
> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
>            if (amdgpu_asic_read_register(adev, se_num, sh_num,
>                              info->read_mmr_reg.dword_offset + i,
>                              &regs[i])) {
>                DRM_DEBUG_KMS("unallowed offset %#x\n",
>                          info->read_mmr_reg.dword_offset + i);
>                kfree(regs);
> +                amdgpu_gfx_off_ctrl(adev, true);
>                return -EFAULT;
>            }
> +        }
> +        amdgpu_gfx_off_ctrl(adev, true);
>        n = copy_to_user(out, regs, min(size, alloc_size));
>        kfree(regs);
>        return n ? -EFAULT : 0;
> -- 
> 2.23.0
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Alex Deucher]

On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>
> Hi Alex,
>
> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
>
> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.

That would be great.  Maybe we can add a delay in that function to
take that into account?

Thanks!

Alex

>
> BR,
> Xiaojie
>
> > On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> > When gfxoff is enabled, accessing gfx registers via MMIO
> > can lead to a hang.
> >
> > Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > index 6ddea7607ad0..5f3b3a705b29 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
> >            return -ENOMEM;
> >        alloc_size = info->read_mmr_reg.count * sizeof(*regs);
> >
> > -        for (i = 0; i < info->read_mmr_reg.count; i++)
> > +        amdgpu_gfx_off_ctrl(adev, false);
> > +        for (i = 0; i < info->read_mmr_reg.count; i++) {
> >            if (amdgpu_asic_read_register(adev, se_num, sh_num,
> >                              info->read_mmr_reg.dword_offset + i,
> >                              &regs[i])) {
> >                DRM_DEBUG_KMS("unallowed offset %#x\n",
> >                          info->read_mmr_reg.dword_offset + i);
> >                kfree(regs);
> > +                amdgpu_gfx_off_ctrl(adev, true);
> >                return -EFAULT;
> >            }
> > +        }
> > +        amdgpu_gfx_off_ctrl(adev, true);
> >        n = copy_to_user(out, regs, min(size, alloc_size));
> >        kfree(regs);
> >        return n ? -EFAULT : 0;
> > --
> > 2.23.0
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Alex Deucher]

On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>
> Hi Alex,
>
> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
>
> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.

That would be great.  Maybe we can add a delay in that function to
take that into account?

Thanks!

Alex

>
> BR,
> Xiaojie
>
> > On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> > When gfxoff is enabled, accessing gfx registers via MMIO
> > can lead to a hang.
> >
> > Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > index 6ddea7607ad0..5f3b3a705b29 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
> >            return -ENOMEM;
> >        alloc_size = info->read_mmr_reg.count * sizeof(*regs);
> >
> > -        for (i = 0; i < info->read_mmr_reg.count; i++)
> > +        amdgpu_gfx_off_ctrl(adev, false);
> > +        for (i = 0; i < info->read_mmr_reg.count; i++) {
> >            if (amdgpu_asic_read_register(adev, se_num, sh_num,
> >                              info->read_mmr_reg.dword_offset + i,
> >                              &regs[i])) {
> >                DRM_DEBUG_KMS("unallowed offset %#x\n",
> >                          info->read_mmr_reg.dword_offset + i);
> >                kfree(regs);
> > +                amdgpu_gfx_off_ctrl(adev, true);
> >                return -EFAULT;
> >            }
> > +        }
> > +        amdgpu_gfx_off_ctrl(adev, true);
> >        n = copy_to_user(out, regs, min(size, alloc_size));
> >        kfree(regs);
> >        return n ? -EFAULT : 0;
> > --
> > 2.23.0
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Yuan, Xiaojie]

Yes. IIRC, some asics' amdgpu_gfx_ctrl() is implemented as synchronous (upon function returns, gfx block is guaranteed to be in power-up state). Anyway, let me confirm about that soon.

BR,
Xiaojie

> On Nov 16, 2019, at 12:52 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> 
>> On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>> 
>> Hi Alex,
>> 
>> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
>> 
>> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.
> 
> That would be great.  Maybe we can add a delay in that function to
> take that into account?
> 
> Thanks!
> 
> Alex
> 
>> 
>> BR,
>> Xiaojie
>> 
>>> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>> 
>>> When gfxoff is enabled, accessing gfx registers via MMIO
>>> can lead to a hang.
>>> 
>>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> index 6ddea7607ad0..5f3b3a705b29 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>>>           return -ENOMEM;
>>>       alloc_size = info->read_mmr_reg.count * sizeof(*regs);
>>> 
>>> -        for (i = 0; i < info->read_mmr_reg.count; i++)
>>> +        amdgpu_gfx_off_ctrl(adev, false);
>>> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
>>>           if (amdgpu_asic_read_register(adev, se_num, sh_num,
>>>                             info->read_mmr_reg.dword_offset + i,
>>>                             &regs[i])) {
>>>               DRM_DEBUG_KMS("unallowed offset %#x\n",
>>>                         info->read_mmr_reg.dword_offset + i);
>>>               kfree(regs);
>>> +                amdgpu_gfx_off_ctrl(adev, true);
>>>               return -EFAULT;
>>>           }
>>> +        }
>>> +        amdgpu_gfx_off_ctrl(adev, true);
>>>       n = copy_to_user(out, regs, min(size, alloc_size));
>>>       kfree(regs);
>>>       return n ? -EFAULT : 0;
>>> --
>>> 2.23.0
>>> 
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Yuan, Xiaojie]

Yes. IIRC, some asics' amdgpu_gfx_ctrl() is implemented as synchronous (upon function returns, gfx block is guaranteed to be in power-up state). Anyway, let me confirm about that soon.

BR,
Xiaojie

> On Nov 16, 2019, at 12:52 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> 
>> On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>> 
>> Hi Alex,
>> 
>> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
>> 
>> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.
> 
> That would be great.  Maybe we can add a delay in that function to
> take that into account?
> 
> Thanks!
> 
> Alex
> 
>> 
>> BR,
>> Xiaojie
>> 
>>> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>> 
>>> When gfxoff is enabled, accessing gfx registers via MMIO
>>> can lead to a hang.
>>> 
>>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> index 6ddea7607ad0..5f3b3a705b29 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>>>           return -ENOMEM;
>>>       alloc_size = info->read_mmr_reg.count * sizeof(*regs);
>>> 
>>> -        for (i = 0; i < info->read_mmr_reg.count; i++)
>>> +        amdgpu_gfx_off_ctrl(adev, false);
>>> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
>>>           if (amdgpu_asic_read_register(adev, se_num, sh_num,
>>>                             info->read_mmr_reg.dword_offset + i,
>>>                             &regs[i])) {
>>>               DRM_DEBUG_KMS("unallowed offset %#x\n",
>>>                         info->read_mmr_reg.dword_offset + i);
>>>               kfree(regs);
>>> +                amdgpu_gfx_off_ctrl(adev, true);
>>>               return -EFAULT;
>>>           }
>>> +        }
>>> +        amdgpu_gfx_off_ctrl(adev, true);
>>>       n = copy_to_user(out, regs, min(size, alloc_size));
>>>       kfree(regs);
>>>       return n ? -EFAULT : 0;
>>> --
>>> 2.23.0
>>> 
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Yuan, Xiaojie]

Hi Alex,

I tried on Navi14 with Gfxoff enabled (gfx in 'OFF' state when I run the test program) and used a test program to read GRBM_STATUS/CP_STAT/GB_ADDR_CONFIG via DRM_IOCTL_AMDGPU_INFO.

All read out values are valid as below (in this read cycle, I saw gfx block is first awakened and then powered off again automatically):

drm version: 3.36.0
grbm_status: 0x00003028
cp_stat: 0x00000000
gb_addr_config: 0x00000043

At least for Navi, reading register when gfxoff seems not a problem...

Test program:

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <unistd.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/ioctl.h>
#include <fcntl.h>
#include <errno.h>
#include <libdrm/drm.h>
#include <libdrm/amdgpu_drm.h>

int get_reg_val(int fd, int reg_offset, int *val)
{
        struct drm_amdgpu_info request;
        int r;

        memset(&request, 0, sizeof(request));
        request.return_pointer = (uintptr_t)val;
        request.return_size = sizeof(uint32_t);
        request.query = AMDGPU_INFO_READ_MMR_REG;
        request.read_mmr_reg.dword_offset = reg_offset;
        request.read_mmr_reg.count = 1;
        request.read_mmr_reg.instance = 0xffffffff;
        request.read_mmr_reg.flags = 0;

        r = ioctl(fd, DRM_IOCTL_AMDGPU_INFO, &request);
        if (r < 0) {
                perror("failed to read register");
                return -errno;
        }

        return 0;
}

int main(int argc, const char *argv[])
{
        const char *device = "/dev/dri/card0";
        int fd;
        int r;
        struct drm_version version;
        struct drm_auth auth;
        int cp_stat;
        int grbm_status;
        int gb_addr_config;

        fd = open(device, O_RDWR | O_CLOEXEC);
        if (fd < 0) {
                perror("failed to open device\n");
                return -errno;
        }

        memset(&version, 0, sizeof(version));

        r = ioctl(fd, DRM_IOCTL_VERSION, &version);
        if (r < 0) {
                perror("failed to get drm version");
                r = -errno;
                goto close;
        }

        printf("drm version: %d.%d.%d\n", version.version_major,
                                          version.version_minor,
                                          version.version_patchlevel);

        r = get_reg_val(fd, 0x2004, &grbm_status);
        if (r < 0)
                goto close;
        printf("grbm_status: 0x%08x\n", grbm_status);

        r = get_reg_val(fd, 0x21a0, &cp_stat);
        if (r < 0)
                goto close;
        printf("cp_stat: 0x%08x\n", cp_stat);

        r = get_reg_val(fd, 0x263e, &gb_addr_config);
        if (r < 0)
                goto close;
        printf("gb_addr_config: 0x%08x\n", gb_addr_config);

close:
        close(fd);

        return r;
}

BR,
Xiaojie

________________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Yuan, Xiaojie <Xiaojie.Yuan@amd.com>
Sent: Saturday, November 16, 2019 1:02 AM
To: Alex Deucher
Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

Yes. IIRC, some asics' amdgpu_gfx_ctrl() is implemented as synchronous (upon function returns, gfx block is guaranteed to be in power-up state). Anyway, let me confirm about that soon.

BR,
Xiaojie

> On Nov 16, 2019, at 12:52 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>
>> On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>>
>> Hi Alex,
>>
>> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
>>
>> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.
>
> That would be great.  Maybe we can add a delay in that function to
> take that into account?
>
> Thanks!
>
> Alex
>
>>
>> BR,
>> Xiaojie
>>
>>> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>
>>> When gfxoff is enabled, accessing gfx registers via MMIO
>>> can lead to a hang.
>>>
>>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> index 6ddea7607ad0..5f3b3a705b29 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>>>           return -ENOMEM;
>>>       alloc_size = info->read_mmr_reg.count * sizeof(*regs);
>>>
>>> -        for (i = 0; i < info->read_mmr_reg.count; i++)
>>> +        amdgpu_gfx_off_ctrl(adev, false);
>>> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
>>>           if (amdgpu_asic_read_register(adev, se_num, sh_num,
>>>                             info->read_mmr_reg.dword_offset + i,
>>>                             &regs[i])) {
>>>               DRM_DEBUG_KMS("unallowed offset %#x\n",
>>>                         info->read_mmr_reg.dword_offset + i);
>>>               kfree(regs);
>>> +                amdgpu_gfx_off_ctrl(adev, true);
>>>               return -EFAULT;
>>>           }
>>> +        }
>>> +        amdgpu_gfx_off_ctrl(adev, true);
>>>       n = copy_to_user(out, regs, min(size, alloc_size));
>>>       kfree(regs);
>>>       return n ? -EFAULT : 0;
>>> --
>>> 2.23.0
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Yuan, Xiaojie]

Hi Alex,

I tried on Navi14 with Gfxoff enabled (gfx in 'OFF' state when I run the test program) and used a test program to read GRBM_STATUS/CP_STAT/GB_ADDR_CONFIG via DRM_IOCTL_AMDGPU_INFO.

All read out values are valid as below (in this read cycle, I saw gfx block is first awakened and then powered off again automatically):

drm version: 3.36.0
grbm_status: 0x00003028
cp_stat: 0x00000000
gb_addr_config: 0x00000043

At least for Navi, reading register when gfxoff seems not a problem...

Test program:

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <unistd.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/ioctl.h>
#include <fcntl.h>
#include <errno.h>
#include <libdrm/drm.h>
#include <libdrm/amdgpu_drm.h>

int get_reg_val(int fd, int reg_offset, int *val)
{
        struct drm_amdgpu_info request;
        int r;

        memset(&request, 0, sizeof(request));
        request.return_pointer = (uintptr_t)val;
        request.return_size = sizeof(uint32_t);
        request.query = AMDGPU_INFO_READ_MMR_REG;
        request.read_mmr_reg.dword_offset = reg_offset;
        request.read_mmr_reg.count = 1;
        request.read_mmr_reg.instance = 0xffffffff;
        request.read_mmr_reg.flags = 0;

        r = ioctl(fd, DRM_IOCTL_AMDGPU_INFO, &request);
        if (r < 0) {
                perror("failed to read register");
                return -errno;
        }

        return 0;
}

int main(int argc, const char *argv[])
{
        const char *device = "/dev/dri/card0";
        int fd;
        int r;
        struct drm_version version;
        struct drm_auth auth;
        int cp_stat;
        int grbm_status;
        int gb_addr_config;

        fd = open(device, O_RDWR | O_CLOEXEC);
        if (fd < 0) {
                perror("failed to open device\n");
                return -errno;
        }

        memset(&version, 0, sizeof(version));

        r = ioctl(fd, DRM_IOCTL_VERSION, &version);
        if (r < 0) {
                perror("failed to get drm version");
                r = -errno;
                goto close;
        }

        printf("drm version: %d.%d.%d\n", version.version_major,
                                          version.version_minor,
                                          version.version_patchlevel);

        r = get_reg_val(fd, 0x2004, &grbm_status);
        if (r < 0)
                goto close;
        printf("grbm_status: 0x%08x\n", grbm_status);

        r = get_reg_val(fd, 0x21a0, &cp_stat);
        if (r < 0)
                goto close;
        printf("cp_stat: 0x%08x\n", cp_stat);

        r = get_reg_val(fd, 0x263e, &gb_addr_config);
        if (r < 0)
                goto close;
        printf("gb_addr_config: 0x%08x\n", gb_addr_config);

close:
        close(fd);

        return r;
}

BR,
Xiaojie

________________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Yuan, Xiaojie <Xiaojie.Yuan@amd.com>
Sent: Saturday, November 16, 2019 1:02 AM
To: Alex Deucher
Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

Yes. IIRC, some asics' amdgpu_gfx_ctrl() is implemented as synchronous (upon function returns, gfx block is guaranteed to be in power-up state). Anyway, let me confirm about that soon.

BR,
Xiaojie

> On Nov 16, 2019, at 12:52 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>
>> On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>>
>> Hi Alex,
>>
>> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
>>
>> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.
>
> That would be great.  Maybe we can add a delay in that function to
> take that into account?
>
> Thanks!
>
> Alex
>
>>
>> BR,
>> Xiaojie
>>
>>> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>
>>> When gfxoff is enabled, accessing gfx registers via MMIO
>>> can lead to a hang.
>>>
>>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> index 6ddea7607ad0..5f3b3a705b29 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>>>           return -ENOMEM;
>>>       alloc_size = info->read_mmr_reg.count * sizeof(*regs);
>>>
>>> -        for (i = 0; i < info->read_mmr_reg.count; i++)
>>> +        amdgpu_gfx_off_ctrl(adev, false);
>>> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
>>>           if (amdgpu_asic_read_register(adev, se_num, sh_num,
>>>                             info->read_mmr_reg.dword_offset + i,
>>>                             &regs[i])) {
>>>               DRM_DEBUG_KMS("unallowed offset %#x\n",
>>>                         info->read_mmr_reg.dword_offset + i);
>>>               kfree(regs);
>>> +                amdgpu_gfx_off_ctrl(adev, true);
>>>               return -EFAULT;
>>>           }
>>> +        }
>>> +        amdgpu_gfx_off_ctrl(adev, true);
>>>       n = copy_to_user(out, regs, min(size, alloc_size));
>>>       kfree(regs);
>>>       return n ? -EFAULT : 0;
>>> --
>>> 2.23.0
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Christian König]

Hi Xiaojie,

could you add that test to the unit tests we have in libdrm?

Would be rather nice to have,
Christian.

Am 18.11.19 um 15:09 schrieb Yuan, Xiaojie:
> Hi Alex,
>
> I tried on Navi14 with Gfxoff enabled (gfx in 'OFF' state when I run the test program) and used a test program to read GRBM_STATUS/CP_STAT/GB_ADDR_CONFIG via DRM_IOCTL_AMDGPU_INFO.
>
> All read out values are valid as below (in this read cycle, I saw gfx block is first awakened and then powered off again automatically):
>
> drm version: 3.36.0
> grbm_status: 0x00003028
> cp_stat: 0x00000000
> gb_addr_config: 0x00000043
>
> At least for Navi, reading register when gfxoff seems not a problem...
>
> Test program:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <stdint.h>
> #include <unistd.h>
> #include <string.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/ioctl.h>
> #include <fcntl.h>
> #include <errno.h>
> #include <libdrm/drm.h>
> #include <libdrm/amdgpu_drm.h>
>
> int get_reg_val(int fd, int reg_offset, int *val)
> {
>          struct drm_amdgpu_info request;
>          int r;
>
>          memset(&request, 0, sizeof(request));
>          request.return_pointer = (uintptr_t)val;
>          request.return_size = sizeof(uint32_t);
>          request.query = AMDGPU_INFO_READ_MMR_REG;
>          request.read_mmr_reg.dword_offset = reg_offset;
>          request.read_mmr_reg.count = 1;
>          request.read_mmr_reg.instance = 0xffffffff;
>          request.read_mmr_reg.flags = 0;
>
>          r = ioctl(fd, DRM_IOCTL_AMDGPU_INFO, &request);
>          if (r < 0) {
>                  perror("failed to read register");
>                  return -errno;
>          }
>
>          return 0;
> }
>
> int main(int argc, const char *argv[])
> {
>          const char *device = "/dev/dri/card0";
>          int fd;
>          int r;
>          struct drm_version version;
>          struct drm_auth auth;
>          int cp_stat;
>          int grbm_status;
>          int gb_addr_config;
>
>          fd = open(device, O_RDWR | O_CLOEXEC);
>          if (fd < 0) {
>                  perror("failed to open device\n");
>                  return -errno;
>          }
>
>          memset(&version, 0, sizeof(version));
>
>          r = ioctl(fd, DRM_IOCTL_VERSION, &version);
>          if (r < 0) {
>                  perror("failed to get drm version");
>                  r = -errno;
>                  goto close;
>          }
>
>          printf("drm version: %d.%d.%d\n", version.version_major,
>                                            version.version_minor,
>                                            version.version_patchlevel);
>
>          r = get_reg_val(fd, 0x2004, &grbm_status);
>          if (r < 0)
>                  goto close;
>          printf("grbm_status: 0x%08x\n", grbm_status);
>
>          r = get_reg_val(fd, 0x21a0, &cp_stat);
>          if (r < 0)
>                  goto close;
>          printf("cp_stat: 0x%08x\n", cp_stat);
>
>          r = get_reg_val(fd, 0x263e, &gb_addr_config);
>          if (r < 0)
>                  goto close;
>          printf("gb_addr_config: 0x%08x\n", gb_addr_config);
>
> close:
>          close(fd);
>
>          return r;
> }
>
> BR,
> Xiaojie
>
> ________________________________________
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Yuan, Xiaojie <Xiaojie.Yuan@amd.com>
> Sent: Saturday, November 16, 2019 1:02 AM
> To: Alex Deucher
> Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface
>
> Yes. IIRC, some asics' amdgpu_gfx_ctrl() is implemented as synchronous (upon function returns, gfx block is guaranteed to be in power-up state). Anyway, let me confirm about that soon.
>
> BR,
> Xiaojie
>
>> On Nov 16, 2019, at 12:52 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>
>>> On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>>>
>>> Hi Alex,
>>>
>>> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
>>>
>>> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.
>> That would be great.  Maybe we can add a delay in that function to
>> take that into account?
>>
>> Thanks!
>>
>> Alex
>>
>>> BR,
>>> Xiaojie
>>>
>>>> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>>
>>>> When gfxoff is enabled, accessing gfx registers via MMIO
>>>> can lead to a hang.
>>>>
>>>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> index 6ddea7607ad0..5f3b3a705b29 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>>>>            return -ENOMEM;
>>>>        alloc_size = info->read_mmr_reg.count * sizeof(*regs);
>>>>
>>>> -        for (i = 0; i < info->read_mmr_reg.count; i++)
>>>> +        amdgpu_gfx_off_ctrl(adev, false);
>>>> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
>>>>            if (amdgpu_asic_read_register(adev, se_num, sh_num,
>>>>                              info->read_mmr_reg.dword_offset + i,
>>>>                              &regs[i])) {
>>>>                DRM_DEBUG_KMS("unallowed offset %#x\n",
>>>>                          info->read_mmr_reg.dword_offset + i);
>>>>                kfree(regs);
>>>> +                amdgpu_gfx_off_ctrl(adev, true);
>>>>                return -EFAULT;
>>>>            }
>>>> +        }
>>>> +        amdgpu_gfx_off_ctrl(adev, true);
>>>>        n = copy_to_user(out, regs, min(size, alloc_size));
>>>>        kfree(regs);
>>>>        return n ? -EFAULT : 0;
>>>> --
>>>> 2.23.0
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Christian König]

Hi Xiaojie,

could you add that test to the unit tests we have in libdrm?

Would be rather nice to have,
Christian.

Am 18.11.19 um 15:09 schrieb Yuan, Xiaojie:
> Hi Alex,
>
> I tried on Navi14 with Gfxoff enabled (gfx in 'OFF' state when I run the test program) and used a test program to read GRBM_STATUS/CP_STAT/GB_ADDR_CONFIG via DRM_IOCTL_AMDGPU_INFO.
>
> All read out values are valid as below (in this read cycle, I saw gfx block is first awakened and then powered off again automatically):
>
> drm version: 3.36.0
> grbm_status: 0x00003028
> cp_stat: 0x00000000
> gb_addr_config: 0x00000043
>
> At least for Navi, reading register when gfxoff seems not a problem...
>
> Test program:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <stdint.h>
> #include <unistd.h>
> #include <string.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/ioctl.h>
> #include <fcntl.h>
> #include <errno.h>
> #include <libdrm/drm.h>
> #include <libdrm/amdgpu_drm.h>
>
> int get_reg_val(int fd, int reg_offset, int *val)
> {
>          struct drm_amdgpu_info request;
>          int r;
>
>          memset(&request, 0, sizeof(request));
>          request.return_pointer = (uintptr_t)val;
>          request.return_size = sizeof(uint32_t);
>          request.query = AMDGPU_INFO_READ_MMR_REG;
>          request.read_mmr_reg.dword_offset = reg_offset;
>          request.read_mmr_reg.count = 1;
>          request.read_mmr_reg.instance = 0xffffffff;
>          request.read_mmr_reg.flags = 0;
>
>          r = ioctl(fd, DRM_IOCTL_AMDGPU_INFO, &request);
>          if (r < 0) {
>                  perror("failed to read register");
>                  return -errno;
>          }
>
>          return 0;
> }
>
> int main(int argc, const char *argv[])
> {
>          const char *device = "/dev/dri/card0";
>          int fd;
>          int r;
>          struct drm_version version;
>          struct drm_auth auth;
>          int cp_stat;
>          int grbm_status;
>          int gb_addr_config;
>
>          fd = open(device, O_RDWR | O_CLOEXEC);
>          if (fd < 0) {
>                  perror("failed to open device\n");
>                  return -errno;
>          }
>
>          memset(&version, 0, sizeof(version));
>
>          r = ioctl(fd, DRM_IOCTL_VERSION, &version);
>          if (r < 0) {
>                  perror("failed to get drm version");
>                  r = -errno;
>                  goto close;
>          }
>
>          printf("drm version: %d.%d.%d\n", version.version_major,
>                                            version.version_minor,
>                                            version.version_patchlevel);
>
>          r = get_reg_val(fd, 0x2004, &grbm_status);
>          if (r < 0)
>                  goto close;
>          printf("grbm_status: 0x%08x\n", grbm_status);
>
>          r = get_reg_val(fd, 0x21a0, &cp_stat);
>          if (r < 0)
>                  goto close;
>          printf("cp_stat: 0x%08x\n", cp_stat);
>
>          r = get_reg_val(fd, 0x263e, &gb_addr_config);
>          if (r < 0)
>                  goto close;
>          printf("gb_addr_config: 0x%08x\n", gb_addr_config);
>
> close:
>          close(fd);
>
>          return r;
> }
>
> BR,
> Xiaojie
>
> ________________________________________
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Yuan, Xiaojie <Xiaojie.Yuan@amd.com>
> Sent: Saturday, November 16, 2019 1:02 AM
> To: Alex Deucher
> Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface
>
> Yes. IIRC, some asics' amdgpu_gfx_ctrl() is implemented as synchronous (upon function returns, gfx block is guaranteed to be in power-up state). Anyway, let me confirm about that soon.
>
> BR,
> Xiaojie
>
>> On Nov 16, 2019, at 12:52 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>
>>> On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>>>
>>> Hi Alex,
>>>
>>> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
>>>
>>> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.
>> That would be great.  Maybe we can add a delay in that function to
>> take that into account?
>>
>> Thanks!
>>
>> Alex
>>
>>> BR,
>>> Xiaojie
>>>
>>>> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>>
>>>> When gfxoff is enabled, accessing gfx registers via MMIO
>>>> can lead to a hang.
>>>>
>>>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> index 6ddea7607ad0..5f3b3a705b29 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>>>>            return -ENOMEM;
>>>>        alloc_size = info->read_mmr_reg.count * sizeof(*regs);
>>>>
>>>> -        for (i = 0; i < info->read_mmr_reg.count; i++)
>>>> +        amdgpu_gfx_off_ctrl(adev, false);
>>>> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
>>>>            if (amdgpu_asic_read_register(adev, se_num, sh_num,
>>>>                              info->read_mmr_reg.dword_offset + i,
>>>>                              &regs[i])) {
>>>>                DRM_DEBUG_KMS("unallowed offset %#x\n",
>>>>                          info->read_mmr_reg.dword_offset + i);
>>>>                kfree(regs);
>>>> +                amdgpu_gfx_off_ctrl(adev, true);
>>>>                return -EFAULT;
>>>>            }
>>>> +        }
>>>> +        amdgpu_gfx_off_ctrl(adev, true);
>>>>        n = copy_to_user(out, regs, min(size, alloc_size));
>>>>        kfree(regs);
>>>>        return n ? -EFAULT : 0;
>>>> --
>>>> 2.23.0
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Yuan, Xiaojie]

Hi Chris,

The info ioctl code path seems already covered at libdrm initialization time:

amdgpu_device_initialize()
-> amdgpu_queury_gpu_info_init()
    -> amdgpu_read_mm_registers()
        -> drmCommandWrite(fd, DRM_AMDGPU_INFO, ...)

BR,
Xiaojie

________________________________________
From: Christian König <ckoenig.leichtzumerken@gmail.com>
Sent: Tuesday, November 19, 2019 12:18 AM
To: Yuan, Xiaojie; Alex Deucher
Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

Hi Xiaojie,

could you add that test to the unit tests we have in libdrm?

Would be rather nice to have,
Christian.

Am 18.11.19 um 15:09 schrieb Yuan, Xiaojie:
> Hi Alex,
>
> I tried on Navi14 with Gfxoff enabled (gfx in 'OFF' state when I run the test program) and used a test program to read GRBM_STATUS/CP_STAT/GB_ADDR_CONFIG via DRM_IOCTL_AMDGPU_INFO.
>
> All read out values are valid as below (in this read cycle, I saw gfx block is first awakened and then powered off again automatically):
>
> drm version: 3.36.0
> grbm_status: 0x00003028
> cp_stat: 0x00000000
> gb_addr_config: 0x00000043
>
> At least for Navi, reading register when gfxoff seems not a problem...
>
> Test program:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <stdint.h>
> #include <unistd.h>
> #include <string.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/ioctl.h>
> #include <fcntl.h>
> #include <errno.h>
> #include <libdrm/drm.h>
> #include <libdrm/amdgpu_drm.h>
>
> int get_reg_val(int fd, int reg_offset, int *val)
> {
>          struct drm_amdgpu_info request;
>          int r;
>
>          memset(&request, 0, sizeof(request));
>          request.return_pointer = (uintptr_t)val;
>          request.return_size = sizeof(uint32_t);
>          request.query = AMDGPU_INFO_READ_MMR_REG;
>          request.read_mmr_reg.dword_offset = reg_offset;
>          request.read_mmr_reg.count = 1;
>          request.read_mmr_reg.instance = 0xffffffff;
>          request.read_mmr_reg.flags = 0;
>
>          r = ioctl(fd, DRM_IOCTL_AMDGPU_INFO, &request);
>          if (r < 0) {
>                  perror("failed to read register");
>                  return -errno;
>          }
>
>          return 0;
> }
>
> int main(int argc, const char *argv[])
> {
>          const char *device = "/dev/dri/card0";
>          int fd;
>          int r;
>          struct drm_version version;
>          struct drm_auth auth;
>          int cp_stat;
>          int grbm_status;
>          int gb_addr_config;
>
>          fd = open(device, O_RDWR | O_CLOEXEC);
>          if (fd < 0) {
>                  perror("failed to open device\n");
>                  return -errno;
>          }
>
>          memset(&version, 0, sizeof(version));
>
>          r = ioctl(fd, DRM_IOCTL_VERSION, &version);
>          if (r < 0) {
>                  perror("failed to get drm version");
>                  r = -errno;
>                  goto close;
>          }
>
>          printf("drm version: %d.%d.%d\n", version.version_major,
>                                            version.version_minor,
>                                            version.version_patchlevel);
>
>          r = get_reg_val(fd, 0x2004, &grbm_status);
>          if (r < 0)
>                  goto close;
>          printf("grbm_status: 0x%08x\n", grbm_status);
>
>          r = get_reg_val(fd, 0x21a0, &cp_stat);
>          if (r < 0)
>                  goto close;
>          printf("cp_stat: 0x%08x\n", cp_stat);
>
>          r = get_reg_val(fd, 0x263e, &gb_addr_config);
>          if (r < 0)
>                  goto close;
>          printf("gb_addr_config: 0x%08x\n", gb_addr_config);
>
> close:
>          close(fd);
>
>          return r;
> }
>
> BR,
> Xiaojie
>
> ________________________________________
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Yuan, Xiaojie <Xiaojie.Yuan@amd.com>
> Sent: Saturday, November 16, 2019 1:02 AM
> To: Alex Deucher
> Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface
>
> Yes. IIRC, some asics' amdgpu_gfx_ctrl() is implemented as synchronous (upon function returns, gfx block is guaranteed to be in power-up state). Anyway, let me confirm about that soon.
>
> BR,
> Xiaojie
>
>> On Nov 16, 2019, at 12:52 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>
>>> On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>>>
>>> Hi Alex,
>>>
>>> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
>>>
>>> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.
>> That would be great.  Maybe we can add a delay in that function to
>> take that into account?
>>
>> Thanks!
>>
>> Alex
>>
>>> BR,
>>> Xiaojie
>>>
>>>> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>>
>>>> When gfxoff is enabled, accessing gfx registers via MMIO
>>>> can lead to a hang.
>>>>
>>>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> index 6ddea7607ad0..5f3b3a705b29 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>>>>            return -ENOMEM;
>>>>        alloc_size = info->read_mmr_reg.count * sizeof(*regs);
>>>>
>>>> -        for (i = 0; i < info->read_mmr_reg.count; i++)
>>>> +        amdgpu_gfx_off_ctrl(adev, false);
>>>> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
>>>>            if (amdgpu_asic_read_register(adev, se_num, sh_num,
>>>>                              info->read_mmr_reg.dword_offset + i,
>>>>                              &regs[i])) {
>>>>                DRM_DEBUG_KMS("unallowed offset %#x\n",
>>>>                          info->read_mmr_reg.dword_offset + i);
>>>>                kfree(regs);
>>>> +                amdgpu_gfx_off_ctrl(adev, true);
>>>>                return -EFAULT;
>>>>            }
>>>> +        }
>>>> +        amdgpu_gfx_off_ctrl(adev, true);
>>>>        n = copy_to_user(out, regs, min(size, alloc_size));
>>>>        kfree(regs);
>>>>        return n ? -EFAULT : 0;
>>>> --
>>>> 2.23.0
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Yuan, Xiaojie]

Hi Chris,

The info ioctl code path seems already covered at libdrm initialization time:

amdgpu_device_initialize()
-> amdgpu_queury_gpu_info_init()
    -> amdgpu_read_mm_registers()
        -> drmCommandWrite(fd, DRM_AMDGPU_INFO, ...)

BR,
Xiaojie

________________________________________
From: Christian König <ckoenig.leichtzumerken@gmail.com>
Sent: Tuesday, November 19, 2019 12:18 AM
To: Yuan, Xiaojie; Alex Deucher
Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

Hi Xiaojie,

could you add that test to the unit tests we have in libdrm?

Would be rather nice to have,
Christian.

Am 18.11.19 um 15:09 schrieb Yuan, Xiaojie:
> Hi Alex,
>
> I tried on Navi14 with Gfxoff enabled (gfx in 'OFF' state when I run the test program) and used a test program to read GRBM_STATUS/CP_STAT/GB_ADDR_CONFIG via DRM_IOCTL_AMDGPU_INFO.
>
> All read out values are valid as below (in this read cycle, I saw gfx block is first awakened and then powered off again automatically):
>
> drm version: 3.36.0
> grbm_status: 0x00003028
> cp_stat: 0x00000000
> gb_addr_config: 0x00000043
>
> At least for Navi, reading register when gfxoff seems not a problem...
>
> Test program:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <stdint.h>
> #include <unistd.h>
> #include <string.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/ioctl.h>
> #include <fcntl.h>
> #include <errno.h>
> #include <libdrm/drm.h>
> #include <libdrm/amdgpu_drm.h>
>
> int get_reg_val(int fd, int reg_offset, int *val)
> {
>          struct drm_amdgpu_info request;
>          int r;
>
>          memset(&request, 0, sizeof(request));
>          request.return_pointer = (uintptr_t)val;
>          request.return_size = sizeof(uint32_t);
>          request.query = AMDGPU_INFO_READ_MMR_REG;
>          request.read_mmr_reg.dword_offset = reg_offset;
>          request.read_mmr_reg.count = 1;
>          request.read_mmr_reg.instance = 0xffffffff;
>          request.read_mmr_reg.flags = 0;
>
>          r = ioctl(fd, DRM_IOCTL_AMDGPU_INFO, &request);
>          if (r < 0) {
>                  perror("failed to read register");
>                  return -errno;
>          }
>
>          return 0;
> }
>
> int main(int argc, const char *argv[])
> {
>          const char *device = "/dev/dri/card0";
>          int fd;
>          int r;
>          struct drm_version version;
>          struct drm_auth auth;
>          int cp_stat;
>          int grbm_status;
>          int gb_addr_config;
>
>          fd = open(device, O_RDWR | O_CLOEXEC);
>          if (fd < 0) {
>                  perror("failed to open device\n");
>                  return -errno;
>          }
>
>          memset(&version, 0, sizeof(version));
>
>          r = ioctl(fd, DRM_IOCTL_VERSION, &version);
>          if (r < 0) {
>                  perror("failed to get drm version");
>                  r = -errno;
>                  goto close;
>          }
>
>          printf("drm version: %d.%d.%d\n", version.version_major,
>                                            version.version_minor,
>                                            version.version_patchlevel);
>
>          r = get_reg_val(fd, 0x2004, &grbm_status);
>          if (r < 0)
>                  goto close;
>          printf("grbm_status: 0x%08x\n", grbm_status);
>
>          r = get_reg_val(fd, 0x21a0, &cp_stat);
>          if (r < 0)
>                  goto close;
>          printf("cp_stat: 0x%08x\n", cp_stat);
>
>          r = get_reg_val(fd, 0x263e, &gb_addr_config);
>          if (r < 0)
>                  goto close;
>          printf("gb_addr_config: 0x%08x\n", gb_addr_config);
>
> close:
>          close(fd);
>
>          return r;
> }
>
> BR,
> Xiaojie
>
> ________________________________________
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Yuan, Xiaojie <Xiaojie.Yuan@amd.com>
> Sent: Saturday, November 16, 2019 1:02 AM
> To: Alex Deucher
> Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface
>
> Yes. IIRC, some asics' amdgpu_gfx_ctrl() is implemented as synchronous (upon function returns, gfx block is guaranteed to be in power-up state). Anyway, let me confirm about that soon.
>
> BR,
> Xiaojie
>
>> On Nov 16, 2019, at 12:52 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>
>>> On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>>>
>>> Hi Alex,
>>>
>>> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
>>>
>>> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.
>> That would be great.  Maybe we can add a delay in that function to
>> take that into account?
>>
>> Thanks!
>>
>> Alex
>>
>>> BR,
>>> Xiaojie
>>>
>>>> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>>
>>>> When gfxoff is enabled, accessing gfx registers via MMIO
>>>> can lead to a hang.
>>>>
>>>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> index 6ddea7607ad0..5f3b3a705b29 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>>>>            return -ENOMEM;
>>>>        alloc_size = info->read_mmr_reg.count * sizeof(*regs);
>>>>
>>>> -        for (i = 0; i < info->read_mmr_reg.count; i++)
>>>> +        amdgpu_gfx_off_ctrl(adev, false);
>>>> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
>>>>            if (amdgpu_asic_read_register(adev, se_num, sh_num,
>>>>                              info->read_mmr_reg.dword_offset + i,
>>>>                              &regs[i])) {
>>>>                DRM_DEBUG_KMS("unallowed offset %#x\n",
>>>>                          info->read_mmr_reg.dword_offset + i);
>>>>                kfree(regs);
>>>> +                amdgpu_gfx_off_ctrl(adev, true);
>>>>                return -EFAULT;
>>>>            }
>>>> +        }
>>>> +        amdgpu_gfx_off_ctrl(adev, true);
>>>>        n = copy_to_user(out, regs, min(size, alloc_size));
>>>>        kfree(regs);
>>>>        return n ? -EFAULT : 0;
>>>> --
>>>> 2.23.0
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Alex Deucher]

Can I get an RB or AB on the patch from anyone?

Thanks!

Alex

On Mon, Nov 18, 2019 at 9:09 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>
> Hi Alex,
>
> I tried on Navi14 with Gfxoff enabled (gfx in 'OFF' state when I run the test program) and used a test program to read GRBM_STATUS/CP_STAT/GB_ADDR_CONFIG via DRM_IOCTL_AMDGPU_INFO.
>
> All read out values are valid as below (in this read cycle, I saw gfx block is first awakened and then powered off again automatically):
>
> drm version: 3.36.0
> grbm_status: 0x00003028
> cp_stat: 0x00000000
> gb_addr_config: 0x00000043
>
> At least for Navi, reading register when gfxoff seems not a problem...
>
> Test program:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <stdint.h>
> #include <unistd.h>
> #include <string.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/ioctl.h>
> #include <fcntl.h>
> #include <errno.h>
> #include <libdrm/drm.h>
> #include <libdrm/amdgpu_drm.h>
>
> int get_reg_val(int fd, int reg_offset, int *val)
> {
>         struct drm_amdgpu_info request;
>         int r;
>
>         memset(&request, 0, sizeof(request));
>         request.return_pointer = (uintptr_t)val;
>         request.return_size = sizeof(uint32_t);
>         request.query = AMDGPU_INFO_READ_MMR_REG;
>         request.read_mmr_reg.dword_offset = reg_offset;
>         request.read_mmr_reg.count = 1;
>         request.read_mmr_reg.instance = 0xffffffff;
>         request.read_mmr_reg.flags = 0;
>
>         r = ioctl(fd, DRM_IOCTL_AMDGPU_INFO, &request);
>         if (r < 0) {
>                 perror("failed to read register");
>                 return -errno;
>         }
>
>         return 0;
> }
>
> int main(int argc, const char *argv[])
> {
>         const char *device = "/dev/dri/card0";
>         int fd;
>         int r;
>         struct drm_version version;
>         struct drm_auth auth;
>         int cp_stat;
>         int grbm_status;
>         int gb_addr_config;
>
>         fd = open(device, O_RDWR | O_CLOEXEC);
>         if (fd < 0) {
>                 perror("failed to open device\n");
>                 return -errno;
>         }
>
>         memset(&version, 0, sizeof(version));
>
>         r = ioctl(fd, DRM_IOCTL_VERSION, &version);
>         if (r < 0) {
>                 perror("failed to get drm version");
>                 r = -errno;
>                 goto close;
>         }
>
>         printf("drm version: %d.%d.%d\n", version.version_major,
>                                           version.version_minor,
>                                           version.version_patchlevel);
>
>         r = get_reg_val(fd, 0x2004, &grbm_status);
>         if (r < 0)
>                 goto close;
>         printf("grbm_status: 0x%08x\n", grbm_status);
>
>         r = get_reg_val(fd, 0x21a0, &cp_stat);
>         if (r < 0)
>                 goto close;
>         printf("cp_stat: 0x%08x\n", cp_stat);
>
>         r = get_reg_val(fd, 0x263e, &gb_addr_config);
>         if (r < 0)
>                 goto close;
>         printf("gb_addr_config: 0x%08x\n", gb_addr_config);
>
> close:
>         close(fd);
>
>         return r;
> }
>
> BR,
> Xiaojie
>
> ________________________________________
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Yuan, Xiaojie <Xiaojie.Yuan@amd.com>
> Sent: Saturday, November 16, 2019 1:02 AM
> To: Alex Deucher
> Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface
>
> Yes. IIRC, some asics' amdgpu_gfx_ctrl() is implemented as synchronous (upon function returns, gfx block is guaranteed to be in power-up state). Anyway, let me confirm about that soon.
>
> BR,
> Xiaojie
>
> > On Nov 16, 2019, at 12:52 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> >> On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
> >>
> >> Hi Alex,
> >>
> >> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
> >>
> >> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.
> >
> > That would be great.  Maybe we can add a delay in that function to
> > take that into account?
> >
> > Thanks!
> >
> > Alex
> >
> >>
> >> BR,
> >> Xiaojie
> >>
> >>> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> >>>
> >>> When gfxoff is enabled, accessing gfx registers via MMIO
> >>> can lead to a hang.
> >>>
> >>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
> >>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >>> ---
> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
> >>> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>> index 6ddea7607ad0..5f3b3a705b29 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
> >>>           return -ENOMEM;
> >>>       alloc_size = info->read_mmr_reg.count * sizeof(*regs);
> >>>
> >>> -        for (i = 0; i < info->read_mmr_reg.count; i++)
> >>> +        amdgpu_gfx_off_ctrl(adev, false);
> >>> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
> >>>           if (amdgpu_asic_read_register(adev, se_num, sh_num,
> >>>                             info->read_mmr_reg.dword_offset + i,
> >>>                             &regs[i])) {
> >>>               DRM_DEBUG_KMS("unallowed offset %#x\n",
> >>>                         info->read_mmr_reg.dword_offset + i);
> >>>               kfree(regs);
> >>> +                amdgpu_gfx_off_ctrl(adev, true);
> >>>               return -EFAULT;
> >>>           }
> >>> +        }
> >>> +        amdgpu_gfx_off_ctrl(adev, true);
> >>>       n = copy_to_user(out, regs, min(size, alloc_size));
> >>>       kfree(regs);
> >>>       return n ? -EFAULT : 0;
> >>> --
> >>> 2.23.0
> >>>
> >>> _______________________________________________
> >>> amd-gfx mailing list
> >>> amd-gfx@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Alex Deucher]

Can I get an RB or AB on the patch from anyone?

Thanks!

Alex

On Mon, Nov 18, 2019 at 9:09 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>
> Hi Alex,
>
> I tried on Navi14 with Gfxoff enabled (gfx in 'OFF' state when I run the test program) and used a test program to read GRBM_STATUS/CP_STAT/GB_ADDR_CONFIG via DRM_IOCTL_AMDGPU_INFO.
>
> All read out values are valid as below (in this read cycle, I saw gfx block is first awakened and then powered off again automatically):
>
> drm version: 3.36.0
> grbm_status: 0x00003028
> cp_stat: 0x00000000
> gb_addr_config: 0x00000043
>
> At least for Navi, reading register when gfxoff seems not a problem...
>
> Test program:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <stdint.h>
> #include <unistd.h>
> #include <string.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/ioctl.h>
> #include <fcntl.h>
> #include <errno.h>
> #include <libdrm/drm.h>
> #include <libdrm/amdgpu_drm.h>
>
> int get_reg_val(int fd, int reg_offset, int *val)
> {
>         struct drm_amdgpu_info request;
>         int r;
>
>         memset(&request, 0, sizeof(request));
>         request.return_pointer = (uintptr_t)val;
>         request.return_size = sizeof(uint32_t);
>         request.query = AMDGPU_INFO_READ_MMR_REG;
>         request.read_mmr_reg.dword_offset = reg_offset;
>         request.read_mmr_reg.count = 1;
>         request.read_mmr_reg.instance = 0xffffffff;
>         request.read_mmr_reg.flags = 0;
>
>         r = ioctl(fd, DRM_IOCTL_AMDGPU_INFO, &request);
>         if (r < 0) {
>                 perror("failed to read register");
>                 return -errno;
>         }
>
>         return 0;
> }
>
> int main(int argc, const char *argv[])
> {
>         const char *device = "/dev/dri/card0";
>         int fd;
>         int r;
>         struct drm_version version;
>         struct drm_auth auth;
>         int cp_stat;
>         int grbm_status;
>         int gb_addr_config;
>
>         fd = open(device, O_RDWR | O_CLOEXEC);
>         if (fd < 0) {
>                 perror("failed to open device\n");
>                 return -errno;
>         }
>
>         memset(&version, 0, sizeof(version));
>
>         r = ioctl(fd, DRM_IOCTL_VERSION, &version);
>         if (r < 0) {
>                 perror("failed to get drm version");
>                 r = -errno;
>                 goto close;
>         }
>
>         printf("drm version: %d.%d.%d\n", version.version_major,
>                                           version.version_minor,
>                                           version.version_patchlevel);
>
>         r = get_reg_val(fd, 0x2004, &grbm_status);
>         if (r < 0)
>                 goto close;
>         printf("grbm_status: 0x%08x\n", grbm_status);
>
>         r = get_reg_val(fd, 0x21a0, &cp_stat);
>         if (r < 0)
>                 goto close;
>         printf("cp_stat: 0x%08x\n", cp_stat);
>
>         r = get_reg_val(fd, 0x263e, &gb_addr_config);
>         if (r < 0)
>                 goto close;
>         printf("gb_addr_config: 0x%08x\n", gb_addr_config);
>
> close:
>         close(fd);
>
>         return r;
> }
>
> BR,
> Xiaojie
>
> ________________________________________
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Yuan, Xiaojie <Xiaojie.Yuan@amd.com>
> Sent: Saturday, November 16, 2019 1:02 AM
> To: Alex Deucher
> Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface
>
> Yes. IIRC, some asics' amdgpu_gfx_ctrl() is implemented as synchronous (upon function returns, gfx block is guaranteed to be in power-up state). Anyway, let me confirm about that soon.
>
> BR,
> Xiaojie
>
> > On Nov 16, 2019, at 12:52 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> >> On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
> >>
> >> Hi Alex,
> >>
> >> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
> >>
> >> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.
> >
> > That would be great.  Maybe we can add a delay in that function to
> > take that into account?
> >
> > Thanks!
> >
> > Alex
> >
> >>
> >> BR,
> >> Xiaojie
> >>
> >>> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> >>>
> >>> When gfxoff is enabled, accessing gfx registers via MMIO
> >>> can lead to a hang.
> >>>
> >>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
> >>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >>> ---
> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
> >>> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>> index 6ddea7607ad0..5f3b3a705b29 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
> >>>           return -ENOMEM;
> >>>       alloc_size = info->read_mmr_reg.count * sizeof(*regs);
> >>>
> >>> -        for (i = 0; i < info->read_mmr_reg.count; i++)
> >>> +        amdgpu_gfx_off_ctrl(adev, false);
> >>> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
> >>>           if (amdgpu_asic_read_register(adev, se_num, sh_num,
> >>>                             info->read_mmr_reg.dword_offset + i,
> >>>                             &regs[i])) {
> >>>               DRM_DEBUG_KMS("unallowed offset %#x\n",
> >>>                         info->read_mmr_reg.dword_offset + i);
> >>>               kfree(regs);
> >>> +                amdgpu_gfx_off_ctrl(adev, true);
> >>>               return -EFAULT;
> >>>           }
> >>> +        }
> >>> +        amdgpu_gfx_off_ctrl(adev, true);
> >>>       n = copy_to_user(out, regs, min(size, alloc_size));
> >>>       kfree(regs);
> >>>       return n ? -EFAULT : 0;
> >>> --
> >>> 2.23.0
> >>>
> >>> _______________________________________________
> >>> amd-gfx mailing list
> >>> amd-gfx@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Yuan, Xiaojie]

Acked-by: Xiaojie Yuan <xiaojie.yuan@amd.com>

BR,
Xiaojie

________________________________________
From: Alex Deucher <alexdeucher@gmail.com>
Sent: Tuesday, November 19, 2019 1:24 AM
To: Yuan, Xiaojie
Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

Can I get an RB or AB on the patch from anyone?

Thanks!

Alex

On Mon, Nov 18, 2019 at 9:09 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>
> Hi Alex,
>
> I tried on Navi14 with Gfxoff enabled (gfx in 'OFF' state when I run the test program) and used a test program to read GRBM_STATUS/CP_STAT/GB_ADDR_CONFIG via DRM_IOCTL_AMDGPU_INFO.
>
> All read out values are valid as below (in this read cycle, I saw gfx block is first awakened and then powered off again automatically):
>
> drm version: 3.36.0
> grbm_status: 0x00003028
> cp_stat: 0x00000000
> gb_addr_config: 0x00000043
>
> At least for Navi, reading register when gfxoff seems not a problem...
>
> Test program:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <stdint.h>
> #include <unistd.h>
> #include <string.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/ioctl.h>
> #include <fcntl.h>
> #include <errno.h>
> #include <libdrm/drm.h>
> #include <libdrm/amdgpu_drm.h>
>
> int get_reg_val(int fd, int reg_offset, int *val)
> {
>         struct drm_amdgpu_info request;
>         int r;
>
>         memset(&request, 0, sizeof(request));
>         request.return_pointer = (uintptr_t)val;
>         request.return_size = sizeof(uint32_t);
>         request.query = AMDGPU_INFO_READ_MMR_REG;
>         request.read_mmr_reg.dword_offset = reg_offset;
>         request.read_mmr_reg.count = 1;
>         request.read_mmr_reg.instance = 0xffffffff;
>         request.read_mmr_reg.flags = 0;
>
>         r = ioctl(fd, DRM_IOCTL_AMDGPU_INFO, &request);
>         if (r < 0) {
>                 perror("failed to read register");
>                 return -errno;
>         }
>
>         return 0;
> }
>
> int main(int argc, const char *argv[])
> {
>         const char *device = "/dev/dri/card0";
>         int fd;
>         int r;
>         struct drm_version version;
>         struct drm_auth auth;
>         int cp_stat;
>         int grbm_status;
>         int gb_addr_config;
>
>         fd = open(device, O_RDWR | O_CLOEXEC);
>         if (fd < 0) {
>                 perror("failed to open device\n");
>                 return -errno;
>         }
>
>         memset(&version, 0, sizeof(version));
>
>         r = ioctl(fd, DRM_IOCTL_VERSION, &version);
>         if (r < 0) {
>                 perror("failed to get drm version");
>                 r = -errno;
>                 goto close;
>         }
>
>         printf("drm version: %d.%d.%d\n", version.version_major,
>                                           version.version_minor,
>                                           version.version_patchlevel);
>
>         r = get_reg_val(fd, 0x2004, &grbm_status);
>         if (r < 0)
>                 goto close;
>         printf("grbm_status: 0x%08x\n", grbm_status);
>
>         r = get_reg_val(fd, 0x21a0, &cp_stat);
>         if (r < 0)
>                 goto close;
>         printf("cp_stat: 0x%08x\n", cp_stat);
>
>         r = get_reg_val(fd, 0x263e, &gb_addr_config);
>         if (r < 0)
>                 goto close;
>         printf("gb_addr_config: 0x%08x\n", gb_addr_config);
>
> close:
>         close(fd);
>
>         return r;
> }
>
> BR,
> Xiaojie
>
> ________________________________________
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Yuan, Xiaojie <Xiaojie.Yuan@amd.com>
> Sent: Saturday, November 16, 2019 1:02 AM
> To: Alex Deucher
> Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface
>
> Yes. IIRC, some asics' amdgpu_gfx_ctrl() is implemented as synchronous (upon function returns, gfx block is guaranteed to be in power-up state). Anyway, let me confirm about that soon.
>
> BR,
> Xiaojie
>
> > On Nov 16, 2019, at 12:52 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> >> On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
> >>
> >> Hi Alex,
> >>
> >> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
> >>
> >> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.
> >
> > That would be great.  Maybe we can add a delay in that function to
> > take that into account?
> >
> > Thanks!
> >
> > Alex
> >
> >>
> >> BR,
> >> Xiaojie
> >>
> >>> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> >>>
> >>> When gfxoff is enabled, accessing gfx registers via MMIO
> >>> can lead to a hang.
> >>>
> >>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
> >>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >>> ---
> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
> >>> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>> index 6ddea7607ad0..5f3b3a705b29 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
> >>>           return -ENOMEM;
> >>>       alloc_size = info->read_mmr_reg.count * sizeof(*regs);
> >>>
> >>> -        for (i = 0; i < info->read_mmr_reg.count; i++)
> >>> +        amdgpu_gfx_off_ctrl(adev, false);
> >>> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
> >>>           if (amdgpu_asic_read_register(adev, se_num, sh_num,
> >>>                             info->read_mmr_reg.dword_offset + i,
> >>>                             &regs[i])) {
> >>>               DRM_DEBUG_KMS("unallowed offset %#x\n",
> >>>                         info->read_mmr_reg.dword_offset + i);
> >>>               kfree(regs);
> >>> +                amdgpu_gfx_off_ctrl(adev, true);
> >>>               return -EFAULT;
> >>>           }
> >>> +        }
> >>> +        amdgpu_gfx_off_ctrl(adev, true);
> >>>       n = copy_to_user(out, regs, min(size, alloc_size));
> >>>       kfree(regs);
> >>>       return n ? -EFAULT : 0;
> >>> --
> >>> 2.23.0
> >>>
> >>> _______________________________________________
> >>> amd-gfx mailing list
> >>> amd-gfx@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Yuan, Xiaojie]

Acked-by: Xiaojie Yuan <xiaojie.yuan@amd.com>

BR,
Xiaojie

________________________________________
From: Alex Deucher <alexdeucher@gmail.com>
Sent: Tuesday, November 19, 2019 1:24 AM
To: Yuan, Xiaojie
Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

Can I get an RB or AB on the patch from anyone?

Thanks!

Alex

On Mon, Nov 18, 2019 at 9:09 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
>
> Hi Alex,
>
> I tried on Navi14 with Gfxoff enabled (gfx in 'OFF' state when I run the test program) and used a test program to read GRBM_STATUS/CP_STAT/GB_ADDR_CONFIG via DRM_IOCTL_AMDGPU_INFO.
>
> All read out values are valid as below (in this read cycle, I saw gfx block is first awakened and then powered off again automatically):
>
> drm version: 3.36.0
> grbm_status: 0x00003028
> cp_stat: 0x00000000
> gb_addr_config: 0x00000043
>
> At least for Navi, reading register when gfxoff seems not a problem...
>
> Test program:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <stdint.h>
> #include <unistd.h>
> #include <string.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/ioctl.h>
> #include <fcntl.h>
> #include <errno.h>
> #include <libdrm/drm.h>
> #include <libdrm/amdgpu_drm.h>
>
> int get_reg_val(int fd, int reg_offset, int *val)
> {
>         struct drm_amdgpu_info request;
>         int r;
>
>         memset(&request, 0, sizeof(request));
>         request.return_pointer = (uintptr_t)val;
>         request.return_size = sizeof(uint32_t);
>         request.query = AMDGPU_INFO_READ_MMR_REG;
>         request.read_mmr_reg.dword_offset = reg_offset;
>         request.read_mmr_reg.count = 1;
>         request.read_mmr_reg.instance = 0xffffffff;
>         request.read_mmr_reg.flags = 0;
>
>         r = ioctl(fd, DRM_IOCTL_AMDGPU_INFO, &request);
>         if (r < 0) {
>                 perror("failed to read register");
>                 return -errno;
>         }
>
>         return 0;
> }
>
> int main(int argc, const char *argv[])
> {
>         const char *device = "/dev/dri/card0";
>         int fd;
>         int r;
>         struct drm_version version;
>         struct drm_auth auth;
>         int cp_stat;
>         int grbm_status;
>         int gb_addr_config;
>
>         fd = open(device, O_RDWR | O_CLOEXEC);
>         if (fd < 0) {
>                 perror("failed to open device\n");
>                 return -errno;
>         }
>
>         memset(&version, 0, sizeof(version));
>
>         r = ioctl(fd, DRM_IOCTL_VERSION, &version);
>         if (r < 0) {
>                 perror("failed to get drm version");
>                 r = -errno;
>                 goto close;
>         }
>
>         printf("drm version: %d.%d.%d\n", version.version_major,
>                                           version.version_minor,
>                                           version.version_patchlevel);
>
>         r = get_reg_val(fd, 0x2004, &grbm_status);
>         if (r < 0)
>                 goto close;
>         printf("grbm_status: 0x%08x\n", grbm_status);
>
>         r = get_reg_val(fd, 0x21a0, &cp_stat);
>         if (r < 0)
>                 goto close;
>         printf("cp_stat: 0x%08x\n", cp_stat);
>
>         r = get_reg_val(fd, 0x263e, &gb_addr_config);
>         if (r < 0)
>                 goto close;
>         printf("gb_addr_config: 0x%08x\n", gb_addr_config);
>
> close:
>         close(fd);
>
>         return r;
> }
>
> BR,
> Xiaojie
>
> ________________________________________
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Yuan, Xiaojie <Xiaojie.Yuan@amd.com>
> Sent: Saturday, November 16, 2019 1:02 AM
> To: Alex Deucher
> Cc: Deucher, Alexander; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface
>
> Yes. IIRC, some asics' amdgpu_gfx_ctrl() is implemented as synchronous (upon function returns, gfx block is guaranteed to be in power-up state). Anyway, let me confirm about that soon.
>
> BR,
> Xiaojie
>
> > On Nov 16, 2019, at 12:52 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> >> On Fri, Nov 15, 2019 at 11:46 AM Yuan, Xiaojie <Xiaojie.Yuan@amd.com> wrote:
> >>
> >> Hi Alex,
> >>
> >> IMHO, driver sending Disallow_Gfxoff message to SMU doesn't mean gfx block will be immediately powered up, so I'm not sure MMIO register access will be successful within this time window(maybe GRBM access will be pending until gfx block is powered up?)
> >>
> >> If you are not in a hurry to commit this fix, I can verify on my Navi boards next Monday.
> >
> > That would be great.  Maybe we can add a delay in that function to
> > take that into account?
> >
> > Thanks!
> >
> > Alex
> >
> >>
> >> BR,
> >> Xiaojie
> >>
> >>> On Nov 15, 2019, at 12:44 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> >>>
> >>> When gfxoff is enabled, accessing gfx registers via MMIO
> >>> can lead to a hang.
> >>>
> >>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
> >>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >>> ---
> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
> >>> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>> index 6ddea7607ad0..5f3b3a705b29 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >>> @@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
> >>>           return -ENOMEM;
> >>>       alloc_size = info->read_mmr_reg.count * sizeof(*regs);
> >>>
> >>> -        for (i = 0; i < info->read_mmr_reg.count; i++)
> >>> +        amdgpu_gfx_off_ctrl(adev, false);
> >>> +        for (i = 0; i < info->read_mmr_reg.count; i++) {
> >>>           if (amdgpu_asic_read_register(adev, se_num, sh_num,
> >>>                             info->read_mmr_reg.dword_offset + i,
> >>>                             &regs[i])) {
> >>>               DRM_DEBUG_KMS("unallowed offset %#x\n",
> >>>                         info->read_mmr_reg.dword_offset + i);
> >>>               kfree(regs);
> >>> +                amdgpu_gfx_off_ctrl(adev, true);
> >>>               return -EFAULT;
> >>>           }
> >>> +        }
> >>> +        amdgpu_gfx_off_ctrl(adev, true);
> >>>       n = copy_to_user(out, regs, min(size, alloc_size));
> >>>       kfree(regs);
> >>>       return n ? -EFAULT : 0;
> >>> --
> >>> 2.23.0
> >>>
> >>> _______________________________________________
> >>> amd-gfx mailing list
> >>> amd-gfx@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Quan, Evan]

Series is reviewed-by: Evan Quan <evan.quan@amd.com>

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
Sent: Friday, November 15, 2019 12:42 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

When gfxoff is enabled, accessing gfx registers via MMIO can lead to a hang.

Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 6ddea7607ad0..5f3b3a705b29 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
 			return -ENOMEM;
 		alloc_size = info->read_mmr_reg.count * sizeof(*regs);
 
-		for (i = 0; i < info->read_mmr_reg.count; i++)
+		amdgpu_gfx_off_ctrl(adev, false);
+		for (i = 0; i < info->read_mmr_reg.count; i++) {
 			if (amdgpu_asic_read_register(adev, se_num, sh_num,
 						      info->read_mmr_reg.dword_offset + i,
 						      &regs[i])) {
 				DRM_DEBUG_KMS("unallowed offset %#x\n",
 					      info->read_mmr_reg.dword_offset + i);
 				kfree(regs);
+				amdgpu_gfx_off_ctrl(adev, true);
 				return -EFAULT;
 			}
+		}
+		amdgpu_gfx_off_ctrl(adev, true);
 		n = copy_to_user(out, regs, min(size, alloc_size));
 		kfree(regs);
 		return n ? -EFAULT : 0;
--
2.23.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

[Quan, Evan]

Series is reviewed-by: Evan Quan <evan.quan@amd.com>

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
Sent: Friday, November 15, 2019 12:42 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: [PATCH 2/2] drm/amdgpu: disable gfxoff when using register read interface

When gfxoff is enabled, accessing gfx registers via MMIO can lead to a hang.

Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 6ddea7607ad0..5f3b3a705b29 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -659,15 +659,19 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
 			return -ENOMEM;
 		alloc_size = info->read_mmr_reg.count * sizeof(*regs);
 
-		for (i = 0; i < info->read_mmr_reg.count; i++)
+		amdgpu_gfx_off_ctrl(adev, false);
+		for (i = 0; i < info->read_mmr_reg.count; i++) {
 			if (amdgpu_asic_read_register(adev, se_num, sh_num,
 						      info->read_mmr_reg.dword_offset + i,
 						      &regs[i])) {
 				DRM_DEBUG_KMS("unallowed offset %#x\n",
 					      info->read_mmr_reg.dword_offset + i);
 				kfree(regs);
+				amdgpu_gfx_off_ctrl(adev, true);
 				return -EFAULT;
 			}
+		}
+		amdgpu_gfx_off_ctrl(adev, true);
 		n = copy_to_user(out, regs, min(size, alloc_size));
 		kfree(regs);
 		return n ? -EFAULT : 0;
--
2.23.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] Revert "drm/amdgpu: don't read registers if gfxoff is enabled (v2)"

[Alex Deucher]

Ping?

On Thu, Nov 14, 2019 at 11:41 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> This reverts commit 5e49d6f654c569c2de920babbaf5cf7c4c4a353f.
>
> Drop this workaround in favor of a better one.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/nv.c    | 27 ++++++++++----------------
>  drivers/gpu/drm/amd/amdgpu/soc15.c | 31 ++++++++++++------------------
>  2 files changed, 22 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index 7283d6198b89..af68f9815f28 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -201,25 +201,17 @@ static uint32_t nv_read_indexed_register(struct amdgpu_device *adev, u32 se_num,
>         return val;
>  }
>
> -static int nv_get_register_value(struct amdgpu_device *adev,
> +static uint32_t nv_get_register_value(struct amdgpu_device *adev,
>                                       bool indexed, u32 se_num,
> -                                     u32 sh_num, u32 reg_offset,
> -                                     u32 *value)
> +                                     u32 sh_num, u32 reg_offset)
>  {
>         if (indexed) {
> -               if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> -                       return -EINVAL;
> -               *value = nv_read_indexed_register(adev, se_num, sh_num, reg_offset);
> +               return nv_read_indexed_register(adev, se_num, sh_num, reg_offset);
>         } else {
> -               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);
> -               }
> +               if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG))
> +                       return adev->gfx.config.gb_addr_config;
> +               return RREG32(reg_offset);
>         }
> -       return 0;
>  }
>
>  static int nv_read_register(struct amdgpu_device *adev, u32 se_num,
> @@ -235,9 +227,10 @@ 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;
>
> -               return nv_get_register_value(adev,
> -                                            nv_allowed_read_registers[i].grbm_indexed,
> -                                            se_num, sh_num, reg_offset, value);
> +               *value = nv_get_register_value(adev,
> +                                              nv_allowed_read_registers[i].grbm_indexed,
> +                                              se_num, sh_num, reg_offset);
> +               return 0;
>         }
>         return -EINVAL;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index 3b55655f79c4..8e1640bc07af 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -363,27 +363,19 @@ static uint32_t soc15_read_indexed_register(struct amdgpu_device *adev, u32 se_n
>         return val;
>  }
>
> -static int soc15_get_register_value(struct amdgpu_device *adev,
> +static uint32_t soc15_get_register_value(struct amdgpu_device *adev,
>                                          bool indexed, u32 se_num,
> -                                        u32 sh_num, u32 reg_offset,
> -                                        u32 *value)
> +                                        u32 sh_num, u32 reg_offset)
>  {
>         if (indexed) {
> -               if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> -                       return -EINVAL;
> -               *value = soc15_read_indexed_register(adev, se_num, sh_num, reg_offset);
> +               return soc15_read_indexed_register(adev, se_num, sh_num, reg_offset);
>         } else {
> -               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);
> -               }
> +               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);
>         }
> -       return 0;
>  }
>
>  static int soc15_read_register(struct amdgpu_device *adev, u32 se_num,
> @@ -399,9 +391,10 @@ static int soc15_read_register(struct amdgpu_device *adev, u32 se_num,
>                                         + en->reg_offset))
>                         continue;
>
> -               return soc15_get_register_value(adev,
> -                                               soc15_allowed_read_registers[i].grbm_indexed,
> -                                               se_num, sh_num, reg_offset, value);
> +               *value = soc15_get_register_value(adev,
> +                                                 soc15_allowed_read_registers[i].grbm_indexed,
> +                                                 se_num, sh_num, reg_offset);
> +               return 0;
>         }
>         return -EINVAL;
>  }
> --
> 2.23.0
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] Revert "drm/amdgpu: don't read registers if gfxoff is enabled (v2)"

[Alex Deucher]

Ping?

On Thu, Nov 14, 2019 at 11:41 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> This reverts commit 5e49d6f654c569c2de920babbaf5cf7c4c4a353f.
>
> Drop this workaround in favor of a better one.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/nv.c    | 27 ++++++++++----------------
>  drivers/gpu/drm/amd/amdgpu/soc15.c | 31 ++++++++++++------------------
>  2 files changed, 22 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index 7283d6198b89..af68f9815f28 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -201,25 +201,17 @@ static uint32_t nv_read_indexed_register(struct amdgpu_device *adev, u32 se_num,
>         return val;
>  }
>
> -static int nv_get_register_value(struct amdgpu_device *adev,
> +static uint32_t nv_get_register_value(struct amdgpu_device *adev,
>                                       bool indexed, u32 se_num,
> -                                     u32 sh_num, u32 reg_offset,
> -                                     u32 *value)
> +                                     u32 sh_num, u32 reg_offset)
>  {
>         if (indexed) {
> -               if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> -                       return -EINVAL;
> -               *value = nv_read_indexed_register(adev, se_num, sh_num, reg_offset);
> +               return nv_read_indexed_register(adev, se_num, sh_num, reg_offset);
>         } else {
> -               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);
> -               }
> +               if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG))
> +                       return adev->gfx.config.gb_addr_config;
> +               return RREG32(reg_offset);
>         }
> -       return 0;
>  }
>
>  static int nv_read_register(struct amdgpu_device *adev, u32 se_num,
> @@ -235,9 +227,10 @@ 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;
>
> -               return nv_get_register_value(adev,
> -                                            nv_allowed_read_registers[i].grbm_indexed,
> -                                            se_num, sh_num, reg_offset, value);
> +               *value = nv_get_register_value(adev,
> +                                              nv_allowed_read_registers[i].grbm_indexed,
> +                                              se_num, sh_num, reg_offset);
> +               return 0;
>         }
>         return -EINVAL;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index 3b55655f79c4..8e1640bc07af 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -363,27 +363,19 @@ static uint32_t soc15_read_indexed_register(struct amdgpu_device *adev, u32 se_n
>         return val;
>  }
>
> -static int soc15_get_register_value(struct amdgpu_device *adev,
> +static uint32_t soc15_get_register_value(struct amdgpu_device *adev,
>                                          bool indexed, u32 se_num,
> -                                        u32 sh_num, u32 reg_offset,
> -                                        u32 *value)
> +                                        u32 sh_num, u32 reg_offset)
>  {
>         if (indexed) {
> -               if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> -                       return -EINVAL;
> -               *value = soc15_read_indexed_register(adev, se_num, sh_num, reg_offset);
> +               return soc15_read_indexed_register(adev, se_num, sh_num, reg_offset);
>         } else {
> -               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);
> -               }
> +               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);
>         }
> -       return 0;
>  }
>
>  static int soc15_read_register(struct amdgpu_device *adev, u32 se_num,
> @@ -399,9 +391,10 @@ static int soc15_read_register(struct amdgpu_device *adev, u32 se_num,
>                                         + en->reg_offset))
>                         continue;
>
> -               return soc15_get_register_value(adev,
> -                                               soc15_allowed_read_registers[i].grbm_indexed,
> -                                               se_num, sh_num, reg_offset, value);
> +               *value = soc15_get_register_value(adev,
> +                                                 soc15_allowed_read_registers[i].grbm_indexed,
> +                                                 se_num, sh_num, reg_offset);
> +               return 0;
>         }
>         return -EINVAL;
>  }
> --
> 2.23.0
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 1/2] Revert "drm/amdgpu: don't read registers if gfxoff is enabled (v2)"

[Quan, Evan]

Reviewed-by: Evan Quan <evan.quan@amd.com>

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> Deucher
> Sent: Saturday, November 16, 2019 12:15 AM
> To: amd-gfx list <amd-gfx@lists.freedesktop.org>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH 1/2] Revert "drm/amdgpu: don't read registers if gfxoff is
> enabled (v2)"
> 
> Ping?
> 
> On Thu, Nov 14, 2019 at 11:41 AM Alex Deucher <alexdeucher@gmail.com>
> wrote:
> >
> > This reverts commit 5e49d6f654c569c2de920babbaf5cf7c4c4a353f.
> >
> > Drop this workaround in favor of a better one.
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/nv.c    | 27 ++++++++++----------------
> >  drivers/gpu/drm/amd/amdgpu/soc15.c | 31
> > ++++++++++++------------------
> >  2 files changed, 22 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c
> > b/drivers/gpu/drm/amd/amdgpu/nv.c index 7283d6198b89..af68f9815f28
> > 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> > @@ -201,25 +201,17 @@ static uint32_t nv_read_indexed_register(struct
> amdgpu_device *adev, u32 se_num,
> >         return val;
> >  }
> >
> > -static int nv_get_register_value(struct amdgpu_device *adev,
> > +static uint32_t nv_get_register_value(struct amdgpu_device *adev,
> >                                       bool indexed, u32 se_num,
> > -                                     u32 sh_num, u32 reg_offset,
> > -                                     u32 *value)
> > +                                     u32 sh_num, u32 reg_offset)
> >  {
> >         if (indexed) {
> > -               if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> > -                       return -EINVAL;
> > -               *value = nv_read_indexed_register(adev, se_num, sh_num,
> reg_offset);
> > +               return nv_read_indexed_register(adev, se_num, sh_num,
> > + reg_offset);
> >         } else {
> > -               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);
> > -               }
> > +               if (reg_offset == SOC15_REG_OFFSET(GC, 0,
> mmGB_ADDR_CONFIG))
> > +                       return adev->gfx.config.gb_addr_config;
> > +               return RREG32(reg_offset);
> >         }
> > -       return 0;
> >  }
> >
> >  static int nv_read_register(struct amdgpu_device *adev, u32 se_num,
> > @@ -235,9 +227,10 @@ 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;
> >
> > -               return nv_get_register_value(adev,
> > -                                            nv_allowed_read_registers[i].grbm_indexed,
> > -                                            se_num, sh_num, reg_offset, value);
> > +               *value = nv_get_register_value(adev,
> > +                                              nv_allowed_read_registers[i].grbm_indexed,
> > +                                              se_num, sh_num, reg_offset);
> > +               return 0;
> >         }
> >         return -EINVAL;
> >  }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c
> > b/drivers/gpu/drm/amd/amdgpu/soc15.c
> > index 3b55655f79c4..8e1640bc07af 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> > @@ -363,27 +363,19 @@ static uint32_t soc15_read_indexed_register(struct
> amdgpu_device *adev, u32 se_n
> >         return val;
> >  }
> >
> > -static int soc15_get_register_value(struct amdgpu_device *adev,
> > +static uint32_t soc15_get_register_value(struct amdgpu_device *adev,
> >                                          bool indexed, u32 se_num,
> > -                                        u32 sh_num, u32 reg_offset,
> > -                                        u32 *value)
> > +                                        u32 sh_num, u32 reg_offset)
> >  {
> >         if (indexed) {
> > -               if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> > -                       return -EINVAL;
> > -               *value = soc15_read_indexed_register(adev, se_num, sh_num,
> reg_offset);
> > +               return soc15_read_indexed_register(adev, se_num,
> > + sh_num, reg_offset);
> >         } else {
> > -               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);
> > -               }
> > +               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);
> >         }
> > -       return 0;
> >  }
> >
> >  static int soc15_read_register(struct amdgpu_device *adev, u32
> > se_num, @@ -399,9 +391,10 @@ static int soc15_read_register(struct
> amdgpu_device *adev, u32 se_num,
> >                                         + en->reg_offset))
> >                         continue;
> >
> > -               return soc15_get_register_value(adev,
> > -                                               soc15_allowed_read_registers[i].grbm_indexed,
> > -                                               se_num, sh_num, reg_offset, value);
> > +               *value = soc15_get_register_value(adev,
> > +
> soc15_allowed_read_registers[i].grbm_indexed,
> > +                                                 se_num, sh_num, reg_offset);
> > +               return 0;
> >         }
> >         return -EINVAL;
> >  }
> > --
> > 2.23.0
> >
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 1/2] Revert "drm/amdgpu: don't read registers if gfxoff is enabled (v2)"

[Quan, Evan]

Reviewed-by: Evan Quan <evan.quan@amd.com>

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> Deucher
> Sent: Saturday, November 16, 2019 12:15 AM
> To: amd-gfx list <amd-gfx@lists.freedesktop.org>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH 1/2] Revert "drm/amdgpu: don't read registers if gfxoff is
> enabled (v2)"
> 
> Ping?
> 
> On Thu, Nov 14, 2019 at 11:41 AM Alex Deucher <alexdeucher@gmail.com>
> wrote:
> >
> > This reverts commit 5e49d6f654c569c2de920babbaf5cf7c4c4a353f.
> >
> > Drop this workaround in favor of a better one.
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/nv.c    | 27 ++++++++++----------------
> >  drivers/gpu/drm/amd/amdgpu/soc15.c | 31
> > ++++++++++++------------------
> >  2 files changed, 22 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c
> > b/drivers/gpu/drm/amd/amdgpu/nv.c index 7283d6198b89..af68f9815f28
> > 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> > @@ -201,25 +201,17 @@ static uint32_t nv_read_indexed_register(struct
> amdgpu_device *adev, u32 se_num,
> >         return val;
> >  }
> >
> > -static int nv_get_register_value(struct amdgpu_device *adev,
> > +static uint32_t nv_get_register_value(struct amdgpu_device *adev,
> >                                       bool indexed, u32 se_num,
> > -                                     u32 sh_num, u32 reg_offset,
> > -                                     u32 *value)
> > +                                     u32 sh_num, u32 reg_offset)
> >  {
> >         if (indexed) {
> > -               if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> > -                       return -EINVAL;
> > -               *value = nv_read_indexed_register(adev, se_num, sh_num,
> reg_offset);
> > +               return nv_read_indexed_register(adev, se_num, sh_num,
> > + reg_offset);
> >         } else {
> > -               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);
> > -               }
> > +               if (reg_offset == SOC15_REG_OFFSET(GC, 0,
> mmGB_ADDR_CONFIG))
> > +                       return adev->gfx.config.gb_addr_config;
> > +               return RREG32(reg_offset);
> >         }
> > -       return 0;
> >  }
> >
> >  static int nv_read_register(struct amdgpu_device *adev, u32 se_num,
> > @@ -235,9 +227,10 @@ 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;
> >
> > -               return nv_get_register_value(adev,
> > -                                            nv_allowed_read_registers[i].grbm_indexed,
> > -                                            se_num, sh_num, reg_offset, value);
> > +               *value = nv_get_register_value(adev,
> > +                                              nv_allowed_read_registers[i].grbm_indexed,
> > +                                              se_num, sh_num, reg_offset);
> > +               return 0;
> >         }
> >         return -EINVAL;
> >  }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c
> > b/drivers/gpu/drm/amd/amdgpu/soc15.c
> > index 3b55655f79c4..8e1640bc07af 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> > @@ -363,27 +363,19 @@ static uint32_t soc15_read_indexed_register(struct
> amdgpu_device *adev, u32 se_n
> >         return val;
> >  }
> >
> > -static int soc15_get_register_value(struct amdgpu_device *adev,
> > +static uint32_t soc15_get_register_value(struct amdgpu_device *adev,
> >                                          bool indexed, u32 se_num,
> > -                                        u32 sh_num, u32 reg_offset,
> > -                                        u32 *value)
> > +                                        u32 sh_num, u32 reg_offset)
> >  {
> >         if (indexed) {
> > -               if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> > -                       return -EINVAL;
> > -               *value = soc15_read_indexed_register(adev, se_num, sh_num,
> reg_offset);
> > +               return soc15_read_indexed_register(adev, se_num,
> > + sh_num, reg_offset);
> >         } else {
> > -               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);
> > -               }
> > +               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);
> >         }
> > -       return 0;
> >  }
> >
> >  static int soc15_read_register(struct amdgpu_device *adev, u32
> > se_num, @@ -399,9 +391,10 @@ static int soc15_read_register(struct
> amdgpu_device *adev, u32 se_num,
> >                                         + en->reg_offset))
> >                         continue;
> >
> > -               return soc15_get_register_value(adev,
> > -                                               soc15_allowed_read_registers[i].grbm_indexed,
> > -                                               se_num, sh_num, reg_offset, value);
> > +               *value = soc15_get_register_value(adev,
> > +
> soc15_allowed_read_registers[i].grbm_indexed,
> > +                                                 se_num, sh_num, reg_offset);
> > +               return 0;
> >         }
> >         return -EINVAL;
> >  }
> > --
> > 2.23.0
> >
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx