<html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix">Am 11.03.20 um 12:30 schrieb Zhu, James:<br> </div> <blockquote type="cite" cite="mid:BYAPR12MB32851E367C4D553AABB13BB7E4FC0@BYAPR12MB3285.namprd12.prod.outlook.com"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> <p style="font-family:Arial;font-size:10pt;color:#0078D7;margin:15pt;" align="Left"> [AMD Official Use Only - Internal Distribution Only]<br> </p> <br> <div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> ping<br> </div> <div> <div id="Signature"><br> <div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" face="Calibri, sans-serif" color="#000000"><b>From:</b> Zhu, James <a class="moz-txt-link-rfc2396E" href="mailto:James.Zhu@amd.com"><James.Zhu@amd.com></a><br> <b>Sent:</b> Monday, March 9, 2020 12:57 PM<br> <b>To:</b> <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <a class="moz-txt-link-rfc2396E" href="mailto:amd-gfx@lists.freedesktop.org"><amd-gfx@lists.freedesktop.org></a><br> <b>Cc:</b> Zhu, James <a class="moz-txt-link-rfc2396E" href="mailto:James.Zhu@amd.com"><James.Zhu@amd.com></a>; Koenig, Christian <a class="moz-txt-link-rfc2396E" href="mailto:Christian.Koenig@amd.com"><Christian.Koenig@amd.com></a><br> <b>Subject:</b> [PATCH v3 1/4] drm/amdgpu/vcn: fix race condition issue for vcn start</font> <div> </div> </div> <div class="BodyFragment"><font size="2"><span style="font-size:11pt"> <div class="PlainText">Fix race condition issue when multiple vcn starts are called.<br> <br> v2: Removed checking the return value of cancel_delayed_work_sync()<br> to prevent possible races here.<br> <br> v3: Add total_submission_cnt to avoid gate power unexpectedly.<br> <br> Signed-off-by: James Zhu <a class="moz-txt-link-rfc2396E" href="mailto:James.Zhu@amd.com"><James.Zhu@amd.com></a><br> ---<br> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 22 +++++++++++++++-------<br> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 2 ++<br> 2 files changed, 17 insertions(+), 7 deletions(-)<br> <br> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c<br> index a41272f..6aafda1 100644<br> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c<br> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c<br> @@ -63,6 +63,8 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)<br> int i, r;<br> <br> INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);<br> + mutex_init(&adev->vcn.vcn_pg_lock);<br> + atomic_set(&adev->vcn.total_submission_cnt, 0);<br> <br> switch (adev->asic_type) {<br> case CHIP_RAVEN:<br> @@ -210,6 +212,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)<br> }<br> <br> release_firmware(adev->vcn.fw);<br> + mutex_destroy(&adev->vcn.vcn_pg_lock);<br> <br> return 0;<br> }<br> @@ -307,7 +310,8 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)<br> fences += fence[j];<br> }<br> <br> - if (fences == 0) {<br> + if (fences == 0 &&<br> + likely(atomic_read(&adev->vcn.total_submission_cnt) == 0)) {<br> amdgpu_gfx_off_ctrl(adev, true);<br> amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,<br> AMD_PG_STATE_GATE);<br> @@ -319,13 +323,14 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)<br> void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)<br> {<br> struct amdgpu_device *adev = ring->adev;<br> - bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);<br> <br> - if (set_clocks) {<br> - amdgpu_gfx_off_ctrl(adev, false);<br> - amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,<br> - AMD_PG_STATE_UNGATE);<br> - }<br> + atomic_inc(&adev->vcn.total_submission_cnt);<br> + cancel_delayed_work_sync(&adev->vcn.idle_work);<br> +<br> + mutex_lock(&adev->vcn.vcn_pg_lock);<br> + amdgpu_gfx_off_ctrl(adev, false);<br> + amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,<br> + AMD_PG_STATE_UNGATE);<br> <br> if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {<br> struct dpg_pause_state new_state;<br> @@ -345,11 +350,14 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)<br> <br> adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);<br> }<br> + mutex_unlock(&adev->vcn.vcn_pg_lock);<br> }<br> <br> void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)<br> {<br> schedule_delayed_work(&ring->adev->vcn.idle_work, VCN_IDLE_TIMEOUT);<br> + if (unlikely(atomic_dec_return(&ring->adev->vcn.total_submission_cnt) < 0))<br> + atomic_set(&ring->adev->vcn.total_submission_cnt, 0);<br> </div> </span></font></div> </div> </div> </div> </div> </blockquote> <br> You need to decrement first and then call <font size="2"><span style="font-size:11pt">schedule_delayed_work() otherwise the work could run with the wrong counter.<br> <br> And the extra check for an under run should be superfluous.<br> <br> Regards,<br> Christian.<br> </span></font><br> <blockquote type="cite" cite="mid:BYAPR12MB32851E367C4D553AABB13BB7E4FC0@BYAPR12MB3285.namprd12.prod.outlook.com"> <div> <div> <div id="Signature"> <div> <div class="BodyFragment"><font size="2"><span style="font-size:11pt"> <div class="PlainText"> }<br> <br> int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring)<br> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h<br> index 6fe0573..111c4cc 100644<br> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h<br> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h<br> @@ -200,6 +200,8 @@ struct amdgpu_vcn {<br> struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];<br> uint32_t num_vcn_enc_sched;<br> uint32_t num_vcn_dec_sched;<br> + struct mutex vcn_pg_lock;<br> + atomic_t total_submission_cnt;<br> <br> unsigned harvest_config;<br> int (*pause_dpg_mode)(struct amdgpu_device *adev,<br> -- <br> 2.7.4<br> <br> </div> </span></font></div> </div> </div> </div> </div> </blockquote> <br> </body> </html>