Re: [PATCH xserver 6/8] xwin/glx: Assume WGL_ARB_make_current_read exists
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
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
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)
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
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