diff --git a/src/broadcom/vulkan/v3dv_cmd_buffer.c b/src/broadcom/vulkan/v3dv_cmd_buffer.c index 6d9e198b05f..7b49a7aadec 100644 --- a/src/broadcom/vulkan/v3dv_cmd_buffer.c +++ b/src/broadcom/vulkan/v3dv_cmd_buffer.c @@ -149,9 +149,21 @@ job_destroy_cloned_gpu_cl_resources(struct v3dv_job *job) { assert(job->type == V3DV_JOB_TYPE_GPU_CL); - list_for_each_entry_safe(struct v3dv_bo, bo, &job->bcl.bo_list, list_link) { - list_del(&bo->list_link); - vk_free(&job->device->vk.alloc, bo); + struct v3dv_cmd_buffer *cmd_buffer = job->cmd_buffer; + if (job->clone_owns_bcl) { + /* For suspending jobs in command buffers with the simultaneous use flag + * we allocate a real copy of the BCL. + */ + assert(job->suspending && + cmd_buffer && + (cmd_buffer->usage_flags & + VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT)); + v3dv_cl_destroy(&job->bcl); + } else { + list_for_each_entry_safe(struct v3dv_bo, bo, &job->bcl.bo_list, list_link) { + list_del(&bo->list_link); + vk_free(&job->device->vk.alloc, bo); + } } list_for_each_entry_safe(struct v3dv_bo, bo, &job->rcl.bo_list, list_link) { @@ -1923,7 +1935,7 @@ clone_bo_list(struct v3dv_device *device, } struct v3dv_job * -v3dv_job_clone(struct v3dv_job *job) +v3dv_job_clone(struct v3dv_job *job, bool skip_bcl) { struct v3dv_job *clone = vk_alloc(&job->device->vk.alloc, sizeof(struct v3dv_job), 8, @@ -1931,7 +1943,16 @@ v3dv_job_clone(struct v3dv_job *job) if (!clone) return NULL; - /* Cloned jobs don't duplicate resources! */ + /* Cloned jobs don't duplicate resources, they share their CLs with the + * oringinal job, since they are typically read-only. The exception to this + * is dynamic rendering suspension paired with + * VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT, since in that case we need + * to patch the BCL with the resume address and for that we need to create a + * copy of the job so we avoid rewriting the resume address for another copy + * of the same job that may be running in the GPU. When we create a job for + * this use case skip_bcl is set to True and the caller will be responsible + * for creating the BCL. + */ *clone = *job; clone->is_clone = true; clone->cmd_buffer = NULL; @@ -1947,8 +1968,10 @@ v3dv_job_clone(struct v3dv_job *job) clone->rcl.job = clone; clone->indirect.job = clone; - if (!clone_bo_list(device, &clone->bcl.bo_list, &job->bcl.bo_list)) + if (!skip_bcl && + !clone_bo_list(device, &clone->bcl.bo_list, &job->bcl.bo_list)) { return NULL; + } if (!clone_bo_list(device, &clone->rcl.bo_list, &job->rcl.bo_list)) return NULL; if (!clone_bo_list(device, &clone->indirect.bo_list, &job->indirect.bo_list)) @@ -1968,7 +1991,7 @@ struct v3dv_job * v3dv_job_clone_in_cmd_buffer(struct v3dv_job *job, struct v3dv_cmd_buffer *cmd_buffer) { - struct v3dv_job *clone = v3dv_job_clone(job); + struct v3dv_job *clone = v3dv_job_clone(job, false); if (!clone) { v3dv_flag_oom(cmd_buffer, NULL); return NULL; diff --git a/src/broadcom/vulkan/v3dv_private.h b/src/broadcom/vulkan/v3dv_private.h index fe2611ff40c..cab80f7e172 100644 --- a/src/broadcom/vulkan/v3dv_private.h +++ b/src/broadcom/vulkan/v3dv_private.h @@ -1235,6 +1235,12 @@ struct v3dv_job { */ bool is_clone; + /* If this is a cloned job, if it has its own BCL resource. This happens + * when we suspend jobs with in command buffers with the + * VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT flag. + */ + bool clone_owns_bcl; + /* VK_KHR_dynamic_rendering */ bool suspending; bool resuming; @@ -1398,7 +1404,7 @@ void v3dv_job_start_frame(struct v3dv_job *job, bool v3dv_job_type_is_gpu(struct v3dv_job *job); struct v3dv_job * -v3dv_job_clone(struct v3dv_job *job); +v3dv_job_clone(struct v3dv_job *job, bool skip_bcl); struct v3dv_job * v3dv_job_clone_in_cmd_buffer(struct v3dv_job *job, diff --git a/src/broadcom/vulkan/v3dv_queue.c b/src/broadcom/vulkan/v3dv_queue.c index 41277f04059..ac981984c4f 100644 --- a/src/broadcom/vulkan/v3dv_queue.c +++ b/src/broadcom/vulkan/v3dv_queue.c @@ -1228,11 +1228,6 @@ v3dv_queue_driver_submit(struct vk_queue *vk_queue, for (int i = 0; i < V3DV_QUEUE_COUNT; i++) queue->last_job_syncs.first[i] = true; - /* FIXME: if suspend/resume chains are recorded into command buffers with - * usage flag VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT, then this won't - * work and we would need to "clone" the jobs instead so we never patch a - * job that may still be executing. - */ struct v3dv_job *first_suspend_job = NULL; struct v3dv_job *current_suspend_job = NULL; for (uint32_t i = 0; i < submit->command_buffer_count; i++) { @@ -1240,6 +1235,13 @@ v3dv_queue_driver_submit(struct vk_queue *vk_queue, container_of(submit->command_buffers[i], struct v3dv_cmd_buffer, vk); list_for_each_entry_safe(struct v3dv_job, job, &cmd_buffer->jobs, list_link) { + if (job->suspending) { + job = v3dv_X(job->device, + cmd_buffer_prepare_suspend_job_for_submit)(job); + if (!job) + return VK_ERROR_OUT_OF_DEVICE_MEMORY; + } + if (job->suspending && !job->resuming) { assert(!first_suspend_job); assert(!current_suspend_job); diff --git a/src/broadcom/vulkan/v3dvx_cmd_buffer.c b/src/broadcom/vulkan/v3dvx_cmd_buffer.c index fa64646ff35..e15343b442a 100644 --- a/src/broadcom/vulkan/v3dvx_cmd_buffer.c +++ b/src/broadcom/vulkan/v3dvx_cmd_buffer.c @@ -2776,3 +2776,111 @@ v3dX(job_patch_resume_address)(struct v3dv_job *first_suspend, first_suspend->suspended_bcl_end = resume->bcl.bo->offset + v3dv_cl_offset(&resume->bcl); } + +static void +job_destroy_cb(VkDevice device, uint64_t pobj, VkAllocationCallbacks *allocb) +{ + struct v3dv_job *clone = (struct v3dv_job *) (uintptr_t) pobj; + v3dv_job_destroy(clone); +} + +/** + * This checks if the command buffer has been created with + * VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT, in which case we won't be + * able to safely patch the resume address into the job (since we could have + * another instance of this job running in the GPU, potentially resuming in a + * different address). In that case, we clone the job and make the clone have + * its own BCL copied from the original job so we can later patch the resume + * address into it safely. + */ +struct v3dv_job * +v3dX(cmd_buffer_prepare_suspend_job_for_submit)(struct v3dv_job *job) +{ + assert(job->suspending); + assert(job->cmd_buffer); + assert(job->type == V3DV_JOB_TYPE_GPU_CL); + + if (!(job->cmd_buffer->usage_flags & VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT)) + return job; + + /* Create the clone job, but skip the BCL since we are going to create + * our own below. + */ + struct v3dv_job *clone = v3dv_job_clone(job, true); + if (!clone) + return NULL; + + /* Compute total size of BCL we need to copy */ + uint32_t bcl_size = 0; + list_for_each_entry(struct v3dv_bo, bo, &job->bcl.bo_list, list_link) + bcl_size += bo->size; + + /* Prepare the BCL for the cloned job. For this we go over the BOs in the + * BCL of the original job and we copy their contents into the single BO + * in the BCL of the cloned job. + */ + clone->clone_owns_bcl = true; + v3dv_cl_init(clone, &clone->bcl); + v3dv_cl_ensure_space(&clone->bcl, bcl_size, 4); + if (!clone->bcl.bo) + return NULL; + + assert(clone->bcl.base); + assert(clone->bcl.base == clone->bcl.next); + + /* Unlink this job from the command buffer's execution list */ + list_inithead(&clone->list_link); + + /* Copy the contents of each BO in the original job's BCL into the single + * BO we have in the clone's BCL. + * + * If the BO is the last in the BCL (which we can tell because it wouldn't + * have emitted a BRANCH instruction to link to another BO) we need to copy + * up to the current BCL offset, otherwise we need to copy up to the BRANCH + * instruction (excluded, since we are putting everything together into a + * single BO here). + */ + list_for_each_entry(struct v3dv_bo, bo, &job->bcl.bo_list, list_link) { + assert(bo->map); + uint32_t copy_size; + if (bo->cl_branch_offset == 0xffffffff) { /* Last BO in BCL */ + assert(bo == list_last_entry(&job->bcl.bo_list, struct v3dv_bo, list_link)); + copy_size = v3dv_cl_offset(&job->bcl); + } else { + assert(bo->cl_branch_offset >= cl_packet_length(BRANCH)); + copy_size = bo->cl_branch_offset - cl_packet_length(BRANCH); + } + + assert(v3dv_cl_offset(&job->bcl) + copy_size < bcl_size); + memcpy(cl_start(&clone->bcl), bo->map, copy_size); + cl_advance_and_end(&clone->bcl, copy_size); + } + + /* Now we need to fixup the pointer to the suspend BRANCH instruction at the + * end of the BCL so it points to the address in the new BCL. We know that + * to suspend a command buffer we always emit a BRANCH+NOP combo, so we just + * need to go back that many bytes in to the BCL to find the instruction. + */ + uint32_t suspend_terminator_size = + cl_packet_length(BRANCH) + cl_packet_length(NOP); + clone->suspend_branch_inst_ptr = (struct v3dv_cl_out *) + (((uint8_t *)cl_start(&clone->bcl)) - suspend_terminator_size); + assert(*(((uint8_t *)clone->suspend_branch_inst_ptr)) == V3DX(BRANCH_opcode)); + + /* This job is not in the execution list of the command buffer so it + * won't be destroyed with it; add it as a private object to get it freed. + * + * FIXME: every time this job is submitted we clone the job and we only + * destroy it when the command buffer is destroyed. If the user keeps the + * command buffer for the entire lifetime of the application, this command + * buffer could grow significantly, so maybe we want to do something smarter + * like having a syncobj bound to these jobs and every time we submit the + * command buffer again we first check these sncobjs to see if we can free + * some of these clones so we avoid blowing up memory. + */ + v3dv_cmd_buffer_add_private_obj( + job->cmd_buffer, (uintptr_t)clone, + (v3dv_cmd_buffer_private_obj_destroy_cb)job_destroy_cb); + + return clone; +} diff --git a/src/broadcom/vulkan/v3dvx_private.h b/src/broadcom/vulkan/v3dvx_private.h index adaa2b6dcf7..061a52ea6a2 100644 --- a/src/broadcom/vulkan/v3dvx_private.h +++ b/src/broadcom/vulkan/v3dvx_private.h @@ -130,6 +130,9 @@ v3dX(cmd_buffer_emit_indexed_indirect)(struct v3dv_cmd_buffer *cmd_buffer, void v3dX(cmd_buffer_suspend)(struct v3dv_cmd_buffer *cmd_buffer); +struct v3dv_job * +v3dX(cmd_buffer_prepare_suspend_job_for_submit)(struct v3dv_job *job); + void v3dX(get_hw_clear_color)(const VkClearColorValue *color, uint32_t internal_type,