From cbb7477e8a796211b664ff7e47334cb1b642556d Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 28 Jul 2015 00:29:31 -0700 Subject: [PATCH] vc4: Ensure that the bin CL is properly capped by increment/flush. We don't want anything to appear after we've kicked off the render (and thus job flush), since that might then get written out to the tile allocation state. Signed-off-by: Eric Anholt --- src/gallium/drivers/vc4/kernel/vc4_drv.h | 4 ++ src/gallium/drivers/vc4/kernel/vc4_gem.c | 2 + src/gallium/drivers/vc4/kernel/vc4_validate.c | 56 ++++++++++--------- 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/src/gallium/drivers/vc4/kernel/vc4_drv.h b/src/gallium/drivers/vc4/kernel/vc4_drv.h index 8dc3c11d2d6..5c8617939c8 100644 --- a/src/gallium/drivers/vc4/kernel/vc4_drv.h +++ b/src/gallium/drivers/vc4/kernel/vc4_drv.h @@ -87,6 +87,7 @@ struct vc4_exec_info { bool found_tile_binning_mode_config_packet; bool found_start_tile_binning_packet; bool found_increment_semaphore_packet; + bool found_flush; uint8_t bin_tiles_x, bin_tiles_y; struct drm_gem_cma_object *tile_bo; uint32_t tile_alloc_offset; @@ -98,6 +99,9 @@ struct vc4_exec_info { uint32_t ct0ca, ct0ea; uint32_t ct1ca, ct1ea; + /* Pointer to the unvalidated bin CL (if present). */ + void *bin_u; + /* Pointers to the shader recs. These paddr gets incremented as CL * packets are relocated in validate_gl_shader_state, and the vaddrs * (u and v) get incremented and size decremented as the shader recs diff --git a/src/gallium/drivers/vc4/kernel/vc4_gem.c b/src/gallium/drivers/vc4/kernel/vc4_gem.c index e4b7fea5968..93f9ec7ed9b 100644 --- a/src/gallium/drivers/vc4/kernel/vc4_gem.c +++ b/src/gallium/drivers/vc4/kernel/vc4_gem.c @@ -112,6 +112,8 @@ vc4_get_bcl(struct drm_device *dev, struct vc4_exec_info *exec) exec->ct0ca = exec->exec_bo->paddr + bin_offset; + exec->bin_u = bin; + exec->shader_rec_v = exec->exec_bo->vaddr + shader_rec_offset; exec->shader_rec_p = exec->exec_bo->paddr + shader_rec_offset; exec->shader_rec_size = args->shader_rec_size; diff --git a/src/gallium/drivers/vc4/kernel/vc4_validate.c b/src/gallium/drivers/vc4/kernel/vc4_validate.c index b3d46212fc8..c57ebecbb24 100644 --- a/src/gallium/drivers/vc4/kernel/vc4_validate.c +++ b/src/gallium/drivers/vc4/kernel/vc4_validate.c @@ -132,6 +132,15 @@ vc4_use_handle(struct vc4_exec_info *exec, mode, obj); } +static bool +validate_bin_pos(struct vc4_exec_info *exec, void *untrusted, uint32_t pos) +{ + /* Note that the untrusted pointer passed to these functions is + * incremented past the packet byte. + */ + return (untrusted - 1 == exec->bin_u + pos); +} + static uint32_t gl_shader_rec_size(uint32_t pointer_bits) { @@ -201,14 +210,15 @@ vc4_check_tex_size(struct vc4_exec_info *exec, struct drm_gem_cma_object *fbo, return true; } + static int -validate_flush_all(VALIDATE_ARGS) +validate_flush(VALIDATE_ARGS) { - if (exec->found_increment_semaphore_packet) { - DRM_ERROR("VC4_PACKET_FLUSH_ALL after " - "VC4_PACKET_INCREMENT_SEMAPHORE\n"); - return -EINVAL; + if (!validate_bin_pos(exec, untrusted, exec->args->bin_cl_size - 1)) { + DRM_ERROR("Bin CL must end with VC4_PACKET_FLUSH\n"); + return false; } + exec->found_flush = true; return 0; } @@ -233,17 +243,13 @@ validate_start_tile_binning(VALIDATE_ARGS) static int validate_increment_semaphore(VALIDATE_ARGS) { - if (exec->found_increment_semaphore_packet) { - DRM_ERROR("Duplicate VC4_PACKET_INCREMENT_SEMAPHORE\n"); + if (!validate_bin_pos(exec, untrusted, exec->args->bin_cl_size - 2)) { + DRM_ERROR("Bin CL must end with " + "VC4_PACKET_INCREMENT_SEMAPHORE\n"); return -EINVAL; } exec->found_increment_semaphore_packet = true; - /* Once we've found the semaphore increment, there should be one FLUSH - * then the end of the command list. The FLUSH actually triggers the - * increment, so we only need to make sure there - */ - return 0; } @@ -257,11 +263,6 @@ validate_indexed_prim_list(VALIDATE_ARGS) uint32_t index_size = (*(uint8_t *)(untrusted + 0) >> 4) ? 2 : 1; struct vc4_shader_state *shader_state; - if (exec->found_increment_semaphore_packet) { - DRM_ERROR("Drawing after VC4_PACKET_INCREMENT_SEMAPHORE\n"); - return -EINVAL; - } - /* Check overflow condition */ if (exec->shader_state_count == 0) { DRM_ERROR("shader state must precede primitives\n"); @@ -295,11 +296,6 @@ validate_gl_array_primitive(VALIDATE_ARGS) uint32_t max_index; struct vc4_shader_state *shader_state; - if (exec->found_increment_semaphore_packet) { - DRM_ERROR("Drawing after VC4_PACKET_INCREMENT_SEMAPHORE\n"); - return -EINVAL; - } - /* Check overflow condition */ if (exec->shader_state_count == 0) { DRM_ERROR("shader state must precede primitives\n"); @@ -447,8 +443,8 @@ static const struct cmd_info { } cmd_info[] = { VC4_DEFINE_PACKET(VC4_PACKET_HALT, "halt", NULL), VC4_DEFINE_PACKET(VC4_PACKET_NOP, "nop", NULL), - VC4_DEFINE_PACKET(VC4_PACKET_FLUSH, "flush", NULL), - VC4_DEFINE_PACKET(VC4_PACKET_FLUSH_ALL, "flush all state", validate_flush_all), + VC4_DEFINE_PACKET(VC4_PACKET_FLUSH, "flush", validate_flush), + VC4_DEFINE_PACKET(VC4_PACKET_FLUSH_ALL, "flush all state", NULL), VC4_DEFINE_PACKET(VC4_PACKET_START_TILE_BINNING, "start tile binning", validate_start_tile_binning), VC4_DEFINE_PACKET(VC4_PACKET_INCREMENT_SEMAPHORE, "increment semaphore", validate_increment_semaphore), @@ -554,8 +550,16 @@ vc4_validate_bin_cl(struct drm_device *dev, return -EINVAL; } - if (!exec->found_increment_semaphore_packet) { - DRM_ERROR("Bin CL missing VC4_PACKET_INCREMENT_SEMAPHORE\n"); + /* The bin CL must be ended with INCREMENT_SEMAPHORE and FLUSH. The + * semaphore is used to trigger the render CL to start up, and the + * FLUSH is what caps the bin lists with + * VC4_PACKET_RETURN_FROM_SUB_LIST (so they jump back to the main + * render CL when they get called to) and actually triggers the queued + * semaphore increment. + */ + if (!exec->found_increment_semaphore_packet || !exec->found_flush) { + DRM_ERROR("Bin CL missing VC4_PACKET_INCREMENT_SEMAPHORE + " + "VC4_PACKET_FLUSH\n"); return -EINVAL; } -- 2.30.2