Au1100 FB driver uplift for 2.6

Au1100 FB driver uplift for 2.6

[Ulrich Eckhardt]

Greetings!

I'm just looking at these changes and I'm delighted to finally see them in. 
However, I have one small problem: the old version allowed to program EXTCLK1 
and the LCD controller clock via 'mode_toyclksrc' in the panel configuration 
database[1], but this has been removed (also the 'mode_backlight' member, 
btw).
FYI, on the TTP1100[2], the pixelclock for some displays types is created via 
EXTCLK1 which doesn't work anymore, which is why I'm asking.

Am I on the wrong way or should I just reimplement it and send a patch?

Uli

[1] I have a few nits on that, too: IMHO it could be made constant and marked 
as '__initdata' so it is discarded after initialization.
[2] Based on DB1100. Are there any pointers on how to port to a new board, 
btw?

Re: Au1100 FB driver uplift for 2.6

[Dan Malek]

On Apr 4, 2005, at 11:17 AM, Ulrich Eckhardt wrote:

> Am I on the wrong way or should I just reimplement it and send a patch?

If you an test it, do it and send a patch.

> [2] Based on DB1100. Are there any pointers on how to port to a new 
> board,
> btw?

One of the discussion items is always how to keep a "generic"
driver and still provide unique setup/control for different types of
boards.  I guess if we can discuss other board ports, it will be
more clear how to do this.

Thanks.


	-- Dan

Porting to new board [was: Re: Au1100 FB driver uplift for 2.6]

[Ulrich Eckhardt]

Dan Malek wrote:
> On Apr 4, 2005, at 11:17 AM, Ulrich Eckhardt wrote:
> > [2] Based on DB1100. Are there any pointers on how to port to a new
> > board, btw?
>
> One of the discussion items is always how to keep a "generic"
> driver and still provide unique setup/control for different types of
> boards.  I guess if we can discuss other board ports, it will be
> more clear how to do this.

OK, I'll be more concrete:
The board I'm talking about has different peripherals than the DB1100. In 
particular, it doesn't have any BCSRs, it only has a single flash-chip with 2 
MB RAM on board, a CAN bus interface and a rather peculiar way to generate 
the clocks for attached displays. 
 Most of this is easily remedied by only compiling a selected driver in and 
I'm doing rather fine loading a DB1100 config and modifying it a bit to get 
the board running. There are a few things though where I could really use a 
preprocessor check for that board:
- the layout table for the on-board flash RAM. The current driver only checks 
for a handful of layouts supported by the DB1x/PB1x boards, but those don't 
fit. This needs more research though.
- a default configuration file would be nice, but optional.
- the mentioned BCSRs are used to control some peripherals. Code that uses 
them needs to be changed.

All in all, I'd just write a different board_setup() routine than the one for 
DB1100 and have a few #ifdef HAVE_DB1x00_BCSR and be done. I'd even make this 
a subtype of the 'standard' DB1100 configuration, because of its strong 
similarities, but there are a few cases where I simply need to determine the 
type in the code.

I'd send a patch for discussion, but I'm far from finished yet. If there are 
any pitfalls I should watch out for or you have any suggestion how to tackle 
this problem, I'm all open ears.

thanks

Uli

Re: Au1100 FB driver uplift for 2.6

[Ulrich Eckhardt]

Dan Malek wrote:
> On Apr 4, 2005, at 11:17 AM, Ulrich Eckhardt wrote:
> > Am I on the wrong way or should I just reimplement it and send a patch?
>
> If you an test it, do it and send a patch.

There will be two patches, this one doesn't reimplement the mentioned 
feature but fixes a few other bugs. 

There are two questions left in that patch, apart from the handling of the 
WWPC display as mentioned in the header. One is, if there was a reason to 
use a fixed-size char array for the panelname. 

The other is about the use of au1100fb_fix and au1100fb_var. Is the 
explanation in the patch accurate? Would it make more sense to write 
directly into fbdev->info.var and fbdev->info.fixed? Also, couldn't 
au1100fb_drv_probe() be declared as __init? After all, a late call to 
au1100fb_drv_probe() will probably have fatal consequences anyway because 
it depends on the above and they are declared __initdata.

Uli

Changes:
 * fixed typos, inconsistent formatting and trailing whitespace
 * added a check to a kalloc() call
 * fixed possible resource leak in au1100fb_drv_probe()
 * replaced the fixed-size panel name with a char pointer 
 * added a few 'const'
 * added display data for PrimeView PD104SL5

---
Index: drivers/video/au1100fb.c
===================================================================
RCS file: /home/cvs/linux/drivers/video/au1100fb.c,v
retrieving revision 1.6
diff -u -r1.6 au1100fb.c
--- drivers/video/au1100fb.c 4 Apr 2005 01:06:20 -0000 1.6
+++ drivers/video/au1100fb.c 6 Apr 2005 09:03:54 -0000
@@ -3,7 +3,7 @@
  * Au1100 LCD Driver.
  *
  * Rewritten for 2.6 by Embedded Alley Solutions
- *  <source@embeddedalley.com>, based on submissions by 
+ *  <source@embeddedalley.com>, based on submissions by
  *   Karl Lessard <klessard@sunrisetelecom.com>
  *   <c.pellegrin@exadron.com>
  *
@@ -56,7 +56,7 @@
 
 #include "au1100fb.h"
 
-/* 
+/*
  * Sanity check. If this is a new Au1100 based board, search for
  * the PB1100 ifdefs to make sure you modify the code accordingly.
  */
@@ -74,11 +74,11 @@
 #define to_au1100fb_device(_info) \
    (_info ? container_of(_info, struct au1100fb_device, info) : NULL);
 
-/* Bitfields format supported by the controller. Note that the order of formats 
+/* Bitfields format supported by the controller. Note that the order of formats
  * SHOULD be the same as in the LCD_CONTROL_SBPPF field, so we can retrieve the
  * right pixel format by doing rgb_bitfields[LCD_CONTROL_SBPPF_XXX >> LCD_CONTROL_SBPPF]
  */
-struct fb_bitfield rgb_bitfields[][4] = 
+struct fb_bitfield rgb_bitfields[][4] =
 {
    /*     Red,     Green,   Blue,       Transp   */
  { { 10, 6, 0 }, { 5, 5, 0 }, { 0, 5, 0 }, { 0, 0, 0 } },
@@ -91,6 +91,8 @@
  { { 8, 4, 0 },  { 4, 4, 0 }, { 0, 4, 0 }, { 0, 0, 0 } },
 };
 
+/* au1100fb_fix and au1100fb_var are local vars of au1100fb_drv_probe() but
+defined here in order to not exhaust the stack. */
 static struct fb_fix_screeninfo au1100fb_fix __initdata = {
  .id  = "AU1100 FB",
  .xpanstep  = 1,
@@ -111,7 +113,7 @@
 /*
  * Set hardware with var settings. This will enable the controller with a specific
  * mode, normally validated with the fb_check_var method
-  */
+ */
 int au1100fb_setmode(struct au1100fb_device *fbdev)
 {
  struct fb_info *info = &fbdev->info;
@@ -142,7 +144,7 @@
    info->var.transp.msb_right = 0;
 
    info->fix.visual = FB_VISUAL_PSEUDOCOLOR;
-   info->fix.line_length = info->var.xres_virtual / 
+   info->fix.line_length = info->var.xres_virtual /
        (8/info->var.bits_per_pixel);
   } else {
    /* non-palettized */
@@ -154,7 +156,7 @@
 
    info->fix.visual = FB_VISUAL_TRUECOLOR;
    info->fix.line_length = info->var.xres_virtual << 1; /* depth=16 */
- }
+  }
  } else {
   /* mono */
   info->fix.visual = FB_VISUAL_MONO10;
@@ -162,7 +164,7 @@
  }
 
  info->screen_size = info->fix.line_length * info->var.yres_virtual;
- 
+
  /* Determine BPP mode and format */
  fbdev->regs->lcd_control = fbdev->panel->control_base |
        ((info->var.rotate/90) << LCD_CONTROL_SM_BIT);
@@ -183,8 +185,8 @@
    * otherwise display the same as the first panel */
   if (info->var.yres_virtual >= (info->var.yres << 1)) {
    fbdev->regs->lcd_dmaaddr1 = LCD_DMA_SA_N(fbdev->fb_phys +
-         (info->fix.line_length * 
-                (info->var.yres_virtual >> 1)));
+         (info->fix.line_length *
+         (info->var.yres_virtual >> 1)));
   } else {
    fbdev->regs->lcd_dmaaddr1 = LCD_DMA_SA_N(fbdev->fb_phys);
   }
@@ -201,7 +203,7 @@
 
  fbdev->regs->lcd_pwmdiv = 0;
  fbdev->regs->lcd_pwmhi = 0;
-   
+
  /* Resume controller */
  fbdev->regs->lcd_control |= LCD_CONTROL_GO;
 
@@ -222,22 +224,22 @@
 
  if (fbi->var.grayscale) {
   /* Convert color to grayscale */
-  red = green = blue = 
+  red = green = blue =
    (19595 * red + 38470 * green + 7471 * blue) >> 16;
  }
-   
+
  if (fbi->fix.visual == FB_VISUAL_TRUECOLOR) {
   /* Place color in the pseudopalette */
   if (regno > 16)
    return -EINVAL;
-   
+
   palette = (u32*)fbi->pseudo_palette;
 
   red   >>= (16 - fbi->var.red.length);
   green >>= (16 - fbi->var.green.length);
   blue  >>= (16 - fbi->var.blue.length);
- 
-  value = (red   << fbi->var.red.offset)  | 
+
+  value = (red   << fbi->var.red.offset)  |
    (green << fbi->var.green.offset)|
    (blue  << fbi->var.blue.offset);
   value &= 0xFFFF;
@@ -249,8 +251,8 @@
 
  } else if (panel_is_color(fbdev->panel)) {
   /* COLOR STN MODE */
-  value = (((panel_swap_rgb(fbdev->panel) ? blue : red) >> 12) & 0x000F) | 
-   ((green >> 8) & 0x00F0) | 
+  value = (((panel_swap_rgb(fbdev->panel) ? blue : red) >> 12) & 0x000F) |
+   ((green >> 8) & 0x00F0) |
    (((panel_swap_rgb(fbdev->panel) ? red : blue) >> 4) & 0x0F00);
   value &= 0xFFF;
  } else {
@@ -260,7 +262,7 @@
  }
 
  palette[regno] = value;
- 
+
  return 0;
 }
 
@@ -277,14 +279,14 @@
  switch (blank_mode) {
 
  case VESA_NO_BLANKING:
-   /* Turn on panel */
-   fbdev->regs->lcd_control |= LCD_CONTROL_GO;
+  /* Turn on panel */
+  fbdev->regs->lcd_control |= LCD_CONTROL_GO;
 #ifdef CONFIG_MIPS_PB1100
-   if (drv_info.panel_idx == 1) {
-    au_writew(au_readw(PB1100_G_CONTROL) 
-       | (PB1100_G_CONTROL_BL | PB1100_G_CONTROL_VDD), 
-   PB1100_G_CONTROL);
-   }
+  if (drv_info.panel_idx == 1) {
+   au_writew(au_readw(PB1100_G_CONTROL)
+     | (PB1100_G_CONTROL_BL | PB1100_G_CONTROL_VDD),
+    PB1100_G_CONTROL);
+  }
 #endif
   au_sync();
   break;
@@ -292,20 +294,20 @@
  case VESA_VSYNC_SUSPEND:
  case VESA_HSYNC_SUSPEND:
  case VESA_POWERDOWN:
-   /* Turn off panel */
-   fbdev->regs->lcd_control &= ~LCD_CONTROL_GO;
+  /* Turn off panel */
+  fbdev->regs->lcd_control &= ~LCD_CONTROL_GO;
 #ifdef CONFIG_MIPS_PB1100
-   if (drv_info.panel_idx == 1) {
-    au_writew(au_readw(PB1100_G_CONTROL) 
-         & ~(PB1100_G_CONTROL_BL | PB1100_G_CONTROL_VDD),
-   PB1100_G_CONTROL);
-   }
+  if (drv_info.panel_idx == 1) {
+   au_writew(au_readw(PB1100_G_CONTROL)
+     & ~(PB1100_G_CONTROL_BL | PB1100_G_CONTROL_VDD),
+    PB1100_G_CONTROL);
+  }
 #endif
   au_sync();
   break;
- default: 
-  break;
 
+ default:
+  break;
  }
  return 0;
 }
@@ -328,7 +330,7 @@
   /* No support for X panning for now! */
   return -EINVAL;
  }
-   
+
  print_dbg("fb_pan_display 2 %p %p", var, fbi);
  dy = var->yoffset - fbi->var.yoffset;
  if (dy) {
@@ -340,14 +342,14 @@
   dmaaddr = fbdev->regs->lcd_dmaaddr0;
   dmaaddr += (fbi->fix.line_length * dy);
 
-  /* TODO: Wait for current frame to finished */
+  /* TODO: Wait for current frame to be finished */
   fbdev->regs->lcd_dmaaddr0 = LCD_DMA_SA_N(dmaaddr);
 
   if (panel_is_dual(fbdev->panel)) {
    dmaaddr = fbdev->regs->lcd_dmaaddr1;
    dmaaddr += (fbi->fix.line_length * dy);
    fbdev->regs->lcd_dmaaddr0 = LCD_DMA_SA_N(dmaaddr);
- }
+  }
  }
  print_dbg("fb_pan_display 3 %p %p", var, fbi);
 
@@ -388,7 +390,7 @@
  if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT)) {
   return -EINVAL;
  }
-    
+
  start = fbdev->fb_phys & PAGE_MASK;
  len = PAGE_ALIGN((start & ~PAGE_MASK) + fbdev->fb_len);
 
@@ -405,7 +407,7 @@
  pgprot_val(vma->vm_page_prot) |= (6 << 9); //CCA=6
 
  vma->vm_flags |= VM_IO;
-    
+
  if (io_remap_page_range(vma, vma->vm_start, off,
     vma->vm_end - vma->vm_start,
     vma->vm_page_prot)) {
@@ -415,7 +417,7 @@
  return 0;
 }
 
-static struct fb_ops au1100fb_ops = 
+static struct fb_ops au1100fb_ops =
 {
  .owner   = THIS_MODULE,
  .fb_setcolreg  = au1100fb_fb_setcolreg,
@@ -440,13 +442,14 @@
  struct resource *regs_res;
  unsigned long page;
  u32 sys_clksrc;
+ int res = 0;
 
  if (!dev)
-   return -EINVAL;
+  return -EINVAL;
 
  /* Allocate new device private */
  if (!(fbdev = kmalloc(sizeof(struct au1100fb_device), GFP_KERNEL))) {
-  print_err("fail to allocate device private record");
+  print_err("failed to allocate device private record");
   return -ENOMEM;
  }
  memset((void*)fbdev, 0, sizeof(struct au1100fb_device));
@@ -456,20 +459,22 @@
  dev_set_drvdata(dev, (void*)fbdev);
 
  /* Allocate region for our registers and map them */
- if (!(regs_res = platform_get_resource(to_platform_device(dev), 
+ if (!(regs_res = platform_get_resource(to_platform_device(dev),
      IORESOURCE_MEM, 0))) {
-  print_err("fail to retrieve registers resource");
-  return -EFAULT;
+  print_err("failed to retrieve registers resource");
+  res = -EFAULT;
+  goto failed;
  }
 
  au1100fb_fix.mmio_start = regs_res->start;
  au1100fb_fix.mmio_len = regs_res->end - regs_res->start + 1;
 
- if (!request_mem_region(au1100fb_fix.mmio_start, au1100fb_fix.mmio_len, 
+ if (!request_mem_region(au1100fb_fix.mmio_start, au1100fb_fix.mmio_len,
     DRIVER_NAME)) {
-  print_err("fail to lock memory region at 0x%08x", 
+  print_err("failed to lock memory region at 0x%08x",
     au1100fb_fix.mmio_start);
-  return -EBUSY;
+  res = -EBUSY;
+  goto failed;
  }
 
  fbdev->regs = (struct au1100fb_regs*)KSEG1ADDR(au1100fb_fix.mmio_start);
@@ -478,17 +483,17 @@
  print_dbg("phys=0x%08x, size=%d", fbdev->regs_phys, fbdev->regs_len);
 
 
-
  /* Allocate the framebuffer to the maximum screen size * nbr of video buffers */
  fbdev->fb_len = fbdev->panel->xres * fbdev->panel->yres *
      (fbdev->panel->bpp >> 3) * AU1100FB_NBR_VIDEO_BUFFERS;
- 
- fbdev->fb_mem = dma_alloc_coherent(dev, PAGE_ALIGN(fbdev->fb_len), 
+
+ fbdev->fb_mem = dma_alloc_coherent(dev, PAGE_ALIGN(fbdev->fb_len),
      &fbdev->fb_phys, GFP_KERNEL);
  if (!fbdev->fb_mem) {
-  print_err("fail to allocate frambuffer (size: %dK))", 
+  print_err("failed to allocate frambuffer (size: %dKiB))",
      fbdev->fb_len / 1024);
-  return -ENOMEM;
+  res = -ENOMEM;
+  goto failed;
  }
 
  au1100fb_fix.smem_start = fbdev->fb_phys;
@@ -499,7 +504,7 @@
   * since we'll be remapping normal memory.
   */
  for (page = (unsigned long)fbdev->fb_mem;
-      page < PAGE_ALIGN((unsigned long)fbdev->fb_mem + fbdev->fb_len); 
+      page < PAGE_ALIGN((unsigned long)fbdev->fb_mem + fbdev->fb_len);
       page += PAGE_SIZE) {
 #if CONFIG_DMA_NONCOHERENT
   SetPageReserved(virt_to_page(CAC_ADDR(page)));
@@ -527,15 +532,17 @@
  fbdev->info.fix = au1100fb_fix;
 
  if (!(fbdev->info.pseudo_palette = kmalloc(sizeof(u32) * 16, GFP_KERNEL))) {
-  return -ENOMEM;
+  print_err("failed to allocate pseudo palette");
+  res = -ENOMEM;
+  goto failed;
  }
  memset(fbdev->info.pseudo_palette, 0, sizeof(u32) * 16);
 
  if (fb_alloc_cmap(&fbdev->info.cmap, AU1100_LCD_NBR_PALETTE_ENTRIES, 0) < 0) {
-  print_err("Fail to allocate colormap (%d entries)",
+  print_err("failed to allocate colormap (%d entries)",
       AU1100_LCD_NBR_PALETTE_ENTRIES);
-  kfree(fbdev->info.pseudo_palette);
-  return -EFAULT;
+  res = -EFAULT;
+  goto failed;
  }
 
  fbdev->info.var = au1100fb_var;
@@ -546,25 +553,30 @@
  /* Register new framebuffer */
  if (register_framebuffer(&fbdev->info) < 0) {
   print_err("cannot register new framebuffer");
+  res = -EFAULT;
   goto failed;
  }
 
  return 0;
 
 failed:
- if (fbdev->regs) {
-  release_mem_region(fbdev->regs_phys, fbdev->regs_len);
+ /** Failure. Release all resources in opposite order of allocation. */
+ if (fbdev->info.cmap.len != 0) {
+  fb_dealloc_cmap(&fbdev->info.cmap);
+ }
+ if (fbdev->info.pseudo_palette){
+  kfree(fbdev->info.pseudo_palette);
  }
  if (fbdev->fb_mem) {
   dma_free_noncoherent(dev, fbdev->fb_len, fbdev->fb_mem, fbdev->fb_phys);
  }
- if (fbdev->info.cmap.len != 0) {
-  fb_dealloc_cmap(&fbdev->info.cmap);
+ if (fbdev->regs) {
+  release_mem_region(fbdev->regs_phys, fbdev->regs_len);
  }
- kfree(fbdev);
  dev_set_drvdata(dev, NULL);
+ kfree(fbdev);
 
- return 0;
+ return res;
 }
 
 int au1100fb_drv_remove(struct device *dev)
@@ -610,17 +622,16 @@
 static struct device_driver au1100fb_driver = {
  .name  = "au1100-lcd",
  .bus  = &platform_bus_type,
-
  .probe  = au1100fb_drv_probe,
-        .remove  = au1100fb_drv_remove,
+ .remove  = au1100fb_drv_remove,
  .suspend = au1100fb_drv_suspend,
-        .resume  = au1100fb_drv_resume,
+ .resume  = au1100fb_drv_resume,
 };
-    
+
 /*-------------------------------------------------------------------------*/
 
 /* Kernel driver */
-    
+
 int au1100fb_setup(char *options)
 {
  char* this_opt;
@@ -631,44 +642,48 @@
  if (num_panels <= 0) {
   print_err("No LCD panels supported by driver!");
   return -EFAULT;
-   }
+ }
 
  if (options) {
   while ((this_opt = strsep(&options,",")) != NULL) {
+   size_t len = strlen(this_opt);
    /* Panel option */
-  if (!strncmp(this_opt, "panel:", 6)) {
+   if (!strncmp(this_opt, "panel:", 6)) {
     int i;
     this_opt += 6;
     for (i = 0; i < num_panels; i++) {
      if (!strncmp(this_opt,
-                 known_lcd_panels[i].name, 
-       strlen(this_opt))) {
+                 known_lcd_panels[i].name,
+                  len)) {
       panel_idx = i;
-     break;
+      break;
+     }
     }
-   }
     if (i >= num_panels) {
       print_warn("Panel %s not supported!", this_opt);
+     return -ENOTSUPP;
     }
    }
    /* Mode option (only option that start with digit) */
    else if (isdigit(this_opt[0])) {
-    mode = kmalloc(strlen(this_opt) + 1, GFP_KERNEL);
-    strncpy(mode, this_opt, strlen(this_opt) + 1);
+    mode = kmalloc(len+1, GFP_KERNEL);
+    if(!mode)
+     return -ENOMEM;
+    strncpy(mode, this_opt, len);
    }
    /* Unsupported option */
    else {
     print_warn("Unsupported option \"%s\"", this_opt);
+   }
   }
-  }
- } 
+ }
 
  drv_info.panel_idx = panel_idx;
  drv_info.opt_mode = mode;
 
  print_info("Panel=%s Mode=%s",
    known_lcd_panels[drv_info.panel_idx].name,
-         drv_info.opt_mode ? drv_info.opt_mode : "default");
+   drv_info.opt_mode ? drv_info.opt_mode : "default");
 
  return 0;
 }
@@ -677,9 +692,9 @@
 {
  char* options;
  int ret;
- 
+
  print_info("" DRIVER_DESC "");
- 
+
  memset(&drv_info, 0, sizeof(drv_info));
 
  if (fb_get_options(DRIVER_NAME, &options))
@@ -688,7 +703,7 @@
  /* Setup driver with options */
  ret = au1100fb_setup(options);
  if (ret < 0) {
-  print_err("Fail to setup driver");
+  print_err("Failed to setup driver");
   return ret;
  }
 
Index: drivers/video/au1100fb.h
===================================================================
RCS file: /home/cvs/linux/drivers/video/au1100fb.h,v
retrieving revision 1.2
diff -u -r1.2 au1100fb.h
--- drivers/video/au1100fb.h 4 Apr 2005 01:06:20 -0000 1.2
+++ drivers/video/au1100fb.h 6 Apr 2005 09:03:54 -0000
@@ -65,20 +65,20 @@
 
 struct au1100fb_panel
 {
- const char name[25];  /* Full name <vendor>_<model> */
+ const char* name;  /* Full name <vendor>_<model> */
 
- u32    control_base;  /* Mode-independent control values */
+ u32 control_base;  /* Mode-independent control values */
  u32 clkcontrol_base; /* Panel pixclock preferences */
 
  u32 horztiming;
  u32 verttiming;
 
  u32 xres;  /* Maximum horizontal resolution */
- u32  yres;  /* Maximum vertical resolution */
- u32  bpp;  /* Maximum depth supported */
+ u32 yres;  /* Maximum vertical resolution */
+ u32 bpp;  /* Maximum depth supported */
 };
 
-struct au1100fb_regs 
+struct au1100fb_regs
 {
  u32  lcd_control;
  u32  lcd_intstatus;
@@ -99,7 +99,7 @@
 
  struct fb_info info;   /* FB driver info record */
 
- struct au1100fb_panel  *panel;  /* Panel connected to this device */
+ const struct au1100fb_panel  *panel;  /* Panel connected to this device */
 
  struct au1100fb_regs*  regs;  /* Registers memory map */
  size_t         regs_len;
@@ -255,7 +255,7 @@
 
 /* List of panels known to work with the AU1100 LCD controller.
  * To add a new panel, enter the same specifications as the
- * Generic_TFT one, and MAKE SURE that it doesn't conflicts 
+ * Generic_TFT one, and MAKE SURE that it doesn't conflicts
  * with the controller restrictions. Restrictions are:
  *
  * STN color panels: max_bpp <= 12
@@ -264,7 +264,7 @@
  * max_xres <= 800
  * max_yres <= 600
  */
-static struct au1100fb_panel known_lcd_panels[] =
+static const struct au1100fb_panel known_lcd_panels[] =
 {
  /* 800x600x16bpp CRT */
  [0] = {
@@ -272,14 +272,24 @@
   .xres = 800,
   .yres = 600,
   .bpp = 16,
-  .control_base = 0x0004886A | 
+  .control_base = 0x0004886A |
    LCD_CONTROL_DEFAULT_PO | LCD_CONTROL_DEFAULT_SBPPF |
    LCD_CONTROL_BPP_16,
   .clkcontrol_base = 0x00020000,
   .horztiming = 0x005aff1f,
   .verttiming = 0x16000e57,
  },
- /* just the standard LCD */
+
+ /* just the standard LCD
+ TODO: There is only one case where the index in this array is checked,
+ and that also only on PB1100 boards. Clarify when exactly this is used
+ and why, and possibly integrate this differently.
+
+ Also, I'm not sure the code is right: in older versions, WWPC had this
+ display hard-coded in, and for PB1100 and DB1100, there was a way to
+ select a display by reading the BCSR. Now, there seems to be a special
+ way of blanking/unblanking this particular display.
+ */
  [1] = {
   .name = "WWPC LCD",
   .xres = 240,
@@ -290,6 +300,7 @@
   .verttiming = 0x0301013F,
   .clkcontrol_base = 0x00018001,
  },
+
  /* Sharp 320x240 TFT panel */
  [2] = {
   .name = "Sharp_LQ038Q5DR01",
@@ -351,7 +362,7 @@
   .clkcontrol_base = LCD_CLKCONTROL_PCD_N(1),
  },
 
-  /* Pb1100 LCDB 640x480 PrimeView TFT panel */
+ /* Pb1100 LCDB 640x480 PrimeView TFT panel */
  [5] = {
   .name = "PrimeView_640x480_16",
   .xres = 640,
@@ -362,6 +373,37 @@
   .verttiming = 0x210805df,
   .clkcontrol_base = 0x00038001,
  },
+
+ /* PrimeView PD104SL5 , a 800x600 16BPP color LCD */
+ [6] = {
+  .name = "PrimeView_PD104SL5",
+  .xres = 800,
+  .yres = 640,
+  .bpp = 16,
+  .control_base =
+   ( LCD_CONTROL_SBPPF_565
+   | LCD_CONTROL_C
+   | LCD_CONTROL_SM_0
+   | LCD_CONTROL_DEFAULT_PO
+   | LCD_CONTROL_CCO
+   | LCD_CONTROL_PT
+   | LCD_CONTROL_PC
+   | LCD_CONTROL_BPP_16 ),
+  .horztiming =
+   ( LCD_HORZTIMING_HN2_N(30)
+   | LCD_HORZTIMING_HN1_N(30)
+   | LCD_HORZTIMING_HPW_N(60)
+   | LCD_HORZTIMING_PPL_N(800) ),
+  .verttiming =
+   ( LCD_VERTTIMING_VN2_N(1)
+   | LCD_VERTTIMING_VN1_N(1)
+   | LCD_VERTTIMING_VPW_N(2)
+   | LCD_VERTTIMING_LPP_N(600) ),
+  .clkcontrol_base =
+   ( LCD_CLKCONTROL_IH
+   | LCD_CLKCONTROL_IV
+   | LCD_CLKCONTROL_PCD_N(1)),
+ }
 };
 
 struct au1100fb_drv_info {

Re: Au1100 FB driver uplift for 2.6

[Ulrich Eckhardt]

Ulrich Eckhardt wrote:
> Dan Malek wrote:
> > On Apr 4, 2005, at 11:17 AM, Ulrich Eckhardt wrote:
> > > Am I on the wrong way or should I just reimplement it and send a patch?
> >
> > If you an test it, do it and send a patch.
>
> There will be two patches, this one doesn't reimplement the mentioned
> feature but fixes a few other bugs.

I'm currently preparing the second part, but I stumbled across something 
puzzling: the number and configuration of the displays differ a lot to 
previous versions. Here's a list of displays and their mode_toyclksrc values:

Old driver:
Sharp_320x240_16      (1 << SYS_CS_ML_BIT) | SYS_CS_DL | SYS_CS_CL)
Generic_640x480_16    (1 << SYS_CS_ML_BIT) | SYS_CS_DL
PrimeView_640x480_16  (1 << SYS_CS_ML_BIT) | SYS_CS_DL
NEON_800x600_16       (1 << SYS_CS_ML_BIT) | SYS_CS_DL
NEON_640x480_16       (1 << SYS_CS_ML_BIT) | SYS_CS_DL

New driver:
CRT_800x600_16        (1 << SYS_CS_ML_BIT)
WWPC LCD              (1 << SYS_CS_ML_BIT)
Sharp_LQ038Q5DR01      (1 << SYS_CS_ML_BIT)
Hitachi_SP14Qxxx      (1 << SYS_CS_ML_BIT)
TFT_640x480_16        (1 << SYS_CS_ML_BIT)
PrimeView_640x480_16  (1 << SYS_CS_ML_BIT)

Changed names:
Sharp_320x240_16    =   Sharp_LQ038Q5DR01
Generic_640x480_16  =   TFT_640x480_16
NEON_800x600_16         (~CRT_800x600_16)
NEON_640x480_16         (missing)

Notes:
- The SYS_CS_DL flag is only evaluated when the SYS_CS_CL flag is also set. 
IOW, setting the flag was useless except for the Sharp_320x240_16 panel in 
the old configuration.
- 'WWPC LCD' contains a space, meaning you can't possibly specify it on the 
kernel commandline - that's obviously a bug.
- The NEON_640x480_16 panel is missing. The other NEON also has no 100% 
equivalent, the CRT panel mostly matches though.
- The parameters for the Sharp display have effectively changed. I don't know 
if this was intentional and I don't have such a display for testing.
- I have an Hitachi SP14Q and a PrimeView PD104SL5 here for testing on a 
- In the old header was the comment that "The fb driver assumes that AUX PLL 
is at 48MHz." This can't be true, because the minimum multiplier is 8 and the 
external clock should be 12MHz, making this 96MHz.
- You can change some frequency for the LCD controller with the divider in 
SYS_CLKSRC and via the LCD_CLKCONTROL, is it possible that you can change one 
frequency and make up for it via another? 
In fact, I'm currently pretty puzzled, because it might be that the 
differences are rather in the board and not in the panels. Also, I fail to 
find where the AUX PLL is started (the default is off), which puzzles me even 
further...

regards,

Uli

Re: Au1100 FB driver uplift for 2.6

[Pete Popov]

Ulrich Eckhardt wrote:
> Dan Malek wrote:
> 
>>On Apr 4, 2005, at 11:17 AM, Ulrich Eckhardt wrote:
>>
>>>Am I on the wrong way or should I just reimplement it and send a patch?
>>
>>If you an test it, do it and send a patch.
> 
> 
> There will be two patches, this one doesn't reimplement the mentioned 
> feature but fixes a few other bugs. 
> 
> There are two questions left in that patch, apart from the handling of the 
> WWPC display as mentioned in the header. One is, if there was a reason to 
> use a fixed-size char array for the panelname. 
> 
> The other is about the use of au1100fb_fix and au1100fb_var. Is the 
> explanation in the patch accurate? Would it make more sense to write 
> directly into fbdev->info.var and fbdev->info.fixed? Also, couldn't 
> au1100fb_drv_probe() be declared as __init? After all, a late call to 
> au1100fb_drv_probe() will probably have fatal consequences anyway because 
> it depends on the above and they are declared __initdata.

I checked in a patch that was an uplift for 2.6 a couple of days ago. I'm sorry, 
I didn't catch your email in time to let you know that someone else is working 
on a patch. If you're willing to pull the latest code and see what it makes 
sense to merge from your code, including the new panel support, and send me that 
patch, I'll apply it.

Pete


> Uli
> 
> Changes:
>  * fixed typos, inconsistent formatting and trailing whitespace
>  * added a check to a kalloc() call
>  * fixed possible resource leak in au1100fb_drv_probe()
>  * replaced the fixed-size panel name with a char pointer 
>  * added a few 'const'
>  * added display data for PrimeView PD104SL5
> 
> ---
> Index: drivers/video/au1100fb.c
> ===================================================================
> RCS file: /home/cvs/linux/drivers/video/au1100fb.c,v
> retrieving revision 1.6
> diff -u -r1.6 au1100fb.c
> --- drivers/video/au1100fb.c 4 Apr 2005 01:06:20 -0000 1.6
> +++ drivers/video/au1100fb.c 6 Apr 2005 09:03:54 -0000
> @@ -3,7 +3,7 @@
>   * Au1100 LCD Driver.
>   *
>   * Rewritten for 2.6 by Embedded Alley Solutions
> - *  <source@embeddedalley.com>, based on submissions by 
> + *  <source@embeddedalley.com>, based on submissions by
>   *   Karl Lessard <klessard@sunrisetelecom.com>
>   *   <c.pellegrin@exadron.com>
>   *
> @@ -56,7 +56,7 @@
>  
>  #include "au1100fb.h"
>  
> -/* 
> +/*
>   * Sanity check. If this is a new Au1100 based board, search for
>   * the PB1100 ifdefs to make sure you modify the code accordingly.
>   */
> @@ -74,11 +74,11 @@
>  #define to_au1100fb_device(_info) \
>     (_info ? container_of(_info, struct au1100fb_device, info) : NULL);
>  
> -/* Bitfields format supported by the controller. Note that the order of formats 
> +/* Bitfields format supported by the controller. Note that the order of formats
>   * SHOULD be the same as in the LCD_CONTROL_SBPPF field, so we can retrieve the
>   * right pixel format by doing rgb_bitfields[LCD_CONTROL_SBPPF_XXX >> LCD_CONTROL_SBPPF]
>   */
> -struct fb_bitfield rgb_bitfields[][4] = 
> +struct fb_bitfield rgb_bitfields[][4] =
>  {
>     /*     Red,     Green,   Blue,       Transp   */
>   { { 10, 6, 0 }, { 5, 5, 0 }, { 0, 5, 0 }, { 0, 0, 0 } },
> @@ -91,6 +91,8 @@
>   { { 8, 4, 0 },  { 4, 4, 0 }, { 0, 4, 0 }, { 0, 0, 0 } },
>  };
>  
> +/* au1100fb_fix and au1100fb_var are local vars of au1100fb_drv_probe() but
> +defined here in order to not exhaust the stack. */
>  static struct fb_fix_screeninfo au1100fb_fix __initdata = {
>   .id  = "AU1100 FB",
>   .xpanstep  = 1,
> @@ -111,7 +113,7 @@
>  /*
>   * Set hardware with var settings. This will enable the controller with a specific
>   * mode, normally validated with the fb_check_var method
> -  */
> + */
>  int au1100fb_setmode(struct au1100fb_device *fbdev)
>  {
>   struct fb_info *info = &fbdev->info;
> @@ -142,7 +144,7 @@
>     info->var.transp.msb_right = 0;
>  
>     info->fix.visual = FB_VISUAL_PSEUDOCOLOR;
> -   info->fix.line_length = info->var.xres_virtual / 
> +   info->fix.line_length = info->var.xres_virtual /
>         (8/info->var.bits_per_pixel);
>    } else {
>     /* non-palettized */
> @@ -154,7 +156,7 @@
>  
>     info->fix.visual = FB_VISUAL_TRUECOLOR;
>     info->fix.line_length = info->var.xres_virtual << 1; /* depth=16 */
> - }
> +  }
>   } else {
>    /* mono */
>    info->fix.visual = FB_VISUAL_MONO10;
> @@ -162,7 +164,7 @@
>   }
>  
>   info->screen_size = info->fix.line_length * info->var.yres_virtual;
> - 
> +
>   /* Determine BPP mode and format */
>   fbdev->regs->lcd_control = fbdev->panel->control_base |
>         ((info->var.rotate/90) << LCD_CONTROL_SM_BIT);
> @@ -183,8 +185,8 @@
>     * otherwise display the same as the first panel */
>    if (info->var.yres_virtual >= (info->var.yres << 1)) {
>     fbdev->regs->lcd_dmaaddr1 = LCD_DMA_SA_N(fbdev->fb_phys +
> -         (info->fix.line_length * 
> -                (info->var.yres_virtual >> 1)));
> +         (info->fix.line_length *
> +         (info->var.yres_virtual >> 1)));
>    } else {
>     fbdev->regs->lcd_dmaaddr1 = LCD_DMA_SA_N(fbdev->fb_phys);
>    }
> @@ -201,7 +203,7 @@
>  
>   fbdev->regs->lcd_pwmdiv = 0;
>   fbdev->regs->lcd_pwmhi = 0;
> -   
> +
>   /* Resume controller */
>   fbdev->regs->lcd_control |= LCD_CONTROL_GO;
>  
> @@ -222,22 +224,22 @@
>  
>   if (fbi->var.grayscale) {
>    /* Convert color to grayscale */
> -  red = green = blue = 
> +  red = green = blue =
>     (19595 * red + 38470 * green + 7471 * blue) >> 16;
>   }
> -   
> +
>   if (fbi->fix.visual == FB_VISUAL_TRUECOLOR) {
>    /* Place color in the pseudopalette */
>    if (regno > 16)
>     return -EINVAL;
> -   
> +
>    palette = (u32*)fbi->pseudo_palette;
>  
>    red   >>= (16 - fbi->var.red.length);
>    green >>= (16 - fbi->var.green.length);
>    blue  >>= (16 - fbi->var.blue.length);
> - 
> -  value = (red   << fbi->var.red.offset)  | 
> +
> +  value = (red   << fbi->var.red.offset)  |
>     (green << fbi->var.green.offset)|
>     (blue  << fbi->var.blue.offset);
>    value &= 0xFFFF;
> @@ -249,8 +251,8 @@
>  
>   } else if (panel_is_color(fbdev->panel)) {
>    /* COLOR STN MODE */
> -  value = (((panel_swap_rgb(fbdev->panel) ? blue : red) >> 12) & 0x000F) | 
> -   ((green >> 8) & 0x00F0) | 
> +  value = (((panel_swap_rgb(fbdev->panel) ? blue : red) >> 12) & 0x000F) |
> +   ((green >> 8) & 0x00F0) |
>     (((panel_swap_rgb(fbdev->panel) ? red : blue) >> 4) & 0x0F00);
>    value &= 0xFFF;
>   } else {
> @@ -260,7 +262,7 @@
>   }
>  
>   palette[regno] = value;
> - 
> +
>   return 0;
>  }
>  
> @@ -277,14 +279,14 @@
>   switch (blank_mode) {
>  
>   case VESA_NO_BLANKING:
> -   /* Turn on panel */
> -   fbdev->regs->lcd_control |= LCD_CONTROL_GO;
> +  /* Turn on panel */
> +  fbdev->regs->lcd_control |= LCD_CONTROL_GO;
>  #ifdef CONFIG_MIPS_PB1100
> -   if (drv_info.panel_idx == 1) {
> -    au_writew(au_readw(PB1100_G_CONTROL) 
> -       | (PB1100_G_CONTROL_BL | PB1100_G_CONTROL_VDD), 
> -   PB1100_G_CONTROL);
> -   }
> +  if (drv_info.panel_idx == 1) {
> +   au_writew(au_readw(PB1100_G_CONTROL)
> +     | (PB1100_G_CONTROL_BL | PB1100_G_CONTROL_VDD),
> +    PB1100_G_CONTROL);
> +  }
>  #endif
>    au_sync();
>    break;
> @@ -292,20 +294,20 @@
>   case VESA_VSYNC_SUSPEND:
>   case VESA_HSYNC_SUSPEND:
>   case VESA_POWERDOWN:
> -   /* Turn off panel */
> -   fbdev->regs->lcd_control &= ~LCD_CONTROL_GO;
> +  /* Turn off panel */
> +  fbdev->regs->lcd_control &= ~LCD_CONTROL_GO;
>  #ifdef CONFIG_MIPS_PB1100
> -   if (drv_info.panel_idx == 1) {
> -    au_writew(au_readw(PB1100_G_CONTROL) 
> -         & ~(PB1100_G_CONTROL_BL | PB1100_G_CONTROL_VDD),
> -   PB1100_G_CONTROL);
> -   }
> +  if (drv_info.panel_idx == 1) {
> +   au_writew(au_readw(PB1100_G_CONTROL)
> +     & ~(PB1100_G_CONTROL_BL | PB1100_G_CONTROL_VDD),
> +    PB1100_G_CONTROL);
> +  }
>  #endif
>    au_sync();
>    break;
> - default: 
> -  break;
>  
> + default:
> +  break;
>   }
>   return 0;
>  }
> @@ -328,7 +330,7 @@
>    /* No support for X panning for now! */
>    return -EINVAL;
>   }
> -   
> +
>   print_dbg("fb_pan_display 2 %p %p", var, fbi);
>   dy = var->yoffset - fbi->var.yoffset;
>   if (dy) {
> @@ -340,14 +342,14 @@
>    dmaaddr = fbdev->regs->lcd_dmaaddr0;
>    dmaaddr += (fbi->fix.line_length * dy);
>  
> -  /* TODO: Wait for current frame to finished */
> +  /* TODO: Wait for current frame to be finished */
>    fbdev->regs->lcd_dmaaddr0 = LCD_DMA_SA_N(dmaaddr);
>  
>    if (panel_is_dual(fbdev->panel)) {
>     dmaaddr = fbdev->regs->lcd_dmaaddr1;
>     dmaaddr += (fbi->fix.line_length * dy);
>     fbdev->regs->lcd_dmaaddr0 = LCD_DMA_SA_N(dmaaddr);
> - }
> +  }
>   }
>   print_dbg("fb_pan_display 3 %p %p", var, fbi);
>  
> @@ -388,7 +390,7 @@
>   if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT)) {
>    return -EINVAL;
>   }
> -    
> +
>   start = fbdev->fb_phys & PAGE_MASK;
>   len = PAGE_ALIGN((start & ~PAGE_MASK) + fbdev->fb_len);
>  
> @@ -405,7 +407,7 @@
>   pgprot_val(vma->vm_page_prot) |= (6 << 9); //CCA=6
>  
>   vma->vm_flags |= VM_IO;
> -    
> +
>   if (io_remap_page_range(vma, vma->vm_start, off,
>      vma->vm_end - vma->vm_start,
>      vma->vm_page_prot)) {
> @@ -415,7 +417,7 @@
>   return 0;
>  }
>  
> -static struct fb_ops au1100fb_ops = 
> +static struct fb_ops au1100fb_ops =
>  {
>   .owner   = THIS_MODULE,
>   .fb_setcolreg  = au1100fb_fb_setcolreg,
> @@ -440,13 +442,14 @@
>   struct resource *regs_res;
>   unsigned long page;
>   u32 sys_clksrc;
> + int res = 0;
>  
>   if (!dev)
> -   return -EINVAL;
> +  return -EINVAL;
>  
>   /* Allocate new device private */
>   if (!(fbdev = kmalloc(sizeof(struct au1100fb_device), GFP_KERNEL))) {
> -  print_err("fail to allocate device private record");
> +  print_err("failed to allocate device private record");
>    return -ENOMEM;
>   }
>   memset((void*)fbdev, 0, sizeof(struct au1100fb_device));
> @@ -456,20 +459,22 @@
>   dev_set_drvdata(dev, (void*)fbdev);
>  
>   /* Allocate region for our registers and map them */
> - if (!(regs_res = platform_get_resource(to_platform_device(dev), 
> + if (!(regs_res = platform_get_resource(to_platform_device(dev),
>       IORESOURCE_MEM, 0))) {
> -  print_err("fail to retrieve registers resource");
> -  return -EFAULT;
> +  print_err("failed to retrieve registers resource");
> +  res = -EFAULT;
> +  goto failed;
>   }
>  
>   au1100fb_fix.mmio_start = regs_res->start;
>   au1100fb_fix.mmio_len = regs_res->end - regs_res->start + 1;
>  
> - if (!request_mem_region(au1100fb_fix.mmio_start, au1100fb_fix.mmio_len, 
> + if (!request_mem_region(au1100fb_fix.mmio_start, au1100fb_fix.mmio_len,
>      DRIVER_NAME)) {
> -  print_err("fail to lock memory region at 0x%08x", 
> +  print_err("failed to lock memory region at 0x%08x",
>      au1100fb_fix.mmio_start);
> -  return -EBUSY;
> +  res = -EBUSY;
> +  goto failed;
>   }
>  
>   fbdev->regs = (struct au1100fb_regs*)KSEG1ADDR(au1100fb_fix.mmio_start);
> @@ -478,17 +483,17 @@
>   print_dbg("phys=0x%08x, size=%d", fbdev->regs_phys, fbdev->regs_len);
>  
>  
> -
>   /* Allocate the framebuffer to the maximum screen size * nbr of video buffers */
>   fbdev->fb_len = fbdev->panel->xres * fbdev->panel->yres *
>       (fbdev->panel->bpp >> 3) * AU1100FB_NBR_VIDEO_BUFFERS;
> - 
> - fbdev->fb_mem = dma_alloc_coherent(dev, PAGE_ALIGN(fbdev->fb_len), 
> +
> + fbdev->fb_mem = dma_alloc_coherent(dev, PAGE_ALIGN(fbdev->fb_len),
>       &fbdev->fb_phys, GFP_KERNEL);
>   if (!fbdev->fb_mem) {
> -  print_err("fail to allocate frambuffer (size: %dK))", 
> +  print_err("failed to allocate frambuffer (size: %dKiB))",
>       fbdev->fb_len / 1024);
> -  return -ENOMEM;
> +  res = -ENOMEM;
> +  goto failed;
>   }
>  
>   au1100fb_fix.smem_start = fbdev->fb_phys;
> @@ -499,7 +504,7 @@
>    * since we'll be remapping normal memory.
>    */
>   for (page = (unsigned long)fbdev->fb_mem;
> -      page < PAGE_ALIGN((unsigned long)fbdev->fb_mem + fbdev->fb_len); 
> +      page < PAGE_ALIGN((unsigned long)fbdev->fb_mem + fbdev->fb_len);
>        page += PAGE_SIZE) {
>  #if CONFIG_DMA_NONCOHERENT
>    SetPageReserved(virt_to_page(CAC_ADDR(page)));
> @@ -527,15 +532,17 @@
>   fbdev->info.fix = au1100fb_fix;
>  
>   if (!(fbdev->info.pseudo_palette = kmalloc(sizeof(u32) * 16, GFP_KERNEL))) {
> -  return -ENOMEM;
> +  print_err("failed to allocate pseudo palette");
> +  res = -ENOMEM;
> +  goto failed;
>   }
>   memset(fbdev->info.pseudo_palette, 0, sizeof(u32) * 16);
>  
>   if (fb_alloc_cmap(&fbdev->info.cmap, AU1100_LCD_NBR_PALETTE_ENTRIES, 0) < 0) {
> -  print_err("Fail to allocate colormap (%d entries)",
> +  print_err("failed to allocate colormap (%d entries)",
>        AU1100_LCD_NBR_PALETTE_ENTRIES);
> -  kfree(fbdev->info.pseudo_palette);
> -  return -EFAULT;
> +  res = -EFAULT;
> +  goto failed;
>   }
>  
>   fbdev->info.var = au1100fb_var;
> @@ -546,25 +553,30 @@
>   /* Register new framebuffer */
>   if (register_framebuffer(&fbdev->info) < 0) {
>    print_err("cannot register new framebuffer");
> +  res = -EFAULT;
>    goto failed;
>   }
>  
>   return 0;
>  
>  failed:
> - if (fbdev->regs) {
> -  release_mem_region(fbdev->regs_phys, fbdev->regs_len);
> + /** Failure. Release all resources in opposite order of allocation. */
> + if (fbdev->info.cmap.len != 0) {
> +  fb_dealloc_cmap(&fbdev->info.cmap);
> + }
> + if (fbdev->info.pseudo_palette){
> +  kfree(fbdev->info.pseudo_palette);
>   }
>   if (fbdev->fb_mem) {
>    dma_free_noncoherent(dev, fbdev->fb_len, fbdev->fb_mem, fbdev->fb_phys);
>   }
> - if (fbdev->info.cmap.len != 0) {
> -  fb_dealloc_cmap(&fbdev->info.cmap);
> + if (fbdev->regs) {
> +  release_mem_region(fbdev->regs_phys, fbdev->regs_len);
>   }
> - kfree(fbdev);
>   dev_set_drvdata(dev, NULL);
> + kfree(fbdev);
>  
> - return 0;
> + return res;
>  }
>  
>  int au1100fb_drv_remove(struct device *dev)
> @@ -610,17 +622,16 @@
>  static struct device_driver au1100fb_driver = {
>   .name  = "au1100-lcd",
>   .bus  = &platform_bus_type,
> -
>   .probe  = au1100fb_drv_probe,
> -        .remove  = au1100fb_drv_remove,
> + .remove  = au1100fb_drv_remove,
>   .suspend = au1100fb_drv_suspend,
> -        .resume  = au1100fb_drv_resume,
> + .resume  = au1100fb_drv_resume,
>  };
> -    
> +
>  /*-------------------------------------------------------------------------*/
>  
>  /* Kernel driver */
> -    
> +
>  int au1100fb_setup(char *options)
>  {
>   char* this_opt;
> @@ -631,44 +642,48 @@
>   if (num_panels <= 0) {
>    print_err("No LCD panels supported by driver!");
>    return -EFAULT;
> -   }
> + }
>  
>   if (options) {
>    while ((this_opt = strsep(&options,",")) != NULL) {
> +   size_t len = strlen(this_opt);
>     /* Panel option */
> -  if (!strncmp(this_opt, "panel:", 6)) {
> +   if (!strncmp(this_opt, "panel:", 6)) {
>      int i;
>      this_opt += 6;
>      for (i = 0; i < num_panels; i++) {
>       if (!strncmp(this_opt,
> -                 known_lcd_panels[i].name, 
> -       strlen(this_opt))) {
> +                 known_lcd_panels[i].name,
> +                  len)) {
>        panel_idx = i;
> -     break;
> +      break;
> +     }
>      }
> -   }
>      if (i >= num_panels) {
>        print_warn("Panel %s not supported!", this_opt);
> +     return -ENOTSUPP;
>      }
>     }
>     /* Mode option (only option that start with digit) */
>     else if (isdigit(this_opt[0])) {
> -    mode = kmalloc(strlen(this_opt) + 1, GFP_KERNEL);
> -    strncpy(mode, this_opt, strlen(this_opt) + 1);
> +    mode = kmalloc(len+1, GFP_KERNEL);
> +    if(!mode)
> +     return -ENOMEM;
> +    strncpy(mode, this_opt, len);
>     }
>     /* Unsupported option */
>     else {
>      print_warn("Unsupported option \"%s\"", this_opt);
> +   }
>    }
> -  }
> - } 
> + }
>  
>   drv_info.panel_idx = panel_idx;
>   drv_info.opt_mode = mode;
>  
>   print_info("Panel=%s Mode=%s",
>     known_lcd_panels[drv_info.panel_idx].name,
> -         drv_info.opt_mode ? drv_info.opt_mode : "default");
> +   drv_info.opt_mode ? drv_info.opt_mode : "default");
>  
>   return 0;
>  }
> @@ -677,9 +692,9 @@
>  {
>   char* options;
>   int ret;
> - 
> +
>   print_info("" DRIVER_DESC "");
> - 
> +
>   memset(&drv_info, 0, sizeof(drv_info));
>  
>   if (fb_get_options(DRIVER_NAME, &options))
> @@ -688,7 +703,7 @@
>   /* Setup driver with options */
>   ret = au1100fb_setup(options);
>   if (ret < 0) {
> -  print_err("Fail to setup driver");
> +  print_err("Failed to setup driver");
>    return ret;
>   }
>  
> Index: drivers/video/au1100fb.h
> ===================================================================
> RCS file: /home/cvs/linux/drivers/video/au1100fb.h,v
> retrieving revision 1.2
> diff -u -r1.2 au1100fb.h
> --- drivers/video/au1100fb.h 4 Apr 2005 01:06:20 -0000 1.2
> +++ drivers/video/au1100fb.h 6 Apr 2005 09:03:54 -0000
> @@ -65,20 +65,20 @@
>  
>  struct au1100fb_panel
>  {
> - const char name[25];  /* Full name <vendor>_<model> */
> + const char* name;  /* Full name <vendor>_<model> */
>  
> - u32    control_base;  /* Mode-independent control values */
> + u32 control_base;  /* Mode-independent control values */
>   u32 clkcontrol_base; /* Panel pixclock preferences */
>  
>   u32 horztiming;
>   u32 verttiming;
>  
>   u32 xres;  /* Maximum horizontal resolution */
> - u32  yres;  /* Maximum vertical resolution */
> - u32  bpp;  /* Maximum depth supported */
> + u32 yres;  /* Maximum vertical resolution */
> + u32 bpp;  /* Maximum depth supported */
>  };
>  
> -struct au1100fb_regs 
> +struct au1100fb_regs
>  {
>   u32  lcd_control;
>   u32  lcd_intstatus;
> @@ -99,7 +99,7 @@
>  
>   struct fb_info info;   /* FB driver info record */
>  
> - struct au1100fb_panel  *panel;  /* Panel connected to this device */
> + const struct au1100fb_panel  *panel;  /* Panel connected to this device */
>  
>   struct au1100fb_regs*  regs;  /* Registers memory map */
>   size_t         regs_len;
> @@ -255,7 +255,7 @@
>  
>  /* List of panels known to work with the AU1100 LCD controller.
>   * To add a new panel, enter the same specifications as the
> - * Generic_TFT one, and MAKE SURE that it doesn't conflicts 
> + * Generic_TFT one, and MAKE SURE that it doesn't conflicts
>   * with the controller restrictions. Restrictions are:
>   *
>   * STN color panels: max_bpp <= 12
> @@ -264,7 +264,7 @@
>   * max_xres <= 800
>   * max_yres <= 600
>   */
> -static struct au1100fb_panel known_lcd_panels[] =
> +static const struct au1100fb_panel known_lcd_panels[] =
>  {
>   /* 800x600x16bpp CRT */
>   [0] = {
> @@ -272,14 +272,24 @@
>    .xres = 800,
>    .yres = 600,
>    .bpp = 16,
> -  .control_base = 0x0004886A | 
> +  .control_base = 0x0004886A |
>     LCD_CONTROL_DEFAULT_PO | LCD_CONTROL_DEFAULT_SBPPF |
>     LCD_CONTROL_BPP_16,
>    .clkcontrol_base = 0x00020000,
>    .horztiming = 0x005aff1f,
>    .verttiming = 0x16000e57,
>   },
> - /* just the standard LCD */
> +
> + /* just the standard LCD
> + TODO: There is only one case where the index in this array is checked,
> + and that also only on PB1100 boards. Clarify when exactly this is used
> + and why, and possibly integrate this differently.
> +
> + Also, I'm not sure the code is right: in older versions, WWPC had this
> + display hard-coded in, and for PB1100 and DB1100, there was a way to
> + select a display by reading the BCSR. Now, there seems to be a special
> + way of blanking/unblanking this particular display.
> + */
>   [1] = {
>    .name = "WWPC LCD",
>    .xres = 240,
> @@ -290,6 +300,7 @@
>    .verttiming = 0x0301013F,
>    .clkcontrol_base = 0x00018001,
>   },
> +
>   /* Sharp 320x240 TFT panel */
>   [2] = {
>    .name = "Sharp_LQ038Q5DR01",
> @@ -351,7 +362,7 @@
>    .clkcontrol_base = LCD_CLKCONTROL_PCD_N(1),
>   },
>  
> -  /* Pb1100 LCDB 640x480 PrimeView TFT panel */
> + /* Pb1100 LCDB 640x480 PrimeView TFT panel */
>   [5] = {
>    .name = "PrimeView_640x480_16",
>    .xres = 640,
> @@ -362,6 +373,37 @@
>    .verttiming = 0x210805df,
>    .clkcontrol_base = 0x00038001,
>   },
> +
> + /* PrimeView PD104SL5 , a 800x600 16BPP color LCD */
> + [6] = {
> +  .name = "PrimeView_PD104SL5",
> +  .xres = 800,
> +  .yres = 640,
> +  .bpp = 16,
> +  .control_base =
> +   ( LCD_CONTROL_SBPPF_565
> +   | LCD_CONTROL_C
> +   | LCD_CONTROL_SM_0
> +   | LCD_CONTROL_DEFAULT_PO
> +   | LCD_CONTROL_CCO
> +   | LCD_CONTROL_PT
> +   | LCD_CONTROL_PC
> +   | LCD_CONTROL_BPP_16 ),
> +  .horztiming =
> +   ( LCD_HORZTIMING_HN2_N(30)
> +   | LCD_HORZTIMING_HN1_N(30)
> +   | LCD_HORZTIMING_HPW_N(60)
> +   | LCD_HORZTIMING_PPL_N(800) ),
> +  .verttiming =
> +   ( LCD_VERTTIMING_VN2_N(1)
> +   | LCD_VERTTIMING_VN1_N(1)
> +   | LCD_VERTTIMING_VPW_N(2)
> +   | LCD_VERTTIMING_LPP_N(600) ),
> +  .clkcontrol_base =
> +   ( LCD_CLKCONTROL_IH
> +   | LCD_CLKCONTROL_IV
> +   | LCD_CLKCONTROL_PCD_N(1)),
> + }
>  };
>  
>  struct au1100fb_drv_info {
> 
> 

Re: Au1100 FB driver uplift for 2.6

[Ulrich Eckhardt]

On Wednesday 06 April 2005 17:53, Pete Popov wrote:
> I checked in a patch that was an uplift for 2.6 a couple of days ago. I'm
> sorry, I didn't catch your email in time to let you know that someone else
> is working on a patch. 

Pete, nothing to be sorry for, I wasn't working on anything. Point is that I 
grabbed a patch from someone else here (around 3 months ago) that updated the 
bitrotted 2.6 code to a usable state. Your version now is in some aspects 
even cleaner that that code and I'm in fact referring to the changes you 
committed a few days ago.

> If you're willing to pull the latest code and see 
> what it makes sense to merge from your code, including the new panel
> support, and send me that patch, I'll apply it.

I continuously track the latest changes, and merging the panel is perfectly 
painless, apart from this one thing with the clock setup.

thanks for your work

Uli