Re: [PATCH 11/13] viafb: viafbdev.c, viafbdev.h
- Date: Mon, 30 Jun 2008 21:46:19 +0200
- From: Jiri Slaby <jirislaby@xxxxxxxxx>
- Subject: Re: [PATCH 11/13] viafb: viafbdev.c, viafbdev.h
On 06/30/2008 09:46 AM, JosephChan@xxxxxxxxxx wrote:
Initialization, remove and some other important functions of viafb.
Signed-off-by: Joseph Chan <josephchan@xxxxxxxxxx>
diff -Nur a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
--- a/drivers/video/via/viafbdev.c 1970-01-01 08:00:00.000000000 +0800
+++ b/drivers/video/via/viafbdev.c 2008-06-30 08:53:33.000000000 +0800
@@ -0,0 +1,2659 @@
[...]
+#ifdef MODULE
+#include <linux/module.h>
+#endif
don't ifdef it
+#define _MASTER_FILE
[...]
+static void get_panel_max_scal_size(struct _panel_size_pos_info
+ *p_max_size)
+{
+ switch (p_max_size->device_type) {
+ case DVI_Device:
+ p_max_size->x = p_max_size->y = 0;
+ break;
+ default:
+ p_max_size->x = p_max_size->y = 0;
+ }
+}
+static void get_panel_max_scal_pos(struct _panel_size_pos_info *p_para)
+{
+ switch (p_para->device_type) {
+ case DVI_Device:
+ p_para->x = p_para->y = 0;
+ break;
+ default:
+ p_para->x = p_para->y = 0;
+ }
+}
+
+static void get_panel_scal_pos(struct _panel_size_pos_info *p_para)
+{
+ switch (p_para->device_type) {
+ case DVI_Device:
+ p_para->x = p_para->y = 0;
+ break;
+ default:
+ p_para->x = p_para->y = 0;
+ }
+}
+static void get_panel_scal_size(struct _panel_size_pos_info *p_para)
+{
+ switch (p_para->device_type) {
+ case DVI_Device:
+ p_para->x = p_para->y = 0;
+ break;
+ default:
+ p_para->x = p_para->y = 0;
+ }
why all the swicthes?
+}
+
+static void set_panel_scal_pos(struct _panel_size_pos_info *p_para)
+{
+ switch (p_para->device_type) {
+ case DVI_Device:
+ break;
+ default:
+ ;
+ }
+}
+
+static void set_panel_scal_size(struct _panel_size_pos_info *p_para)
+{
+ switch (p_para->device_type) {
+ case DVI_Device:
+ break;
+ default:
+ ;
+ }
Sorry?
+}
[...]
+static int viafb_get_cmap_len(struct fb_var_screeninfo *var)
inline candidate?
+{
+ DEBUG_MSG(KERN_INFO "viafb_get_cmap_len!\n");
+
+ return (var->bits_per_pixel == 8) ? 256 : 16;
+}
[...]
+static int viafb_pan_display(struct fb_var_screeninfo *var,
+ struct fb_info *info)
+{
+ unsigned int offset = 0;
do not init it.
+
+ DEBUG_MSG(KERN_INFO "viafb_pan_display!\n");
+
+ offset = (var->xoffset + (var->yoffset * var->xres)) *
+ var->bits_per_pixel / 16;
+
+ DEBUG_MSG(KERN_INFO "\nviafb_pan_display,offset =%d ", offset);
+
+ viafb_write_reg_mask(0x48, 0x3d4, ((offset >> 24) & 0x3), 0x3);
+ viafb_write_reg_mask(0x34, 0x3d4, ((offset >> 16) & 0xff), 0xff);
+ viafb_write_reg_mask(0x0c, 0x3d4, ((offset >> 8) & 0xff), 0xff);
+ viafb_write_reg_mask(0x0d, 0x3d4, (offset & 0xff), 0xff);
+
+ return 0;
+}
[...]
+static int viafb_ioctl(struct fb_info *info, u_int cmd, u_long arg)
+{
I haven't measured this, but how big are these variables?
4*256 + plenty of non-pointers seems like stack-unfriendly code.
+ struct viafb_ioctl_info viainfo;
+ struct viafb_ioctl_mode viamode;
+ struct viafb_ioctl_samm viasamm;
+ struct viafb_driver_version driver_version;
+ struct fb_var_screeninfo sec_var;
+ struct _panel_size_pos_info panel_pos_size_para;
+ u32 state_info = 0;
+ u32 viafb_gamma_table[256];
+ char driver_name[10] = "viafb\0";
the nul-termination is implicit
+
+ u32 __user *argp = (u32 __user *) arg;
+ u32 gpu32 = 0, ss;
+ u32 video_dev_info = 0;
+ struct viafb_ioctl_setting viafb_setting;
+ struct device_t active_dev;
you can use "= { }" instead of memsets
+ ss = sizeof(active_dev);
+ memset(&active_dev, 0, ss);
+ memset(&viafb_setting, 0, sizeof(viafb_setting));
+
+ DEBUG_MSG(KERN_INFO "viafb_ioctl: 0x%X !!\n", cmd);
+
This should be unlocked_ioctl ready, I suppose.
+ switch (cmd) {
+ case VIAFB_GET_CHIP_INFO: /*struct chip_information chip_info ; */
+ if (copy_to_user((void __user *)arg, viaparinfo->chip_info,
some bad alignment, use argp
+ sizeof(struct chip_information)))
+ return -EFAULT;
+ break;
+ case VIAFB_GET_INFO_SIZE:
+ return put_user(sizeof(viainfo), argp);
+ case VIAFB_GET_INFO:
+ return viafb_ioctl_get_viafb_info(arg);
+ case VIAFB_HOTPLUG:
+ return put_user((u32)
cast unneeded
+ viafb_ioctl_hotplug(info->var.xres,
+ info->var.yres,
+ info->var.bits_per_pixel), argp);
+ break;
break unneeded
+ case VIAFB_SET_HOTPLUG_FLAG:
+ if (copy_from_user(&gpu32, argp, sizeof(gpu32)))
+ return -EFAULT;
+ via_fb_hotplug = (gpu32) ? 1 : 0;
+ break;
+ case VIAFB_GET_RESOLUTION:
+ viamode.xres = (u32) via_fb_hotplug_Xres;
+ viamode.yres = (u32) via_fb_hotplug_Yres;
+ viamode.refresh = (u32) via_fb_hotplug_refresh;
+ viamode.bpp = (u32) via_fb_hotplug_bpp;
+ if (viafb_SAMM_ON == 1) {
+ viamode.xres_sec = viafb_second_xres;
+ viamode.yres_sec = viafb_second_yres;
+ viamode.virtual_xres_sec = viafb_second_virtual_xres;
+ viamode.virtual_yres_sec = viafb_second_virtual_yres;
+ viamode.refresh_sec = viafb_refresh1;
+ viamode.bpp_sec = via_fb_bpp1;
+ } else {
+ viamode.xres_sec = 0;
+ viamode.yres_sec = 0;
+ viamode.virtual_xres_sec = 0;
+ viamode.virtual_yres_sec = 0;
+ viamode.refresh_sec = 0;
+ viamode.bpp_sec = 0;
+ }
+ if (copy_to_user((void __user *)arg,
argp
+ &viamode, sizeof(viamode)))
+ return -EFAULT;
+ break;
+ case VIAFB_GET_SAMM_INFO:
+ viasamm.samm_status = viafb_SAMM_ON;
+
+ if (viafb_SAMM_ON == 1) {
+ if (viafb_dual_fb) {
+ viasamm.size_prim = viaparinfo->fbmem_free;
+ viasamm.size_sec = viaparinfo1->fbmem_free;
+ } else {
+ if (viafb_second_size) {
+ viasamm.size_prim =
+ viaparinfo->fbmem_free -
+ viafb_second_size * 1024 * 1024;
+ viasamm.size_sec =
+ viafb_second_size * 1024 * 1024;
+ } else {
+ viasamm.size_prim =
+ viaparinfo->fbmem_free >> 1;
+ viasamm.size_sec =
+ (viaparinfo->fbmem_free >> 1);
+ }
+ }
+ viasamm.mem_base = viaparinfo->fbmem;
+ viasamm.offset_sec = viafb_second_offset;
+ } else {
+ viasamm.size_prim =
+ viaparinfo->memsize - viaparinfo->fbmem_used;
+ viasamm.size_sec = 0;
+ viasamm.mem_base = viaparinfo->fbmem;
+ viasamm.offset_sec = 0;
+ }
+
+ if (copy_to_user((void __user *)arg,
...
+ &viasamm, sizeof(viasamm)))
+ return -EFAULT;
+
+ break;
+ case VIAFB_TURN_ON_OUTPUT_DEVICE:
+ if (copy_from_user(&gpu32, argp, sizeof(gpu32)))
+ return -EFAULT;
+ if (gpu32 & CRT_Device)
+ viafb_crt_enable();
+ if (gpu32 & DVI_Device)
+ viafb_dvi_enable();
+ if (gpu32 & LCD_Device)
+ viafb_lcd_enable();
+ break;
+ case VIAFB_TURN_OFF_OUTPUT_DEVICE:
+ if (copy_from_user(&gpu32, argp, sizeof(gpu32)))
+ return -EFAULT;
+ if (gpu32 & CRT_Device)
+ viafb_crt_disable();
+ if (gpu32 & DVI_Device)
+ viafb_dvi_disable();
+ if (gpu32 & LCD_Device)
+ viafb_lcd_disable();
+ break;
+ case VIAFB_SET_DEVICE:
+ if (copy_from_user(&active_dev, (void *)argp, ss))
sparse unfriendly cast
+ return -EFAULT;
+ viafb_set_device(active_dev);
+ viafb_set_par(info);
+ break;
+ case VIAFB_GET_DEVICE:
+ active_dev.crt = viafb_CRT_ON;
+ active_dev.dvi = viafb_DVI_ON;
+ active_dev.lcd = viafb_LCD_ON;
+ active_dev.samm = viafb_SAMM_ON;
+ active_dev.primary_dev = viafb_primary_dev;
+
+ active_dev.lcd_dsp_cent = viafb_lcd_dsp_method;
+ active_dev.lcd_panel_id = viafb_lcd_panel_id;
+ active_dev.lcd_mode = viafb_lcd_mode;
+
+ active_dev.xres = via_fb_hotplug_Xres;
+ active_dev.yres = via_fb_hotplug_Yres;
+
+ active_dev.xres1 = viafb_second_xres;
+ active_dev.yres1 = viafb_second_yres;
+
+ active_dev.bpp = via_fb_bpp;
+ active_dev.bpp1 = via_fb_bpp1;
+ active_dev.refresh = viafb_refresh;
+ active_dev.refresh1 = viafb_refresh1;
+
+ active_dev.epia_dvi = viafb_platform_epia_dvi;
+ active_dev.lcd_dual_edge = viafb_device_lcd_dualedge;
+ active_dev.bus_width = via_bus_width;
+
+ if (copy_to_user((void __user *)arg, &active_dev, ss))
jsut argp
+ return -EFAULT;
+ break;
+
+ case VIAFB_GET_DRIVER_VERSION:
+ driver_version.iMajorNum = VERSION_MAJOR;
+ driver_version.iKernelNum = VERSION_KERNEL;
+ driver_version.iOSNum = VERSION_OS;
+ driver_version.iMinorNum = VERSION_MINOR;
+
+ if (copy_to_user
+ ((void __user *)arg, &driver_version,
... and so on
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
[...]
+static u8 is_duoview(void)
+{
+ if (0 == viafb_SAMM_ON) {
+ if (viafb_LCD_ON + viafb_LCD2_ON +
+ viafb_DVI_ON + viafb_CRT_ON == 2)
+ return TRUE;
+ return FALSE;
+ } else {
+ return FALSE;
+ }
use already defined true and false.
+}
[...]
+static int parse_port(char *opt_str, int *output_interface)
+{
+ if (!strncmp(opt_str, "DVP0", 4))
+ *output_interface = INTERFACE_DVP0;
+ else if (!strncmp(opt_str, "DVP1", 4))
+ *output_interface = INTERFACE_DVP1;
+ else if (!strncmp(opt_str, "DFP_HIGHLOW", 11))
+ *output_interface = INTERFACE_DFP;
+ else if (!strncmp(opt_str, "DFP_HIGH", 8))
+ *output_interface = INTERFACE_DFP_HIGH;
+ else if (!strncmp(opt_str, "DFP_LOW", 8))
7?
+ *output_interface = INTERFACE_DFP_LOW;
+ else
+ *output_interface = INTERFACE_NONE;
+ return 0;
+}
[...]
+static int __devinit via_pci_probe(void)
+{
+
+ /*unsigned char revision; */
+ unsigned int default_xres, default_yres;
+ char *tmpc, *tmpm;
+ char *tmpc_sec, *tmpm_sec;
+ int vmode_index;
+ u32 tmds_length, lvds_length, crt_length, chip_length, viafb_par_length;
+
+ DEBUG_MSG(KERN_INFO "VIAFB PCI Probe!!\n");
+
+#define BYTES_PER_LONG (BITS_PER_LONG/8)
+#define PADDING(A) (BYTES_PER_LONG - (sizeof(A) % BYTES_PER_LONG))
Is this ALIGN() reimplementation?
+ viafb_par_length = sizeof(struct viafb_par) + PADDING(struct viafb_par);
+ tmds_length = sizeof(struct tmds_setting_information) +
+ PADDING(struct tmds_setting_information);
+ lvds_length = sizeof(struct lvds_setting_information) +
+ PADDING(struct lvds_setting_information);
+ crt_length = sizeof(struct crt_setting_information) +
+ PADDING(struct crt_setting_information);
+ chip_length = sizeof(struct chip_information) +
+ PADDING(struct chip_information);
+#undef PADDING
+#undef BYTES_PER_LONG
[...]
+static void __devexit via_pci_remove(void)
+{
+ DEBUG_MSG(KERN_INFO "via_pci_remove!\n");
+ fb_dealloc_cmap(&viafbinfo->cmap);
+ unregister_framebuffer(viafbinfo);
+ if (viafb_dual_fb)
+ unregister_framebuffer(viafbinfo1);
+ iounmap((void *)viaparinfo->fbmem_virt);
I suppose io_virt is unmapped somewhere else?
+
+ framebuffer_release(viafbinfo);
+ if (viafb_dual_fb)
+ framebuffer_release(viafbinfo1);
+
+ viafb_remove_proc(viaparinfo->proc_entry);
+}
+
+int __init viafb_init(void)
static
+{
+ DEBUG_MSG(KERN_INFO "viafb_init!\n");
Is this useful?
+ printk(KERN_INFO
+ "VIA Graphics Intergration Chipset framebuffer %d.%d initializing\n",
+ VERSION_MAJOR, VERSION_MINOR);
+#ifndef MODULE
+ char *option = NULL;
mixing decl and code. put this code into { }
+ if (fb_get_options("viafb", &option))
+ return -ENODEV;
+ viafb_setup(option);
+#endif
+ return via_pci_probe();
+}
+
+void __exit viafb_exit(void)
+{
+ DEBUG_MSG(KERN_INFO "viafb_exit!\n");
+ if (timer_on) {
+ del_timer(&timer_for3D);
del_timer_sync. You don't need to test for timer_on, do you?
+ timer_on = 0;
+ }
+ via_pci_remove();
+}
+
+int __init viafb_setup(char *options)
static
+{
+ char *this_opt;
+ DEBUG_MSG(KERN_INFO "viafb_setup!\n");
+
+ if (!options || !*options)
+ return 0;
+
+ while ((this_opt = strsep(&options, ",")) != NULL) {
+ if (!*this_opt)
+ continue;
+
+ if (!strncmp(this_opt, "viafb_mode=", 11)) {
+ viafb_mode = kmalloc(strlen(this_opt + 5), GFP_KERNEL);
+ strcpy(viafb_mode, this_opt + 11);
kstrdup + checks for NULL and so further...
+ } else if (!strncmp(this_opt, "viafb_mode1=", 12)) {
+ viafb_mode1 = kmalloc(strlen(this_opt + 11),
+ GFP_KERNEL);
+ strcpy(viafb_mode1, this_opt + 12);
+ } else if (!strncmp(this_opt, "via_fb_bpp=", 11))
+ strict_strtoul(this_opt + 11, 0,
+ (unsigned long *)&via_fb_bpp);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/