Re: [Mesa-dev] [PATCH v03 12/38] i965: Split out enum from brw_eu_defines.h

2017-05-02 Thread Jason Ekstrand 
On Tue, May 2, 2017 at 8:55 AM, Rafael Antognolli <
rafael.antogno...@intel.com> wrote:

> On Tue, May 02, 2017 at 08:44:05AM -0700, Jason Ekstrand wrote:
> > On Tue, May 2, 2017 at 8:28 AM, Rafael Antognolli <
> rafael.antogno...@intel.com>
> > 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 <
> > rafael.antogno...@intel.com>
> > > 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 
> > > Acked-by: Reviewed-by: Kenneth Graunke 
> > > ---
> > >  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 000..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

Re: [Mesa-dev] [PATCH v03 12/38] i965: Split out enum from brw_eu_defines.h

2017-05-02 Thread Rafael Antognolli 
On Tue, May 02, 2017 at 08:44:05AM -0700, Jason Ekstrand wrote:
> On Tue, May 2, 2017 at 8:28 AM, Rafael Antognolli 
> 
> 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 <
> rafael.antogno...@intel.com>
> > 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 
> > Acked-by: Reviewed-by: Kenneth Graunke 
> > ---
> >  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 000..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?

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

Re: [Mesa-dev] [PATCH v03 12/38] i965: Split out enum from brw_eu_defines.h

2017-05-02 Thread Jason Ekstrand 
On Tue, May 2, 2017 at 8:28 AM, Rafael Antognolli <
rafael.antogno...@intel.com> 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 <
> rafael.antogno...@intel.com>
> > 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 
> > Acked-by: Reviewed-by: Kenneth Graunke 
> > ---
> >  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 000..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?


> > +};
> > +
> > +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) | \
> > +

Re: [Mesa-dev] [PATCH v03 12/38] i965: Split out enum from brw_eu_defines.h

2017-05-02 Thread Rafael Antognolli 
On Tue, May 02, 2017 at 07:26:53AM -0700, Jason Ekstrand wrote:
> On Mon, May 1, 2017 at 6:43 PM, Rafael Antognolli 
> 
> 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 
> Acked-by: Reviewed-by: Kenneth Graunke 
> ---
>  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 000..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?

> +};
> +
> +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
>

Re: [Mesa-dev] [PATCH v03 12/38] i965: Split out enum from brw_eu_defines.h

2017-05-02 Thread Jason Ekstrand 
On Mon, May 1, 2017 at 6:43 PM, Rafael Antognolli <
rafael.antogno...@intel.com> 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 
> Acked-by: Reviewed-by: Kenneth Graunke 
> ---
>  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 000..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.


> +};
> +
> +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.


> +   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))
>

These are not


> 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
> -};
>

Re: [Mesa-dev] [PATCH v03 12/38] i965: Split out enum from brw_eu_defines.h

2017-05-02 Thread Rafael Antognolli 
On Tue, May 02, 2017 at 09:38:53AM +0100, Emil Velikov wrote:
> On 2 May 2017 at 09:32, Emil Velikov  wrote:
> > Hi Rafael,
> >
> > On 2 May 2017 at 02:43, Rafael Antognolli  
> > 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 
> >> Acked-by: Reviewed-by: Kenneth Graunke 
> >> ---
> >>  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 \
> > Amazing, thank you!
> >
> >> 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 000..fdae125
> >> --- /dev/null
> >> +++ b/src/intel/compiler/brw_defines_common.h
> >
> >> +
> >> +#ifndef BRW_DEFINES_COMMON_H
> > You want a "#define BRW_DEFINES_COMMON_H" here
> >
> >> +#endif // BRW_DEFINES_COMMON_H
> >> +
> > And this line should be at the end of the file.
> >
> Forgot to mention: the above is trivial, so I don't bother resending
> the series just for that.

Ugh, my bad. Thank you.

Rafael
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v03 12/38] i965: Split out enum from brw_eu_defines.h

2017-05-02 Thread Emil Velikov 
On 2 May 2017 at 09:32, Emil Velikov  wrote:
> Hi Rafael,
>
> On 2 May 2017 at 02:43, Rafael Antognolli  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 
>> Acked-by: Reviewed-by: Kenneth Graunke 
>> ---
>>  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 \
> Amazing, thank you!
>
>> 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 000..fdae125
>> --- /dev/null
>> +++ b/src/intel/compiler/brw_defines_common.h
>
>> +
>> +#ifndef BRW_DEFINES_COMMON_H
> You want a "#define BRW_DEFINES_COMMON_H" here
>
>> +#endif // BRW_DEFINES_COMMON_H
>> +
> And this line should be at the end of the file.
>
Forgot to mention: the above is trivial, so I don't bother resending
the series just for that.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v03 12/38] i965: Split out enum from brw_eu_defines.h

2017-05-02 Thread Emil Velikov 
Hi Rafael,

On 2 May 2017 at 02:43, Rafael Antognolli  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 
> Acked-by: Reviewed-by: Kenneth Graunke 
> ---
>  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 \
Amazing, thank you!

> 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 000..fdae125
> --- /dev/null
> +++ b/src/intel/compiler/brw_defines_common.h

> +
> +#ifndef BRW_DEFINES_COMMON_H
You want a "#define BRW_DEFINES_COMMON_H" here

> +#endif // BRW_DEFINES_COMMON_H
> +
And this line should be at the end of the file.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v03 12/38] i965: Split out enum from brw_eu_defines.h

2017-05-01 Thread Rafael Antognolli 
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 
Acked-by: Reviewed-by: Kenneth Graunke 
---
 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 000..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 */
+};
+
+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))
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