Howto consolidate xgifb (staging) with sisfb

Hi,

I'm currently trying to improve the xgifb driver from staging a bit and my 
final goal is to probably merge this driver with the sis driver (as both card 
families seem to have a lot in common).

However I'm a little bit unsure how to proceed here.

e.g. the sis and the xgifb driver have some nearly duplicated functions in 
common, e.g.
static void XGIfb_bpp_to_var(struct xgifb_video_info *xgifb_info, 			     
struct fb_var_screeninfo *var)
http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/staging/xgifb/XGI_main_26.c;hb=HEAD#l769

and 
static void sisfb_bpp_to_var(struct sis_video_info *ivideo, struct 
fb_var_screeninfo *var)
http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/video/sis/sis_main.c;hb=HEAD#l1129


have the same semantics. (the only difference is the _video_info struct).

-> Is there a way the xgifb driver can call the function from sis?  (I know 
it's static and not EXPORT_SYMBOL'ed)
Or how would you prepare the consolidation here ? 
(e.g. make the functions identical? and when the merge get's done sometime 
simply remove the duplicate?)



And how would you proceed with functions that are nearly identical (but each 
function is missing some bits from the other card)
e.g. 
static int sisfb_get_fix(struct fb_fix_screeninfo *fix, int con, struct 
fb_info *info)
http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/video/sis/sis_main.c;hb=HEAD#l1839
and 
static int XGIfb_get_fix(struct fb_fix_screeninfo *fix, int con,  struct 
fb_info *info)
http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/staging/xgifb/XGI_main_26.c;hb=HEAD#l1365

8 more lines

On Tue, Mar 20, 2012 at 12:02:39AM +0100, Peter Hüwe wrote:

Hi,

I'm currently trying to improve the xgifb driver from staging a bit and my 
final goal is to probably merge this driver with the sis driver (as both card 
families seem to have a lot in common).

However I'm a little bit unsure how to proceed here.

e.g. the sis and the xgifb driver have some nearly duplicated functions in 
common, e.g.
static void XGIfb_bpp_to_var(struct xgifb_video_info *xgifb_info, 			     
struct fb_var_screeninfo *var)
http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/staging/xgifb/XGI_main_26.c;hb=HEAD#l769

and 
static void sisfb_bpp_to_var(struct sis_video_info *ivideo, struct 
fb_var_screeninfo *var)
http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/video/sis/sis_main.c;hb=HEAD#l1129


have the same semantics. (the only difference is the _video_info struct).

-> Is there a way the xgifb driver can call the function from sis?  (I know 
it's static and not EXPORT_SYMBOL'ed)

Declare it not static and EXPORT_SYMBOL it :)

Or how would you prepare the consolidation here ? 
(e.g. make the functions identical? and when the merge get's done sometime 
simply remove the duplicate?)

How about moving things to a "library" module that both modules call
with these functions, pulling them from one of the drivers.

And how would you proceed with functions that are nearly identical (but each 
function is missing some bits from the other card)
e.g. 
static int sisfb_get_fix(struct fb_fix_screeninfo *fix, int con, struct 
fb_info *info)
http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/video/sis/sis_main.c;hb=HEAD#l1839
and 
static int XGIfb_get_fix(struct fb_fix_screeninfo *fix, int con,  struct 
fb_info *info)
http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/staging/xgifb/XGI_main_26.c;hb=HEAD#l1365
could be easily merged, but in which direction? Add the new code to directly 
to sis or 'backport' sis stuff to xgifb? or both (i.e. make them identical 
like in the first case?)

Whatever you feel is best to do here.

greg k-h

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Peter Hüwe wrote:

Hi,

I'm currently trying to improve the xgifb driver from staging a bit and my 
final goal is to probably merge this driver with the sis driver (as both card 
families seem to have a lot in common).

However I'm a little bit unsure how to proceed here.

e.g. the sis and the xgifb driver have some nearly duplicated functions in 
common, e.g.
static void XGIfb_bpp_to_var(struct xgifb_video_info *xgifb_info, 			     
struct fb_var_screeninfo *var)
http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/staging/xgifb/XGI_main_26.c;hb=HEAD#l769

and 
static void sisfb_bpp_to_var(struct sis_video_info *ivideo, struct 
fb_var_screeninfo *var)
http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/video/sis/sis_main.c;hb=HEAD#l1129


have the same semantics. (the only difference is the _video_info struct).

-> Is there a way the xgifb driver can call the function from sis?  (I know 
it's static and not EXPORT_SYMBOL'ed)
Or how would you prepare the consolidation here ? 
(e.g. make the functions identical? and when the merge get's done sometime 
simply remove the duplicate?)



And how would you proceed with functions that are nearly identical (but each 
function is missing some bits from the other card)
e.g. 
static int sisfb_get_fix(struct fb_fix_screeninfo *fix, int con, struct 
fb_info *info)
http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/video/sis/sis_main.c;hb=HEAD#l1839
and 
static int XGIfb_get_fix(struct fb_fix_screeninfo *fix, int con,  struct 
fb_info *info)
http://git.kernel.org/?p=linux/kernel/git/gregkh/staging.git;a=blob;f=drivers/staging/xgifb/XGI_main_26.c;hb=HEAD#l1365

10 more quoted lines

I seem to remember that sisfb supported most of the XGI cards
(V3XT/V5/V8, Z7) as well.... What can xgifb do that sisfb can't? Merging
the blobs to support the Z9 and Z11 IMHO doesn't justify a separate driver.

Regards
Thomas

- --
Thomas Winischhofer
thomas AT winischhofer DOT net

The Online Photo Editor  ***************** http://www.phixr.com

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)

iD8DBQFPaCZMzydIRAktyUcRArf4AJ9Sp16HV7j5BmoJnV1GtRIwWKnayACePBF3
1yNMXuxDnpL77ov0EwLs5m0=EMek
-----END PGP SIGNATURE-----

Hi,

On Tue, Mar 20, 2012 at 12:02:39AM +0100, Peter Hüwe wrote:

I'm currently trying to improve the xgifb driver from staging a bit and my 
final goal is to probably merge this driver with the sis driver (as both card 
families seem to have a lot in common).

However I'm a little bit unsure how to proceed here.

I would first continue deleting unused dead code and definitions from the
staging driver. We have already removed maybe 10000 lines of code without
a loss of functionality, and I believe there is still more work to do.

Then with the remaining code, we could start thinking how to consolidate
the stuff with sisfb. Or even with the other fb drivers, I don't think
stuff like bpp_to_var is specific to these two drivers.

And once xgifb is cleaned up, the partial XGI support would be dropped
from sisfb.

A.

On Tue, Mar 20, 2012 at 5:06 PM, Aaro Koskinen [off-list ref] wrote:

Hi,

On Tue, Mar 20, 2012 at 12:02:39AM +0100, Peter Hüwe wrote:

quoted

I'm currently trying to improve the xgifb driver from staging a bit and my
final goal is to probably merge this driver with the sis driver (as both card
families seem to have a lot in common).

However I'm a little bit unsure how to proceed here.

I would first continue deleting unused dead code and definitions from the
staging driver. We have already removed maybe 10000 lines of code without
a loss of functionality, and I believe there is still more work to do.

Then with the remaining code, we could start thinking how to consolidate
the stuff with sisfb. Or even with the other fb drivers, I don't think
stuff like bpp_to_var is specific to these two drivers.

And once xgifb is cleaned up, the partial XGI support would be dropped
from sisfb.

You could probably just add the pci ids to sisfb;  The xgi chips were
just rebadged sis chips.  IIRC, xgifb was just a fork of sisfb with
the strings changed.  XGI was a combination of the old trident and sis
graphics teams.  Most of the xgi cards were just rebadged sis cards
with the exception of the xp10 which was a trident variant and could
probably be merged with the tridentfb driver if there is one.

For reference see the X drivers:
Sis-based:
http://cgit.freedesktop.org/xorg/driver/xf86-video-sis
http://cgit.freedesktop.org/xorg/driver/xf86-video-xgi

Trident-based:
http://cgit.freedesktop.org/xorg/driver/xf86-video-trident
http://cgit.freedesktop.org/xorg/driver/xf86-video-xgixp

Alex