I thought your patch does something else, but after re-reading it, it seems to do the right thing.
Reviewed-by: Marek Olšák <marek.ol...@amd.com> Marek On Mon, Jul 6, 2015 at 1:38 PM, Marek Olšák <mar...@gmail.com> wrote: > Hi Michel, > > VS_EXPORT_COUNT shouldn't include POS exports (POSITION, MISC (psize, > edgeflag, layer, viewport), and CLIPDIST/CULLDIST), it should only > count PARAM exports. Setting higher VS_EXPORT_COUNT doesn't affect > correctness but it allocates more parameter memory. CLIPDIST is > special in that it's exported twice, first as POS and then as PARAM, > the same will be done for CULLDIST in the future, so these two should > indeed be removed from the list. > > Marek > > > On Mon, Jul 6, 2015 at 10:30 AM, Michel Dänzer <mic...@daenzer.net> wrote: >> From: Michel Dänzer <michel.daen...@amd.com> >> >> This eliminates the error prone logic in si_shader_vs recalculating this >> value. >> >> It also fixes TGSI_SEMANTIC_CLIPDIST outputs incorrectly not being >> counted for VS exports, since they are passed to the fragment shader as >> varyings as well. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91193 >> Signed-off-by: Michel Dänzer <michel.daen...@amd.com> >> --- >> src/gallium/drivers/radeonsi/si_shader.c | 2 ++ >> src/gallium/drivers/radeonsi/si_shader.h | 1 + >> src/gallium/drivers/radeonsi/si_state_shaders.c | 25 >> +++---------------------- >> 3 files changed, 6 insertions(+), 22 deletions(-) >> >> diff --git a/src/gallium/drivers/radeonsi/si_shader.c >> b/src/gallium/drivers/radeonsi/si_shader.c >> index 4d97b58..753b238 100644 >> --- a/src/gallium/drivers/radeonsi/si_shader.c >> +++ b/src/gallium/drivers/radeonsi/si_shader.c >> @@ -1218,6 +1218,8 @@ handle_semantic: >> } >> } >> >> + shader->nr_param_exports = param_count; >> + >> /* We need to add the position output manually if it's missing. */ >> if (!pos_args[0][0]) { >> pos_args[0][0] = lp_build_const_int32(base->gallivm, 0xf); >> /* writemask */ >> diff --git a/src/gallium/drivers/radeonsi/si_shader.h >> b/src/gallium/drivers/radeonsi/si_shader.h >> index b4339ae..8d309b4 100644 >> --- a/src/gallium/drivers/radeonsi/si_shader.h >> +++ b/src/gallium/drivers/radeonsi/si_shader.h >> @@ -165,6 +165,7 @@ struct si_shader { >> >> bool uses_instanceid; >> unsigned nr_pos_exports; >> + unsigned nr_param_exports; >> bool is_gs_copy_shader; >> bool dx10_clamp_mode; /* convert NaNs to 0 */ >> }; >> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c >> b/src/gallium/drivers/radeonsi/si_state_shaders.c >> index eef3baa..a842d9d 100644 >> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c >> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c >> @@ -148,10 +148,9 @@ static void si_shader_gs(struct si_shader *shader) >> >> static void si_shader_vs(struct si_shader *shader) >> { >> - struct tgsi_shader_info *info = &shader->selector->info; >> struct si_pm4_state *pm4; >> unsigned num_sgprs, num_user_sgprs; >> - unsigned nparams, i, vgpr_comp_cnt; >> + unsigned nparams, vgpr_comp_cnt; >> uint64_t va; >> unsigned window_space = >> >> shader->selector->info.properties[TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION]; >> @@ -180,26 +179,8 @@ static void si_shader_vs(struct si_shader *shader) >> } >> assert(num_sgprs <= 104); >> >> - /* Certain attributes (position, psize, etc.) don't count as params. >> - * VS is required to export at least one param and >> r600_shader_from_tgsi() >> - * takes care of adding a dummy export. >> - */ >> - for (nparams = 0, i = 0 ; i < info->num_outputs; i++) { >> - switch (info->output_semantic_name[i]) { >> - case TGSI_SEMANTIC_CLIPVERTEX: >> - case TGSI_SEMANTIC_CLIPDIST: >> - case TGSI_SEMANTIC_CULLDIST: >> - case TGSI_SEMANTIC_POSITION: >> - case TGSI_SEMANTIC_PSIZE: >> - case TGSI_SEMANTIC_EDGEFLAG: >> - break; >> - default: >> - nparams++; >> - } >> - } >> - if (nparams < 1) >> - nparams = 1; >> - >> + /* VS is required to export at least one param. */ >> + nparams = MAX2(shader->nr_param_exports, 1); >> si_pm4_set_reg(pm4, R_0286C4_SPI_VS_OUT_CONFIG, >> S_0286C4_VS_EXPORT_COUNT(nparams - 1)); >> >> -- >> 2.1.4 >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev