From c874caf33dddf3fec9b3c78223192a11edc57ab8 Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Tue, 2 Apr 2024 11:05:32 +0200 Subject: [PATCH] v3dv: fix job pointers from cloned CLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We had these pointing to the original job instead of pointing to the cloned job. This can be confusing, particularly, if we then emit commands that include references to new BOs into the cloned jobs, since we would then try to insert these BOs in the original jobs instead of the clones, which was the situation we had when we implemented resume address patching with dynamic rendering. Reviewed-by: Alejandro Piñeiro Part-of: --- src/broadcom/vulkan/v3dv_cmd_buffer.c | 5 +++++ src/broadcom/vulkan/v3dvx_cmd_buffer.c | 20 -------------------- 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/src/broadcom/vulkan/v3dv_cmd_buffer.c b/src/broadcom/vulkan/v3dv_cmd_buffer.c index a510cbe7df0..6d9e198b05f 100644 --- a/src/broadcom/vulkan/v3dv_cmd_buffer.c +++ b/src/broadcom/vulkan/v3dv_cmd_buffer.c @@ -1942,6 +1942,11 @@ v3dv_job_clone(struct v3dv_job *job) if (job->type == V3DV_JOB_TYPE_GPU_CL) { assert(job->cmd_buffer); struct v3dv_device *device = job->cmd_buffer->device; + + clone->bcl.job = clone; + clone->rcl.job = clone; + clone->indirect.job = clone; + if (!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)) diff --git a/src/broadcom/vulkan/v3dvx_cmd_buffer.c b/src/broadcom/vulkan/v3dvx_cmd_buffer.c index 6b4cf81dc40..fa64646ff35 100644 --- a/src/broadcom/vulkan/v3dvx_cmd_buffer.c +++ b/src/broadcom/vulkan/v3dvx_cmd_buffer.c @@ -2756,26 +2756,6 @@ v3dX(job_patch_resume_address)(struct v3dv_job *first_suspend, assert(suspend && suspend->suspending); assert(suspend->suspend_branch_inst_ptr != NULL); - /* We need to be extra careful when patching resuming addresses when - * secondary command buffers are involved: we execute secondaries by - * cloning them into the primary, but the cloning is not deep, which - * means the command lists still point to the original job we cloned - * from. This is important when we are patching the resume address - * since the BRANCH instruction will try to add the BO of the address - * to the original job (obtianed thought the BCL reference) instead of - * the clone and this will cause a mismatch between the bo_count in the - * clone job and its bo_set. To avoid that, we specify the address through - * an absolute offset rather than a bo + relative_offset and (we also - * avoid specifying the BCL list in the cl_packet_pack call below for extra - * safety). This will ensure the BO is not added automatically when packing - * the branch instruction. We are going to manually add all the BOs from the - * resume job into the first suspended job right below anyway, so this is - * fine. - * - * FIXME: this is very hacky, maybe we should consider making proper clones - * of the secondaries when we merge them into primaries to avoid - * this kind of situations. - */ struct v3dv_bo *resume_bo = list_first_entry(&resume->bcl.bo_list, struct v3dv_bo, list_link); struct cl_packet_struct(BRANCH) branch = {