[PATCH] drm/amdgpu: add safeguards for accessing mmhub CG registers

[Christian König]

Am 26.01.22 um 12:02 schrieb Lang Yu:
> On 01/26/ , Christian König wrote:
>> Am 26.01.22 um 08:53 schrieb Lang Yu:
>>> We observed a gpu hang when querying mmhub CG status(i.e.,
>>> cat amdgpu_pm_info) on cyan skillfish. Acctually, cyan
>>> skillfish doesn't support any CG features.
>>>
>>> Only allow asics which support CG features accessing related
>>> registers. Will add similar safeguards for other IPs in the
>>> furture.
>> I think you should probably add a macro or function for this check, apart
>> from that looks good to me.
> Thanks for you advice. Is it fine to use such a macro?
>
> #define amdgpu_device_cg_flag_isset(flag) ((adev->cg_flags) & (flag))

No, first of all that can also be a function and doesn't need to be a macro.

Then we should probably encapsulate more functionality or otherwise it 
is rather usless.

Think more about a function like amdgpu_gmc_cg_enabled() instead.

Regards,
Christian.

>
> Regards,
> Lang
>
>> Christian.
>>
>>> Signed-off-by: Lang Yu <Lang.Yu at amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 3 +++
>>>    drivers/gpu/drm/amd/amdgpu/mmhub_v1_7.c | 3 +++
>>>    drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c | 3 +++
>>>    drivers/gpu/drm/amd/amdgpu/mmhub_v2_3.c | 3 +++
>>>    drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c | 3 +++
>>>    5 files changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
>>> index 4c9f0c0f3116..1869e2019461 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
>>> @@ -550,6 +550,9 @@ static void mmhub_v1_0_get_clockgating(struct amdgpu_device *adev, u32 *flags)
>>>    {
>>>    	int data, data1;
>>> +	if (!(adev->cg_flags & (AMD_CG_SUPPORT_MC_MGCG | AMD_CG_SUPPORT_MC_LS)))
>>> +		return;
>>> +
>>>    	if (amdgpu_sriov_vf(adev))
>>>    		*flags = 0;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_7.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_7.c
>>> index 3b901f941627..f7b9843b36e6 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_7.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_7.c
>>> @@ -546,6 +546,9 @@ static void mmhub_v1_7_get_clockgating(struct amdgpu_device *adev, u32 *flags)
>>>    {
>>>    	int data, data1;
>>> +	if (!(adev->cg_flags & (AMD_CG_SUPPORT_MC_MGCG | AMD_CG_SUPPORT_MC_LS)))
>>> +		return;
>>> +
>>>    	if (amdgpu_sriov_vf(adev))
>>>    		*flags = 0;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
>>> index 3718ff610ab2..3f5f326379b7 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
>>> @@ -686,6 +686,9 @@ static void mmhub_v2_0_get_clockgating(struct amdgpu_device *adev, u32 *flags)
>>>    {
>>>    	int data, data1;
>>> +	if (!(adev->cg_flags & (AMD_CG_SUPPORT_MC_MGCG | AMD_CG_SUPPORT_MC_LS)))
>>> +		return;
>>> +
>>>    	if (amdgpu_sriov_vf(adev))
>>>    		*flags = 0;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_3.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_3.c
>>> index 9e16da28505a..b23dd9ddfb5c 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_3.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_3.c
>>> @@ -580,6 +580,9 @@ static void mmhub_v2_3_get_clockgating(struct amdgpu_device *adev, u32 *flags)
>>>    {
>>>    	int data, data1, data2, data3;
>>> +	if (!(adev->cg_flags & (AMD_CG_SUPPORT_MC_MGCG | AMD_CG_SUPPORT_MC_LS)))
>>> +		return;
>>> +
>>>    	if (amdgpu_sriov_vf(adev))
>>>    		*flags = 0;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c
>>> index 619106f7d23d..a2d5c8424e2b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c
>>> @@ -651,6 +651,9 @@ static void mmhub_v9_4_get_clockgating(struct amdgpu_device *adev, u32 *flags)
>>>    {
>>>    	int data, data1;
>>> +	if (!(adev->cg_flags & (AMD_CG_SUPPORT_MC_MGCG | AMD_CG_SUPPORT_MC_LS)))
>>> +		return;
>>> +
>>>    	if (amdgpu_sriov_vf(adev))
>>>    		*flags = 0;