From: Gregory CLEMENT <gclement00@gmail.com>
To: Juergen Beisert <jbe@pengutronix.de>,
"gregory.clement" <gregory.clement@free-electrons.com>
Cc: barebox <barebox@lists.infradead.org>
Subject: Re: [PATCH 08/10] ARM STM/i.MX: Add video driver for i.MX23/i.MX28
Date: Wed, 29 Dec 2010 15:55:15 +0100 [thread overview]
Message-ID: <AANLkTinXmZiA1pVk43fMu1w1mXDBHHpUj-LxJ_YJkn6L@mail.gmail.com> (raw)
In-Reply-To: <1292857509-13881-9-git-send-email-jbe@pengutronix.de>
Hi,
I apply this patch for my project and use it with an imx23 base board.
I have a few comments and I think that I found a bug.
I you are interested I can send you patches related to my comments.
2010/12/20 Juergen Beisert <jbe@pengutronix.de>:
> Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
> ---
> arch/arm/mach-stm/include/mach/fb.h | 43 +++
> arch/arm/mach-stm/include/mach/imx23-regs.h | 1 +
> drivers/video/Kconfig | 7 +
> drivers/video/Makefile | 1 +
> drivers/video/stm.c | 540 +++++++++++++++++++++++++++
> 5 files changed, 592 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-stm/include/mach/fb.h
> create mode 100644 drivers/video/stm.c
>
> diff --git a/arch/arm/mach-stm/include/mach/fb.h b/arch/arm/mach-stm/include/mach/fb.h
> new file mode 100644
> index 0000000..65e3be2
> --- /dev/null
> +++ b/arch/arm/mach-stm/include/mach/fb.h
> @@ -0,0 +1,43 @@
> +/*
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __MACH_FB_H
> +# define __MACH_FB_H
> +
> +#include <fb.h>
> +
> +#define STMLCDIF_8BIT 1 /** pixel data bus to the display is of 8 bit width */
> +#define STMLCDIF_16BIT 0 /** pixel data bus to the display is of 16 bit width */
> +#define STMLCDIF_18BIT 2 /** pixel data bus to the display is of 18 bit width */
> +#define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24 bit width */
> +
> +/** LC display uses active high data enable signal */
> +#define FB_SYNC_DE_HIGH_ACT (1 << 27)
> +/** LC display will latch its data at clock's rising edge */
> +#define FB_SYNC_CLK_INVERT (1 << 28)
> +/** output RGB digital data inverted */
> +#define FB_SYNC_DATA_INVERT (1 << 29)
> +/** Stop clock if no data is sent (required for passive displays) */
> +#define FB_SYNC_CLK_IDLE_DIS (1 << 30)
> +/** swap RGB to BGR */
> +#define FB_SYNC_SWAP_RGB (1 << 31)
> +
> +struct imx_fb_videomode {
> + struct fb_videomode *mode_list;
> + unsigned mode_cnt;
> +
> + unsigned dotclk_delay; /**< refer manual HW_LCDIF_VDCTRL4 register */
> + unsigned ld_intf_width; /**< refer STMLCDIF_* macros */
> +};
Should it be acceptable to add new field in this structure ?
For example for my need I added a data_mode flag which is used to configure
command mode polarity (bit 16 in HW_LCDIF_CTRL register)
> +
> +#endif /* __MACH_FB_H */
> +
> diff --git a/arch/arm/mach-stm/include/mach/imx23-regs.h b/arch/arm/mach-stm/include/mach/imx23-regs.h
> index 89ca453..caac19b 100644
> --- a/arch/arm/mach-stm/include/mach/imx23-regs.h
> +++ b/arch/arm/mach-stm/include/mach/imx23-regs.h
> @@ -36,6 +36,7 @@
> #define IMX_CCM_BASE 0x80040000
> #define IMX_I2C1_BASE 0x80058000
> #define IMX_SSP1_BASE 0x80010000
> +#define IMX_FB_BASE 0x80030000
> #define IMX_SSP2_BASE 0x80034000
>
> #endif /* __ASM_ARCH_MX23_REGS_H */
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 7a89a3f..c69308e 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -20,4 +20,11 @@ config DRIVER_VIDEO_IMX_IPU
> Add support for the IPU framebuffer device found on
> i.MX31 and i.MX35 CPUs.
>
> +config DRIVER_VIDEO_STM
> + bool "i.MX23/28 framebuffer driver"
> + depends on ARCH_STM
> + help
> + Say 'Y' here to enable framebuffer and splash screen support for
> + i.MX23 and i.MX28 based systems.
> +
> endif
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 179f0c4..a217a0b 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -1,4 +1,5 @@
> obj-$(CONFIG_VIDEO) += fb.o
>
> +obj-$(CONFIG_DRIVER_VIDEO_STM) += stm.o
> obj-$(CONFIG_DRIVER_VIDEO_IMX) += imx.o
> obj-$(CONFIG_DRIVER_VIDEO_IMX_IPU) += imx-ipu-fb.o
> diff --git a/drivers/video/stm.c b/drivers/video/stm.c
> new file mode 100644
> index 0000000..f0abe4c
> --- /dev/null
> +++ b/drivers/video/stm.c
> @@ -0,0 +1,540 @@
> +/*
> + * Copyright (C) 2010 Juergen Beisert, Pengutronix <jbe@pengutronix.de>
> + *
> + * This is based on code from:
> + * Author: Vitaly Wool <vital@embeddedalley.com>
> + *
> + * Copyright 2008-2009 Freescale Semiconductor, Inc. All Rights Reserved.
> + * Copyright 2008 Embedded Alley Solutions, Inc All Rights Reserved.
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +#include <common.h>
> +#include <init.h>
> +#include <driver.h>
> +#include <malloc.h>
> +#include <errno.h>
> +#include <xfuncs.h>
> +#include <asm/io.h>
> +#include <mach/imx-regs.h>
> +#include <mach/clock.h>
> +#include <mach/fb.h>
> +
> +#define HW_LCDIF_CTRL 0x00
> +# define CTRL_SFTRST (1 << 31)
> +# define CTRL_CLKGATE (1 << 30)
> +# define CTRL_BYPASS_COUNT (1 << 19)
> +# define CTRL_VSYNC_MODE (1 << 18)
> +# define CTRL_DOTCLK_MODE (1 << 17)
> +# define CTRL_DATA_SELECT (1 << 16)
> +# define SET_BUS_WIDTH(x) (((x) & 0x3) << 10)
> +# define SET_WORD_LENGTH(x) (((x) & 0x3) << 8)
> +# define GET_WORD_LENGTH(x) (((x) >> 8) & 0x3)
> +# define CTRL_MASTER (1 << 5)
> +# define CTRL_DF16 (1 << 3)
> +# define CTRL_DF18 (1 << 2)
> +# define CTRL_DF24 (1 << 1)
> +# define CTRL_RUN (1 << 0)
> +
> +#define HW_LCDIF_CTRL1 0x10
> +# define CTRL1_FIFO_CLEAR (1 << 21)
> +# define SET_BYTE_PACKAGING(x) (((x) & 0xf) << 16)
> +# define GET_BYTE_PACKAGING(x) (((x) >> 16) & 0xf)
> +
> +#ifdef CONFIG_ARCH_IMX28
> +# define HW_LCDIF_CTRL2 0x20
> +# define HW_LCDIF_TRANSFER_COUNT 0x30
> +#endif
> +#ifdef CONFIG_ARCH_IMX23
> +# define HW_LCDIF_TRANSFER_COUNT 0x20
> +#endif
> +# define SET_VCOUNT(x) (((x) & 0xffff) << 16)
> +# define SET_HCOUNT(x) ((x) & 0xffff)
> +
> +#ifdef CONFIG_ARCH_IMX28
> +# define HW_LCDIF_CUR_BUF 0x40
> +# define HW_LCDIF_NEXT_BUF 0x50
> +#endif
> +#ifdef CONFIG_ARCH_IMX23
> +# define HW_LCDIF_CUR_BUF 0x30
> +# define HW_LCDIF_NEXT_BUF 0x40
> +#endif
> +
> +#define HW_LCDIF_TIMING 0x60
> +# define SET_CMD_HOLD(x) (((x) & 0xff) << 24)
> +# define SET_CMD_SETUP(x) (((x) & 0xff) << 16)
> +# define SET_DATA_HOLD(x) (((x) & 0xff) << 8)
> +# define SET_DATA_SETUP(x) ((x) & 0xff)
> +
> +#define HW_LCDIF_VDCTRL0 0x70
> +# define VDCTRL0_ENABLE_PRESENT (1 << 28)
> +# define VDCTRL0_VSYNC_POL (1 << 27) /* 0 = low active, 1 = high active */
> +# define VDCTRL0_HSYNC_POL (1 << 26) /* 0 = low active, 1 = high active */
> +# define VDCTRL0_DOTCLK_POL (1 << 25) /* 0 = output@falling, capturing@rising edge */
> +# define VDCTRL0_ENABLE_POL (1 << 24) /* 0 = low active, 1 = high active */
> +# define VDCTRL0_VSYNC_PERIOD_UNIT (1 << 21)
> +# define VDCTRL0_VSYNC_PULSE_WIDTH_UNIT (1 << 20)
> +# define VDCTRL0_HALF_LINE (1 << 19)
> +# define VDCTRL0_HALF_LINE_MODE (1 << 18)
> +# define SET_VSYNC_PULSE_WIDTH(x) ((x) & 0x3ffff)
> +
> +#define HW_LCDIF_VDCTRL1 0x80
> +
> +#define HW_LCDIF_VDCTRL2 0x90
> +#ifdef CONFIG_ARCH_IMX28
> +# define SET_HSYNC_PULSE_WIDTH(x) (((x) & 0x3fff) << 18)
> +#endif
> +#ifdef CONFIG_ARCH_IMX23
> +# define SET_HSYNC_PULSE_WIDTH(x) (((x) & 0xff) << 24)
> +#endif
> +# define SET_HSYNC_PERIOD(x) ((x) & 0x3ffff)
> +
> +#define HW_LCDIF_VDCTRL3 0xa0
> +# define VDCTRL3_MUX_SYNC_SIGNALS (1 << 29)
> +# define VDCTRL3_VSYNC_ONLY (1 << 28)
> +# define SET_HOR_WAIT_CNT(x) (((x) & 0xfff) << 16)
> +# define SET_VERT_WAIT_CNT(x) ((x) & 0xffff)
> +
> +#define HW_LCDIF_VDCTRL4 0xb0
> +#ifdef CONFIG_ARCH_IMX28
> +# define SET_DOTCLK_DLY(x) (((x) & 0x7) << 29)
> +#endif
> +# define VDCTRL4_SYNC_SIGNALS_ON (1 << 18)
> +# define SET_DOTCLK_H_VALID_DATA_CNT(x) ((x) & 0x3ffff)
> +
> +#define HW_LCDIF_DVICTRL0 0xc0
> +#define HW_LCDIF_DVICTRL1 0xd0
> +#define HW_LCDIF_DVICTRL2 0xe0
> +#define HW_LCDIF_DVICTRL3 0xf0
> +#define HW_LCDIF_DVICTRL4 0x100
> +
> +#ifdef CONFIG_ARCH_IMX28
> +# define HW_LCDIF_DATA 0x180
> +#endif
> +#ifdef CONFIG_ARCH_IMX23
> +# define HW_LCDIF_DATA 0x1b0
> +#endif
> +
> +#ifdef CONFIG_ARCH_IMX28
> +# define HW_LCDIF_DEBUG0 0x1d0
> +#endif
> +#ifdef CONFIG_ARCH_IMX23
> +# define HW_LCDIF_DEBUG0 0x1f0
> +#endif
> +# define DEBUG_HSYNC (1 < 26)
> +# define DEBUG_VSYNC (1 < 25)
> +
> +#define RED 0
> +#define GREEN 1
> +#define BLUE 2
> +#define TRANSP 3
> +
> +struct imxfb_info {
> + void __iomem *base;
> + unsigned memory_size;
> + struct fb_info info;
> + struct device_d *hw_dev;
> + struct imx_fb_videomode *pdata;
> +};
> +
> +/* the RGB565 true colour mode */
> +static const struct fb_bitfield def_rgb565[] = {
> + [RED] = {
> + .offset = 11,
> + .length = 5,
> + },
> + [GREEN] = {
> + .offset = 5,
> + .length = 6,
> + },
> + [BLUE] = {
> + .offset = 0,
> + .length = 5,
> + },
> + [TRANSP] = { /* no support for transparency */
> + .length = 0,
> + }
> +};
> +
> +/* the RGB666 true colour mode */
> +static const struct fb_bitfield def_rgb666[] = {
> + [RED] = {
> + .offset = 16,
> + .length = 6,
> + },
> + [GREEN] = {
> + .offset = 8,
> + .length = 6,
> + },
> + [BLUE] = {
> + .offset = 0,
> + .length = 6,
> + },
> + [TRANSP] = { /* no support for transparency */
> + .length = 0,
> + }
> +};
> +
> +/* the RGB888 true colour mode */
> +static const struct fb_bitfield def_rgb888[] = {
> + [RED] = {
> + .offset = 16,
> + .length = 8,
> + },
> + [GREEN] = {
> + .offset = 8,
> + .length = 8,
> + },
> + [BLUE] = {
> + .offset = 0,
> + .length = 8,
> + },
> + [TRANSP] = { /* no support for transparency */
> + .length = 0,
> + }
> +};
> +
> +static inline unsigned calc_line_length(unsigned ppl, unsigned bpp)
> +{
> + if (bpp == 24)
> + bpp = 32;
> + return (ppl * bpp) >> 3;
> +}
> +
> +static int stmfb_memory_mmgt(struct fb_info *fb_info, unsigned size)
> +{
> + struct imxfb_info *fbi = fb_info->priv;
> +
> + if (fbi->memory_size != 0) {
> + free(fb_info->screen_base);
> + fb_info->screen_base = NULL;
> + fbi->memory_size = 0;
> + }
> +
> + if (fbi->memory_size == 0) {
> + fb_info->screen_base = xzalloc(size);
> + fbi->memory_size = size;
> + }
> +
> + return 0;
> +}
> +
> +static void stmfb_enable_controller(struct fb_info *fb_info)
> +{
> + struct imxfb_info *fbi = fb_info->priv;
> + uint32_t reg, last_reg;
> + unsigned loop, edges;
> +
> + /*
> + * Sometimes some data is still present in the FIFO. This leads into
> + * a correct but shifted picture. Clearing the FIFO helps
> + */
> + writel(CTRL1_FIFO_CLEAR, fbi->base + HW_LCDIF_CTRL1 + BIT_SET);
> +
> + /* if it was disabled, re-enable the mode again */
> + reg = readl(fbi->base + HW_LCDIF_CTRL);
> + reg |= CTRL_DOTCLK_MODE;
> + writel(reg, fbi->base + HW_LCDIF_CTRL);
> +
> + /* enable the SYNC signals first, then the DMA engine */
> + reg = readl(fbi->base + HW_LCDIF_VDCTRL4);
> + reg |= VDCTRL4_SYNC_SIGNALS_ON;
> + writel(reg, fbi->base + HW_LCDIF_VDCTRL4);
> +
> + /*
> + * Give the attached LC display or monitor a chance to sync into
> + * our signals.
> + * Wait for at least 2 VSYNCs = four VSYNC edges
> + */
> + edges = 4;
> +
> + while (edges != 0) {
> + loop = 800;
> + last_reg = readl(fbi->base + HW_LCDIF_DEBUG0) & DEBUG_VSYNC;
> + do {
> + reg = readl(fbi->base + HW_LCDIF_DEBUG0) & DEBUG_VSYNC;
> + if (reg != last_reg)
> + break;
> + last_reg = reg;
> + loop--;
> + } while (loop != 0);
> + edges--;
> + }
> +
> + /* stop FIFO reset */
> + writel(CTRL1_FIFO_CLEAR, fbi->base + HW_LCDIF_CTRL1 + BIT_CLR);
> + /* start the engine right now */
> + writel(CTRL_RUN, fbi->base + HW_LCDIF_CTRL + BIT_SET);
> +}
> +
> +static void stmfb_disable_controller(struct fb_info *fb_info)
> +{
> + struct imxfb_info *fbi = fb_info->priv;
> + unsigned loop;
> + uint32_t reg;
> +
> + /*
> + * Even if we disable the controller here, it will still continue
> + * until its FIFOs are running out of data
> + */
> + reg = readl(fbi->base + HW_LCDIF_CTRL);
> + reg &= ~CTRL_DOTCLK_MODE;
> + writel(reg, fbi->base + HW_LCDIF_CTRL);
> +
> + loop = 1000;
> + while (loop) {
> + reg = readl(fbi->base + HW_LCDIF_CTRL);
> + if (!(reg & CTRL_RUN))
> + break;
> + loop--;
> + }
> +
> + reg = readl(fbi->base + HW_LCDIF_VDCTRL4);
> + reg &= ~VDCTRL4_SYNC_SIGNALS_ON;
> + writel(reg, fbi->base + HW_LCDIF_VDCTRL4);
> +}
> +
> +static int stmfb_activate_var(struct fb_info *fb_info)
> +{
> + struct imxfb_info *fbi = fb_info->priv;
> + struct imx_fb_videomode *pdata = fbi->pdata;
> + struct fb_videomode *mode = fb_info->mode;
> + uint32_t reg;
> + int ret;
> + unsigned size;
> +
> + /*
> + * we need at least this amount of memory for the framebuffer
> + */
> + size = calc_line_length(mode->xres, fb_info->bits_per_pixel) *
> + mode->yres;
> +
> + ret = stmfb_memory_mmgt(fb_info, size);
> + if (ret != 0) {
> + dev_err(fbi->hw_dev, "Cannot allocate framebuffer memory\n");
> + return ret;
> + }
> +
> + /** @todo ensure HCLK is active at this point of time! */
> +
> + size = imx_set_lcdifclk(PICOS2KHZ(mode->pixclock) * 1000);
> + if (size == 0) {
> + dev_dbg(fbi->hw_dev, "Unable to set a valid pixel clock\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * bring the controller out of reset and
> + * configure it into DOTCLOCK mode
> + */
> + reg = CTRL_BYPASS_COUNT | /* always in DOTCLOCK mode */
> + CTRL_DOTCLK_MODE;
> + writel(reg, fbi->base + HW_LCDIF_CTRL);
> +
> + /* master mode only */
> + reg |= CTRL_MASTER;
> +
> + /*
> + * Configure videomode and interface mode
> + */
> + reg |= SET_BUS_WIDTH(pdata->ld_intf_width);
> + switch (fb_info->bits_per_pixel) {
> + case 8:
> + reg |= SET_WORD_LENGTH(1);
> + /** @todo refer manual page 2046 for 8 bpp modes */
> + dev_dbg(fbi->hw_dev, "8 bpp mode not supported yet\n");
> + break;
> + case 16:
> + pr_debug("Setting up an RGB565 mode\n");
> + reg |= SET_WORD_LENGTH(0);
> + reg &= ~CTRL_DF16; /* we assume RGB565 */
> + writel(SET_BYTE_PACKAGING(0xf), fbi->base + HW_LCDIF_CTRL1);
> + fb_info->red = def_rgb565[RED];
> + fb_info->green = def_rgb565[GREEN];
> + fb_info->blue = def_rgb565[BLUE];
> + fb_info->transp = def_rgb565[TRANSP];
> + break;
> + case 24:
> + case 32:
> + pr_debug("Setting up an RGB888/666 mode\n");
> + reg |= SET_WORD_LENGTH(3);
> + switch (pdata->ld_intf_width) {
> + case STMLCDIF_8BIT:
> + dev_dbg(fbi->hw_dev,
> + "Unsupported LCD bus width mapping\n");
> + break;
> + case STMLCDIF_16BIT:
> + case STMLCDIF_18BIT:
> + /* 24 bit to 18 bit mapping
> + * which means: ignore the upper 2 bits in
> + * each colour component
> + */
> + reg |= CTRL_DF24;
> + fb_info->red = def_rgb666[RED];
> + fb_info->green = def_rgb666[GREEN];
> + fb_info->blue = def_rgb666[BLUE];
> + fb_info->transp = def_rgb666[TRANSP];
> + break;
> + case STMLCDIF_24BIT:
> + /* real 24 bit */
> + fb_info->red = def_rgb888[RED];
> + fb_info->green = def_rgb888[GREEN];
> + fb_info->blue = def_rgb888[BLUE];
> + fb_info->transp = def_rgb888[TRANSP];
> + break;
> + }
> + /* do not use packed pixels = one pixel per word instead */
> + writel(SET_BYTE_PACKAGING(0x7), fbi->base + HW_LCDIF_CTRL1);
> + break;
> + default:
> + dev_dbg(fbi->hw_dev, "Unhandled colour depth of %u\n",
> + fb_info->bits_per_pixel);
> + return -EINVAL;
> + }
> + writel(reg, fbi->base + HW_LCDIF_CTRL);
> + pr_debug("Setting up CTRL to %08X\n", reg);
> +
> + writel(SET_VCOUNT(mode->yres) |
> + SET_HCOUNT(mode->xres), fbi->base + HW_LCDIF_TRANSFER_COUNT);
> +
> + reg = VDCTRL0_ENABLE_PRESENT | /* always in DOTCLOCK mode */
> + VDCTRL0_VSYNC_PERIOD_UNIT |
> + VDCTRL0_VSYNC_PULSE_WIDTH_UNIT;
> + if (mode->sync & FB_SYNC_HOR_HIGH_ACT)
> + reg |= VDCTRL0_HSYNC_POL;
> + if (mode->sync & FB_SYNC_VERT_HIGH_ACT)
> + reg |= VDCTRL0_VSYNC_POL;
> + if (mode->sync & FB_SYNC_DE_HIGH_ACT)
> + reg |= VDCTRL0_ENABLE_POL;
> + if (mode->sync & FB_SYNC_CLK_INVERT)
> + reg |= VDCTRL0_DOTCLK_POL;
> +
> + reg |= SET_VSYNC_PULSE_WIDTH(mode->vsync_len);
> + writel(reg, fbi->base + HW_LCDIF_VDCTRL0);
> + pr_debug("Setting up VDCTRL0 to %08X\n", reg);
> +
> + /* frame length in lines */
> + writel(mode->upper_margin + mode->vsync_len + mode->lower_margin +
> + mode->yres,
> + fbi->base + HW_LCDIF_VDCTRL1);
> +
> + /* line length in units of clocks or pixels */
> + writel(SET_HSYNC_PULSE_WIDTH(mode->hsync_len) |
> + SET_HSYNC_PERIOD(mode->left_margin + mode->hsync_len +
> + mode->right_margin + mode->xres),
> + fbi->base + HW_LCDIF_VDCTRL2);
> +
> + writel(SET_HOR_WAIT_CNT(mode->left_margin + mode->hsync_len) |
> + SET_VERT_WAIT_CNT(mode->upper_margin + mode->vsync_len),
> + fbi->base + HW_LCDIF_VDCTRL3);
> +
> + writel(
> +#ifdef CONFIG_ARCH_IMX28
> + SET_DOTCLK_DLY(pdata->dotclk_delay) |
> +#endif
> + SET_DOTCLK_H_VALID_DATA_CNT(mode->xres),
> + fbi->base + HW_LCDIF_VDCTRL4);
> +
> + writel((uint32_t)fb_info->screen_base, fbi->base + HW_LCDIF_CUR_BUF);
> + writel((uint32_t)fb_info->screen_base, fbi->base + HW_LCDIF_NEXT_BUF);
> +
> + return 0;
> +}
> +
> +static void stmfb_info(struct device_d *hw_dev)
> +{
> + struct imx_fb_videomode *pdata = hw_dev->platform_data;
> + unsigned u;
> +
> + printf(" Supported video modes:\n");
> + for (u = 0; u < pdata->mode_cnt; u++)
> + printf(" - '%s': %u x %u\n", pdata->mode_list[u].name,
> + pdata->mode_list[u].xres, pdata->mode_list[u].yres);
> +}
> +
> +/*
> + * There is only one video hardware instance available.
> + * It makes no sense to dynamically allocate this data
> + */
> +static struct fb_ops imxfb_ops = {
> + .fb_activate_var = stmfb_activate_var,
> + .fb_enable = stmfb_enable_controller,
> + .fb_disable = stmfb_disable_controller,
> +};
> +
> +static struct imxfb_info fbi = {
> + .info = {
> + .fbops = &imxfb_ops,
> + },
> +};
> +
> +static int stmfb_probe(struct device_d *hw_dev)
> +{
> + struct imx_fb_videomode *pdata = hw_dev->platform_data;
> + int ret;
> +
> + /* just init */
> + fbi.info.priv = &fbi;
> +
> + /* add runtime hardware info */
> + fbi.hw_dev = hw_dev;
> + fbi.base = (void *)hw_dev->map_base;
> + fbi.pdata = pdata;
> +
> + /* add runtime video info */
> + fbi.info.mode_list = pdata->mode_list;
> + fbi.info.num_modes = pdata->mode_cnt;
> + fbi.info.mode = &fbi.info.mode_list[0];
> + fbi.info.xres = fbi.info.mode->xres;
> + fbi.info.yres = fbi.info.mode->yres;
> + fbi.info.bits_per_pixel = 16;
Could we consider to pass .bits_per_pixel through platform _data?
Is there any reason to hardcode this value?
> +
> + ret = register_framebuffer(&fbi.info);
At this point you register the framebuffer but screen_base is not set yet.
So when we want to access framebuffer from mmap we get a pointer to 0x0
instead of getting the pointer to the frambeffer address..
I moved stmfb_memory_mmgt from stmfb_activate_var to be able to
display a picture with bmp command. It worked for me because I have only
one screen available, but if you want to keep the capability to change screen on
the fly, then we will need to update map_base in fb driver.
> + if (ret != 0) {
> + dev_err(hw_dev, "Failed to register framebuffer\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static struct driver_d stmfb_driver = {
> + .name = "stmfb",
> + .probe = stmfb_probe,
> + .info = stmfb_info,
> +};
> +
> +static int stmfb_init(void)
> +{
> + return register_driver(&stmfb_driver);
> +}
> +
> +device_initcall(stmfb_init);
> +
> +/**
> + * @file
> + * @brief LCDIF driver for i.MX23 and i.MX28
> + *
> + * The LCDIF support four modes of operation
> + * - MPU interface (to drive smart displays) -> not supported yet
> + * - VSYNC interface (like MPU interface plus Vsync) -> not supported yet
> + * - Dotclock interface (to drive LC displays with RGB data and sync signals)
> + * - DVI (to drive ITU-R BT656) -> not supported yet
> + *
> + * This driver depends on a correct setup of the pins used for this purpose
> + * (platform specific).
> + *
> + * For the developer: Don't forget to set the data bus width to the display
> + * in the imx_fb_videomode structure. You will else end up with ugly colours.
> + * If you fight against jitter you can vary the clock delay. This is a feature
> + * of the i.MX28 and you can vary it between 2 ns ... 8 ns in 2 ns steps. Give
> + * the required value in the imx_fb_videomode structure.
> + */
> --
> 1.7.2.3
>
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
next prev parent reply other threads:[~2010-12-29 14:55 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-20 15:04 [PATCH] Add support for runtime selection of video mode in barebox Juergen Beisert
2010-12-20 15:05 ` [PATCH 01/10] Add more useful generic macros Juergen Beisert
2010-12-20 15:05 ` [PATCH 02/10] ARM Chumby: Fix memory index Juergen Beisert
2010-12-20 15:05 ` [PATCH 03/10] ARM Chumby: Fix unit of the clocks after the change to Hz Juergen Beisert
2010-12-20 15:05 ` [PATCH 04/10] ARM Chumby: Just fix some typos Juergen Beisert
2010-12-20 15:05 ` [PATCH 05/10] Fix default framebuffer 'enable' set Juergen Beisert
2010-12-20 15:05 ` [PATCH 06/10] Add the feature to change the video mode at runtime Juergen Beisert
2010-12-20 15:05 ` [PATCH 07/10] ARM STM/i.MX: Add a pixel clock calculation routine for i.MX23/i.MX28 Juergen Beisert
2010-12-20 15:05 ` [PATCH 08/10] ARM STM/i.MX: Add video driver " Juergen Beisert
2010-12-29 14:55 ` Gregory CLEMENT [this message]
2010-12-20 15:05 ` [PATCH 09/10] ARM KARO-TX28-STK5: Add a user for video graphics support on i.MX28 Juergen Beisert
2010-12-20 15:05 ` [PATCH 10/10] ARM Chumby-Falconwing: Add a user for video graphics support on i.MX23 Juergen Beisert
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AANLkTinXmZiA1pVk43fMu1w1mXDBHHpUj-LxJ_YJkn6L@mail.gmail.com \
--to=gclement00@gmail.com \
--cc=barebox@lists.infradead.org \
--cc=gregory.clement@free-electrons.com \
--cc=jbe@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox