[Mesa-dev] [PATCH 3/3] r600g: implement fragment and vertex color clamp
Vadim Girlin
vadimgirlin at gmail.com
Fri Jun 24 15:22:20 PDT 2011
On 06/24/2011 11:38 PM, Jerome Glisse wrote:
> On Fri, Jun 24, 2011 at 12:29 PM, Vadim Girlin<vadimgirlin at gmail.com> wrote:
>> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=38440
>>
>> Signed-off-by: Vadim Girlin<vadimgirlin at gmail.com>
>
> As discussed previously, there is better to handle this. I think best
> solution is to always add the instruction and to conditionally execute
> them thanks to the boolean constant. If this reveal to have a too big
> impact on shader, other solution i see is adding a cf block with those
> instructions and to enable or disable that block (cf_nop) and reupload
> shader that would avoid a rebuild.
I know its not optimal to do a full rebuild, but rebuild is needed only
when the application will use the same shader in different clamping
states. It won't be a problem if the application doesn't change clamping
state or if it changes the state but uses each shader in one state only.
So assuming that typical app will not use one shader in both states, it
shouldn't be a problem. Is this assumption wrong? I'm not really sure
because I have no much experience in this. But if it's wrong then it's
probably better for performance to build and cache both versions.
Also it seems last write to color output components is often done with
op2 instructions, so there is another optimization possible - to find
last op2 writes and set clamp bit instead of using additional
instructions. Probably it's not very hard to implement with this patch.
AFAICS it will be impossible with other suggested solutions.
Thanks for pushing it.
Regards,
Vadim
>
> But as a mean time solution i think this patch is ok
>
> Cheers,
> Jerome
>
>> ---
>> src/gallium/drivers/r600/evergreen_state.c | 2 +
>> src/gallium/drivers/r600/r600_pipe.c | 2 +-
>> src/gallium/drivers/r600/r600_pipe.h | 7 +++-
>> src/gallium/drivers/r600/r600_shader.c | 52 +++++++++++++++++++++++---
>> src/gallium/drivers/r600/r600_shader.h | 1 +
>> src/gallium/drivers/r600/r600_state.c | 2 +
>> src/gallium/drivers/r600/r600_state_common.c | 30 ++++++++++++++-
>> 7 files changed, 87 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/gallium/drivers/r600/evergreen_state.c b/src/gallium/drivers/r600/evergreen_state.c
>> index f86e4d4..dfe7896 100644
>> --- a/src/gallium/drivers/r600/evergreen_state.c
>> +++ b/src/gallium/drivers/r600/evergreen_state.c
>> @@ -256,6 +256,8 @@ static void *evergreen_create_rs_state(struct pipe_context *ctx,
>> }
>>
>> rstate =&rs->rstate;
>> + rs->clamp_vertex_color = state->clamp_vertex_color;
>> + rs->clamp_fragment_color = state->clamp_fragment_color;
>> rs->flatshade = state->flatshade;
>> rs->sprite_coord_enable = state->sprite_coord_enable;
>>
>> diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c
>> index 38801d6..12599bf 100644
>> --- a/src/gallium/drivers/r600/r600_pipe.c
>> +++ b/src/gallium/drivers/r600/r600_pipe.c
>> @@ -377,6 +377,7 @@ static int r600_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
>> case PIPE_CAP_TGSI_FS_COORD_PIXEL_CENTER_HALF_INTEGER:
>> case PIPE_CAP_SM3:
>> case PIPE_CAP_SEAMLESS_CUBE_MAP:
>> + case PIPE_CAP_FRAGMENT_COLOR_CLAMP_CONTROL:
>> return 1;
>>
>> /* Supported except the original R600. */
>> @@ -392,7 +393,6 @@ static int r600_get_param(struct pipe_screen* pscreen, enum pipe_cap param)
>> /* Unsupported features. */
>> case PIPE_CAP_STREAM_OUTPUT:
>> case PIPE_CAP_PRIMITIVE_RESTART:
>> - case PIPE_CAP_FRAGMENT_COLOR_CLAMP_CONTROL:
>> case PIPE_CAP_TGSI_INSTANCEID:
>> case PIPE_CAP_TGSI_FS_COORD_ORIGIN_LOWER_LEFT:
>> case PIPE_CAP_TGSI_FS_COORD_PIXEL_CENTER_INTEGER:
>> diff --git a/src/gallium/drivers/r600/r600_pipe.h b/src/gallium/drivers/r600/r600_pipe.h
>> index 63ddd39..dc9aad0 100644
>> --- a/src/gallium/drivers/r600/r600_pipe.h
>> +++ b/src/gallium/drivers/r600/r600_pipe.h
>> @@ -88,6 +88,8 @@ struct r600_pipe_sampler_view {
>>
>> struct r600_pipe_rasterizer {
>> struct r600_pipe_state rstate;
>> + boolean clamp_vertex_color;
>> + boolean clamp_fragment_color;
>> boolean flatshade;
>> unsigned sprite_coord_enable;
>> float offset_units;
>> @@ -125,6 +127,7 @@ struct r600_pipe_shader {
>> struct r600_bo *bo;
>> struct r600_bo *bo_fetch;
>> struct r600_vertex_element vertex_elements;
>> + struct tgsi_token *tokens;
>> };
>>
>> struct r600_pipe_sampler_state {
>> @@ -202,6 +205,8 @@ struct r600_pipe_context {
>> struct pipe_query *saved_render_cond;
>> unsigned saved_render_cond_mode;
>> /* shader information */
>> + boolean clamp_vertex_color;
>> + boolean clamp_fragment_color;
>> boolean spi_dirty;
>> unsigned sprite_coord_enable;
>> boolean flatshade;
>> @@ -265,7 +270,7 @@ void r600_init_query_functions(struct r600_pipe_context *rctx);
>> void r600_init_context_resource_functions(struct r600_pipe_context *r600);
>>
>> /* r600_shader.c */
>> -int r600_pipe_shader_create(struct pipe_context *ctx, struct r600_pipe_shader *shader, const struct tgsi_token *tokens);
>> +int r600_pipe_shader_create(struct pipe_context *ctx, struct r600_pipe_shader *shader);
>> void r600_pipe_shader_destroy(struct pipe_context *ctx, struct r600_pipe_shader *shader);
>> int r600_find_vs_semantic_index(struct r600_shader *vs,
>> struct r600_shader *ps, int id);
>> diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c
>> index 904cc69..2e5d4a6 100644
>> --- a/src/gallium/drivers/r600/r600_shader.c
>> +++ b/src/gallium/drivers/r600/r600_shader.c
>> @@ -118,9 +118,9 @@ static int r600_pipe_shader(struct pipe_context *ctx, struct r600_pipe_shader *s
>> return 0;
>> }
>>
>> -static int r600_shader_from_tgsi(const struct tgsi_token *tokens, struct r600_shader *shader);
>> +static int r600_shader_from_tgsi(struct r600_pipe_context * rctx, struct r600_pipe_shader *pipeshader);
>>
>> -int r600_pipe_shader_create(struct pipe_context *ctx, struct r600_pipe_shader *shader, const struct tgsi_token *tokens)
>> +int r600_pipe_shader_create(struct pipe_context *ctx, struct r600_pipe_shader *shader)
>> {
>> static int dump_shaders = -1;
>> struct r600_pipe_context *rctx = (struct r600_pipe_context *)ctx;
>> @@ -133,10 +133,10 @@ int r600_pipe_shader_create(struct pipe_context *ctx, struct r600_pipe_shader *s
>>
>> if (dump_shaders) {
>> fprintf(stderr, "--------------------------------------------------------------\n");
>> - tgsi_dump(tokens, 0);
>> + tgsi_dump(shader->tokens, 0);
>> }
>> shader->shader.family = r600_get_family(rctx->radeon);
>> - r = r600_shader_from_tgsi(tokens,&shader->shader);
>> + r = r600_shader_from_tgsi(rctx, shader);
>> if (r) {
>> R600_ERR("translation from TGSI failed !\n");
>> return r;
>> @@ -159,6 +159,8 @@ void r600_pipe_shader_destroy(struct pipe_context *ctx, struct r600_pipe_shader
>>
>> r600_bo_reference(rctx->radeon,&shader->bo, NULL);
>> r600_bc_clear(&shader->shader.bc);
>> +
>> + memset(&shader->shader,0,sizeof(struct r600_shader));
>> }
>>
>> /*
>> @@ -594,8 +596,10 @@ static int tgsi_split_literal_constant(struct r600_shader_ctx *ctx)
>> return 0;
>> }
>>
>> -static int r600_shader_from_tgsi(const struct tgsi_token *tokens, struct r600_shader *shader)
>> +static int r600_shader_from_tgsi(struct r600_pipe_context * rctx, struct r600_pipe_shader *pipeshader)
>> {
>> + struct r600_shader *shader =&pipeshader->shader;
>> + struct tgsi_token *tokens = pipeshader->tokens;
>> struct tgsi_full_immediate *immediate;
>> struct tgsi_full_property *property;
>> struct r600_shader_ctx ctx;
>> @@ -616,6 +620,9 @@ static int r600_shader_from_tgsi(const struct tgsi_token *tokens, struct r600_sh
>> shader->processor_type = ctx.type;
>> ctx.bc->type = shader->processor_type;
>>
>> + shader->clamp_color = (((ctx.type == TGSI_PROCESSOR_FRAGMENT)&& rctx->clamp_fragment_color) ||
>> + ((ctx.type == TGSI_PROCESSOR_VERTEX)&& rctx->clamp_vertex_color));
>> +
>> /* register allocations */
>> /* Values [0,127] correspond to GPR[0..127].
>> * Values [128,159] correspond to constant buffer bank 0
>> @@ -725,8 +732,41 @@ static int r600_shader_from_tgsi(const struct tgsi_token *tokens, struct r600_sh
>> goto out_err;
>> }
>> }
>> - /* export output */
>> +
>> noutput = shader->noutput;
>> +
>> + /* clamp color outputs */
>> + if (shader->clamp_color) {
>> + for (i = 0; i< noutput; i++) {
>> + if (shader->output[i].name == TGSI_SEMANTIC_COLOR ||
>> + shader->output[i].name == TGSI_SEMANTIC_BCOLOR) {
>> +
>> + int j;
>> + for (j = 0; j< 4; j++) {
>> + struct r600_bc_alu alu;
>> + memset(&alu, 0, sizeof(struct r600_bc_alu));
>> +
>> + /* MOV_SAT R, R */
>> + alu.inst = BC_INST(ctx.bc, V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOV);
>> + alu.dst.sel = shader->output[i].gpr;
>> + alu.dst.chan = j;
>> + alu.dst.write = 1;
>> + alu.dst.clamp = 1;
>> + alu.src[0].sel = alu.dst.sel;
>> + alu.src[0].chan = j;
>> +
>> + if (j == 3) {
>> + alu.last = 1;
>> + }
>> + r = r600_bc_add_alu(ctx.bc,&alu);
>> + if (r)
>> + return r;
>> + }
>> + }
>> + }
>> + }
>> +
>> + /* export output */
>> for (i = 0, pos0 = 0; i< noutput; i++) {
>> memset(&output[i], 0, sizeof(struct r600_bc_output));
>> output[i].gpr = shader->output[i].gpr;
>> diff --git a/src/gallium/drivers/r600/r600_shader.h b/src/gallium/drivers/r600/r600_shader.h
>> index 8f96ce5..c1e00f3 100644
>> --- a/src/gallium/drivers/r600/r600_shader.h
>> +++ b/src/gallium/drivers/r600/r600_shader.h
>> @@ -46,6 +46,7 @@ struct r600_shader {
>> enum radeon_family family;
>> boolean uses_kill;
>> boolean fs_write_all;
>> + boolean clamp_color;
>> };
>>
>> #endif
>> diff --git a/src/gallium/drivers/r600/r600_state.c b/src/gallium/drivers/r600/r600_state.c
>> index 91da7c5..be07f5f 100644
>> --- a/src/gallium/drivers/r600/r600_state.c
>> +++ b/src/gallium/drivers/r600/r600_state.c
>> @@ -299,6 +299,8 @@ static void *r600_create_rs_state(struct pipe_context *ctx,
>> }
>>
>> rstate =&rs->rstate;
>> + rs->clamp_vertex_color = state->clamp_vertex_color;
>> + rs->clamp_fragment_color = state->clamp_fragment_color;
>> rs->flatshade = state->flatshade;
>> rs->sprite_coord_enable = state->sprite_coord_enable;
>>
>> diff --git a/src/gallium/drivers/r600/r600_state_common.c b/src/gallium/drivers/r600/r600_state_common.c
>> index 259f426..f1b0fc3 100644
>> --- a/src/gallium/drivers/r600/r600_state_common.c
>> +++ b/src/gallium/drivers/r600/r600_state_common.c
>> @@ -28,6 +28,7 @@
>> #include<util/u_format.h>
>> #include<pipebuffer/pb_buffer.h>
>> #include "pipe/p_shader_tokens.h"
>> +#include "tgsi/tgsi_parse.h"
>> #include "r600_formats.h"
>> #include "r600_pipe.h"
>> #include "r600d.h"
>> @@ -99,6 +100,8 @@ void r600_bind_rs_state(struct pipe_context *ctx, void *state)
>> if (state == NULL)
>> return;
>>
>> + rctx->clamp_vertex_color = rs->clamp_vertex_color;
>> + rctx->clamp_fragment_color = rs->clamp_fragment_color;
>> rctx->flatshade = rs->flatshade;
>> rctx->sprite_coord_enable = rs->sprite_coord_enable;
>> rctx->rasterizer = rs;
>> @@ -257,7 +260,9 @@ void *r600_create_shader_state(struct pipe_context *ctx,
>> struct r600_pipe_shader *shader = CALLOC_STRUCT(r600_pipe_shader);
>> int r;
>>
>> - r = r600_pipe_shader_create(ctx, shader, state->tokens);
>> + shader->tokens = tgsi_dup_tokens(state->tokens);
>> +
>> + r = r600_pipe_shader_create(ctx, shader);
>> if (r) {
>> return NULL;
>> }
>> @@ -303,6 +308,7 @@ void r600_delete_ps_shader(struct pipe_context *ctx, void *state)
>> rctx->ps_shader = NULL;
>> }
>>
>> + free(shader->tokens);
>> r600_pipe_shader_destroy(ctx, shader);
>> free(shader);
>> }
>> @@ -316,6 +322,7 @@ void r600_delete_vs_shader(struct pipe_context *ctx, void *state)
>> rctx->vs_shader = NULL;
>> }
>>
>> + free(shader->tokens);
>> r600_pipe_shader_destroy(ctx, shader);
>> free(shader);
>> }
>> @@ -531,6 +538,21 @@ static void r600_vertex_buffer_update(struct r600_pipe_context *rctx)
>> }
>> }
>>
>> +static int r600_shader_rebuild(struct pipe_context * ctx, struct r600_pipe_shader * shader)
>> +{
>> + struct r600_pipe_context *rctx = (struct r600_pipe_context *)ctx;
>> + int r;
>> +
>> + r600_pipe_shader_destroy(ctx, shader);
>> + r = r600_pipe_shader_create(ctx, shader);
>> + if (r) {
>> + return r;
>> + }
>> + r600_context_pipe_state_set(&rctx->ctx,&shader->rstate);
>> +
>> + return 0;
>> +}
>> +
>> void r600_draw_vbo(struct pipe_context *ctx, const struct pipe_draw_info *info)
>> {
>> struct r600_pipe_context *rctx = (struct r600_pipe_context *)ctx;
>> @@ -574,6 +596,12 @@ void r600_draw_vbo(struct pipe_context *ctx, const struct pipe_draw_info *info)
>> if (r600_conv_pipe_prim(draw.info.mode,&prim))
>> return;
>>
>> + if (rctx->vs_shader->shader.clamp_color != rctx->clamp_vertex_color)
>> + r600_shader_rebuild(ctx, rctx->vs_shader);
>> +
>> + if (rctx->ps_shader->shader.clamp_color != rctx->clamp_fragment_color)
>> + r600_shader_rebuild(ctx, rctx->ps_shader);
>> +
>> if (rctx->spi_dirty)
>> r600_spi_update(rctx);
>>
>> --
>> 1.7.5.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
More information about the mesa-dev
mailing list