On 28.01.2015 04:05, Tom Stellard wrote:
> v2:
> - Only emit write SPI_TMPRING_SIZE once per packet.
> - Use context global scratch buffer.
>
> v3:
> - Patch shaders using WRITE_DATA packet instead of map/unmap.
> - Emit ICACHE_FLUSH, CS_PARTIAL_FLUSH, PS_PARTIAL_FLUSH, and
> VS_PARTIAL_FLUSH when patching shaders.
>
> v4:
> - Code cleanups.
> - Remove unnecessary multiplies.
>
> v5:
> - Patch shaders in system memory and re-upload to vram.
[...]
> @@ -158,6 +159,12 @@ static struct pipe_context *si_create_context(struct
> pipe_screen *screen, void *
> sctx->null_const_buf.buffer->width0, 0,
> false);
> }
>
> + /* XXX: This is the maximum value allowed. I'm not sure how compute
> + * this for non-cs shaders. Using the wrong value here can result in
> + * GPU lockups, but the maximum value seems to always work.
> + */
Missing 'to' in "I'm not sure how to compute".
> @@ -571,6 +571,21 @@ void si_draw_vbo(struct pipe_context *ctx, const struct
> pipe_draw_info *info)
> if (sctx->b.flags)
> sctx->atoms.s.cache_flush->dirty = true;
>
> +
> + if (sctx->emit_scratch_reloc) {
Extraneous blank line before the if statement.
> + /* Update the shader state to use the new shader bo. */
> + si_shader_init_pm4_state(shader);
Indentation of the function call looks off.
> +static unsigned si_get_max_scratch_bytes_per_wave(struct si_context *sctx)
> +{
> +
> + return MAX3(si_get_scratch_buffer_bytes_per_wave(sctx, sctx->ps_shader),
> + si_get_scratch_buffer_bytes_per_wave(sctx,
> sctx->gs_shader),
> + si_get_scratch_buffer_bytes_per_wave(sctx,
> sctx->vs_shader));
> +}
[...]
> + if (scratch_needed_size > 0) {
> +
> + if (scratch_needed_size > current_scratch_buffer_size) {
> +
> + /* Create a bigger scratch buffer */
Extraneous blank lines at the beginning of the blocks.
> + /* Update the shaders, so they are using the latest scratch.
> The
> + * scratch buffer may have been changed since these shaders were
> + * last used, so we still need to try to update them, even if
> + * they require scratch buffers smaller than the current size.
> + */
> + if (si_update_scratch_buffer(sctx, sctx->ps_shader)) {
> + si_pm4_bind_state(sctx, ps,
> sctx->ps_shader->current->pm4);
> + sctx->emitted.named.ps = NULL;
> + }
> + if (si_update_scratch_buffer(sctx, sctx->gs_shader)) {
> + si_pm4_bind_state(sctx, gs,
> sctx->gs_shader->current->pm4);
> + sctx->emitted.named.gs = NULL;
> + }
> + if (si_update_scratch_buffer(sctx, sctx->vs_shader)) {
> + si_pm4_bind_state(sctx, vs,
> sctx->vs_shader->current->pm4);
> + sctx->emitted.named.vs = NULL;
> + }
Setting the emitted states to NULL isn't necessary. I think Marek made
that suggestion based on the assumption that the PM4 states would be
modified instead of replaced by new ones, but I don't think that's
practical.
In patch 4, I spotted a typo: 'beofre'
Other than that, the series is
Reviewed-by: Michel Dänzer <michel.daen...@amd.com>
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
