* [PATCH 2/4] drm/amdgpu/vcn: fix race condition issue for dpg unpause mode switch
2020-03-03 18:16 [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start James Zhu
@ 2020-03-03 18:16 ` James Zhu
2020-03-09 16:58 ` [PATCH v3 " James Zhu
2020-03-11 14:16 ` [PATCH v4 " James Zhu
2020-03-03 18:16 ` [PATCH 3/4] drm/amdgpu/vcn2.0: stall DPG when WPTR/RPTR reset James Zhu
` (6 subsequent siblings)
7 siblings, 2 replies; 27+ messages in thread
From: James Zhu @ 2020-03-03 18:16 UTC (permalink / raw)
To: amd-gfx; +Cc: jamesz
Couldn't only rely on enc fence to decide switching to dpg unpaude mode.
Since a enc thread may not schedule a fence in time during multiple
threads running situation.
Signed-off-by: James Zhu <James.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 28 ++++++++++++++++++----------
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
2 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index aa7663f..74cefc7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -64,6 +64,8 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
mutex_init(&adev->vcn.vcn_pg_lock);
+ for (i = 0; i < adev->vcn.num_vcn_inst; i++)
+ atomic_set(&adev->vcn.inst[i].enc_submission_cnt, 0);
switch (adev->asic_type) {
case CHIP_RAVEN:
@@ -332,19 +334,22 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
struct dpg_pause_state new_state;
- unsigned int fences = 0;
- unsigned int i;
- for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
- fences += amdgpu_fence_count_emitted(&adev->vcn.inst[ring->me].ring_enc[i]);
- }
- if (fences)
+ if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC) {
+ atomic_inc(&adev->vcn.inst[ring->me].enc_submission_cnt);
new_state.fw_based = VCN_DPG_STATE__PAUSE;
- else
- new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+ } else {
+ unsigned int fences = 0;
+ unsigned int i;
- if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
- new_state.fw_based = VCN_DPG_STATE__PAUSE;
+ for (i = 0; i < adev->vcn.num_enc_rings; ++i)
+ fences += amdgpu_fence_count_emitted(&adev->vcn.inst[ring->me].ring_enc[i]);
+
+ if (fences || atomic_read(&adev->vcn.inst[ring->me].enc_submission_cnt))
+ new_state.fw_based = VCN_DPG_STATE__PAUSE;
+ else
+ new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+ }
adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
}
@@ -354,6 +359,9 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
{
schedule_delayed_work(&ring->adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
+ if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC &&
+ atomic_dec_return(&ring->adev->vcn.inst[ring->me].enc_submission_cnt) < 0)
+ atomic_set(&ring->adev->vcn.inst[ring->me].enc_submission_cnt, 0);
}
int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 2ae110d..4ca76c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -183,6 +183,7 @@ struct amdgpu_vcn_inst {
void *dpg_sram_cpu_addr;
uint64_t dpg_sram_gpu_addr;
uint32_t *dpg_sram_curr_addr;
+ atomic_t enc_submission_cnt;
};
struct amdgpu_vcn {
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v3 2/4] drm/amdgpu/vcn: fix race condition issue for dpg unpause mode switch
2020-03-03 18:16 ` [PATCH 2/4] drm/amdgpu/vcn: fix race condition issue for dpg unpause mode switch James Zhu
@ 2020-03-09 16:58 ` James Zhu
2020-03-11 14:16 ` [PATCH v4 " James Zhu
1 sibling, 0 replies; 27+ messages in thread
From: James Zhu @ 2020-03-09 16:58 UTC (permalink / raw)
To: amd-gfx; +Cc: jamesz
Couldn't only rely on enc fence to decide switching to dpg unpaude mode.
Since a enc thread may not schedule a fence in time during multiple
threads running situation.
v3: 1. Rename enc_submission_cnt to dpg_enc_submission_cnt
2. Add dpg_enc_submission_cnt check in idle_work_handler
Signed-off-by: James Zhu <James.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 33 ++++++++++++++++++++++-----------
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 6aafda1..8b48f18 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -65,6 +65,8 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
mutex_init(&adev->vcn.vcn_pg_lock);
atomic_set(&adev->vcn.total_submission_cnt, 0);
+ for (i = 0; i < adev->vcn.num_vcn_inst; i++)
+ atomic_set(&adev->vcn.inst[i].dpg_enc_submission_cnt, 0);
switch (adev->asic_type) {
case CHIP_RAVEN:
@@ -298,7 +300,8 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
struct dpg_pause_state new_state;
- if (fence[j])
+ if (fence[j] ||
+ unlikely(atomic_read(&adev->vcn.inst[j].dpg_enc_submission_cnt)))
new_state.fw_based = VCN_DPG_STATE__PAUSE;
else
new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
@@ -334,19 +337,22 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
struct dpg_pause_state new_state;
- unsigned int fences = 0;
- unsigned int i;
- for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
- fences += amdgpu_fence_count_emitted(&adev->vcn.inst[ring->me].ring_enc[i]);
- }
- if (fences)
+ if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC) {
+ atomic_inc(&adev->vcn.inst[ring->me].dpg_enc_submission_cnt);
new_state.fw_based = VCN_DPG_STATE__PAUSE;
- else
- new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+ } else {
+ unsigned int fences = 0;
+ unsigned int i;
- if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
- new_state.fw_based = VCN_DPG_STATE__PAUSE;
+ for (i = 0; i < adev->vcn.num_enc_rings; ++i)
+ fences += amdgpu_fence_count_emitted(&adev->vcn.inst[ring->me].ring_enc[i]);
+
+ if (fences || atomic_read(&adev->vcn.inst[ring->me].dpg_enc_submission_cnt))
+ new_state.fw_based = VCN_DPG_STATE__PAUSE;
+ else
+ new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+ }
adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
}
@@ -356,6 +362,11 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
{
schedule_delayed_work(&ring->adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
+
+ if (ring->adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
+ ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC &&
+ unlikely(atomic_dec_return(&ring->adev->vcn.inst[ring->me].dpg_enc_submission_cnt) < 0))
+ atomic_set(&ring->adev->vcn.inst[ring->me].dpg_enc_submission_cnt, 0);
if (unlikely(atomic_dec_return(&ring->adev->vcn.total_submission_cnt) < 0))
atomic_set(&ring->adev->vcn.total_submission_cnt, 0);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 111c4cc..e913de8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -183,6 +183,7 @@ struct amdgpu_vcn_inst {
void *dpg_sram_cpu_addr;
uint64_t dpg_sram_gpu_addr;
uint32_t *dpg_sram_curr_addr;
+ atomic_t dpg_enc_submission_cnt;
};
struct amdgpu_vcn {
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v4 2/4] drm/amdgpu/vcn: fix race condition issue for dpg unpause mode switch
2020-03-03 18:16 ` [PATCH 2/4] drm/amdgpu/vcn: fix race condition issue for dpg unpause mode switch James Zhu
2020-03-09 16:58 ` [PATCH v3 " James Zhu
@ 2020-03-11 14:16 ` James Zhu
1 sibling, 0 replies; 27+ messages in thread
From: James Zhu @ 2020-03-11 14:16 UTC (permalink / raw)
To: amd-gfx; +Cc: jamesz
Couldn't only rely on enc fence to decide switching to dpg unpaude mode.
Since a enc thread may not schedule a fence in time during multiple
threads running situation.
v3: 1. Rename enc_submission_cnt to dpg_enc_submission_cnt
2. Add dpg_enc_submission_cnt check in idle_work_handler
v4: Remove extra counter check, and reduce counter before idle
work schedule
Signed-off-by: James Zhu <James.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 32 +++++++++++++++++++++-----------
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 2fa2891..ba28fb9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -65,6 +65,8 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
mutex_init(&adev->vcn.vcn_pg_lock);
atomic_set(&adev->vcn.total_submission_cnt, 0);
+ for (i = 0; i < adev->vcn.num_vcn_inst; i++)
+ atomic_set(&adev->vcn.inst[i].dpg_enc_submission_cnt, 0);
switch (adev->asic_type) {
case CHIP_RAVEN:
@@ -298,7 +300,8 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
struct dpg_pause_state new_state;
- if (fence[j])
+ if (fence[j] ||
+ unlikely(atomic_read(&adev->vcn.inst[j].dpg_enc_submission_cnt)))
new_state.fw_based = VCN_DPG_STATE__PAUSE;
else
new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
@@ -334,19 +337,22 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
struct dpg_pause_state new_state;
- unsigned int fences = 0;
- unsigned int i;
- for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
- fences += amdgpu_fence_count_emitted(&adev->vcn.inst[ring->me].ring_enc[i]);
- }
- if (fences)
+ if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC) {
+ atomic_inc(&adev->vcn.inst[ring->me].dpg_enc_submission_cnt);
new_state.fw_based = VCN_DPG_STATE__PAUSE;
- else
- new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+ } else {
+ unsigned int fences = 0;
+ unsigned int i;
- if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
- new_state.fw_based = VCN_DPG_STATE__PAUSE;
+ for (i = 0; i < adev->vcn.num_enc_rings; ++i)
+ fences += amdgpu_fence_count_emitted(&adev->vcn.inst[ring->me].ring_enc[i]);
+
+ if (fences || atomic_read(&adev->vcn.inst[ring->me].dpg_enc_submission_cnt))
+ new_state.fw_based = VCN_DPG_STATE__PAUSE;
+ else
+ new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
+ }
adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
}
@@ -355,6 +361,10 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
{
+ if (ring->adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
+ ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
+ atomic_dec(&ring->adev->vcn.inst[ring->me].dpg_enc_submission_cnt);
+
atomic_dec(&ring->adev->vcn.total_submission_cnt);
schedule_delayed_work(&ring->adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 111c4cc..e913de8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -183,6 +183,7 @@ struct amdgpu_vcn_inst {
void *dpg_sram_cpu_addr;
uint64_t dpg_sram_gpu_addr;
uint32_t *dpg_sram_curr_addr;
+ atomic_t dpg_enc_submission_cnt;
};
struct amdgpu_vcn {
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/4] drm/amdgpu/vcn2.0: stall DPG when WPTR/RPTR reset
2020-03-03 18:16 [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start James Zhu
2020-03-03 18:16 ` [PATCH 2/4] drm/amdgpu/vcn: fix race condition issue for dpg unpause mode switch James Zhu
@ 2020-03-03 18:16 ` James Zhu
2020-03-03 18:16 ` [PATCH 4/4] drm/amdgpu/vcn2.5: " James Zhu
` (5 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: James Zhu @ 2020-03-03 18:16 UTC (permalink / raw)
To: amd-gfx; +Cc: jamesz
Add vcn dpg harware and firmware synchronization to fix race condition
issue among vcn driver, hardware and firmware
Signed-off-by: James Zhu <James.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index c387c81..7719f56 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -1158,8 +1158,12 @@ static int vcn_v2_0_pause_dpg_mode(struct amdgpu_device *adev,
UVD_DPG_PAUSE__NJ_PAUSE_DPG_ACK_MASK,
UVD_DPG_PAUSE__NJ_PAUSE_DPG_ACK_MASK, ret_code);
+
+ /* Stall DPG before WPTR/RPTR reset */
+ WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_POWER_STATUS), UVD_POWER_STATUS__STALL_DPG_POWER_UP_MASK, ~UVD_POWER_STATUS__STALL_DPG_POWER_UP_MASK);
/* Restore */
ring = &adev->vcn.inst->ring_enc[0];
+ ring->wptr = 0;
WREG32_SOC15(UVD, 0, mmUVD_RB_BASE_LO, ring->gpu_addr);
WREG32_SOC15(UVD, 0, mmUVD_RB_BASE_HI, upper_32_bits(ring->gpu_addr));
WREG32_SOC15(UVD, 0, mmUVD_RB_SIZE, ring->ring_size / 4);
@@ -1167,6 +1171,7 @@ static int vcn_v2_0_pause_dpg_mode(struct amdgpu_device *adev,
WREG32_SOC15(UVD, 0, mmUVD_RB_WPTR, lower_32_bits(ring->wptr));
ring = &adev->vcn.inst->ring_enc[1];
+ ring->wptr = 0;
WREG32_SOC15(UVD, 0, mmUVD_RB_BASE_LO2, ring->gpu_addr);
WREG32_SOC15(UVD, 0, mmUVD_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
WREG32_SOC15(UVD, 0, mmUVD_RB_SIZE2, ring->ring_size / 4);
@@ -1175,6 +1180,8 @@ static int vcn_v2_0_pause_dpg_mode(struct amdgpu_device *adev,
WREG32_SOC15(UVD, 0, mmUVD_RBC_RB_WPTR,
RREG32_SOC15(UVD, 0, mmUVD_SCRATCH2) & 0x7FFFFFFF);
+ /* Unstall DPG */
+ WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_POWER_STATUS), 0, ~UVD_POWER_STATUS__STALL_DPG_POWER_UP_MASK);
SOC15_WAIT_ON_RREG(UVD, 0, mmUVD_POWER_STATUS,
UVD_PGFSM_CONFIG__UVDM_UVDU_PWR_ON,
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 4/4] drm/amdgpu/vcn2.5: stall DPG when WPTR/RPTR reset
2020-03-03 18:16 [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start James Zhu
2020-03-03 18:16 ` [PATCH 2/4] drm/amdgpu/vcn: fix race condition issue for dpg unpause mode switch James Zhu
2020-03-03 18:16 ` [PATCH 3/4] drm/amdgpu/vcn2.0: stall DPG when WPTR/RPTR reset James Zhu
@ 2020-03-03 18:16 ` James Zhu
2020-03-10 19:58 ` [PATCH v2 4/4] drm/amdgpu/vcn2.5: add sync " James Zhu
2020-03-03 18:44 ` [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start Christian König
` (4 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: James Zhu @ 2020-03-03 18:16 UTC (permalink / raw)
To: amd-gfx; +Cc: jamesz
Add vcn dpg harware and firmware synchronization to fix race condition
issue among vcn driver, hardware and firmware
Signed-off-by: James Zhu <James.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
index 2d64ba1..189c816 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -1388,8 +1388,11 @@ static int vcn_v2_5_pause_dpg_mode(struct amdgpu_device *adev,
UVD_DPG_PAUSE__NJ_PAUSE_DPG_ACK_MASK,
UVD_DPG_PAUSE__NJ_PAUSE_DPG_ACK_MASK, ret_code);
+ /* Stall DPG before WPTR/RPTR reset */
+ WREG32_P(SOC15_REG_OFFSET(UVD, inst_idx, mmUVD_POWER_STATUS), UVD_POWER_STATUS__STALL_DPG_POWER_UP_MASK, ~UVD_POWER_STATUS__STALL_DPG_POWER_UP_MASK);
/* Restore */
ring = &adev->vcn.inst[inst_idx].ring_enc[0];
+ ring->wptr = 0;
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_BASE_LO, ring->gpu_addr);
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_BASE_HI, upper_32_bits(ring->gpu_addr));
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_SIZE, ring->ring_size / 4);
@@ -1397,6 +1400,7 @@ static int vcn_v2_5_pause_dpg_mode(struct amdgpu_device *adev,
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_WPTR, lower_32_bits(ring->wptr));
ring = &adev->vcn.inst[inst_idx].ring_enc[1];
+ ring->wptr = 0;
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_BASE_LO2, ring->gpu_addr);
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_SIZE2, ring->ring_size / 4);
@@ -1405,6 +1409,8 @@ static int vcn_v2_5_pause_dpg_mode(struct amdgpu_device *adev,
WREG32_SOC15(UVD, inst_idx, mmUVD_RBC_RB_WPTR,
RREG32_SOC15(UVD, inst_idx, mmUVD_SCRATCH2) & 0x7FFFFFFF);
+ /* Unstall DPG */
+ WREG32_P(SOC15_REG_OFFSET(UVD, inst_idx, mmUVD_POWER_STATUS), 0, ~UVD_POWER_STATUS__STALL_DPG_POWER_UP_MASK);
SOC15_WAIT_ON_RREG(UVD, inst_idx, mmUVD_POWER_STATUS,
UVD_PGFSM_CONFIG__UVDM_UVDU_PWR_ON, UVD_POWER_STATUS__UVD_POWER_STATUS_MASK, ret_code);
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 4/4] drm/amdgpu/vcn2.5: add sync when WPTR/RPTR reset
2020-03-03 18:16 ` [PATCH 4/4] drm/amdgpu/vcn2.5: " James Zhu
@ 2020-03-10 19:58 ` James Zhu
2020-03-10 20:03 ` Leo Liu
0 siblings, 1 reply; 27+ messages in thread
From: James Zhu @ 2020-03-10 19:58 UTC (permalink / raw)
To: amd-gfx; +Cc: jamesz
Add vcn harware and firmware synchronization to fix race condition
issue among vcn driver, hardware and firmware
v2: WA: Add scratch 3 to sync with vcn firmware during W/R pointer reset
Signed-off-by: James Zhu <James.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
index 2d64ba1..9480039 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -1034,6 +1034,9 @@ static int vcn_v2_5_start(struct amdgpu_device *adev)
tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL, RB_RPTR_WR_EN, 1);
WREG32_SOC15(UVD, i, mmUVD_RBC_RB_CNTL, tmp);
+ /* Set scratch3 to start dec/enc registers reset */
+ WREG32_SOC15(UVD, i, mmUVD_SCRATCH3, 1);
+
/* programm the RB_BASE for ring buffer */
WREG32_SOC15(UVD, i, mmUVD_LMI_RBC_RB_64BIT_BAR_LOW,
lower_32_bits(ring->gpu_addr));
@@ -1059,6 +1062,9 @@ static int vcn_v2_5_start(struct amdgpu_device *adev)
WREG32_SOC15(UVD, i, mmUVD_RB_BASE_LO2, ring->gpu_addr);
WREG32_SOC15(UVD, i, mmUVD_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
WREG32_SOC15(UVD, i, mmUVD_RB_SIZE2, ring->ring_size / 4);
+
+ /* Clear scratch3 to finish dec/enc registers reset */
+ WREG32_SOC15(UVD, i, mmUVD_SCRATCH3, 0);
}
return 0;
@@ -1388,8 +1394,11 @@ static int vcn_v2_5_pause_dpg_mode(struct amdgpu_device *adev,
UVD_DPG_PAUSE__NJ_PAUSE_DPG_ACK_MASK,
UVD_DPG_PAUSE__NJ_PAUSE_DPG_ACK_MASK, ret_code);
+ /* Stall DPG before WPTR/RPTR reset */
+ WREG32_P(SOC15_REG_OFFSET(UVD, inst_idx, mmUVD_POWER_STATUS), UVD_POWER_STATUS__STALL_DPG_POWER_UP_MASK, ~UVD_POWER_STATUS__STALL_DPG_POWER_UP_MASK);
/* Restore */
ring = &adev->vcn.inst[inst_idx].ring_enc[0];
+ ring->wptr = 0;
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_BASE_LO, ring->gpu_addr);
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_BASE_HI, upper_32_bits(ring->gpu_addr));
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_SIZE, ring->ring_size / 4);
@@ -1397,6 +1406,7 @@ static int vcn_v2_5_pause_dpg_mode(struct amdgpu_device *adev,
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_WPTR, lower_32_bits(ring->wptr));
ring = &adev->vcn.inst[inst_idx].ring_enc[1];
+ ring->wptr = 0;
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_BASE_LO2, ring->gpu_addr);
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
WREG32_SOC15(UVD, inst_idx, mmUVD_RB_SIZE2, ring->ring_size / 4);
@@ -1405,6 +1415,8 @@ static int vcn_v2_5_pause_dpg_mode(struct amdgpu_device *adev,
WREG32_SOC15(UVD, inst_idx, mmUVD_RBC_RB_WPTR,
RREG32_SOC15(UVD, inst_idx, mmUVD_SCRATCH2) & 0x7FFFFFFF);
+ /* Unstall DPG */
+ WREG32_P(SOC15_REG_OFFSET(UVD, inst_idx, mmUVD_POWER_STATUS), 0, ~UVD_POWER_STATUS__STALL_DPG_POWER_UP_MASK);
SOC15_WAIT_ON_RREG(UVD, inst_idx, mmUVD_POWER_STATUS,
UVD_PGFSM_CONFIG__UVDM_UVDU_PWR_ON, UVD_POWER_STATUS__UVD_POWER_STATUS_MASK, ret_code);
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v2 4/4] drm/amdgpu/vcn2.5: add sync when WPTR/RPTR reset
2020-03-10 19:58 ` [PATCH v2 4/4] drm/amdgpu/vcn2.5: add sync " James Zhu
@ 2020-03-10 20:03 ` Leo Liu
0 siblings, 0 replies; 27+ messages in thread
From: Leo Liu @ 2020-03-10 20:03 UTC (permalink / raw)
To: James Zhu, amd-gfx; +Cc: jamesz
On 2020-03-10 3:58 p.m., James Zhu wrote:
> Add vcn harware and firmware synchronization to fix race condition
> issue among vcn driver, hardware and firmware
>
> v2: WA: Add scratch 3 to sync with vcn firmware during W/R pointer reset
>
> Signed-off-by: James Zhu <James.Zhu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> index 2d64ba1..9480039 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> @@ -1034,6 +1034,9 @@ static int vcn_v2_5_start(struct amdgpu_device *adev)
> tmp = REG_SET_FIELD(tmp, UVD_RBC_RB_CNTL, RB_RPTR_WR_EN, 1);
> WREG32_SOC15(UVD, i, mmUVD_RBC_RB_CNTL, tmp);
>
> + /* Set scratch3 to start dec/enc registers reset */
> + WREG32_SOC15(UVD, i, mmUVD_SCRATCH3, 1);
> +
> /* programm the RB_BASE for ring buffer */
> WREG32_SOC15(UVD, i, mmUVD_LMI_RBC_RB_64BIT_BAR_LOW,
> lower_32_bits(ring->gpu_addr));
> @@ -1059,6 +1062,9 @@ static int vcn_v2_5_start(struct amdgpu_device *adev)
> WREG32_SOC15(UVD, i, mmUVD_RB_BASE_LO2, ring->gpu_addr);
> WREG32_SOC15(UVD, i, mmUVD_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
> WREG32_SOC15(UVD, i, mmUVD_RB_SIZE2, ring->ring_size / 4);
> +
> + /* Clear scratch3 to finish dec/enc registers reset */
> + WREG32_SOC15(UVD, i, mmUVD_SCRATCH3, 0);
> }
>
> return 0;
> @@ -1388,8 +1394,11 @@ static int vcn_v2_5_pause_dpg_mode(struct amdgpu_device *adev,
> UVD_DPG_PAUSE__NJ_PAUSE_DPG_ACK_MASK,
> UVD_DPG_PAUSE__NJ_PAUSE_DPG_ACK_MASK, ret_code);
>
> + /* Stall DPG before WPTR/RPTR reset */
> + WREG32_P(SOC15_REG_OFFSET(UVD, inst_idx, mmUVD_POWER_STATUS), UVD_POWER_STATUS__STALL_DPG_POWER_UP_MASK, ~UVD_POWER_STATUS__STALL_DPG_POWER_UP_MASK);
You can wrap up the line shorter? With that fixed, this patch is
Reviewed-by: Leo Liu <leo.liu@amd.com>
Leo
> /* Restore */
> ring = &adev->vcn.inst[inst_idx].ring_enc[0];
> + ring->wptr = 0;
> WREG32_SOC15(UVD, inst_idx, mmUVD_RB_BASE_LO, ring->gpu_addr);
> WREG32_SOC15(UVD, inst_idx, mmUVD_RB_BASE_HI, upper_32_bits(ring->gpu_addr));
> WREG32_SOC15(UVD, inst_idx, mmUVD_RB_SIZE, ring->ring_size / 4);
> @@ -1397,6 +1406,7 @@ static int vcn_v2_5_pause_dpg_mode(struct amdgpu_device *adev,
> WREG32_SOC15(UVD, inst_idx, mmUVD_RB_WPTR, lower_32_bits(ring->wptr));
>
> ring = &adev->vcn.inst[inst_idx].ring_enc[1];
> + ring->wptr = 0;
> WREG32_SOC15(UVD, inst_idx, mmUVD_RB_BASE_LO2, ring->gpu_addr);
> WREG32_SOC15(UVD, inst_idx, mmUVD_RB_BASE_HI2, upper_32_bits(ring->gpu_addr));
> WREG32_SOC15(UVD, inst_idx, mmUVD_RB_SIZE2, ring->ring_size / 4);
> @@ -1405,6 +1415,8 @@ static int vcn_v2_5_pause_dpg_mode(struct amdgpu_device *adev,
>
> WREG32_SOC15(UVD, inst_idx, mmUVD_RBC_RB_WPTR,
> RREG32_SOC15(UVD, inst_idx, mmUVD_SCRATCH2) & 0x7FFFFFFF);
> + /* Unstall DPG */
> + WREG32_P(SOC15_REG_OFFSET(UVD, inst_idx, mmUVD_POWER_STATUS), 0, ~UVD_POWER_STATUS__STALL_DPG_POWER_UP_MASK);
>
> SOC15_WAIT_ON_RREG(UVD, inst_idx, mmUVD_POWER_STATUS,
> UVD_PGFSM_CONFIG__UVDM_UVDU_PWR_ON, UVD_POWER_STATUS__UVD_POWER_STATUS_MASK, ret_code);
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-03 18:16 [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start James Zhu
` (2 preceding siblings ...)
2020-03-03 18:16 ` [PATCH 4/4] drm/amdgpu/vcn2.5: " James Zhu
@ 2020-03-03 18:44 ` Christian König
2020-03-03 19:03 ` James Zhu
2020-03-04 16:34 ` [PATCH v2 " James Zhu
` (3 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: Christian König @ 2020-03-03 18:44 UTC (permalink / raw)
To: James Zhu, amd-gfx; +Cc: jamesz
Am 03.03.20 um 19:16 schrieb James Zhu:
> Fix race condition issue when multiple vcn starts are called.
>
> Signed-off-by: James Zhu <James.Zhu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 ++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
> 2 files changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index f96464e..aa7663f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
> int i, r;
>
> INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
> + mutex_init(&adev->vcn.vcn_pg_lock);
>
> switch (adev->asic_type) {
> case CHIP_RAVEN:
> @@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
> }
>
> release_firmware(adev->vcn.fw);
> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>
> return 0;
> }
> @@ -321,6 +323,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
> struct amdgpu_device *adev = ring->adev;
> bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
>
> + mutex_lock(&adev->vcn.vcn_pg_lock);
That still won't work correctly here.
The whole idea of the cancel_delayed_work_sync() and
schedule_delayed_work() dance is that you have exactly one user of that.
If you have multiple rings that whole thing won't work correctly.
To fix this you need to call mutex_lock() before
cancel_delayed_work_sync() and schedule_delayed_work() before
mutex_unlock().
Regards,
Christian.
> if (set_clocks) {
> amdgpu_gfx_off_ctrl(adev, false);
> amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> @@ -345,6 +348,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
>
> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
> }
> + mutex_unlock(&adev->vcn.vcn_pg_lock);
> }
>
> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 6fe0573..2ae110d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -200,6 +200,7 @@ struct amdgpu_vcn {
> struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
> uint32_t num_vcn_enc_sched;
> uint32_t num_vcn_dec_sched;
> + struct mutex vcn_pg_lock;
>
> unsigned harvest_config;
> int (*pause_dpg_mode)(struct amdgpu_device *adev,
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-03 18:44 ` [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start Christian König
@ 2020-03-03 19:03 ` James Zhu
2020-03-03 22:48 ` James Zhu
0 siblings, 1 reply; 27+ messages in thread
From: James Zhu @ 2020-03-03 19:03 UTC (permalink / raw)
To: christian.koenig, James Zhu, amd-gfx
On 2020-03-03 1:44 p.m., Christian König wrote:
> Am 03.03.20 um 19:16 schrieb James Zhu:
>> Fix race condition issue when multiple vcn starts are called.
>>
>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 ++++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
>> 2 files changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> index f96464e..aa7663f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> @@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>> int i, r;
>> INIT_DELAYED_WORK(&adev->vcn.idle_work,
>> amdgpu_vcn_idle_work_handler);
>> + mutex_init(&adev->vcn.vcn_pg_lock);
>> switch (adev->asic_type) {
>> case CHIP_RAVEN:
>> @@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
>> }
>> release_firmware(adev->vcn.fw);
>> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>> return 0;
>> }
>> @@ -321,6 +323,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring
>> *ring)
>> struct amdgpu_device *adev = ring->adev;
>> bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
>> + mutex_lock(&adev->vcn.vcn_pg_lock);
>
> That still won't work correctly here.
>
> The whole idea of the cancel_delayed_work_sync() and
> schedule_delayed_work() dance is that you have exactly one user of
> that. If you have multiple rings that whole thing won't work correctly.
>
> To fix this you need to call mutex_lock() before
> cancel_delayed_work_sync() and schedule_delayed_work() before
> mutex_unlock().
Big lock definitely works. I am trying to use as smaller lock as
possible here. the share resource which needs protect here are power
gate process and dpg mode switch process.
if we move mutex_unlock() before schedule_delayed_work(. I am wondering
what are the other necessary resources which need protect.
Thanks!
James
>
> Regards,
> Christian.
>
>> if (set_clocks) {
>> amdgpu_gfx_off_ctrl(adev, false);
>> amdgpu_device_ip_set_powergating_state(adev,
>> AMD_IP_BLOCK_TYPE_VCN,
>> @@ -345,6 +348,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring
>> *ring)
>> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
>> }
>> + mutex_unlock(&adev->vcn.vcn_pg_lock);
>> }
>> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> index 6fe0573..2ae110d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> @@ -200,6 +200,7 @@ struct amdgpu_vcn {
>> struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
>> uint32_t num_vcn_enc_sched;
>> uint32_t num_vcn_dec_sched;
>> + struct mutex vcn_pg_lock;
>> unsigned harvest_config;
>> int (*pause_dpg_mode)(struct amdgpu_device *adev,
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-03 19:03 ` James Zhu
@ 2020-03-03 22:48 ` James Zhu
2020-03-04 8:53 ` Christian König
0 siblings, 1 reply; 27+ messages in thread
From: James Zhu @ 2020-03-03 22:48 UTC (permalink / raw)
To: christian.koenig, James Zhu, amd-gfx
On 2020-03-03 2:03 p.m., James Zhu wrote:
>
> On 2020-03-03 1:44 p.m., Christian König wrote:
>> Am 03.03.20 um 19:16 schrieb James Zhu:
>>> Fix race condition issue when multiple vcn starts are called.
>>>
>>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 ++++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
>>> 2 files changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> index f96464e..aa7663f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> @@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>>> int i, r;
>>> INIT_DELAYED_WORK(&adev->vcn.idle_work,
>>> amdgpu_vcn_idle_work_handler);
>>> + mutex_init(&adev->vcn.vcn_pg_lock);
>>> switch (adev->asic_type) {
>>> case CHIP_RAVEN:
>>> @@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
>>> }
>>> release_firmware(adev->vcn.fw);
>>> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>>> return 0;
>>> }
>>> @@ -321,6 +323,7 @@ void amdgpu_vcn_ring_begin_use(struct
>>> amdgpu_ring *ring)
>>> struct amdgpu_device *adev = ring->adev;
>>> bool set_clocks =
>>> !cancel_delayed_work_sync(&adev->vcn.idle_work);
>>> + mutex_lock(&adev->vcn.vcn_pg_lock);
>>
>> That still won't work correctly here.
>>
>> The whole idea of the cancel_delayed_work_sync() and
>> schedule_delayed_work() dance is that you have exactly one user of
>> that. If you have multiple rings that whole thing won't work correctly.
>>
>> To fix this you need to call mutex_lock() before
>> cancel_delayed_work_sync() and schedule_delayed_work() before
>> mutex_unlock().
>
> Big lock definitely works. I am trying to use as smaller lock as
> possible here. the share resource which needs protect here are power
> gate process and dpg mode switch process.
>
> if we move mutex_unlock() before schedule_delayed_work(. I am
> wondering what are the other necessary resources which need protect.
By the way, cancel_delayed_work_sync() supports multiple thread itself,
so I didn't put it into protection area. power gate is shared by all
VCN IP instances and different rings , so it needs be put into
protection area.
each ring's job itself is serialized by scheduler. it doesn't need be
put into this protection area.
>
> Thanks!
>
> James
>
>>
>> Regards,
>> Christian.
>>
>>> if (set_clocks) {
>>> amdgpu_gfx_off_ctrl(adev, false);
>>> amdgpu_device_ip_set_powergating_state(adev,
>>> AMD_IP_BLOCK_TYPE_VCN,
>>> @@ -345,6 +348,7 @@ void amdgpu_vcn_ring_begin_use(struct
>>> amdgpu_ring *ring)
>>> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
>>> }
>>> + mutex_unlock(&adev->vcn.vcn_pg_lock);
>>> }
>>> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>> index 6fe0573..2ae110d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>> @@ -200,6 +200,7 @@ struct amdgpu_vcn {
>>> struct drm_gpu_scheduler
>>> *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
>>> uint32_t num_vcn_enc_sched;
>>> uint32_t num_vcn_dec_sched;
>>> + struct mutex vcn_pg_lock;
>>> unsigned harvest_config;
>>> int (*pause_dpg_mode)(struct amdgpu_device *adev,
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-03 22:48 ` James Zhu
@ 2020-03-04 8:53 ` Christian König
2020-03-04 14:57 ` James Zhu
0 siblings, 1 reply; 27+ messages in thread
From: Christian König @ 2020-03-04 8:53 UTC (permalink / raw)
To: James Zhu, James Zhu, amd-gfx
Am 03.03.20 um 23:48 schrieb James Zhu:
>
> On 2020-03-03 2:03 p.m., James Zhu wrote:
>>
>> On 2020-03-03 1:44 p.m., Christian König wrote:
>>> Am 03.03.20 um 19:16 schrieb James Zhu:
>>>> Fix race condition issue when multiple vcn starts are called.
>>>>
>>>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 ++++
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
>>>> 2 files changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>> index f96464e..aa7663f 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>> @@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>>>> int i, r;
>>>> INIT_DELAYED_WORK(&adev->vcn.idle_work,
>>>> amdgpu_vcn_idle_work_handler);
>>>> + mutex_init(&adev->vcn.vcn_pg_lock);
>>>> switch (adev->asic_type) {
>>>> case CHIP_RAVEN:
>>>> @@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
>>>> }
>>>> release_firmware(adev->vcn.fw);
>>>> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>>>> return 0;
>>>> }
>>>> @@ -321,6 +323,7 @@ void amdgpu_vcn_ring_begin_use(struct
>>>> amdgpu_ring *ring)
>>>> struct amdgpu_device *adev = ring->adev;
>>>> bool set_clocks =
>>>> !cancel_delayed_work_sync(&adev->vcn.idle_work);
>>>> + mutex_lock(&adev->vcn.vcn_pg_lock);
>>>
>>> That still won't work correctly here.
>>>
>>> The whole idea of the cancel_delayed_work_sync() and
>>> schedule_delayed_work() dance is that you have exactly one user of
>>> that. If you have multiple rings that whole thing won't work correctly.
>>>
>>> To fix this you need to call mutex_lock() before
>>> cancel_delayed_work_sync() and schedule_delayed_work() before
>>> mutex_unlock().
>>
>> Big lock definitely works. I am trying to use as smaller lock as
>> possible here. the share resource which needs protect here are power
>> gate process and dpg mode switch process.
>>
>> if we move mutex_unlock() before schedule_delayed_work(. I am
>> wondering what are the other necessary resources which need protect.
>
> By the way, cancel_delayed_work_sync() supports multiple thread
> itself, so I didn't put it into protection area.
Yeah, but that's correct but it still won't working correctly :)
See the problem is that only for the first caller
cancel_delayed_work_sync() returns true because it canceled the delayed
work.
For all others it returns false and those would then think that they
need to ungate the power.
The only solution I see is to either put both the
cancel_delayed_work_sync() and schedule_delayed_work() under the same
mutex protection or start to use an atomic or other counter to note
concurrent processing.
> power gate is shared by all VCN IP instances and different rings , so
> it needs be put into protection area.
>
> each ring's job itself is serialized by scheduler. it doesn't need be
> put into this protection area.
Yes, those should work as expected.
Regards,
Christian.
>
>>
>> Thanks!
>>
>> James
>>
>>>
>>> Regards,
>>> Christian.
>>>
>>>> if (set_clocks) {
>>>> amdgpu_gfx_off_ctrl(adev, false);
>>>> amdgpu_device_ip_set_powergating_state(adev,
>>>> AMD_IP_BLOCK_TYPE_VCN,
>>>> @@ -345,6 +348,7 @@ void amdgpu_vcn_ring_begin_use(struct
>>>> amdgpu_ring *ring)
>>>> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
>>>> }
>>>> + mutex_unlock(&adev->vcn.vcn_pg_lock);
>>>> }
>>>> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>> index 6fe0573..2ae110d 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>> @@ -200,6 +200,7 @@ struct amdgpu_vcn {
>>>> struct drm_gpu_scheduler
>>>> *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
>>>> uint32_t num_vcn_enc_sched;
>>>> uint32_t num_vcn_dec_sched;
>>>> + struct mutex vcn_pg_lock;
>>>> unsigned harvest_config;
>>>> int (*pause_dpg_mode)(struct amdgpu_device *adev,
>>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-04 8:53 ` Christian König
@ 2020-03-04 14:57 ` James Zhu
2020-03-04 15:03 ` Christian König
0 siblings, 1 reply; 27+ messages in thread
From: James Zhu @ 2020-03-04 14:57 UTC (permalink / raw)
To: Christian König, James Zhu, amd-gfx
On 2020-03-04 3:53 a.m., Christian König wrote:
> Am 03.03.20 um 23:48 schrieb James Zhu:
>>
>> On 2020-03-03 2:03 p.m., James Zhu wrote:
>>>
>>> On 2020-03-03 1:44 p.m., Christian König wrote:
>>>> Am 03.03.20 um 19:16 schrieb James Zhu:
>>>>> Fix race condition issue when multiple vcn starts are called.
>>>>>
>>>>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>>>>> ---
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 ++++
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
>>>>> 2 files changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>>> index f96464e..aa7663f 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>>> @@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>>>>> int i, r;
>>>>> INIT_DELAYED_WORK(&adev->vcn.idle_work,
>>>>> amdgpu_vcn_idle_work_handler);
>>>>> + mutex_init(&adev->vcn.vcn_pg_lock);
>>>>> switch (adev->asic_type) {
>>>>> case CHIP_RAVEN:
>>>>> @@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device
>>>>> *adev)
>>>>> }
>>>>> release_firmware(adev->vcn.fw);
>>>>> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>>>>> return 0;
>>>>> }
>>>>> @@ -321,6 +323,7 @@ void amdgpu_vcn_ring_begin_use(struct
>>>>> amdgpu_ring *ring)
>>>>> struct amdgpu_device *adev = ring->adev;
>>>>> bool set_clocks =
>>>>> !cancel_delayed_work_sync(&adev->vcn.idle_work);
>>>>> + mutex_lock(&adev->vcn.vcn_pg_lock);
>>>>
>>>> That still won't work correctly here.
>>>>
>>>> The whole idea of the cancel_delayed_work_sync() and
>>>> schedule_delayed_work() dance is that you have exactly one user of
>>>> that. If you have multiple rings that whole thing won't work
>>>> correctly.
>>>>
>>>> To fix this you need to call mutex_lock() before
>>>> cancel_delayed_work_sync() and schedule_delayed_work() before
>>>> mutex_unlock().
>>>
>>> Big lock definitely works. I am trying to use as smaller lock as
>>> possible here. the share resource which needs protect here are power
>>> gate process and dpg mode switch process.
>>>
>>> if we move mutex_unlock() before schedule_delayed_work(. I am
>>> wondering what are the other necessary resources which need protect.
>>
>> By the way, cancel_delayed_work_sync() supports multiple thread
>> itself, so I didn't put it into protection area.
>
> Yeah, but that's correct but it still won't working correctly :)
>
> See the problem is that only for the first caller
> cancel_delayed_work_sync() returns true because it canceled the
> delayed work.
if the 1st caller gets true. the 2nd caller unfortunately may miss this
pending status, so it will ungate the power which is unexpected.
But in power gate/ungate function, a power state is maintained, so this
miss won't be really triggered to ungate the power.
So I think cancel_delayed_work_sync() / schedule_delayed_work() are not
necessary be protected here.
Best Regards!
James
>
> For all others it returns false and those would then think that they
> need to ungate the power.
>
> The only solution I see is to either put both the
> cancel_delayed_work_sync() and schedule_delayed_work() under the same
> mutex protection or start to use an atomic or other counter to note
> concurrent processing.
>
>> power gate is shared by all VCN IP instances and different rings , so
>> it needs be put into protection area.
>>
>> each ring's job itself is serialized by scheduler. it doesn't need
>> be put into this protection area.
>
> Yes, those should work as expected.
>
> Regards,
> Christian.
>
>>
>>>
>>> Thanks!
>>>
>>> James
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>> if (set_clocks) {
>>>>> amdgpu_gfx_off_ctrl(adev, false);
>>>>> amdgpu_device_ip_set_powergating_state(adev,
>>>>> AMD_IP_BLOCK_TYPE_VCN,
>>>>> @@ -345,6 +348,7 @@ void amdgpu_vcn_ring_begin_use(struct
>>>>> amdgpu_ring *ring)
>>>>> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
>>>>> }
>>>>> + mutex_unlock(&adev->vcn.vcn_pg_lock);
>>>>> }
>>>>> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>> index 6fe0573..2ae110d 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>> @@ -200,6 +200,7 @@ struct amdgpu_vcn {
>>>>> struct drm_gpu_scheduler
>>>>> *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
>>>>> uint32_t num_vcn_enc_sched;
>>>>> uint32_t num_vcn_dec_sched;
>>>>> + struct mutex vcn_pg_lock;
>>>>> unsigned harvest_config;
>>>>> int (*pause_dpg_mode)(struct amdgpu_device *adev,
>>>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-04 14:57 ` James Zhu
@ 2020-03-04 15:03 ` Christian König
2020-03-04 15:09 ` James Zhu
0 siblings, 1 reply; 27+ messages in thread
From: Christian König @ 2020-03-04 15:03 UTC (permalink / raw)
To: James Zhu, Christian König, James Zhu, amd-gfx
Am 04.03.20 um 15:57 schrieb James Zhu:
>
> On 2020-03-04 3:53 a.m., Christian König wrote:
>> Am 03.03.20 um 23:48 schrieb James Zhu:
>>>
>>> On 2020-03-03 2:03 p.m., James Zhu wrote:
>>>>
>>>> On 2020-03-03 1:44 p.m., Christian König wrote:
>>>>> Am 03.03.20 um 19:16 schrieb James Zhu:
>>>>>> Fix race condition issue when multiple vcn starts are called.
>>>>>>
>>>>>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>>>>>> ---
>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 ++++
>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
>>>>>> 2 files changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>>>> index f96464e..aa7663f 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>>>> @@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>>>>>> int i, r;
>>>>>> INIT_DELAYED_WORK(&adev->vcn.idle_work,
>>>>>> amdgpu_vcn_idle_work_handler);
>>>>>> + mutex_init(&adev->vcn.vcn_pg_lock);
>>>>>> switch (adev->asic_type) {
>>>>>> case CHIP_RAVEN:
>>>>>> @@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device
>>>>>> *adev)
>>>>>> }
>>>>>> release_firmware(adev->vcn.fw);
>>>>>> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>>>>>> return 0;
>>>>>> }
>>>>>> @@ -321,6 +323,7 @@ void amdgpu_vcn_ring_begin_use(struct
>>>>>> amdgpu_ring *ring)
>>>>>> struct amdgpu_device *adev = ring->adev;
>>>>>> bool set_clocks =
>>>>>> !cancel_delayed_work_sync(&adev->vcn.idle_work);
>>>>>> + mutex_lock(&adev->vcn.vcn_pg_lock);
>>>>>
>>>>> That still won't work correctly here.
>>>>>
>>>>> The whole idea of the cancel_delayed_work_sync() and
>>>>> schedule_delayed_work() dance is that you have exactly one user of
>>>>> that. If you have multiple rings that whole thing won't work
>>>>> correctly.
>>>>>
>>>>> To fix this you need to call mutex_lock() before
>>>>> cancel_delayed_work_sync() and schedule_delayed_work() before
>>>>> mutex_unlock().
>>>>
>>>> Big lock definitely works. I am trying to use as smaller lock as
>>>> possible here. the share resource which needs protect here are
>>>> power gate process and dpg mode switch process.
>>>>
>>>> if we move mutex_unlock() before schedule_delayed_work(. I am
>>>> wondering what are the other necessary resources which need protect.
>>>
>>> By the way, cancel_delayed_work_sync() supports multiple thread
>>> itself, so I didn't put it into protection area.
>>
>> Yeah, but that's correct but it still won't working correctly :)
>>
>> See the problem is that only for the first caller
>> cancel_delayed_work_sync() returns true because it canceled the
>> delayed work.
>
> if the 1st caller gets true. the 2nd caller unfortunately may miss
> this pending status, so it will ungate the power which is unexpected.
>
> But in power gate/ungate function, a power state is maintained, so
> this miss won't be really triggered to ungate the power.
>
> So I think cancel_delayed_work_sync() / schedule_delayed_work() are
> not necessary be protected here.
Ok that could work as well.
But in this case I would remove checking the return value of
cancel_delayed_work_sync() and just always ungate the power.
This way we prevent ugly bugs from leaking in when this really races
sometimes.
Regards,
Christian.
>
> Best Regards!
>
> James
>
>>
>> For all others it returns false and those would then think that they
>> need to ungate the power.
>>
>> The only solution I see is to either put both the
>> cancel_delayed_work_sync() and schedule_delayed_work() under the same
>> mutex protection or start to use an atomic or other counter to note
>> concurrent processing.
>>
>>> power gate is shared by all VCN IP instances and different rings ,
>>> so it needs be put into protection area.
>>>
>>> each ring's job itself is serialized by scheduler. it doesn't need
>>> be put into this protection area.
>>
>> Yes, those should work as expected.
>>
>> Regards,
>> Christian.
>>
>>>
>>>>
>>>> Thanks!
>>>>
>>>> James
>>>>
>>>>>
>>>>> Regards,
>>>>> Christian.
>>>>>
>>>>>> if (set_clocks) {
>>>>>> amdgpu_gfx_off_ctrl(adev, false);
>>>>>> amdgpu_device_ip_set_powergating_state(adev,
>>>>>> AMD_IP_BLOCK_TYPE_VCN,
>>>>>> @@ -345,6 +348,7 @@ void amdgpu_vcn_ring_begin_use(struct
>>>>>> amdgpu_ring *ring)
>>>>>> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
>>>>>> }
>>>>>> + mutex_unlock(&adev->vcn.vcn_pg_lock);
>>>>>> }
>>>>>> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>>> index 6fe0573..2ae110d 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>>> @@ -200,6 +200,7 @@ struct amdgpu_vcn {
>>>>>> struct drm_gpu_scheduler
>>>>>> *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
>>>>>> uint32_t num_vcn_enc_sched;
>>>>>> uint32_t num_vcn_dec_sched;
>>>>>> + struct mutex vcn_pg_lock;
>>>>>> unsigned harvest_config;
>>>>>> int (*pause_dpg_mode)(struct amdgpu_device *adev,
>>>>>
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-04 15:03 ` Christian König
@ 2020-03-04 15:09 ` James Zhu
0 siblings, 0 replies; 27+ messages in thread
From: James Zhu @ 2020-03-04 15:09 UTC (permalink / raw)
To: christian.koenig, James Zhu, amd-gfx
On 2020-03-04 10:03 a.m., Christian König wrote:
> Am 04.03.20 um 15:57 schrieb James Zhu:
>>
>> On 2020-03-04 3:53 a.m., Christian König wrote:
>>> Am 03.03.20 um 23:48 schrieb James Zhu:
>>>>
>>>> On 2020-03-03 2:03 p.m., James Zhu wrote:
>>>>>
>>>>> On 2020-03-03 1:44 p.m., Christian König wrote:
>>>>>> Am 03.03.20 um 19:16 schrieb James Zhu:
>>>>>>> Fix race condition issue when multiple vcn starts are called.
>>>>>>>
>>>>>>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>>>>>>> ---
>>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 ++++
>>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
>>>>>>> 2 files changed, 5 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>>>>> index f96464e..aa7663f 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>>>>>> @@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device
>>>>>>> *adev)
>>>>>>> int i, r;
>>>>>>> INIT_DELAYED_WORK(&adev->vcn.idle_work,
>>>>>>> amdgpu_vcn_idle_work_handler);
>>>>>>> + mutex_init(&adev->vcn.vcn_pg_lock);
>>>>>>> switch (adev->asic_type) {
>>>>>>> case CHIP_RAVEN:
>>>>>>> @@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device
>>>>>>> *adev)
>>>>>>> }
>>>>>>> release_firmware(adev->vcn.fw);
>>>>>>> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>>>>>>> return 0;
>>>>>>> }
>>>>>>> @@ -321,6 +323,7 @@ void amdgpu_vcn_ring_begin_use(struct
>>>>>>> amdgpu_ring *ring)
>>>>>>> struct amdgpu_device *adev = ring->adev;
>>>>>>> bool set_clocks =
>>>>>>> !cancel_delayed_work_sync(&adev->vcn.idle_work);
>>>>>>> + mutex_lock(&adev->vcn.vcn_pg_lock);
>>>>>>
>>>>>> That still won't work correctly here.
>>>>>>
>>>>>> The whole idea of the cancel_delayed_work_sync() and
>>>>>> schedule_delayed_work() dance is that you have exactly one user
>>>>>> of that. If you have multiple rings that whole thing won't work
>>>>>> correctly.
>>>>>>
>>>>>> To fix this you need to call mutex_lock() before
>>>>>> cancel_delayed_work_sync() and schedule_delayed_work() before
>>>>>> mutex_unlock().
>>>>>
>>>>> Big lock definitely works. I am trying to use as smaller lock as
>>>>> possible here. the share resource which needs protect here are
>>>>> power gate process and dpg mode switch process.
>>>>>
>>>>> if we move mutex_unlock() before schedule_delayed_work(. I am
>>>>> wondering what are the other necessary resources which need protect.
>>>>
>>>> By the way, cancel_delayed_work_sync() supports multiple thread
>>>> itself, so I didn't put it into protection area.
>>>
>>> Yeah, but that's correct but it still won't working correctly :)
>>>
>>> See the problem is that only for the first caller
>>> cancel_delayed_work_sync() returns true because it canceled the
>>> delayed work.
>>
>> if the 1st caller gets true. the 2nd caller unfortunately may miss
>> this pending status, so it will ungate the power which is unexpected.
>>
>> But in power gate/ungate function, a power state is maintained, so
>> this miss won't be really triggered to ungate the power.
>>
>> So I think cancel_delayed_work_sync() / schedule_delayed_work() are
>> not necessary be protected here.
>
> Ok that could work as well.
>
> But in this case I would remove checking the return value of
> cancel_delayed_work_sync() and just always ungate the power.
>
> This way we prevent ugly bugs from leaking in when this really races
> sometimes.
Sure. Thanks!
James
>
> Regards,
> Christian.
>
>>
>> Best Regards!
>>
>> James
>>
>>>
>>> For all others it returns false and those would then think that they
>>> need to ungate the power.
>>>
>>> The only solution I see is to either put both the
>>> cancel_delayed_work_sync() and schedule_delayed_work() under the
>>> same mutex protection or start to use an atomic or other counter to
>>> note concurrent processing.
>>>
>>>> power gate is shared by all VCN IP instances and different rings ,
>>>> so it needs be put into protection area.
>>>>
>>>> each ring's job itself is serialized by scheduler. it doesn't need
>>>> be put into this protection area.
>>>
>>> Yes, those should work as expected.
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>>>
>>>>> Thanks!
>>>>>
>>>>> James
>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Christian.
>>>>>>
>>>>>>> if (set_clocks) {
>>>>>>> amdgpu_gfx_off_ctrl(adev, false);
>>>>>>> amdgpu_device_ip_set_powergating_state(adev,
>>>>>>> AMD_IP_BLOCK_TYPE_VCN,
>>>>>>> @@ -345,6 +348,7 @@ void amdgpu_vcn_ring_begin_use(struct
>>>>>>> amdgpu_ring *ring)
>>>>>>> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
>>>>>>> }
>>>>>>> + mutex_unlock(&adev->vcn.vcn_pg_lock);
>>>>>>> }
>>>>>>> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>>>> index 6fe0573..2ae110d 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>>>>>> @@ -200,6 +200,7 @@ struct amdgpu_vcn {
>>>>>>> struct drm_gpu_scheduler
>>>>>>> *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
>>>>>>> uint32_t num_vcn_enc_sched;
>>>>>>> uint32_t num_vcn_dec_sched;
>>>>>>> + struct mutex vcn_pg_lock;
>>>>>>> unsigned harvest_config;
>>>>>>> int (*pause_dpg_mode)(struct amdgpu_device *adev,
>>>>>>
>>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7CJames.Zhu%40amd.com%7Ce9f5dcb164bc4efacd7908d7c04d254f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637189309893957692&sdata=WcwmGejuagp5T9ojpSPLaU%2BmlHuidkh7SP3jDASpUSI%3D&reserved=0
>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-03 18:16 [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start James Zhu
` (3 preceding siblings ...)
2020-03-03 18:44 ` [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start Christian König
@ 2020-03-04 16:34 ` James Zhu
2020-03-05 11:25 ` Christian König
2020-03-09 16:57 ` [PATCH v3 " James Zhu
` (2 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: James Zhu @ 2020-03-04 16:34 UTC (permalink / raw)
To: amd-gfx; +Cc: jamesz
Fix race condition issue when multiple vcn starts are called.
v2: Removed checking the return value of cancel_delayed_work_sync()
to prevent possible races here.
Signed-off-by: James Zhu <James.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 15 +++++++++------
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index f96464e..8a8406b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
int i, r;
INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
+ mutex_init(&adev->vcn.vcn_pg_lock);
switch (adev->asic_type) {
case CHIP_RAVEN:
@@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
}
release_firmware(adev->vcn.fw);
+ mutex_destroy(&adev->vcn.vcn_pg_lock);
return 0;
}
@@ -319,13 +321,13 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
{
struct amdgpu_device *adev = ring->adev;
- bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
- if (set_clocks) {
- amdgpu_gfx_off_ctrl(adev, false);
- amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
- AMD_PG_STATE_UNGATE);
- }
+ cancel_delayed_work_sync(&adev->vcn.idle_work);
+
+ mutex_lock(&adev->vcn.vcn_pg_lock);
+ amdgpu_gfx_off_ctrl(adev, false);
+ amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+ AMD_PG_STATE_UNGATE);
if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
struct dpg_pause_state new_state;
@@ -345,6 +347,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
}
+ mutex_unlock(&adev->vcn.vcn_pg_lock);
}
void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 6fe0573..2ae110d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -200,6 +200,7 @@ struct amdgpu_vcn {
struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
uint32_t num_vcn_enc_sched;
uint32_t num_vcn_dec_sched;
+ struct mutex vcn_pg_lock;
unsigned harvest_config;
int (*pause_dpg_mode)(struct amdgpu_device *adev,
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-04 16:34 ` [PATCH v2 " James Zhu
@ 2020-03-05 11:25 ` Christian König
2020-03-05 11:27 ` Christian König
0 siblings, 1 reply; 27+ messages in thread
From: Christian König @ 2020-03-05 11:25 UTC (permalink / raw)
To: James Zhu, amd-gfx; +Cc: jamesz
Am 04.03.20 um 17:34 schrieb James Zhu:
> Fix race condition issue when multiple vcn starts are called.
>
> v2: Removed checking the return value of cancel_delayed_work_sync()
> to prevent possible races here.
>
> Signed-off-by: James Zhu <James.Zhu@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 15 +++++++++------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
> 2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index f96464e..8a8406b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
> int i, r;
>
> INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
> + mutex_init(&adev->vcn.vcn_pg_lock);
>
> switch (adev->asic_type) {
> case CHIP_RAVEN:
> @@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
> }
>
> release_firmware(adev->vcn.fw);
> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>
> return 0;
> }
> @@ -319,13 +321,13 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
> void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
> {
> struct amdgpu_device *adev = ring->adev;
> - bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
>
> - if (set_clocks) {
> - amdgpu_gfx_off_ctrl(adev, false);
> - amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> - AMD_PG_STATE_UNGATE);
> - }
> + cancel_delayed_work_sync(&adev->vcn.idle_work);
> +
> + mutex_lock(&adev->vcn.vcn_pg_lock);
> + amdgpu_gfx_off_ctrl(adev, false);
> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> + AMD_PG_STATE_UNGATE);
>
> if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> struct dpg_pause_state new_state;
> @@ -345,6 +347,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
>
> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
> }
> + mutex_unlock(&adev->vcn.vcn_pg_lock);
> }
>
> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 6fe0573..2ae110d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -200,6 +200,7 @@ struct amdgpu_vcn {
> struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
> uint32_t num_vcn_enc_sched;
> uint32_t num_vcn_dec_sched;
> + struct mutex vcn_pg_lock;
>
> unsigned harvest_config;
> int (*pause_dpg_mode)(struct amdgpu_device *adev,
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-05 11:25 ` Christian König
@ 2020-03-05 11:27 ` Christian König
2020-03-05 14:34 ` James Zhu
0 siblings, 1 reply; 27+ messages in thread
From: Christian König @ 2020-03-05 11:27 UTC (permalink / raw)
To: James Zhu, amd-gfx; +Cc: jamesz
Am 05.03.20 um 12:25 schrieb Christian König:
> Am 04.03.20 um 17:34 schrieb James Zhu:
>> Fix race condition issue when multiple vcn starts are called.
>>
>> v2: Removed checking the return value of cancel_delayed_work_sync()
>> to prevent possible races here.
>>
>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>
> Reviewed-by: Christian König <christian.koenig@amd.com>
One thing worth noting is that in theory you could run into the issue
that one ring restarts the timer while another ring is still preparing
the engine for usage.
So the timeout should be large enough to guarantee that this never
causes problems.
Regards,
Christian.
>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 15 +++++++++------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
>> 2 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> index f96464e..8a8406b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> @@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>> int i, r;
>> INIT_DELAYED_WORK(&adev->vcn.idle_work,
>> amdgpu_vcn_idle_work_handler);
>> + mutex_init(&adev->vcn.vcn_pg_lock);
>> switch (adev->asic_type) {
>> case CHIP_RAVEN:
>> @@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
>> }
>> release_firmware(adev->vcn.fw);
>> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>> return 0;
>> }
>> @@ -319,13 +321,13 @@ static void amdgpu_vcn_idle_work_handler(struct
>> work_struct *work)
>> void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
>> {
>> struct amdgpu_device *adev = ring->adev;
>> - bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
>> - if (set_clocks) {
>> - amdgpu_gfx_off_ctrl(adev, false);
>> - amdgpu_device_ip_set_powergating_state(adev,
>> AMD_IP_BLOCK_TYPE_VCN,
>> - AMD_PG_STATE_UNGATE);
>> - }
>> + cancel_delayed_work_sync(&adev->vcn.idle_work);
>> +
>> + mutex_lock(&adev->vcn.vcn_pg_lock);
>> + amdgpu_gfx_off_ctrl(adev, false);
>> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
>> + AMD_PG_STATE_UNGATE);
>> if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
>> struct dpg_pause_state new_state;
>> @@ -345,6 +347,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring
>> *ring)
>> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
>> }
>> + mutex_unlock(&adev->vcn.vcn_pg_lock);
>> }
>> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> index 6fe0573..2ae110d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> @@ -200,6 +200,7 @@ struct amdgpu_vcn {
>> struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
>> uint32_t num_vcn_enc_sched;
>> uint32_t num_vcn_dec_sched;
>> + struct mutex vcn_pg_lock;
>> unsigned harvest_config;
>> int (*pause_dpg_mode)(struct amdgpu_device *adev,
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-05 11:27 ` Christian König
@ 2020-03-05 14:34 ` James Zhu
0 siblings, 0 replies; 27+ messages in thread
From: James Zhu @ 2020-03-05 14:34 UTC (permalink / raw)
To: christian.koenig, James Zhu, amd-gfx
On 2020-03-05 6:27 a.m., Christian König wrote:
> Am 05.03.20 um 12:25 schrieb Christian König:
>> Am 04.03.20 um 17:34 schrieb James Zhu:
>>> Fix race condition issue when multiple vcn starts are called.
>>>
>>> v2: Removed checking the return value of cancel_delayed_work_sync()
>>> to prevent possible races here.
>>>
>>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>>
>> Reviewed-by: Christian König <christian.koenig@amd.com>
>
> One thing worth noting is that in theory you could run into the issue
> that one ring restarts the timer while another ring is still preparing
> the engine for usage.
Yes, you are right, The current timeout setting is 1 sec to guarantee
one dec/enc can be finished for all formats. the preparing process
should nuch less than this setting.
otherwise there are some other bugs that needs to be fixed.
By the way, maybe we shouldn't just rely on fence check to determine if
any job is left. We can maintain a dec/enc submission count(patch 2
already added for enc). how do you think?
Best Regards!
James
>
> So the timeout should be large enough to guarantee that this never
> causes problems.
>
> Regards,
> Christian.
>
>>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 15 +++++++++------
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
>>> 2 files changed, 10 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> index f96464e..8a8406b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>>> @@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>>> int i, r;
>>> INIT_DELAYED_WORK(&adev->vcn.idle_work,
>>> amdgpu_vcn_idle_work_handler);
>>> + mutex_init(&adev->vcn.vcn_pg_lock);
>>> switch (adev->asic_type) {
>>> case CHIP_RAVEN:
>>> @@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
>>> }
>>> release_firmware(adev->vcn.fw);
>>> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>>> return 0;
>>> }
>>> @@ -319,13 +321,13 @@ static void
>>> amdgpu_vcn_idle_work_handler(struct work_struct *work)
>>> void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
>>> {
>>> struct amdgpu_device *adev = ring->adev;
>>> - bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
>>> - if (set_clocks) {
>>> - amdgpu_gfx_off_ctrl(adev, false);
>>> - amdgpu_device_ip_set_powergating_state(adev,
>>> AMD_IP_BLOCK_TYPE_VCN,
>>> - AMD_PG_STATE_UNGATE);
>>> - }
>>> + cancel_delayed_work_sync(&adev->vcn.idle_work);
>>> +
>>> + mutex_lock(&adev->vcn.vcn_pg_lock);
>>> + amdgpu_gfx_off_ctrl(adev, false);
>>> + amdgpu_device_ip_set_powergating_state(adev,
>>> AMD_IP_BLOCK_TYPE_VCN,
>>> + AMD_PG_STATE_UNGATE);
>>> if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
>>> struct dpg_pause_state new_state;
>>> @@ -345,6 +347,7 @@ void amdgpu_vcn_ring_begin_use(struct
>>> amdgpu_ring *ring)
>>> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
>>> }
>>> + mutex_unlock(&adev->vcn.vcn_pg_lock);
>>> }
>>> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>> index 6fe0573..2ae110d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>>> @@ -200,6 +200,7 @@ struct amdgpu_vcn {
>>> struct drm_gpu_scheduler
>>> *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
>>> uint32_t num_vcn_enc_sched;
>>> uint32_t num_vcn_dec_sched;
>>> + struct mutex vcn_pg_lock;
>>> unsigned harvest_config;
>>> int (*pause_dpg_mode)(struct amdgpu_device *adev,
>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-03 18:16 [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start James Zhu
` (4 preceding siblings ...)
2020-03-04 16:34 ` [PATCH v2 " James Zhu
@ 2020-03-09 16:57 ` James Zhu
2020-03-11 11:30 ` Zhu, James
2020-03-11 14:15 ` [PATCH v4 " James Zhu
2020-03-11 15:04 ` [PATCH v5 " James Zhu
7 siblings, 1 reply; 27+ messages in thread
From: James Zhu @ 2020-03-09 16:57 UTC (permalink / raw)
To: amd-gfx; +Cc: jamesz, christian.koenig
Fix race condition issue when multiple vcn starts are called.
v2: Removed checking the return value of cancel_delayed_work_sync()
to prevent possible races here.
v3: Add total_submission_cnt to avoid gate power unexpectedly.
Signed-off-by: James Zhu <James.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 22 +++++++++++++++-------
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 2 ++
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index a41272f..6aafda1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -63,6 +63,8 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
int i, r;
INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
+ mutex_init(&adev->vcn.vcn_pg_lock);
+ atomic_set(&adev->vcn.total_submission_cnt, 0);
switch (adev->asic_type) {
case CHIP_RAVEN:
@@ -210,6 +212,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
}
release_firmware(adev->vcn.fw);
+ mutex_destroy(&adev->vcn.vcn_pg_lock);
return 0;
}
@@ -307,7 +310,8 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
fences += fence[j];
}
- if (fences == 0) {
+ if (fences == 0 &&
+ likely(atomic_read(&adev->vcn.total_submission_cnt) == 0)) {
amdgpu_gfx_off_ctrl(adev, true);
amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
AMD_PG_STATE_GATE);
@@ -319,13 +323,14 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
{
struct amdgpu_device *adev = ring->adev;
- bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
- if (set_clocks) {
- amdgpu_gfx_off_ctrl(adev, false);
- amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
- AMD_PG_STATE_UNGATE);
- }
+ atomic_inc(&adev->vcn.total_submission_cnt);
+ cancel_delayed_work_sync(&adev->vcn.idle_work);
+
+ mutex_lock(&adev->vcn.vcn_pg_lock);
+ amdgpu_gfx_off_ctrl(adev, false);
+ amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+ AMD_PG_STATE_UNGATE);
if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
struct dpg_pause_state new_state;
@@ -345,11 +350,14 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
}
+ mutex_unlock(&adev->vcn.vcn_pg_lock);
}
void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
{
schedule_delayed_work(&ring->adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
+ if (unlikely(atomic_dec_return(&ring->adev->vcn.total_submission_cnt) < 0))
+ atomic_set(&ring->adev->vcn.total_submission_cnt, 0);
}
int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 6fe0573..111c4cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -200,6 +200,8 @@ struct amdgpu_vcn {
struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
uint32_t num_vcn_enc_sched;
uint32_t num_vcn_dec_sched;
+ struct mutex vcn_pg_lock;
+ atomic_t total_submission_cnt;
unsigned harvest_config;
int (*pause_dpg_mode)(struct amdgpu_device *adev,
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v3 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-09 16:57 ` [PATCH v3 " James Zhu
@ 2020-03-11 11:30 ` Zhu, James
2020-03-11 11:38 ` Christian König
0 siblings, 1 reply; 27+ messages in thread
From: Zhu, James @ 2020-03-11 11:30 UTC (permalink / raw)
To: amd-gfx@lists.freedesktop.org; +Cc: Koenig, Christian
[-- Attachment #1.1: Type: text/plain, Size: 4268 bytes --]
[AMD Official Use Only - Internal Distribution Only]
ping
________________________________
From: Zhu, James <James.Zhu@amd.com>
Sent: Monday, March 9, 2020 12:57 PM
To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Zhu, James <James.Zhu@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>
Subject: [PATCH v3 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
Fix race condition issue when multiple vcn starts are called.
v2: Removed checking the return value of cancel_delayed_work_sync()
to prevent possible races here.
v3: Add total_submission_cnt to avoid gate power unexpectedly.
Signed-off-by: James Zhu <James.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 22 +++++++++++++++-------
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 2 ++
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index a41272f..6aafda1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -63,6 +63,8 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
int i, r;
INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
+ mutex_init(&adev->vcn.vcn_pg_lock);
+ atomic_set(&adev->vcn.total_submission_cnt, 0);
switch (adev->asic_type) {
case CHIP_RAVEN:
@@ -210,6 +212,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
}
release_firmware(adev->vcn.fw);
+ mutex_destroy(&adev->vcn.vcn_pg_lock);
return 0;
}
@@ -307,7 +310,8 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
fences += fence[j];
}
- if (fences == 0) {
+ if (fences == 0 &&
+ likely(atomic_read(&adev->vcn.total_submission_cnt) == 0)) {
amdgpu_gfx_off_ctrl(adev, true);
amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
AMD_PG_STATE_GATE);
@@ -319,13 +323,14 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
{
struct amdgpu_device *adev = ring->adev;
- bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
- if (set_clocks) {
- amdgpu_gfx_off_ctrl(adev, false);
- amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
- AMD_PG_STATE_UNGATE);
- }
+ atomic_inc(&adev->vcn.total_submission_cnt);
+ cancel_delayed_work_sync(&adev->vcn.idle_work);
+
+ mutex_lock(&adev->vcn.vcn_pg_lock);
+ amdgpu_gfx_off_ctrl(adev, false);
+ amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+ AMD_PG_STATE_UNGATE);
if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
struct dpg_pause_state new_state;
@@ -345,11 +350,14 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
}
+ mutex_unlock(&adev->vcn.vcn_pg_lock);
}
void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
{
schedule_delayed_work(&ring->adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
+ if (unlikely(atomic_dec_return(&ring->adev->vcn.total_submission_cnt) < 0))
+ atomic_set(&ring->adev->vcn.total_submission_cnt, 0);
}
int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 6fe0573..111c4cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -200,6 +200,8 @@ struct amdgpu_vcn {
struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
uint32_t num_vcn_enc_sched;
uint32_t num_vcn_dec_sched;
+ struct mutex vcn_pg_lock;
+ atomic_t total_submission_cnt;
unsigned harvest_config;
int (*pause_dpg_mode)(struct amdgpu_device *adev,
--
2.7.4
[-- Attachment #1.2: Type: text/html, Size: 8502 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v3 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-11 11:30 ` Zhu, James
@ 2020-03-11 11:38 ` Christian König
2020-03-11 14:15 ` James Zhu
0 siblings, 1 reply; 27+ messages in thread
From: Christian König @ 2020-03-11 11:38 UTC (permalink / raw)
To: Zhu, James, amd-gfx@lists.freedesktop.org
[-- Attachment #1.1: Type: text/plain, Size: 4876 bytes --]
Am 11.03.20 um 12:30 schrieb Zhu, James:
>
> [AMD Official Use Only - Internal Distribution Only]
>
>
> ping
>
> ------------------------------------------------------------------------
> *From:* Zhu, James <James.Zhu@amd.com>
> *Sent:* Monday, March 9, 2020 12:57 PM
> *To:* amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> *Cc:* Zhu, James <James.Zhu@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>
> *Subject:* [PATCH v3 1/4] drm/amdgpu/vcn: fix race condition issue for
> vcn start
> Fix race condition issue when multiple vcn starts are called.
>
> v2: Removed checking the return value of cancel_delayed_work_sync()
> to prevent possible races here.
>
> v3: Add total_submission_cnt to avoid gate power unexpectedly.
>
> Signed-off-by: James Zhu <James.Zhu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 22 +++++++++++++++-------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 2 ++
> 2 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index a41272f..6aafda1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -63,6 +63,8 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
> int i, r;
>
> INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
> + mutex_init(&adev->vcn.vcn_pg_lock);
> + atomic_set(&adev->vcn.total_submission_cnt, 0);
>
> switch (adev->asic_type) {
> case CHIP_RAVEN:
> @@ -210,6 +212,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
> }
>
> release_firmware(adev->vcn.fw);
> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>
> return 0;
> }
> @@ -307,7 +310,8 @@ static void amdgpu_vcn_idle_work_handler(struct
> work_struct *work)
> fences += fence[j];
> }
>
> - if (fences == 0) {
> + if (fences == 0 &&
> + likely(atomic_read(&adev->vcn.total_submission_cnt) == 0)) {
> amdgpu_gfx_off_ctrl(adev, true);
> amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> AMD_PG_STATE_GATE);
> @@ -319,13 +323,14 @@ static void amdgpu_vcn_idle_work_handler(struct
> work_struct *work)
> void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
> {
> struct amdgpu_device *adev = ring->adev;
> - bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
>
> - if (set_clocks) {
> - amdgpu_gfx_off_ctrl(adev, false);
> - amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> - AMD_PG_STATE_UNGATE);
> - }
> + atomic_inc(&adev->vcn.total_submission_cnt);
> + cancel_delayed_work_sync(&adev->vcn.idle_work);
> +
> + mutex_lock(&adev->vcn.vcn_pg_lock);
> + amdgpu_gfx_off_ctrl(adev, false);
> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> + AMD_PG_STATE_UNGATE);
>
> if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> struct dpg_pause_state new_state;
> @@ -345,11 +350,14 @@ void amdgpu_vcn_ring_begin_use(struct
> amdgpu_ring *ring)
>
> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
> }
> + mutex_unlock(&adev->vcn.vcn_pg_lock);
> }
>
> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
> {
> schedule_delayed_work(&ring->adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
> + if
> (unlikely(atomic_dec_return(&ring->adev->vcn.total_submission_cnt) < 0))
> + atomic_set(&ring->adev->vcn.total_submission_cnt, 0);
You need to decrement first and then call schedule_delayed_work()
otherwise the work could run with the wrong counter.
And the extra check for an under run should be superfluous.
Regards,
Christian.
> }
>
> int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 6fe0573..111c4cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -200,6 +200,8 @@ struct amdgpu_vcn {
> struct drm_gpu_scheduler
> *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
> uint32_t num_vcn_enc_sched;
> uint32_t num_vcn_dec_sched;
> + struct mutex vcn_pg_lock;
> + atomic_t total_submission_cnt;
>
> unsigned harvest_config;
> int (*pause_dpg_mode)(struct amdgpu_device *adev,
> --
> 2.7.4
>
[-- Attachment #1.2: Type: text/html, Size: 13499 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-11 11:38 ` Christian König
@ 2020-03-11 14:15 ` James Zhu
0 siblings, 0 replies; 27+ messages in thread
From: James Zhu @ 2020-03-11 14:15 UTC (permalink / raw)
To: Christian König, Zhu, James, amd-gfx@lists.freedesktop.org
[-- Attachment #1.1: Type: text/plain, Size: 5040 bytes --]
On 2020-03-11 7:38 a.m., Christian König wrote:
> Am 11.03.20 um 12:30 schrieb Zhu, James:
>>
>> [AMD Official Use Only - Internal Distribution Only]
>>
>>
>> ping
>>
>> ------------------------------------------------------------------------
>> *From:* Zhu, James <James.Zhu@amd.com>
>> *Sent:* Monday, March 9, 2020 12:57 PM
>> *To:* amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
>> *Cc:* Zhu, James <James.Zhu@amd.com>; Koenig, Christian
>> <Christian.Koenig@amd.com>
>> *Subject:* [PATCH v3 1/4] drm/amdgpu/vcn: fix race condition issue
>> for vcn start
>> Fix race condition issue when multiple vcn starts are called.
>>
>> v2: Removed checking the return value of cancel_delayed_work_sync()
>> to prevent possible races here.
>>
>> v3: Add total_submission_cnt to avoid gate power unexpectedly.
>>
>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 22 +++++++++++++++-------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 2 ++
>> 2 files changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> index a41272f..6aafda1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> @@ -63,6 +63,8 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
>> int i, r;
>>
>> INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
>> + mutex_init(&adev->vcn.vcn_pg_lock);
>> + atomic_set(&adev->vcn.total_submission_cnt, 0);
>>
>> switch (adev->asic_type) {
>> case CHIP_RAVEN:
>> @@ -210,6 +212,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
>> }
>>
>> release_firmware(adev->vcn.fw);
>> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>>
>> return 0;
>> }
>> @@ -307,7 +310,8 @@ static void amdgpu_vcn_idle_work_handler(struct
>> work_struct *work)
>> fences += fence[j];
>> }
>>
>> - if (fences == 0) {
>> + if (fences == 0 &&
>> + likely(atomic_read(&adev->vcn.total_submission_cnt) == 0)) {
>> amdgpu_gfx_off_ctrl(adev, true);
>> amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
>> AMD_PG_STATE_GATE);
>> @@ -319,13 +323,14 @@ static void amdgpu_vcn_idle_work_handler(struct
>> work_struct *work)
>> void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
>> {
>> struct amdgpu_device *adev = ring->adev;
>> - bool set_clocks =
>> !cancel_delayed_work_sync(&adev->vcn.idle_work);
>>
>> - if (set_clocks) {
>> - amdgpu_gfx_off_ctrl(adev, false);
>> - amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
>> - AMD_PG_STATE_UNGATE);
>> - }
>> + atomic_inc(&adev->vcn.total_submission_cnt);
>> + cancel_delayed_work_sync(&adev->vcn.idle_work);
>> +
>> + mutex_lock(&adev->vcn.vcn_pg_lock);
>> + amdgpu_gfx_off_ctrl(adev, false);
>> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
>> + AMD_PG_STATE_UNGATE);
>>
>> if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
>> struct dpg_pause_state new_state;
>> @@ -345,11 +350,14 @@ void amdgpu_vcn_ring_begin_use(struct
>> amdgpu_ring *ring)
>>
>> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
>> }
>> + mutex_unlock(&adev->vcn.vcn_pg_lock);
>> }
>>
>> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
>> {
>> schedule_delayed_work(&ring->adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
>> + if
>> (unlikely(atomic_dec_return(&ring->adev->vcn.total_submission_cnt) < 0))
>> + atomic_set(&ring->adev->vcn.total_submission_cnt, 0);
>
> You need to decrement first and then call schedule_delayed_work()
> otherwise the work could run with the wrong counter.
>
> And the extra check for an under run should be superfluous.
Thanks! Please check V4
James
>
> Regards,
> Christian.
>
>> }
>>
>> int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> index 6fe0573..111c4cc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> @@ -200,6 +200,8 @@ struct amdgpu_vcn {
>> struct drm_gpu_scheduler
>> *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
>> uint32_t num_vcn_enc_sched;
>> uint32_t num_vcn_dec_sched;
>> + struct mutex vcn_pg_lock;
>> + atomic_t total_submission_cnt;
>>
>> unsigned harvest_config;
>> int (*pause_dpg_mode)(struct amdgpu_device *adev,
>> --
>> 2.7.4
>>
>
[-- Attachment #1.2: Type: text/html, Size: 14668 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-03 18:16 [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start James Zhu
` (5 preceding siblings ...)
2020-03-09 16:57 ` [PATCH v3 " James Zhu
@ 2020-03-11 14:15 ` James Zhu
2020-03-11 14:39 ` Christian König
2020-03-11 15:04 ` [PATCH v5 " James Zhu
7 siblings, 1 reply; 27+ messages in thread
From: James Zhu @ 2020-03-11 14:15 UTC (permalink / raw)
To: amd-gfx; +Cc: jamesz
Fix race condition issue when multiple vcn starts are called.
v2: Removed checking the return value of cancel_delayed_work_sync()
to prevent possible races here.
v3: Add total_submission_cnt to avoid gate power unexpectedly.
v4: Remove extra counter check, and reduce counter before idle
work schedule
Signed-off-by: James Zhu <James.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 22 +++++++++++++++-------
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 2 ++
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index a41272f..2fa2891 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -63,6 +63,8 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
int i, r;
INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
+ mutex_init(&adev->vcn.vcn_pg_lock);
+ atomic_set(&adev->vcn.total_submission_cnt, 0);
switch (adev->asic_type) {
case CHIP_RAVEN:
@@ -210,6 +212,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
}
release_firmware(adev->vcn.fw);
+ mutex_destroy(&adev->vcn.vcn_pg_lock);
return 0;
}
@@ -307,7 +310,8 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
fences += fence[j];
}
- if (fences == 0) {
+ if (fences == 0 &&
+ likely(atomic_read(&adev->vcn.total_submission_cnt) == 0)) {
amdgpu_gfx_off_ctrl(adev, true);
amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
AMD_PG_STATE_GATE);
@@ -319,13 +323,14 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
{
struct amdgpu_device *adev = ring->adev;
- bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
- if (set_clocks) {
- amdgpu_gfx_off_ctrl(adev, false);
- amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
- AMD_PG_STATE_UNGATE);
- }
+ atomic_inc(&adev->vcn.total_submission_cnt);
+ cancel_delayed_work_sync(&adev->vcn.idle_work);
+
+ mutex_lock(&adev->vcn.vcn_pg_lock);
+ amdgpu_gfx_off_ctrl(adev, false);
+ amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+ AMD_PG_STATE_UNGATE);
if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
struct dpg_pause_state new_state;
@@ -345,10 +350,13 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
}
+ mutex_unlock(&adev->vcn.vcn_pg_lock);
}
void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
{
+ atomic_dec(&ring->adev->vcn.total_submission_cnt);
+
schedule_delayed_work(&ring->adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 6fe0573..111c4cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -200,6 +200,8 @@ struct amdgpu_vcn {
struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
uint32_t num_vcn_enc_sched;
uint32_t num_vcn_dec_sched;
+ struct mutex vcn_pg_lock;
+ atomic_t total_submission_cnt;
unsigned harvest_config;
int (*pause_dpg_mode)(struct amdgpu_device *adev,
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-11 14:15 ` [PATCH v4 " James Zhu
@ 2020-03-11 14:39 ` Christian König
0 siblings, 0 replies; 27+ messages in thread
From: Christian König @ 2020-03-11 14:39 UTC (permalink / raw)
To: James Zhu, amd-gfx; +Cc: jamesz
Am 11.03.20 um 15:15 schrieb James Zhu:
> Fix race condition issue when multiple vcn starts are called.
>
> v2: Removed checking the return value of cancel_delayed_work_sync()
> to prevent possible races here.
>
> v3: Add total_submission_cnt to avoid gate power unexpectedly.
>
> v4: Remove extra counter check, and reduce counter before idle
> work schedule
>
> Signed-off-by: James Zhu <James.Zhu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 22 +++++++++++++++-------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 2 ++
> 2 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index a41272f..2fa2891 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -63,6 +63,8 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
> int i, r;
>
> INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
> + mutex_init(&adev->vcn.vcn_pg_lock);
> + atomic_set(&adev->vcn.total_submission_cnt, 0);
>
> switch (adev->asic_type) {
> case CHIP_RAVEN:
> @@ -210,6 +212,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
> }
>
> release_firmware(adev->vcn.fw);
> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>
> return 0;
> }
> @@ -307,7 +310,8 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
> fences += fence[j];
> }
>
> - if (fences == 0) {
> + if (fences == 0 &&
> + likely(atomic_read(&adev->vcn.total_submission_cnt) == 0)) {
The indentation here looks off, maybe just write that as !fences &&
!atomic_read(&adev->vcn.total_submission_cnt).
Apart from that looks good to me now,
Christian.
> amdgpu_gfx_off_ctrl(adev, true);
> amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> AMD_PG_STATE_GATE);
> @@ -319,13 +323,14 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
> void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
> {
> struct amdgpu_device *adev = ring->adev;
> - bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
>
> - if (set_clocks) {
> - amdgpu_gfx_off_ctrl(adev, false);
> - amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> - AMD_PG_STATE_UNGATE);
> - }
> + atomic_inc(&adev->vcn.total_submission_cnt);
> + cancel_delayed_work_sync(&adev->vcn.idle_work);
> +
> + mutex_lock(&adev->vcn.vcn_pg_lock);
> + amdgpu_gfx_off_ctrl(adev, false);
> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> + AMD_PG_STATE_UNGATE);
>
> if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> struct dpg_pause_state new_state;
> @@ -345,10 +350,13 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
>
> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
> }
> + mutex_unlock(&adev->vcn.vcn_pg_lock);
> }
>
> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
> {
> + atomic_dec(&ring->adev->vcn.total_submission_cnt);
> +
> schedule_delayed_work(&ring->adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 6fe0573..111c4cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -200,6 +200,8 @@ struct amdgpu_vcn {
> struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
> uint32_t num_vcn_enc_sched;
> uint32_t num_vcn_dec_sched;
> + struct mutex vcn_pg_lock;
> + atomic_t total_submission_cnt;
>
> unsigned harvest_config;
> int (*pause_dpg_mode)(struct amdgpu_device *adev,
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-03 18:16 [PATCH 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start James Zhu
` (6 preceding siblings ...)
2020-03-11 14:15 ` [PATCH v4 " James Zhu
@ 2020-03-11 15:04 ` James Zhu
2020-03-11 15:13 ` Christian König
7 siblings, 1 reply; 27+ messages in thread
From: James Zhu @ 2020-03-11 15:04 UTC (permalink / raw)
To: amd-gfx; +Cc: jamesz
Fix race condition issue when multiple vcn starts are called.
v2: Removed checking the return value of cancel_delayed_work_sync()
to prevent possible races here.
v3: Add total_submission_cnt to avoid gate power unexpectedly.
v4: Remove extra counter check, and reduce counter before idle
work schedule
Signed-off-by: James Zhu <James.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 21 ++++++++++++++-------
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 2 ++
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index a41272f..6dacf78 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -63,6 +63,8 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
int i, r;
INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
+ mutex_init(&adev->vcn.vcn_pg_lock);
+ atomic_set(&adev->vcn.total_submission_cnt, 0);
switch (adev->asic_type) {
case CHIP_RAVEN:
@@ -210,6 +212,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
}
release_firmware(adev->vcn.fw);
+ mutex_destroy(&adev->vcn.vcn_pg_lock);
return 0;
}
@@ -307,7 +310,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
fences += fence[j];
}
- if (fences == 0) {
+ if (!fences && !atomic_read(&adev->vcn.total_submission_cnt)) {
amdgpu_gfx_off_ctrl(adev, true);
amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
AMD_PG_STATE_GATE);
@@ -319,13 +322,14 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
{
struct amdgpu_device *adev = ring->adev;
- bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
- if (set_clocks) {
- amdgpu_gfx_off_ctrl(adev, false);
- amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
- AMD_PG_STATE_UNGATE);
- }
+ atomic_inc(&adev->vcn.total_submission_cnt);
+ cancel_delayed_work_sync(&adev->vcn.idle_work);
+
+ mutex_lock(&adev->vcn.vcn_pg_lock);
+ amdgpu_gfx_off_ctrl(adev, false);
+ amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
+ AMD_PG_STATE_UNGATE);
if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
struct dpg_pause_state new_state;
@@ -345,10 +349,13 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
}
+ mutex_unlock(&adev->vcn.vcn_pg_lock);
}
void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
{
+ atomic_dec(&ring->adev->vcn.total_submission_cnt);
+
schedule_delayed_work(&ring->adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 6fe0573..111c4cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -200,6 +200,8 @@ struct amdgpu_vcn {
struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
uint32_t num_vcn_enc_sched;
uint32_t num_vcn_dec_sched;
+ struct mutex vcn_pg_lock;
+ atomic_t total_submission_cnt;
unsigned harvest_config;
int (*pause_dpg_mode)(struct amdgpu_device *adev,
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start
2020-03-11 15:04 ` [PATCH v5 " James Zhu
@ 2020-03-11 15:13 ` Christian König
0 siblings, 0 replies; 27+ messages in thread
From: Christian König @ 2020-03-11 15:13 UTC (permalink / raw)
To: James Zhu, amd-gfx; +Cc: jamesz
Am 11.03.20 um 16:04 schrieb James Zhu:
> Fix race condition issue when multiple vcn starts are called.
>
> v2: Removed checking the return value of cancel_delayed_work_sync()
> to prevent possible races here.
>
> v3: Add total_submission_cnt to avoid gate power unexpectedly.
>
> v4: Remove extra counter check, and reduce counter before idle
> work schedule
>
> Signed-off-by: James Zhu <James.Zhu@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 21 ++++++++++++++-------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 2 ++
> 2 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index a41272f..6dacf78 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -63,6 +63,8 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
> int i, r;
>
> INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
> + mutex_init(&adev->vcn.vcn_pg_lock);
> + atomic_set(&adev->vcn.total_submission_cnt, 0);
>
> switch (adev->asic_type) {
> case CHIP_RAVEN:
> @@ -210,6 +212,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
> }
>
> release_firmware(adev->vcn.fw);
> + mutex_destroy(&adev->vcn.vcn_pg_lock);
>
> return 0;
> }
> @@ -307,7 +310,7 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
> fences += fence[j];
> }
>
> - if (fences == 0) {
> + if (!fences && !atomic_read(&adev->vcn.total_submission_cnt)) {
> amdgpu_gfx_off_ctrl(adev, true);
> amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> AMD_PG_STATE_GATE);
> @@ -319,13 +322,14 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
> void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
> {
> struct amdgpu_device *adev = ring->adev;
> - bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
>
> - if (set_clocks) {
> - amdgpu_gfx_off_ctrl(adev, false);
> - amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> - AMD_PG_STATE_UNGATE);
> - }
> + atomic_inc(&adev->vcn.total_submission_cnt);
> + cancel_delayed_work_sync(&adev->vcn.idle_work);
> +
> + mutex_lock(&adev->vcn.vcn_pg_lock);
> + amdgpu_gfx_off_ctrl(adev, false);
> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> + AMD_PG_STATE_UNGATE);
>
> if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> struct dpg_pause_state new_state;
> @@ -345,10 +349,13 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
>
> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
> }
> + mutex_unlock(&adev->vcn.vcn_pg_lock);
> }
>
> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
> {
> + atomic_dec(&ring->adev->vcn.total_submission_cnt);
> +
> schedule_delayed_work(&ring->adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 6fe0573..111c4cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -200,6 +200,8 @@ struct amdgpu_vcn {
> struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
> uint32_t num_vcn_enc_sched;
> uint32_t num_vcn_dec_sched;
> + struct mutex vcn_pg_lock;
> + atomic_t total_submission_cnt;
>
> unsigned harvest_config;
> int (*pause_dpg_mode)(struct amdgpu_device *adev,
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread