[Mesa-dev] [PATCH V4 1/5] i965 Gen4/5: Compute interpolation status for every varying in one place.
Paul Berry
stereotype441 at gmail.com
Tue Jul 30 14:52:14 PDT 2013
On 23 July 2013 01:16, Chris Forbes <chrisf at ijw.co.nz> wrote:
> The program keys are updated accordingly, but the values are not used
> yet.
>
> [V1-2]: Signed-off-by: Olivier Galibert <galibert at pobox.com>
>
> V3: Updated for vue_map changes, intel -> brw merge, etc. (Chris Forbes)
> V4: Compute interpolation map as a new state atom rather than tacking it
> on the front of the clip setup
>
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> ---
> src/mesa/drivers/dri/i965/Makefile.sources | 1 +
> src/mesa/drivers/dri/i965/brw_clip.c | 8 ++-
> src/mesa/drivers/dri/i965/brw_clip.h | 1 +
> src/mesa/drivers/dri/i965/brw_context.h | 7 ++
> src/mesa/drivers/dri/i965/brw_interpolation_map.c | 85
> +++++++++++++++++++++++
> src/mesa/drivers/dri/i965/brw_sf.c | 7 +-
> src/mesa/drivers/dri/i965/brw_sf.h | 1 +
> src/mesa/drivers/dri/i965/brw_state.h | 1 +
> src/mesa/drivers/dri/i965/brw_state_upload.c | 3 +
> 9 files changed, 112 insertions(+), 2 deletions(-)
> create mode 100644 src/mesa/drivers/dri/i965/brw_interpolation_map.c
>
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources
> b/src/mesa/drivers/dri/i965/Makefile.sources
> index 1f401fe..ac8487b 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -62,6 +62,7 @@ i965_FILES = \
> brw_gs.c \
> brw_gs_emit.c \
> brw_gs_state.c \
> + brw_interpolation_map.c \
> brw_lower_texture_gradients.cpp \
> brw_misc_state.c \
> brw_program.c \
> diff --git a/src/mesa/drivers/dri/i965/brw_clip.c
> b/src/mesa/drivers/dri/i965/brw_clip.c
> index 2ebf3f6..60d85e8 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip.c
> @@ -141,6 +141,10 @@ brw_upload_clip_prog(struct brw_context *brw)
>
> /* Populate the key:
> */
> +
> + /* BRW_NEW_INTERPOLATION_MAP */
> + memcpy(key.interpolation_mode, brw->interpolation_mode,
> BRW_VARYING_SLOT_COUNT);
>
Random idea, which you may feel free to disregard: What if we make
interpolation_mode a struct, e.g.:
struct interpolation_mode_map
{
unsigned char mode[BRW_VARYING_SLOT_COUNT];
}
That way, this memcpy can just change to:
key.interpolation_mode = brw->interpolation_mode;
And the compiler takes care of making sure the types match and figuring out
the right number of bytes to copy.
The disadvantage, of course, is that everywhere you use
key.interpolation_mode[...] in the rest of the series you'll have to
replace that with key.interpolation_mode.mode[...], which looks pretty
silly :)
Either way, this patch is:
Reviewed-by: Paul Berry <stereotype441 at gmail.com>
> +
> /* BRW_NEW_REDUCED_PRIMITIVE */
> key.primitive = brw->reduced_primitive;
> /* BRW_NEW_VUE_MAP_GEOM_OUT */
> @@ -256,7 +260,9 @@ const struct brw_tracked_state brw_clip_prog = {
> _NEW_TRANSFORM |
> _NEW_POLYGON |
> _NEW_BUFFERS),
> - .brw = (BRW_NEW_REDUCED_PRIMITIVE | BRW_NEW_VUE_MAP_GEOM_OUT)
> + .brw = (BRW_NEW_REDUCED_PRIMITIVE |
> + BRW_NEW_VUE_MAP_GEOM_OUT |
> + BRW_NEW_INTERPOLATION_MAP)
> },
> .emit = brw_upload_clip_prog
> };
> diff --git a/src/mesa/drivers/dri/i965/brw_clip.h
> b/src/mesa/drivers/dri/i965/brw_clip.h
> index 02259d4..fcbe2a0 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip.h
> +++ b/src/mesa/drivers/dri/i965/brw_clip.h
> @@ -43,6 +43,7 @@
> */
> struct brw_clip_prog_key {
> GLbitfield64 attrs;
> + unsigned char interpolation_mode[BRW_VARYING_SLOT_COUNT];
> GLuint primitive:4;
> GLuint nr_userclip:4;
> GLuint do_flat_shading:1;
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 86f9f71..29e522c 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -154,6 +154,7 @@ enum brw_state_id {
> BRW_STATE_STATS_WM,
> BRW_STATE_UNIFORM_BUFFER,
> BRW_STATE_META_IN_PROGRESS,
> + BRW_STATE_INTERPOLATION_MAP,
> };
>
> #define BRW_NEW_URB_FENCE (1 << BRW_STATE_URB_FENCE)
> @@ -186,6 +187,7 @@ enum brw_state_id {
> #define BRW_NEW_STATS_WM (1 << BRW_STATE_STATS_WM)
> #define BRW_NEW_UNIFORM_BUFFER (1 << BRW_STATE_UNIFORM_BUFFER)
> #define BRW_NEW_META_IN_PROGRESS (1 << BRW_STATE_META_IN_PROGRESS)
> +#define BRW_NEW_INTERPOLATION_MAP (1 << BRW_STATE_INTERPOLATION_MAP)
>
> struct brw_state_flags {
> /** State update flags signalled by mesa internals */
> @@ -1203,6 +1205,11 @@ struct brw_context
> uint32_t render_target_format[MESA_FORMAT_COUNT];
> bool format_supported_as_render_target[MESA_FORMAT_COUNT];
>
> + /* Interpolation modes, one byte per vue slot.
> + * Used Gen4/5 by the clip|sf|wm stages. Ignored on Gen6+.
> + */
> + unsigned char interpolation_mode[BRW_VARYING_SLOT_COUNT];
> +
> /* PrimitiveRestart */
> struct {
> bool in_progress;
> diff --git a/src/mesa/drivers/dri/i965/brw_interpolation_map.c
> b/src/mesa/drivers/dri/i965/brw_interpolation_map.c
> new file mode 100644
> index 0000000..a007078
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_interpolation_map.c
> @@ -0,0 +1,85 @@
> +/*
> + * Copyright © 2013 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.
> + */
> +
> +#include "brw_state.h"
> +
> +
> +/* Set up interpolation modes for every element in the VUE */
> +static void
> +brw_setup_vue_interpolation(struct brw_context *brw)
> +{
> + const struct gl_fragment_program *fprog = brw->fragment_program;
> + struct brw_vue_map *vue_map = &brw->vue_map_geom_out;
> +
> + memset(brw->interpolation_mode, INTERP_QUALIFIER_NONE,
> BRW_VARYING_SLOT_COUNT);
> +
> + brw->state.dirty.brw |= BRW_NEW_INTERPOLATION_MAP;
> +
> + if (!fprog)
> + return;
> +
> + for (int i=0; i < vue_map->num_slots; i++) {
> + int varying = vue_map->slot_to_varying[i];
> + if (varying == -1)
> + continue;
> +
> + /* HPOS always wants noperspective. setting it up here allows
> + * us to not need special handling in the SF program. */
> + if (varying == VARYING_SLOT_POS) {
> + brw->interpolation_mode[i] = INTERP_QUALIFIER_NOPERSPECTIVE;
> + continue;
> + }
> +
> + int frag_attrib = varying;
> + if (varying == VARYING_SLOT_BFC0 || varying == VARYING_SLOT_BFC1)
> + frag_attrib = varying - VARYING_SLOT_BFC0 + VARYING_SLOT_COL0;
> +
> + if (!(fprog->Base.InputsRead & BITFIELD64_BIT(frag_attrib)))
> + continue;
> +
> + enum glsl_interp_qualifier mode =
> fprog->InterpQualifier[frag_attrib];
> +
> + /* If the mode is not specified, the default varies: Color values
> + * follow GL_SHADE_MODEL; everything else is smooth.
> + */
> + if (mode == INTERP_QUALIFIER_NONE) {
> + if (frag_attrib == VARYING_SLOT_COL0 || frag_attrib ==
> VARYING_SLOT_COL1)
> + mode = brw->ctx.Light.ShadeModel == GL_FLAT
> + ? INTERP_QUALIFIER_FLAT : INTERP_QUALIFIER_SMOOTH;
> + else
> + mode = INTERP_QUALIFIER_SMOOTH;
> + }
> +
> + brw->interpolation_mode[i] = mode;
> + }
> +}
> +
> +
> +const struct brw_tracked_state brw_interpolation_map = {
> + .dirty = {
> + .mesa = _NEW_LIGHT,
> + .brw = (BRW_NEW_FRAGMENT_PROGRAM |
> + BRW_NEW_VUE_MAP_GEOM_OUT)
> + },
> + .emit = brw_setup_vue_interpolation
> +};
> diff --git a/src/mesa/drivers/dri/i965/brw_sf.c
> b/src/mesa/drivers/dri/i965/brw_sf.c
> index d73973c..933cff2 100644
> --- a/src/mesa/drivers/dri/i965/brw_sf.c
> +++ b/src/mesa/drivers/dri/i965/brw_sf.c
> @@ -189,6 +189,9 @@ brw_upload_sf_prog(struct brw_context *brw)
> if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) != render_to_fbo)
> key.sprite_origin_lower_left = true;
>
> + /* BRW_NEW_INTERPOLATION_MAP */
> + memcpy(key.interpolation_mode, brw->interpolation_mode,
> BRW_VARYING_SLOT_COUNT);
> +
> /* _NEW_LIGHT | _NEW_PROGRAM */
> key.do_flat_shading = (ctx->Light.ShadeModel == GL_FLAT);
> key.do_twoside_color = ((ctx->Light.Enabled &&
> ctx->Light.Model.TwoSide) ||
> @@ -215,7 +218,9 @@ const struct brw_tracked_state brw_sf_prog = {
> .dirty = {
> .mesa = (_NEW_HINT | _NEW_LIGHT | _NEW_POLYGON | _NEW_POINT |
> _NEW_TRANSFORM | _NEW_BUFFERS | _NEW_PROGRAM),
> - .brw = (BRW_NEW_REDUCED_PRIMITIVE | BRW_NEW_VUE_MAP_GEOM_OUT)
> + .brw = (BRW_NEW_REDUCED_PRIMITIVE |
> + BRW_NEW_VUE_MAP_GEOM_OUT |
> + BRW_NEW_INTERPOLATION_MAP)
> },
> .emit = brw_upload_sf_prog
> };
> diff --git a/src/mesa/drivers/dri/i965/brw_sf.h
> b/src/mesa/drivers/dri/i965/brw_sf.h
> index caeb0d0..55a860d 100644
> --- a/src/mesa/drivers/dri/i965/brw_sf.h
> +++ b/src/mesa/drivers/dri/i965/brw_sf.h
> @@ -46,6 +46,7 @@
>
> struct brw_sf_prog_key {
> GLbitfield64 attrs;
> + unsigned char interpolation_mode[BRW_VARYING_SLOT_COUNT];
> uint8_t point_sprite_coord_replace;
> GLuint primitive:2;
> GLuint do_twoside_color:1;
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h
> b/src/mesa/drivers/dri/i965/brw_state.h
> index ed1df87..321bffe 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -76,6 +76,7 @@ extern const struct brw_tracked_state
> brw_wm_binding_table;
> extern const struct brw_tracked_state brw_vs_binding_table;
> extern const struct brw_tracked_state brw_wm_ubo_surfaces;
> extern const struct brw_tracked_state brw_wm_unit;
> +extern const struct brw_tracked_state brw_interpolation_map;
>
> extern const struct brw_tracked_state brw_psp_urb_cbs;
>
> diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c
> b/src/mesa/drivers/dri/i965/brw_state_upload.c
> index c9ba781..e3ef245 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
> @@ -41,6 +41,9 @@ static const struct brw_tracked_state *gen4_atoms[] =
> {
> &brw_vs_prog, /* must do before GS prog, state base address. */
> &brw_gs_prog, /* must do before state base address */
> +
> + &brw_interpolation_map,
> +
> &brw_clip_prog, /* must do before state base address */
> &brw_sf_prog, /* must do before state base address */
> &brw_wm_prog, /* must do before state base address */
> --
> 1.8.3.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130730/f6507f17/attachment-0001.html>
More information about the mesa-dev
mailing list