[PATCH xf86-video-mali 1/2] Fix function prototype mismatches and X-server crash on vt switch

140 views
Skip to first unread message

Hans de Goede

unread,
Dec 20, 2012, 7:39:29 PM12/20/12
to linux...@googlegroups.com, Hans de Goede
When building against Xorg-1.13 I get the following compiler warnings:
mali_fbdev.c: In function 'MaliHWEnterVT':
mali_fbdev.c:662:5: warning: passing argument 1 of 'MaliHWAdjustFrame' from incompatible pointer type [enabled by default]
mali_fbdev.c:615:6: note: expected 'ScreenPtr' but argument is of type 'ScrnInfoPtr'
mali_fbdev.c: In function 'MaliHWSwitchModeWeak':
mali_fbdev.c:722:55: warning: return from incompatible pointer type [enabled by default]
mali_fbdev.c: In function 'MaliHWAdjustFrameWeak':
mali_fbdev.c:723:55: warning: return from incompatible pointer type [enabled by default]
mali_fbdev.c: In function 'MaliHWEnterVTWeak':
mali_fbdev.c:724:55: warning: return from incompatible pointer type [enabled by default]
mali_fbdev.c: In function 'MaliHWLeaveVTWeak':
mali_fbdev.c:725:55: warning: return from incompatible pointer type [enabled by default]
mali_fbdev.c: In function 'MaliHWValidModeWeak':
mali_fbdev.c:726:55: warning: return from incompatible pointer type [enabled by default]
mali_fbdev.c: In function 'MaliProbe':
mali_fbdev.c:746:15: warning: initialization discards 'const' qualifier from pointer target type [enabled by default]

And worse, when I try to switch to another virtual console, Xorg crashes.
This patches:
-Introduces compat-api.h, used by most other Xorg drivers to deal with
Xorg-1.13 API changes
-Uses this to fix the warnings, also removing a ton of #ifdefs
-As a bonus also fixes the VT switch crash, which was likely caused by
some function parameters being wrong

Signed-off-by: Hans de Goede <hdeg...@redhat.com>
---
src/compat-api.h | 99 ++++++++++++++++++++++++++++++++++++
src/mali_fbdev.c | 151 ++++++++-----------------------------------------------
2 files changed, 120 insertions(+), 130 deletions(-)
create mode 100644 src/compat-api.h

diff --git a/src/compat-api.h b/src/compat-api.h
new file mode 100644
index 0000000..6bc946f
--- /dev/null
+++ b/src/compat-api.h
@@ -0,0 +1,99 @@
+/*
+ * Copyright 2012 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Author: Dave Airlie <air...@redhat.com>
+ */
+
+/* this file provides API compat between server post 1.13 and pre it,
+ it should be reused inside as many drivers as possible */
+#ifndef COMPAT_API_H
+#define COMPAT_API_H
+
+#ifndef GLYPH_HAS_GLYPH_PICTURE_ACCESSOR
+#define GetGlyphPicture(g, s) GlyphPicture((g))[(s)->myNum]
+#define SetGlyphPicture(g, s, p) GlyphPicture((g))[(s)->myNum] = p
+#endif
+
+#ifndef XF86_HAS_SCRN_CONV
+#define xf86ScreenToScrn(s) xf86Screens[(s)->myNum]
+#define xf86ScrnToScreen(s) screenInfo.screens[(s)->scrnIndex]
+#endif
+
+#ifndef XF86_SCRN_INTERFACE
+
+#define SCRN_ARG_TYPE int
+#define SCRN_INFO_PTR(arg1) ScrnInfoPtr pScrn = xf86Screens[(arg1)]
+
+#define SCREEN_ARG_TYPE int
+#define SCREEN_PTR(arg1) ScreenPtr pScreen = screenInfo.screens[(arg1)]
+
+#define SCREEN_INIT_ARGS_DECL int i, ScreenPtr pScreen, int argc, char **argv
+
+#define BLOCKHANDLER_ARGS_DECL int arg, pointer blockData, pointer pTimeout, pointer pReadmask
+#define BLOCKHANDLER_ARGS arg, blockData, pTimeout, pReadmask
+
+#define CLOSE_SCREEN_ARGS_DECL int scrnIndex, ScreenPtr pScreen
+#define CLOSE_SCREEN_ARGS scrnIndex, pScreen
+
+#define ADJUST_FRAME_ARGS_DECL int arg, int x, int y, int flags
+#define ADJUST_FRAME_ARGS(arg, x, y) (arg)->scrnIndex, x, y, 0
+
+#define SWITCH_MODE_ARGS_DECL int arg, DisplayModePtr mode, int flags
+#define SWITCH_MODE_ARGS(arg, m) (arg)->scrnIndex, m, 0
+
+#define FREE_SCREEN_ARGS_DECL int arg, int flags
+
+#define VT_FUNC_ARGS_DECL int arg, int flags
+#define VT_FUNC_ARGS pScrn->scrnIndex, 0
+
+#define XF86_SCRN_ARG(x) ((x)->scrnIndex)
+#else
+#define SCRN_ARG_TYPE ScrnInfoPtr
+#define SCRN_INFO_PTR(arg1) ScrnInfoPtr pScrn = (arg1)
+
+#define SCREEN_ARG_TYPE ScreenPtr
+#define SCREEN_PTR(arg1) ScreenPtr pScreen = (arg1)
+
+#define SCREEN_INIT_ARGS_DECL ScreenPtr pScreen, int argc, char **argv
+
+#define BLOCKHANDLER_ARGS_DECL ScreenPtr arg, pointer pTimeout, pointer pReadmask
+#define BLOCKHANDLER_ARGS arg, pTimeout, pReadmask
+
+#define CLOSE_SCREEN_ARGS_DECL ScreenPtr pScreen
+#define CLOSE_SCREEN_ARGS pScreen
+
+#define ADJUST_FRAME_ARGS_DECL ScrnInfoPtr arg, int x, int y
+#define ADJUST_FRAME_ARGS(arg, x, y) arg, x, y
+
+#define SWITCH_MODE_ARGS_DECL ScrnInfoPtr arg, DisplayModePtr mode
+#define SWITCH_MODE_ARGS(arg, m) arg, m
+
+#define FREE_SCREEN_ARGS_DECL ScrnInfoPtr arg
+
+#define VT_FUNC_ARGS_DECL ScrnInfoPtr arg
+#define VT_FUNC_ARGS pScrn
+
+#define XF86_SCRN_ARG(x) (x)
+
+#endif
+
+#endif
diff --git a/src/mali_fbdev.c b/src/mali_fbdev.c
index 33ba04a..090cbda 100644
--- a/src/mali_fbdev.c
+++ b/src/mali_fbdev.c
@@ -40,6 +40,7 @@
#include "xf86xv.h"
#include "xf86Crtc.h"
#include "micmap.h"
+#include "compat-api.h"

#include "mali_def.h"
#include "mali_fbdev.h"
@@ -58,13 +59,8 @@ static void MaliIdentify(int flags);
static Bool MaliProbe(DriverPtr drv, int flags);
static Bool MaliPreInit(ScrnInfoPtr pScrn, int flags);

-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
-static Bool MaliScreenInit(ScreenPtr pScreen, int argc, char **argv);
-static Bool MaliCloseScreen(ScreenPtr pScreen);
-#else
-static Bool MaliScreenInit(int Index, ScreenPtr pScreen, int argc, char **argv);
-static Bool MaliCloseScreen(int scrnIndex, ScreenPtr pScreen);
-#endif
+static Bool MaliScreenInit(SCREEN_INIT_ARGS_DECL);
+static Bool MaliCloseScreen(CLOSE_SCREEN_ARGS_DECL);

static int pix24bpp = 0;
static int malihwPrivateIndex = -1;
@@ -508,7 +504,7 @@ void MaliHWRestore(ScrnInfoPtr pScrn)
xf86DrvMsg(pScrn->scrnIndex, X_ERROR,"FBIOPUT_VSCREENINFO: %s\n", strerror(errno));
}

-Bool MaliHWProbe( char *device, char **namep )
+Bool MaliHWProbe( const char *device, char **namep )
{
int fd;

@@ -569,17 +565,9 @@ Bool MaliHWSaveScreen(ScreenPtr pScreen, int mode)
return TRUE;
}

-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
-ModeStatus MaliHWValidMode(ScreenPtr pScreen, DisplayModePtr mode, Bool verbose, int flags)
-#else
-ModeStatus MaliHWValidMode(int scrnIndex, DisplayModePtr mode, Bool verbose, int flags)
-#endif
+ModeStatus MaliHWValidMode(SCRN_ARG_TYPE arg, DisplayModePtr mode, Bool verbose, int flags)
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
- ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
-#else
- ScrnInfoPtr pScrn = xf86Screens[scrnIndex];
-#endif
+ SCRN_INFO_PTR(arg);

TRACE_ENTER();

@@ -591,42 +579,23 @@ ModeStatus MaliHWValidMode(int scrnIndex, DisplayModePtr mode, Bool verbose, int
return MODE_OK;
}

-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
-Bool MaliHWSwitchMode(ScreenPtr pScreen, DisplayModePtr mode, int flags)
-#else
-Bool MaliHWSwitchMode(int scrnIndex, DisplayModePtr mode, int flags)
-#endif
+Bool MaliHWSwitchMode(SWITCH_MODE_ARGS_DECL)
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
- ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
-#else
- ScrnInfoPtr pScrn = xf86Screens[scrnIndex];
-#endif
+ SCRN_INFO_PTR(arg);

TRACE_ENTER();
- IGNORE(flags);

if (!MaliHWSetMode(pScrn, mode, FALSE)) return FALSE;

return TRUE;
}

-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
-void MaliHWAdjustFrame(ScreenPtr pScreen, int x, int y, int flags)
-#else
-void MaliHWAdjustFrame(int scrnIndex, int x, int y, int flags)
-#endif
+void MaliHWAdjustFrame(ADJUST_FRAME_ARGS_DECL)
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
- ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
-#else
- ScrnInfoPtr pScrn = xf86Screens[scrnIndex];
-#endif
-
+ SCRN_INFO_PTR(arg);
MaliHWPtr fPtr = MALIHWPTR(pScrn);

TRACE_ENTER();
- IGNORE(flags);

if ( x < 0 || x + fPtr->var.xres > fPtr->var.xres_virtual || y < 0 || y + fPtr->var.yres > fPtr->var.yres_virtual ) return;

@@ -634,53 +603,27 @@ void MaliHWAdjustFrame(int scrnIndex, int x, int y, int flags)
fPtr->var.yoffset = y;
if ( -1 == ioctl( fPtr->fd, FBIOPAN_DISPLAY, (void*)&fPtr->var) )
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
- xf86DrvMsgVerb(pScrn->scrnIndex, X_WARNING, 5, "FBIOPAN_DISPLAY: %s\n", strerror(errno));
-#else
- xf86DrvMsgVerb(scrnIndex, X_WARNING, 5, "FBIOPAN_DISPLAY: %s\n", strerror(errno));
-#endif
+ xf86DrvMsgVerb(pScrn->scrnIndex, X_WARNING, 5, "FBIOPAN_DISPLAY: %s\n", strerror(errno));
}
}

-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
-Bool MaliHWEnterVT(ScreenPtr pScreen, int flags)
-#else
-Bool MaliHWEnterVT(int scrnIndex, int flags)
-#endif
+Bool MaliHWEnterVT(VT_FUNC_ARGS_DECL)
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
- ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
-#else
- ScrnInfoPtr pScrn = xf86Screens[scrnIndex];
-#endif
+ SCRN_INFO_PTR(arg);

TRACE_ENTER();
- IGNORE(flags);

if (!MaliHWModeInit(pScrn, pScrn->currentMode)) return FALSE;
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
- MaliHWAdjustFrame(pScrn, pScrn->frameX0, pScrn->frameY0, 0);
-#else
- MaliHWAdjustFrame(pScrn->scrnIndex, pScrn->frameX0, pScrn->frameY0, 0);
-#endif
+ MaliHWAdjustFrame(ADJUST_FRAME_ARGS(pScrn, pScrn->frameX0, pScrn->frameY0));

return TRUE;
}

-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
-void MaliHWLeaveVT(ScreenPtr pScreen, int flags)
-#else
-void MaliHWLeaveVT(int scrnIndex, int flags)
-#endif
+void MaliHWLeaveVT(VT_FUNC_ARGS_DECL)
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
- ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
-#else
- ScrnInfoPtr pScrn = xf86Screens[scrnIndex];
-#endif
+ SCRN_INFO_PTR(arg);

TRACE_ENTER();
- IGNORE(flags);

MaliHWRestore(pScrn);
}
@@ -743,7 +686,7 @@ static Bool MaliProbe( DriverPtr drv, int flags )

for (i = 0; i < numDevSections; i++)
{
- char *dev = xf86FindOptionValue( devSections[i]->options, "fbdev" );
+ const char *dev = xf86FindOptionValue( devSections[i]->options, "fbdev" );
if ( MaliHWProbe( dev, NULL ) )
{
pScrn = NULL;
@@ -1108,17 +1051,9 @@ static Bool MaliPreInit(ScrnInfoPtr pScrn, int flags)
return TRUE;
}

-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
-static Bool MaliScreenInit(ScreenPtr pScreen, int argc, char **argv)
-#else
-static Bool MaliScreenInit(int scrnIndex, ScreenPtr pScreen, int argc, char **argv)
-#endif
+static Bool MaliScreenInit(SCREEN_INIT_ARGS_DECL)
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
-#else
- ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
-#endif
MaliPtr fPtr = MALIPTR(pScrn);
VisualPtr visual;
int init_picture = 0;
@@ -1144,20 +1079,12 @@ static Bool MaliScreenInit(int scrnIndex, ScreenPtr pScreen, int argc, char **ar
fPtr->dri_render = DRI_2;
fPtr->dri_open = TRUE;
}
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
else xf86DrvMsg(pScrn->scrnIndex,X_ERROR,"DRI2 initialization failed\n");
-#else
- else xf86DrvMsg(scrnIndex,X_ERROR,"DRI2 initialization failed\n");
-#endif
}

if (NULL == (fPtr->fbmem = MaliHWMapVidmem(pScrn)))
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
xf86DrvMsg(pScrn->scrnIndex,X_ERROR,"mapping of video memory failed\n");
-#else
- xf86DrvMsg(scrnIndex,X_ERROR,"mapping of video memory failed\n");
-#endif
return FALSE;
}
fPtr->fboff = MaliHWLinearOffset(pScrn);
@@ -1166,19 +1093,11 @@ static Bool MaliScreenInit(int scrnIndex, ScreenPtr pScreen, int argc, char **ar

if (!MaliHWModeInit(pScrn, pScrn->currentMode))
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
xf86DrvMsg(pScrn->scrnIndex,X_ERROR,"mode initialization failed\n");
-#else
- xf86DrvMsg(scrnIndex,X_ERROR,"mode initialization failed\n");
-#endif
return FALSE;
}
MaliHWSaveScreen(pScreen, SCREEN_SAVER_ON);
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
- MaliHWAdjustFrame(pScreen,0,0,0);
-#else
- MaliHWAdjustFrame(scrnIndex,0,0,0);
-#endif
+ MaliHWAdjustFrame(ADJUST_FRAME_ARGS(pScrn, 0, 0));

/* mi layer */
miClearVisualTypes();
@@ -1186,11 +1105,7 @@ static Bool MaliScreenInit(int scrnIndex, ScreenPtr pScreen, int argc, char **ar
{
if (!miSetVisualTypes(pScrn->depth, TrueColorMask, pScrn->rgbBits, TrueColor))
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
xf86DrvMsg(pScrn->scrnIndex,X_ERROR,"visual type setup failed for %d bits per pixel [1]\n", pScrn->bitsPerPixel);
-#else
- xf86DrvMsg(scrnIndex,X_ERROR,"visual type setup failed for %d bits per pixel [1]\n", pScrn->bitsPerPixel);
-#endif
return FALSE;
}
}
@@ -1198,21 +1113,13 @@ static Bool MaliScreenInit(int scrnIndex, ScreenPtr pScreen, int argc, char **ar
{
if (!miSetVisualTypes(pScrn->depth, miGetDefaultVisualMask(pScrn->depth), pScrn->rgbBits, pScrn->defaultVisual))
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
xf86DrvMsg(pScrn->scrnIndex,X_ERROR,"visual type setup failed for %d bits per pixel [2]\n", pScrn->bitsPerPixel);
-#else
- xf86DrvMsg(scrnIndex,X_ERROR,"visual type setup failed for %d bits per pixel [2]\n", pScrn->bitsPerPixel);
-#endif
return FALSE;
}
}
if (!miSetPixmapDepths())
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
xf86DrvMsg(pScrn->scrnIndex,X_ERROR,"pixmap depth setup failed\n");
-#else
- xf86DrvMsg(scrnIndex,X_ERROR,"pixmap depth setup failed\n");
-#endif
return FALSE;
}

@@ -1280,11 +1187,7 @@ static Bool MaliScreenInit(int scrnIndex, ScreenPtr pScreen, int argc, char **ar

if (!miCreateDefColormap(pScreen))
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
xf86DrvMsg(pScrn->scrnIndex, X_ERROR,"internal error: miCreateDefColormap failed in FBDevScreenInit()\n");
-#else
- xf86DrvMsg(scrnIndex, X_ERROR,"internal error: miCreateDefColormap failed in FBDevScreenInit()\n");
-#endif
return FALSE;
}

@@ -1323,17 +1226,9 @@ static Bool MaliScreenInit(int scrnIndex, ScreenPtr pScreen, int argc, char **ar
return TRUE;
}

-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
-static Bool MaliCloseScreen(ScreenPtr pScreen)
-#else
-static Bool MaliCloseScreen(int scrnIndex, ScreenPtr pScreen)
-#endif
+static Bool MaliCloseScreen(CLOSE_SCREEN_ARGS_DECL)
{
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
-#else
- ScrnInfoPtr pScrn = xf86Screens[scrnIndex];
-#endif
MaliPtr fPtr = MALIPTR(pScrn);

TRACE_ENTER();
@@ -1344,11 +1239,7 @@ static Bool MaliCloseScreen(int scrnIndex, ScreenPtr pScreen)

pScreen->CreateScreenResources = fPtr->CreateScreenResources;
pScreen->CloseScreen = fPtr->CloseScreen;
-#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,12,99,901,0)
- (*pScreen->CloseScreen)(pScreen);
-#else
- (*pScreen->CloseScreen)(scrnIndex, pScreen);
-#endif
+ (*pScreen->CloseScreen)(CLOSE_SCREEN_ARGS);

if ( fPtr->dri_open && fPtr->dri_render == DRI_2 )
{
--
1.8.0.2

Hans de Goede

unread,
Dec 20, 2012, 7:39:30 PM12/20/12
to linux...@googlegroups.com, Hans de Goede
The driver does not use any symbols from libMali, so it should have never
been linked in before. Not linking against it allows building a fully FOSS
stack for having a (somewhat) accelerated Xorg.

Note that this patch instead links directly against pixman-1, which was being
picked up indirectly through Mali before.

Signed-off-by: Hans de Goede <hdeg...@redhat.com>
---
src/Makefile.am | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 75f8850..4e75e25 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -9,7 +9,7 @@
MALI_DDK="/work/trunk"

mali_drv_la_LTLIBRARIES = mali_drv.la
-mali_drv_la_LDFLAGS = -module -avoid-version -L$(MALI_DDK)/lib -lMali -lUMP -lpthread
+mali_drv_la_LDFLAGS = -module -avoid-version -L$(MALI_DDK)/lib -lpixman-1 -lUMP -lpthread
mali_drv_ladir = @moduledir@/drivers

AM_CFLAGS = @XORG_CFLAGS@ \
--
1.8.0.2

Jari Helaakoski

unread,
Dec 21, 2012, 3:03:45 AM12/21/12
to linux...@googlegroups.com


2012/12/21 Hans de Goede <hdeg...@redhat.com>

When building against Xorg-1.13 I get the following compiler warnings:
mali_fbdev.c: In function 'MaliHWEnterVT':
mali_fbdev.c:662:5: warning: passing argument 1 of 'MaliHWAdjustFrame' from incompatible pointer type [enabled by default]
mali_fbdev.c:615:6: note: expected 'ScreenPtr' but argument is of type 'ScrnInfoPtr'
mali_fbdev.c: In function 'MaliHWSwitchModeWeak':
mali_fbdev.c:722:55: warning: return from incompatible pointer type [enabled by default]
mali_fbdev.c: In function 'MaliHWAdjustFrameWeak':
mali_fbdev.c:723:55: warning: return from incompatible pointer type [enabled by default]
mali_fbdev.c: In function 'MaliHWEnterVTWeak':
mali_fbdev.c:724:55: warning: return from incompatible pointer type [enabled by default]
mali_fbdev.c: In function 'MaliHWLeaveVTWeak':
mali_fbdev.c:725:55: warning: return from incompatible pointer type [enabled by default]
mali_fbdev.c: In function 'MaliHWValidModeWeak':
mali_fbdev.c:726:55: warning: return from incompatible pointer type [enabled by default]
mali_fbdev.c: In function 'MaliProbe':
mali_fbdev.c:746:15: warning: initialization discards 'const' qualifier from pointer target type [enabled by default]

And worse, when I try to switch to another virtual console, Xorg crashes.
This patches:
-Introduces compat-api.h, used by most other Xorg drivers to deal with
 Xorg-1.13 API changes
Nice, But where is old API handled? Seems that compat-api.h has only 1.13 stuff defined.

-Jari 

Jari Helaakoski

unread,
Dec 21, 2012, 3:05:59 AM12/21/12
to linux...@googlegroups.com


2012/12/21 Jari Helaakoski <tek...@gmail.com>
Never mind, didn't notice.
+#ifndef XF86_SCRN_INTERFACE 



-Jari 

Alejandro Mery

unread,
Dec 26, 2012, 11:16:10 AM12/26/12
to linux...@googlegroups.com
thank you! both applied to the `r3p0-04rel0` branch.

Siarhei Siamashka

unread,
Dec 27, 2012, 9:11:27 PM12/27/12
to linux...@googlegroups.com, hdeg...@redhat.com
Indeed the fix seems to be quite useful. But would you like to know how
this "(somewhat) accelerated" translates to actual performance?

One benchmark that is reasonably well correlated with reality for
testing 2D graphics performance in X server is cairo-perf-trace.
Basically it can record the activity of real applications into traces
and then replay these traces for benchmarking purposes. More
information is available at:
http://cworth.org/intel/performance_measurement/

There is even a repository with a collection of traces here:
http://cgit.freedesktop.org/cairo-traces

However these traces were primarily recorded for the fast desktop
systems, and are painfully slow to replay on slow ARM hardware. I have
trimmed them a bit and pushed here:
http://cgit.freedesktop.org/~siamashka/trimmed-cairo-traces/

Benchmarking xf86-video-mali driver can be done by setting
CAIRO_TEST_TARGET=xlib environment variable before running
cairo-perf-trace. While the traces are being replayed (rendering
something on offscreen pixmaps), profiling for the X server can
be done by executing "perf record -p `pidof X`" before starting
the benchmark.

Below are the results for xf86-video-mali. And I have already changed
UMP allocations to use UMP_REF_DRV_CONSTRAINT_USE_CACHE flag here:
https://github.com/linux-sunxi/xf86-video-mali/blob/r3p0-04rel0/src/mali_exa.c#L312
Otherwise 2D performance would be a total disaster.

==================== xf86-video-mali ========================

[ # ] backend test min(s) median(s) stddev. count
[ 0] xlib t-firefox-planet-gnome 43.513 43.513 0.00% 1/1
[ 1] xlib t-chromium-tabs 26.666 26.666 0.00% 1/1
[ 2] xlib t-gvim 177.005 177.005 0.00% 1/1
[ 3] xlib t-gnome-system-monitor 23.878 23.878 0.00% 1/1
[ 4] xlib t-firefox-canvas-alpha 103.676 103.676 0.00% 1/1
[ 5] xlib t-firefox-scrolling 206.212 206.212 0.00% 1/1
[ 6] xlib t-firefox-chalkboard 8.164 8.164 0.00% 1/1
[ 7] xlib t-swfdec-youtube 13.323 13.323 0.00% 1/1
[ 8] xlib t-firefox-particles 94.602 94.602 0.00% 1/1
[ 9] xlib t-firefox-fishbowl 12.469 12.469 0.00% 1/1
[ 10] xlib t-firefox-fishtank 7.103 7.103 0.00% 1/1
[ 11] xlib t-xfce4-terminal-a1 278.290 278.290 0.00% 1/1
[ 12] xlib t-poppler-reseau 43.901 43.901 0.00% 1/1
[ 13] xlib t-grads-heat-map 36.009 36.009 0.00% 1/1
[ 14] xlib t-firefox-talos-gfx 796.893 796.893 0.00% 1/1
[ 15] xlib t-firefox-asteroids 9.536 9.536 0.00% 1/1
[ 16] xlib t-midori-zoomed 15.491 15.491 0.00% 1/1
[ 17] xlib t-swfdec-giant-steps 32.885 32.885 0.00% 1/1
[ 18] xlib t-firefox-talos-svg 24.255 24.255 0.00% 1/1
[ 19] xlib t-poppler 96.171 96.171 0.00% 1/1
[ 20] xlib t-firefox-canvas 113.101 113.101 0.00% 1/1
[ 21] xlib t-evolution 130.715 130.715 0.00% 1/1
[ 22] xlib t-firefox-paintball 10.477 10.477 0.00% 1/1
[ 23] xlib t-gnome-terminal-vim 153.667 153.667 0.00% 1/1

sun4i ~ # perf report -n -s dso

# Overhead Samples Shared Object
# ........ .......... .....................
#
61.68% 1502962 [kernel.kallsyms]
12.76% 311026 libpixman-1.so.0.28.2
5.68% 138421 libexa.so
4.98% 121437 libc-2.15.so
4.23% 103137 [ump]
3.50% 85293 Xorg
2.41% 58812 libUMP.so
2.04% 49688 libfb.so
1.69% 41252 mali_drv.so
0.71% 17418 ld-2.15.so
0.13% 3271 [unknown]
0.09% 2118 libpthread-2.15.so
0.04% 923 libgcrypt.so.11.7.0
0.03% 822 librt-2.15.so
0.00% 15 libgpg-error.so.0.8.0
0.00% 3 libudev.so.0.11.5

A lot of time is spent in the kernel handling UMP ioctls. Those pesky
real applications tend to create and destroy pixmaps at an amazing rate.
And ordinary malloc/calloc allocations can do this job way faster than
UMP. So UMP still has some tricks in its sleeve for making the
experience with 2D graphics really unforgettable for the end users in
addition to trying uncached memory allocations.

The real pixels rasterizing job for XRENDER workload is done by
libpixman-1.so.0.28.2 here, and it takes ~13% of time in the X server.
Another interesting observation is that 5.68% of time is spent in
libexa.so. EXA is a convenience layer for adding hardware 2D
acceleration, and xf86-video-mali registers some empty EXA hooks which
do nothing right now. But even without doing anything useful, this EXA
framework is as expensive as half of the full software rendering job
done by pixman.

There is also libc-2.15.so here, what is it actually doing? Is it
memcpy for copying pixels around and offloading some part of libpixman
work? Let's check:

sun4i ~ # perf report -n | grep libc

1.66% 40343 X libc-2.15.so [.] _int_malloc
1.29% 31479 X libc-2.15.so [.] _int_free
0.91% 22095 X libc-2.15.so [.] malloc
0.39% 9614 X libc-2.15.so [.] __GI___ioctl
0.31% 7531 X libc-2.15.so [.] free
0.09% 2076 X libc-2.15.so [.] __aeabi_read_tp
0.07% 1827 X libc-2.15.so [.] memset
0.07% 1621 X libc-2.15.so [.] memcmp

No, that was just malloc/free.

Impressive, isn't it? Now let's suppose that we add G2D hardware
acceleration support to xf86-video-mali. How much of the performance
improvement are we going to expect? Even with the perfect G2D
acceleration taking zero time for any operations, we can only remove
the time currently used by libpixman from the picture.

And now let's take a look at xf86-video-fbdev and run the same benchmark
for it:

==================== xf86-video-fbdev ========================

[ # ] backend test min(s) median(s) stddev. count
[ 0] xlib t-firefox-planet-gnome 6.100 6.100 0.00% 1/1
[ 1] xlib t-chromium-tabs 2.940 2.940 0.00% 1/1
[ 2] xlib t-gvim 15.470 15.470 0.00% 1/1
[ 3] xlib t-gnome-system-monitor 17.214 17.214 0.00% 1/1
[ 4] xlib t-firefox-canvas-alpha 18.964 18.964 0.00% 1/1
[ 5] xlib t-firefox-scrolling 15.460 15.460 0.00% 1/1
[ 6] xlib t-firefox-chalkboard 16.266 16.266 0.00% 1/1
[ 7] xlib t-swfdec-youtube 6.882 6.882 0.00% 1/1
[ 8] xlib t-firefox-particles 20.290 20.290 0.00% 1/1
[ 9] xlib t-firefox-fishbowl 9.229 9.229 0.00% 1/1
[ 10] xlib t-firefox-fishtank 6.676 6.676 0.00% 1/1
[ 11] xlib t-xfce4-terminal-a1 13.010 13.010 0.00% 1/1
[ 12] xlib t-poppler-reseau 11.500 11.500 0.00% 1/1
[ 13] xlib t-grads-heat-map 2.972 2.972 0.00% 1/1
[ 14] xlib t-firefox-talos-gfx 12.612 12.612 0.00% 1/1
[ 15] xlib t-firefox-asteroids 8.678 8.678 0.00% 1/1
[ 16] xlib t-midori-zoomed 7.826 7.826 0.00% 1/1
[ 17] xlib t-swfdec-giant-steps 21.822 21.822 0.00% 1/1
[ 18] xlib t-firefox-talos-svg 12.253 12.253 0.00% 1/1
[ 19] xlib t-poppler 6.449 6.449 0.00% 1/1
[ 20] xlib t-firefox-canvas 22.022 22.022 0.00% 1/1
[ 21] xlib t-evolution 10.777 10.777 0.00% 1/1
[ 22] xlib t-firefox-paintball 9.730 9.730 0.00% 1/1
[ 23] xlib t-gnome-terminal-vim 10.312 10.312 0.00% 1/1

sun4i ~ # perf report -n -s dso

# Overhead Samples Shared Object
# ........ .......... .....................
#
68.08% 142074 libpixman-1.so.0.28.2
21.34% 44529 [kernel.kallsyms]
4.66% 9718 Xorg
4.37% 9116 libc-2.15.so
0.88% 1846 libfb.so
0.22% 463 ld-2.15.so
0.22% 461 libgcrypt.so.11.7.0
0.14% 284 librt-2.15.so
0.08% 165 [unknown]
0.01% 27 libpthread-2.15.so
0.00% 6 libshadow.so
0.00% 5 libgpg-error.so.0.8.0
0.00% 2 libudev.so.0.11.5
0.00% 1 librecord.so

Now the time in X server is primarily spent in libpixman-1.so.0.28.2 as
it should normally be.

The only practical use for xf86-video-mali is the interoperation with
the mali 3D driver blobs. Without mali blobs it's just a piece of junk,
which is good for nothing. The DDX drivers xf86-video-fbdev or
xf86-video-modesetting (in the case of KMS) are going to provide much
better 2D performance.

Is it strictly necessary to use xf86-video-mali? Or maybe one could try
to provide some sort of a wrapper for the blobs, which would handle all
the 3D rendering in a separate disp layer without interfering and
crippling 2D graphics?

The 2D graphics could be accelerated by just NEON+G2D or by NEON alone
if G2D hardware is not available. Though even making sure that G2D is
actually useful and is accelerating something is not going to be easy.

--
Best regards,
Siarhei Siamashka

Siarhei Siamashka

unread,
Dec 27, 2012, 10:39:58 PM12/27/12
to Siarhei Siamashka, linux...@googlegroups.com, hdeg...@redhat.com
Actually this was not just enabling cache but having UMP_LOCK_ENABLED=1.
It could not open the /dev/umplock (which is actually even good
for performance, because some umplock ioctls are avoided), but still
performs some expensive ump_cache_operations_control() and
ump_switch_hw_usage_secure_id() calls which contribute to the
domination of kernel activity and also probably mess up the CPU caches
distorting statistics (libexa.so percentage anomaly).

The real run with only UMP_REF_DRV_CONSTRAINT_USE_CACHE flag set
for allocations and no umplock:

[ # ] backend test min(s) median(s) stddev. count
[ 0] xlib t-firefox-planet-gnome 10.705 10.705 0.00% 1/1
[ 1] xlib t-chromium-tabs 13.845 13.845 0.00% 1/1
[ 2] xlib t-gvim 97.713 97.713 0.00% 1/1
[ 3] xlib t-gnome-system-monitor 20.082 20.082 0.00% 1/1
[ 4] xlib t-firefox-canvas-alpha 36.471 36.471 0.00% 1/1
[ 5] xlib t-firefox-scrolling 23.384 23.384 0.00% 1/1
[ 6] xlib t-firefox-chalkboard 7.566 7.566 0.00% 1/1
[ 7] xlib t-swfdec-youtube 8.420 8.420 0.00% 1/1
[ 8] xlib t-firefox-particles 20.939 20.939 0.00% 1/1
[ 9] xlib t-firefox-fishbowl 7.946 7.946 0.00% 1/1
[ 10] xlib t-firefox-fishtank 5.749 5.749 0.00% 1/1
[ 11] xlib t-xfce4-terminal-a1 27.714 27.714 0.00% 1/1
[ 12] xlib t-poppler-reseau 17.863 17.863 0.00% 1/1
[ 13] xlib t-grads-heat-map 8.185 8.185 0.00% 1/1
[ 14] xlib t-firefox-talos-gfx 48.066 48.066 0.00% 1/1
[ 15] xlib t-firefox-asteroids 7.487 7.487 0.00% 1/1
[ 16] xlib t-midori-zoomed 9.068 9.068 0.00% 1/1
[ 17] xlib t-swfdec-giant-steps 25.567 25.567 0.00% 1/1
[ 18] xlib t-firefox-talos-svg 16.697 16.697 0.00% 1/1
[ 19] xlib t-poppler 22.848 22.848 0.00% 1/1
[ 20] xlib t-firefox-canvas 41.791 41.791 0.00% 1/1
[ 21] xlib t-evolution 44.728 44.728 0.00% 1/1
[ 22] xlib t-firefox-paintball 5.268 5.268 0.00% 1/1
[ 23] xlib t-gnome-terminal-vim 15.681 15.681 0.00% 1/1

real 9m28.756s
user 0m58.110s
sys 0m7.550s

# Overhead Samples Shared Object
# ........ .......... .....................
#
46.14% 226173 [kernel.kallsyms]
31.68% 155313 libpixman-1.so.0.28.2
6.40% 31393 libc-2.15.so
4.89% 23986 Xorg
4.59% 22479 libexa.so
1.87% 9188 libfb.so
1.78% 8719 [ump]
0.86% 4231 mali_drv.so
0.76% 3720 libUMP.so
0.45% 2185 ld-2.15.so
0.21% 1014 libpthread-2.15.so
0.19% 912 [unknown]
0.12% 564 libgcrypt.so.11.7.0
0.07% 356 librt-2.15.so
0.00% 7 libgpg-error.so.0.8.0

At least libexa.so overhead gets more or less reasonable without
umplock.

And finally a run for the original uncached memory allocations
(unmodified xf86-video-mali):

[ # ] backend test min(s) median(s) stddev. count
[ 0] xlib t-firefox-planet-gnome 14.232 14.232 0.00% 1/1
[ 1] xlib t-chromium-tabs 27.626 27.626 0.00% 1/1
[ 2] xlib t-gvim 173.952 173.952 0.00% 1/1
[ 3] xlib t-gnome-system-monitor 31.386 31.386 0.00% 1/1
[ 4] xlib t-firefox-canvas-alpha 77.296 77.296 0.00% 1/1
[ 5] xlib t-firefox-scrolling 32.566 32.566 0.00% 1/1
[ 6] xlib t-firefox-chalkboard 10.224 10.224 0.00% 1/1
[ 7] xlib t-swfdec-youtube 9.843 9.843 0.00% 1/1
[ 8] xlib t-firefox-particles 35.238 35.238 0.00% 1/1
[ 9] xlib t-firefox-fishbowl 14.876 14.876 0.00% 1/1
[ 10] xlib t-firefox-fishtank 11.748 11.748 0.00% 1/1
[ 11] xlib t-xfce4-terminal-a1 37.611 37.611 0.00% 1/1
[ 12] xlib t-poppler-reseau 19.127 19.127 0.00% 1/1
[ 13] xlib t-grads-heat-map 8.449 8.449 0.00% 1/1
[ 14] xlib t-firefox-talos-gfx 60.467 60.467 0.00% 1/1
[ 15] xlib t-firefox-asteroids 9.432 9.432 0.00% 1/1
[ 16] xlib t-midori-zoomed 10.217 10.217 0.00% 1/1
[ 17] xlib t-swfdec-giant-steps 58.222 58.222 0.00% 1/1
[ 18] xlib t-firefox-talos-svg 30.992 30.992 0.00% 1/1
[ 19] xlib t-poppler 24.251 24.251 0.00% 1/1
[ 20] xlib t-firefox-canvas 83.824 83.824 0.00% 1/1
[ 21] xlib t-evolution 68.971 68.971 0.00% 1/1
[ 22] xlib t-firefox-paintball 7.419 7.419 0.00% 1/1
[ 23] xlib t-gnome-terminal-vim 17.287 17.287 0.00% 1/1

real 15m32.752s
user 0m57.780s
sys 0m7.890s

# Overhead Samples Shared Object
# ........ .......... .....................
#
45.31% 386874 libpixman-1.so.0.28.2
42.85% 365927 [kernel.kallsyms]
3.38% 28868 libc-2.15.so
2.58% 21991 Xorg
2.43% 20716 libexa.so
1.00% 8538 [ump]
0.98% 8363 libfb.so
0.48% 4105 mali_drv.so
0.41% 3491 libUMP.so
0.25% 2156 ld-2.15.so
0.12% 1041 [unknown]
0.11% 907 libpthread-2.15.so
0.07% 599 libgcrypt.so.11.7.0
0.04% 344 librt-2.15.so
0.00% 7 libgpg-error.so.0.8.0
0.00% 3 libudev.so.0.11.5
0.00% 1 libdbe.so

Now pixman at last gets to the top of the chart if it has to work with
uncached memory buffers.

But xf86-video-mali does not stand a chance against xf86-video-fbdev
in any of these configurations.
Reply all
Reply to author
Forward
0 new messages