<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, May 2, 2017 at 8:55 AM, Rafael Antognolli <<a href="mailto:rafael.antognolli@intel.com" target="_blank">rafael.antognolli@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, May 02, 2017 at 08:44:05AM -0700, Jason Ekstrand wrote: > On Tue, May 2, 2017 at 8:28 AM, Rafael Antognolli <<a href="mailto:rafael.antognolli@intel.com">rafael.antognolli@intel.com</a>> > wrote: > > On Tue, May 02, 2017 at 07:26:53AM -0700, Jason Ekstrand wrote: > > On Mon, May 1, 2017 at 6:43 PM, Rafael Antognolli < > <a href="mailto:rafael.antognolli@intel.com">rafael.antognolli@intel.com</a>> > > wrote: > > > > We need to use some enums inside genX_state_upload.c, but including > the > > whole header will cause several conflicts between things defined in > this > > header and the genxml auto-generated headers. > > > > So create a separate header that is included both by brw_eu_defines.h > > and genX_state_upload.c. > > > > Signed-off-by: Rafael Antognolli <<a href="mailto:rafael.antognolli@intel.com">rafael.antognolli@intel.com</a>> > > Acked-by: Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> > > --- > > src/intel/Makefile.sources | 1 +- > > src/intel/compiler/brw_defines_common.h | 46 > ++++++++++++++++++++++++++- > > src/intel/compiler/brw_eu_defines.h | 22 +------------ > > 3 files changed, 48 insertions(+), 21 deletions(-) > > create mode 100644 src/intel/compiler/brw_defines_common.h > > > > diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources > > index e9a39a6..652f376 100644 > > --- a/src/intel/Makefile.sources > > +++ b/src/intel/Makefile.sources > > @@ -27,6 +27,7 @@ COMPILER_FILES = \ > > compiler/brw_compiler.h \ > > compiler/brw_dead_control_flow.cpp \ > > compiler/brw_dead_control_flow.h \ > > + compiler/brw_defines_common.h \ > > compiler/brw_disasm.c \ > > compiler/brw_eu.c \ > > compiler/brw_eu_compact.c \ > > diff --git a/src/intel/compiler/brw_defines_common.h b/src/intel/ > compiler/ > > brw_defines_common.h > > new file mode 100644 > > index 0000000..fdae125 > > --- /dev/null > > +++ b/src/intel/compiler/brw_defines_common.h > > @@ -0,0 +1,46 @@ > > +/* > > + * Copyright © 2017 Intel Corporation > > + * > > + * Permission is hereby granted, free of charge, to any person > obtaining a > > + * copy of this software and associated documentation files (the > > "Software"), > > + * to deal in the Software without restriction, including without > > limitation > > + * the rights to use, copy, modify, merge, publish, distribute, > > sublicense, > > + * and/or sell copies of the Software, and to permit persons to whom > the > > + * Software is furnished to do so, subject to the following > conditions: > > + * > > + * The above copyright notice and this permission notice (including > the > > next > > + * paragraph) shall be included in all copies or substantial > portions of > > the > > + * Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > EXPRESS > > OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO > EVENT > > SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES > OR > > OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > ARISING > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > OTHER > > DEALINGS > > + * IN THE SOFTWARE. > > + */ > > + > > +#ifndef BRW_DEFINES_COMMON_H > > +#endif // BRW_DEFINES_COMMON_H > > + > > +enum brw_pixel_shader_computed_depth_mode { > > + BRW_PSCDEPTH_OFF = 0, /* PS does not compute depth */ > > + BRW_PSCDEPTH_ON = 1, /* PS computes depth; no guarantee about > value > > */ > > + BRW_PSCDEPTH_ON_GE = 2, /* PS guarantees output depth >= source > depth * > > / > > + BRW_PSCDEPTH_ON_LE = 3, /* PS guarantees output depth <= source > depth * > > / > > > > > > These are provided by the pack header at least on gen8. If they're not > > available elsewhere, we should just add them. > > Hmm... good point. They are values to "Pixel Shader Computed Depth Mode" > in 3DSTATE_PS_EXTRA, which is only available in gen7+, so where would be > a good place for gen6? > > > 3DSTATE_WM::PixelShaderComputedDepthMode is a boolean so it doesn't have actual > enum values. That said, do we need the enum values for gen6? </div></div>We need writes_depth on gen6 too. So, I can just compare it to 0: bool writes_depth = wm_prog_data->computed_depth_mode != 0; Is that fine? <div class="HOEnZb"><div class="h5"></div></div></blockquote><div> </div><div>I think so. It's a little awkward to compare to a constant rather than an enum value but given that == 0 means OFF and != 0 means ON with potentially some extra information, I think that's fine. Then again... </div><div>One other thing that you could do given that these things live in wm_prog_data would be to move the enums into brw_compiler.h. That's probably a good thing to do anyway. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> > > +}; > > + > > +enum brw_barycentric_mode { > > + BRW_BARYCENTRIC_PERSPECTIVE_PIXEL = 0, > > + BRW_BARYCENTRIC_PERSPECTIVE_CENTROID = 1, > > + BRW_BARYCENTRIC_PERSPECTIVE_SAMPLE = 2, > > + BRW_BARYCENTRIC_NONPERSPECTIVE_PIXEL = 3, > > + BRW_BARYCENTRIC_NONPERSPECTIVE_CENTROID = 4, > > + BRW_BARYCENTRIC_NONPERSPECTIVE_SAMPLE = 5, > > > > > > These are also in the pack header. > > OK. > > > > > + BRW_BARYCENTRIC_MODE_COUNT = 6 > > +}; > > +#define BRW_BARYCENTRIC_NONPERSPECTIVE_BITS \ > > + ((1 << BRW_BARYCENTRIC_NONPERSPECTIVE_PIXEL) | \ > > + (1 << BRW_BARYCENTRIC_NONPERSPECTIVE_CENTROID) | \ > > + (1 << BRW_BARYCENTRIC_NONPERSPECTIVE_SAMPLE)) > > > > > > I think I can just define in genX_state_upload.c something like: > > #define BARYCENTRYC_NONPERSPECTIVE_BITS \ > BIM_PERSPECTIVE_PIXEL | \ > BIM_PERSPECTIVE_CENTROID | \ > BIM_PERSPECTIVE_SAMPLE > > And then I don't have to include that file. > > > > > diff --git a/src/intel/compiler/brw_eu_defines.h b/src/intel/compiler > / > > brw_eu_defines.h > > index 13a70f6..51f9cbc 100644 > > --- a/src/intel/compiler/brw_eu_defines.h > > +++ b/src/intel/compiler/brw_eu_defines.h > > @@ -33,6 +33,7 @@ > > #define BRW_EU_DEFINES_H > > > > #include "util/macros.h" > > +#include "brw_defines_common.h" > > > > /* The following hunk, up-to "Execution Unit" is used by both the > > * intel/compiler and i965 codebase. */ > > @@ -72,27 +73,6 @@ > > #define _3DPRIM_TRIFAN_NOSTIPPLE 0x16 > > #define _3DPRIM_PATCHLIST(n) ({ assert(n > 0 && n <= 32); 0x20 + (n > - 1); > > }) > > > > -enum brw_barycentric_mode { > > - BRW_BARYCENTRIC_PERSPECTIVE_PIXEL = 0, > > - BRW_BARYCENTRIC_PERSPECTIVE_CENTROID = 1, > > - BRW_BARYCENTRIC_PERSPECTIVE_SAMPLE = 2, > > - BRW_BARYCENTRIC_NONPERSPECTIVE_PIXEL = 3, > > - BRW_BARYCENTRIC_NONPERSPECTIVE_CENTROID = 4, > > - BRW_BARYCENTRIC_NONPERSPECTIVE_SAMPLE = 5, > > - BRW_BARYCENTRIC_MODE_COUNT = 6 > > -}; > > -#define BRW_BARYCENTRIC_NONPERSPECTIVE_BITS \ > > - ((1 << BRW_BARYCENTRIC_NONPERSPECTIVE_PIXEL) | \ > > - (1 << BRW_BARYCENTRIC_NONPERSPECTIVE_CENTROID) | \ > > - (1 << BRW_BARYCENTRIC_NONPERSPECTIVE_SAMPLE)) > > - > > -enum brw_pixel_shader_computed_depth_mode { > > - BRW_PSCDEPTH_OFF = 0, /* PS does not compute depth */ > > - BRW_PSCDEPTH_ON = 1, /* PS computes depth; no guarantee about > value > > */ > > - BRW_PSCDEPTH_ON_GE = 2, /* PS guarantees output depth >= source > depth * > > / > > - BRW_PSCDEPTH_ON_LE = 3, /* PS guarantees output depth <= source > depth * > > / > > -}; > > - > > /* Bitfields for the URB_WRITE message, DW2 of message header: */ > > #define URB_WRITE_PRIM_END 0x1 > > #define URB_WRITE_PRIM_START 0x2 > > -- > > git-series 0.9.1 > > _______________________________________________ > > mesa-dev mailing list > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > > > > > > </div></div></blockquote></div> </div></div>