Friendly ping - does this patch look good now?
On Thu, Apr 7, 2016 at 3:48 PM, Alexandre Courbot <acour...@nvidia.com> wrote: > 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 again. In the > meantime, the cursor will be invisible to the user. > > This patch addresses that by adding _xf86CrtcFuncs::set_cursor_check and > _xf86CursorInfoRec::ShowCursorCheck hook variants that return booleans. > This allows to propagate errors up to xf86CursorSetCursor(), which can > then fall back to using the SW cursor immediately. > > Signed-off-by: Alexandre Courbot <acour...@nvidia.com> > Cc: Michael Thayer <michael.tha...@oracle.com> > --- > Changes since v2: > - Fixed a bug in xf86_crtc_show_cursor raised by Michael > > Changes since v1: > - Keep the old hooks unchanged to preserve compatibility > > hw/xfree86/drivers/modesetting/drmmode_display.c | 15 ++++++---- > hw/xfree86/modes/xf86Crtc.h | 4 +++ > hw/xfree86/modes/xf86Cursors.c | 35 > ++++++++++++++++++------ > hw/xfree86/ramdac/xf86Cursor.h | 1 + > hw/xfree86/ramdac/xf86HWCurs.c | 18 +++++++++--- > 5 files changed, 54 insertions(+), 19 deletions(-) > > diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c > b/hw/xfree86/drivers/modesetting/drmmode_display.c > index f573a27f4985..28d663c3d22e 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 > -drmmode_show_cursor(xf86CrtcPtr crtc) > +static Bool > +drmmode_show_cursor_check(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 > @@ -844,7 +847,7 @@ static const xf86CrtcFuncsRec drmmode_crtc_funcs = { > .set_mode_major = drmmode_set_mode_major, > .set_cursor_colors = drmmode_set_cursor_colors, > .set_cursor_position = drmmode_set_cursor_position, > - .show_cursor = drmmode_show_cursor, > + .show_cursor_check = drmmode_show_cursor_check, > .hide_cursor = drmmode_hide_cursor, > .load_cursor_argb = drmmode_load_cursor_argb, > > diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h > index 8b0160845a02..4bc4d6fc78fd 100644 > --- a/hw/xfree86/modes/xf86Crtc.h > +++ b/hw/xfree86/modes/xf86Crtc.h > @@ -187,6 +187,8 @@ typedef struct _xf86CrtcFuncs { > */ > void > (*show_cursor) (xf86CrtcPtr crtc); > + Bool > + (*show_cursor_check) (xf86CrtcPtr crtc); > > /** > * Hide cursor > @@ -982,6 +984,8 @@ extern _X_EXPORT void > */ > extern _X_EXPORT void > xf86_show_cursors(ScrnInfoPtr scrn); > +extern _X_EXPORT Bool > + xf86_show_cursors_check(ScrnInfoPtr scrn); > > /** > * Called by the driver to turn cursors off > diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c > index 5df1ab73a37e..2ba150ce6441 100644 > --- a/hw/xfree86/modes/xf86Cursors.c > +++ b/hw/xfree86/modes/xf86Cursors.c > @@ -333,17 +333,26 @@ 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_in_range) > + return TRUE; > + > + if (!crtc->cursor_shown) { > + if (crtc->funcs->show_cursor_check) { > + crtc->cursor_shown = crtc->funcs->show_cursor_check(crtc); > + } else { > + crtc->funcs->show_cursor(crtc); > + crtc->cursor_shown = TRUE; > + } > } > + > + return crtc->cursor_shown; > } > > -void > -xf86_show_cursors(ScrnInfoPtr scrn) > +Bool > +xf86_show_cursors_check(ScrnInfoPtr scrn) > { > xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn); > int c; > @@ -352,9 +361,17 @@ 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 > +xf86_show_cursors(ScrnInfoPtr scrn) > +{ > + xf86_show_cursors_check(scrn); > } > > void > @@ -631,7 +648,7 @@ xf86_cursors_init(ScreenPtr screen, int max_width, int > max_height, int flags) > cursor_info->SetCursorPosition = xf86_set_cursor_position; > cursor_info->LoadCursorImageCheck = xf86_load_cursor_image; > cursor_info->HideCursor = xf86_hide_cursors; > - cursor_info->ShowCursor = xf86_show_cursors; > + cursor_info->ShowCursorCheck = xf86_show_cursors_check; > cursor_info->UseHWCursor = xf86_use_hw_cursor; > if (flags & HARDWARE_CURSOR_ARGB) { > cursor_info->UseHWCursorARGB = xf86_use_hw_cursor_argb; > diff --git a/hw/xfree86/ramdac/xf86Cursor.h b/hw/xfree86/ramdac/xf86Cursor.h > index 6e88240d9ab7..a1afb1c86ed9 100644 > --- a/hw/xfree86/ramdac/xf86Cursor.h > +++ b/hw/xfree86/ramdac/xf86Cursor.h > @@ -16,6 +16,7 @@ typedef struct _xf86CursorInfoRec { > Bool (*LoadCursorImageCheck) (ScrnInfoPtr pScrn, unsigned char *bits); > void (*HideCursor) (ScrnInfoPtr pScrn); > void (*ShowCursor) (ScrnInfoPtr pScrn); > + Bool (*ShowCursorCheck) (ScrnInfoPtr pScrn); > unsigned char *(*RealizeCursor) (struct _xf86CursorInfoRec *, CursorPtr); > Bool (*UseHWCursor) (ScreenPtr, CursorPtr); > > diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c > index 84febe0df5cd..458781cae7e9 100644 > --- a/hw/xfree86/ramdac/xf86HWCurs.c > +++ b/hw/xfree86/ramdac/xf86HWCurs.c > @@ -89,7 +89,8 @@ xf86InitHardwareCursor(ScreenPtr pScreen, xf86CursorInfoPtr > infoPtr) > if (!infoPtr->SetCursorPosition || > !xf86DriverHasLoadCursorImage(infoPtr) || > !infoPtr->HideCursor || > - !infoPtr->ShowCursor || !infoPtr->SetCursorColors) > + (!infoPtr->ShowCursor && !infoPtr->ShowCursorCheck) || > + !infoPtr->SetCursorColors) > return FALSE; > > if (infoPtr->RealizeCursor) { > @@ -119,6 +120,15 @@ xf86InitHardwareCursor(ScreenPtr pScreen, > xf86CursorInfoPtr infoPtr) > return TRUE; > } > > +static Bool > +xf86ShowCursor(xf86CursorInfoPtr infoPtr) > +{ > + if (*infoPtr->ShowCursorCheck) > + return (*infoPtr->ShowCursorCheck) (infoPtr->pScrn); > + (*infoPtr->ShowCursor) (infoPtr->pScrn); > + return TRUE; > +} > + > Bool > xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y) > { > @@ -161,8 +171,8 @@ xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, > int y) > > (*infoPtr->SetCursorPosition) (infoPtr->pScrn, x, y); > > - (*infoPtr->ShowCursor) (infoPtr->pScrn); > - return TRUE; > + return xf86ShowCursor(infoPtr); > + > } > > void > @@ -184,7 +194,7 @@ xf86SetTransparentCursor(ScreenPtr pScreen) > xf86DriverLoadCursorImage (infoPtr, > ScreenPriv->transparentData); > > - (*infoPtr->ShowCursor) (infoPtr->pScrn); > + xf86ShowCursor(infoPtr); > } > > void > -- > 2.8.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