Created attachment 315431 [details] [review]
winsys-egl-kms: bypass initial output setup if kms fd passed in

Review of attachment 315431 [details] [review]:

In general, it makes complete sense to me.

::: cogl/winsys/cogl-winsys-egl-kms.c
@@ +670,3 @@
+
+  if (kms_renderer->opened_fd < 0)
+    return TRUE;

Looking at what follows: wouldn't this break using COGL_KMS_MIRROR? I know that Mutter handles mirroring by itself, but I've recently seen that the suggestion to use that environment variable elsewhere, so we may end up breaking that.

You're probably thinking of bug 750610 comment 5 where I suggested trying it. I got that quickly from reading the source, before realizing that it probably doesn't work.  Mutter immediately overrides the crtc configuration.

(In reply to Ray Strode [halfline] from comment #3)
> You're probably thinking of bug 750610 comment 5 where I suggested trying
> it.

Likely.

> I got that quickly from reading the source, before realizing that it
> probably doesn't work.  Mutter immediately overrides the crtc configuration.

Fair. ACK from me.

Attachment 315431 [details] pushed as 1887521 - winsys-egl-kms: bypass initial output setup if kms fd passed in

This commit makes gnome-shell not start for me:

I get a glClear for a framebuffer that cogl claims is 0 by 0, and the gl call crashes mesa
(I don't have debug symbols for mesa)

Reverting I get a working shell.

Ray is out for a while. If this commit turns out to be problematic, we may have to back it out for now

(In reply to Giovanni Campagna from comment #6)
> This commit makes gnome-shell not start for me:
> 
> I get a glClear for a framebuffer that cogl claims is 0 by 0, and the gl
> call crashes mesa
> (I don't have debug symbols for mesa)
> 
> Reverting I get a working shell.

Likewise, I get a crash in mesa for both of my development systems (r600_dri.so[1] and i965_dri.so[2]). Reverting to an earlier version allows gnome-shell to start properly.


[1]: _cogl_framebuffer_gl_clear => _mesa_Clear => st_clear => st_validate_state => update_framebuffer_state => update_framebuffer_size

[2]: _cogl_framebuffer_gl_clear => _mesa_Clear => brw_clear => brw_workaround_depthstencil_alignment => get_stencil_miptree

Indeed commit 188752158701e3a406e7fd5850b3eaf9c4798cd7 does cause problems with Intel and gnome-shell. The cursor appears, and then the display server dies

I get 
Glib-GObject-CRITICAL **; g_object_unref: assertion 'G_IS_OBJECT (object) failed
Cogl-CRITICAL **: cogl_framebuffer_set_viewport: assertion 'width > 0 && height > 0' failed

 
Reverting cogl to a583492ea2aa3ea8e78c269bd5db3f52f82aa79c allows gnome-shell to run as a Wayland Display server.

Any updates on this? 188752158701e3a406e7fd5850b3eaf9c4798cd7 causes problems on Intel, however I'm not sure if it's just on Intel

Any other diagnostic info I can provide?

Seems to cause same issue with Nouveau on NVC3. Gentoo includes this patch already and gnome-shell fails to start. I removed the patch in Gentoo and rebuild cogl and gnome-shell works again.

I tried again today and it's still crashing in the same way. Patch needs reverting.

Full backtrace:

+ Trace 235980

hey sorry for sitting on this.  i was on paternity leave and then it fell off my radar when I got back, will push fix

Created attachment 321882 [details] [review]
winsys-egl-kms: dont create 0x0 surface

commit 188752158 changed cogl to stop needlessly creating its own
monitor output configuration when mutter would just soon overwrite
it anyway.

Unfortunately, that commit is causing a crash in some cases because
cogl will now create and later draw to a 0x0 egl surface until mutter
sets the monitor layout.

This commit changes cogl to avoid creating and using a surface, before
it knows how big of a surface to create.

(i'm pushing this now since I don't expect to get any review comments, but if anyone has any please chime in!)

Attachment 321882 [details] pushed as 1c1ad69 - winsys-egl-kms: dont create 0x0 surface