[MPlayer-dev-eng] [PATCH] mga_vid for 2.6 - updated...again
Attila Kinali
attila at kinali.ch
Sun Aug 8 10:36:40 CEST 2004
On Tue, Jul 13, 2004 at 10:50:10PM +0200, Ferdinand O. Tempel wrote:
> Attached you'll find a new and improved patch for getting mga_vid to
> work on 2.6 kernels. This time it won't break compiling on non-devfs 2.4
> kernels (this time I tested). Kudos to Reimar for that one. I can't
> vouch for 2.4 kernels *with* devfs, but I hear it already worked on
> that.
> This one is tested on:
> - 2.6.7
> - 2.6.7 + devfs
> - 2.4.26
The test wit 2.4.x + devfs is missing, see below.
Also you fixed only a few of the things i pointed out.
> --- main/drivers/mga_vid.c 2004-07-07 20:56:26.000000000 +0200
> +++ mga-work/mga_vid.c 2004-07-13 22:34:39.000000000 +0200
> @@ -1,15 +1,22 @@
> +#define MODULENAME "mga_vid"
> +
> //#define CRTC2
>
> // YUY2 support (see config.format) added by A'rpi/ESP-team
> // double buffering added by A'rpi/ESP-team
> // brightness/contrast introduced by eyck
> // multiple card support by Attila Kinali <attila at kinali.ch>
> +// ported to the 2.6 series kernels by F.O. Tempel
> +// thankfully using the ground work done by Ed Sweetman (for the devfs work)
> +// and Gergely Nagy for pushing into the right direction with his patch for 2.6.0-test1
>
> // Set this value, if autodetection fails! (video ram size in megabytes)
> // #define MGA_MEMORY_SIZE 16
>
> //#define MGA_ALLOW_IRQ
>
> +#define MGA_MAX_CARDS 16
Why do you move this define ?
> @@ -117,7 +134,8 @@
> #ifndef min
> #define min(x,y) (((x)<(y))?(x):(y))
> #endif
> -
> +// These functions are provided by the 2.6.0 kernel these days.
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)
> #if LINUX_VERSION_CODE < KERNEL_VERSION(2,4,0)
This is redundand, if the second if matches, the first will
never match.
If you care abou the inclusion of simple_strtol at line 150
you should remove there the check for >2.5.5 (yes, remove,
no need to support a testing kernel)
> #include <linux/ctype.h>
>
> @@ -348,17 +366,30 @@
> #define ICLEAR 0x1e18
> #define STATUS 0x1e14
>
> -
> -// global devfs handle for /dev/mga_vid
> +/* Global handles for cdev and devfs */
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
> +static struct cdev *mga_vid_cdev;
> +static dev_t mga_cdev_handle;
> +#ifdef CONFIG_DEVFS_FS
> +typedef struct devfs_entry *devfs_handle_t;
> +devfs_handle_t dev_handle = NULL;
This should be static.
> +#endif
> +#else
> #ifdef CONFIG_DEVFS_FS
> -static devfs_handle_t dev_handle = NULL;
> + devfs_handle_t dev_handle = NULL;
Don't remove the static here.
> +#endif
> #endif
>
> // card local config
> typedef struct mga_card_s {
>
> // local devfs handle for /dev/mga_vidX
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
> #ifdef CONFIG_DEVFS_FS
> + struct devfs_entry *devfs_handle_t;
> +#endif
> +#endif
> +#if CONFIG_DEVFS_FS
> devfs_handle_t dev_handle;
> #endif
>
> @@ -397,7 +428,7 @@
> int next_frame;
> } mga_card_t;
>
> -#define MGA_MAX_CARDS 16
> +static int mga_max_cards = MGA_MAX_CARDS;
> // this is used as init value for the parameter arrays
> // it should have exactly MGA_MAX_CARDS elements
> #define MGA_MAX_CARDS_INIT_ARRAY {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
> @@ -411,11 +442,19 @@
> static int mga_contrast[MGA_MAX_CARDS] = MGA_MAX_CARDS_INIT_ARRAY;
> static int mga_top_reserved[MGA_MAX_CARDS] = MGA_MAX_CARDS_INIT_ARRAY;
>
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
> +module_param_array(mga_ram_size, int, mga_max_cards, 0);
> +module_param_array(mga_top_reserved, int, mga_max_cards, 0);
> +module_param_array(mga_brightness, int, mga_max_cards, 0);
> +module_param_array(mga_contrast, int, mga_max_cards, 0);
Is there a reason why you use here a variable instead of the
define (aka MGA_MAX_CARDS) ? It's redundand and just usefull
for code obfuscation.
> +module_param(major, int, 0);
> +#else
> MODULE_PARM(mga_ram_size, "1-" __MODULE_STRING(MGA_MAX_CARDS) "i");
> MODULE_PARM(mga_top_reserved, "1-" __MODULE_STRING(MGA_MAX_CARDS) "i");
> MODULE_PARM(mga_brightness, "1-" __MODULE_STRING(MGA_MAX_CARDS) "i");
> MODULE_PARM(mga_contrast, "1-" __MODULE_STRING(MGA_MAX_CARDS) "i");
> MODULE_PARM(major, "i");
> +#endif
>
> #ifdef CRTC2
> static void crtc2_frame_sel(mga_card_t * card, int frame)
> @@ -1100,7 +1140,7 @@
>
> #ifdef MGA_ALLOW_IRQ
>
> -static void enable_irq(mga_card_t * card){
> +static void mga_enable_irq(mga_card_t * card){
Don't change the names of static functions.
> long int cc;
>
> cc = readl(card->mmio_base + IEN);
> @@ -1116,16 +1156,18 @@
>
> }
>
> -static void disable_irq(mga_card_t * card){
> +static void mga_disable_irq(mga_card_t * card){
Don't change the names of static functions.
>
> writeb(0x11, card->mmio_base + CRTCX);
> writeb(0x20, card->mmio_base + CRTCD); /* clear 0, enable off */
>
> }
>
> -static void mga_handle_irq(int irq, void *dev_id, struct pt_regs *pregs) {
> +static int mga_handle_irq(int irq, void *dev_id, struct pt_regs *pregs) {
void -> int change requires a kernel version check.
> // static int frame=0;
> -// static int counter=0;
> +#ifdef MP_DEBUG
> + static int counter=0;
> +#endif
> long int cc;
> mga_card_t * card = dev_id;
>
> @@ -1136,7 +1178,7 @@
> // check whether the interrupt is really for us (irq sharing)
> if ( irq != -1 ) {
> cc = readl(card->mmio_base + STATUS);
> - if ( ! (cc & 0x10) ) return; /* vsyncpen */
> + if ( ! (cc & 0x10) ) return 0; /* vsyncpen */
Use the defined value as return value instead of 0.
> // debug_irqcnt++;
> }
>
> @@ -1152,15 +1194,15 @@
> // i han echt kei ahnig was das obe heisse s?ll
> crtc2_frame_sel(card->next_frame);
> #endif
> -
> -#if 0
> +
> +#ifdef MP_DEBUG
> ++counter;
> if(!(counter&63)){
> printk("mga irq counter = %d\n",counter);
> }
> #endif
Dont change this #if 0 to #ifdev MP_DEBUG, it may have negative
effects on system, beside flooding the kernel log.
>
> -// } else {
> + // } else {
> // debug_irqignore = 1;
> // }
>
> @@ -1173,7 +1215,7 @@
> // writel( card->regs.besglobctl, card->mmio_base + BESGLOBCTL);
>
>
> - return;
> + return 0;
Again return the correct value instead of 0.
>
> }
>
> @@ -1257,7 +1299,7 @@
> mga_vid_write_regs(card, 0);
> }
> #ifdef MGA_ALLOW_IRQ
> - if ( card->irq != -1 ) enable_irq(card);
> + if ( card->irq != -1 ) mga_enable_irq(card);
> #endif
> card->next_frame=0;
> break;
> @@ -1268,7 +1310,7 @@
> #endif
> card->vid_src_ready = 0;
> #ifdef MGA_ALLOW_IRQ
> - if ( card->irq != -1 ) disable_irq(card);
> + if (card->irq != -1 ) mga_disable_irq(card);
Two times unnecessary name change.
> #endif
> card->regs.besctl &= ~1;
> card->regs.besglobctl &= ~(1<<6); // UYVY format selected
> - return(0);
> + return 0;
Unnecessary cosmetic.
> }
>
> static int mga_vid_release(struct inode *inode, struct file *file)
> @@ -1539,8 +1591,10 @@
> return(-EBUSY);
>
> card->vid_in_use = 1;
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)
> MOD_INC_USE_COUNT;
> - return(0);
> +#endif
> + return 0;
Unnecessary cosmetic.
> }
>
> #if LINUX_VERSION_CODE >= 0x020400
> @@ -1573,10 +1627,6 @@
> static void cards_init(mga_card_t * card, struct pci_dev * dev, int card_number, int is_g400)
> {
> unsigned int card_option;
> -// temp buffer for device filename creation used only by devfs
> -#ifdef CONFIG_DEVFS_FS
> - char buffer[16];
> -#endif
>
> memset(card,0,sizeof(mga_card_t));
> card->irq = -1;
> @@ -1647,10 +1697,11 @@
> // case 0x13: card->ram_size = 8; break;
> default: card->ram_size = 8;
> }
> - }
> +}
> +
> #if 0
> // printk("List resources -----------\n");
> - for(temp=0;temp<DEVICE_COUNT_RESOURCE;temp++){
> + for(int temp=0;temp<DEVICE_COUNT_RESOURCE;temp++){
for(int blah... may not compile with older compilers.
Leave this as it was.
> struct resource *res=&dev->resource[temp];
> if(res->flags){
> int size=(1+res->end-res->start)>>20;
> @@ -1665,7 +1716,6 @@
> #endif
> }
>
> -
> #ifdef MGA_ALLOW_IRQ
> if ( card->irq != -1 ) {
> int tmp = request_irq(card->irq, mga_handle_irq, SA_INTERRUPT | SA_SHIRQ, "Syncfb Time Base", card);
> @@ -1683,16 +1733,15 @@
> printk(KERN_INFO "syncfb (mga): IRQ disabled in mga_vid.c\n");
> card->irq=-1;
> #endif
> -
> - // register devfs, let the kernel give us major and minor numbers
> -#ifdef CONFIG_DEVFS_FS
> - snprintf(buffer, 16, "mga_vid%d", card_number);
> +// register devfs, let the kernel give us major and minor numbers
> +#if CONFIG_DEVFS_FS && LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)
> + char buffer[16];
> + snprintf(buffer, 16, "%s%d", MODULENAME, card_number);
> card->dev_handle = devfs_register(NULL, buffer, DEVFS_FL_AUTO_DEVNUM,
> 0, 0,
> S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IFCHR,
> &mga_vid_fops, card);
> #endif
Where is the code for 2.4 devfs ?
This will only work on 2.6 and is thus a los on functionality.
> -
> }
>
> /*
> @@ -1701,73 +1750,108 @@
>
> static int mga_vid_initialize(void)
> {
> - int i;
> -
> -// printk(KERN_INFO "Matrox MGA G200/G400 YUV Video interface v0.01 (c) Aaron Holtzman \n");
> + int i;
> printk(KERN_INFO "Matrox MGA G200/G400/G450/G550 YUV Video interface v2.01 (c) Aaron Holtzman & A'rpi\n");
> -
> - for(i = 0; i < MGA_MAX_CARDS; i++)
> +
> + if(mga_vid_find_card())
> {
> - if (mga_ram_size[i]) {
> - if (mga_ram_size[i]<4 || mga_ram_size[i]>64) {
> - printk(KERN_ERR "mga_vid: invalid RAMSIZE: %d MB\n", mga_ram_size[i]);
> - return -EINVAL;
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
> + /* Have the kernel generate a major device number */
> + if(major == 0)
> + {
> + if(!alloc_chrdev_region(&mga_cdev_handle, 0, mga_cards_num, "mga_vid")) {
> + major = MAJOR(mga_cdev_handle);
> + printk(KERN_INFO "mga_vid: using major: %d (dynamically alocated!)\n", major);
> + }
> + }
> + else
> + {
> + mga_cdev_handle = MKDEV(major,0);
> + if(!register_chrdev_region(mga_cdev_handle, mga_cards_num, "mga_vid"))
> + printk(KERN_INFO "mga_vid: using major: %d (assigned or default!)\n", major);
> + }
> + /* Allocate a cdev for this character device, and fill in some parameters it needs */
> + mga_vid_cdev = cdev_alloc();
> + mga_vid_cdev->owner = THIS_MODULE;
> + strcpy(mga_vid_cdev->kobj.name, MODULENAME);
> + mga_vid_cdev->ops = &mga_vid_fops;
> + /* Add this character device to the system */
> + cdev_add(mga_vid_cdev, mga_cdev_handle, mga_cards_num);
> +#endif
> + for(i = 0; i < mga_cards_num; i++)
> + {
> +#if CONFIG_DEVFS_FS && LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
> + /* Create the device, and register a symlink for the first card found.
> + * Lets not break default behaviour, eh? */
> + devfs_mk_cdev(MKDEV(major,i), S_IFCHR | S_IRUSR | S_IRGRP | S_IWUSR, "video/mga_vid%d", i);
Is there a reason why you put the device files into video/ ?
Is there a standard somewhere ?
And if, imho video/ is missleading, this is a graphics card driver, not
a video device driver.
> + if( i == 0 ) {
> + devfs_mk_symlink(MODULENAME,"video/mga_vid0");
> + }
> +#endif
> + if (mga_ram_size[i]) {
> + if (mga_ram_size[i]<4 || mga_ram_size[i]>64) {
> + printk(KERN_ERR "mga_vid: invalid RAMSIZE: %d MB\n", mga_ram_size[i]);
> + return -EINVAL;
> + }
> }
> }
> - }
>
> - if(register_chrdev(major, "mga_vid", &mga_vid_fops))
> - {
> - printk(KERN_ERR "mga_vid: unable to get major: %d\n", major);
> - return -EIO;
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,0)
> + if(register_chrdev(major, MODULENAME, &mga_vid_fops))
> + {
> + printk(KERN_ERR "mga_vid: unable to get major: %d\n", major);
> + return -EIO;
> + }
> +#endif
> + return 0;
> }
> -
> - if (!mga_vid_find_card())
> + else
> {
> - printk(KERN_ERR "mga_vid: no supported devices found\n");
> - unregister_chrdev(major, "mga_vid");
> - return -EINVAL;
> + return -EFAULT;
> }
> -#ifdef CONFIG_DEVFS_FS
> - else {
> - // we assume that this always succeedes
> - dev_handle = devfs_register(NULL, "mga_vid", DEVFS_FL_AUTO_DEVNUM,
> - 0,0,
> - S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IFCHR,
> - &mga_vid_fops, mga_cards[0]);
> - }
> -#endif
> -
> - return(0);
> }
>
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
> +module_init(mga_vid_initialize);
> +#else
> int init_module(void)
> {
> return mga_vid_initialize();
> }
> +#endif
>
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
> +static void mga_cleanup_module(void)
> +#else
> void cleanup_module(void)
> +#endif
What is the sense of this #if ? it's a static function and thus
only visible in this file anyways. Beside you pass the address
of the function to the cernel by module_exit() anyways.
> {
> int i;
> mga_card_t * card;
>
> + printk(KERN_INFO "mga_vid: Cleaning up module\n");
> for (i = 0; i < MGA_MAX_CARDS; i++)
> {
> card = mga_cards[i];
> if(card)
> {
> #ifdef MGA_ALLOW_IRQ
> - if (card->irq != -1)
> - free_irq(card->irq, &(card->irq));
> + if ( card->irq != -1)
> +// free_irq(card->irq, &(card->irq));
> + free_irq(card->irq, card);
If free_irq changed with the 2.6 kernel, then you need here an other
#if. Although i must admit, that the previous version looks bogus.
Have to check that.
> #endif
>
> if(card->mmio_base)
> iounmap(card->mmio_base);
> if(card->param_buff)
> kfree(card->param_buff);
> -#ifdef CONFIG_DEVFS_FS
> +#ifdef CONFIG_DEVFS_FS
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
> + devfs_remove("video/%s%d",MODULENAME, i);
> +#else
> if(card->dev_handle) devfs_unregister(card->dev_handle);
> #endif
> +#endif
>
> kfree(card);
> mga_cards[i]=NULL;
Do you want to continue with this patch or shall i clean it up ?
Attila Kinali
More information about the MPlayer-dev-eng
mailing list