[PATCH 07/18] drm/amdgpu/vcn: separate gating state by instance
Christian König
ckoenig.leichtzumerken at gmail.com
Wed Oct 2 11:28:38 UTC 2024
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.
>
> - 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