[Mesa-dev] [PATCH] i965: Replace 0 with ISL_FORMAT_UNSUPPORTED in format table (v2)

2017-06-01 Thread Chad Versace
From: Chad Versace 

When given an *unsupported* mesa_format,
brw_isl_format_for_mesa_format() returned 0, a *valid* isl_format,
ISL_FORMAT_R32G32B32A32_FLOAT.  The problem is that
brw_isl_format_for_mesa_format's inner table used 0 instead of
ISL_FORMAT_UNSUPPORTED to indicate unsupported mesa formats.

Some callers of brw_isl_format_for_mesa_format() were aware of this
weirdness, and worked around it. This patch removes those workarounds.

Tested on Broadwell as below, no regressions:

> deqp-egl --deqp-case='dEQP-EGL.functional.image.modify.*'
Test run totals:
  Passed:24/37 (64.9%)
  Failed:0/37 (0.0%)
  Not supported: 13/37 (35.1%)
  Warnings:  0/37 (0.0%)

> deqp-gles3 --deqp-case='dEQP-GLES3.functional.texture.format.*'
Test run totals:
  Passed:530/530 (100.0%)
  Failed:0/530 (0.0%)
  Not supported: 0/530 (0.0%)
  Warnings:  0/530 (0.0%)


v2: Ensure that all array elements are initialized to
  ISL_FORMAT_UNSUPPORTED, even when new formats are added to enum
  mesa_format, by using an designated range initializer.
---

 src/mesa/drivers/dri/i965/brw_surface_formats.c  | 96 ++--
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  2 +-
 2 files changed, 6 insertions(+), 92 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c 
b/src/mesa/drivers/dri/i965/brw_surface_formats.c
index 52d3acb..f878317 100644
--- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
+++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
@@ -36,31 +36,19 @@ brw_isl_format_for_mesa_format(mesa_format mesa_format)
 * staying in sync, so we initialize to 0 even though
 * ISL_FORMAT_R32G32B32A32_FLOAT happens to also be 0.
 */
-   static const uint32_t table[MESA_FORMAT_COUNT] =
-   {
-  [MESA_FORMAT_A8B8G8R8_UNORM] = 0,
+   static const enum isl_format table[MESA_FORMAT_COUNT] = {
+  [0 ... MESA_FORMAT_COUNT-1] = ISL_FORMAT_UNSUPPORTED,
+
   [MESA_FORMAT_R8G8B8A8_UNORM] = ISL_FORMAT_R8G8B8A8_UNORM,
   [MESA_FORMAT_B8G8R8A8_UNORM] = ISL_FORMAT_B8G8R8A8_UNORM,
-  [MESA_FORMAT_A8R8G8B8_UNORM] = 0,
-  [MESA_FORMAT_X8B8G8R8_UNORM] = 0,
   [MESA_FORMAT_R8G8B8X8_UNORM] = ISL_FORMAT_R8G8B8X8_UNORM,
   [MESA_FORMAT_B8G8R8X8_UNORM] = ISL_FORMAT_B8G8R8X8_UNORM,
-  [MESA_FORMAT_X8R8G8B8_UNORM] = 0,
-  [MESA_FORMAT_BGR_UNORM8] = 0,
   [MESA_FORMAT_RGB_UNORM8] = ISL_FORMAT_R8G8B8_UNORM,
   [MESA_FORMAT_B5G6R5_UNORM] = ISL_FORMAT_B5G6R5_UNORM,
-  [MESA_FORMAT_R5G6B5_UNORM] = 0,
   [MESA_FORMAT_B4G4R4A4_UNORM] = ISL_FORMAT_B4G4R4A4_UNORM,
-  [MESA_FORMAT_A4R4G4B4_UNORM] = 0,
-  [MESA_FORMAT_A1B5G5R5_UNORM] = 0,
   [MESA_FORMAT_B5G5R5A1_UNORM] = ISL_FORMAT_B5G5R5A1_UNORM,
-  [MESA_FORMAT_A1R5G5B5_UNORM] = 0,
-  [MESA_FORMAT_L4A4_UNORM] = 0,
   [MESA_FORMAT_L8A8_UNORM] = ISL_FORMAT_L8A8_UNORM,
-  [MESA_FORMAT_A8L8_UNORM] = 0,
   [MESA_FORMAT_L16A16_UNORM] = ISL_FORMAT_L16A16_UNORM,
-  [MESA_FORMAT_A16L16_UNORM] = 0,
-  [MESA_FORMAT_B2G3R3_UNORM] = 0,
   [MESA_FORMAT_A_UNORM8] = ISL_FORMAT_A8_UNORM,
   [MESA_FORMAT_A_UNORM16] = ISL_FORMAT_A16_UNORM,
   [MESA_FORMAT_L_UNORM8] = ISL_FORMAT_L8_UNORM,
@@ -71,29 +59,16 @@ brw_isl_format_for_mesa_format(mesa_format mesa_format)
   [MESA_FORMAT_YCBCR] = ISL_FORMAT_YCRCB_SWAPUVY,
   [MESA_FORMAT_R_UNORM8] = ISL_FORMAT_R8_UNORM,
   [MESA_FORMAT_R8G8_UNORM] = ISL_FORMAT_R8G8_UNORM,
-  [MESA_FORMAT_G8R8_UNORM] = 0,
   [MESA_FORMAT_R_UNORM16] = ISL_FORMAT_R16_UNORM,
   [MESA_FORMAT_R16G16_UNORM] = ISL_FORMAT_R16G16_UNORM,
-  [MESA_FORMAT_G16R16_UNORM] = 0,
   [MESA_FORMAT_B10G10R10A2_UNORM] = ISL_FORMAT_B10G10R10A2_UNORM,
-  [MESA_FORMAT_S8_UINT_Z24_UNORM] = 0,
-  [MESA_FORMAT_Z24_UNORM_S8_UINT] = 0,
-  [MESA_FORMAT_Z_UNORM16] = 0,
-  [MESA_FORMAT_Z24_UNORM_X8_UINT] = 0,
-  [MESA_FORMAT_X8_UINT_Z24_UNORM] = 0,
-  [MESA_FORMAT_Z_UNORM32] = 0,
   [MESA_FORMAT_S_UINT8] = ISL_FORMAT_R8_UINT,
 
-  [MESA_FORMAT_BGR_SRGB8] = 0,
-  [MESA_FORMAT_A8B8G8R8_SRGB] = 0,
   [MESA_FORMAT_B8G8R8A8_SRGB] = ISL_FORMAT_B8G8R8A8_UNORM_SRGB,
-  [MESA_FORMAT_A8R8G8B8_SRGB] = 0,
   [MESA_FORMAT_R8G8B8A8_SRGB] = ISL_FORMAT_R8G8B8A8_UNORM_SRGB,
-  [MESA_FORMAT_X8R8G8B8_SRGB] = 0,
   [MESA_FORMAT_B8G8R8X8_SRGB] = ISL_FORMAT_B8G8R8X8_UNORM_SRGB,
   [MESA_FORMAT_L_SRGB8] = ISL_FORMAT_L8_UNORM_SRGB,
   [MESA_FORMAT_L8A8_SRGB] = ISL_FORMAT_L8A8_UNORM_SRGB,
-  [MESA_FORMAT_A8L8_SRGB] = 0,
   [MESA_FORMAT_SRGB_DXT1] = ISL_FORMAT_BC1_UNORM_SRGB,
   [MESA_FORMAT_SRGBA_DXT1] = ISL_FORMAT_BC1_UNORM_SRGB,
   [MESA_FORMAT_SRGBA_DXT3] = ISL_FORMAT_BC2_UNORM_SRGB,
@@ -109,7 +84,6 @@ brw_isl_format_for_mesa_format(mesa_format mesa_format)
   [MESA_FORMAT_RGBA_FLOAT32] = ISL_FORMAT_R32G32B32A32_FLOAT,
   [MESA_FORMAT_RGBA_FLOAT16] = ISL_FORMAT_R16G16B16A16_FLOAT,
   [MESA_FORMAT_RGB_FLO

Re: [Mesa-dev] [PATCH] i965: Replace 0 with ISL_FORMAT_UNSUPPORTED in format table (v2)

2017-06-01 Thread Matt Turner
On Thu, Jun 1, 2017 at 12:55 PM, Chad Versace  wrote:
> From: Chad Versace 
>
> When given an *unsupported* mesa_format,
> brw_isl_format_for_mesa_format() returned 0, a *valid* isl_format,
> ISL_FORMAT_R32G32B32A32_FLOAT.  The problem is that
> brw_isl_format_for_mesa_format's inner table used 0 instead of
> ISL_FORMAT_UNSUPPORTED to indicate unsupported mesa formats.
>
> Some callers of brw_isl_format_for_mesa_format() were aware of this
> weirdness, and worked around it. This patch removes those workarounds.
>
> Tested on Broadwell as below, no regressions:
>
> > deqp-egl --deqp-case='dEQP-EGL.functional.image.modify.*'
> Test run totals:
>   Passed:24/37 (64.9%)
>   Failed:0/37 (0.0%)
>   Not supported: 13/37 (35.1%)
>   Warnings:  0/37 (0.0%)
>
> > deqp-gles3 --deqp-case='dEQP-GLES3.functional.texture.format.*'
> Test run totals:
>   Passed:530/530 (100.0%)
>   Failed:0/530 (0.0%)
>   Not supported: 0/530 (0.0%)
>   Warnings:  0/530 (0.0%)
>
>
> v2: Ensure that all array elements are initialized to
>   ISL_FORMAT_UNSUPPORTED, even when new formats are added to enum
>   mesa_format, by using an designated range initializer.
> ---
>
>  src/mesa/drivers/dri/i965/brw_surface_formats.c  | 96 
> ++--
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  2 +-
>  2 files changed, 6 insertions(+), 92 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c 
> b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> index 52d3acb..f878317 100644
> --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
> +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> @@ -36,31 +36,19 @@ brw_isl_format_for_mesa_format(mesa_format mesa_format)
>  * staying in sync, so we initialize to 0 even though
>  * ISL_FORMAT_R32G32B32A32_FLOAT happens to also be 0.
>  */
> -   static const uint32_t table[MESA_FORMAT_COUNT] =
> -   {
> -  [MESA_FORMAT_A8B8G8R8_UNORM] = 0,
> +   static const enum isl_format table[MESA_FORMAT_COUNT] = {
> +  [0 ... MESA_FORMAT_COUNT-1] = ISL_FORMAT_UNSUPPORTED,
> +

Awesome.

Reviewed-by: Matt Turner 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev