[Mesa-dev] [PATCH] i965: add 2xMSAA and 16xMSAA to DRI configs for Gen9.

[Rogovin, Kevin]

Hi,

 Please do not push yet. I will post a V2 that makes the pointer thing not so offensively ugly. Also, I missed the opportunity to also fix the DRI conf for Gen8, as it does have 2xMSAA.

 -Kevin

-----Original Message-----
From: Ben Widawsky [mailto:ben at bwidawsk.net] 
Sent: Thursday, August 24, 2017 8:10 PM
To: Rogovin, Kevin <kevin.rogovin at intel.com>
Cc: mesa-dev at lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH] i965: add 2xMSAA and 16xMSAA to DRI configs for Gen9.

On 17-08-24 14:16:39, kevin.rogovin at intel.com wrote:
>From: Kevin Rogovin <kevin.rogovin at intel.com>
>
>Special thanks to Eero Tamminen for reporting rasterizer numbers being 
>twice what it should be for 2xMSAA under a benchmark.
>
>Signed-off-by: Kevin Rogovin <kevin.rogovin at intel.com>
>---
> src/mesa/drivers/dri/i965/intel_screen.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
>diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
>b/src/mesa/drivers/dri/i965/intel_screen.c
>index 579554f..67eb776 100644
>--- a/src/mesa/drivers/dri/i965/intel_screen.c
>+++ b/src/mesa/drivers/dri/i965/intel_screen.c
>@@ -1882,7 +1882,7 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
>    };
>
>    static const uint8_t singlesample_samples[1] = {0};
>-   static const uint8_t multisample_samples[2]  = {4, 8};
>+   static const uint8_t multisample_samples_2_4_8_16[]  = {2, 4, 8, 
>+ 16};
>
>    struct intel_screen *screen = dri_screen->driverPrivate;
>    const struct gen_device_info *devinfo = &screen->devinfo; @@ 
>-1959,6 +1959,7 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
>     * supported.  Singlebuffer configs are not supported because no one wants
>     * them.
>     */
>+

No unnecessary whitespace changes, please.

>    for (unsigned i = 0; i < ARRAY_SIZE(formats); i++) {
>       if (devinfo->gen < 6)
>          break;
>@@ -1966,6 +1967,7 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
>       __DRIconfig **new_configs;
>       const int num_depth_stencil_bits = 2;
>       int num_msaa_modes = 0;
>+      const uint8_t *multisample_samples = NULL;
>
>       depth_bits[0] = 0;
>       stencil_bits[0] = 0;
>@@ -1978,10 +1980,16 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
>          stencil_bits[1] = 8;
>       }
>
>-      if (devinfo->gen >= 7)
>+      if (devinfo->gen >= 9) {
>+         multisample_samples = multisample_samples_2_4_8_16;
>+         num_msaa_modes = 4;
>+      } else if (devinfo->gen >= 7) {
>+         multisample_samples = multisample_samples_2_4_8_16 + 1;
>          num_msaa_modes = 2;
>-      else if (devinfo->gen == 6)
>+      } else if (devinfo->gen == 6) {
>+         multisample_samples = multisample_samples_2_4_8_16 + 1;
>          num_msaa_modes = 1;
>+      }

I think it'd be a little cleaner to just make GEN specific arrays. Easier to read, and you can just USE ARRAY_SIZE.... but I honestly don't care much.

if (devinfo->gen >= 9) {
   multisample_samples = multisample_samples_gen9;
   num_msaa_modes = ARRAY_SIZE(multisample_samples_gen9);
}

>
>       new_configs = driCreateConfigs(formats[i],
>                                      depth_bits,

Kind of shocking to me that we missed this previously for both when we added 2x MSAA and later 16x. Indeed looking at glxinfo, I see no 2x or 16x visuals.