On Thu, Jul 2, 2015 at 12:57 PM, Matt Turner <matts...@gmail.com> wrote:
> On Wed, Jul 1, 2015 at 4:03 PM, Ben Widawsky
> <benjamin.widaw...@intel.com> wrote:
>> Certain compressed formats require this setting. The docs don't go into much
>> detail as to why it's needed exactly.
>>
>> This fixes 0 piglit failures with a GBM gpu piglit run.
>
> That's a really weird way of saying that.
>
>>
>> Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
>> ---
>>
>> I had this one sitting around for almost 2 months. I'm not sure why I didn't
>> send it out sooner. It seems like it's needed.
>>
>> ---
>> src/mesa/drivers/dri/i965/brw_defines.h | 1 +
>> src/mesa/drivers/dri/i965/gen8_surface_state.c | 26
>> ++++++++++++++++++++++++++
>> 2 files changed, 27 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
>> b/src/mesa/drivers/dri/i965/brw_defines.h
>> index 66b9abc..f55fd49 100644
>> --- a/src/mesa/drivers/dri/i965/brw_defines.h
>> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
>> @@ -276,6 +276,7 @@
>> #define GEN8_SURFACE_TILING_W (1 << 12)
>> #define GEN8_SURFACE_TILING_X (2 << 12)
>> #define GEN8_SURFACE_TILING_Y (3 << 12)
>> +#define GEN8_SURFACE_SAMPLER_L2_BYPASS_DISABLE (1 << 9)
>> #define BRW_SURFACE_RC_READ_WRITE (1 << 8)
>> #define BRW_SURFACE_MIPLAYOUT_SHIFT 10
>> #define BRW_SURFACE_MIPMAPLAYOUT_BELOW 0
>> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> b/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> index ca5ed17..a245379 100644
>> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> @@ -264,6 +264,19 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
>> surf[0] |= BRW_SURFACE_CUBEFACE_ENABLES;
>> }
>>
>> + if (brw->is_cherryview || brw->gen >= 9) {
>> + /* "This bit must be set for the following surface types: BC2_UNORM
>> + * BC3_UNORM BC5_UNORM BC5_SNORM BC7_UNORM"
>
> Don't do naked quotes -- Use the normal style.
>
>> + */
>> + switch (format) {
>> + case BRW_SURFACEFORMAT_BC2_UNORM:
>> + case BRW_SURFACEFORMAT_BC3_UNORM:
>> + case BRW_SURFACEFORMAT_BC5_SNORM:
>
> Missing BRW_SURFACEFORMAT_BC5_UNORM.
>
>> + case BRW_SURFACEFORMAT_BC7_UNORM:
>> + surf[0] |= GEN8_SURFACE_SAMPLER_L2_BYPASS_DISABLE;
>
> It wouldn't surprise me if static analysis tools complain about the
> missing break.
>
> Add a break or make it an if statement (which would be two lines
> shorter). All together, how about
>
> /* From the CHV PRM, Volume 2d, page 321 (RENDER_SURFACE_STATE dword 0
> * bit 9 "Sampler L2 Bypass Mode Disable" Programming Notes):
> *
> * This bit must be set for the following surface types: BC2_UNORM
> * BC3_UNORM BC5_UNORM BC5_SNORM BC7_UNORM
> */
> if (format == BRW_SURFACEFORMAT_BC2_UNORM ||
> format == BRW_SURFACEFORMAT_BC3_UNORM ||
> format == BRW_SURFACEFORMAT_BC5_SNORM ||
> format == BRW_SURFACEFORMAT_BC5_UNORM ||
Bah, would be nice to so BC5_UNORM and then BC5_SNORM to better match
the comment.
> format == BRW_SURFACEFORMAT_BC7_UNORM)
> surf[0] |= GEN8_SURFACE_SAMPLER_L2_BYPASS_DISABLE;
> }
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
