| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4C3740E5.5080200@metafoo.de>
Date: Fri, 09 Jul 2010 17:31:49 +0200
From: Lars-Peter Clausen <lars@...afoo.de>
To: Jaya Kumar <jayakumar.lkml@...il.com>
CC: Andrew Morton <akpm@...ux-foundation.org>,
linux-mips@...ux-mips.org, linux-kernel@...r.kernel.org,
Ralf Baechle <ralf@...ux-mips.org>, linux-fbdev@...r.kernel.org
Subject: Re: [PATCH v2 16/26] fbdev: Add JZ4740 framebuffer driver
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Jaya Kumar wrote:
> On Mon, Jul 5, 2010 at 6:27 AM, Lars-Peter Clausen <lars@...afoo.de> wrote:
>> Hi Andrew
>>
>> v2 of this patch has been around for two weeks without any comments. I've fixed all
>> the issues you had with v1. If you this version is ok, an Acked-By would be nice :)
>>
>> Thanks
>> - Lars
>>
>> Lars-Peter Clausen wrote:
>>> This patch adds support for the LCD controller on JZ4740 SoCs.
>
> Hi Lars, Andrew,
>
> I think this driver looks okay but I had some quick questions:
>
>>> +config FB_JZ4740
>>> + tristate "JZ4740 LCD framebuffer support"
>>> + depends on FB
>>> + select FB_SYS_FILLRECT
>>> + select FB_SYS_COPYAREA
>>> + select FB_SYS_IMAGEBLIT
>
> Out of curiosity, why FB_SYS and not FB_CFB? The users of FB_SYS are
> drivers that use virtual memory for their framebuffer. I didn't see
> any vmalloc in this driver.
The framebuffer is allocated with dma_alloc_coherent and thus lives in system memory.
>
>>> + help
>>> + Framebuffer support for the JZ4740 SoC.
>>> +
>>> source "drivers/video/omap/Kconfig"
>>> source "drivers/video/omap2/Kconfig"
>>>
>>> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
>>> index 3c3bf86..fd2df57 100644
>>> --- a/drivers/video/Makefile
>>> +++ b/drivers/video/Makefile
>>> @@ -132,6 +132,7 @@ obj-$(CONFIG_FB_CARMINE) += carminefb.o
>>> obj-$(CONFIG_FB_MB862XX) += mb862xx/
>>> obj-$(CONFIG_FB_MSM) += msm/
>>> obj-$(CONFIG_FB_NUC900) += nuc900fb.o
>>> +obj-$(CONFIG_FB_JZ4740) += jz4740_fb.o
>
> Just a minor nit, the typical naming in fbdev appears to be
> controllerfb.c as opposed to controller_fb.c .
>
>>> # Platform or fallback drivers go here
>>> obj-$(CONFIG_FB_UVESA) += uvesafb.o
>>> diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
>>> new file mode 100644
>>> index 0000000..8d03181
>>> --- /dev/null
>>> +++ b/drivers/video/jz4740_fb.c
>>> @@ -0,0 +1,817 @@
>>> +/*
>>> + * Copyright (C) 2009-2010, Lars-Peter Clausen <lars@...afoo.de>
>>> + * JZ4740 SoC LCD framebuffer driver
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License as published by the
>>> + * Free Software Foundation; either version 2 of the License, or (at your
>>> + * option) any later version.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along
>>> + * with this program; if not, write to the Free Software Foundation, Inc.,
>>> + * 675 Mass Ave, Cambridge, MA 02139, USA.
>>> + *
>>> + */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/delay.h>
>>> +
>>> +#include <linux/console.h>
>>> +#include <linux/fb.h>
>>> +
>>> +#include <linux/dma-mapping.h>
>>> +
>>> +#include <linux/jz4740_fb.h>
>
> I see a lot of register defines below. Is there any reason why they
> shouldn't go in above j*fb.h? Also, I'm not sure if linux/j*.h is the
> best place. I think most fbdev drivers place their headers in
> drivers/video itself unless they need to share something driver
> specific with userspace which is rare. Is there a specific reason why
> you put it in include/linux?
>
>
The register definitions are only used by the driver, so there is no reason why they
should be in a different file.
The header file defines the platform data struct for this driver. But I guess the
better location for it is include/video.
>>> +#include <asm/mach-jz4740/gpio.h>
>>> +
>>> +#define JZ_REG_LCD_CFG 0x00
>>> +#define JZ_REG_LCD_VSYNC 0x04
>>> +#define JZ_REG_LCD_HSYNC 0x08
>>> +#define JZ_REG_LCD_VAT 0x0C
>>> +#define JZ_REG_LCD_DAH 0x10
>>> +#define JZ_REG_LCD_DAV 0x14
>>> +#define JZ_REG_LCD_PS 0x18
>>> +#define JZ_REG_LCD_CLS 0x1C
>>> +#define JZ_REG_LCD_SPL 0x20
>>> +#define JZ_REG_LCD_REV 0x24
>>> +#define JZ_REG_LCD_CTRL 0x30
>>> +#define JZ_REG_LCD_STATE 0x34
>>> +#define JZ_REG_LCD_IID 0x38
>>> +#define JZ_REG_LCD_DA0 0x40
>>> +#define JZ_REG_LCD_SA0 0x44
>>> +#define JZ_REG_LCD_FID0 0x48
>>> +#define JZ_REG_LCD_CMD0 0x4C
>>> +#define JZ_REG_LCD_DA1 0x50
>>> +#define JZ_REG_LCD_SA1 0x54
>>> +#define JZ_REG_LCD_FID1 0x58
>>> +#define JZ_REG_LCD_CMD1 0x5C
>>> +
>>> +#define JZ_LCD_CFG_SLCD BIT(31)
>>> +#define JZ_LCD_CFG_PS_DISABLE BIT(23)
>>> +#define JZ_LCD_CFG_CLS_DISABLE BIT(22)
>>> +#define JZ_LCD_CFG_SPL_DISABLE BIT(21)
>>> +#define JZ_LCD_CFG_REV_DISABLE BIT(20)
>>> +#define JZ_LCD_CFG_HSYNCM BIT(19)
>>> +#define JZ_LCD_CFG_PCLKM BIT(18)
>>> +#define JZ_LCD_CFG_INV BIT(17)
>>> +#define JZ_LCD_CFG_SYNC_DIR BIT(16)
>>> +#define JZ_LCD_CFG_PS_POLARITY BIT(15)
>>> +#define JZ_LCD_CFG_CLS_POLARITY BIT(14)
>>> +#define JZ_LCD_CFG_SPL_POLARITY BIT(13)
>>> +#define JZ_LCD_CFG_REV_POLARITY BIT(12)
>>> +#define JZ_LCD_CFG_HSYNC_ACTIVE_LOW BIT(11)
>>> +#define JZ_LCD_CFG_PCLK_FALLING_EDGE BIT(10)
>>> +#define JZ_LCD_CFG_DE_ACTIVE_LOW BIT(9)
>>> +#define JZ_LCD_CFG_VSYNC_ACTIVE_LOW BIT(8)
>>> +#define JZ_LCD_CFG_18_BIT BIT(7)
>>> +#define JZ_LCD_CFG_PDW (BIT(5) | BIT(4))
>>> +#define JZ_LCD_CFG_MODE_MASK 0xf
>>> +
>>> +#define JZ_LCD_CTRL_BURST_4 (0x0 << 28)
>>> +#define JZ_LCD_CTRL_BURST_8 (0x1 << 28)
>>> +#define JZ_LCD_CTRL_BURST_16 (0x2 << 28)
>>> +#define JZ_LCD_CTRL_RGB555 BIT(27)
>>> +#define JZ_LCD_CTRL_OFUP BIT(26)
>>> +#define JZ_LCD_CTRL_FRC_GRAYSCALE_16 (0x0 << 24)
>>> +#define JZ_LCD_CTRL_FRC_GRAYSCALE_4 (0x1 << 24)
>>> +#define JZ_LCD_CTRL_FRC_GRAYSCALE_2 (0x2 << 24)
>>> +#define JZ_LCD_CTRL_PDD_MASK (0xff << 16)
>>> +#define JZ_LCD_CTRL_EOF_IRQ BIT(13)
>>> +#define JZ_LCD_CTRL_SOF_IRQ BIT(12)
>>> +#define JZ_LCD_CTRL_OFU_IRQ BIT(11)
>>> +#define JZ_LCD_CTRL_IFU0_IRQ BIT(10)
>>> +#define JZ_LCD_CTRL_IFU1_IRQ BIT(9)
>>> +#define JZ_LCD_CTRL_DD_IRQ BIT(8)
>>> +#define JZ_LCD_CTRL_QDD_IRQ BIT(7)
>>> +#define JZ_LCD_CTRL_REVERSE_ENDIAN BIT(6)
>>> +#define JZ_LCD_CTRL_LSB_FISRT BIT(5)
>>> +#define JZ_LCD_CTRL_DISABLE BIT(4)
>>> +#define JZ_LCD_CTRL_ENABLE BIT(3)
>>> +#define JZ_LCD_CTRL_BPP_1 0x0
>>> +#define JZ_LCD_CTRL_BPP_2 0x1
>>> +#define JZ_LCD_CTRL_BPP_4 0x2
>>> +#define JZ_LCD_CTRL_BPP_8 0x3
>>> +#define JZ_LCD_CTRL_BPP_15_16 0x4
>>> +#define JZ_LCD_CTRL_BPP_18_24 0x5
>>> +
>>> +#define JZ_LCD_CMD_SOF_IRQ BIT(15)
>>> +#define JZ_LCD_CMD_EOF_IRQ BIT(16)
>>> +#define JZ_LCD_CMD_ENABLE_PAL BIT(12)
>>> +
>>> +#define JZ_LCD_SYNC_MASK 0x3ff
>>> +
>>> +#define JZ_LCD_STATE_DISABLED BIT(0)
>>> +
>>> +struct jzfb_framedesc {
>>> + uint32_t next;
>>> + uint32_t addr;
>>> + uint32_t id;
>>> + uint32_t cmd;
>>> +} __packed;
>>> +
>>> +struct jzfb {
>>> + struct fb_info *fb;
>>> + struct platform_device *pdev;
>>> + void __iomem *base;
>>> + struct resource *mem;
>>> + struct jz4740_fb_platform_data *pdata;
>>> +
>>> + size_t vidmem_size;
>>> + void *vidmem;
>>> + dma_addr_t vidmem_phys;
>>> + struct jzfb_framedesc *framedesc;
>>> + dma_addr_t framedesc_phys;
>>> +
>>> + struct clk *ldclk;
>>> + struct clk *lpclk;
>>> +
>>> + unsigned is_enabled:1;
>
> Out of curiosity, why the bitfield?
>
>>> + struct mutex lock;
>>> +
>>> + uint32_t pseudo_palette[16];
>>> +};
>>> +
>>> +static const struct fb_fix_screeninfo jzfb_fix __devinitdata = {
>>> + .id = "JZ4740 FB",
>>> + .type = FB_TYPE_PACKED_PIXELS,
>>> + .visual = FB_VISUAL_TRUECOLOR,
>>> + .xpanstep = 0,
>>> + .ypanstep = 0,
>>> + .ywrapstep = 0,
>>> + .accel = FB_ACCEL_NONE,
>>> +};
>>> +
>>> +static const struct jz_gpio_bulk_request jz_lcd_ctrl_pins[] = {
>>> + JZ_GPIO_BULK_PIN(LCD_PCLK),
>>> + JZ_GPIO_BULK_PIN(LCD_HSYNC),
>>> + JZ_GPIO_BULK_PIN(LCD_VSYNC),
>>> + JZ_GPIO_BULK_PIN(LCD_DE),
>>> + JZ_GPIO_BULK_PIN(LCD_PS),
>>> + JZ_GPIO_BULK_PIN(LCD_REV),
>>> +};
>>> +
>>> +static const struct jz_gpio_bulk_request jz_lcd_data_pins[] = {
>>> + JZ_GPIO_BULK_PIN(LCD_DATA0),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA1),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA2),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA3),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA4),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA5),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA6),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA7),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA8),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA9),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA10),
f>>> + JZ_GPIO_BULK_PIN(LCD_DATA11),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA12),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA13),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA14),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA15),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA16),
>>> + JZ_GPIO_BULK_PIN(LCD_DATA17),
>>> +};
>>> +
>>> +static unsigned int jzfb_num_ctrl_pins(struct jzfb *jzfb)
>>> +{
>>> + unsigned int num;
>>> +
>>> + switch (jzfb->pdata->lcd_type) {
>>> + case JZ_LCD_TYPE_GENERIC_16_BIT:
>>> + num = 4;
>>> + break;
>>> + case JZ_LCD_TYPE_GENERIC_18_BIT:
>>> + num = 4;
>>> + break;
>>> + case JZ_LCD_TYPE_8BIT_SERIAL:
>>> + num = 3;
>>> + break;
>
> The lcd type serial looks interesting to me. It'd be nice to have some
> descriptive comments so that we know what its used with.
>
Uhm, its a standard 8bit serial connection. 3 channels (R,G,B) and 8 bits per
channel. And the channels are send one after another over the wire.
>>> + default:
>>> + num = 0;
>>> + break;
>>> + }
>>> + return num;
>>> +}
>>> +
>>> +static unsigned int jzfb_num_data_pins(struct jzfb *jzfb)
>>> +{
>>> + unsigned int num;
>>> +
>>> + switch (jzfb->pdata->lcd_type) {
>>> + case JZ_LCD_TYPE_GENERIC_16_BIT:
>>> + num = 16;
>>> + break;
>>> + case JZ_LCD_TYPE_GENERIC_18_BIT:
>>> + num = 18;
>>> + break;
>>> + case JZ_LCD_TYPE_8BIT_SERIAL:
>>> + num = 8;
>>> + break;
>>> + default:
>>> + num = 0;
>>> + break;
>>> + }
>>> + return num;
>>> +}
>>> +
>>> +/* Based on CNVT_TOHW macro from skeletonfb.c */
>>> +static inline uint32_t jzfb_convert_color_to_hw(unsigned val,
>>> + struct fb_bitfield *bf)
>>> +{
>>> + return (((val << bf->length) + 0x7FFF - val) >> 16) << bf->offset;
>>> +}
>>> +
>>> +static int jzfb_setcolreg(unsigned regno, unsigned red, unsigned green,
>>> + unsigned blue, unsigned transp, struct fb_info *fb)
>>> +{
>>> + uint32_t color;
>>> +
>>> + if (regno >= 16)
>>> + return -EINVAL;
>>> +
>>> + color = jzfb_convert_color_to_hw(red, &fb->var.red);
>>> + color |= jzfb_convert_color_to_hw(green, &fb->var.green);
>>> + color |= jzfb_convert_color_to_hw(blue, &fb->var.blue);
>>> + color |= jzfb_convert_color_to_hw(transp, &fb->var.transp);
>>> +
>>> + ((uint32_t *)(fb->pseudo_palette))[regno] = color;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int jzfb_get_controller_bpp(struct jzfb *jzfb)
>>> +{
>>> + switch (jzfb->pdata->bpp) {
>>> + case 18:
>>> + case 24:
>>> + return 32;
>>> + case 15:
>>> + return 16;
>>> + default:
>>> + return jzfb->pdata->bpp;
>>> + }
>>> +}
>>> +
>>> +static struct fb_videomode *jzfb_get_mode(struct jzfb *jzfb,
>>> + struct fb_var_screeninfo *var)
>>> +{
>>> + size_t i;
>>> + struct fb_videomode *mode = jzfb->pdata->modes;
>>> +
>>> + for (i = 0; i < jzfb->pdata->num_modes; ++i, ++mode) {
>>> + if (mode->xres == var->xres && mode->yres == var->yres)
>>> + return mode;
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +
>>> +static int jzfb_check_var(struct fb_var_screeninfo *var, struct fb_info *fb)
>>> +{
>>> + struct jzfb *jzfb = fb->par;
>>> + struct fb_videomode *mode;
>>> +
>>> + if (var->bits_per_pixel != jzfb_get_controller_bpp(jzfb) &&
>>> + var->bits_per_pixel != jzfb->pdata->bpp)
>>> + return -EINVAL;
>>> +
>>> + mode = jzfb_get_mode(jzfb, var);
>>> + if (mode == NULL)
>>> + return -EINVAL;
>>> +
>>> + fb_videomode_to_var(var, mode);
>>> +
>>> + switch (jzfb->pdata->bpp) {
>>> + case 8:
>>> + break;
>>> + case 15:
>>> + var->red.offset = 10;
>>> + var->red.length = 5;
>>> + var->green.offset = 6;
>>> + var->green.length = 5;
>>> + var->blue.offset = 0;
>>> + var->blue.length = 5;
>>> + break;
>>> + case 16:
>>> + var->red.offset = 11;
>>> + var->red.length = 5;
>>> + var->green.offset = 5;
>>> + var->green.length = 6;
>>> + var->blue.offset = 0;
>>> + var->blue.length = 5;
>>> + break;
>>> + case 18:
>>> + var->red.offset = 16;
>>> + var->red.length = 6;
>>> + var->green.offset = 8;
>>> + var->green.length = 6;
>>> + var->blue.offset = 0;
>>> + var->blue.length = 6;
>>> + var->bits_per_pixel = 32;
>>> + break;
>>> + case 32:
>>> + case 24:
>>> + var->transp.offset = 24;
>>> + var->transp.length = 8;
>>> + var->red.offset = 16;
>>> + var->red.length = 8;
>>> + var->green.offset = 8;
>>> + var->green.length = 8;
>>> + var->blue.offset = 0;
>>> + var->blue.length = 8;
>>> + var->bits_per_pixel = 32;
>>> + break;
>>> + default:
>>> + break;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int jzfb_set_par(struct fb_info *info)
>>> +{
>>> + struct jzfb *jzfb = info->par;
>>> + struct fb_var_screeninfo *var = &info->var;
>>> + struct fb_videomode *mode;
>>> + uint16_t hds, vds;
>>> + uint16_t hde, vde;
>>> + uint16_t ht, vt;
>>> + uint32_t ctrl;
>>> + uint32_t cfg;
>>> + unsigned long rate;
>>> +
>>> + mode = jzfb_get_mode(jzfb, var);
>>> + if (mode == NULL)
>>> + return -EINVAL;
>>> +
>>> + if (mode == info->mode)
>>> + return 0;
>>> +
>>> + info->mode = mode;
>>> +
>>> + hds = mode->hsync_len + mode->left_margin;
>>> + hde = hds + mode->xres;
>>> + ht = hde + mode->right_margin;
>>> +
>>> + vds = mode->vsync_len + mode->upper_margin;
>>> + vde = vds + mode->yres;
>>> + vt = vde + mode->lower_margin;
>>> +
>>> + ctrl = JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_16;
>>> +
>>> + switch (jzfb->pdata->bpp) {
>>> + case 1:
>>> + ctrl |= JZ_LCD_CTRL_BPP_1;
>>> + break;
>>> + case 2:
>>> + ctrl |= JZ_LCD_CTRL_BPP_2;
>>> + break;
>>> + case 4:
>>> + ctrl |= JZ_LCD_CTRL_BPP_4;
>>> + break;
>>> + case 8:
>>> + ctrl |= JZ_LCD_CTRL_BPP_8;
>>> + break;
>>> + case 15:
>>> + ctrl |= JZ_LCD_CTRL_RGB555; /* Falltrough */
>>> + case 16:
>>> + ctrl |= JZ_LCD_CTRL_BPP_15_16;
>>> + break;
>>> + case 18:
>>> + case 24:
>>> + case 32:
>>> + ctrl |= JZ_LCD_CTRL_BPP_18_24;
>>> + break;
>>> + default:
>>> + break;
>>> + }
>>> +
>>> + cfg = JZ_LCD_CFG_PS_DISABLE | JZ_LCD_CFG_CLS_DISABLE |
>>> + JZ_LCD_CFG_SPL_DISABLE | JZ_LCD_CFG_REV_DISABLE;
>>> +
>>> + if (!(mode->sync & FB_SYNC_HOR_HIGH_ACT))
>>> + cfg |= JZ_LCD_CFG_HSYNC_ACTIVE_LOW;
>>> +
>>> + if (!(mode->sync & FB_SYNC_VERT_HIGH_ACT))
>>> + cfg |= JZ_LCD_CFG_VSYNC_ACTIVE_LOW;
>>> +
>>> + if (jzfb->pdata->pixclk_falling_edge)
>>> + cfg |= JZ_LCD_CFG_PCLK_FALLING_EDGE;
>>> +
>>> + if (jzfb->pdata->date_enable_active_low)
>>> + cfg |= JZ_LCD_CFG_DE_ACTIVE_LOW;
>>> +
>>> + if (jzfb->pdata->lcd_type == JZ_LCD_TYPE_GENERIC_18_BIT)
>>> + cfg |= JZ_LCD_CFG_18_BIT;
>>> +
>>> + cfg |= jzfb->pdata->lcd_type & 0xf;
>>> +
>>> + if (mode->pixclock) {
>>> + rate = PICOS2KHZ(mode->pixclock) * 1000;
>>> + mode->refresh = rate / vt / ht;
>>> + } else {
>>> + if (jzfb->pdata->lcd_type == JZ_LCD_TYPE_8BIT_SERIAL)
>>> + rate = mode->refresh * (vt + 2 * mode->xres) * ht;
>>> + else
>>> + rate = mode->refresh * vt * ht;
>>> +
>>> + mode->pixclock = KHZ2PICOS(rate / 1000);
>>> + }
>>> +
>>> + mutex_lock(&jzfb->lock);
>>> + if (!jzfb->is_enabled)
>>> + clk_enable(jzfb->ldclk);
>>> + else
>>> + ctrl |= JZ_LCD_CTRL_ENABLE;
>>> +
>>> + writel(mode->hsync_len, jzfb->base + JZ_REG_LCD_HSYNC);
>>> + writel(mode->vsync_len, jzfb->base + JZ_REG_LCD_VSYNC);
>>> +
>>> + writel((ht << 16) | vt, jzfb->base + JZ_REG_LCD_VAT);
>>> +
>>> + writel((hds << 16) | hde, jzfb->base + JZ_REG_LCD_DAH);
>>> + writel((vds << 16) | vde, jzfb->base + JZ_REG_LCD_DAV);
>>> +
>>> + writel(cfg, jzfb->base + JZ_REG_LCD_CFG);
>>> +
>>> + writel(ctrl, jzfb->base + JZ_REG_LCD_CTRL);
>>> +
>>> + if (!jzfb->is_enabled)
>>> + clk_disable(jzfb->ldclk);
>>> +
>>> + mutex_unlock(&jzfb->lock);
>>> +
>>> + clk_set_rate(jzfb->lpclk, rate);
>>> + clk_set_rate(jzfb->ldclk, rate * 3);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void jzfb_enable(struct jzfb *jzfb)
>>> +{
>>> + uint32_t ctrl;
>>> +
>>> + clk_enable(jzfb->ldclk);
>>> +
>>> + jz_gpio_bulk_resume(jz_lcd_ctrl_pins, jzfb_num_ctrl_pins(jzfb));
>>> + jz_gpio_bulk_resume(jz_lcd_data_pins, jzfb_num_data_pins(jzfb));
>>> +
>>> + writel(0, jzfb->base + JZ_REG_LCD_STATE);
>>> +
>>> + writel(jzfb->framedesc->next, jzfb->base + JZ_REG_LCD_DA0);
>>> +
>>> + ctrl = readl(jzfb->base + JZ_REG_LCD_CTRL);
>>> + ctrl |= JZ_LCD_CTRL_ENABLE;
>>> + ctrl &= ~JZ_LCD_CTRL_DISABLE;
>>> + writel(ctrl, jzfb->base + JZ_REG_LCD_CTRL);
>>> +}
>>> +
>>> +static void jzfb_disable(struct jzfb *jzfb)
>>> +{
>>> + uint32_t ctrl;
>>> +
>>> + ctrl = readl(jzfb->base + JZ_REG_LCD_CTRL);
>>> + ctrl |= JZ_LCD_CTRL_DISABLE;
>>> + writel(ctrl, jzfb->base + JZ_REG_LCD_CTRL);
>>> + do {
>>> + ctrl = readl(jzfb->base + JZ_REG_LCD_STATE);
>>> + } while (!(ctrl & JZ_LCD_STATE_DISABLED));
>>> +
>>> + jz_gpio_bulk_suspend(jz_lcd_ctrl_pins, jzfb_num_ctrl_pins(jzfb));
>>> + jz_gpio_bulk_suspend(jz_lcd_data_pins, jzfb_num_data_pins(jzfb));
>>> +
>>> + clk_disable(jzfb->ldclk);
>>> +}
>>> +
>>> +static int jzfb_blank(int blank_mode, struct fb_info *info)
>>> +{
>>> + struct jzfb *jzfb = info->par;
>>> +
>>> + switch (blank_mode) {
>>> + case FB_BLANK_UNBLANK:
>>> + mutex_lock(&jzfb->lock);
>>> + if (jzfb->is_enabled) {
>>> + mutex_unlock(&jzfb->lock);
>>> + return 0;
>>> + }
>>> +
>>> + jzfb_enable(jzfb);
>>> + jzfb->is_enabled = 1;
>>> +
>>> + mutex_unlock(&jzfb->lock);
>>> + break;
>>> + default:
>>> + mutex_lock(&jzfb->lock);
>>> + if (!jzfb->is_enabled) {
>>> + mutex_unlock(&jzfb->lock);
>>> + return 0;
>>> + }
>>> +
>>> + jzfb_disable(jzfb);
>>> + jzfb->is_enabled = 0;
>>> +
>>> + mutex_unlock(&jzfb->lock);
>>> + break;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int jzfb_alloc_devmem(struct jzfb *jzfb)
>>> +{
>>> + int max_videosize = 0;
>>> + struct fb_videomode *mode = jzfb->pdata->modes;
>>> + void *page;
>>> + int i;
>>> +
>>> + for (i = 0; i < jzfb->pdata->num_modes; ++mode, ++i) {
>>> + if (max_videosize < mode->xres * mode->yres)
>>> + max_videosize = mode->xres * mode->yres;
>>> + }
>>> +
>>> + max_videosize *= jzfb_get_controller_bpp(jzfb) >> 3;
>>> +
>>> + jzfb->framedesc = dma_alloc_coherent(&jzfb->pdev->dev,
>>> + sizeof(*jzfb->framedesc),
>>> + &jzfb->framedesc_phys, GFP_KERNEL);
>>> +
>>> + if (!jzfb->framedesc)
>>> + return -ENOMEM;
>>> +
>>> + jzfb->vidmem_size = PAGE_ALIGN(max_videosize);
>>> + jzfb->vidmem = dma_alloc_coherent(&jzfb->pdev->dev,
>>> + jzfb->vidmem_size,
>>> + &jzfb->vidmem_phys, GFP_KERNEL);
>>> +
>>> + if (!jzfb->vidmem)
>>> + goto err_free_framedesc;
>>> +
>>> + for (page = jzfb->vidmem;
>>> + page < jzfb->vidmem + PAGE_ALIGN(jzfb->vidmem_size);
>>> + page += PAGE_SIZE) {
>>> + SetPageReserved(virt_to_page(page));
>>> + }
>
> It'd be nice to know the reasoning for the SetPageReserved.
>
It's to make sure that the framebuffer memory is not swapped out. But actually I'm
not sure if it's really needed, since I'm not that familar with how the swap
management code choose pages to swap out. But there are similar drivers seem to do
the same thing.
> Thanks,
> jaya
>
Thanks for reviewing,
- - Lars
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAkw3QN8ACgkQBX4mSR26RiNg3wCfeg7uauWGHNx5z9StBcbloOrj
luYAnjjGFuYcD8O7zBKjlzOHOMD8PPuJ
=qrO3
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists