From: Iago Toral Quiroga Date: Thu, 10 Mar 2022 15:18:10 +0000 (+0100) Subject: v3dv: don't signal semaphores/fences from a wait thread X-Git-Tag: upstream/22.3.5~11536 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=3b8ab8a9ce70495b39f3b40188b38fe1c3d93c82;p=platform%2Fupstream%2Fmesa.git v3dv: don't signal semaphores/fences from a wait thread When we have a wait thread we can't ensure that the last job in the last command buffer will be the one to signal semaphores because in this case there is no gurantee that jobs from command buffers in the batch will be submitted to the GPU in order, as those put in a wait thread will be submitted later when the event wait operation is completed. Instead, we need to wait for all outstanding wait threads to complete and only then we should signal any semaphores or fences. This also fixes a bug where the wait for events was the last job in the command buffer. In this case, once the event wait is completed we have no additional jobs to submit and thus would never try to signal semaphores or fences. Reviewed-by: Melissa Wen Part-of: --- diff --git a/src/broadcom/vulkan/v3dv_queue.c b/src/broadcom/vulkan/v3dv_queue.c index 88ee3c5..ea87de5 100644 --- a/src/broadcom/vulkan/v3dv_queue.c +++ b/src/broadcom/vulkan/v3dv_queue.c @@ -416,6 +416,15 @@ event_wait_thread_func(void *_info) struct v3dv_queue *queue = &job->device->queue; list_for_each_entry_from(struct v3dv_job, pjob, job->list_link.next, &job->cmd_buffer->jobs, list_link) { + /* We can't signal semaphores from wait threads because in this case + * we can't ensure job completion order any more (i.e. if the wait for + * events is in the first command buffer of a batch then the last job + * from the last command buffer in that batch can't signal). We always + * need to signal from the master thread in that case, when we know we + * are done submitting all jobs from all command buffers. + */ + pjob->do_sem_signal = false; + /* We don't want to spawn more than one wait thread per command buffer. * If this job also requires a wait for events, we will do the wait here. */ @@ -628,16 +637,18 @@ fence_get_sync(struct v3dv_fence *fence) static VkResult process_semaphores_to_signal(struct v3dv_device *device, - uint32_t count, const VkSemaphore *sems) + uint32_t count, const VkSemaphore *sems, + bool is_master_thread) { if (count == 0) return VK_SUCCESS; /* If multisync is supported, we are signalling semaphores in the last job * of the last command buffer and, therefore, we do not need to process any - * semaphores here. + * semaphores here, unless we come from a wait thread, because in that case + * we never signal. */ - if (device->pdevice->caps.multisync) + if (device->pdevice->caps.multisync && !is_master_thread) return VK_SUCCESS; int render_fd = device->pdevice->render_fd; @@ -1346,13 +1357,6 @@ add_signal_semaphores_to_wait_list(struct v3dv_device *device, if (pSubmit->signalSemaphoreCount == 0) return; - /* If multisync is supported, we just signal semaphores in the last job of - * the last command buffer and, therefore, we do not need to add any - * semaphores here. - */ - if (device->pdevice->caps.multisync) - return; - /* Otherwise, we put all the semaphores in a list and we signal all of them * together from the submit master thread when the last wait thread in the * submit completes. @@ -1451,7 +1455,8 @@ queue_submit_cmd_buffer_batch(struct v3dv_queue *queue, if (!has_wait_threads) { return process_semaphores_to_signal(queue->device, pSubmit->signalSemaphoreCount, - pSubmit->pSignalSemaphores); + pSubmit->pSignalSemaphores, + false); } else { assert(*wait_info); add_signal_semaphores_to_wait_list(queue->device, pSubmit, *wait_info); @@ -1478,7 +1483,8 @@ master_wait_thread_func(void *_wait_info) VkResult result; result = process_semaphores_to_signal(wait_info->device, wait_info->signal_semaphore_count, - wait_info->signal_semaphores); + wait_info->signal_semaphores, + true); if (result != VK_SUCCESS) fprintf(stderr, "Wait thread semaphore signaling failed.");