[Mesa-dev] [PATCH V2 13/22] i965: Use BRW_SURFACE_* in place of GL_TEXTURE_*

[Pohjolainen, Topi]

On Fri, Apr 17, 2015 at 04:51:34PM -0700, Anuj Phogat wrote:
> Makes no functional changes in the code.
> 
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> Reviewed-by: Chris Forbes <chrisf at ijw.co.nz>
> ---
>  src/mesa/drivers/dri/i965/gen8_surface_state.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> index d2eceff..1ec57c0 100644
> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> @@ -172,6 +172,7 @@ gen8_update_texture_surface(struct gl_context *ctx,
>     struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
>     struct intel_mipmap_tree *aux_mt = NULL;
>     uint32_t aux_mode = 0;
> +   uint32_t surf_type;
>     mesa_format format = intelObj->_Format;
>     uint32_t mocs_wb = brw->gen >= 9 ? SKL_MOCS_WB : BDW_MOCS_WB;
>  
> @@ -201,29 +202,31 @@ gen8_update_texture_surface(struct gl_context *ctx,
>        aux_mode = GEN8_SURFACE_AUX_MODE_MCS;
>     }
>  
> +   surf_type = translate_tex_target(tObj->Target);
> +

I would rather just write:

     const uint32_t surf_type = translate_tex_target(tObj->Target);

>     /* If this is a view with restricted NumLayers, then our effective depth
>      * is not just the miptree depth.
>      */
>     uint32_t effective_depth =
> -      (tObj->Immutable && tObj->Target != GL_TEXTURE_3D) ? tObj->NumLayers
> -                                                         : mt->logical_depth0;
> +      (tObj->Immutable && surf_type != BRW_SURFACE_3D) ? tObj->NumLayers
> +                                                       : mt->logical_depth0;
>  
>     uint32_t tex_format = translate_tex_format(brw, format, sampler->sRGBDecode);
>  
>     uint32_t *surf = allocate_surface_state(brw, surf_offset);
>  
> -   surf[0] = translate_tex_target(tObj->Target) << BRW_SURFACE_TYPE_SHIFT |
> +

I guess you didn't mean to have extra newline here.

> +   surf[0] = surf_type << BRW_SURFACE_TYPE_SHIFT |

Lets use SET_FIELD(surf_type, BRW_SURFACE_TYPE) instead.

With those:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

Having said that, would you mind rebasing this on top of my series that
moves all the decision making common to all hardware platforms into one
place?

>               tex_format << BRW_SURFACE_FORMAT_SHIFT |
>               vertical_alignment(mt) |
>               horizontal_alignment(mt) |
>               tiling_mode;
>  
> -   if (tObj->Target == GL_TEXTURE_CUBE_MAP ||
> -       tObj->Target == GL_TEXTURE_CUBE_MAP_ARRAY) {
> +   if (surf_type == BRW_SURFACE_CUBE) {
>        surf[0] |= BRW_SURFACE_CUBEFACE_ENABLES;
>     }
>  
> -   if (mt->logical_depth0 > 1 && tObj->Target != GL_TEXTURE_3D)
> +   if (mt->logical_depth0 > 1 && surf_type != BRW_SURFACE_3D)
>        surf[0] |= GEN8_SURFACE_IS_ARRAY;
>  
>     surf[1] = SET_FIELD(mocs_wb, GEN8_SURFACE_MOCS) | mt->qpitch >> 2;
> -- 
> 2.3.4
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev