* [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5
@ 2025-03-04 23:01 Alex Deucher
2025-03-05 14:20 ` Boyuan Zhang
0 siblings, 1 reply; 12+ messages in thread
From: Alex Deucher @ 2025-03-04 23:01 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher
VCN 2.5 uses the PG callback to enable VCN DPM which is
a global state. As such, we need to make sure all instances
are in the same state.
v2: switch to a ref count (Lijo)
v3: switch to its own idle work handler
Fixes: 4ce4fe27205c ("drm/amdgpu/vcn: use per instance callbacks for idle work handler")
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 126 +++++++++++++++++++++++++-
1 file changed, 122 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
index dff1a88590363..fa66521b940de 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -107,6 +107,121 @@ static int amdgpu_ih_clientid_vcns[] = {
SOC15_IH_CLIENTID_VCN1
};
+static void vcn_v2_5_idle_work_handler(struct work_struct *work)
+{
+ struct amdgpu_vcn_inst *vcn_inst =
+ container_of(work, struct amdgpu_vcn_inst, idle_work.work);
+ struct amdgpu_device *adev = vcn_inst->adev;
+ unsigned int fences = 0, fence[AMDGPU_MAX_VCN_INSTANCES] = {0};
+ unsigned int i, j;
+ int r = 0;
+
+ for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
+ struct amdgpu_vcn_inst *v = &adev->vcn.inst[i];
+
+ if (adev->vcn.harvest_config & (1 << i))
+ continue;
+
+ for (j = 0; j < v->num_enc_rings; ++j)
+ fence[i] += amdgpu_fence_count_emitted(&v->ring_enc[j]);
+
+ /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
+ if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
+ !v->using_unified_queue) {
+ struct dpg_pause_state new_state;
+
+ if (fence[i] ||
+ unlikely(atomic_read(&v->dpg_enc_submission_cnt)))
+ new_state.fw_based = VCN_DPG_STATE__PAUSE;
+ else
+ new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+
+ v->pause_dpg_mode(v, &new_state);
+ }
+
+ fence[i] += amdgpu_fence_count_emitted(&v->ring_dec);
+ fences += fence[i];
+
+ }
+
+ if (!fences && !atomic_read(&adev->vcn.inst[0].total_submission_cnt)) {
+ amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+ AMD_PG_STATE_GATE);
+ r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
+ false);
+ if (r)
+ dev_warn(adev->dev, "(%d) failed to disable video power profile mode\n", r);
+ } else {
+ schedule_delayed_work(&adev->vcn.inst[0].idle_work, VCN_IDLE_TIMEOUT);
+ }
+}
+
+static void vcn_v2_5_ring_begin_use(struct amdgpu_ring *ring)
+{
+ struct amdgpu_device *adev = ring->adev;
+ int r = 0, i;
+
+ atomic_inc(&adev->vcn.inst[0].total_submission_cnt);
+
+ if (!cancel_delayed_work_sync(&adev->vcn.inst[0].idle_work)) {
+ r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
+ true);
+ if (r)
+ dev_warn(adev->dev, "(%d) failed to switch to video power profile mode\n", r);
+ }
+
+ mutex_lock(&adev->vcn.inst[0].vcn_pg_lock);
+ amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+ AMD_PG_STATE_UNGATE);
+
+ for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
+ struct amdgpu_vcn_inst *v = &adev->vcn.inst[i];
+
+ if (adev->vcn.harvest_config & (1 << i))
+ continue;
+ /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
+ if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
+ !v->using_unified_queue) {
+ struct dpg_pause_state new_state;
+
+ if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC) {
+ atomic_inc(&v->dpg_enc_submission_cnt);
+ new_state.fw_based = VCN_DPG_STATE__PAUSE;
+ } else {
+ unsigned int fences = 0;
+ unsigned int i;
+
+ for (i = 0; i < v->num_enc_rings; ++i)
+ fences += amdgpu_fence_count_emitted(&v->ring_enc[i]);
+
+ if (fences || atomic_read(&v->dpg_enc_submission_cnt))
+ new_state.fw_based = VCN_DPG_STATE__PAUSE;
+ else
+ new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+ }
+
+ v->pause_dpg_mode(v, &new_state);
+ }
+ }
+ mutex_unlock(&adev->vcn.inst[0].vcn_pg_lock);
+}
+
+static void vcn_v2_5_ring_end_use(struct amdgpu_ring *ring)
+{
+ struct amdgpu_device *adev = ring->adev;
+
+ /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
+ if (ring->adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
+ ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC &&
+ !adev->vcn.inst[ring->me].using_unified_queue)
+ atomic_dec(&adev->vcn.inst[ring->me].dpg_enc_submission_cnt);
+
+ atomic_dec(&adev->vcn.inst[0].total_submission_cnt);
+
+ schedule_delayed_work(&adev->vcn.inst[0].idle_work,
+ VCN_IDLE_TIMEOUT);
+}
+
/**
* vcn_v2_5_early_init - set function pointers and load microcode
*
@@ -201,6 +316,9 @@ static int vcn_v2_5_sw_init(struct amdgpu_ip_block *ip_block)
if (r)
return r;
+ /* Override the work func */
+ adev->vcn.inst[j].idle_work.work.func = vcn_v2_5_idle_work_handler;
+
amdgpu_vcn_setup_ucode(adev, j);
r = amdgpu_vcn_resume(adev, j);
@@ -1661,8 +1779,8 @@ static const struct amdgpu_ring_funcs vcn_v2_5_dec_ring_vm_funcs = {
.insert_start = vcn_v2_0_dec_ring_insert_start,
.insert_end = vcn_v2_0_dec_ring_insert_end,
.pad_ib = amdgpu_ring_generic_pad_ib,
- .begin_use = amdgpu_vcn_ring_begin_use,
- .end_use = amdgpu_vcn_ring_end_use,
+ .begin_use = vcn_v2_5_ring_begin_use,
+ .end_use = vcn_v2_5_ring_end_use,
.emit_wreg = vcn_v2_0_dec_ring_emit_wreg,
.emit_reg_wait = vcn_v2_0_dec_ring_emit_reg_wait,
.emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
@@ -1759,8 +1877,8 @@ static const struct amdgpu_ring_funcs vcn_v2_5_enc_ring_vm_funcs = {
.insert_nop = amdgpu_ring_insert_nop,
.insert_end = vcn_v2_0_enc_ring_insert_end,
.pad_ib = amdgpu_ring_generic_pad_ib,
- .begin_use = amdgpu_vcn_ring_begin_use,
- .end_use = amdgpu_vcn_ring_end_use,
+ .begin_use = vcn_v2_5_ring_begin_use,
+ .end_use = vcn_v2_5_ring_end_use,
.emit_wreg = vcn_v2_0_enc_ring_emit_wreg,
.emit_reg_wait = vcn_v2_0_enc_ring_emit_reg_wait,
.emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
--
2.48.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5
2025-03-04 23:01 [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5 Alex Deucher
@ 2025-03-05 14:20 ` Boyuan Zhang
0 siblings, 0 replies; 12+ messages in thread
From: Boyuan Zhang @ 2025-03-05 14:20 UTC (permalink / raw)
To: Alex Deucher, amd-gfx
[-- Attachment #1: Type: text/plain, Size: 6446 bytes --]
On 2025-03-04 18:01, Alex Deucher wrote:
> VCN 2.5 uses the PG callback to enable VCN DPM which is
> a global state. As such, we need to make sure all instances
> are in the same state.
>
> v2: switch to a ref count (Lijo)
> v3: switch to its own idle work handler
>
> Fixes: 4ce4fe27205c ("drm/amdgpu/vcn: use per instance callbacks for idle work handler")
> Signed-off-by: Alex Deucher<alexander.deucher@amd.com>
Reviewed-by: Boyuan Zhang <Boyuan.Zhang@amd.com>
<mailto:Boyuan.Zhang@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 126 +++++++++++++++++++++++++-
> 1 file changed, 122 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> index dff1a88590363..fa66521b940de 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> @@ -107,6 +107,121 @@ static int amdgpu_ih_clientid_vcns[] = {
> SOC15_IH_CLIENTID_VCN1
> };
>
> +static void vcn_v2_5_idle_work_handler(struct work_struct *work)
> +{
> + struct amdgpu_vcn_inst *vcn_inst =
> + container_of(work, struct amdgpu_vcn_inst, idle_work.work);
> + struct amdgpu_device *adev = vcn_inst->adev;
> + unsigned int fences = 0, fence[AMDGPU_MAX_VCN_INSTANCES] = {0};
> + unsigned int i, j;
> + int r = 0;
> +
> + for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> + struct amdgpu_vcn_inst *v = &adev->vcn.inst[i];
> +
> + if (adev->vcn.harvest_config & (1 << i))
> + continue;
> +
> + for (j = 0; j < v->num_enc_rings; ++j)
> + fence[i] += amdgpu_fence_count_emitted(&v->ring_enc[j]);
> +
> + /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
> + if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
> + !v->using_unified_queue) {
> + struct dpg_pause_state new_state;
> +
> + if (fence[i] ||
> + unlikely(atomic_read(&v->dpg_enc_submission_cnt)))
> + new_state.fw_based = VCN_DPG_STATE__PAUSE;
> + else
> + new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> +
> + v->pause_dpg_mode(v, &new_state);
> + }
> +
> + fence[i] += amdgpu_fence_count_emitted(&v->ring_dec);
> + fences += fence[i];
> +
> + }
> +
> + if (!fences && !atomic_read(&adev->vcn.inst[0].total_submission_cnt)) {
> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> + AMD_PG_STATE_GATE);
> + r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
> + false);
> + if (r)
> + dev_warn(adev->dev, "(%d) failed to disable video power profile mode\n", r);
> + } else {
> + schedule_delayed_work(&adev->vcn.inst[0].idle_work, VCN_IDLE_TIMEOUT);
> + }
> +}
> +
> +static void vcn_v2_5_ring_begin_use(struct amdgpu_ring *ring)
> +{
> + struct amdgpu_device *adev = ring->adev;
> + int r = 0, i;
> +
> + atomic_inc(&adev->vcn.inst[0].total_submission_cnt);
> +
> + if (!cancel_delayed_work_sync(&adev->vcn.inst[0].idle_work)) {
> + r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
> + true);
> + if (r)
> + dev_warn(adev->dev, "(%d) failed to switch to video power profile mode\n", r);
> + }
> +
> + mutex_lock(&adev->vcn.inst[0].vcn_pg_lock);
> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> + AMD_PG_STATE_UNGATE);
> +
> + for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> + struct amdgpu_vcn_inst *v = &adev->vcn.inst[i];
> +
> + if (adev->vcn.harvest_config & (1 << i))
> + continue;
> + /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
> + if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
> + !v->using_unified_queue) {
> + struct dpg_pause_state new_state;
> +
> + if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC) {
> + atomic_inc(&v->dpg_enc_submission_cnt);
> + new_state.fw_based = VCN_DPG_STATE__PAUSE;
> + } else {
> + unsigned int fences = 0;
> + unsigned int i;
> +
> + for (i = 0; i < v->num_enc_rings; ++i)
> + fences += amdgpu_fence_count_emitted(&v->ring_enc[i]);
> +
> + if (fences || atomic_read(&v->dpg_enc_submission_cnt))
> + new_state.fw_based = VCN_DPG_STATE__PAUSE;
> + else
> + new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> + }
> +
> + v->pause_dpg_mode(v, &new_state);
> + }
> + }
> + mutex_unlock(&adev->vcn.inst[0].vcn_pg_lock);
> +}
> +
> +static void vcn_v2_5_ring_end_use(struct amdgpu_ring *ring)
> +{
> + struct amdgpu_device *adev = ring->adev;
> +
> + /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
> + if (ring->adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
> + ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC &&
> + !adev->vcn.inst[ring->me].using_unified_queue)
> + atomic_dec(&adev->vcn.inst[ring->me].dpg_enc_submission_cnt);
> +
> + atomic_dec(&adev->vcn.inst[0].total_submission_cnt);
> +
> + schedule_delayed_work(&adev->vcn.inst[0].idle_work,
> + VCN_IDLE_TIMEOUT);
> +}
> +
> /**
> * vcn_v2_5_early_init - set function pointers and load microcode
> *
> @@ -201,6 +316,9 @@ static int vcn_v2_5_sw_init(struct amdgpu_ip_block *ip_block)
> if (r)
> return r;
>
> + /* Override the work func */
> + adev->vcn.inst[j].idle_work.work.func = vcn_v2_5_idle_work_handler;
> +
> amdgpu_vcn_setup_ucode(adev, j);
>
> r = amdgpu_vcn_resume(adev, j);
> @@ -1661,8 +1779,8 @@ static const struct amdgpu_ring_funcs vcn_v2_5_dec_ring_vm_funcs = {
> .insert_start = vcn_v2_0_dec_ring_insert_start,
> .insert_end = vcn_v2_0_dec_ring_insert_end,
> .pad_ib = amdgpu_ring_generic_pad_ib,
> - .begin_use = amdgpu_vcn_ring_begin_use,
> - .end_use = amdgpu_vcn_ring_end_use,
> + .begin_use = vcn_v2_5_ring_begin_use,
> + .end_use = vcn_v2_5_ring_end_use,
> .emit_wreg = vcn_v2_0_dec_ring_emit_wreg,
> .emit_reg_wait = vcn_v2_0_dec_ring_emit_reg_wait,
> .emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
> @@ -1759,8 +1877,8 @@ static const struct amdgpu_ring_funcs vcn_v2_5_enc_ring_vm_funcs = {
> .insert_nop = amdgpu_ring_insert_nop,
> .insert_end = vcn_v2_0_enc_ring_insert_end,
> .pad_ib = amdgpu_ring_generic_pad_ib,
> - .begin_use = amdgpu_vcn_ring_begin_use,
> - .end_use = amdgpu_vcn_ring_end_use,
> + .begin_use = vcn_v2_5_ring_begin_use,
> + .end_use = vcn_v2_5_ring_end_use,
> .emit_wreg = vcn_v2_0_enc_ring_emit_wreg,
> .emit_reg_wait = vcn_v2_0_enc_ring_emit_reg_wait,
> .emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
[-- Attachment #2: Type: text/html, Size: 7582 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5
@ 2025-03-05 19:17 Alex Deucher
2025-03-06 15:05 ` Alex Deucher
0 siblings, 1 reply; 12+ messages in thread
From: Alex Deucher @ 2025-03-05 19:17 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher
VCN 2.5 uses the PG callback to enable VCN DPM which is
a global state. As such, we need to make sure all instances
are in the same state.
v2: switch to a ref count (Lijo)
v3: switch to its own idle work handler
v4: fix logic in DPG handling
Fixes: 4ce4fe27205c ("drm/amdgpu/vcn: use per instance callbacks for idle work handler")
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 120 +++++++++++++++++++++++++-
1 file changed, 116 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
index dff1a88590363..ff03436698a4f 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -107,6 +107,115 @@ static int amdgpu_ih_clientid_vcns[] = {
SOC15_IH_CLIENTID_VCN1
};
+static void vcn_v2_5_idle_work_handler(struct work_struct *work)
+{
+ struct amdgpu_vcn_inst *vcn_inst =
+ container_of(work, struct amdgpu_vcn_inst, idle_work.work);
+ struct amdgpu_device *adev = vcn_inst->adev;
+ unsigned int fences = 0, fence[AMDGPU_MAX_VCN_INSTANCES] = {0};
+ unsigned int i, j;
+ int r = 0;
+
+ for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
+ struct amdgpu_vcn_inst *v = &adev->vcn.inst[i];
+
+ if (adev->vcn.harvest_config & (1 << i))
+ continue;
+
+ for (j = 0; j < v->num_enc_rings; ++j)
+ fence[i] += amdgpu_fence_count_emitted(&v->ring_enc[j]);
+
+ /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
+ if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
+ !v->using_unified_queue) {
+ struct dpg_pause_state new_state;
+
+ if (fence[i] ||
+ unlikely(atomic_read(&v->dpg_enc_submission_cnt)))
+ new_state.fw_based = VCN_DPG_STATE__PAUSE;
+ else
+ new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+
+ v->pause_dpg_mode(v, &new_state);
+ }
+
+ fence[i] += amdgpu_fence_count_emitted(&v->ring_dec);
+ fences += fence[i];
+
+ }
+
+ if (!fences && !atomic_read(&adev->vcn.inst[0].total_submission_cnt)) {
+ amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+ AMD_PG_STATE_GATE);
+ r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
+ false);
+ if (r)
+ dev_warn(adev->dev, "(%d) failed to disable video power profile mode\n", r);
+ } else {
+ schedule_delayed_work(&adev->vcn.inst[0].idle_work, VCN_IDLE_TIMEOUT);
+ }
+}
+
+static void vcn_v2_5_ring_begin_use(struct amdgpu_ring *ring)
+{
+ struct amdgpu_device *adev = ring->adev;
+ struct amdgpu_vcn_inst *v = &adev->vcn.inst[ring->me];
+ int r = 0;
+
+ atomic_inc(&adev->vcn.inst[0].total_submission_cnt);
+
+ if (!cancel_delayed_work_sync(&adev->vcn.inst[0].idle_work)) {
+ r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
+ true);
+ if (r)
+ dev_warn(adev->dev, "(%d) failed to switch to video power profile mode\n", r);
+ }
+
+ mutex_lock(&adev->vcn.inst[0].vcn_pg_lock);
+ amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+ AMD_PG_STATE_UNGATE);
+
+ /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
+ if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
+ !v->using_unified_queue) {
+ struct dpg_pause_state new_state;
+
+ if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC) {
+ atomic_inc(&v->dpg_enc_submission_cnt);
+ new_state.fw_based = VCN_DPG_STATE__PAUSE;
+ } else {
+ unsigned int fences = 0;
+ unsigned int i;
+
+ for (i = 0; i < v->num_enc_rings; ++i)
+ fences += amdgpu_fence_count_emitted(&v->ring_enc[i]);
+
+ if (fences || atomic_read(&v->dpg_enc_submission_cnt))
+ new_state.fw_based = VCN_DPG_STATE__PAUSE;
+ else
+ new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+ }
+ v->pause_dpg_mode(v, &new_state);
+ }
+ mutex_unlock(&adev->vcn.inst[0].vcn_pg_lock);
+}
+
+static void vcn_v2_5_ring_end_use(struct amdgpu_ring *ring)
+{
+ struct amdgpu_device *adev = ring->adev;
+
+ /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
+ if (ring->adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
+ ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC &&
+ !adev->vcn.inst[ring->me].using_unified_queue)
+ atomic_dec(&adev->vcn.inst[ring->me].dpg_enc_submission_cnt);
+
+ atomic_dec(&adev->vcn.inst[0].total_submission_cnt);
+
+ schedule_delayed_work(&adev->vcn.inst[0].idle_work,
+ VCN_IDLE_TIMEOUT);
+}
+
/**
* vcn_v2_5_early_init - set function pointers and load microcode
*
@@ -201,6 +310,9 @@ static int vcn_v2_5_sw_init(struct amdgpu_ip_block *ip_block)
if (r)
return r;
+ /* Override the work func */
+ adev->vcn.inst[j].idle_work.work.func = vcn_v2_5_idle_work_handler;
+
amdgpu_vcn_setup_ucode(adev, j);
r = amdgpu_vcn_resume(adev, j);
@@ -1661,8 +1773,8 @@ static const struct amdgpu_ring_funcs vcn_v2_5_dec_ring_vm_funcs = {
.insert_start = vcn_v2_0_dec_ring_insert_start,
.insert_end = vcn_v2_0_dec_ring_insert_end,
.pad_ib = amdgpu_ring_generic_pad_ib,
- .begin_use = amdgpu_vcn_ring_begin_use,
- .end_use = amdgpu_vcn_ring_end_use,
+ .begin_use = vcn_v2_5_ring_begin_use,
+ .end_use = vcn_v2_5_ring_end_use,
.emit_wreg = vcn_v2_0_dec_ring_emit_wreg,
.emit_reg_wait = vcn_v2_0_dec_ring_emit_reg_wait,
.emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
@@ -1759,8 +1871,8 @@ static const struct amdgpu_ring_funcs vcn_v2_5_enc_ring_vm_funcs = {
.insert_nop = amdgpu_ring_insert_nop,
.insert_end = vcn_v2_0_enc_ring_insert_end,
.pad_ib = amdgpu_ring_generic_pad_ib,
- .begin_use = amdgpu_vcn_ring_begin_use,
- .end_use = amdgpu_vcn_ring_end_use,
+ .begin_use = vcn_v2_5_ring_begin_use,
+ .end_use = vcn_v2_5_ring_end_use,
.emit_wreg = vcn_v2_0_enc_ring_emit_wreg,
.emit_reg_wait = vcn_v2_0_enc_ring_emit_reg_wait,
.emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
--
2.48.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5
2025-03-05 19:17 Alex Deucher
@ 2025-03-06 15:05 ` Alex Deucher
2025-03-07 15:22 ` Alex Deucher
0 siblings, 1 reply; 12+ messages in thread
From: Alex Deucher @ 2025-03-06 15:05 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx
Ping?
Thanks,
Alex
On Wed, Mar 5, 2025 at 2:42 PM Alex Deucher <alexander.deucher@amd.com> wrote:
>
> VCN 2.5 uses the PG callback to enable VCN DPM which is
> a global state. As such, we need to make sure all instances
> are in the same state.
>
> v2: switch to a ref count (Lijo)
> v3: switch to its own idle work handler
> v4: fix logic in DPG handling
>
> Fixes: 4ce4fe27205c ("drm/amdgpu/vcn: use per instance callbacks for idle work handler")
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 120 +++++++++++++++++++++++++-
> 1 file changed, 116 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> index dff1a88590363..ff03436698a4f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> @@ -107,6 +107,115 @@ static int amdgpu_ih_clientid_vcns[] = {
> SOC15_IH_CLIENTID_VCN1
> };
>
> +static void vcn_v2_5_idle_work_handler(struct work_struct *work)
> +{
> + struct amdgpu_vcn_inst *vcn_inst =
> + container_of(work, struct amdgpu_vcn_inst, idle_work.work);
> + struct amdgpu_device *adev = vcn_inst->adev;
> + unsigned int fences = 0, fence[AMDGPU_MAX_VCN_INSTANCES] = {0};
> + unsigned int i, j;
> + int r = 0;
> +
> + for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> + struct amdgpu_vcn_inst *v = &adev->vcn.inst[i];
> +
> + if (adev->vcn.harvest_config & (1 << i))
> + continue;
> +
> + for (j = 0; j < v->num_enc_rings; ++j)
> + fence[i] += amdgpu_fence_count_emitted(&v->ring_enc[j]);
> +
> + /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
> + if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
> + !v->using_unified_queue) {
> + struct dpg_pause_state new_state;
> +
> + if (fence[i] ||
> + unlikely(atomic_read(&v->dpg_enc_submission_cnt)))
> + new_state.fw_based = VCN_DPG_STATE__PAUSE;
> + else
> + new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> +
> + v->pause_dpg_mode(v, &new_state);
> + }
> +
> + fence[i] += amdgpu_fence_count_emitted(&v->ring_dec);
> + fences += fence[i];
> +
> + }
> +
> + if (!fences && !atomic_read(&adev->vcn.inst[0].total_submission_cnt)) {
> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> + AMD_PG_STATE_GATE);
> + r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
> + false);
> + if (r)
> + dev_warn(adev->dev, "(%d) failed to disable video power profile mode\n", r);
> + } else {
> + schedule_delayed_work(&adev->vcn.inst[0].idle_work, VCN_IDLE_TIMEOUT);
> + }
> +}
> +
> +static void vcn_v2_5_ring_begin_use(struct amdgpu_ring *ring)
> +{
> + struct amdgpu_device *adev = ring->adev;
> + struct amdgpu_vcn_inst *v = &adev->vcn.inst[ring->me];
> + int r = 0;
> +
> + atomic_inc(&adev->vcn.inst[0].total_submission_cnt);
> +
> + if (!cancel_delayed_work_sync(&adev->vcn.inst[0].idle_work)) {
> + r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
> + true);
> + if (r)
> + dev_warn(adev->dev, "(%d) failed to switch to video power profile mode\n", r);
> + }
> +
> + mutex_lock(&adev->vcn.inst[0].vcn_pg_lock);
> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> + AMD_PG_STATE_UNGATE);
> +
> + /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
> + if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
> + !v->using_unified_queue) {
> + struct dpg_pause_state new_state;
> +
> + if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC) {
> + atomic_inc(&v->dpg_enc_submission_cnt);
> + new_state.fw_based = VCN_DPG_STATE__PAUSE;
> + } else {
> + unsigned int fences = 0;
> + unsigned int i;
> +
> + for (i = 0; i < v->num_enc_rings; ++i)
> + fences += amdgpu_fence_count_emitted(&v->ring_enc[i]);
> +
> + if (fences || atomic_read(&v->dpg_enc_submission_cnt))
> + new_state.fw_based = VCN_DPG_STATE__PAUSE;
> + else
> + new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> + }
> + v->pause_dpg_mode(v, &new_state);
> + }
> + mutex_unlock(&adev->vcn.inst[0].vcn_pg_lock);
> +}
> +
> +static void vcn_v2_5_ring_end_use(struct amdgpu_ring *ring)
> +{
> + struct amdgpu_device *adev = ring->adev;
> +
> + /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
> + if (ring->adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
> + ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC &&
> + !adev->vcn.inst[ring->me].using_unified_queue)
> + atomic_dec(&adev->vcn.inst[ring->me].dpg_enc_submission_cnt);
> +
> + atomic_dec(&adev->vcn.inst[0].total_submission_cnt);
> +
> + schedule_delayed_work(&adev->vcn.inst[0].idle_work,
> + VCN_IDLE_TIMEOUT);
> +}
> +
> /**
> * vcn_v2_5_early_init - set function pointers and load microcode
> *
> @@ -201,6 +310,9 @@ static int vcn_v2_5_sw_init(struct amdgpu_ip_block *ip_block)
> if (r)
> return r;
>
> + /* Override the work func */
> + adev->vcn.inst[j].idle_work.work.func = vcn_v2_5_idle_work_handler;
> +
> amdgpu_vcn_setup_ucode(adev, j);
>
> r = amdgpu_vcn_resume(adev, j);
> @@ -1661,8 +1773,8 @@ static const struct amdgpu_ring_funcs vcn_v2_5_dec_ring_vm_funcs = {
> .insert_start = vcn_v2_0_dec_ring_insert_start,
> .insert_end = vcn_v2_0_dec_ring_insert_end,
> .pad_ib = amdgpu_ring_generic_pad_ib,
> - .begin_use = amdgpu_vcn_ring_begin_use,
> - .end_use = amdgpu_vcn_ring_end_use,
> + .begin_use = vcn_v2_5_ring_begin_use,
> + .end_use = vcn_v2_5_ring_end_use,
> .emit_wreg = vcn_v2_0_dec_ring_emit_wreg,
> .emit_reg_wait = vcn_v2_0_dec_ring_emit_reg_wait,
> .emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
> @@ -1759,8 +1871,8 @@ static const struct amdgpu_ring_funcs vcn_v2_5_enc_ring_vm_funcs = {
> .insert_nop = amdgpu_ring_insert_nop,
> .insert_end = vcn_v2_0_enc_ring_insert_end,
> .pad_ib = amdgpu_ring_generic_pad_ib,
> - .begin_use = amdgpu_vcn_ring_begin_use,
> - .end_use = amdgpu_vcn_ring_end_use,
> + .begin_use = vcn_v2_5_ring_begin_use,
> + .end_use = vcn_v2_5_ring_end_use,
> .emit_wreg = vcn_v2_0_enc_ring_emit_wreg,
> .emit_reg_wait = vcn_v2_0_enc_ring_emit_reg_wait,
> .emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5
2025-03-06 15:05 ` Alex Deucher
@ 2025-03-07 15:22 ` Alex Deucher
2025-03-07 21:14 ` Zhang, Boyuan
0 siblings, 1 reply; 12+ messages in thread
From: Alex Deucher @ 2025-03-07 15:22 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx
Ping? This fixes a regression on VCN 2.5.
Thanks,
Alex
On Thu, Mar 6, 2025 at 10:05 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> Ping?
>
> Thanks,
>
> Alex
>
> On Wed, Mar 5, 2025 at 2:42 PM Alex Deucher <alexander.deucher@amd.com> wrote:
> >
> > VCN 2.5 uses the PG callback to enable VCN DPM which is
> > a global state. As such, we need to make sure all instances
> > are in the same state.
> >
> > v2: switch to a ref count (Lijo)
> > v3: switch to its own idle work handler
> > v4: fix logic in DPG handling
> >
> > Fixes: 4ce4fe27205c ("drm/amdgpu/vcn: use per instance callbacks for idle work handler")
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 120 +++++++++++++++++++++++++-
> > 1 file changed, 116 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > index dff1a88590363..ff03436698a4f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > @@ -107,6 +107,115 @@ static int amdgpu_ih_clientid_vcns[] = {
> > SOC15_IH_CLIENTID_VCN1
> > };
> >
> > +static void vcn_v2_5_idle_work_handler(struct work_struct *work)
> > +{
> > + struct amdgpu_vcn_inst *vcn_inst =
> > + container_of(work, struct amdgpu_vcn_inst, idle_work.work);
> > + struct amdgpu_device *adev = vcn_inst->adev;
> > + unsigned int fences = 0, fence[AMDGPU_MAX_VCN_INSTANCES] = {0};
> > + unsigned int i, j;
> > + int r = 0;
> > +
> > + for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> > + struct amdgpu_vcn_inst *v = &adev->vcn.inst[i];
> > +
> > + if (adev->vcn.harvest_config & (1 << i))
> > + continue;
> > +
> > + for (j = 0; j < v->num_enc_rings; ++j)
> > + fence[i] += amdgpu_fence_count_emitted(&v->ring_enc[j]);
> > +
> > + /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
> > + if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
> > + !v->using_unified_queue) {
> > + struct dpg_pause_state new_state;
> > +
> > + if (fence[i] ||
> > + unlikely(atomic_read(&v->dpg_enc_submission_cnt)))
> > + new_state.fw_based = VCN_DPG_STATE__PAUSE;
> > + else
> > + new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> > +
> > + v->pause_dpg_mode(v, &new_state);
> > + }
> > +
> > + fence[i] += amdgpu_fence_count_emitted(&v->ring_dec);
> > + fences += fence[i];
> > +
> > + }
> > +
> > + if (!fences && !atomic_read(&adev->vcn.inst[0].total_submission_cnt)) {
> > + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> > + AMD_PG_STATE_GATE);
> > + r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
> > + false);
> > + if (r)
> > + dev_warn(adev->dev, "(%d) failed to disable video power profile mode\n", r);
> > + } else {
> > + schedule_delayed_work(&adev->vcn.inst[0].idle_work, VCN_IDLE_TIMEOUT);
> > + }
> > +}
> > +
> > +static void vcn_v2_5_ring_begin_use(struct amdgpu_ring *ring)
> > +{
> > + struct amdgpu_device *adev = ring->adev;
> > + struct amdgpu_vcn_inst *v = &adev->vcn.inst[ring->me];
> > + int r = 0;
> > +
> > + atomic_inc(&adev->vcn.inst[0].total_submission_cnt);
> > +
> > + if (!cancel_delayed_work_sync(&adev->vcn.inst[0].idle_work)) {
> > + r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
> > + true);
> > + if (r)
> > + dev_warn(adev->dev, "(%d) failed to switch to video power profile mode\n", r);
> > + }
> > +
> > + mutex_lock(&adev->vcn.inst[0].vcn_pg_lock);
> > + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> > + AMD_PG_STATE_UNGATE);
> > +
> > + /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
> > + if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
> > + !v->using_unified_queue) {
> > + struct dpg_pause_state new_state;
> > +
> > + if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC) {
> > + atomic_inc(&v->dpg_enc_submission_cnt);
> > + new_state.fw_based = VCN_DPG_STATE__PAUSE;
> > + } else {
> > + unsigned int fences = 0;
> > + unsigned int i;
> > +
> > + for (i = 0; i < v->num_enc_rings; ++i)
> > + fences += amdgpu_fence_count_emitted(&v->ring_enc[i]);
> > +
> > + if (fences || atomic_read(&v->dpg_enc_submission_cnt))
> > + new_state.fw_based = VCN_DPG_STATE__PAUSE;
> > + else
> > + new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> > + }
> > + v->pause_dpg_mode(v, &new_state);
> > + }
> > + mutex_unlock(&adev->vcn.inst[0].vcn_pg_lock);
> > +}
> > +
> > +static void vcn_v2_5_ring_end_use(struct amdgpu_ring *ring)
> > +{
> > + struct amdgpu_device *adev = ring->adev;
> > +
> > + /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
> > + if (ring->adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
> > + ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC &&
> > + !adev->vcn.inst[ring->me].using_unified_queue)
> > + atomic_dec(&adev->vcn.inst[ring->me].dpg_enc_submission_cnt);
> > +
> > + atomic_dec(&adev->vcn.inst[0].total_submission_cnt);
> > +
> > + schedule_delayed_work(&adev->vcn.inst[0].idle_work,
> > + VCN_IDLE_TIMEOUT);
> > +}
> > +
> > /**
> > * vcn_v2_5_early_init - set function pointers and load microcode
> > *
> > @@ -201,6 +310,9 @@ static int vcn_v2_5_sw_init(struct amdgpu_ip_block *ip_block)
> > if (r)
> > return r;
> >
> > + /* Override the work func */
> > + adev->vcn.inst[j].idle_work.work.func = vcn_v2_5_idle_work_handler;
> > +
> > amdgpu_vcn_setup_ucode(adev, j);
> >
> > r = amdgpu_vcn_resume(adev, j);
> > @@ -1661,8 +1773,8 @@ static const struct amdgpu_ring_funcs vcn_v2_5_dec_ring_vm_funcs = {
> > .insert_start = vcn_v2_0_dec_ring_insert_start,
> > .insert_end = vcn_v2_0_dec_ring_insert_end,
> > .pad_ib = amdgpu_ring_generic_pad_ib,
> > - .begin_use = amdgpu_vcn_ring_begin_use,
> > - .end_use = amdgpu_vcn_ring_end_use,
> > + .begin_use = vcn_v2_5_ring_begin_use,
> > + .end_use = vcn_v2_5_ring_end_use,
> > .emit_wreg = vcn_v2_0_dec_ring_emit_wreg,
> > .emit_reg_wait = vcn_v2_0_dec_ring_emit_reg_wait,
> > .emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
> > @@ -1759,8 +1871,8 @@ static const struct amdgpu_ring_funcs vcn_v2_5_enc_ring_vm_funcs = {
> > .insert_nop = amdgpu_ring_insert_nop,
> > .insert_end = vcn_v2_0_enc_ring_insert_end,
> > .pad_ib = amdgpu_ring_generic_pad_ib,
> > - .begin_use = amdgpu_vcn_ring_begin_use,
> > - .end_use = amdgpu_vcn_ring_end_use,
> > + .begin_use = vcn_v2_5_ring_begin_use,
> > + .end_use = vcn_v2_5_ring_end_use,
> > .emit_wreg = vcn_v2_0_enc_ring_emit_wreg,
> > .emit_reg_wait = vcn_v2_0_enc_ring_emit_reg_wait,
> > .emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
> > --
> > 2.48.1
> >
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5
2025-03-07 15:22 ` Alex Deucher
@ 2025-03-07 21:14 ` Zhang, Boyuan
0 siblings, 0 replies; 12+ messages in thread
From: Zhang, Boyuan @ 2025-03-07 21:14 UTC (permalink / raw)
To: Alex Deucher, Deucher, Alexander; +Cc: amd-gfx@lists.freedesktop.org
[-- Attachment #1: Type: text/plain, Size: 8795 bytes --]
[AMD Official Use Only - AMD Internal Distribution Only]
V4 is Reviewed-by: Boyuan Zhang <Boyuan.Zhang@amd.com><mailto:Boyuan.Zhang@amd.com>
________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Alex Deucher <alexdeucher@gmail.com>
Sent: March 7, 2025 10:22 AM
To: Deucher, Alexander <Alexander.Deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5
Ping? This fixes a regression on VCN 2.5.
Thanks,
Alex
On Thu, Mar 6, 2025 at 10:05 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> Ping?
>
> Thanks,
>
> Alex
>
> On Wed, Mar 5, 2025 at 2:42 PM Alex Deucher <alexander.deucher@amd.com> wrote:
> >
> > VCN 2.5 uses the PG callback to enable VCN DPM which is
> > a global state. As such, we need to make sure all instances
> > are in the same state.
> >
> > v2: switch to a ref count (Lijo)
> > v3: switch to its own idle work handler
> > v4: fix logic in DPG handling
> >
> > Fixes: 4ce4fe27205c ("drm/amdgpu/vcn: use per instance callbacks for idle work handler")
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 120 +++++++++++++++++++++++++-
> > 1 file changed, 116 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > index dff1a88590363..ff03436698a4f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > @@ -107,6 +107,115 @@ static int amdgpu_ih_clientid_vcns[] = {
> > SOC15_IH_CLIENTID_VCN1
> > };
> >
> > +static void vcn_v2_5_idle_work_handler(struct work_struct *work)
> > +{
> > + struct amdgpu_vcn_inst *vcn_inst =
> > + container_of(work, struct amdgpu_vcn_inst, idle_work.work);
> > + struct amdgpu_device *adev = vcn_inst->adev;
> > + unsigned int fences = 0, fence[AMDGPU_MAX_VCN_INSTANCES] = {0};
> > + unsigned int i, j;
> > + int r = 0;
> > +
> > + for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> > + struct amdgpu_vcn_inst *v = &adev->vcn.inst[i];
> > +
> > + if (adev->vcn.harvest_config & (1 << i))
> > + continue;
> > +
> > + for (j = 0; j < v->num_enc_rings; ++j)
> > + fence[i] += amdgpu_fence_count_emitted(&v->ring_enc[j]);
> > +
> > + /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
> > + if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
> > + !v->using_unified_queue) {
> > + struct dpg_pause_state new_state;
> > +
> > + if (fence[i] ||
> > + unlikely(atomic_read(&v->dpg_enc_submission_cnt)))
> > + new_state.fw_based = VCN_DPG_STATE__PAUSE;
> > + else
> > + new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> > +
> > + v->pause_dpg_mode(v, &new_state);
> > + }
> > +
> > + fence[i] += amdgpu_fence_count_emitted(&v->ring_dec);
> > + fences += fence[i];
> > +
> > + }
> > +
> > + if (!fences && !atomic_read(&adev->vcn.inst[0].total_submission_cnt)) {
> > + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> > + AMD_PG_STATE_GATE);
> > + r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
> > + false);
> > + if (r)
> > + dev_warn(adev->dev, "(%d) failed to disable video power profile mode\n", r);
> > + } else {
> > + schedule_delayed_work(&adev->vcn.inst[0].idle_work, VCN_IDLE_TIMEOUT);
> > + }
> > +}
> > +
> > +static void vcn_v2_5_ring_begin_use(struct amdgpu_ring *ring)
> > +{
> > + struct amdgpu_device *adev = ring->adev;
> > + struct amdgpu_vcn_inst *v = &adev->vcn.inst[ring->me];
> > + int r = 0;
> > +
> > + atomic_inc(&adev->vcn.inst[0].total_submission_cnt);
> > +
> > + if (!cancel_delayed_work_sync(&adev->vcn.inst[0].idle_work)) {
> > + r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
> > + true);
> > + if (r)
> > + dev_warn(adev->dev, "(%d) failed to switch to video power profile mode\n", r);
> > + }
> > +
> > + mutex_lock(&adev->vcn.inst[0].vcn_pg_lock);
> > + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> > + AMD_PG_STATE_UNGATE);
> > +
> > + /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
> > + if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
> > + !v->using_unified_queue) {
> > + struct dpg_pause_state new_state;
> > +
> > + if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC) {
> > + atomic_inc(&v->dpg_enc_submission_cnt);
> > + new_state.fw_based = VCN_DPG_STATE__PAUSE;
> > + } else {
> > + unsigned int fences = 0;
> > + unsigned int i;
> > +
> > + for (i = 0; i < v->num_enc_rings; ++i)
> > + fences += amdgpu_fence_count_emitted(&v->ring_enc[i]);
> > +
> > + if (fences || atomic_read(&v->dpg_enc_submission_cnt))
> > + new_state.fw_based = VCN_DPG_STATE__PAUSE;
> > + else
> > + new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> > + }
> > + v->pause_dpg_mode(v, &new_state);
> > + }
> > + mutex_unlock(&adev->vcn.inst[0].vcn_pg_lock);
> > +}
> > +
> > +static void vcn_v2_5_ring_end_use(struct amdgpu_ring *ring)
> > +{
> > + struct amdgpu_device *adev = ring->adev;
> > +
> > + /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
> > + if (ring->adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
> > + ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC &&
> > + !adev->vcn.inst[ring->me].using_unified_queue)
> > + atomic_dec(&adev->vcn.inst[ring->me].dpg_enc_submission_cnt);
> > +
> > + atomic_dec(&adev->vcn.inst[0].total_submission_cnt);
> > +
> > + schedule_delayed_work(&adev->vcn.inst[0].idle_work,
> > + VCN_IDLE_TIMEOUT);
> > +}
> > +
> > /**
> > * vcn_v2_5_early_init - set function pointers and load microcode
> > *
> > @@ -201,6 +310,9 @@ static int vcn_v2_5_sw_init(struct amdgpu_ip_block *ip_block)
> > if (r)
> > return r;
> >
> > + /* Override the work func */
> > + adev->vcn.inst[j].idle_work.work.func = vcn_v2_5_idle_work_handler;
> > +
> > amdgpu_vcn_setup_ucode(adev, j);
> >
> > r = amdgpu_vcn_resume(adev, j);
> > @@ -1661,8 +1773,8 @@ static const struct amdgpu_ring_funcs vcn_v2_5_dec_ring_vm_funcs = {
> > .insert_start = vcn_v2_0_dec_ring_insert_start,
> > .insert_end = vcn_v2_0_dec_ring_insert_end,
> > .pad_ib = amdgpu_ring_generic_pad_ib,
> > - .begin_use = amdgpu_vcn_ring_begin_use,
> > - .end_use = amdgpu_vcn_ring_end_use,
> > + .begin_use = vcn_v2_5_ring_begin_use,
> > + .end_use = vcn_v2_5_ring_end_use,
> > .emit_wreg = vcn_v2_0_dec_ring_emit_wreg,
> > .emit_reg_wait = vcn_v2_0_dec_ring_emit_reg_wait,
> > .emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
> > @@ -1759,8 +1871,8 @@ static const struct amdgpu_ring_funcs vcn_v2_5_enc_ring_vm_funcs = {
> > .insert_nop = amdgpu_ring_insert_nop,
> > .insert_end = vcn_v2_0_enc_ring_insert_end,
> > .pad_ib = amdgpu_ring_generic_pad_ib,
> > - .begin_use = amdgpu_vcn_ring_begin_use,
> > - .end_use = amdgpu_vcn_ring_end_use,
> > + .begin_use = vcn_v2_5_ring_begin_use,
> > + .end_use = vcn_v2_5_ring_end_use,
> > .emit_wreg = vcn_v2_0_enc_ring_emit_wreg,
> > .emit_reg_wait = vcn_v2_0_enc_ring_emit_reg_wait,
> > .emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
> > --
> > 2.48.1
> >
[-- Attachment #2: Type: text/html, Size: 19768 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5
@ 2025-03-04 16:22 Alex Deucher
2025-03-04 20:52 ` Boyuan Zhang
2025-03-04 23:12 ` Alex Deucher
0 siblings, 2 replies; 12+ messages in thread
From: Alex Deucher @ 2025-03-04 16:22 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher
VCN 2.5 uses the PG callback to enable VCN DPM which is
a global state. As such, we need to make sure all instances
are in the same state.
v2: switch to a ref count (Lijo)
Fixes: 4ce4fe27205c ("drm/amdgpu/vcn: use per instance callbacks for idle work handler")
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 4 +++
drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 41 +++++++++++++++++++------
2 files changed, 36 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 26c9c2d90f455..3bc4fe4aeb481 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -358,6 +358,10 @@ struct amdgpu_vcn {
bool per_inst_fw;
unsigned fw_version;
+ /* VCN 2.5 global PG handling */
+ struct mutex global_pg_lock;
+ unsigned int global_pg_count;
+ enum amd_powergating_state global_pg_state;
};
struct amdgpu_fw_shared_rb_ptrs_struct {
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
index dff1a88590363..972f0842ea47b 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -172,6 +172,8 @@ static int vcn_v2_5_sw_init(struct amdgpu_ip_block *ip_block)
uint32_t *ptr;
struct amdgpu_device *adev = ip_block->adev;
+ mutex_init(&adev->vcn.global_pg_lock);
+
for (j = 0; j < adev->vcn.num_vcn_inst; j++) {
volatile struct amdgpu_fw_shared *fw_shared;
@@ -1853,21 +1855,42 @@ static int vcn_v2_5_set_pg_state(struct amdgpu_vcn_inst *vinst,
enum amd_powergating_state state)
{
struct amdgpu_device *adev = vinst->adev;
- int ret;
+ struct amdgpu_vcn_inst *v;
+ int ret = 0, i;
if (amdgpu_sriov_vf(adev))
return 0;
- if (state == vinst->cur_state)
- return 0;
+ mutex_lock(&adev->vcn.global_pg_lock);
+ if (state == AMD_PG_STATE_GATE) {
+ if (adev->vcn.global_pg_count == 0)
+ goto unlock;
+ adev->vcn.global_pg_count--;
+ if (adev->vcn.global_pg_count == 0 &&
+ adev->vcn.global_pg_state == AMD_PG_STATE_UNGATE) {
+ for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
+ v = &adev->vcn.inst[i];
+
+ ret = vcn_v2_5_stop(v);
+ }
+ adev->vcn.global_pg_state = AMD_PG_STATE_GATE;
+ }
+ } else {
+ if (adev->vcn.global_pg_count == 0 &&
+ adev->vcn.global_pg_state == AMD_PG_STATE_GATE) {
+ for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
+ v = &adev->vcn.inst[i];
- if (state == AMD_PG_STATE_GATE)
- ret = vcn_v2_5_stop(vinst);
- else
- ret = vcn_v2_5_start(vinst);
+ ret = vcn_v2_5_start(v);
+ }
+ adev->vcn.global_pg_state = AMD_PG_STATE_UNGATE;
+ }
+ adev->vcn.global_pg_count++;
+ }
- if (!ret)
- vinst->cur_state = state;
+unlock:
+ vinst->cur_state = state;
+ mutex_unlock(&adev->vcn.global_pg_lock);
return ret;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5
2025-03-04 16:22 Alex Deucher
@ 2025-03-04 20:52 ` Boyuan Zhang
2025-03-04 21:05 ` Alex Deucher
2025-03-04 23:12 ` Alex Deucher
1 sibling, 1 reply; 12+ messages in thread
From: Boyuan Zhang @ 2025-03-04 20:52 UTC (permalink / raw)
To: Alex Deucher, amd-gfx
On 2025-03-04 11:22, Alex Deucher wrote:
> VCN 2.5 uses the PG callback to enable VCN DPM which is
> a global state. As such, we need to make sure all instances
> are in the same state.
>
> v2: switch to a ref count (Lijo)
>
> Fixes: 4ce4fe27205c ("drm/amdgpu/vcn: use per instance callbacks for idle work handler")
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 4 +++
> drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 41 +++++++++++++++++++------
> 2 files changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 26c9c2d90f455..3bc4fe4aeb481 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -358,6 +358,10 @@ struct amdgpu_vcn {
>
> bool per_inst_fw;
> unsigned fw_version;
> + /* VCN 2.5 global PG handling */
> + struct mutex global_pg_lock;
> + unsigned int global_pg_count;
> + enum amd_powergating_state global_pg_state;
> };
>
> struct amdgpu_fw_shared_rb_ptrs_struct {
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> index dff1a88590363..972f0842ea47b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> @@ -172,6 +172,8 @@ static int vcn_v2_5_sw_init(struct amdgpu_ip_block *ip_block)
> uint32_t *ptr;
> struct amdgpu_device *adev = ip_block->adev;
>
> + mutex_init(&adev->vcn.global_pg_lock);
> +
> for (j = 0; j < adev->vcn.num_vcn_inst; j++) {
> volatile struct amdgpu_fw_shared *fw_shared;
>
> @@ -1853,21 +1855,42 @@ static int vcn_v2_5_set_pg_state(struct amdgpu_vcn_inst *vinst,
> enum amd_powergating_state state)
> {
> struct amdgpu_device *adev = vinst->adev;
> - int ret;
> + struct amdgpu_vcn_inst *v;
> + int ret = 0, i;
>
> if (amdgpu_sriov_vf(adev))
> return 0;
>
> - if (state == vinst->cur_state)
> - return 0;
> + mutex_lock(&adev->vcn.global_pg_lock);
> + if (state == AMD_PG_STATE_GATE) {
> + if (adev->vcn.global_pg_count == 0)
> + goto unlock;
> + adev->vcn.global_pg_count--;
> + if (adev->vcn.global_pg_count == 0 &&
> + adev->vcn.global_pg_state == AMD_PG_STATE_UNGATE) {
> + for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> + v = &adev->vcn.inst[i];
> +
> + ret = vcn_v2_5_stop(v);
> + }
> + adev->vcn.global_pg_state = AMD_PG_STATE_GATE;
> + }
> + } else {
> + if (adev->vcn.global_pg_count == 0 &&
> + adev->vcn.global_pg_state == AMD_PG_STATE_GATE) {
> + for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> + v = &adev->vcn.inst[i];
>
> - if (state == AMD_PG_STATE_GATE)
> - ret = vcn_v2_5_stop(vinst);
> - else
> - ret = vcn_v2_5_start(vinst);
> + ret = vcn_v2_5_start(v);
> + }
> + adev->vcn.global_pg_state = AMD_PG_STATE_UNGATE;
> + }
> + adev->vcn.global_pg_count++;
> + }
>
> - if (!ret)
> - vinst->cur_state = state;
> +unlock:
> + vinst->cur_state = state;
I guess we don't need to bother this per instant (vinst->cur_state) at
all in this case? Other than this, this patch is
Reviewed-by: Boyuan Zhang <Boyuan.Zhang@amd.com>
> + mutex_unlock(&adev->vcn.global_pg_lock);
>
> return ret;
> }
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5
2025-03-04 20:52 ` Boyuan Zhang
@ 2025-03-04 21:05 ` Alex Deucher
0 siblings, 0 replies; 12+ messages in thread
From: Alex Deucher @ 2025-03-04 21:05 UTC (permalink / raw)
To: Boyuan Zhang; +Cc: Alex Deucher, amd-gfx
On Tue, Mar 4, 2025 at 4:00 PM Boyuan Zhang <Boyuan.Zhang@amd.com> wrote:
>
>
> On 2025-03-04 11:22, Alex Deucher wrote:
> > VCN 2.5 uses the PG callback to enable VCN DPM which is
> > a global state. As such, we need to make sure all instances
> > are in the same state.
> >
> > v2: switch to a ref count (Lijo)
> >
> > Fixes: 4ce4fe27205c ("drm/amdgpu/vcn: use per instance callbacks for idle work handler")
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 4 +++
> > drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 41 +++++++++++++++++++------
> > 2 files changed, 36 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> > index 26c9c2d90f455..3bc4fe4aeb481 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> > @@ -358,6 +358,10 @@ struct amdgpu_vcn {
> >
> > bool per_inst_fw;
> > unsigned fw_version;
> > + /* VCN 2.5 global PG handling */
> > + struct mutex global_pg_lock;
> > + unsigned int global_pg_count;
> > + enum amd_powergating_state global_pg_state;
> > };
> >
> > struct amdgpu_fw_shared_rb_ptrs_struct {
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > index dff1a88590363..972f0842ea47b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > @@ -172,6 +172,8 @@ static int vcn_v2_5_sw_init(struct amdgpu_ip_block *ip_block)
> > uint32_t *ptr;
> > struct amdgpu_device *adev = ip_block->adev;
> >
> > + mutex_init(&adev->vcn.global_pg_lock);
> > +
> > for (j = 0; j < adev->vcn.num_vcn_inst; j++) {
> > volatile struct amdgpu_fw_shared *fw_shared;
> >
> > @@ -1853,21 +1855,42 @@ static int vcn_v2_5_set_pg_state(struct amdgpu_vcn_inst *vinst,
> > enum amd_powergating_state state)
> > {
> > struct amdgpu_device *adev = vinst->adev;
> > - int ret;
> > + struct amdgpu_vcn_inst *v;
> > + int ret = 0, i;
> >
> > if (amdgpu_sriov_vf(adev))
> > return 0;
> >
> > - if (state == vinst->cur_state)
> > - return 0;
> > + mutex_lock(&adev->vcn.global_pg_lock);
> > + if (state == AMD_PG_STATE_GATE) {
> > + if (adev->vcn.global_pg_count == 0)
> > + goto unlock;
> > + adev->vcn.global_pg_count--;
> > + if (adev->vcn.global_pg_count == 0 &&
> > + adev->vcn.global_pg_state == AMD_PG_STATE_UNGATE) {
> > + for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> > + v = &adev->vcn.inst[i];
> > +
> > + ret = vcn_v2_5_stop(v);
> > + }
> > + adev->vcn.global_pg_state = AMD_PG_STATE_GATE;
> > + }
> > + } else {
> > + if (adev->vcn.global_pg_count == 0 &&
> > + adev->vcn.global_pg_state == AMD_PG_STATE_GATE) {
> > + for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> > + v = &adev->vcn.inst[i];
> >
> > - if (state == AMD_PG_STATE_GATE)
> > - ret = vcn_v2_5_stop(vinst);
> > - else
> > - ret = vcn_v2_5_start(vinst);
> > + ret = vcn_v2_5_start(v);
> > + }
> > + adev->vcn.global_pg_state = AMD_PG_STATE_UNGATE;
> > + }
> > + adev->vcn.global_pg_count++;
> > + }
> >
> > - if (!ret)
> > - vinst->cur_state = state;
> > +unlock:
> > + vinst->cur_state = state;
>
>
> I guess we don't need to bother this per instant (vinst->cur_state) at
> all in this case? Other than this, this patch is
I figured it would be good to keep it up to date just in case, but
it's not strictly required.
>
> Reviewed-by: Boyuan Zhang <Boyuan.Zhang@amd.com>
Thanks,
Alex
>
>
> > + mutex_unlock(&adev->vcn.global_pg_lock);
> >
> > return ret;
> > }
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5
2025-03-04 16:22 Alex Deucher
2025-03-04 20:52 ` Boyuan Zhang
@ 2025-03-04 23:12 ` Alex Deucher
1 sibling, 0 replies; 12+ messages in thread
From: Alex Deucher @ 2025-03-04 23:12 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx
On Tue, Mar 4, 2025 at 11:29 AM Alex Deucher <alexander.deucher@amd.com> wrote:
>
> VCN 2.5 uses the PG callback to enable VCN DPM which is
> a global state. As such, we need to make sure all instances
> are in the same state.
Actually ref counting won't work because the gate and ungate calls may
not be balanced. I just sent a v3 which just adds a new work handler
for vcn 2.5.
Alex
>
> v2: switch to a ref count (Lijo)
>
> Fixes: 4ce4fe27205c ("drm/amdgpu/vcn: use per instance callbacks for idle work handler")
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 4 +++
> drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 41 +++++++++++++++++++------
> 2 files changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 26c9c2d90f455..3bc4fe4aeb481 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -358,6 +358,10 @@ struct amdgpu_vcn {
>
> bool per_inst_fw;
> unsigned fw_version;
> + /* VCN 2.5 global PG handling */
> + struct mutex global_pg_lock;
> + unsigned int global_pg_count;
> + enum amd_powergating_state global_pg_state;
> };
>
> struct amdgpu_fw_shared_rb_ptrs_struct {
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> index dff1a88590363..972f0842ea47b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> @@ -172,6 +172,8 @@ static int vcn_v2_5_sw_init(struct amdgpu_ip_block *ip_block)
> uint32_t *ptr;
> struct amdgpu_device *adev = ip_block->adev;
>
> + mutex_init(&adev->vcn.global_pg_lock);
> +
> for (j = 0; j < adev->vcn.num_vcn_inst; j++) {
> volatile struct amdgpu_fw_shared *fw_shared;
>
> @@ -1853,21 +1855,42 @@ static int vcn_v2_5_set_pg_state(struct amdgpu_vcn_inst *vinst,
> enum amd_powergating_state state)
> {
> struct amdgpu_device *adev = vinst->adev;
> - int ret;
> + struct amdgpu_vcn_inst *v;
> + int ret = 0, i;
>
> if (amdgpu_sriov_vf(adev))
> return 0;
>
> - if (state == vinst->cur_state)
> - return 0;
> + mutex_lock(&adev->vcn.global_pg_lock);
> + if (state == AMD_PG_STATE_GATE) {
> + if (adev->vcn.global_pg_count == 0)
> + goto unlock;
> + adev->vcn.global_pg_count--;
> + if (adev->vcn.global_pg_count == 0 &&
> + adev->vcn.global_pg_state == AMD_PG_STATE_UNGATE) {
> + for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> + v = &adev->vcn.inst[i];
> +
> + ret = vcn_v2_5_stop(v);
> + }
> + adev->vcn.global_pg_state = AMD_PG_STATE_GATE;
> + }
> + } else {
> + if (adev->vcn.global_pg_count == 0 &&
> + adev->vcn.global_pg_state == AMD_PG_STATE_GATE) {
> + for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> + v = &adev->vcn.inst[i];
>
> - if (state == AMD_PG_STATE_GATE)
> - ret = vcn_v2_5_stop(vinst);
> - else
> - ret = vcn_v2_5_start(vinst);
> + ret = vcn_v2_5_start(v);
> + }
> + adev->vcn.global_pg_state = AMD_PG_STATE_UNGATE;
> + }
> + adev->vcn.global_pg_count++;
> + }
>
> - if (!ret)
> - vinst->cur_state = state;
> +unlock:
> + vinst->cur_state = state;
> + mutex_unlock(&adev->vcn.global_pg_lock);
>
> return ret;
> }
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5
@ 2025-03-04 14:20 Alex Deucher
2025-03-04 14:37 ` Lazar, Lijo
0 siblings, 1 reply; 12+ messages in thread
From: Alex Deucher @ 2025-03-04 14:20 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher
VCN 2.5 uses the PG callback to enable VCN DPM which is
a global state. As such, we need to make sure all instances
are in the same state. Use amdgpu_device_ip_set_powergating_state()
rather than the per instance set_pg_state() callback.
Fixes: 4ce4fe27205c ("drm/amdgpu/vcn: use per instance callbacks for idle work handler")
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 8d8b39e6d197a..b93102e9fb43e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -437,7 +437,14 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
fences += fence[i];
if (!fences && !atomic_read(&vcn_inst->total_submission_cnt)) {
- vcn_inst->set_pg_state(vcn_inst, AMD_PG_STATE_GATE);
+ /* VCN 2.5 PG is actually DPM enablement which is global so
+ * update all instances
+ */
+ if (amdgpu_ip_version(adev, UVD_HWIP, 0) == IP_VERSION(2, 5, 0))
+ amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+ AMD_PG_STATE_GATE);
+ else
+ vcn_inst->set_pg_state(vcn_inst, AMD_PG_STATE_GATE);
r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
false);
if (r)
@@ -463,7 +470,14 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
}
mutex_lock(&vcn_inst->vcn_pg_lock);
- vcn_inst->set_pg_state(vcn_inst, AMD_PG_STATE_UNGATE);
+ /* VCN 2.5 PG is actually DPM enablement which is global so
+ * update all instances
+ */
+ if (amdgpu_ip_version(adev, UVD_HWIP, 0) == IP_VERSION(2, 5, 0))
+ amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+ AMD_PG_STATE_UNGATE);
+ else
+ vcn_inst->set_pg_state(vcn_inst, AMD_PG_STATE_UNGATE);
/* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
--
2.48.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5
2025-03-04 14:20 Alex Deucher
@ 2025-03-04 14:37 ` Lazar, Lijo
0 siblings, 0 replies; 12+ messages in thread
From: Lazar, Lijo @ 2025-03-04 14:37 UTC (permalink / raw)
To: Alex Deucher, amd-gfx
On 3/4/2025 7:50 PM, Alex Deucher wrote:
> VCN 2.5 uses the PG callback to enable VCN DPM which is
> a global state. As such, we need to make sure all instances
> are in the same state. Use amdgpu_device_ip_set_powergating_state()
> rather than the per instance set_pg_state() callback.
>
> Fixes: 4ce4fe27205c ("drm/amdgpu/vcn: use per instance callbacks for idle work handler")
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 8d8b39e6d197a..b93102e9fb43e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -437,7 +437,14 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
> fences += fence[i];
>
> if (!fences && !atomic_read(&vcn_inst->total_submission_cnt)) {
> - vcn_inst->set_pg_state(vcn_inst, AMD_PG_STATE_GATE);
> + /* VCN 2.5 PG is actually DPM enablement which is global so
> + * update all instances
> + */
> + if (amdgpu_ip_version(adev, UVD_HWIP, 0) == IP_VERSION(2, 5, 0))
> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> + AMD_PG_STATE_GATE);
What about moving this logic to vcn_v2_5_set_pg_state? Apart from that,
the number of outstanding fences are counted only for the corresponding
instance. Won't this be problematic when multiple instances are active
and one of them gets to idle? May need a refcount as well.
Thanks,
Lijo
> + else
> + vcn_inst->set_pg_state(vcn_inst, AMD_PG_STATE_GATE);
> r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_VIDEO,
> false);
> if (r)
> @@ -463,7 +470,14 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
> }
>
> mutex_lock(&vcn_inst->vcn_pg_lock);
> - vcn_inst->set_pg_state(vcn_inst, AMD_PG_STATE_UNGATE);
> + /* VCN 2.5 PG is actually DPM enablement which is global so
> + * update all instances
> + */
> + if (amdgpu_ip_version(adev, UVD_HWIP, 0) == IP_VERSION(2, 5, 0))
> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> + AMD_PG_STATE_UNGATE);
> + else
> + vcn_inst->set_pg_state(vcn_inst, AMD_PG_STATE_UNGATE);
>
> /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled by FW */
> if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-03-07 21:14 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-04 23:01 [PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5 Alex Deucher
2025-03-05 14:20 ` Boyuan Zhang
-- strict thread matches above, loose matches on Subject: below --
2025-03-05 19:17 Alex Deucher
2025-03-06 15:05 ` Alex Deucher
2025-03-07 15:22 ` Alex Deucher
2025-03-07 21:14 ` Zhang, Boyuan
2025-03-04 16:22 Alex Deucher
2025-03-04 20:52 ` Boyuan Zhang
2025-03-04 21:05 ` Alex Deucher
2025-03-04 23:12 ` Alex Deucher
2025-03-04 14:20 Alex Deucher
2025-03-04 14:37 ` Lazar, Lijo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox