On 1/28/2022 2:46 PM, Lang Yu wrote:
On 01/28/ , Lazar, Lijo wrote:


On 1/28/2022 2:22 PM, Lang Yu wrote:
On 01/28/ , Lazar, Lijo wrote:


On 1/28/2022 12:24 PM, Lang Yu wrote:
We observed a GPU hang when querying GMC CG state(i.e.,
cat amdgpu_pm_info) on cyan skillfish. Acctually, cyan
skillfish doesn't support any CG features.

Only allow ASICs which support GMC CG features accessing
related registers. As some ASICs support GMC CG but cg_flags
are not set. Use GC IP version instead of cg_flags to
determine whether GMC CG is supported or not.

v2:
    - Use a function to encapsulate more functionality.(Christian)
    - Use IP verion to determine whether CG is supported or not.(Lijo)

Signed-off-by: Lang Yu <lang...@amd.com>
---
    drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 10 ++++++++++
    drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  1 +
    drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  |  3 +++
    drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   |  3 +++
    drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   |  3 +++
    5 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index d426de48d299..be1f03b02af6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -876,3 +876,13 @@ int amdgpu_gmc_vram_checking(struct amdgpu_device *adev)
        return 0;
    }
+
+bool amdgpu_gmc_cg_enabled(struct amdgpu_device *adev)
+{
+       switch (adev->ip_versions[GC_HWIP][0]) {
+       case IP_VERSION(10, 1, 3):
+               return false;
+       default:
+               return true;
+       }
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index 93505bb0a36c..b916e73c7de1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -338,4 +338,5 @@ uint64_t amdgpu_gmc_vram_mc2pa(struct amdgpu_device *adev, 
uint64_t mc_addr);
    uint64_t amdgpu_gmc_vram_pa(struct amdgpu_device *adev, struct amdgpu_bo 
*bo);
    uint64_t amdgpu_gmc_vram_cpu_pa(struct amdgpu_device *adev, struct 
amdgpu_bo *bo);
    int amdgpu_gmc_vram_checking(struct amdgpu_device *adev);
+bool amdgpu_gmc_cg_enabled(struct amdgpu_device *adev);
    #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 73ab0eebe4e2..4e46f618d6c1 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -1156,6 +1156,9 @@ static void gmc_v10_0_get_clockgating_state(void *handle, 
u32 *flags)
    {
        struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+       if (!amdgpu_gmc_cg_enabled(adev))
+               return;
+

I think Christian suggested amdgpu_gmc_cg_enabled function assuming it's a
common logic for all ASICs based on flags. Now that assumption has changed.
Now the logic is a specific IP version doesn't enable CG which is known
beforehand. So we could maintain the check in the specific IP version block
itself (gmc 10 in this example). No need to call another common function
which checks IP version again.

Thanks. You mean just like this?

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 73ab0eebe4e2..bddaf2417344 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -1156,6 +1156,9 @@ static void gmc_v10_0_get_clockgating_state(void *handle, 
u32 *flags)
   {
          struct amdgpu_device *adev = (struct amdgpu_device *)handle;

+       if (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 1, 3))
                *flags = 0;

Yes, add the above line also.

It may clear CG mask of other IP block. Does it make sense? Thanks!

Ah! right. No need to clear the flags.

Thanks,
Lijo

Regards,
Lang

Thanks,
Lijo
+               return;
+
          adev->mmhub.funcs->get_clockgating(adev, flags);

          if (adev->ip_versions[ATHUB_HWIP][0] >= IP_VERSION(2, 1, 0))

Regards,
Lang

Thanks,
Lijo

        adev->mmhub.funcs->get_clockgating(adev, flags);
        if (adev->ip_versions[ATHUB_HWIP][0] >= IP_VERSION(2, 1, 0))
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index ca9841d5669f..ff9dff2a6cf1 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -1695,6 +1695,9 @@ static void gmc_v8_0_get_clockgating_state(void *handle, 
u32 *flags)
        struct amdgpu_device *adev = (struct amdgpu_device *)handle;
        int data;
+       if (!amdgpu_gmc_cg_enabled(adev))
+               return;
+
        if (amdgpu_sriov_vf(adev))
                *flags = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 4595027a8c63..faf017609dfe 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1952,6 +1952,9 @@ static void gmc_v9_0_get_clockgating_state(void *handle, 
u32 *flags)
    {
        struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+       if (!amdgpu_gmc_cg_enabled(adev))
+               return;
+
        adev->mmhub.funcs->get_clockgating(adev, flags);
        athub_v1_0_get_clockgating(adev, flags);

Reply via email to