[Mesa-dev] [PATCH] dri_util: remove ALLOW_RGB10_CONFIGS option (v2)

[Mario Kleiner]

On 01/11/2018 02:39 PM, Marek Olšák wrote:
> 
> 
> On Jan 11, 2018 7:14 AM, "Mario Kleiner" <mario.kleiner.de at gmail.com 
> <mailto:mario.kleiner.de at gmail.com>> wrote:
> 
>     On 01/10/2018 07:04 AM, Tapani Pälli wrote:
> 
>         Hi Marek;
> 
>         This one works but only if you add
> 
>         DRI_CONF_ALLOW_RGB10_CONFIGS("false")
> 
>         to the DRI_CONF_SECTION_MISCELLANEOUS section in intel_screen.
>         With that change: Reviewed-by: Tapani Pälli
>         <tapani.palli at intel.com <mailto:tapani.palli at intel.com>>
> 
> 
>     With this patch now committed to master, rgb10 visuals on i965 are
>     completely dead, as far as my testing goes. rgb10 is always off, and
>     the 'allow_rgb10_configs' option in drirc gets ignored for
>     enumeration of visuals/fbconfigs, e.g., in glxinfo.
> 
>     Before it worked on my machines, defaulted to on, and could be
>     controlled via drirc.
> 
>     As far as i can see, setting up &screen->optionCache for i965
>     happens too late, only at glXCreateContext() time ->
>     brwCreateContext() -> brw_process_driconf_options().
> 
>     The old way read the options file as part of driCreateNewScreen2(),
>     which was called as part of __glXInitialize, e.g., from
>     glXChooseVisual() -- early enough to affect the
>     enumeration/selection of visuals.
> 
>     So i don't know if the old way was the right way, but it did give
>     the right behavior for i965 whereas the new one doesn't.
> 
>     Ideas?
> 
> 
> I think that's not completely true. I965 loads drirc options for each 
> screen and then again for each context, which is weird.
> 
> Generally, the screen object isn't allowed to be modified by 
> glXCreateContext.
> 
> Marek
> 

Ok, the patch just sent out retains the new way of things, and defaults 
i965 to rgb10 off, but parses drirc again, so one can enable it via a

<option name="allow_rgb10_configs" value="true"/>

snippet. Tested under Wayland+Weston and X11 XOrg Server 1.19.3 with GLX 
and EGL apps.

-mario

> 
>     -mario
> 
> 
>         On 01/09/2018 04:04 PM, Marek Olšák wrote:
> 
>             From: Marek Olšák <marek.olsak at amd.com
>             <mailto:marek.olsak at amd.com>>
> 
>             This is unused because it's for libGL/libEGL, not drivers.
> 
>             v2: i965 was wrong, because it used dri_util instead of its
>             own config.
>             ---
>                src/mesa/drivers/dri/common/dri_util.c   | 4 ----
>                src/mesa/drivers/dri/i965/intel_screen.c | 2 +-
>                2 files changed, 1 insertion(+), 5 deletions(-)
> 
>             diff --git a/src/mesa/drivers/dri/common/dri_util.c
>             b/src/mesa/drivers/dri/common/dri_util.c
>             index d4fba0b..e6a7d23 100644
>             --- a/src/mesa/drivers/dri/common/dri_util.c
>             +++ b/src/mesa/drivers/dri/common/dri_util.c
>             @@ -48,24 +48,20 @@
>                #include "main/version.h"
>                #include "main/debug_output.h"
>                #include "main/errors.h"
>                #include "main/macros.h"
>                const char __dri2ConfigOptions[] =
>                   DRI_CONF_BEGIN
>                      DRI_CONF_SECTION_PERFORMANCE
>                         DRI_CONF_VBLANK_MODE(DRI_CONF_VBLANK_DEF_INTERVAL_1)
>                      DRI_CONF_SECTION_END
>             -
>             -      DRI_CONF_SECTION_MISCELLANEOUS
>             -         DRI_CONF_ALLOW_RGB10_CONFIGS("true")
>             -      DRI_CONF_SECTION_END
>                   DRI_CONF_END;
>               
>             /*****************************************************************/
>                /** \name Screen handling functions          */
>               
>             /*****************************************************************/
>                /*@{*/
>                static void
>                setupLoaderExtensions(__DRIscreen *psp,
>                              const __DRIextension **extensions)
>             diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
>             b/src/mesa/drivers/dri/i965/intel_screen.c
>             index 3e016b5..89db821 100644
>             --- a/src/mesa/drivers/dri/i965/intel_screen.c
>             +++ b/src/mesa/drivers/dri/i965/intel_screen.c
>             @@ -2057,21 +2057,21 @@
>             intel_screen_make_configs(__DRIscreen *dri_screen)
>                   __DRIconfig **configs = NULL;
>                   /* Expose only BGRA ordering if the loader doesn't
>             support RGBA ordering. */
>                   unsigned num_formats;
>                   if (intel_loader_get_cap(dri_screen,
>             DRI_LOADER_CAP_RGBA_ORDERING))
>                      num_formats = ARRAY_SIZE(formats);
>                   else
>                      num_formats = ARRAY_SIZE(formats) - 2; /* all -
>             RGBA_ORDERING formats */
>                   /* Shall we expose 10 bpc formats? */
>             -   bool allow_rgb10_configs =
>             driQueryOptionb(&dri_screen->optionCache,
>             +   bool allow_rgb10_configs =
>             driQueryOptionb(&screen->optionCache,
>                                "allow_rgb10_configs");
>                   /* Generate singlesample configs without accumulation
>             buffer. */
>                   for (unsigned i = 0; i < num_formats; i++) {
>                      __DRIconfig **new_configs;
>                      int num_depth_stencil_bits = 2;
>                      if (!allow_rgb10_configs &&
>                          (formats[i] == MESA_FORMAT_B10G10R10A2_UNORM ||
>                           formats[i] == MESA_FORMAT_B10G10R10X2_UNORM))
> 
>         _______________________________________________
>         mesa-dev mailing list
>         mesa-dev at lists.freedesktop.org
>         <mailto:mesa-dev at lists.freedesktop.org>
>         https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>         <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
> 
>