Re: Centralize XXXCopyMungedData
On Wed, Feb 26, 2003 at 04:30:44PM +0100, Egbert Eich wrote: > Thomas Winischhofer writes: > > Egbert Eich wrote: > > > > Erm, I know that... :) What I actually meant was if Björn starts > > patching *drivers* (and that's what his statements sounded like) he'd > > better take care. > > > > OK, sorry, I misunderstood you. > Patching individual drivers should be left to their maintainers > or at least to somebody who has got HW to test. And a nice documentation of the new function would help a lot for that. BTW, is there any such documentation for the Render extension, and more precisely the XAA functions. Friendly, Sven Luther ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
Re: Centralize XXXCopyMungedData
Thomas Winischhofer writes: > Egbert Eich wrote: > > Erm, I know that... :) What I actually meant was if Björn starts > patching *drivers* (and that's what his statements sounded like) he'd > better take care. > OK, sorry, I misunderstood you. Patching individual drivers should be left to their maintainers or at least to somebody who has got HW to test. Egbert. ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
Re: Centralize XXXCopyMungedData
Egbert Eich wrote: > Just my $0.02: Watch out when centralizing the memory allocation > routines, different hardware (ie drivers) use different granularities. > Yes. Any functions provided by the core server will have the status of helper functions. Therefore it they don't meet your needs you can replace them with your own. Erm, I know that... :) What I actually meant was if Björn starts patching *drivers* (and that's what his statements sounded like) he'd better take care. Thomas -- Thomas Winischhofer Vienna/Austria mailto:[EMAIL PROTECTED]http://www.winischhofer.net/ ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
Re: Centralize XXXCopyMungedData
Thomas Winischhofer writes: > > Just my $0.02: Watch out when centralizing the memory allocation > routines, different hardware (ie drivers) use different granularities. > Yes. Any functions provided by the core server will have the status of helper functions. Therefore it they don't meet your needs you can replace them with your own. Egbert. ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
Re: Centralize XXXCopyMungedData
Björn Augustsson wrote: I agree with you that this code doesn't have to be replicated by each driver. There is a lot more code in the video drivers which could be cept in a centralized place. I certainly agree. Gotta start somewhere though... Just my $0.02: Watch out when centralizing the memory allocation routines, different hardware (ie drivers) use different granularities. Thomas -- Thomas Winischhofer Vienna/Austria mailto:[EMAIL PROTECTED]http://www.winischhofer.net/ ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
Re: Centralize XXXCopyMungedData
On Mon, Feb 24, 2003 at 10:41:10AM +0100, Egbert Eich wrote: > Björn Augustsson writes: > > OK, that wasn't so clever... > > > > The rest of the patch is attached... :) > > > > /August. > > Hi Bjoern, > > I agree with you that this code doesn't have to be replicated > by each driver. There is a lot more code in the video drivers > which could be cept in a centralized place. I certainly agree. Gotta start somewhere though... > Would you please submit your patch to [EMAIL PROTECTED] Doing it now. /August. -- Bj|rn Augustsson DCE/DFS Sysadmin IT Systems & Services Chalmers tekniska h|gskola Chalmers University of Technology "Damn spooky analog crap." -- John Carmack. ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
Re: Centralize XXXCopyMungedData
Björn Augustsson writes: > OK, that wasn't so clever... > > The rest of the patch is attached... :) > > /August. Hi Bjoern, I agree with you that this code doesn't have to be replicated by each driver. There is a lot more code in the video drivers which could be cept in a centralized place. Would you please submit your patch to [EMAIL PROTECTED] Regards, Egbert. ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
Re: Centralize XXXCopyMungedData
OK, that wasn't so clever...
The rest of the patch is attached... :)
/August.
--
Bj|rn Augustsson DCE/DFS Sysadmin IT Systems & Services
Chalmers tekniska h|gskola Chalmers University of Technology
"Damn spooky analog crap." -- John Carmack.
--- /dev/null 2003-01-11 23:38:27.0 +0100
+++ xc/programs/Xserver/hw/xfree86/common/xvconv.c 2003-02-24 03:17:20.0
+0100
@@ -0,0 +1,79 @@
+/* $XFree86: $ */
+
+/*
+** File:
+**
+** xvconv.c --- Xv color conversion routine.
+**
+** Author:
+**
+** Bjorn Augustsson <[EMAIL PROTECTED]>
+**
+** Revisions:
+**
+** 11.feb.2003 august
+** - centralize CopyMungedData from many drivers
+**
+*/
+
+#include "Xmd.h" /* For CARDXX */
+#include "Xarch.h" /* For endianness macros */
+#include "xvconv.h"
+
+void
+YV12toUYVY(
+ const unsigned char *src1,
+ const unsigned char *src2,
+ const unsigned char *src3,
+ unsigned char *dst1,
+ int srcPitch,
+ int srcPitch2,
+ int dstPitch,
+ int h,
+ int w
+){
+ CARD32 *dst;
+ const CARD8 *s1, *s2, *s3;
+ int i, j;
+
+ w >>= 1;
+
+ for(j = 0; j < h; j++) {
+dst = (CARD32*)dst1;
+s1 = src1; s2 = src2; s3 = src3;
+i = w;
+while(i > 4) {
+#if X_BYTE_ORDER == X_BIG_ENDIAN
+ dst[0] = (s1[0] << 24) | (s1[1] << 8) | (s3[0] << 16) | s2[0];
+ dst[1] = (s1[2] << 24) | (s1[3] << 8) | (s3[1] << 16) | s2[1];
+ dst[2] = (s1[4] << 24) | (s1[5] << 8) | (s3[2] << 16) | s2[2];
+ dst[3] = (s1[6] << 24) | (s1[7] << 8) | (s3[3] << 16) | s2[3];
+#else
+ dst[0] = s1[0] | (s1[1] << 16) | (s3[0] << 8) | (s2[0] << 24);
+ dst[1] = s1[2] | (s1[3] << 16) | (s3[1] << 8) | (s2[1] << 24);
+ dst[2] = s1[4] | (s1[5] << 16) | (s3[2] << 8) | (s2[2] << 24);
+ dst[3] = s1[6] | (s1[7] << 16) | (s3[3] << 8) | (s2[3] << 24);
+#endif
+ dst += 4; s2 += 4; s3 += 4; s1 += 8;
+ i -= 4;
+}
+
+while(i--) {
+#if X_BYTE_ORDER == X_BIG_ENDIAN
+ dst[0] = (s1[0] << 24) | (s1[1] << 8) | (s3[0] << 16) | s2[0];
+#else
+ dst[0] = s1[0] | (s1[1] << 16) | (s3[0] << 8) | (s2[0] << 24);
+#endif
+ dst++; s2++; s3++;
+ s1 += 2;
+}
+
+dst1 += dstPitch;
+src1 += srcPitch;
+if(j & 1) {
+src2 += srcPitch2;
+src3 += srcPitch2;
+}
+ }
+}
+
--- /dev/null 2003-01-11 23:38:27.0 +0100
+++ xc/programs/Xserver/hw/xfree86/common/xvconv.h 2003-02-24 03:19:02.0
+0100
@@ -0,0 +1,20 @@
+/* $XFree86: $ */
+
+#ifndef _XVCONV_H
+#define _XVCONV_H
+
+
+void YV12toUYVY(
+ const unsigned char *src1,
+ const unsigned char *src2,
+ const unsigned char *src3,
+ unsigned char *dst1,
+ int srcPitch,
+ int srcPitch2,
+ int dstPitch,
+ int h,
+ int w);
+
+
+#endif /* _XVCONV_H */
+
Centralize XXXCopyMungedData
Hi folks!
Here's an interesting exercise for you:
cd xc/programs/Xserver/hw/xfree86/drivers
grep -i mungeddata */*.c
Scary, huh?
All of those functions do the same thing, modulo bugs and performance.
And there's a few more of them, with different names. (grep -i copyYV)
Now, this function is smack in the middle of the critical path for
pretty much any Xv app, so performance matters here. And most of these
functions have a lot of room for improvement. The code is a nice
candidate for the various MMX/SSE/MVI/whatever extensions different
arches have. But it helps to have it in a central place then. So...
Attached is a patch that
a) Adds new files xc/programs/Xserver/hw/xfree86/common/xvconv.[ch]
which contains working code for this. (from the nv driver, originally)
b) Renames the function from CopyMungedData to YV12toUYVY,
since that's what it does.
c) Makes the Imakefile in common respect the BuildXvExt and BuildXvMCExt
defines.
d) Removes the private versions of the code from a couple of drivers
(radeon, mga, s3, nv) and makes them use the central one.
Now, while I'm pretty sure it does fix real bugs (anyone with a radeon
in a big-endian machine feel like trying a Xv program?), I consider this
mostly a cleanup, (the possible bugs are obviously pretty obscure, since
they haven't beeen found yet) and it can certainly wait until after 4.3.
I indend to follow up with some faster code, and maybe patches to
make more drivers use this (unless other driver maintainers do it
first) later.
I'm not sure about the naming (the new files, and the function. I
considered xf86XVYV12toUYVY, but that was just too scary...)
It's not clear to me if the functions called xf86* are supposed to
be some kind of interface or not. I also thought about putting it in the
common/xf86xv.c file. Comments?
/August. Oh yeah, and it makes a non DoLoadableServer server a bit
smaller too. :)
--
Bj|rn Augustsson DCE/DFS Sysadmin IT Systems & Services
Chalmers tekniska h|gskola Chalmers University of Technology
"Damn spooky analog crap." -- John Carmack.
Index: programs/Xserver/hw/xfree86/common/Imakefile
===
RCS file: /cvs/xc/programs/Xserver/hw/xfree86/common/Imakefile,v
retrieving revision 3.148
diff -u -r3.148 Imakefile
--- programs/Xserver/hw/xfree86/common/Imakefile2003/02/17 17:06:41 3.148
+++ programs/Xserver/hw/xfree86/common/Imakefile2003/02/24 01:26:24
@@ -50,11 +50,21 @@
#endif
#if BuildRandR
- RANDRINCS = -I../../../randr
+ RANDRINCS = -I../../../randr
RANDRSRC = xf86RandR.c
RANDROBJ = xf86RandR.o
#endif
+#if BuildXvExt
+XVSRC = xf86xv.c xvconv.c
+XVOBJ = xf86xv.o xvconv.o
+#endif
+
+#if BuildXvMCExt
+XVMCSRC = xf86xvmc.c
+XVMCOBJ = xf86xvmc.o
+#endif
+
MODPATHDEFINES = -DDEFAULT_MODULE_PATH=\"$(MODULEDIR)\"
LOGDEFINES = -DDEFAULT_LOGPREFIX=\"$(LOGDIRECTORY)/XLogFile.\"
@@ -112,8 +122,6 @@
xf86Option.c \
xf86VidMode.c \
xf86fbman.c \
- xf86xv.c \
- xf86xvmc.c \
xf86cmap.c\
xf86PM.c \
$(DEBUGSRC) \
@@ -124,7 +132,9 @@
$(XKBDDXSRC) \
$(BETASRC) \
$(SERVERSRCS) \
-$(RANDRSRC)
+$(RANDRSRC) \
+ $(XVSRC) \
+ $(XVMCSRC)
OBJS = \
xf86Configure.o \
@@ -149,8 +159,6 @@
xf86Option.o \
xf86VidMode.o \
xf86fbman.o \
- xf86xv.o \
- xf86xvmc.o \
xf86cmap.o\
xf86PM.o \
$(DEBUGOBJ) \
@@ -159,7 +167,9 @@
$(XKBDDXOBJ) \
$(BETAOBJ) \
$(KBD).o \
-$(RANDROBJ)
+$(RANDROBJ) \
+ $(XVOBJ) \
+ $(XVMCOBJ)
OFILES = \
xf86Init.o \
Index: programs/Xserver/hw/xfree86/drivers/ati/radeon_video.c
===
RCS file: /cvs/xc/programs/Xserver/hw/xfree86/drivers/ati/radeon_video.c,v
retrieving revision 1.24
diff -u -r1.24 radeon_video.c
--- programs/Xserver/hw/xfree86/drivers/ati/radeon_video.c 2003/02/19 01:19:43
1.24
+++ programs/Xserver/hw/xfree86/drivers/ati/radeon_video.c 2003/02/24 01:26:25
@@ -10,6 +10,7 @@
#include "Xv.h"
#include "fourcc.h"
+#include "xvconv.h"
#define OFF_DELAY 250 /* milliseconds */
#define FREE_DELAY 15000
@@ -869,52 +870,6 @@
}
}
-static void
-RADEONCopyMungedData(
- unsigned char *src1,
- unsigned char *src2,
- unsigned char *src3,
- unsigned char *dst1,
- int srcPitch,
- int srcPitch2,
- int dstPitch,
- int h,
- int w
-){
- CARD32 *dst;
- CARD8 *s1, *s2, *s3;
- int i, j;
-
- w >>= 1;
-
- for(j = 0; j < h; j++) {
- dst = (pointer)dst1;
- s1 = src1; s2 = src2; s3 = src3;
- i = w;
- while(i > 4) {
- dst[0] = s1[0] | (s1[1] << 16) | (s3[0] << 8) | (s2[0] << 24);
- dst[1] = s1[2] | (s1[3] << 16) | (s3[1] << 8) | (s2[1] << 24);
- dst[2] = s1[4] | (s1[5
