<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Mar 12, 2014 at 11:55 AM, Neil Roberts <span dir="ltr"><<a href="mailto:neil@linux.intel.com" target="_blank">neil@linux.intel.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Jasper,<br>
<br>
Thanks for the patches on the wip/wayland-for-demo branch. I've pushed<br>
the first patch to the cogl-1.18 and master branches.<br>
<br>
<a href="https://git.gnome.org/browse/cogl/commit/?id=af9057d35f331e2c9509958f" target="_blank">https://git.gnome.org/browse/cogl/commit/?id=af9057d35f331e2c9509958f</a><br>
<br>
I have a couple of minor comments about the second patch:<br>
<br>
> diff --git a/cogl/winsys/cogl-winsys-egl-kms.c b/cogl/winsys/cogl-winsys-egl-kms.c<br>
> index 4af9848..41954ef 100644<br>
> --- a/cogl/winsys/cogl-winsys-egl-kms.c<br>
> +++ b/cogl/winsys/cogl-winsys-egl-kms.c<br>
> @@ -285,14 +285,21 @@ _cogl_winsys_renderer_connect (CoglRenderer *renderer,<br>
>     * we're doing here... */<br>
>    g_setenv ("EGL_PLATFORM", "drm", 1);<br>
><br>
> -  kms_renderer->fd = open (device_name, O_RDWR);<br>
> -  if (kms_renderer->fd < 0)<br>
> +  if (renderer->kms_fd > 0)<br>
<br>
Shouldn't this be '>= 0'? I could imagine something potentially using<br>
fd 0 if it has closed its stdin.<br>
<br>
It would be nice if you could make it avoid closing the user's fd if it<br>
hits the close_fd label. I was going to say that it should also avoid<br>
closing the fd when the renderer is destroyed, but it looks that is<br>
missing anyway even if Cogl opens the device so we can leave that to a<br>
separate patch.<br></blockquote><div><br></div><div>Good catches.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Have you been able to test the last patch? When it returns from<br>
flip_all_crtcs it looks like flip->pending would still be zero which<br>
would make it immediately proceed to delete the framebuffer that it just<br>
set to scan out from. Won't that break?<br></blockquote><div><br></div><div>I haven't ever tested it, as the only driver that doesn't have a native flip is simpledrm, which is only used in pathological cases in virt. We can't unconditionally bump flip->pending, since then we'll be stuck waiting for a page flip event that will never come. I'll look into extracting the "flip" logic into something we can use in both paths.<br>
<br>Anyway, the patch isn't too important, the really important one is the ability to set a native KMS FD. So let's defer on this for now.<br><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

I think we would also need to make it queue the swap_notify_idle because<br>
Cogl is meant to have the invariant that whenever you call<br>
cogl_onscreen_swap_buffers you will always get a swap notification.<br></blockquote><div><br></div><div>Yeah.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

Regards,<br>
- Neil<br>
</blockquote></div><br><br clear="all"><br>-- <br>  Jasper<br>
</div></div>