Thanks for the catch Philip. I must have missed this with the renoir warnings.
I sent fix.
Jon
-----Original Message-----
From: Yang, Philip <Philip.Yang@xxxxxxx>
Sent: Thursday, October 17, 2019 1:40 PM
To: Quan, Evan <Evan.Quan@xxxxxxx>; Kim, Jonathan <Jonathan.Kim@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Kuehling, Felix <Felix.Kuehling@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: disable c-states on xgmi perfmons
I got compiler warnings after update this morning, because the variables are not initialized in df_v3_6_set_df_cstate() return failed path.
CC [M] drivers/gpu/drm/amd/amdgpu/gmc_v9_0.o
CC [M] drivers/gpu/drm/amd/amdgpu/gfxhub_v1_1.o
/home/yangp/git/compute_staging/kernel/drivers/gpu/drm/amd/amdgpu/df_v3_6.c:96:8:
warning: return type defaults to ‘int’ [-Wreturn-type]
static df_v3_6_set_df_cstate(struct amdgpu_device *adev, int allow)
^~~~~~~~~~~~~~~~~~~~~
/home/yangp/git/compute_staging/kernel/drivers/gpu/drm/amd/amdgpu/df_v3_6.c:
In function ‘df_v3_6_pmc_get_count’:
/home/yangp/git/compute_staging/kernel/drivers/gpu/drm/amd/amdgpu/df_v3_6.c:564:22:
warning: ‘hi_val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
*count = ((hi_val | 0ULL) << 32) | (lo_val | 0ULL);
~~~~~~~~^~~~~~~
/home/yangp/git/compute_staging/kernel/drivers/gpu/drm/amd/amdgpu/df_v3_6.c:564:47:
warning: ‘lo_val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
*count = ((hi_val | 0ULL) << 32) | (lo_val | 0ULL);
~~~~~~~~^~~~~~~
CC [M] drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.o
Regards,
Philip
On 2019-10-16 10:54 p.m., Quan, Evan wrote:
> Reviewed-by: Evan Quan <evan.quan@xxxxxxx>
>
>> -----Original Message-----
>> From: Kim, Jonathan <Jonathan.Kim@xxxxxxx>
>> Sent: 2019年10月17日 10:06
>> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
>> Cc: Kuehling, Felix <Felix.Kuehling@xxxxxxx>; Quan, Evan
>> <Evan.Quan@xxxxxxx>; Kim, Jonathan <Jonathan.Kim@xxxxxxx>; Kim,
>> Jonathan <Jonathan.Kim@xxxxxxx>
>> Subject: [PATCH] drm/amdgpu: disable c-states on xgmi perfmons
>>
>> read or writes to df registers when gpu df is in c-states will result
>> in hang. df c-states should be disabled prior to read or writes then
>> re-enabled after read or writes.
>>
>> v2: use old powerplay routines for vega20
>>
>> Change-Id: I6d5a83e4fe13e29c73dfb03a94fe7c611e867fec
>> Signed-off-by: Jonathan Kim <Jonathan.Kim@xxxxxxx>
>> ---
>> drivers/gpu/drm/amd/amdgpu/df_v3_6.c | 36
>> +++++++++++++++++++++++++++-
>> 1 file changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
>> b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
>> index 16fbd2bc8ad1..f403c62c944e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
>> @@ -93,6 +93,21 @@ const struct attribute_group
>> *df_v3_6_attr_groups[] = {
>> NULL
>> };
>>
>> +static df_v3_6_set_df_cstate(struct amdgpu_device *adev, int allow)
>> +{
>> + int r = 0;
>> +
>> + if (is_support_sw_smu(adev)) {
>> + r = smu_set_df_cstate(&adev->smu, allow);
>> + } else if (adev->powerplay.pp_funcs
>> + && adev->powerplay.pp_funcs->set_df_cstate) {
>> + r = adev->powerplay.pp_funcs->set_df_cstate(
>> + adev->powerplay.pp_handle, allow);
>> + }
>> +
>> + return r;
>> +}
>> +
>> static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
>> uint32_t ficaa_val)
>> {
>> @@ -102,6 +117,9 @@ static uint64_t df_v3_6_get_fica(struct
>> amdgpu_device *adev,
>> address = adev->nbio.funcs->get_pcie_index_offset(adev);
>> data = adev->nbio.funcs->get_pcie_data_offset(adev);
>>
>> + if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
>> + return 0xFFFFFFFFFFFFFFFF;
>> +
>> spin_lock_irqsave(&adev->pcie_idx_lock, flags);
>> WREG32(address,
>> smnDF_PIE_AON_FabricIndirectConfigAccessAddress3);
>> WREG32(data, ficaa_val);
>> @@ -114,6 +132,8 @@ static uint64_t df_v3_6_get_fica(struct
>> amdgpu_device *adev,
>>
>> spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>>
>> + df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
>> +
>> return (((ficadh_val & 0xFFFFFFFFFFFFFFFF) << 32) | ficadl_val);
>> }
>>
>> @@ -125,6 +145,9 @@ static void df_v3_6_set_fica(struct amdgpu_device
>> *adev, uint32_t ficaa_val,
>> address = adev->nbio.funcs->get_pcie_index_offset(adev);
>> data = adev->nbio.funcs->get_pcie_data_offset(adev);
>>
>> + if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
>> + return;
>> +
>> spin_lock_irqsave(&adev->pcie_idx_lock, flags);
>> WREG32(address,
>> smnDF_PIE_AON_FabricIndirectConfigAccessAddress3);
>> WREG32(data, ficaa_val);
>> @@ -134,8 +157,9 @@ static void df_v3_6_set_fica(struct amdgpu_device
>> *adev, uint32_t ficaa_val,
>>
>> WREG32(address,
>> smnDF_PIE_AON_FabricIndirectConfigAccessDataHi3);
>> WREG32(data, ficadh_val);
>> -
>> spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>> +
>> + df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
>> }
>>
>> /*
>> @@ -153,12 +177,17 @@ static void df_v3_6_perfmon_rreg(struct
>> amdgpu_device *adev,
>> address = adev->nbio.funcs->get_pcie_index_offset(adev);
>> data = adev->nbio.funcs->get_pcie_data_offset(adev);
>>
>> + if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
>> + return;
>> +
>> spin_lock_irqsave(&adev->pcie_idx_lock, flags);
>> WREG32(address, lo_addr);
>> *lo_val = RREG32(data);
>> WREG32(address, hi_addr);
>> *hi_val = RREG32(data);
>> spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>> +
>> + df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
>> }
>>
>> /*
>> @@ -175,12 +204,17 @@ static void df_v3_6_perfmon_wreg(struct
>> amdgpu_device *adev, uint32_t lo_addr,
>> address = adev->nbio.funcs->get_pcie_index_offset(adev);
>> data = adev->nbio.funcs->get_pcie_data_offset(adev);
>>
>> + if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
>> + return;
>> +
>> spin_lock_irqsave(&adev->pcie_idx_lock, flags);
>> WREG32(address, lo_addr);
>> WREG32(data, lo_val);
>> WREG32(address, hi_addr);
>> WREG32(data, hi_val);
>> spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>> +
>> + df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
>> }
>>
>> /* get the number of df counters available */
>> --
>> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
