[Mesa-dev] [PATCH V4 1/5] i965 Gen4/5: Compute interpolation status for every varying in one place.
Chris Forbes
chrisf at ijw.co.nz
Tue Jul 30 15:34:27 PDT 2013
I like that. It also provides a sensible type to pass to the new `is
there any flat shading?` helper.
On Wed, Jul 31, 2013 at 9:52 AM, Paul Berry <stereotype441 at gmail.com> wrote:
> 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
>
>
More information about the mesa-dev
mailing list