On 09/16/2013 09:38 AM, Eric Anholt wrote:
> Kenneth Graunke <kenn...@whitecape.org> writes:
>
>> This was an embarassingly large amount of copy and pasted code,
>> and it wasn't particularly simple code either. By factoring it out
>> into a helper function, we consolidate the complexity.
>>
>> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
>> ---
>> src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 144
>> +++++++++-------------
>> 1 file changed, 58 insertions(+), 86 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
>> b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
>> index 37e3174..8413308 100644
>> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
>> @@ -224,6 +224,37 @@ gen7_check_surface_setup(uint32_t *surf, bool
>> is_render_target)
>> }
>> }
>>
>> +static void
>> +gen7_emit_buffer_surface_state(struct brw_context *brw,
>> + uint32_t *out_offset,
>> + drm_intel_bo *bo,
>> + unsigned buffer_offset,
>> + unsigned surface_format,
>> + unsigned buffer_size,
>> + unsigned pitch,
>> + unsigned mocs)
>> +{
>> + uint32_t *surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
>> + 8 * 4, 32, out_offset);
>> + memset(surf, 0, 8 * 4);
>> +
>> + surf[0] = BRW_SURFACE_BUFFER << BRW_SURFACE_TYPE_SHIFT |
>> + surface_format << BRW_SURFACE_FORMAT_SHIFT |
>> + BRW_SURFACE_RC_READ_WRITE;
>> + surf[1] = bo->offset + buffer_offset; /* reloc */
>> + surf[2] = SET_FIELD(buffer_size & 0x7f, GEN7_SURFACE_WIDTH) |
>> + SET_FIELD((buffer_size >> 7) & 0x3fff, GEN7_SURFACE_HEIGHT);
>> + surf[3] = SET_FIELD((buffer_size >> 21) & 0x3f, BRW_SURFACE_DEPTH) |
>> + (pitch - 1);
>> + surf[4] = 0;
>> + surf[5] = SET_FIELD(mocs, GEN7_SURFACE_MOCS);
>> +
>> + /* Emit relocation to surface contents */
>> + drm_intel_bo_emit_reloc(brw->batch.bo, *out_offset + 4,
>> + bo, buffer_offset, I915_GEM_DOMAIN_SAMPLER, 0);
>
> I think this will segfault if BO is NULL
Clearly. I don't know how I missed that...thanks for catching it.
[snip]
> But here you might pass in a NULL bo. Same for patch 4. I'm confused
> that this didn't produce a piglit regression.
Fixed in both patches. I'm not sure why it didn't cause regressions.
>> }
>
> I think the savings is well worth it in patches 1, 3, and 4, but I'm not
> a fan of patch 2. I've got a lot of branches outstanding where I'm
> trying to finally finish off mt->first_level and the tile offset usage,
> and sharing code between renderbuffers and textures is just going to
> make that harder to do. (I'm already really tempted to split gen4/5
> From gen6 RB setup, since I think I could land gen4/5 without
> regressing, while gen6 is the unstable hw)
>
Hmm. I'd really like to try and share some code, but I don't want to
get in the way of the work that you're doing to kill off tile offsets.
I think I'll drop patch 2 for now, and maybe try again once the code
isn't in as much flux.
--Ken
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
