Re: [RFC] DeepColor Visual Class Extension

[Alex Goins]

On Thu, 2 Nov 2017, Adam Jackson wrote:

> On Thu, 2017-10-26 at 19:50 -0700, Alex Goins wrote:
> 
> > DPCGetWindowDisplayCapabilities
> > 
> > window: WINDOW
> > =>
> > output: OUTPUT
> > colorspace_list: LISTofCOLORSPACEPRIORITY
> > 
> > Errors: Window
> > 
> > DPCGetWindowDisplayCapabilities functions identically to
> > DPCGetDisplayCapabilities, but rather than explicitly specifying an 
> > output,
> > the user must specify a window. The request then retrieves a list of 
> > color
> > spaces/encodings and their associated scores from the output upon which 
> > the
> > window is centered.
> 
> Does this need to do something special for root windows? Is "whatever
> the root is centered on" good enough? How do you break ties for a 2x2
> grid of identically-sized monitors? ("Pick something" is fine, anyone
> doing that with four non-identical monitors is kind of being a jerk.)
> 
> Does it make sense to send an event when this would change, ie, when
> the window's center crosses outputs? The app is going to want it, it
> seems silly to require a round trip to get it. If so, does the ordering
> matter with respect to the associated ConfigureNotify?

Good points. For root windows, maybe it should return the result for the primary
output? Similarly, ties could prefer the primary output, followed by "pick
something."

Having an associated event makes sense. I don't think ordering should matter
much, as the info is just a suggestion to the application that it might want to
pick a different color space. It doesn't even have to do it, as it's expected
that supported colorspaces are supported on all outputs, albeit posssibly with
necessary conversion.

Ordering matters more when the application actually makes the change.

> > DPCOverrideCompositorCapabilities
> > 
> > output: OUTPUT
> > colorspace_list: LISTofCOLORSPACEPRIORITY
> > 
> > Errors: Output, Match
> > 
> > Used by a composite manager to override the set of possible color
> > spaces/encodings and associated scores for composition for a given 
> > output.
> > 
> > If used before requesting RedirectSubwindows on the root window, changes
> > will not take effect until a subsequent redirection of the root window
> > hierarchy by the requester has completed. Color spaces/encodings 
> > associated
> > with each output must be identical, but scores may vary. If a composite
> > manager fails to fulfill these requirements before requesting
> > RedirectSubwindows on the root window, the server will instead empty 
> > the set
> > of capabilities on each output, generating a DPCCompositorChangeNotify 
> > event
> > for each.
> > 
> > If used after the root window hierarchy has been redirected by the
> > requester, changes take effect immediately. The set of color
> > spaces/encodings specified must match those of other outputs on the 
> > screen,
> > or a Match error results. Scores, however, may differ from other 
> > outputs.
> > 
> > DPCCompositorChangeNotify events will be generated on the appropriate 
> > root
> > window when the compositor capabilities are changed by this request. If
> > applicable, the changes do not take effect until after the subsequent
> > redirection of the root window hierarchy has completed, thereby 
> > delaying the
> > generation of the event.
> 
> I think this request should take a list of { output, colorspace_list }
> tuples so the request can complete atomically. Once you've redirected
> the root window there's no reasonable way for the server to defer
> sending the CompositorChangeNotify events, so updating the scores for
> every output could generate one event per output, clients would need to
> guess how many more colorspace changes are coming before re-rendering,
> or else do redundant, possibly racy work.
> 
> > DPCCompositorChangeNotify
> > 
> > requester: WINDOW   window requesting notification
> > output: OUTPUT  output affected by change
> > colorspace_list: LISTofCOLORSPACEPRIORITY  updated compositor 
> > capabilities
> 
> Likewise I think this event should return a list.

My initial reason for not having a list was that it would result in a list of
lists. Is there a precedent for this? It seems like it would complicate the
encoding unless the nested list is fixed length.

Thanks,
Alex

> - ajax
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH glproto 2/2] meson: Add meson build support

[Dylan Baker]

This does everything the autotools build does except muck with your git
config. It does so much faster, and has an "ext_glproto" dependency that
is meant to be used with meson's wrap capability.

Signed-off-by: Dylan Baker 
---
 meson.build | 42 ++
 1 file changed, 42 insertions(+)
 create mode 100644 meson.build

diff --git a/meson.build b/meson.build
new file mode 100644
index 000..aaa2fe6
--- /dev/null
+++ b/meson.build
@@ -0,0 +1,42 @@
+# Copyright © 2017 Intel Corporation
+
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+project('glproto', license : 'MIT', version : '1.4.17')
+
+install_headers(
+  'include/GL/glxint.h',
+  'include/GL/glxmd.h',
+  'include/GL/glxproto.h',
+  'include/GL/glxtokens.h',
+  subdir : 'GL')
+install_headers('include/GL/internal/glcore.h', subdir : 'GL/internal')
+
+pkg = import('pkgconfig')
+
+pkg.generate(
+  name : 'GLProto',
+  filebase : 'glproto',
+  description : 'GL extension headers',
+  version : meson.project_version(),
+)
+
+ext_glproto = declare_dependency(
+  include_directories : include_directories('include')
+)
-- 
2.15.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH glproto 1/2] Move headers into structured include directory

[Dylan Baker]

So that meson can use that as an include path for dependencies.

Signed-off-by: Dylan Baker 
---
 Makefile.am  | 10 +-
 glxint.h => include/GL/glxint.h  |  0
 glxmd.h => include/GL/glxmd.h|  0
 glxproto.h => include/GL/glxproto.h  |  0
 glxtokens.h => include/GL/glxtokens.h|  0
 glcore.h => include/GL/internal/glcore.h |  0
 6 files changed, 5 insertions(+), 5 deletions(-)
 rename glxint.h => include/GL/glxint.h (100%)
 rename glxmd.h => include/GL/glxmd.h (100%)
 rename glxproto.h => include/GL/glxproto.h (100%)
 rename glxtokens.h => include/GL/glxtokens.h (100%)
 rename glcore.h => include/GL/internal/glcore.h (100%)

diff --git a/Makefile.am b/Makefile.am
index 40cb57c..0c50dcc 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,13 +1,13 @@
 gldir = $(includedir)/GL
 gl_HEADERS = \
-   glxint.h \
-   glxmd.h \
-   glxproto.h \
-   glxtokens.h
+   include/GL/glxint.h \
+   include/GL/glxmd.h \
+   include/GL/glxproto.h \
+   include/GL/glxtokens.h
 
 glinternaldir = $(includedir)/GL/internal
 glinternal_HEADERS = \
-   glcore.h
+   include/GL/internal/glcore.h
 
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = glproto.pc
diff --git a/glxint.h b/include/GL/glxint.h
similarity index 100%
rename from glxint.h
rename to include/GL/glxint.h
diff --git a/glxmd.h b/include/GL/glxmd.h
similarity index 100%
rename from glxmd.h
rename to include/GL/glxmd.h
diff --git a/glxproto.h b/include/GL/glxproto.h
similarity index 100%
rename from glxproto.h
rename to include/GL/glxproto.h
diff --git a/glxtokens.h b/include/GL/glxtokens.h
similarity index 100%
rename from glxtokens.h
rename to include/GL/glxtokens.h
diff --git a/glcore.h b/include/GL/internal/glcore.h
similarity index 100%
rename from glcore.h
rename to include/GL/internal/glcore.h
-- 
2.15.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH glproto 0/2] meson build system

[Dylan Baker]

This is a pretty straightforward port of glproto to meson. glproto is pretty
simple, and doesn't need to be rebuilt often, so the benefit is not in configure
speed (although it is much faster). The real benefit here is that with this
series glproto can be used with meson's wrap functionality, which allows meson
to automatically fetch, configure, and build dependencies as part of the build
process. This isn't all that useful for platforms with competent package mangers
or ports systems, but for platforms like macOS and Windows this is invaluable.

Dylan Baker (2):
  Move headers into structured include directory
  meson: Add meson build support

 Makefile.am  | 10 
 glxint.h => include/GL/glxint.h  |  0
 glxmd.h => include/GL/glxmd.h|  0
 glxproto.h => include/GL/glxproto.h  |  0
 glxtokens.h => include/GL/glxtokens.h|  0
 glcore.h => include/GL/internal/glcore.h |  0
 meson.build  | 42 
 7 files changed, 47 insertions(+), 5 deletions(-)
 rename glxint.h => include/GL/glxint.h (100%)
 rename glxmd.h => include/GL/glxmd.h (100%)
 rename glxproto.h => include/GL/glxproto.h (100%)
 rename glxtokens.h => include/GL/glxtokens.h (100%)
 rename glcore.h => include/GL/internal/glcore.h (100%)
 create mode 100644 meson.build

-- 
2.15.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver] xkb: initialize tsyms

[Giuseppe Bilotta]

This fixes some “Conditional jump depends on uninitialized value(s)”
errors spotted by valgrind.

Signed-off-by: Giuseppe Bilotta 
---
 xkb/xkbUtils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xkb/xkbUtils.c b/xkb/xkbUtils.c
index 25b5a364e..8975ade8d 100644
--- a/xkb/xkbUtils.c
+++ b/xkb/xkbUtils.c
@@ -222,7 +222,7 @@ XkbUpdateKeyTypesFromCore(DeviceIntPtr pXDev,
 XkbDescPtr xkb;
 unsigned key, nG, explicit;
 int types[XkbNumKbdGroups];
-KeySym tsyms[XkbMaxSymsPerKey], *syms;
+KeySym tsyms[XkbMaxSymsPerKey] = {NoSymbol}, *syms;
 XkbMapChangesPtr mc;
 
 xkb = pXDev->key->xkbInfo->desc;
-- 
2.14.1.439.g647b9b4702

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 2/2] glx: Implement GLX_EXT_no_config_context (v2)

[Adam Jackson]

Only enabled for the DRI backends at the moment. In principle WGL/CGL
could support this - it's sort of implied by GL 3.0 support - but in
practice I don't know that it would actually work.

This is currently a draft extension, under review at:

https://github.com/KhronosGroup/OpenGL-Registry/pull/102

v2: Require that the two screen numbers match, per v4 of spec.

Signed-off-by: Adam Jackson 
---
 glx/createcontext.c| 20 +---
 glx/extension_string.c |  1 +
 glx/extension_string.h |  1 +
 glx/glxcmds.c  |  6 +++---
 glx/glxdri2.c  | 13 +++--
 glx/glxdriswrast.c |  8 ++--
 6 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/glx/createcontext.c b/glx/createcontext.c
index 1216f9412..76316db67 100644
--- a/glx/createcontext.c
+++ b/glx/createcontext.c
@@ -93,7 +93,7 @@ __glXDisp_CreateContextAttribsARB(__GLXclientState * cl, 
GLbyte * pc)
 __GLXcontext *ctx = NULL;
 __GLXcontext *shareCtx = NULL;
 __GLXscreen *glxScreen;
-__GLXconfig *config;
+__GLXconfig *config = NULL;
 int err;
 
 /* The GLX_ARB_create_context_robustness spec says:
@@ -136,8 +136,10 @@ __glXDisp_CreateContextAttribsARB(__GLXclientState * cl, 
GLbyte * pc)
 if (!validGlxScreen(client, req->screen, , ))
 return __glXError(GLXBadFBConfig);
 
-if (!validGlxFBConfig(client, glxScreen, req->fbconfig, , ))
-return __glXError(GLXBadFBConfig);
+if (req->fbconfig) {
+if (!validGlxFBConfig(client, glxScreen, req->fbconfig, , ))
+return __glXError(GLXBadFBConfig);
+}
 
 /* Validate the context with which the new context should share resources.
  */
@@ -182,6 +184,9 @@ __glXDisp_CreateContextAttribsARB(__GLXclientState * cl, 
GLbyte * pc)
 break;
 
 case GLX_RENDER_TYPE:
+/* Not valid for GLX_EXT_no_config_context */
+if (!req->fbconfig)
+return BadValue;
 render_type = attribs[2 * i + 1];
 break;
 
@@ -206,6 +211,15 @@ __glXDisp_CreateContextAttribsARB(__GLXclientState * cl, 
GLbyte * pc)
 break;
 #endif
 
+case GLX_SCREEN:
+/* Only valid for GLX_EXT_no_config_context */
+if (req->fbconfig)
+return BadValue;
+/* Must match the value in the request header */
+if (attribs[2 * i + 1] != req->screen)
+return BadValue;
+break;
+
 default:
 if (!req->isDirect)
 return BadValue;
diff --git a/glx/extension_string.c b/glx/extension_string.c
index d1da4815c..102f9dd42 100644
--- a/glx/extension_string.c
+++ b/glx/extension_string.c
@@ -86,6 +86,7 @@ static const struct extension_info known_glx_extensions[] = {
 { GLX(EXT_framebuffer_sRGB),VER(0,0), N, },
 { GLX(EXT_import_context),  VER(0,0), Y, },
 { GLX(EXT_libglvnd),VER(0,0), N, },
+{ GLX(EXT_no_config_context),   VER(0,0), N, },
 { GLX(EXT_stereo_tree), VER(0,0), N, },
 { GLX(EXT_texture_from_pixmap), VER(0,0), N, },
 { GLX(EXT_visual_info), VER(0,0), Y, },
diff --git a/glx/extension_string.h b/glx/extension_string.h
index a10d7108a..f049f5840 100644
--- a/glx/extension_string.h
+++ b/glx/extension_string.h
@@ -48,6 +48,7 @@ enum {
 EXT_fbconfig_packed_float_bit,
 EXT_import_context_bit,
 EXT_libglvnd_bit,
+EXT_no_config_context_bit,
 EXT_stereo_tree_bit,
 EXT_texture_from_pixmap_bit,
 EXT_visual_info_bit,
diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index 8681ef13f..324558b6e 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -1738,13 +1738,13 @@ DoQueryContext(__GLXclientState * cl, GLXContextID gcId)
 sendBuf[0] = GLX_SHARE_CONTEXT_EXT;
 sendBuf[1] = (int) (ctx->share_id);
 sendBuf[2] = GLX_VISUAL_ID_EXT;
-sendBuf[3] = (int) (ctx->config->visualID);
+sendBuf[3] = (int) (ctx->config ? ctx->config->visualID : 0);
 sendBuf[4] = GLX_SCREEN_EXT;
 sendBuf[5] = (int) (ctx->pGlxScreen->pScreen->myNum);
 sendBuf[6] = GLX_FBCONFIG_ID;
-sendBuf[7] = (int) (ctx->config->fbconfigID);
+sendBuf[7] = (int) (ctx->config ? ctx->config->fbconfigID : 0);
 sendBuf[8] = GLX_RENDER_TYPE;
-sendBuf[9] = (int) (ctx->config->renderType);
+sendBuf[9] = (int) (ctx->config ? ctx->config->renderType : GLX_DONT_CARE);
 
 if (client->swapped) {
 int length = reply.length;
diff --git a/glx/glxdri2.c b/glx/glxdri2.c
index 0a5463445..bc8c23b14 100644
--- a/glx/glxdri2.c
+++ b/glx/glxdri2.c
@@ -441,6 +441,7 @@ create_driver_context(__GLXDRIcontext * context,
   int *error)
 {
 context->driContext = NULL;
+__DRIconfig *driConfig = config ? config->driConfig : NULL;
 
 if (screen->dri2->base.version >= 3) {
 uint32_t ctx_attribs[4 * 2];
@@ -483,10 +484,8 @@ create_driver_context(__GLXDRIcontext * context,
 }
 
 

[PATCH xserver 1/2] glx: Fix glXQueryContext for GLX_FBCONFIG_ID and GLX_RENDER_TYPE (v2)

[Adam Jackson]

Just never filled in, oops. Seems to have gone unnoticed because
normally glXQueryContext simply returns the values filled in by the
client library when the context was created. The only path by which you
normally get to a GLXQueryContext request is glXImportContext, and then
only if the context is already indirect.

However, that's a statement about Mesa's libGL (and anything else that
inherited that bit of the SGI SI more or less intact). Nothing prevents
a mischeivous client from issuing that request of a direct context, and
if they did we'd be in trouble because we never bothered to preserve the
associated fbconfig in the context state, so we'd crash looking up
GLX_VISUAL_ID_EXT. So let's fix that too.

v2: Fixed missing preservation of the config in DRI2 (Eric Anholt)

Signed-off-by: Adam Jackson 
---
 glx/glxcmds.c| 7 ++-
 glx/glxdri2.c| 1 +
 glx/glxdriswrast.c   | 1 +
 hw/xquartz/GL/indirect.c | 2 +-
 4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index 241abc6a5..8681ef13f 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -215,6 +215,7 @@ __glXdirectContextCreate(__GLXscreen * screen,
 if (context == NULL)
 return NULL;
 
+context->config = modes;
 context->destroy = __glXdirectContextDestroy;
 context->loseCurrent = __glXdirectContextLoseCurrent;
 
@@ -1718,7 +1719,7 @@ DoQueryContext(__GLXclientState * cl, GLXContextID gcId)
 ClientPtr client = cl->client;
 __GLXcontext *ctx;
 xGLXQueryContextInfoEXTReply reply;
-int nProps = 3;
+int nProps = 5;
 int sendBuf[nProps * 2];
 int nReplyBytes;
 int err;
@@ -1740,6 +1741,10 @@ DoQueryContext(__GLXclientState * cl, GLXContextID gcId)
 sendBuf[3] = (int) (ctx->config->visualID);
 sendBuf[4] = GLX_SCREEN_EXT;
 sendBuf[5] = (int) (ctx->pGlxScreen->pScreen->myNum);
+sendBuf[6] = GLX_FBCONFIG_ID;
+sendBuf[7] = (int) (ctx->config->fbconfigID);
+sendBuf[8] = GLX_RENDER_TYPE;
+sendBuf[9] = (int) (ctx->config->renderType);
 
 if (client->swapped) {
 int length = reply.length;
diff --git a/glx/glxdri2.c b/glx/glxdri2.c
index 8516368ac..0a5463445 100644
--- a/glx/glxdri2.c
+++ b/glx/glxdri2.c
@@ -552,6 +552,7 @@ __glXDRIscreenCreateContext(__GLXscreen * baseScreen,
 return NULL;
 }
 
+context->base.config = glxConfig;
 context->base.destroy = __glXDRIcontextDestroy;
 context->base.makeCurrent = __glXDRIcontextMakeCurrent;
 context->base.loseCurrent = __glXDRIcontextLoseCurrent;
diff --git a/glx/glxdriswrast.c b/glx/glxdriswrast.c
index 97068546e..ba0e3a822 100644
--- a/glx/glxdriswrast.c
+++ b/glx/glxdriswrast.c
@@ -231,6 +231,7 @@ __glXDRIscreenCreateContext(__GLXscreen * baseScreen,
 if (context == NULL)
 return NULL;
 
+context->base.config = glxConfig;
 context->base.destroy = __glXDRIcontextDestroy;
 context->base.makeCurrent = __glXDRIcontextMakeCurrent;
 context->base.loseCurrent = __glXDRIcontextLoseCurrent;
diff --git a/hw/xquartz/GL/indirect.c b/hw/xquartz/GL/indirect.c
index 2d88ef284..6738946ff 100644
--- a/hw/xquartz/GL/indirect.c
+++ b/hw/xquartz/GL/indirect.c
@@ -156,7 +156,7 @@ __glXAquaScreenCreateContext(__GLXscreen *screen,
 memset(context, 0, sizeof *context);
 
 context->base.pGlxScreen = screen;
-
+context->base.config = conf;
 context->base.destroy = __glXAquaContextDestroy;
 context->base.makeCurrent = __glXAquaContextMakeCurrent;
 context->base.loseCurrent = __glXAquaContextLoseCurrent;
-- 
2.14.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 0/2] GLX_EXT_no_config_context v2

[Adam Jackson]

Fixed a few minor issues from last time. The spec is essentially done at
this point, would be good to get this reviewed and merged.

- ajax

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] glx: Duplicate relevant fbconfigs for compositing visuals

[Adam Jackson]

On Thu, 2017-11-02 at 15:56 +0100, Thomas Hellstrom wrote:
> Ping?
> 
> It would be good to resolve this.

remote: I: patch #179065 updated using rev 
f84e59a4f474d22860bac8aec2947798a86db69b.
remote: I: 1 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   30f4d440e..f84e59a4f  master -> master

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] modesetting: fail PreInit() if the device has zero connectors

[Giuseppe Bilotta]

On Fri, Nov 3, 2017 at 10:09 AM, Hans de Goede  wrote:
> Weird, on my XPS15 9550 where the nvidia GPU does not have/drives any
> outputs
> I do get 2 devices in xrandr --listproviders as expected. You may want to
> start
> with figuring out why the normal setup where you load nouveau normally is
> not
> working. Perhaps once that works, it will also powerdown the GPU properly.

I have an XPS15 9350 with a similar situation. You get two devices if
nouveau is loaded before Xorg starts.

If I understand Tobias' mail, his situation is with nouveau modprobed
_while X is running already_, not if it's loaded before X starts. I've
tried it in the past and I got the same segfaults that he is getting.
I have AutoAddGPU set to false specifically to avoid this segfault,
since I use a similar setup (no driver loaded, modprobe as needed),
although in my case it's more out of a need to be able to switch to
nvidia's proprietary as-needed.

In my case I have verified that without loading nouveau or nvidia the
GPU is powered up, which is supoptimal battery-wise, and I'm not sure
nvidia powers down the GPU while not in use (nouveau does, byt to me
is mostly useless, because the performance it achieves on the dGPU is
less than the one I get on Intel's iGP, and I still need the nvidia
one for CUDA), but as Tobias mentioned, that's a separate issue from
the segfault that comes from runtime nouveau modprobing.

-- 
Giuseppe "Oblomov" Bilotta
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] modesetting: fail PreInit() if the device has zero connectors

[Hans de Goede]

HI,

On 30-10-17 10:42, Tobias Jakobi wrote:

Hi again,

I took a closer look at the problem, but I can't really figure out what is going
wrong.

So, the assert that is triggered is always the following one:

Xorg: 
/var/tmp/portage/x11-base/xorg-server-1.19.5/work/xorg-server-1.19.5/dix/privates.c:385:
 dixRegisterPrivateKey: Assertion `!global_keys[type].created' failed.


This happens when glamor_init() calls dixRegisterPrivateKey() with
type=PRIVATE_PIXMAP.

I think the only place where .created is incremented is dixInitScreenPrivates(),
which is called from AllocatePixmap(). But apparantly this is never called
before glamor_init().

I then had a look at modeset, which uses dixRegisterScreenSpecificPrivateKey()
instead of dixRegisterPrivateKey(). I then proceeded to replace all calls with
type=PRIVATE_PIXMAP and type=PRIVATE_GC in glamor with this. This helps
somewhat, but now it crashes later.

Same assertion failure, but now in Xv:

Xorg: 
/var/tmp/portage/x11-base/xorg-server-1.19.5/work/xorg-server-1.19.5/dix/privates.c:385:
 dixRegisterPrivateKey: Assertion `!global_keys[type].created' failed.

Thread 1 "Xorg" received signal SIGABRT, Aborted.
0x7fada58d1178 in raise () from /lib64/libc.so.6
#0  0x7fada58d1178 in raise () from /lib64/libc.so.6
No symbol table info available.
#1  0x7fada58d25fa in abort () from /lib64/libc.so.6
No symbol table info available.
#2  0x7fada58ca0b7 in ?? () from /lib64/libc.so.6
No symbol table info available.
#3  0x7fada58ca162 in __assert_fail () from /lib64/libc.so.6
No symbol table info available.
#4  0x00461871 in dixRegisterPrivateKey (key=0x8bf7a0 
, type=PRIVATE_WINDOW, size=0)
 at 
/var/tmp/portage/x11-base/xorg-server-1.19.5/work/xorg-server-1.19.5/dix/privates.c:385
 t = PRIVATE_XSELINUX
 offset = 21715344
 bytes = 8
 __PRETTY_FUNCTION__ = "dixRegisterPrivateKey"
#5  0x004abbdd in xf86XVScreenInit (pScreen=0xf54de0, 
adaptors=0x7fffebda4eb8, num=1)
 at 
/var/tmp/portage/x11-base/xorg-server-1.19.5/work/xorg-server-1.19.5/hw/xfree86/common/xf86xv.c:239
 pScrn = 0x8
 ScreenPriv = 0xecb850
#6  0x7fad9e5a3a67 in ScreenInit (pScreen=0xf54de0, argc=0, argv=0x0)
 at 
/var/tmp/portage/x11-base/xorg-server-1.19.5/work/xorg-server-1.19.5/hw/xfree86/drivers/modesetting/driver.c:1683
 glamor_adaptor = 0x14fd760
 pScrn = 0xed3680
 ms = 0xefdd80
 visual = 0x99f4e8


I could of course continue to also replace these calls, but honestly it feels
like opening a can of worms.

My impression is that modeset does some initialization in the wrong order, which
by chance works in the non-hotplug scenario. Which led me to investigate what
happens with non-hotplug.

The funny thing is, nothing happens. modeset isn't even loaded for the device.
So something prematurely stops all the probing for /dev/dri/card1.

I'd like to investigate this further, but at the moment I have no idea what to
look for.


Weird, on my XPS15 9550 where the nvidia GPU does not have/drives any outputs
I do get 2 devices in xrandr --listproviders as expected. You may want to start
with figuring out why the normal setup where you load nouveau normally is not
working. Perhaps once that works, it will also powerdown the GPU properly.

Regards,

Hans






With best wishes,
Tobias



Hans de Goede wrote:

Hi,

On 20-10-17 19:08, tobias.jako...@uni-bielefeld.de wrote:

On laptop systems with a dedicated (powerful) GPU A, you usually
have all connectors routed to another (less-powerful) GPU B.

With my setup (GPU A = Nvidia, GPU B = Intel) I keep GPU A switched
off by not loading the nouveau kernel driver during boot.

Loading nouveau while X is running then crashes the server:
[   540.775] (EE) 0: /usr/bin/X (xorg_backtrace+0x41) [0x57fa31]
[   540.775] (EE) 1: /usr/bin/X (0x40+0x183429) [0x583429]
[   540.775] (EE) 2: /lib64/libpthread.so.0 (0x7ff02d508000+0x10ec0)
[0x7ff02d518ec0]
[   540.775] (EE) 3: /lib64/libc.so.6 (gsignal+0x38) [0x7ff02d1a2178]
[   540.775] (EE) 4: /lib64/libc.so.6 (abort+0x16a) [0x7ff02d1a35fa]
[   540.775] (EE) 5: /lib64/libc.so.6 (0x7ff02d16f000+0x2c0b7) [0x7ff02d19b0b7]
[   540.775] (EE) 6: /lib64/libc.so.6 (0x7ff02d16f000+0x2c162) [0x7ff02d19b162]
[   540.775] (EE) 7: /usr/bin/X (dixRegisterPrivateKey+0x247) [0x452197]
[   540.775] (EE) 8: /usr/lib64/xorg/modules/libglamoregl.so
(glamor_init+0x160) [0x7ff00ee564d0]
[   540.775] (EE) 9: /usr/lib64/xorg/modules/drivers/modesetting_drv.so
(0x7ff01c19a000+0x83e1) [0x7ff01c1a23e1]
[   540.775] (EE) 10: /usr/bin/X (AddGPUScreen+0xf3) [0x4348b3]
[   540.775] (EE) 11: /usr/bin/X (0x40+0x90271) [0x490271]
[   540.775] (EE) 12: /usr/bin/X (0x40+0x9547b) [0x49547b]
[   540.775] (EE) 13: /usr/bin/X (0x40+0x905d7) [0x4905d7]
[   540.775] (EE) 14: /usr/bin/X (xf86VTEnter+0x1bb) [0x47399b]
[   540.775] (EE) 15: /usr/bin/X (WakeupHandler+0xda) [0x438e3a]
[   540.775] (EE) 16: /usr/bin/X