Hi,
Please see minor comments below.
On Thu, 05 Apr 2012 15:29:29 +0900
Donghwa Lee <dh09....@samsung.com> wrote:
> EXYNOS SoC platform has MIPI-DSI controller and MIPI-DSI
> based LCD Panel could be used with it. This patch supports MIPI-DSI driver
> based Samsung SoC chip.
>
> LCD panel driver based MIPI-DSI should be registered to MIPI-DSI driver at
> board file and LCD panel driver specific function registered to mipi_dsim_ddi
> structure at lcd panel init function called system init.
> In the MIPI-DSI driver, find lcd panel driver by using registered
> lcd panel name, and then initialize lcd panel driver.
>
> Signed-off-by: Donghwa Lee <dh09....@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
> Signed-off-by: Inki Dae <inki....@samsung.com>
> ---
> arch/arm/include/asm/arch-exynos/dsim.h | 181 +++++++
> arch/arm/include/asm/arch-exynos/mipi_dsim.h | 400 +++++++++++++++
> drivers/video/exynos_mipi_dsi.c | 257 ++++++++++
> drivers/video/exynos_mipi_dsi_common.c | 645 +++++++++++++++++++++++++
> drivers/video/exynos_mipi_dsi_common.h | 48 ++
> drivers/video/exynos_mipi_dsi_lowlevel.c | 669
> ++++++++++++++++++++++++++
> drivers/video/exynos_mipi_dsi_lowlevel.h | 111 +++++
> 7 files changed, 2311 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/include/asm/arch-exynos/dsim.h
> create mode 100644 arch/arm/include/asm/arch-exynos/mipi_dsim.h
> create mode 100644 drivers/video/exynos_mipi_dsi.c
> create mode 100644 drivers/video/exynos_mipi_dsi_common.c
> create mode 100644 drivers/video/exynos_mipi_dsi_common.h
> create mode 100644 drivers/video/exynos_mipi_dsi_lowlevel.c
> create mode 100644 drivers/video/exynos_mipi_dsi_lowlevel.h
...
> diff --git a/arch/arm/include/asm/arch-exynos/mipi_dsim.h
> b/arch/arm/include/asm/arch-exynos/mipi_dsim.h
> new file mode 100644
> index 0000000..4ae6830
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-exynos/mipi_dsim.h
...
> +/*
> + * struct mipi_dsim_device - global interface for mipi-dsi driver.
> + *
> + * @dev: driver model representation of the device.
> + * @id: unique device id.
> + * @clock: pointer to MIPI-DSI clock of clock framework.
> + * @irq: interrupt number to MIPI-DSI controller.
> + * @reg_base: base address to memory mapped SRF of MIPI-DSI controller.
> + * (virtual address)
> + * @lock: the mutex protecting this data structure.
These above documented structure members are not in the below
struct for U-Boot, so please remove their description.
> + * @dsim_info: infomation for configuring mipi-dsi controller.
The member in the struct is named dsim_config, please update this
description accordingly.
> + * @master_ops: callbacks to mipi-dsi operations.
> + * @dsim_lcd_dev: pointer to activated ddi device.
> + * (it would be registered by mipi-dsi driver.)
> + * @dsim_lcd_drv: pointer to activated_ddi driver.
> + * (it would be registered by mipi-dsi driver.)
> + * @lcd_info: pointer to mipi_lcd_info structure.
Drop @lcd_info description.
> + * @state: specifies status of MIPI-DSI controller.
> + * the status could be RESET, INIT, STOP, HSCLKEN and ULPS.
> + * @resume_complete: indicates whether resume operation is completed or not.
Drop @resume_complete.
> + * @data_lane: specifiec enabled data lane number.
> + * this variable would be set by driver according to e_no_data_lane
> + * automatically.
> + * @e_clk_src: select byte clock source.
> + * @pd: pointer to MIPI-DSI driver platform data.
> + */
> +struct mipi_dsim_device {
> + struct mipi_dsim_config *dsim_config;
> + struct mipi_dsim_master_ops *master_ops;
> + struct mipi_dsim_lcd_device *dsim_lcd_dev;
> + struct mipi_dsim_lcd_driver *dsim_lcd_drv;
> +
> + unsigned int state;
> + unsigned int data_lane;
> + enum mipi_dsim_byte_clk_src e_clk_src;
> +
> + struct exynos_platform_mipi_dsim *pd;
> +};
> +
> +/*
> + * struct exynos_platform_mipi_dsim - interface to platform data
> + * for mipi-dsi driver.
> + *
> + * @lcd_panel_name: specifies lcd panel name registered to mipi-dsi driver.
> + * lcd panel driver searched would be actived.
> + * @dsim_config: pointer of structure for configuring mipi-dsi controller.
> + * @lcd_panel_info: pointer for lcd panel specific structure.
> + * this structure specifies width, height, timing and polarity and so on.
> + * @mipi_power: callback pointer for enabling or disabling mipi power.
> + * @phy_enable: pointer to a callback controlling D-PHY enable/reset
lcd_power from below struct is not in the description above.
> + */
> +struct exynos_platform_mipi_dsim {
> + char lcd_panel_name[PANEL_NAME_SIZE];
> +
> + struct mipi_dsim_config *dsim_config;
> + void *lcd_panel_info;
> +
> + int (*lcd_power)(void);
> + int (*mipi_power)(void);
> + void (*phy_enable)(unsigned int enable, unsigned int dev_index);
> +};
Please add an empty line here.
> +/*
> + * struct mipi_dsim_master_ops - callbacks to mipi-dsi operations.
> + *
> + * @cmd_write: transfer command to lcd panel at LP mode.
> + * @cmd_read: read command from rx register.
> + * @get_dsim_frame_done: get the status that all screen data have been
> + * transferred to mipi-dsi.
> + * @clear_dsim_frame_done: clear frame done status.
> + * @get_fb_frame_done: get frame done status of display controller.
> + * @trigger: trigger display controller.
> + * - this one would be used only in case of CPU mode.
> + */
> +
and drop this empty line here.
> +struct mipi_dsim_master_ops {
> + int (*cmd_write)(struct mipi_dsim_device *dsim, unsigned int data_id,
> + unsigned int data0, unsigned int data1);
> + int (*cmd_read)(struct mipi_dsim_device *dsim, unsigned int data_id,
> + unsigned int data0, unsigned int data1);
> + int (*get_dsim_frame_done)(struct mipi_dsim_device *dsim);
> + int (*clear_dsim_frame_done)(struct mipi_dsim_device *dsim);
> +
> + int (*get_fb_frame_done)(void);
> + void (*trigger)(struct fb_info *info);
> +};
> +
> +/*
> + * device structure for mipi-dsi based lcd panel.
> + *
> + * @name: name of the device to use with this device, or an
> + * alias for that name.
> + * @dev: driver model representation of the device.
drop @dev description.
> + * @id: id of device to be registered.
> + * @bus_id: bus id for identifing connected bus
> + * and this bus id should be same as id of mipi_dsim_device.
> + * @irq: irq number for signaling when framebuffer transfer of
> + * lcd panel module is completed.
> + * this irq would be used only for MIPI-DSI based CPU mode lcd panel.
drop @irq description.
> + * @master: pointer to mipi-dsi master device object.
> + * @platform_data: lcd panel specific platform data.
> + */
> +struct mipi_dsim_lcd_device {
> + char *name;
> + char *panel_id;
This panel_id seems to be not used anywhere? Drop it?
> + int panel_type;
> + int id;
> + int bus_id;
> + int reverse;
> +
> + struct mipi_dsim_device *master;
> + void *platform_data;
> +};
> +
> +/*
> + * driver structure for mipi-dsi based lcd panel.
> + *
> + * this structure should be registered by lcd panel driver.
> + * mipi-dsi driver seeks lcd panel registered through name field
> + * and calls these callback functions in appropriate time.
> + *
> + * @name: name of the driver to use with this device, or an
> + * alias for that name.
> + * @id: id of driver to be registered.
> + * this id would be used for finding device object registered.
> + */
> +struct mipi_dsim_lcd_driver {
> + char *name;
> + int id;
> +
> + int (*mipi_panel_init)(struct mipi_dsim_device *dsim_dev);
> + void (*mipi_display_on)(struct mipi_dsim_device *dsim_dev);
Only 'name' and 'id' is documented above. Document these callbacks, too.
...
> +/*
> + * exynos_dsim_phy_enable - global MIPI-DSI receiver D-PHY control
> + * @pdev: MIPI-DSIM platform device
Drop @pdev description.
> + * @on: true to enable D-PHY and deassert its reset
> + * false to disable D-PHY
> + */
> +int exynos_dsim_phy_enable(int on);
...
> diff --git a/drivers/video/exynos_mipi_dsi.c b/drivers/video/exynos_mipi_dsi.c
> new file mode 100644
> index 0000000..704f6a7
> --- /dev/null
> +++ b/drivers/video/exynos_mipi_dsi.c
> @@ -0,0 +1,257 @@
> +/*
> + * Copyright (C) 2012 Samsung Electronics
> + *
> + * Author: InKi Dae <inki....@samsung.com>
> + * Author: Donghwa Lee <dh09....@samsung.com>
> + *
> + * 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.
> + *
> + * 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., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <ubi_uboot.h>
> +#include <common.h>
> +#include <lcd.h>
> +#include <asm/errno.h>
Please drop ubi_uboot.h, lcd.h and errno.h here and better use
#include <common.h>
#include <malloc.h>
#include <linux/err.h>
> +#include <asm/arch/dsim.h>
> +#include <asm/arch/mipi_dsim.h>
> +#include <asm/arch/power.h>
> +#include <asm/arch/cpu.h>
> +#include <asm/arch/clk.h>
> +#include <linux/types.h>
types.h can be dropped too, I think.
> +#include <linux/list.h>
> +
> +#include "exynos_mipi_dsi_lowlevel.h"
> +#include "exynos_mipi_dsi_common.h"
> +
> +#define master_to_driver(a) (a->dsim_lcd_drv)
> +#define master_to_device(a) (a->dsim_lcd_dev)
> +
> +static struct exynos_platform_mipi_dsim *dsim_pd;
> +
> +struct mipi_dsim_ddi {
> + int bus_id;
> + struct list_head list;
> + struct mipi_dsim_lcd_device *dsim_lcd_dev;
> + struct mipi_dsim_lcd_driver *dsim_lcd_drv;
> +};
> +
> +static LIST_HEAD(dsim_ddi_list);
> +static LIST_HEAD(dsim_lcd_dev_list);
> +
> +int exynos_mipi_dsi_register_lcd_device(struct mipi_dsim_lcd_device *lcd_dev)
> +{
> + struct mipi_dsim_ddi *dsim_ddi;
> +
> + if (!lcd_dev) {
> + debug("mipi_dsim_lcd_device is NULL.\n");
> + return -EFAULT;
> + }
> +
> + if (!lcd_dev->name) {
> + debug("dsim_lcd_device name is NULL.\n");
> + return -EFAULT;
> + }
> +
> + dsim_ddi = kzalloc(sizeof(struct mipi_dsim_ddi), GFP_KERNEL);
> + if (!dsim_ddi) {
> + debug("failed to allocate dsim_ddi object.\n");
> + return -EFAULT;
> + }
> +
> + dsim_ddi->dsim_lcd_dev = lcd_dev;
> +
> + list_add_tail(&dsim_ddi->list, &dsim_ddi_list);
> +
> + return 0;
> +}
> +
> +struct mipi_dsim_ddi
> + *exynos_mipi_dsi_find_lcd_device(struct mipi_dsim_lcd_driver *lcd_drv)
> +{
> + struct mipi_dsim_ddi *dsim_ddi;
> + struct mipi_dsim_lcd_device *lcd_dev;
> +
> + list_for_each_entry(dsim_ddi, &dsim_ddi_list, list) {
> + lcd_dev = dsim_ddi->dsim_lcd_dev;
> + if (!lcd_dev)
> + continue;
> +
> + if (lcd_drv->id >= 0) {
> + if ((strcmp(lcd_drv->name, lcd_dev->name)) == 0 &&
> + lcd_drv->id == lcd_dev->id) {
> + /**
> + * bus_id would be used to identify
> + * connected bus.
> + */
> + dsim_ddi->bus_id = lcd_dev->bus_id;
> +
> + return dsim_ddi;
> + }
> + } else {
> + if ((strcmp(lcd_drv->name, lcd_dev->name)) == 0) {
> + /**
> + * bus_id would be used to identify
> + * connected bus.
> + */
> + dsim_ddi->bus_id = lcd_dev->bus_id;
> +
> + return dsim_ddi;
> + }
> + }
> +
> + kfree(dsim_ddi);
> + list_del(&dsim_ddi_list);
> + }
> +
> + return NULL;
> +}
> +
> +int exynos_mipi_dsi_register_lcd_driver(struct mipi_dsim_lcd_driver *lcd_drv)
> +{
> + struct mipi_dsim_ddi *dsim_ddi;
> +
> + if (!lcd_drv) {
> + debug("mipi_dsim_lcd_driver is NULL.\n");
> + return -EFAULT;
> + }
> +
> + if (!lcd_drv->name) {
> + debug("dsim_lcd_driver name is NULL.\n");
> + return -EFAULT;
> + }
> +
> + dsim_ddi = exynos_mipi_dsi_find_lcd_device(lcd_drv);
> + if (!dsim_ddi) {
> + debug("mipi_dsim_ddi object not found.\n");
> + return -EFAULT;
> + }
> +
> + dsim_ddi->dsim_lcd_drv = lcd_drv;
> +
> + debug("registered panel driver(%s) to mipi-dsi driver.\n",
> + lcd_drv->name);
> +
> + return 0;
> +
> +}
> +
> +struct mipi_dsim_ddi
> + *exynos_mipi_dsi_bind_lcd_ddi(struct mipi_dsim_device *dsim,
> + const char *name)
> +{
> + struct mipi_dsim_ddi *dsim_ddi;
> + struct mipi_dsim_lcd_driver *lcd_drv;
> + struct mipi_dsim_lcd_device *lcd_dev;
> +
> + list_for_each_entry(dsim_ddi, &dsim_ddi_list, list) {
> + lcd_drv = dsim_ddi->dsim_lcd_drv;
> + lcd_dev = dsim_ddi->dsim_lcd_dev;
> + if (!lcd_drv || !lcd_dev)
> + continue;
Please remove one tab before 'contunue'.
> +
> + debug("lcd_drv->id = %d, lcd_dev->id = %d\n",
> + lcd_drv->id, lcd_dev->id);
> +
> + if ((strcmp(lcd_drv->name, name) == 0)) {
> + lcd_dev->master = dsim;
> +
> + dsim->dsim_lcd_dev = lcd_dev;
> + dsim->dsim_lcd_drv = lcd_drv;
> +
> + return dsim_ddi;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +/* define MIPI-DSI Master operations. */
> +static struct mipi_dsim_master_ops master_ops = {
> + .cmd_write = exynos_mipi_dsi_wr_data,
> + .get_dsim_frame_done = exynos_mipi_dsi_get_frame_done_status,
> + .clear_dsim_frame_done = exynos_mipi_dsi_clear_frame_done,
> +};
> +
> +int exynos_mipi_dsi_init(void)
> +{
> + struct mipi_dsim_device *dsim;
> + struct mipi_dsim_config *dsim_config;
> + struct mipi_dsim_ddi *dsim_ddi;
> +
> + dsim = kzalloc(sizeof(struct mipi_dsim_device), GFP_KERNEL);
> + if (!dsim) {
> + debug("failed to allocate dsim object.\n");
> + return -EFAULT;
> + }
> +
> + /* get mipi_dsim_config. */
> + dsim_config = dsim_pd->dsim_config;
> + if (dsim_config == NULL) {
> + debug("failed to get dsim config data.\n");
> + return -EFAULT;
> + }
> +
> + dsim->pd = dsim_pd;
> + dsim->dsim_config = dsim_config;
> + dsim->master_ops = &master_ops;
> +
> +
Please remove one empty line.
> + /* bind lcd ddi matched with panel name. */
> + dsim_ddi = exynos_mipi_dsi_bind_lcd_ddi(dsim, dsim_pd->lcd_panel_name);
> + if (!dsim_ddi) {
> + debug("mipi_dsim_ddi object not found.\n");
> + return -ENOSYS;
> + }
...
> diff --git a/drivers/video/exynos_mipi_dsi_common.c
> b/drivers/video/exynos_mipi_dsi_common.c
> new file mode 100644
> index 0000000..08611b6
> --- /dev/null
> +++ b/drivers/video/exynos_mipi_dsi_common.c
...
> +static unsigned int dpll_table[15] = {
> + 100, 120, 170, 220, 270,
> + 320, 390, 450, 510, 560,
> + 640, 690, 770, 870, 950 };
Please put '};' to the next line.
> +
> +static void exynos_mipi_dsi_long_data_wr(struct mipi_dsim_device *dsim,
> + unsigned int data0, unsigned int data1)
> +{
> + unsigned int data_cnt = 0, payload = 0;
> +
> + /* in case that data count is more then 4 */
> + for (data_cnt = 0; data_cnt < data1; data_cnt += 4) {
> + /*
> + * after sending 4bytes per one time,
> + * send remainder data less then 4.
> + */
> + if ((data1 - data_cnt) < 4) {
> + if ((data1 - data_cnt) == 3) {
> + payload = *(u8 *)(data0 + data_cnt) |
> + (*(u8 *)(data0 + (data_cnt + 1))) << 8 |
Please indent by tab in the above line, it will fit into 80 chars limit.
> + (*(u8 *)(data0 + (data_cnt + 2))) << 16;
> + debug("count = 3 payload = %x, %x %x %x\n",
> + payload, *(u8 *)(data0 + data_cnt),
> + *(u8 *)(data0 + (data_cnt + 1)),
> + *(u8 *)(data0 + (data_cnt + 2)));
> + } else if ((data1 - data_cnt) == 2) {
> + payload = *(u8 *)(data0 + data_cnt) |
> + (*(u8 *)(data0 + (data_cnt + 1))) << 8;
> + debug("count = 2 payload = %x, %x %x\n", payload,
> + *(u8 *)(data0 + data_cnt),
> + *(u8 *)(data0 + (data_cnt + 1)));
> + } else if ((data1 - data_cnt) == 1) {
> + payload = *(u8 *)(data0 + data_cnt);
> + }
> +
> + exynos_mipi_dsi_wr_tx_data(dsim, payload);
> + /* send 4bytes per one time. */
> + } else {
> + payload = *(u8 *)(data0 + data_cnt) |
> + (*(u8 *)(data0 + (data_cnt + 1))) << 8 |
> + (*(u8 *)(data0 + (data_cnt + 2))) << 16 |
> + (*(u8 *)(data0 + (data_cnt + 3))) << 24;
> +
> + debug("count = 4 payload = %x, %x %x %x %x\n",
> + payload, *(u8 *)(data0 + data_cnt),
> + *(u8 *)(data0 + (data_cnt + 1)),
> + *(u8 *)(data0 + (data_cnt + 2)),
> + *(u8 *)(data0 + (data_cnt + 3)));
> +
> + exynos_mipi_dsi_wr_tx_data(dsim, payload);
> + }
exynos_mipi_dsi_wr_tx_data() is called in both above cases, so you can
simplify to call exynos_mipi_dsi_wr_tx_data() only here.
> + }
> +}
> +
> +int exynos_mipi_dsi_wr_data(struct mipi_dsim_device *dsim, unsigned int
> data_id,
> + unsigned int data0, unsigned int data1)
> +{
> + unsigned int timeout = TRY_GET_FIFO_TIMEOUT;
> + unsigned long delay_val, delay;
> + unsigned int check_rx_ack = 0;
> +
> + if (dsim->state == DSIM_STATE_ULPS) {
> + debug("state is ULPS.\n");
> +
> + return -EINVAL;
> + }
> +
> + delay_val = MHZ / dsim->dsim_config->esc_clk;
> + delay = 10 * delay_val;
> +
> + udelay(delay * 1000);
we have a common mdelay(), so please use
mdelay(delay);
> +
> + /* only if transfer mode is LPDT, wait SFR becomes empty. */
> + if (dsim->state == DSIM_STATE_STOP) {
> + while (!(exynos_mipi_dsi_get_fifo_state(dsim) &
> + SFR_HEADER_EMPTY)) {
> + if ((timeout--) > 0)
> + udelay(1000);
> + else {
> + debug("SRF header fifo is not empty.\n");
> + return -EINVAL;
> + }
> + }
> + }
> +
> + switch (data_id) {
> + /* short packet types of packet types for command. */
> + case MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM:
> + case MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM:
> + case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM:
> + case MIPI_DSI_DCS_SHORT_WRITE:
> + case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
> + case MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE:
> + debug("data0 = %x data1 = %x\n",
> + data0, data1);
> + exynos_mipi_dsi_wr_tx_header(dsim, data_id, data0, data1);
> + if (check_rx_ack)
> + /* process response func should be implemented */
> + return 0;
> + else
> + return -EINVAL;
Please use following style for multiline if statements
if (check_rx_ack) {
/* process response func should be implemented */
return 0;
} else {
return -EINVAL;
}
> +
> + /* general command */
> + case MIPI_DSI_COLOR_MODE_OFF:
> + case MIPI_DSI_COLOR_MODE_ON:
> + case MIPI_DSI_SHUTDOWN_PERIPHERAL:
> + case MIPI_DSI_TURN_ON_PERIPHERAL:
> + exynos_mipi_dsi_wr_tx_header(dsim, data_id, data0, data1);
> + if (check_rx_ack)
> + /* process response func should be implemented. */
> + return 0;
> + else
> + return -EINVAL;
same applies here. There are other similar statements in this file.
Please fix them too.
> +
> + /* packet types for video data */
> + case MIPI_DSI_V_SYNC_START:
> + case MIPI_DSI_V_SYNC_END:
> + case MIPI_DSI_H_SYNC_START:
> + case MIPI_DSI_H_SYNC_END:
> + case MIPI_DSI_END_OF_TRANSMISSION:
> + return 0;
> +
> + /* short and response packet types for command */
> + case MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM:
> + case MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM:
> + case MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM:
> + case MIPI_DSI_DCS_READ:
> + exynos_mipi_dsi_clear_all_interrupt(dsim);
> + exynos_mipi_dsi_wr_tx_header(dsim, data_id, data0, data1);
> + /* process response func should be implemented. */
> + return 0;
> +
> + /* long packet type and null packet */
> + case MIPI_DSI_NULL_PACKET:
> + case MIPI_DSI_BLANKING_PACKET:
> + return 0;
> + case MIPI_DSI_GENERIC_LONG_WRITE:
> + case MIPI_DSI_DCS_LONG_WRITE:
> + {
> + unsigned int size, data_cnt = 0, payload = 0;
> +
> + size = data1 * 4;
> +
The local variable 'size' is not really used here, I get a compiler
warning:
exynos_mipi_dsi_common.c: In function 'exynos_mipi_dsi_wr_data':
exynos_mipi_dsi_common.c:200:16: warning: variable 'size' set but not used
[-Wunused-but-set-variable]
So please drop it.
> + /* if data count is less then 4, then send 3bytes data. */
> + if (data1 < 4) {
> + payload = *(u8 *)(data0) |
> + *(u8 *)(data0 + 1) << 8 |
> + *(u8 *)(data0 + 2) << 16;
> +
> + exynos_mipi_dsi_wr_tx_data(dsim, payload);
> +
> + debug("count = %d payload = %x,%x %x %x\n",
> + data1, payload,
> + *(u8 *)(data0 + data_cnt),
> + *(u8 *)(data0 + (data_cnt + 1)),
> + *(u8 *)(data0 + (data_cnt + 2)));
> + /* in case that data count is more then 4 */
> + } else
> + exynos_mipi_dsi_long_data_wr(dsim, data0, data1);
} else {
/* in case that data count is more then 4 */
exynos_mipi_dsi_long_data_wr(dsim, data0, data1);
}
> +
> + /* put data into header fifo */
> + exynos_mipi_dsi_wr_tx_header(dsim, data_id, data1 & 0xff,
> + (data1 & 0xff00) >> 8);
> +
> + }
> + if (check_rx_ack)
> + /* process response func should be implemented. */
> + return 0;
> + else
> + return -EINVAL;
> +
> + /* packet typo for video data */
> + case MIPI_DSI_PACKED_PIXEL_STREAM_16:
> + case MIPI_DSI_PACKED_PIXEL_STREAM_18:
> + case MIPI_DSI_PIXEL_STREAM_3BYTE_18:
> + case MIPI_DSI_PACKED_PIXEL_STREAM_24:
> + if (check_rx_ack)
> + /* process response func should be implemented. */
> + return 0;
> + else
> + return -EINVAL;
> + default:
> + debug("data id %x is not supported current DSI spec.\n",
> + data_id);
> +
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
...
> +int exynos_mipi_dsi_set_clock(struct mipi_dsim_device *dsim,
> + unsigned int byte_clk_sel, unsigned int enable)
> +{
> + unsigned int esc_div;
> + unsigned long esc_clk_error_rate;
> + unsigned long hs_clk = 0, byte_clk = 0, escape_clk = 0;
> +
> + if (enable) {
> + dsim->e_clk_src = byte_clk_sel;
> +
> + /* Escape mode clock and byte clock source */
> + exynos_mipi_dsi_set_byte_clock_src(dsim, byte_clk_sel);
> +
> + /* DPHY, DSIM Link : D-PHY clock out */
> + if (byte_clk_sel == DSIM_PLL_OUT_DIV8) {
> + hs_clk = exynos_mipi_dsi_change_pll(dsim,
> + dsim->dsim_config->p, dsim->dsim_config->m,
> + dsim->dsim_config->s);
> + if (hs_clk == 0) {
> + debug("failed to get hs clock.\n");
> + return -EINVAL;
> + }
> +
> + byte_clk = hs_clk / 8;
> + exynos_mipi_dsi_enable_pll_bypass(dsim, 0);
> + exynos_mipi_dsi_pll_on(dsim, 1);
> + /* DPHY : D-PHY clock out, DSIM link : external clock out */
> + } else if (byte_clk_sel == DSIM_EXT_CLK_DIV8)
> + debug("not support EXT CLK source for MIPI DSIM\n");
> + else if (byte_clk_sel == DSIM_EXT_CLK_BYPASS)
> + debug("not support EXT CLK source for MIPI DSIM\n");
> +
> + /* escape clock divider */
> + esc_div = byte_clk / (dsim->dsim_config->esc_clk);
> + debug("esc_div = %d, byte_clk = %lu, esc_clk = %lu\n",
> + esc_div, byte_clk, dsim->dsim_config->esc_clk);
> + if ((byte_clk / esc_div) >= (20 * MHZ) ||
> + (byte_clk / esc_div) >
> + dsim->dsim_config->esc_clk)
> + esc_div += 1;
if ((byte_clk / esc_div) >= (20 * MHZ) ||
(byte_clk / esc_div) > dsim->dsim_config->esc_clk)
esc_div += 1;
...
> +int exynos_mipi_dsi_init_link(struct mipi_dsim_device *dsim)
> +{
> + unsigned int time_out = 100;
> +
> + switch (dsim->state) {
> + case DSIM_STATE_INIT:
> + exynos_mipi_dsi_init_fifo_pointer(dsim, 0x1f);
> +
> + /* dsi configuration */
> + exynos_mipi_dsi_init_config(dsim);
> + exynos_mipi_dsi_enable_lane(dsim, DSIM_LANE_CLOCK, 1);
> + exynos_mipi_dsi_enable_lane(dsim, dsim->data_lane, 1);
> +
> + /* set clock configuration */
> + exynos_mipi_dsi_set_clock(dsim,
> + dsim->dsim_config->e_byte_clk, 1);
> +
> + /* check clock and data lane state are stop state */
> + while (!(exynos_mipi_dsi_is_lane_state(dsim))) {
> + time_out--;
> + if (time_out == 0) {
> + debug("DSI Master is not stop state.\n");
> + debug("Check initialization process\n");
> +
> + return -EINVAL;
> + }
> + }
> +
> + if (time_out != 0) {
This check is not necessary here since this condition is always true.
Please remove it.
> + debug("DSI Master driver has been completed.\n");
> + debug("DSI Master state is stop state\n");
> + }
> +
> + dsim->state = DSIM_STATE_STOP;
> +
> + /* BTA sequence counters */
> + exynos_mipi_dsi_set_stop_state_counter(dsim,
> + dsim->dsim_config->stop_holding_cnt);
> + exynos_mipi_dsi_set_bta_timeout(dsim,
> + dsim->dsim_config->bta_timeout);
> + exynos_mipi_dsi_set_lpdr_timeout(dsim,
> + dsim->dsim_config->rx_timeout);
> +
> +
Please drop empty line.
> + return 0;
> + default:
> + debug("DSI Master is already init.\n");
> + return 0;
> + }
> +
> + return 0;
> +}
...
> diff --git a/drivers/video/exynos_mipi_dsi_lowlevel.c
> b/drivers/video/exynos_mipi_dsi_lowlevel.c
> new file mode 100644
> index 0000000..cc52ede
> --- /dev/null
> +++ b/drivers/video/exynos_mipi_dsi_lowlevel.c
...
> +#include <common.h>
> +#include <lcd.h>
> +#include <asm/errno.h>
Please drop lcd.h and asm/errno.h
> +#include <asm/arch/dsim.h>
> +#include <asm/arch/mipi_dsim.h>
> +#include <asm/arch/power.h>
> +#include <asm/arch/cpu.h>
> +#include <linux/types.h>
> +#include <linux/list.h>
Please drop linux/types.h and linux/list.h
...
> +void exynos_mipi_dsi_sw_release(struct mipi_dsim_device *dsim)
> +{
> + struct exynos_mipi_dsim *mipi_dsim =
> + (struct exynos_mipi_dsim *)samsung_get_base_mipi_dsim();
> + unsigned int reg = readl(&mipi_dsim->intsrc);
> +
> + reg |= INTSRC_SWRST_RELEASE;
> +
> + writel(reg, &mipi_dsim->intsrc);
> +
> + reg = 0;
> + reg = readl(&mipi_dsim->intsrc);
Please replace the above two lines by
readl(&mipi_dsim->intsrc);
...
> +void exynos_mipi_dsi_init_config(struct mipi_dsim_device *dsim)
> +{
> + struct mipi_dsim_config *dsim_config = dsim->dsim_config;
> + struct exynos_mipi_dsim *mipi_dsim =
> + (struct exynos_mipi_dsim *)samsung_get_base_mipi_dsim();
> + unsigned int cfg = (readl(&mipi_dsim->config)) &
> + ~((1 << DSIM_EOT_PACKET_SHIFT) |
> + (0x1f << DSIM_HSA_MODE_SHIFT) |
> + (0x3 << DSIM_NUM_OF_DATALANE_SHIFT));
> +
> + cfg = (dsim_config->auto_flush << DSIM_AUTO_FLUSH_SHIFT) |
Shouldn't this be
cfg |= (dsim_config->auto_flush << DSIM_AUTO_FLUSH_SHIFT) |
?
> + (dsim_config->eot_disable << DSIM_EOT_PACKET_SHIFT) |
> + (dsim_config->auto_vertical_cnt << DSIM_AUTO_MODE_SHIFT) |
> + (dsim_config->hse << DSIM_HSE_MODE_SHIFT) |
> + (dsim_config->hfp << DSIM_HFP_MODE_SHIFT) |
> + (dsim_config->hbp << DSIM_HBP_MODE_SHIFT) |
> + (dsim_config->hsa << DSIM_HSA_MODE_SHIFT) |
> + (dsim_config->e_no_data_lane << DSIM_NUM_OF_DATALANE_SHIFT);
> +
> + writel(cfg, &mipi_dsim->config);
> +}
...
> +void exynos_mipi_dsi_enable_lane(struct mipi_dsim_device *dsim,
> + unsigned int lane, unsigned int enable)
> +{
> + unsigned int reg;
> + struct exynos_mipi_dsim *mipi_dsim =
> + (struct exynos_mipi_dsim *)samsung_get_base_mipi_dsim();
> +
> + reg = readl(&mipi_dsim->config);
> +
> + if (enable)
> + reg |= DSIM_LANE_ENx(lane);
> + else
> + reg &= ~DSIM_LANE_ENx(lane);
> +
> + writel(reg, &mipi_dsim->config);
> +}
> +
> +
Please drop one empty line.
...
> +void exynos_mipi_dsi_enable_esc_clk_on_lane(struct mipi_dsim_device *dsim,
> + unsigned int lane_sel, unsigned int enable)
> +{
> + struct exynos_mipi_dsim *mipi_dsim =
> + (struct exynos_mipi_dsim *)samsung_get_base_mipi_dsim();
> + unsigned int reg = readl(&mipi_dsim->clkctrl);
> +
> + if (enable)
> + reg |= DSIM_LANE_ESC_CLKEN(lane_sel);
> + else
> +
Please drop empty line.
> + reg &= ~DSIM_LANE_ESC_CLKEN(lane_sel);
> +
> + writel(reg, &mipi_dsim->clkctrl);
> +}
...
> +void exynos_mipi_dsi_clear_all_interrupt(struct mipi_dsim_device *dsim)
> +{
> + struct exynos_mipi_dsim *mipi_dsim =
> + (struct exynos_mipi_dsim *)samsung_get_base_mipi_dsim();
> + unsigned int reg = readl(&mipi_dsim->intsrc);
> +
> + reg |= 0xffffffff;
> +
> + writel(reg, &mipi_dsim->intsrc);
Is reading the intsrc register necessary here? Can't you just do
writel(0xffffffff, &mipi_dsim->intsrc);
?
...
> +unsigned int exynos_mipi_dsi_get_fifo_state(struct mipi_dsim_device *dsim)
> +{
> + unsigned int ret;
> + struct exynos_mipi_dsim *mipi_dsim =
> + (struct exynos_mipi_dsim *)samsung_get_base_mipi_dsim();
> +
> + ret = readl(&mipi_dsim->fifoctrl) & ~(0x1f);
> +
> + return ret;
you can eliminate local variable 'ret' here and just do
return readl(&mipi_dsim->fifoctrl) & ~(0x1f);
> +}
Thanks,
Anatolij
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
