[PATCH 07/18] drm/amdgpu/vcn: separate gating state by instance
Boyuan Zhang
Boyuan.Zhang at amd.com
Fri Oct 4 18:42:43 UTC 2024
On 2024-10-02 07:28, Christian König wrote:
> Am 02.10.24 um 06:36 schrieb boyuan.zhang at amd.com:
>> From: Boyuan Zhang <boyuan.zhang at amd.com>
>>
>> vcn gating state should now be based on instance. For example,
>> instance 0
>> can be gated while instance 1 is ungated, or vice versa.
>>
>> Therefore, change the cur_state to be an array, so that it can track the
>> gating status for each vcn instance now.
>>
>> Signed-off-by: Boyuan Zhang <boyuan.zhang at amd.com>
>
> Mhm, some questions. See below.
>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 2 +-
>> drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 6 ++--
>> drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 8 ++---
>> drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 23 +++++++------
>> drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 38 +++++++++++----------
>> drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 41 ++++++++++++-----------
>> drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c | 41 +++++++++++++----------
>> drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c | 23 +++++++------
>> drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c | 23 +++++++------
>> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 3 +-
>> 10 files changed, 114 insertions(+), 94 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> index 2a1f3dbb14d3..f6717f780acd 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> @@ -308,7 +308,7 @@ struct amdgpu_vcn {
>> struct delayed_work idle_work;
>> const struct firmware *fw[AMDGPU_MAX_VCN_INSTANCES]; /* VCN
>> firmware */
>> unsigned num_enc_rings;
>> - enum amd_powergating_state cur_state;
>> + enum amd_powergating_state cur_state[AMDGPU_MAX_VCN_INSTANCES];
>> bool indirect_sram;
>> uint8_t num_vcn_inst;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> index 6717ff692d8d..bafdd6d5ff24 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> @@ -279,7 +279,7 @@ static int vcn_v1_0_hw_fini(void *handle)
>> cancel_delayed_work_sync(&adev->vcn.idle_work);
>> if ((adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) ||
>> - (adev->vcn.cur_state != AMD_PG_STATE_GATE &&
>> + (adev->vcn.cur_state[0] != AMD_PG_STATE_GATE &&
>> RREG32_SOC15(VCN, 0, mmUVD_STATUS))) {
>> vcn_v1_0_set_powergating_state(adev, AMD_PG_STATE_GATE);
>> }
>> @@ -1813,7 +1813,7 @@ static int vcn_v1_0_set_powergating_state(void
>> *handle,
>> int ret;
>> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> - if (state == adev->vcn.cur_state)
>> + if (state == adev->vcn.cur_state[0])
>> return 0;
>> if (state == AMD_PG_STATE_GATE)
>> @@ -1822,7 +1822,7 @@ static int vcn_v1_0_set_powergating_state(void
>> *handle,
>> ret = vcn_v1_0_start(adev);
>> if (!ret)
>> - adev->vcn.cur_state = state;
>> + adev->vcn.cur_state[0] = state;
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
>> index be6998ed56bc..72e36fbaad39 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
>> @@ -316,7 +316,7 @@ static int vcn_v2_0_hw_fini(void *handle)
>> cancel_delayed_work_sync(&adev->vcn.idle_work);
>> if ((adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) ||
>> - (adev->vcn.cur_state != AMD_PG_STATE_GATE &&
>> + (adev->vcn.cur_state[0] != AMD_PG_STATE_GATE &&
>> RREG32_SOC15(VCN, 0, mmUVD_STATUS)))
>> vcn_v2_0_set_powergating_state(adev, AMD_PG_STATE_GATE);
>> @@ -1812,11 +1812,11 @@ static int
>> vcn_v2_0_set_powergating_state(void *handle,
>> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> if (amdgpu_sriov_vf(adev)) {
>> - adev->vcn.cur_state = AMD_PG_STATE_UNGATE;
>> + adev->vcn.cur_state[0] = AMD_PG_STATE_UNGATE;
>> return 0;
>> }
>> - if (state == adev->vcn.cur_state)
>> + if (state == adev->vcn.cur_state[0])
>> return 0;
>> if (state == AMD_PG_STATE_GATE)
>> @@ -1825,7 +1825,7 @@ static int vcn_v2_0_set_powergating_state(void
>> *handle,
>> ret = vcn_v2_0_start(adev);
>> if (!ret)
>> - adev->vcn.cur_state = state;
>> + adev->vcn.cur_state[0] = state;
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>> index 7ab0314b27ec..09b342ad02cd 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>> @@ -397,9 +397,10 @@ static int vcn_v2_5_hw_fini(void *handle)
>> continue;
>> if ((adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) ||
>> - (adev->vcn.cur_state != AMD_PG_STATE_GATE &&
>> - RREG32_SOC15(VCN, i, mmUVD_STATUS)))
>> + (adev->vcn.cur_state[i] != AMD_PG_STATE_GATE &&
>> + RREG32_SOC15(VCN, i, mmUVD_STATUS))) {
>> vcn_v2_5_set_powergating_state(adev, AMD_PG_STATE_GATE);
>> + }
>> if (amdgpu_ras_is_supported(adev, AMDGPU_RAS_BLOCK__VCN))
>> amdgpu_irq_put(adev, &adev->vcn.inst[i].ras_poison_irq,
>> 0);
>> @@ -1832,16 +1833,18 @@ static int
>> vcn_v2_5_set_powergating_state(void *handle,
>> if (amdgpu_sriov_vf(adev))
>> return 0;
>> - if(state == adev->vcn.cur_state)
>> - return 0;
>> + for (int i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>> + if (state == adev->vcn.cur_state[i])
>> + return 0;
>
> Shouldn't this only check the first instance for the HW generations
> which work with only one instance?
>
> Could be that this becomes redundant when the function is called per
> instance later on.
>
> Regards,
> Christian.
Yes, there are some redundant codes, removed all of them in v2.
For vcn generations with only one instance (v1.0, v2.0) always use 0 as
instance number. For vcn generations with multiple instances (v2.5,
v3.0, v4.0, v4.0.3, v4.0.5, v5.0.0) use the actual instance number.
Please check the updated v2 patch set.
Regards,
Boyuan
>
>> - if (state == AMD_PG_STATE_GATE)
>> - ret = vcn_v2_5_stop(adev);
>> - else
>> - ret = vcn_v2_5_start(adev);
>> + if (state == AMD_PG_STATE_GATE)
>> + ret = vcn_v2_5_stop(adev);
>> + else
>> + ret = vcn_v2_5_start(adev);
>> - if(!ret)
>> - adev->vcn.cur_state = state;
>> + if (!ret)
>> + adev->vcn.cur_state[i] = state;
>> + }
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>> index 38452446fcb0..ee9a40443d65 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>> @@ -430,8 +430,8 @@ static int vcn_v3_0_hw_fini(void *handle)
>> if (!amdgpu_sriov_vf(adev)) {
>> if ((adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) ||
>> - (adev->vcn.cur_state != AMD_PG_STATE_GATE &&
>> - RREG32_SOC15(VCN, i, mmUVD_STATUS))) {
>> + (adev->vcn.cur_state[i] != AMD_PG_STATE_GATE &&
>> + RREG32_SOC15(VCN, i, mmUVD_STATUS))) {
>> vcn_v3_0_set_powergating_state(adev,
>> AMD_PG_STATE_GATE);
>> }
>> }
>> @@ -2163,25 +2163,27 @@ static int
>> vcn_v3_0_set_powergating_state(void *handle,
>> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> int ret;
>> - /* for SRIOV, guest should not control VCN Power-gating
>> - * MMSCH FW should control Power-gating and clock-gating
>> - * guest should avoid touching CGC and PG
>> - */
>> - if (amdgpu_sriov_vf(adev)) {
>> - adev->vcn.cur_state = AMD_PG_STATE_UNGATE;
>> - return 0;
>> - }
>> + for (int i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>> + /* for SRIOV, guest should not control VCN Power-gating
>> + * MMSCH FW should control Power-gating and clock-gating
>> + * guest should avoid touching CGC and PG
>> + */
>> + if (amdgpu_sriov_vf(adev)) {
>> + adev->vcn.cur_state[i] = AMD_PG_STATE_UNGATE;
>> + return 0;
>> + }
>> - if (state == adev->vcn.cur_state)
>> - return 0;
>> + if (state == adev->vcn.cur_state[i])
>> + return 0;
>> - if (state == AMD_PG_STATE_GATE)
>> - ret = vcn_v3_0_stop(adev);
>> - else
>> - ret = vcn_v3_0_start(adev);
>> + if (state == AMD_PG_STATE_GATE)
>> + ret = vcn_v3_0_stop(adev);
>> + else
>> + ret = vcn_v3_0_start(adev);
>> - if (!ret)
>> - adev->vcn.cur_state = state;
>> + if (!ret)
>> + adev->vcn.cur_state[i] = state;
>> + }
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>> index 078bb30497e0..aaecc680b631 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>> @@ -355,11 +355,12 @@ static int vcn_v4_0_hw_fini(void *handle)
>> for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>> if (adev->vcn.harvest_config & (1 << i))
>> continue;
>> +
>> if (!amdgpu_sriov_vf(adev)) {
>> if ((adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) ||
>> - (adev->vcn.cur_state != AMD_PG_STATE_GATE &&
>> - RREG32_SOC15(VCN, i, regUVD_STATUS))) {
>> - vcn_v4_0_set_powergating_state(adev,
>> AMD_PG_STATE_GATE);
>> + (adev->vcn.cur_state[i] != AMD_PG_STATE_GATE &&
>> + RREG32_SOC15(VCN, i, regUVD_STATUS))) {
>> + vcn_v4_0_set_powergating_state(adev,
>> AMD_PG_STATE_GATE);
>> }
>> }
>> if (amdgpu_ras_is_supported(adev, AMDGPU_RAS_BLOCK__VCN))
>> @@ -2040,25 +2041,27 @@ static int
>> vcn_v4_0_set_powergating_state(void *handle, enum amd_powergating_sta
>> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> int ret;
>> - /* for SRIOV, guest should not control VCN Power-gating
>> - * MMSCH FW should control Power-gating and clock-gating
>> - * guest should avoid touching CGC and PG
>> - */
>> - if (amdgpu_sriov_vf(adev)) {
>> - adev->vcn.cur_state = AMD_PG_STATE_UNGATE;
>> - return 0;
>> - }
>> + for (int i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>> + /* for SRIOV, guest should not control VCN Power-gating
>> + * MMSCH FW should control Power-gating and clock-gating
>> + * guest should avoid touching CGC and PG
>> + */
>> + if (amdgpu_sriov_vf(adev)) {
>> + adev->vcn.cur_state[i] = AMD_PG_STATE_UNGATE;
>> + return 0;
>> + }
>> - if (state == adev->vcn.cur_state)
>> - return 0;
>> + if (state == adev->vcn.cur_state[i])
>> + return 0;
>> - if (state == AMD_PG_STATE_GATE)
>> - ret = vcn_v4_0_stop(adev);
>> - else
>> - ret = vcn_v4_0_start(adev);
>> + if (state == AMD_PG_STATE_GATE)
>> + ret = vcn_v4_0_stop(adev);
>> + else
>> + ret = vcn_v4_0_start(adev);
>> - if (!ret)
>> - adev->vcn.cur_state = state;
>> + if (!ret)
>> + adev->vcn.cur_state[i] = state;
>> + }
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
>> index be7b0bfba27a..87c04c512357 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
>> @@ -318,8 +318,11 @@ static int vcn_v4_0_3_hw_fini(void *handle)
>> cancel_delayed_work_sync(&adev->vcn.idle_work);
>> - if (adev->vcn.cur_state != AMD_PG_STATE_GATE)
>> - vcn_v4_0_3_set_powergating_state(adev, AMD_PG_STATE_GATE);
>> + for (int i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>> + if (adev->vcn.cur_state[i] != AMD_PG_STATE_GATE) {
>> + vcn_v4_0_3_set_powergating_state(adev, AMD_PG_STATE_GATE);
>> + }
>> + }
>> return 0;
>> }
>> @@ -1627,25 +1630,27 @@ static int
>> vcn_v4_0_3_set_powergating_state(void *handle,
>> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> int ret;
>> - /* for SRIOV, guest should not control VCN Power-gating
>> - * MMSCH FW should control Power-gating and clock-gating
>> - * guest should avoid touching CGC and PG
>> - */
>> - if (amdgpu_sriov_vf(adev)) {
>> - adev->vcn.cur_state = AMD_PG_STATE_UNGATE;
>> - return 0;
>> - }
>> + for (int i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>> + /* for SRIOV, guest should not control VCN Power-gating
>> + * MMSCH FW should control Power-gating and clock-gating
>> + * guest should avoid touching CGC and PG
>> + */
>> + if (amdgpu_sriov_vf(adev)) {
>> + adev->vcn.cur_state[i] = AMD_PG_STATE_UNGATE;
>> + return 0;
>> + }
>> - if (state == adev->vcn.cur_state)
>> - return 0;
>> + if (state == adev->vcn.cur_state[i])
>> + return 0;
>> - if (state == AMD_PG_STATE_GATE)
>> - ret = vcn_v4_0_3_stop(adev);
>> - else
>> - ret = vcn_v4_0_3_start(adev);
>> + if (state == AMD_PG_STATE_GATE)
>> + ret = vcn_v4_0_3_stop(adev);
>> + else
>> + ret = vcn_v4_0_3_start(adev);
>> - if (!ret)
>> - adev->vcn.cur_state = state;
>> + if (!ret)
>> + adev->vcn.cur_state[i] = state;
>> + }
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
>> index 0e70df04ceb9..2092360e133a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
>> @@ -305,10 +305,11 @@ static int vcn_v4_0_5_hw_fini(void *handle)
>> for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>> if (adev->vcn.harvest_config & (1 << i))
>> continue;
>> +
>> if (!amdgpu_sriov_vf(adev)) {
>> if ((adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) ||
>> - (adev->vcn.cur_state != AMD_PG_STATE_GATE &&
>> - RREG32_SOC15(VCN, i, regUVD_STATUS))) {
>> + (adev->vcn.cur_state[i] != AMD_PG_STATE_GATE &&
>> + RREG32_SOC15(VCN, i, regUVD_STATUS))) {
>> vcn_v4_0_5_set_powergating_state(adev,
>> AMD_PG_STATE_GATE);
>> }
>> }
>> @@ -1534,16 +1535,18 @@ static int
>> vcn_v4_0_5_set_powergating_state(void *handle, enum amd_powergating_s
>> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> int ret;
>> - if (state == adev->vcn.cur_state)
>> - return 0;
>> + for (int i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>> + if (state == adev->vcn.cur_state[i])
>> + return 0;
>> - if (state == AMD_PG_STATE_GATE)
>> - ret = vcn_v4_0_5_stop(adev);
>> - else
>> - ret = vcn_v4_0_5_start(adev);
>> + if (state == AMD_PG_STATE_GATE)
>> + ret = vcn_v4_0_5_stop(adev);
>> + else
>> + ret = vcn_v4_0_5_start(adev);
>> - if (!ret)
>> - adev->vcn.cur_state = state;
>> + if (!ret)
>> + adev->vcn.cur_state[i] = state;
>> + }
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
>> index 6b38927024d1..045ac9c8d8be 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
>> @@ -269,10 +269,11 @@ static int vcn_v5_0_0_hw_fini(void *handle)
>> for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>> if (adev->vcn.harvest_config & (1 << i))
>> continue;
>> +
>> if (!amdgpu_sriov_vf(adev)) {
>> if ((adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) ||
>> - (adev->vcn.cur_state != AMD_PG_STATE_GATE &&
>> - RREG32_SOC15(VCN, i, regUVD_STATUS))) {
>> + (adev->vcn.cur_state[i] != AMD_PG_STATE_GATE &&
>> + RREG32_SOC15(VCN, i, regUVD_STATUS))) {
>> vcn_v5_0_0_set_powergating_state(adev,
>> AMD_PG_STATE_GATE);
>> }
>> }
>> @@ -1261,16 +1262,18 @@ static int
>> vcn_v5_0_0_set_powergating_state(void *handle, enum amd_powergating_s
>> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> int ret;
>> - if (state == adev->vcn.cur_state)
>> - return 0;
>> + for (int i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>> + if (state == adev->vcn.cur_state[i])
>> + return 0;
>> - if (state == AMD_PG_STATE_GATE)
>> - ret = vcn_v5_0_0_stop(adev);
>> - else
>> - ret = vcn_v5_0_0_start(adev);
>> + if (state == AMD_PG_STATE_GATE)
>> + ret = vcn_v5_0_0_stop(adev);
>> + else
>> + ret = vcn_v5_0_0_start(adev);
>> - if (!ret)
>> - adev->vcn.cur_state = state;
>> + if (!ret)
>> + adev->vcn.cur_state[i] = state;
>> + }
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> index 449925105889..86001682e13e 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> @@ -2112,7 +2112,8 @@ static int smu_hw_fini(void *handle)
>> smu_dpm_set_vpe_enable(smu, false);
>> smu_dpm_set_umsch_mm_enable(smu, false);
>> - adev->vcn.cur_state = AMD_PG_STATE_GATE;
>> + for (int i = 0; i < adev->vcn.num_vcn_inst; ++i)
>> + adev->vcn.cur_state[i] = AMD_PG_STATE_GATE;
>> adev->jpeg.cur_state = AMD_PG_STATE_GATE;
>> if (!smu->pm_enabled)
>
More information about the amd-gfx
mailing list