"Pohjolainen, Topi" <[email protected]> writes: > On Thu, May 07, 2015 at 05:15:35PM +0300, Francisco Jerez wrote: >> From: Topi Pohjolainen <[email protected]> >> >> All generations do the same exact dispatch and it could be >> therefore done in the hardware independent stage. >> >> Reviewed-by: Matt Turner <[email protected]> >> Reviewed-by: Kenneth Graunke <[email protected]> >> Signed-off-by: Topi Pohjolainen <[email protected]> >> [ Francisco Jerez: Non-trivial rebase. ] >> Reviewed-by: Francisco Jerez <[email protected]> >> --- >> src/mesa/drivers/dri/i965/brw_context.h | 3 - >> src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 31 ++++++---- >> src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 69 >> +++++++++++------------ >> src/mesa/drivers/dri/i965/gen8_surface_state.c | 67 >> ++++++++++------------ >> 4 files changed, 83 insertions(+), 87 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_context.h >> b/src/mesa/drivers/dri/i965/brw_context.h >> index 2fcdcfa..a6282f4 100644 >> --- a/src/mesa/drivers/dri/i965/brw_context.h >> +++ b/src/mesa/drivers/dri/i965/brw_context.h >> @@ -1698,9 +1698,6 @@ void brw_create_constant_surface(struct brw_context >> *brw, >> uint32_t size, >> uint32_t *out_offset, >> bool dword_pitch); >> -void brw_update_buffer_texture_surface(struct gl_context *ctx, >> - unsigned unit, >> - uint32_t *surf_offset); >> void >> brw_update_sol_surface(struct brw_context *brw, >> struct gl_buffer_object *buffer_obj, >> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c >> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c >> index 160dd2f..2b8040c 100644 >> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c >> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c >> @@ -274,10 +274,10 @@ gen4_emit_buffer_surface_state(struct brw_context *brw, >> } >> } >> >> -void >> -brw_update_buffer_texture_surface(struct gl_context *ctx, >> - unsigned unit, >> - uint32_t *surf_offset) >> +static void >> +update_buffer_texture_surface(struct gl_context *ctx, >> + unsigned unit, >> + uint32_t *surf_offset) >> { >> struct brw_context *brw = brw_context(ctx); >> struct gl_texture_object *tObj = ctx->Texture.Unit[unit]._Current; >> @@ -320,12 +320,6 @@ brw_update_texture_surface(struct gl_context *ctx, >> struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit); >> uint32_t *surf; >> >> - /* BRW_NEW_TEXTURE_BUFFER */ >> - if (tObj->Target == GL_TEXTURE_BUFFER) { >> - brw_update_buffer_texture_surface(ctx, unit, surf_offset); >> - return; >> - } >> - >> surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, >> 6 * 4, 32, surf_offset); >> >> @@ -795,6 +789,21 @@ const struct brw_tracked_state >> gen6_renderbuffer_surfaces = { >> .emit = update_renderbuffer_surfaces, >> }; >> >> +static void >> +update_texture_surface(struct gl_context *ctx, >> + unsigned unit, >> + uint32_t *surf_offset, >> + bool for_gather) >> +{ >> + struct brw_context *brw = brw_context(ctx); >> + struct gl_texture_object *obj = ctx->Texture.Unit[unit]._Current; >> + >> + if (obj->Target == GL_TEXTURE_BUFFER) { >> + update_buffer_texture_surface(ctx, unit, surf_offset); > > In order to avoid extra level of indentation I used the following. I would > have preferred it here also. > > if (obj->Target == GL_TEXTURE_BUFFER) { > update_buffer_texture_surface(ctx, unit, surf_offset); > return; > } > I kept this as an indented block because it's harmless IMHO and it seemed a somewhat lesser evil than: 1/ Define all texture-specific variables (i.e. things that are not applicable to buffer textures, including some pointer dereferences) at the top level, which is what you did, but it seemed a bit dodgy. 2/ Mix statements and declarations. (Granted, this file is likely already relying on other C99 features, so it wouldn't matter in practice, it's just a codestyle itch) 3/ Declare stuff and leave it uninitialized until later.
That said, the reason was largely subjective, and I don't really have a
strong preference. As you are still the author of this commit you're
free to format it as you wish, you can keep my R-b if you simply
reindent this function.
>> + } else {
>> + brw->vtbl.update_texture_surface(ctx, unit, surf_offset, for_gather);
>> + }
>> +}
>>
>> static void
>> update_stage_texture_surfaces(struct brw_context *brw,
>> @@ -824,7 +833,7 @@ update_stage_texture_surfaces(struct brw_context *brw,
>>
>> /* _NEW_TEXTURE */
>> if (ctx->Texture.Unit[unit]._Current) {
>> - brw->vtbl.update_texture_surface(ctx, unit, surf_offset + s,
>> for_gather);
>> + update_texture_surface(ctx, unit, surf_offset + s, for_gather);
>> }
>> }
>> }
>> 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 15ab2b0..098b5c8 100644
>> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
>> @@ -356,43 +356,38 @@ gen7_update_texture_surface(struct gl_context *ctx,
>> struct brw_context *brw = brw_context(ctx);
>> struct gl_texture_object *obj = ctx->Texture.Unit[unit]._Current;
>>
>> - if (obj->Target == GL_TEXTURE_BUFFER) {
>> - brw_update_buffer_texture_surface(ctx, unit, surf_offset);
>> -
>> - } else {
>> - struct intel_texture_object *intel_obj = intel_texture_object(obj);
>> - struct intel_mipmap_tree *mt = intel_obj->mt;
>> - struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
>> - /* If this is a view with restricted NumLayers, then our effective
>> depth
>> - * is not just the miptree depth.
>> - */
>> - const unsigned depth = (obj->Immutable && obj->Target !=
>> GL_TEXTURE_3D ?
>> - obj->NumLayers : mt->logical_depth0);
>> -
>> - /* Handling GL_ALPHA as a surface format override breaks 1.30+ style
>> - * texturing functions that return a float, as our code generation
>> always
>> - * selects the .x channel (which would always be 0).
>> - */
>> - struct gl_texture_image *firstImage = obj->Image[0][obj->BaseLevel];
>> - const bool alpha_depth = obj->DepthMode == GL_ALPHA &&
>> - (firstImage->_BaseFormat == GL_DEPTH_COMPONENT ||
>> - firstImage->_BaseFormat == GL_DEPTH_STENCIL);
>> - const unsigned swizzle = (unlikely(alpha_depth) ? SWIZZLE_XYZW :
>> - brw_get_texture_swizzle(&brw->ctx, obj));
>> -
>> - unsigned format = translate_tex_format(
>> - brw, intel_obj->_Format, sampler->sRGBDecode);
>> -
>> - if (for_gather && format == BRW_SURFACEFORMAT_R32G32_FLOAT)
>> - format = BRW_SURFACEFORMAT_R32G32_FLOAT_LD;
>> -
>> - gen7_emit_texture_surface_state(brw, mt, obj->Target,
>> - obj->MinLayer, obj->MinLayer + depth,
>> - obj->MinLevel + obj->BaseLevel,
>> - obj->MinLevel + intel_obj->_MaxLevel
>> + 1,
>> - format, swizzle,
>> - surf_offset, false, for_gather);
>> - }
>> + struct intel_texture_object *intel_obj = intel_texture_object(obj);
>> + struct intel_mipmap_tree *mt = intel_obj->mt;
>> + struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
>> + /* If this is a view with restricted NumLayers, then our effective depth
>> + * is not just the miptree depth.
>> + */
>> + const unsigned depth = (obj->Immutable && obj->Target != GL_TEXTURE_3D ?
>> + obj->NumLayers : mt->logical_depth0);
>> +
>> + /* Handling GL_ALPHA as a surface format override breaks 1.30+ style
>> + * texturing functions that return a float, as our code generation always
>> + * selects the .x channel (which would always be 0).
>> + */
>> + struct gl_texture_image *firstImage = obj->Image[0][obj->BaseLevel];
>> + const bool alpha_depth = obj->DepthMode == GL_ALPHA &&
>> + (firstImage->_BaseFormat == GL_DEPTH_COMPONENT ||
>> + firstImage->_BaseFormat == GL_DEPTH_STENCIL);
>> + const unsigned swizzle = (unlikely(alpha_depth) ? SWIZZLE_XYZW :
>> + brw_get_texture_swizzle(&brw->ctx, obj));
>> +
>> + unsigned format = translate_tex_format(
>> + brw, intel_obj->_Format, sampler->sRGBDecode);
>> +
>> + if (for_gather && format == BRW_SURFACEFORMAT_R32G32_FLOAT)
>> + format = BRW_SURFACEFORMAT_R32G32_FLOAT_LD;
>> +
>> + gen7_emit_texture_surface_state(brw, mt, obj->Target,
>> + obj->MinLayer, obj->MinLayer + depth,
>> + obj->MinLevel + obj->BaseLevel,
>> + obj->MinLevel + intel_obj->_MaxLevel + 1,
>> + format, swizzle,
>> + surf_offset, false, for_gather);
>> }
>>
>> /**
>> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> b/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> index d0c2d80..2f9b98e 100644
>> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> @@ -255,44 +255,39 @@ gen8_update_texture_surface(struct gl_context *ctx,
>> struct brw_context *brw = brw_context(ctx);
>> struct gl_texture_object *obj = ctx->Texture.Unit[unit]._Current;
>>
>> - if (obj->Target == GL_TEXTURE_BUFFER) {
>> - brw_update_buffer_texture_surface(ctx, unit, surf_offset);
>> + struct gl_texture_image *firstImage = obj->Image[0][obj->BaseLevel];
>> + struct intel_texture_object *intel_obj = intel_texture_object(obj);
>> + struct intel_mipmap_tree *mt = intel_obj->mt;
>> + struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
>> + /* If this is a view with restricted NumLayers, then our effective depth
>> + * is not just the miptree depth.
>> + */
>> + const unsigned depth = (obj->Immutable && obj->Target != GL_TEXTURE_3D ?
>> + obj->NumLayers : mt->logical_depth0);
>>
>> - } else {
>> - struct gl_texture_image *firstImage = obj->Image[0][obj->BaseLevel];
>> - struct intel_texture_object *intel_obj = intel_texture_object(obj);
>> - struct intel_mipmap_tree *mt = intel_obj->mt;
>> - struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
>> - /* If this is a view with restricted NumLayers, then our effective
>> depth
>> - * is not just the miptree depth.
>> - */
>> - const unsigned depth = (obj->Immutable && obj->Target !=
>> GL_TEXTURE_3D ?
>> - obj->NumLayers : mt->logical_depth0);
>> -
>> - /* Handling GL_ALPHA as a surface format override breaks 1.30+ style
>> - * texturing functions that return a float, as our code generation
>> always
>> - * selects the .x channel (which would always be 0).
>> - */
>> - const bool alpha_depth = obj->DepthMode == GL_ALPHA &&
>> - (firstImage->_BaseFormat == GL_DEPTH_COMPONENT ||
>> - firstImage->_BaseFormat == GL_DEPTH_STENCIL);
>> - const unsigned swizzle = (unlikely(alpha_depth) ? SWIZZLE_XYZW :
>> - brw_get_texture_swizzle(&brw->ctx, obj));
>> -
>> - unsigned format = translate_tex_format(brw, intel_obj->_Format,
>> - sampler->sRGBDecode);
>> - if (obj->StencilSampling && firstImage->_BaseFormat ==
>> GL_DEPTH_STENCIL) {
>> - mt = mt->stencil_mt;
>> - format = BRW_SURFACEFORMAT_R8_UINT;
>> - }
>> -
>> - gen8_emit_texture_surface_state(brw, mt, obj->Target,
>> - obj->MinLayer, obj->MinLayer + depth,
>> - obj->MinLevel + obj->BaseLevel,
>> - obj->MinLevel + intel_obj->_MaxLevel
>> + 1,
>> - format, swizzle, surf_offset,
>> - false, for_gather);
>> + /* Handling GL_ALPHA as a surface format override breaks 1.30+ style
>> + * texturing functions that return a float, as our code generation always
>> + * selects the .x channel (which would always be 0).
>> + */
>> + const bool alpha_depth = obj->DepthMode == GL_ALPHA &&
>> + (firstImage->_BaseFormat == GL_DEPTH_COMPONENT ||
>> + firstImage->_BaseFormat == GL_DEPTH_STENCIL);
>> + const unsigned swizzle = (unlikely(alpha_depth) ? SWIZZLE_XYZW :
>> + brw_get_texture_swizzle(&brw->ctx, obj));
>> +
>> + unsigned format = translate_tex_format(brw, intel_obj->_Format,
>> + sampler->sRGBDecode);
>> + if (obj->StencilSampling && firstImage->_BaseFormat == GL_DEPTH_STENCIL)
>> {
>> + mt = mt->stencil_mt;
>> + format = BRW_SURFACEFORMAT_R8_UINT;
>> }
>> +
>> + gen8_emit_texture_surface_state(brw, mt, obj->Target,
>> + obj->MinLayer, obj->MinLayer + depth,
>> + obj->MinLevel + obj->BaseLevel,
>> + obj->MinLevel + intel_obj->_MaxLevel + 1,
>> + format, swizzle, surf_offset,
>> + false, for_gather);
>> }
>>
>> /**
>> --
>> 2.3.5
>>
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
