On 18/03/14 11:17, Michael Thayer wrote:
On 05/03/14 19:45, Adam Jackson wrote:
On Thu, 2014-02-27 at 15:46 +0100, Michael Thayer wrote:
Another would be
to add a return value to the DDX CRTC functions "load_cursor_argb", so
that if the KMS driver failed to set the cursor, "modesetting" could
pass this on to the X server.
Actually we really should make load_cursor_argb return something other
than void. Some particularly incapable server GPUs have format
restrictions on their hardware cursors that mean _some_ ARGB cursors
could be made to work but not all; and right now, the fallback code in
-modesetting doesn't get this case right, and you end up with no cursor
at all.
I have done an initial patch to do this, see below. If I get positive
feedback I will resend it formatted to be applied to the tree, but I
expect a couple of iterations.
I have been looking at what this would take, and what rather puts me off
is that it would mean patching all supported video drivers without even
the means to test them (the patches will be mostly be adding "return
TRUE" in the right place of course, but you know how it is; how is this
sort of thing normally handled?)
This question is still relevant.
[...]
Even nicer would be a way in addition of telling the server that a
particular hardware cursor is linked to a particular input device, and
that it should use a software cursor if it wants the cursor to be
somewhere other than the position reported by the device; I can't
immediately think though where that would fit in cleanly, and neither
can I immediately think of any other potential users of such an interface.
This could be achieved by making SetCursorPosition() return a success
value too. Would this be an acceptable idea?
Regards,
Michael
---
diff --git a/hw/xfree86/common/xf86Module.h b/hw/xfree86/common/xf86Module.h
index e8c24f2..62ac95d 100644
--- a/hw/xfree86/common/xf86Module.h
+++ b/hw/xfree86/common/xf86Module.h
@@ -80,7 +80,7 @@ typedef enum {
* mask is 0xFFFF0000.
*/
#define ABI_ANSIC_VERSION SET_ABI_VERSION(0, 4)
-#define ABI_VIDEODRV_VERSION SET_ABI_VERSION(16, 0)
+#define ABI_VIDEODRV_VERSION SET_ABI_VERSION(17, 0)
#define ABI_XINPUT_VERSION SET_ABI_VERSION(21, 0)
#define ABI_EXTENSION_VERSION SET_ABI_VERSION(8, 0)
#define ABI_FONT_VERSION SET_ABI_VERSION(0, 6)
diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
index c127d78..5407deb 100644
--- a/hw/xfree86/modes/xf86Crtc.h
+++ b/hw/xfree86/modes/xf86Crtc.h
@@ -186,13 +186,13 @@ typedef struct _xf86CrtcFuncs {
/**
* Load monochrome image
*/
- void
+ Bool
(*load_cursor_image) (xf86CrtcPtr crtc, CARD8 *image);
/**
* Load ARGB image
*/
- void
+ Bool
(*load_cursor_argb) (xf86CrtcPtr crtc, CARD32 *image);
/**
diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c
index 2b0db34..10ef6f6 100644
--- a/hw/xfree86/modes/xf86Cursors.c
+++ b/hw/xfree86/modes/xf86Cursors.c
@@ -211,7 +211,7 @@ set_bit(CARD8 *image, xf86CursorInfoPtr cursor_info,
int x, int y, Bool mask)
/*
* Load a two color cursor into a driver that supports only ARGB cursors
*/
-static void
+static Bool
xf86_crtc_convert_cursor_to_argb(xf86CrtcPtr crtc, unsigned char *src)
{
ScrnInfoPtr scrn = crtc->scrn;
@@ -244,7 +244,7 @@ xf86_crtc_convert_cursor_to_argb(xf86CrtcPtr crtc,
unsigned char *src)
bits = 0;
cursor_image[y * cursor_info->MaxWidth + x] = bits;
}
- crtc->funcs->load_cursor_argb(crtc, cursor_image);
+ return crtc->funcs->load_cursor_argb(crtc, cursor_image);
}
/*
@@ -415,7 +415,7 @@ xf86_set_cursor_position(ScrnInfoPtr scrn, int x, int y)
/*
* Load a two-color cursor into a crtc, performing rotation as needed
*/
-static void
+static Bool
xf86_crtc_load_cursor_image(xf86CrtcPtr crtc, CARD8 *src)
{
ScrnInfoPtr scrn = crtc->scrn;
@@ -450,13 +450,13 @@ xf86_crtc_load_cursor_image(xf86CrtcPtr crtc,
CARD8 *src)
set_bit(cursor_image, cursor_info, x, y, TRUE);
}
}
- crtc->funcs->load_cursor_image(crtc, cursor_image);
+ return crtc->funcs->load_cursor_image(crtc, cursor_image);
}
/*
* Load a cursor image into all active CRTCs
*/
-static void
+static Bool
xf86_load_cursor_image(ScrnInfoPtr scrn, unsigned char *src)
{
xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
@@ -466,12 +466,17 @@ xf86_load_cursor_image(ScrnInfoPtr scrn, unsigned
char *src)
xf86CrtcPtr crtc = xf86_config->crtc[c];
if (crtc->enabled) {
- if (crtc->funcs->load_cursor_image)
- xf86_crtc_load_cursor_image(crtc, src);
- else if (crtc->funcs->load_cursor_argb)
- xf86_crtc_convert_cursor_to_argb(crtc, src);
+ if (crtc->funcs->load_cursor_image) {
+ if (!xf86_crtc_load_cursor_image(crtc, src))
+ return FALSE;
+ } else if (crtc->funcs->load_cursor_argb) {
+ if (!xf86_crtc_convert_cursor_to_argb(crtc, src))
+ return FALSE;
+ } else
+ return FALSE;
}
}
+ return TRUE;
}
static Bool
@@ -516,7 +521,7 @@ xf86_use_hw_cursor_argb(ScreenPtr screen, CursorPtr
cursor)
return TRUE;
}
-static void
+static Bool
xf86_crtc_load_cursor_argb(xf86CrtcPtr crtc, CursorPtr cursor)
{
ScrnInfoPtr scrn = crtc->scrn;
@@ -544,10 +549,10 @@ xf86_crtc_load_cursor_argb(xf86CrtcPtr crtc,
CursorPtr cursor)
cursor_image[y * image_width + x] = bits;
}
- crtc->funcs->load_cursor_argb(crtc, cursor_image);
+ return crtc->funcs->load_cursor_argb(crtc, cursor_image);
}
-static void
+static Bool
xf86_load_cursor_argb(ScrnInfoPtr scrn, CursorPtr cursor)
{
xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
@@ -557,8 +562,10 @@ xf86_load_cursor_argb(ScrnInfoPtr scrn, CursorPtr
cursor)
xf86CrtcPtr crtc = xf86_config->crtc[c];
if (crtc->enabled)
- xf86_crtc_load_cursor_argb(crtc, cursor);
+ if (!xf86_crtc_load_cursor_argb(crtc, cursor))
+ return FALSE;
}
+ return TRUE;
}
Bool
@@ -608,6 +615,8 @@ xf86_cursors_init(ScreenPtr screen, int max_width,
int max_height, int flags)
* Called when anything on the screen is reconfigured.
*
* Reloads cursor images as needed, then adjusts cursor positions
+ * @note We assume that all hardware cursors to be loaded have already been
+ * found to be usable by the hardware.
*/
void
diff --git a/hw/xfree86/ramdac/IBM.c b/hw/xfree86/ramdac/IBM.c
index bc71623..872d3d4 100644
--- a/hw/xfree86/ramdac/IBM.c
+++ b/hw/xfree86/ramdac/IBM.c
@@ -570,7 +570,7 @@ IBMramdac640SetCursorColors(ScrnInfoPtr pScrn, int
bg, int fg)
(*ramdacPtr->WriteData) (pScrn, bg);
}
-static void
+static Bool
IBMramdac526LoadCursorImage(ScrnInfoPtr pScrn, unsigned char *src)
{
RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
@@ -582,9 +582,10 @@ IBMramdac526LoadCursorImage(ScrnInfoPtr pScrn,
unsigned char *src)
*/
for (i = 0; i < 1024; i++)
(*ramdacPtr->WriteDAC) (pScrn, IBMRGB_curs_array + i, 0x00,
(*src++));
+ return TRUE;
}
-static void
+static Bool
IBMramdac640LoadCursorImage(ScrnInfoPtr pScrn, unsigned char *src)
{
RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
@@ -596,6 +597,7 @@ IBMramdac640LoadCursorImage(ScrnInfoPtr pScrn,
unsigned char *src)
*/
for (i = 0; i < 1024; i++)
(*ramdacPtr->WriteDAC) (pScrn, RGB640_CURS_WRITE + i, 0x00,
(*src++));
+ return TRUE;
}
static Bool
diff --git a/hw/xfree86/ramdac/TI.c b/hw/xfree86/ramdac/TI.c
index 393b774..7d4e0d7 100644
--- a/hw/xfree86/ramdac/TI.c
+++ b/hw/xfree86/ramdac/TI.c
@@ -642,7 +642,7 @@ TIramdacSetCursorColors(ScrnInfoPtr pScrn, int bg,
int fg)
(*ramdacPtr->WriteDAC) (pScrn, TIDAC_CURS_COLOR, 0, (fg &
0x000000ff));
}
-static void
+static Bool
TIramdacLoadCursorImage(ScrnInfoPtr pScrn, unsigned char *src)
{
RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
@@ -657,6 +657,7 @@ TIramdacLoadCursorImage(ScrnInfoPtr pScrn, unsigned
char *src)
/* NOT_DONE: might need a delay here */
(*ramdacPtr->WriteDAC) (pScrn, TIDAC_CURS_RAM_DATA, 0, *(src++));
}
+ return TRUE;
}
static Bool
diff --git a/hw/xfree86/ramdac/xf86Cursor.c b/hw/xfree86/ramdac/xf86Cursor.c
index 860704e..fac6822 100644
--- a/hw/xfree86/ramdac/xf86Cursor.c
+++ b/hw/xfree86/ramdac/xf86Cursor.c
@@ -352,12 +352,13 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr
pScreen, CursorPtr pCurs,
(*ScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen,
NullCursor, x, y);
- xf86SetCursor(pScreen, cursor, x, y);
- ScreenPriv->SWCursor = FALSE;
- ScreenPriv->isUp = TRUE;
+ if (xf86SetCursor(pScreen, cursor, x, y)) {
+ ScreenPriv->SWCursor = FALSE;
+ ScreenPriv->isUp = TRUE;
- miPointerSetWaitForUpdate(pScreen,
!infoPtr->pScrn->silkenMouse);
- return;
+ miPointerSetWaitForUpdate(pScreen,
!infoPtr->pScrn->silkenMouse);
+ return;
+ }
}
miPointerSetWaitForUpdate(pScreen, TRUE);
diff --git a/hw/xfree86/ramdac/xf86Cursor.h b/hw/xfree86/ramdac/xf86Cursor.h
index 5658e7b..1ecbdcd 100644
--- a/hw/xfree86/ramdac/xf86Cursor.h
+++ b/hw/xfree86/ramdac/xf86Cursor.h
@@ -12,7 +12,7 @@ typedef struct _xf86CursorInfoRec {
int MaxHeight;
void (*SetCursorColors) (ScrnInfoPtr pScrn, int bg, int fg);
void (*SetCursorPosition) (ScrnInfoPtr pScrn, int x, int y);
- void (*LoadCursorImage) (ScrnInfoPtr pScrn, unsigned char *bits);
+ Bool (*LoadCursorImage) (ScrnInfoPtr pScrn, unsigned char *bits);
void (*HideCursor) (ScrnInfoPtr pScrn);
void (*ShowCursor) (ScrnInfoPtr pScrn);
unsigned char *(*RealizeCursor) (struct _xf86CursorInfoRec *,
CursorPtr);
@@ -20,7 +20,7 @@ typedef struct _xf86CursorInfoRec {
#ifdef ARGB_CURSOR
Bool (*UseHWCursorARGB) (ScreenPtr, CursorPtr);
- void (*LoadCursorARGB) (ScrnInfoPtr, CursorPtr);
+ Bool (*LoadCursorARGB) (ScrnInfoPtr, CursorPtr);
#endif
} xf86CursorInfoRec, *xf86CursorInfoPtr;
diff --git a/hw/xfree86/ramdac/xf86CursorPriv.h
b/hw/xfree86/ramdac/xf86CursorPriv.h
index a5d2aab..f34c1c7 100644
--- a/hw/xfree86/ramdac/xf86CursorPriv.h
+++ b/hw/xfree86/ramdac/xf86CursorPriv.h
@@ -37,7 +37,7 @@ typedef struct {
void *transparentData;
} xf86CursorScreenRec, *xf86CursorScreenPtr;
-void xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y);
+Bool xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y);
void xf86SetTransparentCursor(ScreenPtr pScreen);
void xf86MoveCursor(ScreenPtr pScreen, int x, int y);
void xf86RecolorCursor(ScreenPtr pScreen, CursorPtr pCurs, Bool
displayed);
diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
index 3b69698..0b5caa2 100644
--- a/hw/xfree86/ramdac/xf86HWCurs.c
+++ b/hw/xfree86/ramdac/xf86HWCurs.c
@@ -119,7 +119,7 @@ xf86InitHardwareCursor(ScreenPtr pScreen,
xf86CursorInfoPtr infoPtr)
return TRUE;
}
-void
+Bool
xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
{
xf86CursorScreenPtr ScreenPriv =
@@ -130,7 +130,7 @@ xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs,
int x, int y)
if (pCurs == NullCursor) {
(*infoPtr->HideCursor) (infoPtr->pScrn);
- return;
+ return TRUE;
}
bits =
@@ -152,18 +152,21 @@ xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs,
int x, int y)
(*infoPtr->HideCursor) (infoPtr->pScrn);
#ifdef ARGB_CURSOR
- if (pCurs->bits->argb && infoPtr->LoadCursorARGB)
- (*infoPtr->LoadCursorARGB) (infoPtr->pScrn, pCurs);
- else
+ if (pCurs->bits->argb && infoPtr->LoadCursorARGB) {
+ if (!(*infoPtr->LoadCursorARGB) (infoPtr->pScrn, pCurs))
+ return FALSE;
+ } else
#endif
if (bits)
- (*infoPtr->LoadCursorImage) (infoPtr->pScrn, bits);
+ if (!(*infoPtr->LoadCursorImage) (infoPtr->pScrn, bits))
+ return FALSE;
xf86RecolorCursor(pScreen, pCurs, 1);
(*infoPtr->SetCursorPosition) (infoPtr->pScrn, x, y);
(*infoPtr->ShowCursor) (infoPtr->pScrn);
+ return TRUE;
}
void
--
ORACLE Deutschland B.V. & Co. KG Michael Thayer
Werkstrasse 24 VirtualBox engineering
71384 Weinstadt, Germany mailto:[email protected]
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603
Geschäftsführer: Jürgen Kunz
Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Astrid Kepper, Val Maher
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
