From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anatolij Gustschin Date: Sun, 07 Dec 2008 01:33:04 +0100 Subject: [U-Boot] [PATCH] video: Add new driver for Silicon Motion SM501/SM502 Part 2/2 In-Reply-To: <4938472f.fLMN/1P3p+J9eQgM%stefan.althoefer@web.de> References: <4938472f.fLMN/1P3p+J9eQgM%stefan.althoefer@web.de> Message-ID: <493B19C0.4040402@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Stefan, Stefan Althoefer wrote: > [PATCH] video: Add new driver for Silicon Motion SM501/SM502 > > This patch adds a new driver for SM501/SM502. Compared to the > existing driver it allows dynamic selection of resolution > (environment: videomode). > > The drive is based on Vincent Sanders and Ben Dooks' linux > kernel driver. > > Use CONFIG_VIDEO_SM501NEW to enable the driver. > > This has been tested on Janz emPC-A400. On this platform > the SM501 is connected via PCI. > > > The patch is against "latest" u-boot git-repository > > Please (still) be patient if style of submission or patches are > offending. > > Signed-off-by: Stefan Althoefer > ---- see patch header comments in previous email, same applies here. > diff -uprN u-boot-orig//drivers/video/sm501new.c u-boot/drivers/video/sm501new.c > --- u-boot-orig//drivers/video/sm501new.c 1970-01-01 01:00:00.000000000 +0100 > +++ u-boot/drivers/video/sm501new.c 2008-12-03 11:47:22.000000000 +0100 > @@ -0,0 +1,1533 @@ > +/* Large parts of this have been taken from the linux kernel source code > + * with the following licencse: > + * > + * Copyright (c) 2006 Simtec Electronics > + * Vincent Sanders > + * Ben Dooks > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Framebuffer driver for the Silicon Motion SM501 > + */ > + > +/* Ported to u-boot: > + * > + * Copyright (c) 2008 StefanAlthoefer (as at janz.de) > + */ > + > + > +#include > +#include is exports.h needed here? > +#include > +#include is watchdog.h really needed? > +#include > +#include image.h ? > +#include > +#include > +#include > +#include > +#include "videomodes.h" > +#include "sm501new-regs.h" > +#include "sm501new.h" > + > + > +#ifdef CONFIG_VIDEO_SM501NEW please, remove above #ifdef and corresponding #endif. Conditional compiling is already handled by Makefile. > +#undef DEBUG > + > +/* this should be in pci_ids.h */ > +#define PCI_DEVICE_ID_SMI_501 0x0501 please, remove above comment and move PCI_DEVICE_ID_SMI_501 definition to include/pci_ids.h > +#define BIG_ENDIAN_HOST > +#define VIDEO_MEM_SIZE (4*1024*1024) > + > +#define SM501_FRAMEBUFFER_ADDR 0 > + > +#define HEAD_CRT 0 > +#define HEAD_PANEL 1 > + > +#if defined(BIG_ENDIAN_HOST) > +static inline unsigned int LONGSWAP(unsigned int x) > +{ > + return ( > + ((x<<24) & 0xff000000) | > + ((x<< 8) & 0x00ff0000) | > + ((x>> 8) & 0x0000ff00) | > + ((x>>24) & 0x000000ff) ); > +} > +static inline unsigned int readl(void *addr) > +{ > + return LONGSWAP((*(volatile unsigned int *)(addr))); > +} > +static inline void writel(unsigned int data, void *addr) > +{ > + /*printf("%p <- %x\n", addr, data); */ > + *(volatile unsigned int *)(addr) = LONGSWAP(data); > +} > +#else > +#define readl(addr) (*(volatile unsigned int*)(addr)) > +#define writel(b,addr) ((*(volatile unsigned int *) (addr)) = (b)) > +#endif please, use existing code for readl, writel and byte swapping. > +struct sm501_devdata { > + struct sm501_platdata *platdata; > + unsigned long pm_misc; > + int unit_power[20]; > + unsigned int pdev_id; > + unsigned int irq; > + void *regs; > + void *dc; > + void *vmem; pm_misc, pdev_id and irq are not used in the code, so remove them. > + GraphicDevice *gd; gd is asigned but not dereferenced in the code, another candidate to remove? > + struct ctfb_res_modes *mode; > + int bits_per_pixel; > + int xres_virtual; > + int yres_virtual; > +}; > + > +struct sm501_devdata smi_devd; > + > + > +static void mdelay(unsigned int delay) > +{ > + while( delay-- > 0 ){ > + udelay(1000); > + } > +} > + > +#define MHZ (1000 * 1000) > + > + > +#ifdef DEBUG > +#define dev_dbg(XXX,...) printf(__VA_ARGS__) > +#define dev_info(XXX,...) printf(__VA_ARGS__) > +#define dev_err(XXX,...) printf(__VA_ARGS__) please, use existing debug() macro for debugging purposes, it's in include/common.h. Debug is debug, info is info, error is error. It is a good idea to report errors unconditionally, I mean independent of DEBUG definition. Errors should always be reported, so I suggest using printf() for error messages. Many dev_info() calls in this driver should be changed to debug() calls, see appropriate comments below. For some short info messages (chip version info, video memory size) printf() is enough, I think. Also, first argument isn't used, so simply drop it. > +static void sm501_dump_regs(struct sm501_devdata *sm) > +{ sm501_dump_regs() is defined but not used. Please, use it or drop it entirely. > + void *regs = sm->regs; > + > + dev_info(sm->dev, "System Control %08x\n", > + readl(regs + SM501_SYSTEM_CONTROL)); Actually, this is debug output, so please use debug() macro. This comment applies if sm501_dump_regs() is going to be used. > +static void sm501_dump_gate(struct sm501_devdata *sm) > +{ > + dev_info(sm->dev, "CurrentGate %08x\n", > + readl(sm->regs + SM501_CURRENT_GATE)); > + dev_info(sm->dev, "CurrentClock %08x\n", > + readl(sm->regs + SM501_CURRENT_CLOCK)); > + dev_info(sm->dev, "PowerModeControl %08x\n", > + readl(sm->regs + SM501_POWER_MODE_CONTROL)); please, use debug() instead of dev_info() here. > +static inline void sm501_mdelay(struct sm501_devdata *sm, unsigned int delay) > +{ > + mdelay(delay); > +} please, drop sm501_mdelay() and simply use mdelay(). > +unsigned long sm501_gpio_get(struct sm501_devdata *sm, > + unsigned long gpio) > +{ unused code, please remove it. > +void sm501_gpio_set(struct sm501_devdata *sm, > + unsigned long gpio, > + unsigned int to, > + unsigned int dir) > +{ ditto. > +/* sm501_init_dev > + * > + * Common init code for an SM501 > +*/ > + > +static int sm501_init_dev(struct sm501_devdata *sm) > +{ > + unsigned long mem_avail; > + unsigned long dramctrl; > + unsigned long devid; > + int ret; > + > + devid = readl(sm->regs + SM501_DEVICEID); > + > + if ((devid & SM501_DEVICEID_IDMASK) != SM501_DEVICEID_SM501) { > + dev_err(sm->dev, "incorrect device id %08lx\n", devid); printf() for error messages is ok. > + return -1; > + } > + > + dramctrl = readl(sm->regs + SM501_DRAM_CONTROL); > + mem_avail = sm501_mem_local[(dramctrl >> 13) & 0x7]; > + > + dev_info(sm->dev, "SM501 At %p: Version %08lx, %ld Mb, IRQ %d\n", > + sm->regs, devid, (unsigned long)mem_avail >> 20, sm->irq); IRQ doesn't make sense here, so I suggest to drop it. Also use debug() instead of dev_info(). > + sm501_dump_gate(sm); > + sm501_dump_clk(sm); > + > + /* check to see if we have some device initialisation */ > + > + if (sm->platdata) { > + struct sm501_platdata *pdata = sm->platdata; > + > + if (pdata->init) { > + sm501_init_regs(sm, sm->platdata->init); > + } > + } > + > + ret = sm501_check_clocks(sm); > + if (ret) { > + dev_err(sm->dev, "M1X and M clocks sourced from different " > + "PLLs\n"); > + return -1; > + } > + > + > + return 0; > +} > + > + > + > + > + > + > + > + > + > + please remove empty lines, max. 2 empty lines allowed. > +/* Initialisation data for PCI devices */ > + > + > +static struct sm501_initdata sm501_pci_initdata = { > + .gpio_high = { > + .set = 0x3F000000, /* 24bit panel */ > + .mask = 0x0, > + }, > + .misc_timing = { > + .set = 0x010100, /* SDRAM timing */ > + .mask = 0x1F1F00, > + }, > + .misc_control = { > + .set = SM501_MISC_PNL_24BIT, > + .mask = 0, > + }, > + > + .devices = SM501_USE_ALL, "devices" isn't used in the code, a candidate to remove? > + > + /* Errata AB-3 says that 72MHz is the fastest available > + * for 33MHZ PCI with proper bus-mastering operation */ > + > + .mclk = 72 * MHZ, > + .m1xclk = 144 * MHZ, > +}; > + > +static struct sm501_platdata_fbsub sm501_pdata_fbsub = { > + .flags = (SM501FB_FLAG_USE_INIT_MODE | > + SM501FB_FLAG_USE_HWCURSOR | > + SM501FB_FLAG_USE_HWACCEL | > + SM501FB_FLAG_DISABLE_AT_EXIT), > +}; > + > +static struct sm501_platdata_fb sm501_fb_pdata = { > + .fb_route = SM501_FB_OWN, > + .fb_crt = &sm501_pdata_fbsub, > + .fb_pnl = &sm501_pdata_fbsub, > +}; > + > +static struct sm501_platdata sm501_pci_platdata = { > + .init = &sm501_pci_initdata, > + .fb = &sm501_fb_pdata, > +}; sm501_fb_pdata isn't referenced in the code, so please remove ".fb = &sm501_fb_pdata," in the sm501_pci_platdata declaration. Also drop sm501_fb_pdata and sm501_pdata_fbsub declarations above. > + > + > + max. 2 empty lines. > +static int sm501fb_set_par_common(struct sm501_devdata *sm, int head) > +{ > + struct ctfb_res_modes *var = sm->mode; > + unsigned long pixclock; /* pixelclock in Hz */ > + unsigned long sm501pixclock; /* pixelclock the 501 can achive in Hz */ > + /*unsigned int mem_type;*/ > + unsigned int clock_type; > + unsigned int head_addr; > + > + dev_dbg(fbi->dev, "%s: %dx%d, bpp = %d, virtual %dx%d\n", > + __func__, var->xres, var->yres, sm->bits_per_pixel, > + sm->xres_virtual, sm->yres_virtual); > + > + switch (head) { > + case HEAD_CRT: > + /*mem_type = SM501_MEMF_CRT;*/ > + clock_type = SM501_CLOCK_V2XCLK; > + head_addr = SM501_DC_CRT_FB_ADDR; > + break; > + > + case HEAD_PANEL: > + /*mem_type = SM501_MEMF_PANEL;*/ > + clock_type = SM501_CLOCK_P2XCLK; > + head_addr = SM501_DC_PANEL_FB_ADDR; > + break; > + > + default: > + /*mem_type = 0;*/ /* stop compiler warnings */ > + head_addr = 0; > + clock_type = 0; > + } > + > +#if 0 > + switch (sm->bits_per_pixel) { > + case 8: > + info->fix.visual = FB_VISUAL_PSEUDOCOLOR; > + break; > + > + case 16: > + info->fix.visual = FB_VISUAL_DIRECTCOLOR; > + break; > + > + case 32: > + info->fix.visual = FB_VISUAL_TRUECOLOR; > + break; > + } > + > + /* allocate fb memory within 501 */ > + info->fix.line_length = (var->xres_virtual * var->bits_per_pixel)/8; > + info->fix.smem_len = info->fix.line_length * var->yres_virtual; > + > + dev_dbg(fbi->dev, "%s: line length = %u\n", __func__, > + info->fix.line_length); > + > + if (sm501_alloc_mem(fbi, &par->screen, mem_type, > + info->fix.smem_len)) { > + dev_err(fbi->dev, "no memory available\n"); > + return -ENOMEM; > + } > + > + info->fix.smem_start = fbi->fbmem_res->start + par->screen.sm_addr; > + > + info->screen_base = fbi->fbmem + par->screen.sm_addr; > + info->screen_size = info->fix.smem_len; > +#endif please remove unused code (between #if 0 and #endif). > +/* sm501fb_pan_crt > + * > + * pan the CRT display output within an virtual framebuffer > +*/ > +#if 0 > +static int sm501fb_pan_crt(struct sm501_devdata *sm) > +{ > + struct ctfb_res_modes *var = sm->mode; > + unsigned int bytes_pixel = sm->bits_per_pixel / 8; > + unsigned long reg; > + unsigned long xoffs; > + > + xoffs = /*var->xoffset*/0 * bytes_pixel; > + > + reg = readl(sm->dc + SM501_DC_CRT_CONTROL); > + > + reg &= ~SM501_DC_CRT_CONTROL_PIXEL_MASK; > + reg |= ((xoffs & 15) / bytes_pixel) << 4; > + writel(reg, sm->dc + SM501_DC_CRT_CONTROL); > + > + reg = (par->screen.sm_addr + xoffs + > + /*var->yoffset*/0 * info->fix.line_length); > + writel(reg | SM501_ADDR_FLIP, fbi->dc + SM501_DC_CRT_FB_ADDR); > + > + sm501fb_sync_regs(sm); > + return 0; > +} > +#endif unused code, please remove. > + > +/* sm501fb_pan_pnl > + * > + * pan the panel display output within an virtual framebuffer > +*/ > +static int sm501fb_pan_pnl(struct sm501_devdata *sm) > +{ > + /*struct ctfb_res_modes *var = sm->mode;*/ drop above unused code. > + unsigned long reg; > + > + reg = /*var->xoffset*/0 | (sm->xres_virtual << 16); remove /*var->xoffset*/. > + writel(reg, sm->dc + SM501_DC_PANEL_FB_WIDTH); > + > + reg = /*var->yoffset*/0 | (sm->yres_virtual << 16); Ditto. > + writel(reg, sm->dc + SM501_DC_PANEL_FB_HEIGHT); > + > + sm501fb_sync_regs(sm); > + return 0; > +} > + > +/* sm501fb_set_par_crt > + * > + * Set the CRT video mode from the fb_info structure > +*/ > + > +static int sm501fb_set_par_crt(struct sm501_devdata *sm) > +{ > + struct ctfb_res_modes *var = sm->mode; > + unsigned long control; /* control register */ > + int ret; > + > + /* activate new configuration */ > + > + dev_dbg(fbi->dev, "%s(%p)\n", __func__, sm); > + > + /* enable CRT DAC - note 0 is on!*/ > + sm501_misc_control(sm, 0, SM501_MISC_DAC_POWER); > + > + control = readl(sm->dc + SM501_DC_CRT_CONTROL); > + dev_dbg(fbi->dev, "old control is %08lx\n", control); > + > + control &= (SM501_DC_CRT_CONTROL_PIXEL_MASK | > + SM501_DC_CRT_CONTROL_GAMMA | > + SM501_DC_CRT_CONTROL_BLANK | > + SM501_DC_CRT_CONTROL_SEL | > + SM501_DC_CRT_CONTROL_CP | > + SM501_DC_CRT_CONTROL_TVP); > + > + /* set the sync polarities before we check data source */ > + > + if ((var->sync & FB_SYNC_HOR_HIGH_ACT) == 0) > + control |= SM501_DC_CRT_CONTROL_HSP; > + > + if ((var->sync & FB_SYNC_VERT_HIGH_ACT) == 0) > + control |= SM501_DC_CRT_CONTROL_VSP; > + > + if ((control & SM501_DC_CRT_CONTROL_SEL) == 0) { > + /* the head is displaying panel data... */ > + > + dev_dbg(fbi->dev, "%s CRT display panel data\n", __func__); > + /*sm501_alloc_mem(fbi, &par->screen, SM501_MEMF_CRT, 0);*/ drop commented out sm501_alloc_mem code (elsewhere in the file too). > +/* sm501fb_set_par_pnl > + * > + * Set the panel video mode from the fb_info structure > +*/ > + > +static int sm501fb_set_par_pnl(struct sm501_devdata *sm) > +{ > + struct ctfb_res_modes *var = sm->mode; > + unsigned long control; > + unsigned long reg; > + int ret; > + > + dev_dbg(fbi->dev, "%s(%p)\n", __func__, sm); > + > + /* activate this new configuration */ > + > + ret = sm501fb_set_par_common(sm, HEAD_PANEL); > + if (ret) > + return ret; > + > + sm501fb_pan_pnl(sm); > + sm501fb_set_par_geometry(sm, HEAD_PANEL); > + > + /* update control register */ > + > + control = readl(sm->dc + SM501_DC_PANEL_CONTROL); > + control &= (SM501_DC_PANEL_CONTROL_GAMMA | > + SM501_DC_PANEL_CONTROL_VDD | > + SM501_DC_PANEL_CONTROL_DATA | > + SM501_DC_PANEL_CONTROL_BIAS | > + SM501_DC_PANEL_CONTROL_FPEN | > + SM501_DC_PANEL_CONTROL_CP | > + SM501_DC_PANEL_CONTROL_CK | > + SM501_DC_PANEL_CONTROL_HP | > + SM501_DC_PANEL_CONTROL_VP | > + SM501_DC_PANEL_CONTROL_HPD | > + SM501_DC_PANEL_CONTROL_VPD); > + > + control |= SM501_FIFO_3; /* fill if >3 free slots */ > + > + switch(sm->bits_per_pixel) { > + case 8: > + control |= SM501_DC_PANEL_CONTROL_8BPP; > + break; > + > + case 16: > + control |= SM501_DC_PANEL_CONTROL_16BPP; > + break; > + > + case 32: > + control |= SM501_DC_PANEL_CONTROL_32BPP; > + /*sm501fb_setup_gamma(fbi, SM501_DC_PANEL_PALETTE);*/ drop commented out sm501fb_setup_gamma, also elsewhere in the file. > + break; > + > + default: > + dev_dbg(fbi->dev, "unkown pixel format\n"); > + } > + > + writel(0x0, sm->dc + SM501_DC_PANEL_PANNING_CONTROL); > + > + /* panel plane top left and bottom right location */ > + > + writel(0x00, sm->dc + SM501_DC_PANEL_TL_LOC); > + > + reg = var->xres - 1; > + reg |= (var->yres - 1) << 16; > + > + writel(reg, sm->dc + SM501_DC_PANEL_BR_LOC); > + > + /* program panel control register */ > + > + control |= SM501_DC_PANEL_CONTROL_TE; /* enable PANEL timing */ > + control |= SM501_DC_PANEL_CONTROL_EN; /* enable PANEL gfx plane */ > + > + if ((var->sync & FB_SYNC_HOR_HIGH_ACT) == 0) > + control |= SM501_DC_PANEL_CONTROL_HSP; > + > + if ((var->sync & FB_SYNC_VERT_HIGH_ACT) == 0) > + control |= SM501_DC_PANEL_CONTROL_VSP; > + > + writel(control, sm->dc + SM501_DC_PANEL_CONTROL); > + sm501fb_sync_regs(sm); > + > + /* power the panel up */ > + sm501fb_panel_power(sm, 1); > + return 0; > +} > + > +void video_set_lut( > + unsigned int index, /* color number */ > + unsigned char r, /* red */ > + unsigned char g, /* green */ > + unsigned char b /* blue */ > + ) > +{ > + /* FIXME: to be done */ > +} > + > +/******************************************************************************* > + * > + * Init video chip with common Linux graphic modes (lilo) > + */ > +void *video_hw_init(void) > +{ > + GraphicDevice *pGD = (GraphicDevice *)&smi; > + unsigned short device_id; > + pci_dev_t devbusfn; > + int videomode; > + unsigned long t1, hsynch, vsynch; > + unsigned int pci_mem_base, *vm; > + unsigned int pci_reg_base; > + char *penv; > + int tmp, i, bits_per_pixel; > + int vmem_size; > + struct ctfb_res_modes *res_mode; > + struct ctfb_res_modes var_mode; > + > + /* Search for video chip */ > + printf("Video: "); > + > + if ((devbusfn = pci_find_devices(supported, 0)) < 0) > + { > + printf ("Controller not found !\n"); > + return (NULL); > + } > + > + /* PCI setup */ > + pci_write_config_dword (devbusfn, PCI_COMMAND, (PCI_COMMAND_MEMORY | PCI_COMMAND_IO)); > + pci_read_config_word (devbusfn, PCI_DEVICE_ID, &device_id); > + pci_read_config_dword (devbusfn, PCI_BASE_ADDRESS_0, &pci_mem_base); > + /*pci_mem_base = pci_mem_to_phys (devbusfn, pci_mem_base);*/ > + pci_read_config_dword (devbusfn, PCI_BASE_ADDRESS_1, &pci_reg_base); > + /*pci_reg_base = pci_mem_to_phys (devbusfn, pci_reg_base);*/ > + > + dev_dbg(xxx, "mem_base=0x%x reg_base=0x%x\n", pci_mem_base, pci_reg_base); > + > + tmp = 0; > + > + videomode = CONFIG_SYS_DEFAULT_VIDEO_MODE; > + /* get video mode via environment */ > + if ((penv = getenv ("videomode")) != NULL) { > + /* deceide if it is a string */ > + if (penv[0] <= '9') { > + videomode = (int) simple_strtoul (penv, NULL, 16); > + tmp = 1; > + } > + } else { > + tmp = 1; > + } > + if (tmp) { > + /* parameter are vesa modes */ > + /* search params */ > + for (i = 0; i < VESA_MODES_COUNT; i++) { > + if (vesa_modes[i].vesanr == videomode) > + break; > + } > + if (i == VESA_MODES_COUNT) { > + printf ("no VESA Mode found, switching to mode 0x%x ", CONFIG_SYS_DEFAULT_VIDEO_MODE); above line too long, max. 80 characters please. > + i = 0; > + } > + res_mode = > + (struct ctfb_res_modes *) &res_mode_init[vesa_modes[i]. > + resindex]; > + bits_per_pixel = vesa_modes[i].bits_per_pixel; > + } else { > + > + res_mode = (struct ctfb_res_modes *) &var_mode; > + bits_per_pixel = video_get_params (res_mode, penv); > + } > + > + /* calculate hsynch and vsynch freq (info only) */ > + t1 = (res_mode->left_margin + res_mode->xres + > + res_mode->right_margin + res_mode->hsync_len) / 8; > + t1 *= 8; > + t1 *= res_mode->pixclock; > + t1 /= 1000; > + hsynch = 1000000000L / t1; > + t1 *= > + (res_mode->upper_margin + res_mode->yres + > + res_mode->lower_margin + res_mode->vsync_len); > + t1 /= 1000; > + vsynch = 1000000000L / t1; > + > + /* fill in Graphic device struct */ > + sprintf (pGD->modeIdent, "%dx%dx%d %ldkHz %ldHz", res_mode->xres, > + res_mode->yres, bits_per_pixel, (hsynch / 1000), > + (vsynch / 1000)); > + printf ("%s\n", pGD->modeIdent); > + pGD->winSizeX = res_mode->xres; > + pGD->winSizeY = res_mode->yres; > + pGD->plnSizeX = res_mode->xres; > + pGD->plnSizeY = res_mode->yres; > + switch (bits_per_pixel) { > + case 8: > + pGD->gdfBytesPP = 1; > + pGD->gdfIndex = GDF__8BIT_INDEX; > + break; > + case 15: > + pGD->gdfBytesPP = 2; > + pGD->gdfIndex = GDF_15BIT_555RGB; > + break; > + case 16: > + pGD->gdfBytesPP = 2; > + pGD->gdfIndex = GDF_16BIT_565RGB; > + break; > + case 24: > + pGD->gdfBytesPP = 4; > + pGD->gdfIndex = GDF_32BIT_X888RGB; > + break; > + case 32: > + pGD->gdfBytesPP = 4; > + pGD->gdfIndex = GDF_32BIT_X888RGB; > + break; > + } > + > + pGD->isaBase = 0; > + pGD->pciBase = pci_mem_base; > + pGD->dprBase = pci_reg_base; > + pGD->vprBase = 0; > + pGD->cprBase = 0; > + pGD->frameAdrs = pci_mem_base; > + pGD->memSize = VIDEO_MEM_SIZE; > + > + smi_devd.platdata = &sm501_pci_platdata; > + smi_devd.regs = (void *)pci_reg_base; > + smi_devd.dc = (void *)pci_reg_base + 0x80000; > + smi_devd.vmem = (void *)pci_mem_base; > + smi_devd.gd = pGD; "smi_devd.gd" isn't referenced elsewhere in the code, a candidate to remove? > + smi_devd.mode = res_mode; > + switch (bits_per_pixel) { > + case 8: smi_devd.bits_per_pixel = 8; break; > + case 16: smi_devd.bits_per_pixel = 16; break; > + case 24: smi_devd.bits_per_pixel = 32; break; > + default: smi_devd.bits_per_pixel = 32; break; > + } > + smi_devd.xres_virtual = pGD->plnSizeX; > + smi_devd.yres_virtual = pGD->plnSizeY; > + > + switch( (readl(smi_devd.regs + SM501_DRAM_CONTROL) >> 13) & 0x7 ){ > + case 0: vmem_size = 4*1024*1024; break; > + case 1: vmem_size = 8*1024*1024; break; > + case 2: vmem_size = 16*1024*1024; break; > + case 3: vmem_size = 32*1024*1024; break; > + case 4: vmem_size = 64*1024*1024; break; > + case 5: vmem_size = 2*1024*1024; break; > + default: vmem_size = 2*1024*1024; break; > + } > + printf("SM501: %d MB Video memory\n", vmem_size/(1024*1024)); > + > + /* Blank video memory */ > + vm = (unsigned int *)pGD->frameAdrs; > + for(i=0; i < vmem_size/sizeof(int); i++){ > + *vm++ = 0; > + } > + > + sm501_init_dev(&smi_devd); > + > + /* enable display controller */ > + sm501_unit_power(&smi_devd, SM501_GATE_DISPLAY, 1); > + > +#if 0 > + /* setup cursors */ > + > + sm501_init_cursor(info->fb[HEAD_CRT], SM501_DC_CRT_HWC_ADDR); > + sm501_init_cursor(info->fb[HEAD_PANEL], SM501_DC_PANEL_HWC_ADDR); > +#endif unused code, please remove it. > + > + /* > + * Panel setup > + */ > + sm501fb_set_par_pnl(&smi_devd); > + > + /* > + * CRT setup (we want CRT display panel data) > + */ > + sm501fb_set_par_crt(&smi_devd); > + > + return ((void*)&smi); > +} > + > + > +#if 0 > +int t_smi (int argc, char *argv[]) > +{ > + int i; > + ulong ret; > + ulong t,t1; > + > + app_startup(argv); > + > + pci_init(); > + drv_video_init(); > + > +#if 0 > + video_puts("loading ...\n"); > + video_puts("be patient\n"); > +#endif > + > +#if 0 > + if (argc == 1) { > + /* Print the ABI version */ > + > + printf("Video hw Init\n"); > + pci_init(); > + video_hw_init(); > + > + return 0; > + } else { > + printf("Video Init\n"); > + pci_init(); > + drv_video_init(); > + } > +#endif > +} > +#endif unused code, please drop it. Also check for proper multi-line comment style, please. Best regards, Anatolij -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de