Re: [PATCH xserver 6/8] xwin/glx: Assume WGL_ARB_make_current_read exists

2016-03-22 Thread Jon Turney

On 21/03/2016 20:29, Adam Jackson wrote:

This seems to be fairly universal these days, and if it doesn't exist
the only thing you break is separate drawable and readable, which is a
fairly rare feature to use. So pretend it's always there and just throw
an error on MakeCurrent if it doesn't.

As a result we can enable GLX 1.4 unconditionally.

Signed-off-by: Adam Jackson 
---
  hw/xwin/glx/indirect.c | 36 
  1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/hw/xwin/glx/indirect.c b/hw/xwin/glx/indirect.c



@@ -1408,6 +1396,7 @@ static int
  glxWinContextMakeCurrent(__GLXcontext * base)
  {
  __GLXWinContext *gc = (__GLXWinContext *) base;
+__GLXWinScreen *scr = (__GLXWinScreen *) base->pGlxScreen;


Doesn't compile because this type doesn't follow the naming pattern, so 
I think this line should be:


glxWinScreen *scr = (glxWinScreen *) base->pGlxScreen



  BOOL ret;
  HDC drawDC;
  HDC readDC = NULL;
@@ -1440,7 +1429,14 @@ glxWinContextMakeCurrent(__GLXcontext * base)
  }

  if ((gc->base.readPriv != NULL) && (gc->base.readPriv != 
gc->base.drawPriv)) {
-// XXX: should only occur with WGL_ARB_make_current_read
+/*
+ * We enable this unconditionally, but the renderer _might_ not support
+ * it. It's fairly rare to use this feature so just error out if it
+ * can't work.
+ */
+if (!scr->has_WGL_ARB_make_current_read)
+return False;
+
  /*
 If there is a separate read drawable, create a separate read DC, 
and
 use the wglMakeContextCurrent extension to make the context 
current drawing



___
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 xserver] Switch to SW cursor right after HW cursor failure

2016-03-22 Thread Alexandre Courbot

On 03/18/2016 07:31 PM, Michael Thayer wrote:

Hello Alex,

On 18.03.2016 10:42, Alexandre Courbot wrote:

Hello Michael,

On 03/18/2016 06:09 PM, Michael Thayer wrote:

In that context, see this patch to Modesetting which I send a couple of
weeks ago, which should incidentally also fix your issue:

https://patchwork.freedesktop.org/patch/75985/


I applied your patch, but sadly it didn't fix my issue. I agree that
your approach (actually returning drmmode_set_cursor() errors instead of
relying on a side-effect) is better though. But it appears that I would
need drmmode_show_cursor()'s errors to be reported for the same approach
to work in my case.

I see that the load_cursor_argb_check() hook has been added apparently
to palliate the fact that load_cursor_argb() does not return an error
status, would it be acceptable to apply the same principle to
show_cursor()?


It certainly makes sense to me (noting that the *_check() hooks were
Keith's idea).  Not sure off-hand how much you would have to change in
the main X server code to make that work.


Not too much as it turns out ; actually since this hook is used in only 
one place, we can safely change its signature instead of adding a 
variant. I have just sent the patch that does this.


Thanks for your feedback!
Alex.

___
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] xfree86: Immediately handle failure to set HW cursor

2016-03-22 Thread Alexandre Courbot
There is currently no reliable way to report failure to set a HW
cursor. Still such failures can happen if e.g. the MODE_CURSOR DRM
ioctl fails (which currently happens at least with modesetting on Tegra
for format incompatibility reasons).

As failures are currently handled by setting the HW cursor size to
(0,0), the fallback to SW cursor will not happen until the next time the
cursor changes and xf86CursorSetCursor() is called. In the meantime, the
cursor will be invisible to the user.

This patch addresses this by making the _xf86CrtcFuncs::set_cursor and
_xf86CursorInfoRec::ShowCursor hooks return a boolean. This allows to
propagate errors up to xf86CursorSetCursor(), which makes it fall back
to using the SW cursor immediately.

Signed-off-by: Alexandre Courbot 
---
 hw/xfree86/drivers/modesetting/drmmode_display.c | 11 +++
 hw/xfree86/modes/xf86Crtc.h  |  4 ++--
 hw/xfree86/modes/xf86Cursors.c   | 18 ++
 hw/xfree86/ramdac/IBM.c  |  8 ++--
 hw/xfree86/ramdac/TI.c   |  4 +++-
 hw/xfree86/ramdac/xf86Cursor.h   |  2 +-
 hw/xfree86/ramdac/xf86HWCurs.c   |  3 +--
 7 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index f573a27f4985..1ff9ed477624 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -532,7 +532,7 @@ drmmode_set_cursor_position(xf86CrtcPtr crtc, int x, int y)
 drmModeMoveCursor(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id, x, y);
 }
 
-static void
+static Bool
 drmmode_set_cursor(xf86CrtcPtr crtc)
 {
 drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
@@ -551,7 +551,7 @@ drmmode_set_cursor(xf86CrtcPtr crtc)
   handle, ms->cursor_width, ms->cursor_height,
   cursor->bits->xhot, cursor->bits->yhot);
 if (!ret)
-return;
+return TRUE;
 if (ret == -EINVAL)
 use_set_cursor2 = FALSE;
 }
@@ -566,7 +566,10 @@ drmmode_set_cursor(xf86CrtcPtr crtc)
 cursor_info->MaxWidth = cursor_info->MaxHeight = 0;
 drmmode_crtc->drmmode->sw_cursor = TRUE;
 /* fallback to swcursor */
+   return FALSE;
 }
+
+return TRUE;
 }
 
 static void
@@ -599,12 +602,12 @@ drmmode_hide_cursor(xf86CrtcPtr crtc)
  ms->cursor_width, ms->cursor_height);
 }
 
-static void
+static Bool
 drmmode_show_cursor(xf86CrtcPtr crtc)
 {
 drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
 drmmode_crtc->cursor_up = TRUE;
-drmmode_set_cursor(crtc);
+return drmmode_set_cursor(crtc);
 }
 
 static void
diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
index 8b0160845a02..a81110c2b5c5 100644
--- a/hw/xfree86/modes/xf86Crtc.h
+++ b/hw/xfree86/modes/xf86Crtc.h
@@ -185,7 +185,7 @@ typedef struct _xf86CrtcFuncs {
 /**
  * Show cursor
  */
-void
+Bool
  (*show_cursor) (xf86CrtcPtr crtc);
 
 /**
@@ -980,7 +980,7 @@ extern _X_EXPORT void
 /**
  * Called from EnterVT to turn the cursors back on
  */
-extern _X_EXPORT void
+extern _X_EXPORT Bool
  xf86_show_cursors(ScrnInfoPtr scrn);
 
 /**
diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c
index 5df1ab73a37e..e9ae713e7bba 100644
--- a/hw/xfree86/modes/xf86Cursors.c
+++ b/hw/xfree86/modes/xf86Cursors.c
@@ -333,16 +333,16 @@ xf86_hide_cursors(ScrnInfoPtr scrn)
 }
 }
 
-static void
+static Bool
 xf86_crtc_show_cursor(xf86CrtcPtr crtc)
 {
-if (!crtc->cursor_shown && crtc->cursor_in_range) {
-crtc->funcs->show_cursor(crtc);
-crtc->cursor_shown = TRUE;
-}
+if (!crtc->cursor_shown && crtc->cursor_in_range)
+crtc->cursor_shown = crtc->funcs->show_cursor(crtc);
+
+return crtc->cursor_shown;
 }
 
-void
+Bool
 xf86_show_cursors(ScrnInfoPtr scrn)
 {
 xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
@@ -352,9 +352,11 @@ xf86_show_cursors(ScrnInfoPtr scrn)
 for (c = 0; c < xf86_config->num_crtc; c++) {
 xf86CrtcPtr crtc = xf86_config->crtc[c];
 
-if (crtc->enabled)
-xf86_crtc_show_cursor(crtc);
+if (crtc->enabled && !xf86_crtc_show_cursor(crtc))
+return FALSE;
 }
+
+return TRUE;
 }
 
 void
diff --git a/hw/xfree86/ramdac/IBM.c b/hw/xfree86/ramdac/IBM.c
index 6822be5f1dad..23fb55411b1e 100644
--- a/hw/xfree86/ramdac/IBM.c
+++ b/hw/xfree86/ramdac/IBM.c
@@ -468,16 +468,18 @@ IBMramdac640SetBpp(ScrnInfoPtr pScrn, RamDacRegRecPtr 
ramdacReg)
 }
 }
 
-static void
+static Bool
 IBMramdac526ShowCursor(ScrnInfoPtr pScrn)
 {
 RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
 
 /* Enable cursor - X11 mode */
 (*ramdacPtr->WriteDAC) (pScrn, IBMRGB_curs, 0x00, 0x07);
+
+return TRUE;
 }
 
-static vo

Re: [PATCH xserver 5/8] xquartz/glx: Error out for MakeContextCurrent(draw != read)

2016-03-22 Thread Jeremy Huddleston Sequoia
With the change to !=:
Reviewed-by: Jeremy Huddleston Sequoia 

> On Mar 21, 2016, at 13:29, Adam Jackson  wrote:
> 
> CGL doesn't have a way to express this directly, unlike EGL WGL and GLX.
> It might be implementable, but it's never actually worked, and it's a
> fairly niche feature so we're better off throwing an error if someone
> attempts it.
> 
> Signed-off-by: Adam Jackson 
> ---
> hw/xquartz/GL/indirect.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/hw/xquartz/GL/indirect.c b/hw/xquartz/GL/indirect.c
> index 54c9073..c0648c4 100644
> --- a/hw/xquartz/GL/indirect.c
> +++ b/hw/xquartz/GL/indirect.c
> @@ -387,6 +387,9 @@ __glXAquaContextMakeCurrent(__GLXcontext *baseContext)
> 
> GLAQUA_DEBUG_MSG("glAquaMakeCurrent (ctx 0x%p)\n", baseContext);
> 
> +if (context->base.drawPriv !+ context->base.readPriv)
> +return 0;
> +
> if (attach(context, drawPriv))
> return /*error*/ 0;
> 
> -- 
> 2.5.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



smime.p7s
Description: S/MIME cryptographic signature
___
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 xserver 3/8] {xwin, xquartz}/glx: Always enable GLX_{ARB, SGIS}_multisample

2016-03-22 Thread Jeremy Huddleston Sequoia
The xquartz -s look good given the change to core.

Reviewed-by: Jeremy Huddleston Sequoia 

> On Mar 21, 2016, at 13:29, Adam Jackson  wrote:
> 
> This is enabled unconditionally in the GLX core. For xwin, if the
> backend doesn't support WGL_ARB_multisample, there will simply be no
> fbconfigs that support it.
> 
> Signed-off-by: Adam Jackson 
> ---
> hw/xquartz/GL/indirect.c |  3 ---
> hw/xwin/glx/indirect.c   | 24 ++--
> 2 files changed, 6 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/xquartz/GL/indirect.c b/hw/xquartz/GL/indirect.c
> index 4f3e2e4..54c9073 100644
> --- a/hw/xquartz/GL/indirect.c
> +++ b/hw/xquartz/GL/indirect.c
> @@ -555,9 +555,6 @@ __glXAquaScreenProbe(ScreenPtr pScreen)
> __glXEnableExtension(screen->glx_enable_bits, "GLX_OML_swap_method");
> __glXEnableExtension(screen->glx_enable_bits, "GLX_SGIX_fbconfig");
> 
> -__glXEnableExtension(screen->glx_enable_bits, "GLX_SGIS_multisample");
> -__glXEnableExtension(screen->glx_enable_bits, "GLX_ARB_multisample");
> -
> //__glXEnableExtension(screen->glx_enable_bits, "GLX_ARB_create_context");
> //__glXEnableExtension(screen->glx_enable_bits, 
> "GLX_ARB_create_context_profile");
> 
> diff --git a/hw/xwin/glx/indirect.c b/hw/xwin/glx/indirect.c
> index a01757b..b4b773f 100644
> --- a/hw/xwin/glx/indirect.c
> +++ b/hw/xwin/glx/indirect.c
> @@ -641,6 +641,10 @@ glxWinScreenProbe(ScreenPtr pScreen)
> __glXEnableExtension(screen->glx_enable_bits, "GLX_OML_swap_method");
> __glXEnableExtension(screen->glx_enable_bits, "GLX_SGIX_fbconfig");
> 
> +// GLX_ARB_multisample is always enabled, even if no configs support 
> it
> +if (strstr(wgl_extensions, "WGL_ARB_multisample"))
> +screen->has_WGL_ARB_multisample = TRUE;
> +
> if (strstr(wgl_extensions, "WGL_ARB_make_current_read")) {
> __glXEnableExtension(screen->glx_enable_bits,
>  "GLX_SGI_make_current_read");
> @@ -677,16 +681,6 @@ glxWinScreenProbe(ScreenPtr pScreen)
> screen->has_WGL_ARB_pbuffer = TRUE;
> }
> 
> -if (strstr(wgl_extensions, "WGL_ARB_multisample")) {
> -__glXEnableExtension(screen->glx_enable_bits,
> - "GLX_ARB_multisample");
> -__glXEnableExtension(screen->glx_enable_bits,
> - "GLX_SGIS_multisample");
> -LogMessage(X_INFO,
> -   "AIGLX: enabled GLX_ARB_multisample and 
> GLX_SGIS_multisample\n");
> -screen->has_WGL_ARB_multisample = TRUE;
> -}
> -
> screen->base.destroy = glxWinScreenDestroy;
> screen->base.createContext = glxWinCreateContext;
> screen->base.createDrawable = glxWinCreateDrawable;
> @@ -747,14 +741,8 @@ glxWinScreenProbe(ScreenPtr pScreen)
> // ARB_multisample -> 1.4
> //
> if (screen->has_WGL_ARB_pbuffer && glx_sgi_make_current_read) {
> -if (screen->has_WGL_ARB_multisample) {
> -screen->base.GLXmajor = 1;
> -screen->base.GLXminor = 4;
> -}
> -else {
> -screen->base.GLXmajor = 1;
> -screen->base.GLXminor = 3;
> -}
> +screen->base.GLXmajor = 1;
> +screen->base.GLXminor = 4;
> }
> }
> LogMessage(X_INFO, "AIGLX: Set GLX version to %d.%d\n",
> -- 
> 2.5.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



smime.p7s
Description: S/MIME cryptographic signature
___
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