Krzysztof, > -----Original Message----- > From: Krzysztof Helt [mailto:krzysztof...@poczta.fm] > Sent: Friday, June 26, 2009 10:49 AM > To: Rajashekhara, Sudhakar > Cc: linux-fbdev-de...@lists.sourceforge.net; davinci-linux-open- > sou...@linux.davincidsp.com; Steve Chen; Pavel Kiryukhin > Subject: Re: [Linux-fbdev-devel] [PATCH v3] Frame Buffer driver for TI > DA8xx/OMAP-L1xx > > On Wed, 24 Jun 2009 06:58:21 -0400 > "Rajashekhara, Sudhakar" <sudhakar....@ti.com> wrote: > > > Adds LCD controller (LCDC) driver for TI's DA8xx/OMAP-L1xx architecture. > > LCDC specifications can be found at http://www.ti.com/litv/pdf/sprufm0a. > > > > LCDC on DA8xx consists of two independent controllers, the Raster Controller > > and the LCD Interface Display Driver (LIDD) controller. LIDD further > > supports > > character and graphic displays. > > > > This patch adds support for the graphic display (Sharp LQ035Q3DG01) found on > > the DA830 based EVM. The EVM details can be found at: > > http://support.spectrumdigital.com/boards/dskda830/revc/. > > > > Signed-off-by: Sudhakar Rajashekhara <sudhakar....@ti.com> > > Signed-off-by: Pavel Kiryukhin <pkiryuk...@ru.mvista.com> > > Signed-off-by: Steve Chen <sc...@mvista.com> > > --- > > Since the previous version, error paths in probing function has been > > corected, removed set_par() function and added code in fb_setcolreg() > > function for TRUECOLOR mode. > > > > drivers/video/Kconfig | 11 + > > drivers/video/Makefile | 1 + > > drivers/video/da8xx-fb.c | 887 > > ++++++++++++++++++++++++++++++++++++++++++++++ > > include/video/da8xx-fb.h | 106 ++++++ > > 4 files changed, 1005 insertions(+), 0 deletions(-) > > create mode 100644 drivers/video/da8xx-fb.c > > create mode 100644 include/video/da8xx-fb.h > > > > diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c > > new file mode 100644 > > index 0000000..a41ae62 > > --- /dev/null > > +++ b/drivers/video/da8xx-fb.c > > @@ -0,0 +1,887 @@ > > (fast forward) > > > + > > +static int fb_setcolreg(unsigned regno, unsigned red, unsigned green, > > + unsigned blue, unsigned transp, > > + struct fb_info *info) > > +{ > > + struct da8xx_fb_par *par = info->par; > > + unsigned short *palette = (unsigned short *)par->v_palette_base; > > + u_short pal; > > + > > + if (regno > 255) > > + return 1; > > + > > + if (info->fix.visual == FB_VISUAL_DIRECTCOLOR) > > + return 1; > > + > > + if (info->var.bits_per_pixel == 8) { > > + red >>= 8; > > + green >>= 8; > > + blue >>= 8; > > This function is still incorrect. Each color component value passed to the > function is 16 bit. > > I assume you have 16-bit word for each palette entry and you want > to pack 4-bit colors like this 0RGB. The shift should be > > red >>= 12; > > so you reduced the component length to 4 bits then > > red <<= 8; > > so it is put on correct position in the palette entry. > You fold these two shifts so it is > > red >>= 4; > > The same logic should be used for all color components. > > > + > > + pal = (red & 0x0f00); > > + pal |= (green & 0x00f0); > > + pal |= (blue & 0x000f); > > + > > + palette[regno] = pal; > > + > > + } else if ((info->var.bits_per_pixel == 16) && regno < 16) { > > + memset(palette, 0, 16); > > I do not understand what is achieved by zeroing the palette area. > > This is what you should use: > > red >>= (16 - info->var.red.len) > red <<= info->var.red.offset; > > The same logic should be used for all color components. > Then pack them into palette register: > > par->pseudo_palette[ regno ] = red | green | blue; > > The true color modes do not use the palette entry. I assume > that the value below forces controller to use true color mode. > > > + palette[0] = 0x4000; > > + } > > + > > + return 0; > > +} > > + > > Regards, > Krzysztof
I'll test the driver with the modifications suggested by you and will submit the updated version of the patch tomorrow. Thanks, Sudhakar _______________________________________________ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
