On 03/01/2011 01:06 PM, Maurus Cuelenaere wrote: > This patch adds SLCD support to the Ingenic Jz4740 framebuffer driver. > Besides adding the new LCD types to the platform data, this adds chip select > and register select active low support (SLCD only). Also, this exports some > functions related to starting and stopping the SLCD image transfer and sending > commands. > > A lot of this code was based on work by Maarten ter Huurne: > https://github.com/mthuurne/opendingux-kernel/commit/3dba5b5e7a868b1067acae8d1b5a19121b77bc6a > > Signed-off-by: Maurus Cuelenaere <mcuelenaere@xxxxxxxxx> Looks mostly good :) > --- > arch/mips/include/asm/mach-jz4740/jz4740_fb.h | 33 ++ > drivers/video/jz4740_fb.c | 435 ++++++++++++++++++++++--- > 2 files changed, 429 insertions(+), 39 deletions(-) > > diff --git a/arch/mips/include/asm/mach-jz4740/jz4740_fb.h b/arch/mips/include/asm/mach-jz4740/jz4740_fb.h > index 6a50e6f..eaaac43 100644 > --- a/arch/mips/include/asm/mach-jz4740/jz4740_fb.h > +++ b/arch/mips/include/asm/mach-jz4740/jz4740_fb.h > @@ -30,6 +30,13 @@ enum jz4740_fb_lcd_type { > JZ_LCD_TYPE_DUAL_COLOR_STN = 10, > JZ_LCD_TYPE_DUAL_MONOCHROME_STN = 11, > JZ_LCD_TYPE_8BIT_SERIAL = 12, > + > + JZ_SLCD_TYPE_PARALLEL_8_BIT = 1 | (1 << 5), > + JZ_SLCD_TYPE_PARALLEL_16_BIT = 0 | (1 << 5), > + JZ_SLCD_TYPE_PARALLEL_18_BIT = 2 | (1 << 5), > + JZ_SLCD_TYPE_SERIAL_8_BIT = 1 | (3 << 5), > + JZ_SLCD_TYPE_SERIAL_16_BIT = 0 | (3 << 5), > + JZ_SLCD_TYPE_SERIAL_18_BIT = 2 | (3 << 5), I would prefer JZ_LCD_TYPE_SLCD_... > }; Maybe add #define JZ_LCD_TYPE_SLCD (1 << 5) #define JZ_LCD_TYPE_SLCD_SERIAL (JZ_LCD_TYPE_SLCD | (1 << 6)) > > #define JZ4740_FB_SPECIAL_TFT_CONFIG(start, stop) (((start) << 16) | (stop)) > @@ -62,6 +69,32 @@ struct jz4740_fb_platform_data { > > unsigned pixclk_falling_edge:1; > unsigned date_enable_active_low:1; > + unsigned chip_select_active_low:1; > + unsigned register_select_active_low:1; > }; > > +struct platform_device; > + > +/* > + * JzFB SLCD related functions > + * > + * jz4740_fb_slcd_disable_transfer: disables the current image transfer going > + * on between memory and the LCD controller > + * jz4740_fb_slcd_enable_transfer: the reverse operation of the above > + * jz4740_fb_slcd_send_cmd: sends a command without any data to the LCD > + * controller > + * jz4740_fb_slcd_send_cmd_data: sends a command with a data argument to the LCD > + * controller > + * > + * These functions can sleep and thus should not be called from an atomic > + * context. Also, make sure you disable the SLCD image transfer *before* sending > + * any commands and do not forget to re-enable it. > + */ > +extern void jz4740_fb_slcd_disable_transfer(struct platform_device *pdev); > +extern void jz4740_fb_slcd_enable_transfer(struct platform_device *pdev); > +extern void jz4740_fb_slcd_send_cmd_data(struct platform_device *pdev, > + unsigned int cmd, unsigned int data); > +extern void jz4740_fb_slcd_send_cmd(struct platform_device *pdev, > + unsigned int cmd); > + > #endif > diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c > index 2f3ea57..4064812 100644 > --- a/drivers/video/jz4740_fb.c > +++ b/drivers/video/jz4740_fb.c > @@ -26,6 +26,7 @@ > > #include <linux/dma-mapping.h> > > +#include <asm/mach-jz4740/dma.h> > #include <asm/mach-jz4740/jz4740_fb.h> > #include <asm/mach-jz4740/gpio.h> > > @@ -107,6 +108,40 @@ > > #define JZ_LCD_STATE_DISABLED BIT(0) > > +#define JZ_REG_SLCD_CFG 0xA0 > +#define JZ_REG_SLCD_CTRL 0xA4 > +#define JZ_REG_SLCD_STATE 0xA8 > +#define JZ_REG_SLCD_DATA 0xAC > +#define JZ_REG_SLCD_FIFO 0xB0 > + > +#define JZ_SLCD_CFG_BURST_8_WORD BIT(14) > +#define JZ_SLCD_CFG_DWIDTH_MASK (7 << 10) > +#define JZ_SLCD_CFG_DWIDTH_18 (0 << 10) > +#define JZ_SLCD_CFG_DWIDTH_16 (1 << 10) > +#define JZ_SLCD_CFG_DWIDTH_8_x3 (2 << 10) > +#define JZ_SLCD_CFG_DWIDTH_8_x2 (3 << 10) > +#define JZ_SLCD_CFG_DWIDTH_8_x1 (4 << 10) > +#define JZ_SLCD_CFG_DWIDTH_9_x2 (7 << 10) > +#define JZ_SLCD_CFG_CWIDTH_MASK (3 << 8) > +#define JZ_SLCD_CFG_CWIDTH(n) ((n) << 8) > +#define JZ_SLCD_CFG_CWIDTH_16BIT (0 << 8) > +#define JZ_SLCD_CFG_CWIDTH_8BIT (1 << 8) > +#define JZ_SLCD_CFG_CWIDTH_18BIT (2 << 8) > +#define JZ_SLCD_CFG_CS_ACTIVE_HIGH BIT(4) > +#define JZ_SLCD_CFG_RS_CMD_HIGH BIT(3) > +#define JZ_SLCD_CFG_CLK_ACTIVE_RISING BIT(1) > +#define JZ_SLCD_CFG_TYPE_SERIAL BIT(0) > + > +#define JZ_SLCD_CTRL_DMA_EN (1 << 0) > + > +#define JZ_SLCD_STATE_BUSY (1 << 0) > + > +#define JZ_SLCD_DATA_RS_DATA (0 << 31) > +#define JZ_SLCD_DATA_RS_COMMAND (1 << 31) > + > +#define JZ_SLCD_FIFO_RS_DATA (0 << 31) > +#define JZ_SLCD_FIFO_RS_COMMAND (1 << 31) > + > struct jzfb_framedesc { > uint32_t next; > uint32_t addr; > @@ -118,6 +153,7 @@ struct jzfb { > struct fb_info *fb; > struct platform_device *pdev; > void __iomem *base; > + phys_t phys_base; > struct resource *mem; > struct jz4740_fb_platform_data *pdata; > > @@ -126,6 +162,7 @@ struct jzfb { > dma_addr_t vidmem_phys; > struct jzfb_framedesc *framedesc; > dma_addr_t framedesc_phys; > + struct jz4740_dma_chan *slcd_dma; > > struct clk *ldclk; > struct clk *lpclk; > @@ -136,6 +173,9 @@ struct jzfb { > uint32_t pseudo_palette[16]; > }; > > +#define JZFB_IS_SLCD(jzfb) ((jzfb)->pdata->lcd_type & (1 << 5)) > +#define JZFB_IS_SLCD_SERIAL_TYPE(jzfb) ((jzfb)->pdata->lcd_type & (2 << 5)) These two should be static inline bool. > +static void send_panel_command(struct jzfb *jzfb, u32 cmd) You've been using u{16,32} throughout the whole patch, since the existing code uses uint_{16,32}t you should stick to them. > +{ > + u16 slcd_cfg = readw(jzfb->base + JZ_REG_SLCD_CFG); > + > + jzfb_slcd_wait(jzfb); > + > + switch (slcd_cfg & JZ_SLCD_CFG_CWIDTH_MASK) { > + case JZ_SLCD_CFG_CWIDTH_8BIT: > + writel(JZ_SLCD_DATA_RS_COMMAND | ((cmd & 0xff00) >> 8), > + jzfb->base + JZ_REG_SLCD_DATA); > + jzfb_slcd_wait(jzfb); > + writel(JZ_SLCD_DATA_RS_COMMAND | (cmd & 0xff), > + jzfb->base + JZ_REG_SLCD_DATA); > + break; > + > + case JZ_SLCD_CFG_CWIDTH_16BIT: > + writel(JZ_SLCD_DATA_RS_COMMAND | (cmd & 0xffff), > + jzfb->base + JZ_REG_SLCD_DATA); > + break; > + > + case JZ_SLCD_CFG_CWIDTH_18BIT: > + writel(JZ_SLCD_DATA_RS_COMMAND | ((cmd & 0xff00) << 2) | > + ((cmd & 0xff) << 1), jzfb->base + JZ_REG_SLCD_DATA); > + break; > + } > +} > + > +static void send_panel_data(struct jzfb *jzfb, u32 data) > +{ > + u16 slcd_cfg = readw(jzfb->base + JZ_REG_SLCD_CFG); > + > + switch (slcd_cfg & JZ_SLCD_CFG_DWIDTH_MASK) { > + case JZ_SLCD_CFG_DWIDTH_18: > + case JZ_SLCD_CFG_DWIDTH_9_x2: > + data = ((data & 0xff) << 1) | ((data & 0xff00) << 2); > + data = ((data << 6) & 0xfc0000) | ((data << 4) & 0xfc00) | > + ((data << 2) & 0x0000fc); > + break; > + > + case JZ_SLCD_CFG_DWIDTH_16: > + default: > + data &= 0xffff; > + break; > + } > + > + jzfb_slcd_wait(jzfb); > + writel(JZ_SLCD_DATA_RS_DATA | data, jzfb->base + JZ_REG_SLCD_DATA); > +} > + > +void jz4740_fb_slcd_disable_transfer(struct platform_device *pdev) > +{ > + struct jzfb *jzfb = platform_get_drvdata(pdev); > + > + mutex_lock(&jzfb->lock); > + > + if (jzfb->is_enabled) { > + jz4740_dma_disable(jzfb->slcd_dma); > + jzfb_slcd_wait(jzfb); > + } > + > + mutex_unlock(&jzfb->lock); > +} > +EXPORT_SYMBOL_GPL(jz4740_fb_slcd_disable_transfer); > + > +void jz4740_fb_slcd_enable_transfer(struct platform_device *pdev) > +{ > + struct jzfb *jzfb = platform_get_drvdata(pdev); > + > + mutex_lock(&jzfb->lock); > + > + if (jzfb->is_enabled) > + jzfb_slcd_start_dma(jzfb); > + > + mutex_unlock(&jzfb->lock); > +} > +EXPORT_SYMBOL_GPL(jz4740_fb_slcd_enable_transfer); > + > +void jz4740_fb_slcd_send_cmd_data(struct platform_device *pdev, > + unsigned int cmd, unsigned int data) > +{ > + struct jzfb *jzfb = platform_get_drvdata(pdev); > + > + mutex_lock(&jzfb->lock); > + > + if (!jzfb->is_enabled) > + clk_enable(jzfb->ldclk); > + > + send_panel_command(jzfb, cmd); > + send_panel_data(jzfb, data); > + > + if (!jzfb->is_enabled) { > + jzfb_slcd_wait(jzfb); > + clk_disable(jzfb->ldclk); > + } > + > + mutex_unlock(&jzfb->lock); > +} > +EXPORT_SYMBOL_GPL(jz4740_fb_slcd_send_cmd_data); > + > +void jz4740_fb_slcd_send_cmd(struct platform_device *pdev, unsigned int cmd) > +{ > + struct jzfb *jzfb = platform_get_drvdata(pdev); > + > + mutex_lock(&jzfb->lock); > + > + if (!jzfb->is_enabled) > + clk_enable(jzfb->ldclk); > + > + send_panel_command(jzfb, cmd); > + > + if (!jzfb->is_enabled) { > + jzfb_slcd_wait(jzfb); > + clk_disable(jzfb->ldclk); > + } > + > + mutex_unlock(&jzfb->lock); > +} > +EXPORT_SYMBOL_GPL(jz4740_fb_slcd_send_cmd); jz4740_fb_slcd_send_cmd_data and jz4740_fb_slcd_send_cmd are almost identical it might makes sense to merge them. I don't like the idea of adding a jzfb specific interface for talking to the lcd panels, because it will tightly couple the panel driver to the framebuffer driver and it won't be possible to reuse for some board using the same panel but a different SoC. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
