# Re: \[PATCH\] i.MX Framebuffer: Use iowrite/ioread instead of direct pointer deref - From: Krzysztof Helt \ - Date: 2008-08-20 15:56:44 - List: linux-fbdev - Page: https://ratatoskr.run/linux-fbdev/2008/08/13458660 - Thread: https://ratatoskr.run/linux-fbdev/2008/08/13458644/t.md ```text On Tue, 19 Aug 2008 17:06:41 +0200 Sascha Hauer [ref] wrote: > From: Juergen Beisert > > This patch prepares the current i.MX1 framebuffer driver for usage in the > whole i.MX family. It switches to iowrite/ioread for register accesses. > Also it moves the register definitions to the driver where they belong. > > Signed-off-by: Juergen Beisert > --- > drivers/video/imxfb.c | 208 ++++++++++++++++++++++++++++++++++++++++--------- > drivers/video/imxfb.h | 4 +- > 2 files changed, 173 insertions(+), 39 deletions(-) > > diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c > index a990d43..0622589 100644 > --- a/drivers/video/imxfb.c > +++ b/drivers/video/imxfb.c > @@ -16,7 +16,6 @@ > * linux-arm-kernel@lists.arm.linux.org.uk > */ > > -//#define DEBUG 1 > > #include > #include > @@ -33,7 +32,6 @@ > #include > #include > > -#include > #include > #include > > @@ -44,6 +42,124 @@ > > #include "imxfb.h" > > +#define DRIVER_NAME "imx-fb" > + All constants below should really be put into the imxfb.h. There is already such a file. > +#define LCDC_SSA 0x00 > + > +#define LCDC_SIZE 0x04 > +#define SIZE_XMAX(x) ((((x) >> 4) & 0x3f) << 20) > +#ifdef CONFIG_ARCH_IMX Is it possible to use this driver without CONFIG_ARCH_IMX defined? > +#define SIZE_YMAX(y) ((y) & 0x1ff) > +#else > +#define SIZE_YMAX(y) ((y) & 0x3ff) > +#endif > + > +#define LCDC_VPW 0x08 > +#define VPW_VPW(x) ((x) & 0x3ff) > + > +#define LCDC_CPOS 0x0C > +#define CPOS_CC1 (1<<31) > +#define CPOS_CC0 (1<<30) > +#define CPOS_OP (1<<28) > +#define CPOS_CXP(x) (((x) & 3ff) << 16) > +#ifdef CONFIG_ARCH_IMX > +#define CPOS_CYP(y) ((y) & 0x1ff) > +#else > +#define CPOS_CYP(y) ((y) & 0x3ff) > +#endif > + > +#define LCDC_LCWHB 0x10 > +#define LCWHB_BK_EN (1<<31) > +#define LCWHB_CW(w) (((w) & 0x1f) << 24) > +#define LCWHB_CH(h) (((h) & 0x1f) << 16) > +#define LCWHB_BD(x) ((x) & 0xff) > + > +#define LCDC_LCHCC 0x14 > +#ifdef CONFIG_ARCH_IMX > +#define LCHCC_CUR_COL_R(r) (((r) & 0x1f) << 11) > +#define LCHCC_CUR_COL_G(g) (((g) & 0x3f) << 5) > +#define LCHCC_CUR_COL_B(b) ((b) & 0x1f) > +#else > +#define LCHCC_CUR_COL_R(r) (((r) & 0x3f) << 12) > +#define LCHCC_CUR_COL_G(g) (((g) & 0x3f) << 6) > +#define LCHCC_CUR_COL_B(b) ((b) & 0x3f) > +#endif > + > +#define LCDC_PCR 0x18 > +#define PCR_TFT (1<<31) > +#define PCR_COLOR (1<<30) > +#define PCR_PBSIZ_1 (0<<28) > +#define PCR_PBSIZ_2 (1<<28) > +#define PCR_PBSIZ_4 (2<<28) > +#define PCR_PBSIZ_8 (3<<28) > +#define PCR_BPIX_1 (0<<25) > +#define PCR_BPIX_2 (1<<25) > +#define PCR_BPIX_4 (2<<25) > +#define PCR_BPIX_8 (3<<25) > +#define PCR_BPIX_12 (4<<25) > +#define PCR_BPIX_16 (4<<25) > +#define PCR_PIXPOL (1<<24) > +#define PCR_FLMPOL (1<<23) > +#define PCR_LPPOL (1<<22) > +#define PCR_CLKPOL (1<<21) > +#define PCR_OEPOL (1<<20) > +#define PCR_SCLKIDLE (1<<19) > +#define PCR_END_SEL (1<<18) > +#define PCR_END_BYTE_SWAP (1<<17) > +#define PCR_REV_VS (1<<16) > +#define PCR_ACD_SEL (1<<15) > +#define PCR_ACD(x) (((x) & 0x7f) << 8) > +#define PCR_SCLK_SEL (1<<7) > +#define PCR_SHARP (1<<6) > +#define PCR_PCD(x) ((x) & 0x3f) > + > +#define LCDC_HCR 0x1C > +#define HCR_H_WIDTH(x) (((x) & 0x3f) << 26) > +#define HCR_H_WAIT_1(x) (((x) & 0xff) << 8) > +#define HCR_H_WAIT_2(x) ((x) & 0xff) > + > +#define LCDC_VCR 0x20 > +#define VCR_V_WIDTH(x) (((x) & 0x3f) << 26) > +#define VCR_V_WAIT_1(x) (((x) & 0xff) << 8) > +#define VCR_V_WAIT_2(x) ((x) & 0xff) > + > +#define LCDC_POS 0x24 > +#define POS_POS(x) ((x) & 1f) > + > +#define LCDC_LSCR1 0x28 > +#define LSCR1_PS_RISE_DELAY(x) (((x) & 0x7f) << 26) > +#define LSCR1_CLS_RISE_DELAY(x) (((x) & 0x3f) << 16) > +#define LSCR1_REV_TOGGLE_DELAY(x) (((x) & 0xf) << 8) > +#define LSCR1_GRAY2(x) (((x) & 0xf) << 4) > +#define LSCR1_GRAY1(x) (((x) & 0xf)) > + > +#define LCDC_PWMR 0x2C > +#define PWMR_CLS(x) (((x) & 0x1ff) << 16) > +#define PWMR_LDMSK (1<<15) > +#define PWMR_SCR1 (1<<10) > +#define PWMR_SCR0 (1<<9) > +#define PWMR_CC_EN (1<<8) > +#define PWMR_PW(x) ((x) & 0xff) > + > +#define LCDC_DMACR 0x30 > +#define DMACR_BURST (1<<31) > +#define DMACR_HM(x) (((x) & 0xf) << 16) > +#define DMACR_TM(x) ((x) &0xf) > + > +#define LCDC_RMCR 0x34 > +#define RMCR_LCDC_EN (1<<1) > +#define RMCR_SELF_REF (1<<0) > + > +#define LCDC_LCDICR 0x38 > +#define LCDICR_INT_SYN (1<<2) > +#define LCDICR_INT_CON (1) > + > +#define LCDC_LCDISR 0x40 > +#define LCDISR_UDR_ERR (1<<3) > +#define LCDISR_ERR_RES (1<<2) > +#define LCDISR_EOF (1<<1) > +#define LCDISR_BOF (1<<0) > + > static struct imxfb_rgb def_rgb_16 = { > .red = { .offset = 8, .length = 4, }, > .green = { .offset = 4, .length = 4, }, > @@ -67,7 +183,6 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf) > return chan << bf->offset; > } > > -#define LCDC_PALETTE(x) __REG2(IMX_LCDC_BASE+0x800, (x)<<2) > static int > imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue, > u_int trans, struct fb_info *info) > @@ -81,7 +196,7 @@ imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue, > (CNVT_TOHW(green,4) << 4) | > CNVT_TOHW(blue, 4); > > - LCDC_PALETTE(regno) = val; > + iowrite32(val, fbi->regs + 0x800 + (regno << 2)); One may define an inline function imxfb_write(fbi, val, reg) : imxfb_iowrite(fbi, val, reg) { iowrite32(val, fbi->regs + reg); } It would make the code more readable. The imxfb_ioread() could be used as well. > ret = 0; > } > return ret; > @@ -235,18 +350,23 @@ static void imxfb_enable_controller(struct imxfb_info *fbi) > pr_debug("Enabling LCD controller\n"); > > /* initialize LCDC */ > - LCDC_RMCR &= ~RMCR_LCDC_EN; /* just to be safe... */ > + iowrite32(ioread32(fbi->regs + LCDC_RMCR) & ~RMCR_LCDC_EN, > + fbi->regs + LCDC_RMCR); /* just to be safe... */ > + > + iowrite32(fbi->screen_dma, fbi->regs + LCDC_SSA); > > - LCDC_SSA = fbi->screen_dma; > /* physical screen start address */ > - LCDC_VPW = VPW_VPW(fbi->max_xres * fbi->max_bpp / 8 / 4); > + iowrite32(VPW_VPW(fbi->max_xres * fbi->max_bpp / 8 / 4), > + fbi->regs + LCDC_VPW); > > - LCDC_POS = 0x00000000; /* panning offset 0 (0 pixel offset) */ > + /* panning offset 0 (0 pixel offset) */ > + iowrite32(0x00000000, fbi->regs + LCDC_POS); > > /* disable hardware cursor */ > - LCDC_CPOS &= ~(CPOS_CC0 | CPOS_CC1); > + iowrite32(ioread32(fbi->regs + LCDC_CPOS) & ~(CPOS_CC0 | CPOS_CC1), > + fbi->regs + LCDC_CPOS); > > - LCDC_RMCR = RMCR_LCDC_EN; > + iowrite32(RMCR_LCDC_EN, fbi->regs + LCDC_RMCR); > > if(fbi->backlight_power) > fbi->backlight_power(1); > @@ -263,7 +383,7 @@ static void imxfb_disable_controller(struct imxfb_info *fbi) > if(fbi->lcd_power) > fbi->lcd_power(0); > > - LCDC_RMCR = 0; > + iowrite32(0, fbi->regs + LCDC_RMCR); > } > > static int imxfb_blank(int blank, struct fb_info *info) > @@ -340,19 +460,22 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf > info->fix.id, var->lower_margin); > #endif > > - LCDC_HCR = HCR_H_WIDTH(var->hsync_len) | > - HCR_H_WAIT_1(var->left_margin) | > - HCR_H_WAIT_2(var->right_margin); > + iowrite32(HCR_H_WIDTH(var->hsync_len) | > + HCR_H_WAIT_1(var->left_margin) | > + HCR_H_WAIT_2(var->right_margin), > + fbi->regs + LCDC_HCR); > > - LCDC_VCR = VCR_V_WIDTH(var->vsync_len) | > - VCR_V_WAIT_1(var->upper_margin) | > - VCR_V_WAIT_2(var->lower_margin); > + iowrite32(VCR_V_WIDTH(var->vsync_len) | > + VCR_V_WAIT_1(var->upper_margin) | > + VCR_V_WAIT_2(var->lower_margin), > + fbi->regs + LCDC_VCR); > > - LCDC_SIZE = SIZE_XMAX(var->xres) | SIZE_YMAX(var->yres); > - LCDC_PCR = fbi->pcr; > - LCDC_PWMR = fbi->pwmr; > - LCDC_LSCR1 = fbi->lscr1; > - LCDC_DMACR = fbi->dmacr; > + iowrite32(SIZE_XMAX(var->xres) | SIZE_YMAX(var->yres), > + fbi->regs + LCDC_SIZE); > + iowrite32(fbi->pcr, fbi->regs + LCDC_PCR); > + iowrite32(fbi->pwmr, fbi->regs + LCDC_PWMR); > + iowrite32(fbi->lscr1, fbi->regs + LCDC_LSCR1); > + iowrite32(fbi->dmacr, fbi->regs + LCDC_DMACR); > > return 0; > } > @@ -384,10 +507,10 @@ static int imxfb_resume(struct platform_device *dev) > #define imxfb_resume NULL > #endif > > -static int __init imxfb_init_fbinfo(struct device *dev) > +static int __init imxfb_init_fbinfo(struct platform_device *pdev) > { > - struct imxfb_mach_info *inf = dev->platform_data; > - struct fb_info *info = dev_get_drvdata(dev); > + struct imxfb_mach_info *inf = pdev->dev.platform_data; > + struct fb_info *info = dev_get_drvdata(&pdev->dev); > struct imxfb_info *fbi = info->par; > > pr_debug("%s\n",__func__); > @@ -397,7 +520,7 @@ static int __init imxfb_init_fbinfo(struct device *dev) > return -ENOMEM; > > memset(fbi, 0, sizeof(struct imxfb_info)); > - fbi->dev = dev; > + fbi->pdev = pdev; > > strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id)); > > @@ -465,7 +588,7 @@ static int __init imxfb_map_video_memory(struct fb_info *info) > struct imxfb_info *fbi = info->par; > > fbi->map_size = PAGE_ALIGN(info->fix.smem_len); > - fbi->map_cpu = dma_alloc_writecombine(fbi->dev, fbi->map_size, > + fbi->map_cpu = dma_alloc_writecombine(&fbi->pdev->dev, fbi->map_size, Use &pdev->dev instead &fbi->pdev->dev > &fbi->map_dma,GFP_KERNEL); > > if (fbi->map_cpu) { > @@ -506,14 +629,21 @@ static int __init imxfb_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, info); > > - ret = imxfb_init_fbinfo(&pdev->dev); > + ret = imxfb_init_fbinfo(pdev); > if( ret < 0 ) > goto failed_init; > > - res = request_mem_region(res->start, res->end - res->start + 1, "IMXFB"); > + res = request_mem_region(res->start, res->end - res->start + 1, > + DRIVER_NAME); There is a function resource_size(res) which does (res->end - res->start + 1). It seems useful here and at few places below. > if (!res) { > ret = -EBUSY; > - goto failed_regs; > + goto failed_req; > + } > + > + fbi->regs = ioremap(res->start, res->end - res->start + 1); > + if (fbi->regs == NULL) { > + printk(KERN_ERR"Cannot map frame buffer registers\n"); > + goto failed_ioremap; > } > > if (!inf->fixed_screen_cpu) { > @@ -559,18 +689,20 @@ failed_register: > failed_cmap: > if (!inf->fixed_screen_cpu) > dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu, > - fbi->map_dma); > + fbi->map_dma); > failed_map: > - kfree(info->pseudo_palette); > -failed_regs: > + iounmap(fbi->regs); > +failed_ioremap: > release_mem_region(res->start, res->end - res->start); > +failed_req: > + kfree(info->pseudo_palette); > failed_init: > platform_set_drvdata(pdev, NULL); > framebuffer_release(info); > return ret; > } > > -static int imxfb_remove(struct platform_device *pdev) > +static int __devexit imxfb_remove(struct platform_device *pdev) > { > struct fb_info *info = platform_get_drvdata(pdev); > struct imxfb_info *fbi = info->par; > @@ -586,6 +718,7 @@ static int imxfb_remove(struct platform_device *pdev) > kfree(info->pseudo_palette); > framebuffer_release(info); > > + iounmap(fbi->regs); The fbi pointed does not exist here (after framebuffer_release(info)). > release_mem_region(res->start, res->end - res->start + 1); > platform_set_drvdata(pdev, NULL); > > @@ -600,19 +733,18 @@ void imxfb_shutdown(struct platform_device * dev) > } > > static struct platform_driver imxfb_driver = { > - .probe = imxfb_probe, > .suspend = imxfb_suspend, > .resume = imxfb_resume, > - .remove = imxfb_remove, > + .remove = __devexit_p(imxfb_remove), > .shutdown = imxfb_shutdown, > .driver = { > - .name = "imx-fb", > + .name = DRIVER_NAME, > }, > }; > > int __init imxfb_init(void) > { > - return platform_driver_register(&imxfb_driver); > + return platform_driver_probe(&imxfb_driver, imxfb_probe); > } > > static void __exit imxfb_cleanup(void) > diff --git a/drivers/video/imxfb.h b/drivers/video/imxfb.h > index e837a8b..766297e 100644 > --- a/drivers/video/imxfb.h > +++ b/drivers/video/imxfb.h > @@ -29,7 +29,9 @@ struct imxfb_rgb { > #define NR_RGB 2 > > struct imxfb_info { > - struct device *dev; > + struct platform_device *pdev; Is this field really useful? It can be dropped. > + void __iomem *regs; > + > struct imxfb_rgb *rgb[NR_RGB]; > > u_int max_bpp; > -- > 1.5.6.3 > > > ------------------------------------------------------------------------- > This SF.Net email is sponsored by the Moblin Your Move Developer's challenge > Build the coolest Linux based applications with Moblin SDK & win great prizes > Grand prize is a trip for two to an Open Source event anywhere in the world > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > _______________________________________________ > Linux-fbdev-devel mailing list > Linux-fbdev-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel > ---------------------------------------------------------------------- Igrzyska na Dzikim Zachodzie! Sprawdz >> http://link.interia.pl/f1edc ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ ```