Abdiel Janulgue <abdiel.janul...@linux.intel.com> writes:
> If there are UBO constant entries, append them to
> stage_state->push_const_size.
> The gather pool contains the combined entries of both ordinary uniforms
> and UBO constants.
>
> Signed-off-by: Abdiel Janulgue <abdiel.janul...@linux.intel.com>
> ---
> src/mesa/drivers/dri/i965/gen6_vs_state.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c
> b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> index b78166e..843df94 100644
> --- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> @@ -59,7 +59,9 @@ gen6_upload_push_constants(struct brw_context *brw,
> struct gl_context *ctx = &brw->ctx;
>
> if (prog_data->nr_params == 0) {
> - stage_state->push_const_size = 0;
> + if (prog_data->nr_ubo_params == 0) {
> + stage_state->push_const_size = 0;
> + }
The 'if (prog_data->nr_ubo_params == 0)' conditional here only seems to
be required because you don't assign stage_state->push_const_size
consistently below depending on whether 'prog_data->nr_params == 0' or
not. Why don't you drop this assignment, the
'stage_state->push_const_size = ALIGN(prog_data->nr_params, 8) / 8;'
assignment in the other branch of this if, and the
'stage_state->push_const_size = ALIGN(prog_data->nr_params +
prog_data->nr_ubo_params, 8) / 8;' assignment introduced in this patch
and instead assign it once, right before the ' if (brw->gather_pool.bo
!= NULL)' conditional, like:
| stage_state->push_const_size = DIV_ROUND_UP(prog_data->nr_params +
| prog_data->nr_ubo_params, 8);
> } else {
> /* Updates the ParamaterValues[i] pointers for all parameters of the
> * basic type of PROGRAM_STATE_VAR.
> @@ -122,10 +124,24 @@ gen6_upload_push_constants(struct brw_context *brw,
> }
> /* Allocate gather pool space for uniform and UBO entries in 512-bit
> chunks*/
> if (brw->gather_pool.bo != NULL) {
> + unsigned gather_pool_next_offset = brw->gather_pool.next_offset;
> +
Set stage_state->push_const_offset to brw->gather_pool.next_offset up
front to avoid the need for this temporary.
> if (prog_data->nr_params > 0) {
> int num_consts = ALIGN(prog_data->nr_params, 4) / 4;
> + gather_pool_next_offset += (ALIGN(num_consts, 4) / 4) * 64;
> + }
> +
> + if (prog_data->nr_ubo_params > 0) {
> + stage_state->push_const_size = ALIGN(prog_data->nr_params +
> prog_data->nr_ubo_params, 8) / 8;
> + uint32_t num_constants = ALIGN(prog_data->nr_ubo_params, 4) / 4;
> + gather_pool_next_offset += (ALIGN(num_constants, 4) / 4) * 64;
> + }
You set push_const_size to the 32B-aligned value of the sum of nr_params
and nr_ubo_params, but then increment gather_pool.next_offset by the sum
of the separately aligned values of both terms, which AFAICT may
overestimate the amount of space actually used from the pool. I suggest
you drop the two conditionals above and instead do:
| brw->gather_pool.next_offset += DIV_ROUND_UP(prog_data->nr +
prog_data->nr_ubo_params, 16) * 64;
> +
> + if (gather_pool_next_offset > brw->gather_pool.next_offset) {
> stage_state->push_const_offset = brw->gather_pool.next_offset;
> - brw->gather_pool.next_offset += (ALIGN(num_consts, 4) / 4) * 64;
> + brw->gather_pool.next_offset = gather_pool_next_offset;
> + assert(brw->gather_pool.next_offset < brw->gather_pool.bo->size);
> + assert(stage_state->push_const_offset <
> brw->gather_pool.next_offset);
> }
This block should also become unnecessary if you make the changes I
suggested earlier.
> }
> }
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
