Created attachment 16615 [details]
Photo of unpatched overlay test image (luminance 0-47)

Created attachment 16616 [details]
Photo of overlay with patched driver (test image, luminance 0-47)

The first part of this patch is definitely correct.  bits 7:5 of RADEON_OV0_SCALE_CNTL are different between r1xx and r2xx.  I'll review the gamma curse stuff as well and commit this as soon as I get my fdo account fixed up.

(In reply to comment #0)
> Created an attachment (id=16614) [details]
> Fix for gamma curve on r300 series

> So the slope values for
> RADEON_OV0_GAMMA_000_00F,
> RADEON_OV0_GAMMA_010_01F and
> RADEON_OV0_GAMMA_020_03F
> need to be shifted by (2, 2, 1) to the left respectively.
Probably the values in the r200 gamma table are just wrong - at least datasheets seem to indicate no difference between r200 and r300 in that area (and for that matter, those segments which are defined on r100 too should be the same as far as I can tell, except the offset for the two last segments, and they'd indeed pretty much be the same with those shifts).
I know there were some earlier complaints about gamma tables being wrong, but IIRC noone really investigated this a bit deeper.

> 
> I also found, that RADEONSetOverlayGamma() does not allow arbitrary
> gamma values, but only 8 differnt ones, where only value 0 (gamma 1.0)
> would work properly, since
> 
> |    /* Set gamma */
> |    RADEONWaitForIdleMMIO(pScrn);
> |    ov0_scale_cntl = INREG(RADEON_OV0_SCALE_CNTL) &
> ~RADEON_SCALER_GAMMA_SEL_MASK;
> |    OUTREG(RADEON_OV0_SCALE_CNTL, ov0_scale_cntl | (gamma << 0x00000005));
> 
> breaks my overlay (pink screen) for 'gamma' values other than '0'.
Yup, that's r100 only (the test should be < CHIP_FAMILY_R200 though, not <= - I think r200 family will ignore those bits however, while r300 has them redefined).
So on r100 you can only have 8 different gamma values, since you can't adjust all segments freely. Maybe we should try to get rid of the tables entirely for r200/r300 and just calculate all the segment values as needed, with arbitrary gamma value.

(In reply to comment #4)
> Probably the values in the r200 gamma table are just wrong - at least
> datasheets seem to indicate no difference between r200 and r300 in that area

I sure hope these datasheets will also eventually be publicly available. :)
http://www.x.org/docs/AMD/R3xx_3D_Registers.pdf looks promising in that regard.
(And I'd also be interested in Imageon w100 datasheets for my Zaurus, but
that's probably a rather small target group...  I really _do_ wonder if this thing can do proper busmaster DMA)

> I know there were some earlier complaints about gamma tables being wrong, but
> IIRC noone really investigated this a bit deeper.

I never noticed the problem on my r200 and the r300 with my old TFT (6bit S-IPS panel + dither), but on my new Samsung (8bit S-PVA panel) it much easier to spot .

> (and for that matter, those segments which are defined on r100 too should be
> the same as far as I can tell, except the offset for the two last segments, and
> they'd indeed pretty much be the same with those shifts).

I suspected that r200 and r300 are the same, but without a r200 to test (I gave my old r200 away, unfortunately, and also don't have a AGP board here) I didn't want to touch that part of the code. :)

> > |    /* Set gamma */
> > |    RADEONWaitForIdleMMIO(pScrn);
> > |    ov0_scale_cntl = INREG(RADEON_OV0_SCALE_CNTL) &
> > ~RADEON_SCALER_GAMMA_SEL_MASK;
> > |    OUTREG(RADEON_OV0_SCALE_CNTL, ov0_scale_cntl | (gamma << 0x00000005));
> > 
> > breaks my overlay (pink screen) for 'gamma' values other than '0'.
> Yup, that's r100 only (the test should be < CHIP_FAMILY_R200 though, not <= - 

See above, I wanted to keep the logic for chips other than r300 unchanged.

> Maybe we should try to get rid of the tables entirely for
> r200/r300 and just calculate all the segment values as needed, with arbitrary
> gamma value.

Something like this?  (Still my debugging stuff, but works for me :))
--- xserver-xorg-video-ati-6.8.0/src/radeon_video.c	2008-02-19 02:10:46.000000000 +0100
+++ xserver-xorg-video-ati-6.8.0.patched_arbitrary_gamma/src/radeon_video.c	2008-05-16 23:14:53.000000000 +0200
@@ -106,6 +106,7 @@
 static Atom xvRedIntensity, xvGreenIntensity, xvBlueIntensity;
 static Atom xvContrast, xvHue, xvColor, xvAutopaintColorkey, xvSetDefaults;
 static Atom xvGamma, xvColorspace;
+static Atom xvOV0FlagControl, xvOV0Gamma00000f, xvOV0Gamma01001f, xvOV0Gamma02003f;
 static Atom xvCRTC;
 static Atom xvEncoding, xvFrequency, xvVolume, xvMute,
 	     xvDecBrightness, xvDecContrast, xvDecHue, xvDecColor, xvDecSaturation,
@@ -367,7 +368,7 @@
 
 #endif
 
-#define NUM_ATTRIBUTES 22
+#define NUM_ATTRIBUTES 26
 #define NUM_DEC_ATTRIBUTES (NUM_ATTRIBUTES+12)
 
 static XF86AttributeRec Attributes[NUM_DEC_ATTRIBUTES+1] =
@@ -379,6 +380,10 @@
    {XvSettable             , 0, 1, "XV_SET_DEFAULTS"},
    {XvSettable | XvGettable, 0, 1, "XV_AUTOPAINT_COLORKEY"},
    {XvSettable | XvGettable, 0, ~0,"XV_COLORKEY"},
+   {XvSettable | XvGettable, 0, ~0,"XV_OV0_FLAG_CNTL"},
+   {XvSettable | XvGettable, 0, ~0,"XV_OV0_GAMMA_000_00F"},
+   {XvSettable | XvGettable, 0, ~0,"XV_OV0_GAMMA_010_01F"},
+   {XvSettable | XvGettable, 0, ~0,"XV_OV0_GAMMA_020_03F"},
    {XvSettable | XvGettable, 0, 1, "XV_DOUBLE_BUFFER"},
    {XvSettable | XvGettable,     0,  255, "XV_OVERLAY_ALPHA"},
    {XvSettable | XvGettable,     0,  255, "XV_GRAPHICS_ALPHA"},
@@ -679,9 +684,9 @@
 static GAMMA_CURVE_R200 gamma_curve_r200[8] =
  {
 	/* Gamma 1.0 */
-      {0x00000040, 0x00000000,
-       0x00000040, 0x00000020,
-       0x00000080, 0x00000040,
+      {0x00000100, 0x00000000,
+       0x00000100, 0x00000020,
+       0x00000100, 0x00000040,
        0x00000100, 0x00000080,
        0x00000100, 0x00000100,
        0x00000100, 0x00000100,
@@ -840,8 +845,40 @@
        0.9135}
 };
 
+
+static double gammafn(double x, double g)
+{
+	return pow(x, 1.0/g);
+}
+
+static CARD32 calc_gamma_r300(CARD32 gamma, int idx)
+{
+	double y0, y1, y2, dy, range;
+	double g;
+	int offsets[] = {0x000, 0x010, 0x020, 0x040, 0x080, 0x0c0,
+	                 0x100,               0x140, 0x180, 0x1c0,
+	                 0x200,               0x240, 0x280, 0x2c0,
+	                 0x300,               0x340, 0x380, 0x3c0,
+	                 0x400};
+	CARD32 offset, slope;
+
+	idx = ClipValue(idx, 0, sizeof(offsets)/sizeof(int)-1);
+	g = (double)gamma / 1000.0;
+
+	range = offsets[idx+1] - offsets[idx];
+	y0 = gammafn((double)offsets[idx<4?idx:idx&~1]/(double)0x400, g);
+	y1 = gammafn((double)offsets[idx]/(double)0x400, g);
+	y2 = gammafn((double)offsets[idx+1]/(double)0x400, g);
+	dy = y2-y1;
+
+	offset = y0*0x800+0.5;
+	slope = dy*0x40000/range+0.5;
+
+	return offset | (slope << 0x10);
+}
+
 static void
-RADEONSetOverlayGamma(ScrnInfoPtr pScrn, CARD32 gamma)
+RADEONSetOverlayGamma(ScrnInfoPtr pScrn, CARD32 gamma, CARD32 user_gamma)
 {
     RADEONInfoPtr    info = RADEONPTR(pScrn);
     unsigned char   *RADEONMMIO = info->MMIO;
@@ -850,64 +887,28 @@
     /* Set gamma */
     RADEONWaitForIdleMMIO(pScrn);
     ov0_scale_cntl = INREG(RADEON_OV0_SCALE_CNTL) & ~RADEON_SCALER_GAMMA_SEL_MASK;
-    OUTREG(RADEON_OV0_SCALE_CNTL, ov0_scale_cntl | (gamma << 0x00000005));
+    OUTREG(RADEON_OV0_SCALE_CNTL, ov0_scale_cntl);
 
     /* Load gamma curve adjustments */
     if (info->ChipFamily >= CHIP_FAMILY_R200) {
-    	OUTREG(RADEON_OV0_GAMMA_000_00F,
-	    (gamma_curve_r200[gamma].GAMMA_0_F_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_0_F_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_010_01F,
-	    (gamma_curve_r200[gamma].GAMMA_10_1F_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_10_1F_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_020_03F,
-	    (gamma_curve_r200[gamma].GAMMA_20_3F_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_20_3F_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_040_07F,
-	    (gamma_curve_r200[gamma].GAMMA_40_7F_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_40_7F_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_080_0BF,
-	    (gamma_curve_r200[gamma].GAMMA_80_BF_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_80_BF_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_0C0_0FF,
-	    (gamma_curve_r200[gamma].GAMMA_C0_FF_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_C0_FF_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_100_13F,
-	    (gamma_curve_r200[gamma].GAMMA_100_13F_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_100_13F_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_140_17F,
-	    (gamma_curve_r200[gamma].GAMMA_140_17F_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_140_17F_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_180_1BF,
-	    (gamma_curve_r200[gamma].GAMMA_180_1BF_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_180_1BF_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_1C0_1FF,
-	    (gamma_curve_r200[gamma].GAMMA_1C0_1FF_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_1C0_1FF_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_200_23F,
-	    (gamma_curve_r200[gamma].GAMMA_200_23F_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_200_23F_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_240_27F,
-	    (gamma_curve_r200[gamma].GAMMA_240_27F_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_240_27F_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_280_2BF,
-	    (gamma_curve_r200[gamma].GAMMA_280_2BF_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_280_2BF_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_2C0_2FF,
-	    (gamma_curve_r200[gamma].GAMMA_2C0_2FF_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_2C0_2FF_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_300_33F,
-	    (gamma_curve_r200[gamma].GAMMA_300_33F_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_300_33F_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_340_37F,
-	    (gamma_curve_r200[gamma].GAMMA_340_37F_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_340_37F_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_380_3BF,
-	    (gamma_curve_r200[gamma].GAMMA_380_3BF_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_380_3BF_SLOPE << 0x00000010));
-    	OUTREG(RADEON_OV0_GAMMA_3C0_3FF,
-	    (gamma_curve_r200[gamma].GAMMA_3C0_3FF_OFFSET << 0x00000000) |
-	    (gamma_curve_r200[gamma].GAMMA_3C0_3FF_SLOPE << 0x00000010));
+    	OUTREG(RADEON_OV0_GAMMA_000_00F, calc_gamma_r300(user_gamma, 0));
+    	OUTREG(RADEON_OV0_GAMMA_010_01F, calc_gamma_r300(user_gamma, 1));
+    	OUTREG(RADEON_OV0_GAMMA_020_03F, calc_gamma_r300(user_gamma, 2));
+    	OUTREG(RADEON_OV0_GAMMA_040_07F, calc_gamma_r300(user_gamma, 3));
+    	OUTREG(RADEON_OV0_GAMMA_080_0BF, calc_gamma_r300(user_gamma, 4));
+    	OUTREG(RADEON_OV0_GAMMA_0C0_0FF, calc_gamma_r300(user_gamma, 5));
+    	OUTREG(RADEON_OV0_GAMMA_100_13F, calc_gamma_r300(user_gamma, 6));
+    	OUTREG(RADEON_OV0_GAMMA_140_17F, calc_gamma_r300(user_gamma, 7));
+    	OUTREG(RADEON_OV0_GAMMA_180_1BF, calc_gamma_r300(user_gamma, 8));
+    	OUTREG(RADEON_OV0_GAMMA_1C0_1FF, calc_gamma_r300(user_gamma, 9));
+    	OUTREG(RADEON_OV0_GAMMA_200_23F, calc_gamma_r300(user_gamma, 10));
+    	OUTREG(RADEON_OV0_GAMMA_240_27F, calc_gamma_r300(user_gamma, 11));
+    	OUTREG(RADEON_OV0_GAMMA_280_2BF, calc_gamma_r300(user_gamma, 12));
+    	OUTREG(RADEON_OV0_GAMMA_2C0_2FF, calc_gamma_r300(user_gamma, 13));
+    	OUTREG(RADEON_OV0_GAMMA_300_33F, calc_gamma_r300(user_gamma, 14));
+    	OUTREG(RADEON_OV0_GAMMA_340_37F, calc_gamma_r300(user_gamma, 15));
+    	OUTREG(RADEON_OV0_GAMMA_380_3BF, calc_gamma_r300(user_gamma, 16));
+    	OUTREG(RADEON_OV0_GAMMA_3C0_3FF, calc_gamma_r300(user_gamma, 17));
     } else {
     	OUTREG(RADEON_OV0_GAMMA_000_00F,
 	    (gamma_curve_r100[gamma].GAMMA_0_F_OFFSET << 0x00000000) |
@@ -1080,7 +1081,7 @@
     }
 
     /* set gamma */
-    RADEONSetOverlayGamma(pScrn, gamma);
+    RADEONSetOverlayGamma(pScrn, gamma, user_gamma);
 
     /* color transforms */
     OUTREG(RADEON_OV0_LIN_TRANS_A, dwOvRCb | dwOvLuma);
@@ -1217,6 +1218,10 @@
     xvSetDefaults       = MAKE_ATOM("XV_SET_DEFAULTS");
     xvCRTC              = MAKE_ATOM("XV_CRTC");
 
+    xvOV0FlagControl  = MAKE_ATOM("XV_OV0_FLAG_CNTL");
+    xvOV0Gamma00000f  = MAKE_ATOM("XV_OV0_GAMMA_000_00F");
+    xvOV0Gamma01001f  = MAKE_ATOM("XV_OV0_GAMMA_010_01F");
+    xvOV0Gamma02003f  = MAKE_ATOM("XV_OV0_GAMMA_020_03F");
     xvOvAlpha	      = MAKE_ATOM("XV_OVERLAY_ALPHA");
     xvGrAlpha	      = MAKE_ATOM("XV_GRAPHICS_ALPHA");
     xvAlphaMode       = MAKE_ATOM("XV_ALPHA_MODE");
@@ -1285,7 +1290,7 @@
 	 * segments are programmable in the older Radeons.
 	 */
 
-    RADEONSetOverlayGamma(pScrn, 0); /* gamma = 1.0 */
+    RADEONSetOverlayGamma(pScrn, 0, 1000); /* gamma = 1.0 */
 
     if(pPriv->VIP!=NULL){
         RADEONVIP_reset(pScrn,pPriv);
@@ -1868,6 +1873,22 @@
         if(pPriv->i2c != NULL) RADEON_board_setmisc(pPriv);
 		if(pPriv->uda1380 != NULL) xf86_uda1380_setvolume(pPriv->uda1380, value);
    } 
+   else if(attribute == xvOV0FlagControl)
+   {
+        OUTREG(RADEON_OV0_FLAG_CNTL, value);
+   }
+   else if(attribute == xvOV0Gamma00000f)
+   {
+        OUTREG(RADEON_OV0_GAMMA_000_00F, value);
+   }
+   else if(attribute == xvOV0Gamma01001f)
+   {
+        OUTREG(RADEON_OV0_GAMMA_010_01F, value);
+   }
+   else if(attribute == xvOV0Gamma02003f)
+   {
+        OUTREG(RADEON_OV0_GAMMA_020_03F, value);
+   }
    else if(attribute == xvOverlayDeinterlacingMethod) 
    {
         if(value<0)value = 0;
@@ -1944,6 +1965,7 @@
 		       pointer	    data)
 {
     RADEONInfoPtr	info = RADEONPTR(pScrn);
+    unsigned char	*RADEONMMIO = info->MMIO;
     RADEONPortPrivPtr	pPriv = (RADEONPortPrivPtr)data;
 
     if (info->accelOn) RADEON_SYNC(info, pScrn);
@@ -2025,6 +2047,14 @@
         *value = pPriv->instance_id;
     else if(attribute == xvAdjustment)
   	*value = pPriv->adjustment;
+    else if(attribute == xvOV0FlagControl)
+	*value = INREG(RADEON_OV0_FLAG_CNTL);
+    else if(attribute == xvOV0Gamma00000f)
+	*value = INREG(RADEON_OV0_GAMMA_000_00F);
+    else if(attribute == xvOV0Gamma01001f)
+	*value = INREG(RADEON_OV0_GAMMA_010_01F);
+    else if(attribute == xvOV0Gamma02003f)
+	*value = INREG(RADEON_OV0_GAMMA_020_03F);
     else
 	return BadMatch;
 

(In reply to comment #5)
> > I know there were some earlier complaints about gamma tables being wrong, but
> > IIRC noone really investigated this a bit deeper.
> 
> I never noticed the problem on my r200 and the r300 with my old TFT (6bit S-IPS
> panel + dither), but on my new Samsung (8bit S-PVA panel) it much easier to
> spot .
How did you generate the test image to see this? I've got a r200 here (and the panel should be 8bit s-pva too) and could look at it.

> > Maybe we should try to get rid of the tables entirely for
> > r200/r300 and just calculate all the segment values as needed, with arbitrary
> > gamma value.
> 
> Something like this?  (Still my debugging stuff, but works for me :))
Yes, that's what I had in mind (of course without the debug xvattr values...). Not sure it's entirely correct, where does that 1000 divide come from? Shouldn't that be 1023.5 (the range of the values)? Though to get rid of the tables completely, the OvGammaCont value would also need to be calculated - I wouldn't know how though it almost looks linear (with the maximum reached at gamma 1.0 and the minimum at about gamma 2.1).

I've gone ahead and pushed the first part of this since it makes a noticeable improvement on r2xx/r3xx and the OV0_SCALE_CNTL setup is obviously wrong on r2xx+.
The defaults for gamma 1.0 slope should be 0x100 according to the databooks, so it appears that is correct too.
b7c80d0c86646105d2bce5d4d59ba6c45aa7cafc

(In reply to comment #6)
> (In reply to comment #5)
> > > I know there were some earlier complaints about gamma tables being wrong, but
> > > IIRC noone really investigated this a bit deeper.
> > 
> > I never noticed the problem on my r200 and the r300 with my old TFT (6bit S-IPS
> > panel + dither), but on my new Samsung (8bit S-PVA panel) it much easier to
> > spot .
> How did you generate the test image to see this? I've got a r200 here (and the
> panel should be 8bit s-pva too) and could look at it.

I found some XV test code on the net and modified it
(You may have to set XV_AUTOPAINT_COLORKEY):

/* -------------------------------------------
 * ---		  XV Testcode		   ---
 * ---		    by AW		   ---*/


#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <time.h>
#include <fcntl.h>
#include <sys/ipc.h>
#include <sys/shm.h>

#include <X11/Xlib.h>
#include <X11/Xutil.h>
#include <X11/Xatom.h>
#include <X11/extensions/Xv.h>
#include <X11/extensions/Xvlib.h>
#include <X11/extensions/XShm.h>


extern int 	XShmQueryExtension(Display*);
extern int 	XShmGetEventBase(Display*);
extern XvImage  *XvShmCreateImage(Display*, XvPortID, int, char*, int, int, XShmSegmentInfo*);

int main (int argc, char* argv[]) {
  int		yuv_width = 48;
  int		yuv_height = 48;

#define FRAME_SIZE  (768*576 + (768*576/4)*2)
  
  int		xv_port = -1;
  int		adaptor, encodings, attributes, formats;
  int		i, j, ret, p, _d, _w, _h;

  char		*buf = malloc(FRAME_SIZE);
  char		*buf2 = malloc(FRAME_SIZE);
  char		*temp;

  int		vidfd = open("/dev/video0", O_RDWR);
  
  XvAdaptorInfo		*ai;
  XvEncodingInfo	*ei;
  XvAttribute		*at;
  XvImageFormatValues	*fo;

  XvImage		*yuv_image;

#define GUID_YUV12_PLANAR 0x32315659
#define GUID_YUY2_PACKED 0x32595559

  unsigned int		p_version, p_release,
  			p_request_base, p_event_base, p_error_base;
  int			p_num_adaptors;
   	
  Display		*dpy;
  Window		window, _dw;
  XSizeHints		hint;
  XSetWindowAttributes	xswa;
  XVisualInfo		vinfo;
  int			screen;
  unsigned long		mask;
  XEvent		event;
  GC			gc;

  /** for shm */
  int 			shmem_flag = 0;
  XShmSegmentInfo	yuv_shminfo;
  int			CompletionType;

  int			dist = 2;


  printf("starting up video testapp...\n\n");
  
  adaptor = -1;
	
  dpy = XOpenDisplay(NULL);
  if (dpy == NULL) {
    printf("Cannot open Display.\n");
    exit (-1);
  }
  
  screen = DefaultScreen(dpy);
  
  /** find best display */
  if (XMatchVisualInfo(dpy, screen, 24, TrueColor, &vinfo)) {
    printf(" found 24bit TrueColor\n");
  } else
    if (XMatchVisualInfo(dpy, screen, 16, TrueColor, &vinfo)) {
      printf(" found 16bit TrueColor\n");
    } else
      if (XMatchVisualInfo(dpy, screen, 15, TrueColor, &vinfo)) {
	printf(" found 15bit TrueColor\n");
      } else
  	if (XMatchVisualInfo(dpy, screen, 8, PseudoColor, &vinfo)) {
	  printf(" found 8bit PseudoColor\n");
  	} else
	  if (XMatchVisualInfo(dpy, screen, 8, GrayScale, &vinfo)) {
	    printf(" found 8bit GrayScale\n");
	  } else
	    if (XMatchVisualInfo(dpy, screen, 8, StaticGray, &vinfo)) {
	      printf(" found 8bit StaticGray\n");
	    } else
	      if (XMatchVisualInfo(dpy, screen, 1, StaticGray, &vinfo)) {
  		printf(" found 1bit StaticGray\n");
	      } else {
  		printf("requires 16 bit display\n");
  		exit (-1);
	      }
  
  CompletionType = -1;	
  
  hint.x = 1;
  hint.y = 1;
  hint.width = yuv_width;
  hint.height = yuv_height;
  hint.flags = PPosition | PSize;
  
  xswa.colormap =  XCreateColormap(dpy, DefaultRootWindow(dpy), vinfo.visual, AllocNone);
  xswa.event_mask = StructureNotifyMask | ExposureMask;
  xswa.background_pixel = 0;
  xswa.border_pixel = 0;
  
  mask = CWBackPixel | CWBorderPixel | CWColormap | CWEventMask;
  
  window = XCreateWindow(dpy, DefaultRootWindow(dpy),
			 0, 0,
			 yuv_width,
			 yuv_height,
			 0, vinfo.depth,
			 InputOutput,
			 vinfo.visual,
			 mask, &xswa);
  
  XStoreName(dpy, window, "firstxv");
  XSetIconName(dpy, window, "firstxv");
  
  XSelectInput(dpy, window, StructureNotifyMask);
  
  /** Map window */
  XMapWindow(dpy, window);
  
  /** Wait for map. */
  do {
    XNextEvent(dpy, &event);
  }
  while (event.type != MapNotify || event.xmap.event != window);
  
  if (XShmQueryExtension(dpy)) shmem_flag = 1;
  if (!shmem_flag) {
    printf("no shmem available.\n");
    exit (-1);
  }
  
  if (shmem_flag==1) CompletionType = XShmGetEventBase(dpy) + ShmCompletion;
  
  
  /**--------------------------------- XV ------------------------------------*/
  printf("beginning to parse the Xvideo extension...\n\n");
  
  /** query and print Xvideo properties */
  ret = XvQueryExtension(dpy, &p_version, &p_release, &p_request_base,
			 &p_event_base, &p_error_base);
  if (ret != Success) {
    if (ret == XvBadExtension)
      printf("XvBadExtension returned at XvQueryExtension.\n");
    else
      if (ret == XvBadAlloc)
	printf("XvBadAlloc returned at XvQueryExtension.\n");
      else
	printf("other error happened at XvQueryExtension.\n");
  }
  printf("========================================\n");
  printf("XvQueryExtension returned the following:\n");
  printf("p_version      : %u\n", p_version);
  printf("p_release      : %u\n", p_release);
  printf("p_request_base : %u\n", p_request_base);
  printf("p_event_base   : %u\n", p_event_base);
  printf("p_error_base   : %u\n", p_error_base);
  printf("========================================\n");
  
  ret = XvQueryAdaptors(dpy, DefaultRootWindow(dpy),
			&p_num_adaptors, &ai);
  
  if (ret != Success) {
    if (ret == XvBadExtension)
      printf("XvBadExtension returned at XvQueryExtension.\n");
    else
      if (ret == XvBadAlloc)
	printf("XvBadAlloc returned at XvQueryExtension.\n");
      else
	printf("other error happaned at XvQueryAdaptors.\n");
  }
  printf("=======================================\n");
  printf("XvQueryAdaptors returned the following:\n");
  printf("%d adaptors available.\n", p_num_adaptors);
  for (i = 0; i < p_num_adaptors; i++) {
    printf(" name:        %s\n"
	   " type:        %s%s%s%s%s\n"
	   " ports:       %ld\n"
	   " first port:  %ld\n",
	   ai[i].name,
	   (ai[i].type & XvInputMask)	? "input | "	: "",
	   (ai[i].type & XvOutputMask)	? "output | "	: "",
	   (ai[i].type & XvVideoMask)	? "video | "	: "",
	   (ai[i].type & XvStillMask)	? "still | "	: "",
	   (ai[i].type & XvImageMask)	? "image | "	: "",
	   ai[i].num_ports,
	   ai[i].base_id);
    xv_port = ai[i].base_id;
    
    printf("adaptor %d ; format list:\n", i);
    for (j = 0; j < ai[i].num_formats; j++) {
      printf(" depth=%d, visual=%ld\n",
	     ai[i].formats[j].depth,
	     ai[i].formats[j].visual_id);
    }
    for (p = ai[i].base_id; p < ai[i].base_id+ai[i].num_ports; p++) {
      
      printf(" encoding list for port %d\n", p);
      if (XvQueryEncodings(dpy, p, &encodings, &ei) != Success) {
	printf("XvQueryEncodings failed.\n");
	continue;
      }
      for (j = 0; j < encodings; j++) {
	printf("  id=%ld, name=%s, size=%ldx%ld, numerator=%d, denominator=%d\n",
	       ei[j].encoding_id, ei[j].name, ei[j].width, ei[j].height,
	       ei[j].rate.numerator, ei[j].rate.denominator);
      }
      XvFreeEncodingInfo(ei);
      
      printf(" attribute list for port %d\n", p);
      at = XvQueryPortAttributes(dpy, p, &attributes);
      for (j = 0; j < attributes; j++) {
	printf("  name:       %s\n"
	       "  flags:     %s%s\n"
	       "  min_color:  %i\n"
	       "  max_color:  %i\n",
	       at[j].name,
	       (at[j].flags & XvGettable) ? " get" : "",
	       (at[j].flags & XvSettable) ? " set" : "",						
	       at[j].min_value, at[j].max_value);
      }
      if (at)
	XFree(at);
      
      printf(" image format list for port %d\n", p);
      fo = XvListImageFormats(dpy, p, &formats);
      for (j = 0; j < formats; j++) {
	printf("  0x%x (%4.4s) %s\n",
	       fo[j].id,
	       (char *)&fo[j].id,
	       (fo[j].format == XvPacked) ? "packed" : "planar");
      }
      if (fo)
	XFree(fo);
    }
    printf("\n");
  }
  if (p_num_adaptors > 0)
    XvFreeAdaptorInfo(ai);
  if (xv_port == -1)
    exit (0);

  printf("using xv_port %d\n", xv_port);
  
  gc = XCreateGC(dpy, window, 0, 0);		
  
  yuv_image = XvCreateImage(dpy, xv_port, GUID_YUV12_PLANAR, 0, yuv_width, yuv_height);
  yuv_image->data = malloc(yuv_image->data_size);
  
  memset(yuv_image->data, 128, yuv_image->data_size);
  
  for (i = 0; i < yuv_image->height; i++) {
	memset(&yuv_image->data[yuv_image->width*i], i, yuv_image->width);
  }
  
  while (1) {
    XGetGeometry(dpy, window, &_dw, &_d, &_d, &_w, &_h, &_d, &_d);
      
    XvPutImage(dpy, xv_port, window, gc, yuv_image,
               0, 0, yuv_image->width, yuv_image->height,
               0, 0, _w, _h);
    XFlush(dpy);
    usleep(500000);
  }
  
  return 0;
}


> > > Maybe we should try to get rid of the tables entirely for
> > > r200/r300 and just calculate all the segment values as needed, with arbitrary
> > > gamma value.
> > 
> > Something like this?  (Still my debugging stuff, but works for me :))
> Yes, that's what I had in mind (of course without the debug xvattr values...).
> Not sure it's entirely correct, where does that 1000 divide come from?

Because XV_GAMMA 1000 == Gamma 1.0

(In reply to comment #7)
> I've gone ahead and pushed the first part of this since it makes a noticeable
> improvement on r2xx/r3xx and the OV0_SCALE_CNTL setup is obviously wrong on
> r2xx+.
> The defaults for gamma 1.0 slope should be 0x100 according to the databooks, so
> it appears that is correct too.
> b7c80d0c86646105d2bce5d4d59ba6c45aa7cafc

Ok. I still see 3 problems. One is purely cosmetic (no need for reading/writing ov0_scale_cntl at all for r200 and up chips in the setgamma function), but the other two look real. First, it seems (for r100 chips) the gamma set in the ov0_scale_cntl reg gets overwritten in the RadeonDisplayVideo function (makes me wonder actually why those bogus gamma_sel bits on r200 and up have any effect at all since they just gets set to 0 anyway?).
Also, if the first 3 values for the gamma 1.0 curve were wrong, those for the other gamma curves look wrong to me too.

(In reply to comment #8)

> I found some XV test code on the net and modified it
> (You may have to set XV_AUTOPAINT_COLORKEY):
Ok. Please use attachments for this kind of stuff next time.

> > > Something like this?  (Still my debugging stuff, but works for me :))
> > Yes, that's what I had in mind (of course without the debug xvattr values...).
> > Not sure it's entirely correct, where does that 1000 divide come from?
> 
> Because XV_GAMMA 1000 == Gamma 1.0
Ah right. My first look was a bit too fast :-).

(In reply to comment #9)
> (In reply to comment #7)
> > I've gone ahead and pushed the first part of this since it makes a noticeable
> > improvement on r2xx/r3xx and the OV0_SCALE_CNTL setup is obviously wrong on
> > r2xx+.
> > The defaults for gamma 1.0 slope should be 0x100 according to the databooks, so
> > it appears that is correct too.
> > b7c80d0c86646105d2bce5d4d59ba6c45aa7cafc
> 
> Ok. I still see 3 problems. One is purely cosmetic (no need for reading/writing
> ov0_scale_cntl at all for r200 and up chips in the setgamma function), but the

good point.

> other two look real. First, it seems (for r100 chips) the gamma set in the
> ov0_scale_cntl reg gets overwritten in the RadeonDisplayVideo function (makes
> me wonder actually why those bogus gamma_sel bits on r200 and up have any
> effect at all since they just gets set to 0 anyway?).

Probably not.

> Also, if the first 3 values for the gamma 1.0 curve were wrong, those for the
> other gamma curves look wrong to me too.
> 

Definitely possible.  I got the tables from ati years ago.

(In reply to comment #9)
> Ok. I still see 3 problems. One is purely cosmetic (no need for reading/writing
> ov0_scale_cntl at all for r200 and up chips in the setgamma function), but the
> other two look real. First, it seems (for r100 chips) the gamma set in the
> ov0_scale_cntl reg gets overwritten in the RadeonDisplayVideo function (makes
> me wonder actually why those bogus gamma_sel bits on r200 and up have any
> effect at all since they just gets set to 0 anyway?).

Well, I did notice that they only had an effect until the image was moved or
unpaused (I did my most of my tests with a paused MPlayer window, before I
modified the XVideo test application).

> Also, if the first 3 values for the gamma 1.0 curve were wrong, those for the
> other gamma curves look wrong to me too.

Yeah, that was my impression too.

Closing this old bug since the gamma tables were fixed. And I don't use that notebook anymore so it's unlikely that I'll do something about the table calculation thing.