Skip to contentSkip to conversation

video: Add new driver for Silicon Motion SM501/SM502 Part 2/2

PATCH New Stefan Althoefer 2 participants 2 messages Dec 4, 2008 – Dec 7, 2008 Framebuffer layer

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 video: Add new driver for Silicon Motion SM501/SM502 Part 2/2

[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.

----

drivers/video/Makefile renamed from /drivers/video/Makefile +1 -0
diff -uprN u-boot-orig//drivers/video/Makefile u-boot/drivers/video/Makefile
@@ -33,6 +33,7 @@ COBJS-$(CONFIG_VIDEO_MB862xx) += mb862xx
 COBJS-$(CONFIG_VIDEO_SED13806) += sed13806.o
 COBJS-$(CONFIG_SED156X) += sed156x.o
 COBJS-$(CONFIG_VIDEO_SM501) += sm501.o
+COBJS-$(CONFIG_VIDEO_SM501NEW) += sm501new.o
 COBJS-$(CONFIG_VIDEO_SMI_LYNXEM) += smiLynxEM.o
 COBJS-y += videomodes.o
 
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +1533 -0

1,533 lines not on this page · raw message

#1 lore raw

PATCH 1/1 video: Add new driver for Silicon Motion SM501/SM502 Part 2/2

Hello Stefan,

Quoted 15 lines · Stefan Althoefer

[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.

Quoted 7 lines

The patch is against "latest" u-boot git-repository

Please (still) be patient if style of submission or patches are offending.

----

see patch header comments in previous email, same applies here.

<snip>

Quoted 16 lines

diff -uprN u-boot-orig//drivers/video/sm501new.c u-boot/drivers/video/sm501new.c

drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +12 -0
@@ -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 <[email protected]>
+ *      Ben Dooks <[email protected]>
+ *
+ * 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

drivers/video/sm501new.c line 22

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+ */
+
+/* Ported to u-boot:
+ * 
+ * Copyright (c) 2008 StefanAlthoefer (as at janz.de)
+ */
+
+
+#include <common.h>
+#include <exports.h>

is exports.h needed here?

drivers/video/sm501new.c line 24

Quoted 2 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +2 -0
+#include <config.h>
+#include <watchdog.h>

is watchdog.h really needed?

drivers/video/sm501new.c line 26

Quoted 2 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +2 -0
+#include <command.h>
+#include <image.h>

image.h ?

drivers/video/sm501new.c line 36

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+#include <asm/byteorder.h>
+#include <pci.h>
+#include <video_fb.h>
+#include <video.h>
+#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.

drivers/video/sm501new.c line 41

Quoted 4 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +4 -0
+#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

Quoted 20 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +20 -0
+#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)));

drivers/video/sm501new.c line 72

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+}
+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.

<snip>

drivers/video/sm501new.c line 95

Quoted 9 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +9 -0
+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.

drivers/video/sm501new.c line 97

Quoted 1 line
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +1 -0
+	GraphicDevice                    *gd;

gd is asigned but not dereferenced in the code, another candidate to remove?

Quoted 13 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +13 -0
+	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);

drivers/video/sm501new.c line 120

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+	}
+}
+
+#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.

<snip>

drivers/video/sm501new.c line 105

Quoted 2 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +2 -0
+static void sm501_dump_regs(struct sm501_devdata *sm)
+{

sm501_dump_regs() is defined but not used. Please, use it or drop it entirely.

drivers/video/sm501new.c line 247

Quoted 4 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +4 -0
+	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.

<snip>

drivers/video/sm501new.c line 269

Quoted 8 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +8 -0
+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.

<snip>

drivers/video/sm501new.c line 294

Quoted 4 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +4 -0
+static inline void sm501_mdelay(struct sm501_devdata *sm, unsigned int delay)
+{
+	mdelay(delay);
+}

please, drop sm501_mdelay() and simply use mdelay().

<snip>

drivers/video/sm501new.c line 347

Quoted 3 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +3 -0
+unsigned long sm501_gpio_get(struct sm501_devdata *sm,
+			     unsigned long gpio)
+{

unused code, please remove it.

<snip>

drivers/video/sm501new.c line 363

Quoted 5 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +5 -0
+void sm501_gpio_set(struct sm501_devdata *sm,
+		    unsigned long gpio,
+		    unsigned int to,
+		    unsigned int dir)
+{

ditto.

<snip>

Quoted 6 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +6 -0
+/* sm501_init_dev
+ *
+ * Common init code for an SM501
+*/
+
+static int sm501_init_dev(struct sm501_devdata *sm)

drivers/video/sm501new.c line 743

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+{
+	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.

drivers/video/sm501new.c line 751

Quoted 8 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +8 -0
+		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().

Quoted 23 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +23 -0
+	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;
+}

drivers/video/sm501new.c line 785

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+
+
+
+
+
+
+
+
+
+

please remove empty lines, max. 2 empty lines allowed.

Quoted 8 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +8 -0
+/* Initialisation data for PCI devices */
+
+
+static struct sm501_initdata sm501_pci_initdata = {
+	.gpio_high	= {
+		.set	= 0x3F000000,		/* 24bit panel */
+		.mask	= 0x0,
+	},

drivers/video/sm501new.c line 803

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+	.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?

Quoted 15 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +15 -0
+
+	/* 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),
+};
+

drivers/video/sm501new.c line 828

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+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.

drivers/video/sm501new.c line 20

Quoted 3 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +3 -0
+
+
+

max. 2 empty lines.

<snip>

Quoted 55 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +55 -0
+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,

drivers/video/sm501new.c line 950

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+			    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).

<snip>

Quoted 18 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +18 -0
+/* 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;

drivers/video/sm501new.c line 1053

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+	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.

drivers/video/sm501new.c line 1061

Quoted 8 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +8 -0
+
+/* 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.

drivers/video/sm501new.c line 1064

Quoted 3 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +3 -0
+	unsigned long reg;
+
+	reg = /*var->xoffset*/0 | (sm->xres_virtual << 16);

remove /*var->xoffset*/.

drivers/video/sm501new.c line 1067

Quoted 3 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +3 -0
+	writel(reg, sm->dc + SM501_DC_PANEL_FB_WIDTH);
+
+	reg = /*var->yoffset*/0 | (sm->yres_virtual << 16);

Ditto.

Quoted 37 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +37 -0
+	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)

drivers/video/sm501new.c line 1114

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -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).

<snip>

Quoted 42 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +42 -0
+/* 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:

drivers/video/sm501new.c line 1268

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+		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.

Quoted 96 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +96 -0
+		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;

drivers/video/sm501new.c line 1374

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+	}
+	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.

Quoted 60 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +60 -0
+			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;

drivers/video/sm501new.c line 1444

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+	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?

Quoted 28 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +28 -0
+	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);

drivers/video/sm501new.c line 1482

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+	
+	/* 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.

Quoted 39 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +39 -0
+
+	/*
+	 * 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();

drivers/video/sm501new.c line 1531

Quoted 10 lines
drivers/video/sm501new.c renamed from /drivers/video/sm501new.c +10 -0
+		
+		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

Signature · 3 lines
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