Bug 2761

Created attachment 2136 [details]
Source code of XGI Volari Z7

Source code release of XGI Volari Z7 for XOrg X11 6.8.2

Roland, Egbert, what about adding the xgi driver sources to CVS head? 

BTW, the driver works fine for me on all of my XGI boards. I've boards for
testing available (Volari V3XT/V8 and Volari Z7). V3XT/V8 share the same Device
ID. Volari Z7 seems to be a preproduction sample (PCI board).

I've tested with X.Org 6.8.2 on IA32 and AMD64. 

I'll add a patch who disables the autoload of the DRI module, which I didn't
like as this prevents the Xserver from starting, when the glx module is not
loaded (unresolved GLX symbols).

Created attachment 2296 [details] [review]
Disable the autoload of the DRI module

Created attachment 2297 [details] [review]
fixes build with gcc4 on AMD64.

I'm going to commit this.
Two notes (one is a question):
1. platform dependent types like unsigned long should be avoided when dealing   
   with registers. We use CARD8 CARD16 and CARD32 for this. I will change patch 
   in attachment 2296 [details] [review] accordingly.
2. Why should the driver load the DRI module? Can we eliminate this code   
   entirely?

The DRI module loading is not necessary for Volari Z7 but it is required for 
3D features of Volari V5, V8, V3XT. This is caused by the unified source for
Volari series.

Of course, but xgi is the only driver, which does load the DRI module
automatically. 

will the 3D module be open source?

*** Bug 1905 has been marked as a duplicate of this bug. ***

We give a patch to fix the bug of video playback for Z7 by
disabling the XVideo if XGI Z7 is detected.

Source file: xgi_opt.c

Add the code fragment 

  // Disable XV support for XGI Z7 chip
  //------------------------------------
  if (PCI_CHIP_XGIXG20 == pXGI->Chipset)
     pXGI->NoXvideo = TRUE;
  //------------------------------------

as follows,
 
    /* NoAccel
     * Turns off 2D acceleration
     */
    if(xf86ReturnOptValBool(pXGI->Options, OPTION_NOACCEL, FALSE)) {
        pXGI->NoAccel = TRUE;
#if XF86_VERSION_CURRENT < XF86_VERSION_NUMERIC(4,2,99,0,0)
    pXGI->NoXvideo = TRUE;
    xf86DrvMsg(pScrn->scrnIndex, X_CONFIG, "2D Acceleration and Xv 
disabled\n");
#else
    xf86DrvMsg(pScrn->scrnIndex, X_CONFIG, "2D Acceleration disabled\n");
#endif
 
    }

  // Disable XV support for XGI Z7 chip
  //------------------------------------
  if (PCI_CHIP_XGIXG20 == pXGI->Chipset)
     pXGI->NoXvideo = TRUE;
  //------------------------------------
  
    /* SWCursor
     * HWCursor
     * Chooses whether to use the hardware or software cursor
     */
    from = X_DEFAULT;
    if(xf86GetOptValBool(pXGI->Options, OPTION_HW_CURSOR, &pXGI->HWCursor)) {
        from = X_CONFIG;
    }

When submitting patches, always ensure that the patch is in the standard
"unified diff" format.  You can create a unified diff by doing:

diff -Naur old-root-dir new-root-dir > xorg-x11-purpose-of-patch.patch

ie: diff -Naur xc.orig/ xc/ > xorg-x11-xgi-fix-foo.patch

Then attaching the resulting patch as a bugzilla file attachment using
the attachment link above.  Always submit patches using the unified
diff format.

Hope this helps.

Created attachment 2590 [details] [review]
patch for disabling xvideo on Z7

Not a bug comment, really, just a thankyou to XGI for using the community
systems as they were designed to be used. You've done exceptionally well for
"the new kid on the block." Seconding Alex's question: is the 3D stuff going in
as well?

Now if only the other graphics card manufacturers would regard this as a
competitive challenge, and follow suit.

Alex, Leon, I don't want to discourage you, but XGI told me (this was in April 
2004) that the 3D part won't be Open Source. Unlikely that XGI changed its 
mind meanwhile. 

(In reply to comment #14)
> Now if only the other graphics card manufacturers would regard this as a
> competitive challenge, and follow suit.

I'm curious which other graphics vendors you're referring to here?

- ATI continues as they always have in the past to contribute directly to
  the 2D ati "radeon" driver on a regular basis

- Mark Vojkovich, an employee of Nvidia, contributes to the open source "nv"
  driver on a regular basis as a personal hobby.  This would not be possible
  if Nvidia did not provide him with the documentation he needs.

- Intel contributes directly to the 2D and 3D i[89]xx driver development by
  funding development of these drivers through Tungsten Graphics

- VIA contributes directly to the 2D and 3D "via" driver


I don't think there is any question that we all absolutely applaud XGI's
effort to provide open source drivers for their products.  I'm curious
how you think it is a competitive advantage however, unless the
"competition" you're referring to is not one of the vendors I've listed
above?

The only vendors that do not contribute 2D driver source directly to Xorg
on a frequent basis currently consists of Matrox, S3 (for the savage, s3,
s3virge drivers), Cirrus, Trident (for the trident driver), and a variety
of rather old "legacy" hardware from vendors that either no longer in the
market, or whom hold small market niches.

Providing full featured open source 3D drivers would make any vendor stand
out in Linux space at this point.  Right now, only Intel and VIA seem to
be actively doing this.  Hopefully other vendors will follow their lead.

Jong, 
 
Using gcc4 you can see, that several variables are not initialized in the  
driver. IMHO this should really be fixed. It's "only" a warning, but since gcc  
now is able to distinguish between "may be used uninitialized in this  
function" and "is used uninitialized in this function" the build on SuSE now  
fails (we treat this as fatal warning now).  
 
Since I don't know the driver at all I must admit, that I don't know how to 
fix this the right way. :-( 
  
vb_setmode.c: In function 'XGI_GetLcdPtr':  
vb_setmode.c:6685: warning: 'tempdi' is used uninitialized in this function  
vb_setmode.c: In function 'XGI_GetTVPtr':  
vb_setmode.c:7138: warning: 'tempdi' is used uninitialized in this function  
vb_setmode.c: In function 'XGI_GetCRT2Data':  
vb_setmode.c:4938: warning: 'tempbx' is used uninitialized in this function  
vb_setmode.c: In function 'XGI_DisableBridge':  
vb_setmode.c:7968: warning: 'tempbl' is used uninitialized in this function  
[...]  
init.c: In function 'XGI_New_SetCRT1Group':  
init.c:1417: warning: 'RefreshRateTableIndex' is used uninitialized in this  
function  
init.c:1176: warning: 'resindex' is used uninitialized in this function  
  
You can reproduce these warnings with gcc4. You can find packages for SuSE  
9.2/9.3 (x86/x86_64) and a README how to use it here:  
  
  ftp://ftp.suse.com/pub/projects/gcc/GCC4.0.0-Pre 

Created attachment 2717 [details] [review]
Remove warnings for Volari V5,V8, V3XT

Remove warnings for Volari V5,V8, V3XT

Created attachment 2718 [details] [review]
Remove warnings for Volari Z7

Remove warnings for Volari Z7

Thanks. But which one of the patch should be applied to the driver? AFAIK the 
driver is a unified driver, which supports  V5/V8/V3XT and Z7. At least it 
works on all of my cards I have for testing here (V8, V3XT and Z7). 
Unfortunately the patches are somewhat different. :-( 

When comparing the results of using the different patches I can see only  
cosmetic changes, so it shouldn't matter which one to apply. Looking at the  
dates in the patches, "2d-40.patch" seems to be the newer one. 
  
  

After applying the patch I still get 2 "uninitialized" warnings. 
 
init.c:1417: warning: 'RefreshRateTableIndex' is used uninitialized in this 
             function 
init.c:1176: warning: 'resindex' is used uninitialized in this function 
 
I don't think these are false positives. 

Created attachment 2819 [details] [review]
Patch for compile warnings of init.c

Patch for compile warnings of init.c

Thanks for the patch. So can someone with CVS commit rights finally commit 
this new driver to CVS, so it will be included with X.Org 6.9/7.0? 

Egbert will take care of this now.

Egbert, please use the tarball and all the patches for an initial checkin. You
need only one the "Remove warnings for ..." patches (see my comments #20/21).

(In reply to comment #3)
> BTW, the driver works fine for me on all of my XGI boards. I've boards for
> 
> I've tested with X.Org 6.8.2 on IA32 and AMD64. 
> 
Sorry, but what kernel have You used and haven'y You used hack methods,
described in http://www.webspawner.com/users/dell5160/

I didn't use any special kernel and only tested the driver on boards, which   
are supported by this driver (Volari V3XT/V8 and Volari Z7). XP5 is probably  
a different one. 

Created attachment 2988 [details] [review]
Detect num_open_faces bug

Fixed patch.

blocker for 7.0.

The sis driver now fully supports the Volari Z7, V3XT, V5 and V8, apart from the
binary 3D module.

The SiS Linux kernel framebuffer driver now also supports these chips, and the
current version (yet only available on my website) will be submitted for
inclusion in the mainline kernel soon. 

As regards 3D, my current plans are to extend the "sis" Linux DRM kernel module
to support these XGI chips as well (which is easy... it's just adding the PCI
IDs - the DRM doesn't do anything hardware specific; the XGI DRM kernel module
is otherwise functionally identical with the existing sis module) and then

1) either wait for 3D specs or the source of the DRI driver (I am currently
negotiating an NDA with SiS and XGI),

2) or, in the meantime, let XGI adapt their client side DRI driver so that it
can deal with the SiS DRM and SiS X drivers (which is also easy, they just need
to change some symbol names to call the SiS DRM ioctls)

But since no one seriously thought about shipping a binary module, wouldn't it
be appropriate to close this bug?

(In reply to comment #31)
> But since no one seriously thought about shipping a binary module, wouldn't it
> be appropriate to close this bug?

Excellent news.  I don't see any compelling reason to keep this open any longer
given that sis(4) supports these cards now.  I'm calling this one fixed.

Would it be possible to port this to Darwin / Mac OS?

It could be ported to Darwin but there's no such a plan in XGI so far.

-----Original Message-----
From: bugzilla-daemon@freedesktop.org
[mailto:bugzilla-daemon@freedesktop.org] 
Sent: Friday, May 14, 2010 7:45 AM
To: Jong Lin
Subject: [Bug 2761] Release source code of XGI Volari Z7

https://bugs.freedesktop.org/show_bug.cgi?id=2761

--- Comment #33 from BC Holloway <ynfotech@gmail.com> 2010-05-13
16:45:19 PDT ---
Would it be possible to port this to Darwin / Mac OS?