| Message ID | 1381352915-17219-1-git-send-email-marek@goldelico.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 10/10/13 00:08, Marek Belisko wrote: > For communicating with driver is used gpio bitbanging because TD028 does > not have a standard compliant SPI interface. It is a 3-wire thing with > direction reversal. Isn't that SPI_3WIRE? > Communication with display is used only during panel enable/disable so it's > not performance issue. > > Signed-off-by: Marek Belisko <marek@goldelico.com> > Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> > --- > drivers/video/omap2/displays-new/Kconfig | 6 + > drivers/video/omap2/displays-new/Makefile | 1 + > .../omap2/displays-new/panel-tpo-td028ttec1.c | 537 +++++++++++++++++++++ > include/video/omap-panel-data.h | 22 + > 4 files changed, 566 insertions(+) > create mode 100644 drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c > > diff --git a/drivers/video/omap2/displays-new/Kconfig b/drivers/video/omap2/displays-new/Kconfig > index 6c90885..3f86432 100644 > --- a/drivers/video/omap2/displays-new/Kconfig > +++ b/drivers/video/omap2/displays-new/Kconfig > @@ -56,6 +56,11 @@ config DISPLAY_PANEL_SHARP_LS037V7DW01 > help > LCD Panel used in TI's SDP3430 and EVM boards > > +config DISPLAY_PANEL_TPO_TD028TTEC1 > + tristate "TPO TD028TTEC1 LCD Panel" > + help > + LCD panel used by Openmoko. > + > config DISPLAY_PANEL_TPO_TD043MTEA1 > tristate "TPO TD043MTEA1 LCD Panel" > depends on SPI > @@ -70,4 +75,5 @@ config DISPLAY_PANEL_NEC_NL8048HL11 > This NEC NL8048HL11 panel is TFT LCD used in the > Zoom2/3/3630 sdp boards. > > + Extra change. > endmenu > diff --git a/drivers/video/omap2/displays-new/Makefile b/drivers/video/omap2/displays-new/Makefile > index 5aeb11b..0323a8a 100644 > --- a/drivers/video/omap2/displays-new/Makefile > +++ b/drivers/video/omap2/displays-new/Makefile > @@ -8,5 +8,6 @@ obj-$(CONFIG_DISPLAY_PANEL_DSI_CM) += panel-dsi-cm.o > obj-$(CONFIG_DISPLAY_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o > obj-$(CONFIG_DISPLAY_PANEL_LGPHILIPS_LB035Q02) += panel-lgphilips-lb035q02.o > obj-$(CONFIG_DISPLAY_PANEL_SHARP_LS037V7DW01) += panel-sharp-ls037v7dw01.o > +obj-$(CONFIG_DISPLAY_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o > obj-$(CONFIG_DISPLAY_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o > obj-$(CONFIG_DISPLAY_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o > diff --git a/drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c b/drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c > new file mode 100644 > index 0000000..b63586e > --- /dev/null > +++ b/drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c > @@ -0,0 +1,537 @@ > +/* > + * Toppoly TD028TTEC1 panel support > + * > + * Copyright (C) 2008 Nokia Corporation > + * Author: Tomi Valkeinen <tomi.valkeinen@nokia.com> > + * > + * Neo 1973 code (jbt6k74.c): > + * Copyright (C) 2006-2007 by OpenMoko, Inc. > + * Author: Harald Welte <laforge@openmoko.org> > + * > + * Ported and adapted from Neo 1973 U-Boot by: > + * H. Nikolaus Schaller <hns@goldelico.com> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published by > + * the Free Software Foundation. > + * > + * 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, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <linux/module.h> > +#include <linux/delay.h> > +#include <linux/platform_device.h> > +#include <linux/gpio.h> > +#include <video/omapdss.h> > +#include <video/omap-panel-data.h> > + > +struct panel_drv_data { > + struct omap_dss_device dssdev; > + struct omap_dss_device *in; > + > + int data_lines; > + > + struct omap_video_timings videomode; > + > + int cs_gpio; > + int scl_gpio; > + int din_gpio; > + int dout_gpio; Three wires, but four gpios? What am I missing here... > + u_int16_t tx_buf[4]; Hmm, what's wrong with "u16"? > +}; > + > +static struct omap_video_timings td028ttec1_panel_timings = { > + .x_res = 480, > + .y_res = 640, > + .pixel_clock = 22153, > + .hfp = 24, > + .hsw = 8, > + .hbp = 8, > + .vfp = 4, > + .vsw = 2, > + .vbp = 2, > + > + .vsync_level = OMAPDSS_SIG_ACTIVE_LOW, > + .hsync_level = OMAPDSS_SIG_ACTIVE_LOW, > + > + .data_pclk_edge = OMAPDSS_DRIVE_SIG_FALLING_EDGE, > + .de_level = OMAPDSS_SIG_ACTIVE_HIGH, > + .sync_pclk_edge = OMAPDSS_DRIVE_SIG_OPPOSITE_EDGES, > +}; > + > +#define JBT_COMMAND 0x000 > +#define JBT_DATA 0x100 > + > +/* 150uS minimum clock cycle, we have two of this plus our other > + * instructions */ > + > +#define SPI_DELAY() udelay(200) Would usleep_range work here? See Documentation/timers/timers-howto.txt > +static int jbt_spi_xfer(struct panel_drv_data *data, int wordnum, int bitlen) Hmm, if it's not SPI, maybe it shouldn't be called SPI? > +{ > + u_int16_t tmpdout = 0; > + int i, j; > + > + gpio_set_value(data->cs_gpio, 0); > + > + for (i = 0; i < wordnum; i++) { > + tmpdout = data->tx_buf[i]; > + > + for (j = 0; j < bitlen; j++) { > + gpio_set_value(data->scl_gpio, 0); > + if (tmpdout & (1 << (bitlen-1))) { > + gpio_set_value(data->dout_gpio, 1); > + if (gpio_get_value(data->din_gpio) == 0) > + return 1; > + } else { > + gpio_set_value(data->dout_gpio, 0); > + if (gpio_get_value(data->din_gpio) != 0) > + return 1; > + } > + SPI_DELAY(); > + gpio_set_value(data->scl_gpio, 1); > + SPI_DELAY(); > + tmpdout <<= 1; > + } > + } > + > + gpio_set_value(data->cs_gpio, 1); > + > + return 0; > +} > + > +#define JBT_COMMAND 0x000 > +#define JBT_DATA 0x100 > + > +int jbt_reg_write_nodata(struct panel_drv_data *ddata, u_int8_t reg) What's wrong with "u8"? Have I missed a memo? =) > +{ > + int rc; > + > + ddata->tx_buf[0] = JBT_COMMAND | reg; > + > + rc = jbt_spi_xfer(ddata, 1, 9); > + if (rc) > + dev_warn(ddata->dssdev.dev, "Failed to write reg: %x\n", reg); > + > + return rc; > +} > + > +int jbt_reg_write(struct panel_drv_data *ddata, u_int8_t reg, u_int8_t data) > +{ > + int rc; > + > + ddata->tx_buf[0] = JBT_COMMAND | reg; > + ddata->tx_buf[1] = JBT_DATA | data; > + > + rc = jbt_spi_xfer(ddata, 2, 9); > + if (rc) > + dev_warn(ddata->dssdev.dev, "Failed to write reg: %x\n", reg); > + > + return rc; > +} > + > +int jbt_reg_write16(struct panel_drv_data *ddata, u_int8_t reg, u_int16_t data) > +{ > + int rc; > + > + ddata->tx_buf[0] = JBT_COMMAND | reg; > + ddata->tx_buf[1] = JBT_DATA | (data >> 8); > + ddata->tx_buf[2] = JBT_DATA | (data & 0xff); > + > + rc = jbt_spi_xfer(ddata, 3, 9); > + if (rc) > + dev_warn(ddata->dssdev.dev, "Failed to write reg: %x\n", reg); > + > + return rc; > +} > + > +enum jbt_register { > + JBT_REG_SLEEP_IN = 0x10, > + JBT_REG_SLEEP_OUT = 0x11, > + > + JBT_REG_DISPLAY_OFF = 0x28, > + JBT_REG_DISPLAY_ON = 0x29, > + > + JBT_REG_RGB_FORMAT = 0x3a, > + JBT_REG_QUAD_RATE = 0x3b, > + > + JBT_REG_POWER_ON_OFF = 0xb0, > + JBT_REG_BOOSTER_OP = 0xb1, > + JBT_REG_BOOSTER_MODE = 0xb2, > + JBT_REG_BOOSTER_FREQ = 0xb3, > + JBT_REG_OPAMP_SYSCLK = 0xb4, > + JBT_REG_VSC_VOLTAGE = 0xb5, > + JBT_REG_VCOM_VOLTAGE = 0xb6, > + JBT_REG_EXT_DISPL = 0xb7, > + JBT_REG_OUTPUT_CONTROL = 0xb8, > + JBT_REG_DCCLK_DCEV = 0xb9, > + JBT_REG_DISPLAY_MODE1 = 0xba, > + JBT_REG_DISPLAY_MODE2 = 0xbb, > + JBT_REG_DISPLAY_MODE = 0xbc, > + JBT_REG_ASW_SLEW = 0xbd, > + JBT_REG_DUMMY_DISPLAY = 0xbe, > + JBT_REG_DRIVE_SYSTEM = 0xbf, > + > + JBT_REG_SLEEP_OUT_FR_A = 0xc0, > + JBT_REG_SLEEP_OUT_FR_B = 0xc1, > + JBT_REG_SLEEP_OUT_FR_C = 0xc2, > + JBT_REG_SLEEP_IN_LCCNT_D = 0xc3, > + JBT_REG_SLEEP_IN_LCCNT_E = 0xc4, > + JBT_REG_SLEEP_IN_LCCNT_F = 0xc5, > + JBT_REG_SLEEP_IN_LCCNT_G = 0xc6, > + > + JBT_REG_GAMMA1_FINE_1 = 0xc7, > + JBT_REG_GAMMA1_FINE_2 = 0xc8, > + JBT_REG_GAMMA1_INCLINATION = 0xc9, > + JBT_REG_GAMMA1_BLUE_OFFSET = 0xca, > + > + JBT_REG_BLANK_CONTROL = 0xcf, > + JBT_REG_BLANK_TH_TV = 0xd0, > + JBT_REG_CKV_ON_OFF = 0xd1, > + JBT_REG_CKV_1_2 = 0xd2, > + JBT_REG_OEV_TIMING = 0xd3, > + JBT_REG_ASW_TIMING_1 = 0xd4, > + JBT_REG_ASW_TIMING_2 = 0xd5, > + > + JBT_REG_HCLOCK_VGA = 0xec, > + JBT_REG_HCLOCK_QVGA = 0xed, > +}; > + > +#define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev) > + > +static int td028ttec1_panel_connect(struct omap_dss_device *dssdev) > +{ > + struct panel_drv_data *ddata = to_panel_data(dssdev); > + struct omap_dss_device *in = ddata->in; > + int r; > + > + if (omapdss_device_is_connected(dssdev)) > + return 0; > + > + r = in->ops.dpi->connect(in, dssdev); > + if (r) > + return r; > + > + return 0; > +} > + > +static void td028ttec1_panel_disconnect(struct omap_dss_device *dssdev) > +{ > + struct panel_drv_data *ddata = to_panel_data(dssdev); > + struct omap_dss_device *in = ddata->in; > + > + if (!omapdss_device_is_connected(dssdev)) > + return; > + > + in->ops.dpi->disconnect(in, dssdev); > +} > + > +static int td028ttec1_panel_enable(struct omap_dss_device *dssdev) > +{ > + struct panel_drv_data *ddata = to_panel_data(dssdev); > + struct omap_dss_device *in = ddata->in; > + int r; > + > + if (!omapdss_device_is_connected(dssdev)) > + return -ENODEV; > + > + if (omapdss_device_is_enabled(dssdev)) > + return 0; > + > + in->ops.dpi->set_data_lines(in, ddata->data_lines); > + in->ops.dpi->set_timings(in, &ddata->videomode); > + > + r = in->ops.dpi->enable(in); > + if (r) > + return r; > + > + dev_dbg(dssdev->dev, "td028ttec1_panel_enable() - state %d\n", > + dssdev->state); > + > + /* three times command zero */ > + r |= jbt_reg_write_nodata(ddata, 0x00); > + udelay(1000); These are big delays, and I guess the timing is not strict here, so maybe some sleep variant would be better. > + r |= jbt_reg_write_nodata(ddata, 0x00); > + udelay(1000); > + r |= jbt_reg_write_nodata(ddata, 0x00); > + udelay(1000); > + > + /* deep standby out */ > + r |= jbt_reg_write(ddata, JBT_REG_POWER_ON_OFF, 0x17); > + > + /* RGB I/F on, RAM write off, QVGA through, SIGCON enable */ > + r |= jbt_reg_write(ddata, JBT_REG_DISPLAY_MODE, 0x80); > + > + /* Quad mode off */ > + r |= jbt_reg_write(ddata, JBT_REG_QUAD_RATE, 0x00); > + > + /* AVDD on, XVDD on */ > + r |= jbt_reg_write(ddata, JBT_REG_POWER_ON_OFF, 0x16); > + > + /* Output control */ > + r |= jbt_reg_write16(ddata, JBT_REG_OUTPUT_CONTROL, 0xfff9); > + > + /* Sleep mode off */ > + r |= jbt_reg_write_nodata(ddata, JBT_REG_SLEEP_OUT); > + > + /* at this point we have like 50% grey */ > + > + /* initialize register set */ > + r |= jbt_reg_write(ddata, JBT_REG_DISPLAY_MODE1, 0x01); > + r |= jbt_reg_write(ddata, JBT_REG_DISPLAY_MODE2, 0x00); > + r |= jbt_reg_write(ddata, JBT_REG_RGB_FORMAT, 0x60); > + r |= jbt_reg_write(ddata, JBT_REG_DRIVE_SYSTEM, 0x10); > + r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_OP, 0x56); > + r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_MODE, 0x33); > + r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_FREQ, 0x11); > + r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_FREQ, 0x11); > + r |= jbt_reg_write(ddata, JBT_REG_OPAMP_SYSCLK, 0x02); > + r |= jbt_reg_write(ddata, JBT_REG_VSC_VOLTAGE, 0x2b); > + r |= jbt_reg_write(ddata, JBT_REG_VCOM_VOLTAGE, 0x40); > + r |= jbt_reg_write(ddata, JBT_REG_EXT_DISPL, 0x03); > + r |= jbt_reg_write(ddata, JBT_REG_DCCLK_DCEV, 0x04); > + /* > + * default of 0x02 in JBT_REG_ASW_SLEW responsible for 72Hz requirement > + * to avoid red / blue flicker > + */ > + r |= jbt_reg_write(ddata, JBT_REG_ASW_SLEW, 0x04); > + r |= jbt_reg_write(ddata, JBT_REG_DUMMY_DISPLAY, 0x00); > + > + r |= jbt_reg_write(ddata, JBT_REG_SLEEP_OUT_FR_A, 0x11); > + r |= jbt_reg_write(ddata, JBT_REG_SLEEP_OUT_FR_B, 0x11); > + r |= jbt_reg_write(ddata, JBT_REG_SLEEP_OUT_FR_C, 0x11); > + r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_D, 0x2040); > + r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_E, 0x60c0); > + r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_F, 0x1020); > + r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_G, 0x60c0); > + > + r |= jbt_reg_write16(ddata, JBT_REG_GAMMA1_FINE_1, 0x5533); > + r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_FINE_2, 0x00); > + r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_INCLINATION, 0x00); > + r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_BLUE_OFFSET, 0x00); > + r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_BLUE_OFFSET, 0x00); > + > + r |= jbt_reg_write16(ddata, JBT_REG_HCLOCK_VGA, 0x1f0); > + r |= jbt_reg_write(ddata, JBT_REG_BLANK_CONTROL, 0x02); > + r |= jbt_reg_write16(ddata, JBT_REG_BLANK_TH_TV, 0x0804); > + r |= jbt_reg_write16(ddata, JBT_REG_BLANK_TH_TV, 0x0804); > + > + r |= jbt_reg_write(ddata, JBT_REG_CKV_ON_OFF, 0x01); > + r |= jbt_reg_write16(ddata, JBT_REG_CKV_1_2, 0x0000); > + > + r |= jbt_reg_write16(ddata, JBT_REG_OEV_TIMING, 0x0d0e); > + r |= jbt_reg_write16(ddata, JBT_REG_ASW_TIMING_1, 0x11a4); > + r |= jbt_reg_write(ddata, JBT_REG_ASW_TIMING_2, 0x0e); > + > + r |= jbt_reg_write_nodata(ddata, JBT_REG_DISPLAY_ON); > + > + dssdev->state = OMAP_DSS_DISPLAY_ACTIVE; > + > + return r; > +} > + > +static void td028ttec1_panel_disable(struct omap_dss_device *dssdev) > +{ > + struct panel_drv_data *ddata = to_panel_data(dssdev); > + struct omap_dss_device *in = ddata->in; > + > + if (!omapdss_device_is_enabled(dssdev)) > + return; > + > + dev_dbg(dssdev->dev, "td028ttec1_panel_disable()\n"); > + > + in->ops.dpi->disable(in); > + > + jbt_reg_write_nodata(ddata, JBT_REG_DISPLAY_OFF); > + jbt_reg_write16(ddata, JBT_REG_OUTPUT_CONTROL, 0x8002); > + jbt_reg_write_nodata(ddata, JBT_REG_SLEEP_IN); > + jbt_reg_write(ddata, JBT_REG_POWER_ON_OFF, 0x00); > + > + dssdev->state = OMAP_DSS_DISPLAY_DISABLED; > +} > + > +static void td028ttec1_panel_set_timings(struct omap_dss_device *dssdev, > + struct omap_video_timings *timings) > +{ > + struct panel_drv_data *ddata = to_panel_data(dssdev); > + struct omap_dss_device *in = ddata->in; > + > + ddata->videomode = *timings; > + dssdev->panel.timings = *timings; > + > + in->ops.dpi->set_timings(in, timings); > +} > + > +static void td028ttec1_panel_get_timings(struct omap_dss_device *dssdev, > + struct omap_video_timings *timings) > +{ > + struct panel_drv_data *ddata = to_panel_data(dssdev); > + > + *timings = ddata->videomode; > +} > + > +static int td028ttec1_panel_check_timings(struct omap_dss_device *dssdev, > + struct omap_video_timings *timings) > +{ > + struct panel_drv_data *ddata = to_panel_data(dssdev); > + struct omap_dss_device *in = ddata->in; > + > + return in->ops.dpi->check_timings(in, timings); > +} > + > +static struct omap_dss_driver td028ttec1_ops = { > + .connect = td028ttec1_panel_connect, > + .disconnect = td028ttec1_panel_disconnect, > + > + .enable = td028ttec1_panel_enable, > + .disable = td028ttec1_panel_disable, > + > + .set_timings = td028ttec1_panel_set_timings, > + .get_timings = td028ttec1_panel_get_timings, > + .check_timings = td028ttec1_panel_check_timings, > +}; > + > +static int td028ttec1_panel_probe_pdata(struct platform_device *pdev) > +{ > + const struct panel_tpo_td028tec1_platform_data *pdata; > + struct panel_drv_data *ddata = platform_get_drvdata(pdev); > + struct omap_dss_device *dssdev, *in; > + > + pdata = dev_get_platdata(&pdev->dev); > + > + in = omap_dss_find_output(pdata->source); > + if (in == NULL) { > + dev_err(&pdev->dev, "failed to find video source '%s'\n", > + pdata->source); > + return -EPROBE_DEFER; > + } > + > + ddata->in = in; > + > + ddata->data_lines = pdata->data_lines; > + > + dssdev = &ddata->dssdev; > + dssdev->name = pdata->name; > + > + ddata->cs_gpio = pdata->cs_gpio; > + ddata->scl_gpio = pdata->scl_gpio; > + ddata->din_gpio = pdata->din_gpio; > + ddata->dout_gpio = pdata->dout_gpio; > + > + return 0; > +} > + > +static int td028ttec1_panel_probe(struct platform_device *pdev) > +{ > + struct panel_drv_data *ddata; > + struct omap_dss_device *dssdev; > + int r; > + > + ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL); > + if (ddata == NULL) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, ddata); > + > + if (dev_get_platdata(&pdev->dev)) { > + r = td028ttec1_panel_probe_pdata(pdev); > + if (r) > + return r; > + } else { > + return -ENODEV; > + } > + > + if (gpio_is_valid(ddata->cs_gpio)) { > + r = devm_gpio_request_one(&pdev->dev, ddata->cs_gpio, > + GPIOF_OUT_INIT_HIGH, "lcd cs"); > + if (r) > + goto err_gpio; > + } > + > + if (gpio_is_valid(ddata->scl_gpio)) { > + r = devm_gpio_request_one(&pdev->dev, ddata->scl_gpio, > + GPIOF_OUT_INIT_HIGH, "lcd scl"); > + if (r) > + goto err_gpio; > + } > + > + if (gpio_is_valid(ddata->dout_gpio)) { > + r = devm_gpio_request_one(&pdev->dev, ddata->dout_gpio, > + GPIOF_OUT_INIT_LOW, "lcd dout"); > + if (r) > + goto err_gpio; > + } > + > + if (gpio_is_valid(ddata->din_gpio)) { > + r = devm_gpio_request_one(&pdev->dev, ddata->din_gpio, > + GPIOF_IN, "lcd din"); > + if (r) > + goto err_gpio; > + } > + > + /* according to data sheet: wait 50ms (Tpos of LCM). However, 50ms > + * seems unreliable with later LCM batches, increasing to 90ms */ > + mdelay(90); What is this delay for? Usually sleeps/delays should be between two "HW actions". Here there's nothing below this line that would count as such an action. > + ddata->videomode = td028ttec1_panel_timings; > + > + dssdev = &ddata->dssdev; > + dssdev->dev = &pdev->dev; > + dssdev->driver = &td028ttec1_ops; > + dssdev->type = OMAP_DISPLAY_TYPE_DPI; > + dssdev->owner = THIS_MODULE; > + dssdev->panel.timings = ddata->videomode; > + dssdev->phy.dpi.data_lines = ddata->data_lines; > + > + r = omapdss_register_display(dssdev); > + if (r) { > + dev_err(&pdev->dev, "Failed to register panel\n"); > + goto err_reg; > + } > + > + return 0; > + > +err_reg: > +err_gpio: > + omap_dss_put_device(ddata->in); > + return r; > +} > + Tomi
Hi Tomi, Am 10.10.2013 um 10:19 schrieb Tomi Valkeinen: > Hi, > > On 10/10/13 00:08, Marek Belisko wrote: >> For communicating with driver is used gpio bitbanging because TD028 does >> not have a standard compliant SPI interface. It is a 3-wire thing with >> direction reversal. > > Isn't that SPI_3WIRE? Maybe - but we have no complete datasheet and I don't know an official spec of a 3-wire SPI protocol to compare how 3-wire should work. And I think (but I may be wrong) that SPI_3WIRE is an optional feature of the SoC specific SPI drivers and in my understanding the OMAP has no such driver: http://lxr.free-electrons.com/source/drivers/spi/spi-omap2-mcspi.c#L874 And spi-bitbang.c hasn't either. So I would prefer to leave this as an area for optimizations for future work. Maybe we can add some more comments on this. > >> Communication with display is used only during panel enable/disable so it's >> not performance issue. >> >> Signed-off-by: Marek Belisko <marek@goldelico.com> >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> >> --- >> drivers/video/omap2/displays-new/Kconfig | 6 + >> drivers/video/omap2/displays-new/Makefile | 1 + >> .../omap2/displays-new/panel-tpo-td028ttec1.c | 537 +++++++++++++++++++++ >> include/video/omap-panel-data.h | 22 + >> 4 files changed, 566 insertions(+) >> create mode 100644 drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c >> >> diff --git a/drivers/video/omap2/displays-new/Kconfig b/drivers/video/omap2/displays-new/Kconfig >> index 6c90885..3f86432 100644 >> --- a/drivers/video/omap2/displays-new/Kconfig >> +++ b/drivers/video/omap2/displays-new/Kconfig >> @@ -56,6 +56,11 @@ config DISPLAY_PANEL_SHARP_LS037V7DW01 >> help >> LCD Panel used in TI's SDP3430 and EVM boards >> >> +config DISPLAY_PANEL_TPO_TD028TTEC1 >> + tristate "TPO TD028TTEC1 LCD Panel" >> + help >> + LCD panel used by Openmoko. >> + >> config DISPLAY_PANEL_TPO_TD043MTEA1 >> tristate "TPO TD043MTEA1 LCD Panel" >> depends on SPI >> @@ -70,4 +75,5 @@ config DISPLAY_PANEL_NEC_NL8048HL11 >> This NEC NL8048HL11 panel is TFT LCD used in the >> Zoom2/3/3630 sdp boards. >> >> + > > Extra change. > >> endmenu >> diff --git a/drivers/video/omap2/displays-new/Makefile b/drivers/video/omap2/displays-new/Makefile >> index 5aeb11b..0323a8a 100644 >> --- a/drivers/video/omap2/displays-new/Makefile >> +++ b/drivers/video/omap2/displays-new/Makefile >> @@ -8,5 +8,6 @@ obj-$(CONFIG_DISPLAY_PANEL_DSI_CM) += panel-dsi-cm.o >> obj-$(CONFIG_DISPLAY_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o >> obj-$(CONFIG_DISPLAY_PANEL_LGPHILIPS_LB035Q02) += panel-lgphilips-lb035q02.o >> obj-$(CONFIG_DISPLAY_PANEL_SHARP_LS037V7DW01) += panel-sharp-ls037v7dw01.o >> +obj-$(CONFIG_DISPLAY_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o >> obj-$(CONFIG_DISPLAY_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o >> obj-$(CONFIG_DISPLAY_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o >> diff --git a/drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c b/drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c >> new file mode 100644 >> index 0000000..b63586e >> --- /dev/null >> +++ b/drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c >> @@ -0,0 +1,537 @@ >> +/* >> + * Toppoly TD028TTEC1 panel support >> + * >> + * Copyright (C) 2008 Nokia Corporation >> + * Author: Tomi Valkeinen <tomi.valkeinen@nokia.com> >> + * >> + * Neo 1973 code (jbt6k74.c): >> + * Copyright (C) 2006-2007 by OpenMoko, Inc. >> + * Author: Harald Welte <laforge@openmoko.org> >> + * >> + * Ported and adapted from Neo 1973 U-Boot by: >> + * H. Nikolaus Schaller <hns@goldelico.com> >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License version 2 as published by >> + * the Free Software Foundation. >> + * >> + * 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, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include <linux/module.h> >> +#include <linux/delay.h> >> +#include <linux/platform_device.h> >> +#include <linux/gpio.h> >> +#include <video/omapdss.h> >> +#include <video/omap-panel-data.h> >> + >> +struct panel_drv_data { >> + struct omap_dss_device dssdev; >> + struct omap_dss_device *in; >> + >> + int data_lines; >> + >> + struct omap_video_timings videomode; >> + >> + int cs_gpio; >> + int scl_gpio; >> + int din_gpio; >> + int dout_gpio; > > Three wires, but four gpios? What am I missing here... We have wired up the 3-wire SPI interface of the display to 4 wires of the McSPI interface because we initially thought that it could work in 4 wire mode as well. The next step we did was to port the driver code from the Openmoko Neo1973 U-Boot which used the gpio-bitbang mechanism. Since it worked and is used only for enabling/disabling the display and for no other purpose we never felt it important to check or modify the code to use SPI. But you are right - we don't use the din_gpio really since we never read registers from the chip. So 3 gpios could be sufficient. Or we must handle the case that din_gpio == dout_gpio in panel_probe to avoid duplicate use of the same gpio. > >> + u_int16_t tx_buf[4]; > > Hmm, what's wrong with "u16"? Nothing. Maybe a relict from taking years old u-boot code and not recognising because the compiler did not complain... > >> +}; >> + >> +static struct omap_video_timings td028ttec1_panel_timings = { >> + .x_res = 480, >> + .y_res = 640, >> + .pixel_clock = 22153, >> + .hfp = 24, >> + .hsw = 8, >> + .hbp = 8, >> + .vfp = 4, >> + .vsw = 2, >> + .vbp = 2, >> + >> + .vsync_level = OMAPDSS_SIG_ACTIVE_LOW, >> + .hsync_level = OMAPDSS_SIG_ACTIVE_LOW, >> + >> + .data_pclk_edge = OMAPDSS_DRIVE_SIG_FALLING_EDGE, >> + .de_level = OMAPDSS_SIG_ACTIVE_HIGH, >> + .sync_pclk_edge = OMAPDSS_DRIVE_SIG_OPPOSITE_EDGES, >> +}; >> + >> +#define JBT_COMMAND 0x000 >> +#define JBT_DATA 0x100 >> + >> +/* 150uS minimum clock cycle, we have two of this plus our other >> + * instructions */ >> + >> +#define SPI_DELAY() udelay(200) > > Would usleep_range work here? Yes, I think that works as well. The 150uS is a minimum and I think it would also work much slower. > > See Documentation/timers/timers-howto.txt > >> +static int jbt_spi_xfer(struct panel_drv_data *data, int wordnum, int bitlen) > > Hmm, if it's not SPI, maybe it shouldn't be called SPI? Yes, we can remove the _spi_ in this name. > >> +{ >> + u_int16_t tmpdout = 0; >> + int i, j; >> + >> + gpio_set_value(data->cs_gpio, 0); >> + >> + for (i = 0; i < wordnum; i++) { >> + tmpdout = data->tx_buf[i]; >> + >> + for (j = 0; j < bitlen; j++) { >> + gpio_set_value(data->scl_gpio, 0); >> + if (tmpdout & (1 << (bitlen-1))) { >> + gpio_set_value(data->dout_gpio, 1); >> + if (gpio_get_value(data->din_gpio) == 0) >> + return 1; >> + } else { >> + gpio_set_value(data->dout_gpio, 0); >> + if (gpio_get_value(data->din_gpio) != 0) >> + return 1; >> + } >> + SPI_DELAY(); >> + gpio_set_value(data->scl_gpio, 1); >> + SPI_DELAY(); >> + tmpdout <<= 1; >> + } >> + } >> + >> + gpio_set_value(data->cs_gpio, 1); >> + >> + return 0; >> +} >> + >> +#define JBT_COMMAND 0x000 >> +#define JBT_DATA 0x100 >> + >> +int jbt_reg_write_nodata(struct panel_drv_data *ddata, u_int8_t reg) > > What's wrong with "u8"? Have I missed a memo? =) > >> +{ >> + int rc; >> + >> + ddata->tx_buf[0] = JBT_COMMAND | reg; >> + >> + rc = jbt_spi_xfer(ddata, 1, 9); >> + if (rc) >> + dev_warn(ddata->dssdev.dev, "Failed to write reg: %x\n", reg); >> + >> + return rc; >> +} >> + >> +int jbt_reg_write(struct panel_drv_data *ddata, u_int8_t reg, u_int8_t data) >> +{ >> + int rc; >> + >> + ddata->tx_buf[0] = JBT_COMMAND | reg; >> + ddata->tx_buf[1] = JBT_DATA | data; >> + >> + rc = jbt_spi_xfer(ddata, 2, 9); >> + if (rc) >> + dev_warn(ddata->dssdev.dev, "Failed to write reg: %x\n", reg); >> + >> + return rc; >> +} >> + >> +int jbt_reg_write16(struct panel_drv_data *ddata, u_int8_t reg, u_int16_t data) >> +{ >> + int rc; >> + >> + ddata->tx_buf[0] = JBT_COMMAND | reg; >> + ddata->tx_buf[1] = JBT_DATA | (data >> 8); >> + ddata->tx_buf[2] = JBT_DATA | (data & 0xff); >> + >> + rc = jbt_spi_xfer(ddata, 3, 9); >> + if (rc) >> + dev_warn(ddata->dssdev.dev, "Failed to write reg: %x\n", reg); >> + >> + return rc; >> +} >> + >> +enum jbt_register { >> + JBT_REG_SLEEP_IN = 0x10, >> + JBT_REG_SLEEP_OUT = 0x11, >> + >> + JBT_REG_DISPLAY_OFF = 0x28, >> + JBT_REG_DISPLAY_ON = 0x29, >> + >> + JBT_REG_RGB_FORMAT = 0x3a, >> + JBT_REG_QUAD_RATE = 0x3b, >> + >> + JBT_REG_POWER_ON_OFF = 0xb0, >> + JBT_REG_BOOSTER_OP = 0xb1, >> + JBT_REG_BOOSTER_MODE = 0xb2, >> + JBT_REG_BOOSTER_FREQ = 0xb3, >> + JBT_REG_OPAMP_SYSCLK = 0xb4, >> + JBT_REG_VSC_VOLTAGE = 0xb5, >> + JBT_REG_VCOM_VOLTAGE = 0xb6, >> + JBT_REG_EXT_DISPL = 0xb7, >> + JBT_REG_OUTPUT_CONTROL = 0xb8, >> + JBT_REG_DCCLK_DCEV = 0xb9, >> + JBT_REG_DISPLAY_MODE1 = 0xba, >> + JBT_REG_DISPLAY_MODE2 = 0xbb, >> + JBT_REG_DISPLAY_MODE = 0xbc, >> + JBT_REG_ASW_SLEW = 0xbd, >> + JBT_REG_DUMMY_DISPLAY = 0xbe, >> + JBT_REG_DRIVE_SYSTEM = 0xbf, >> + >> + JBT_REG_SLEEP_OUT_FR_A = 0xc0, >> + JBT_REG_SLEEP_OUT_FR_B = 0xc1, >> + JBT_REG_SLEEP_OUT_FR_C = 0xc2, >> + JBT_REG_SLEEP_IN_LCCNT_D = 0xc3, >> + JBT_REG_SLEEP_IN_LCCNT_E = 0xc4, >> + JBT_REG_SLEEP_IN_LCCNT_F = 0xc5, >> + JBT_REG_SLEEP_IN_LCCNT_G = 0xc6, >> + >> + JBT_REG_GAMMA1_FINE_1 = 0xc7, >> + JBT_REG_GAMMA1_FINE_2 = 0xc8, >> + JBT_REG_GAMMA1_INCLINATION = 0xc9, >> + JBT_REG_GAMMA1_BLUE_OFFSET = 0xca, >> + >> + JBT_REG_BLANK_CONTROL = 0xcf, >> + JBT_REG_BLANK_TH_TV = 0xd0, >> + JBT_REG_CKV_ON_OFF = 0xd1, >> + JBT_REG_CKV_1_2 = 0xd2, >> + JBT_REG_OEV_TIMING = 0xd3, >> + JBT_REG_ASW_TIMING_1 = 0xd4, >> + JBT_REG_ASW_TIMING_2 = 0xd5, >> + >> + JBT_REG_HCLOCK_VGA = 0xec, >> + JBT_REG_HCLOCK_QVGA = 0xed, >> +}; >> + >> +#define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev) >> + >> +static int td028ttec1_panel_connect(struct omap_dss_device *dssdev) >> +{ >> + struct panel_drv_data *ddata = to_panel_data(dssdev); >> + struct omap_dss_device *in = ddata->in; >> + int r; >> + >> + if (omapdss_device_is_connected(dssdev)) >> + return 0; >> + >> + r = in->ops.dpi->connect(in, dssdev); >> + if (r) >> + return r; >> + >> + return 0; >> +} >> + >> +static void td028ttec1_panel_disconnect(struct omap_dss_device *dssdev) >> +{ >> + struct panel_drv_data *ddata = to_panel_data(dssdev); >> + struct omap_dss_device *in = ddata->in; >> + >> + if (!omapdss_device_is_connected(dssdev)) >> + return; >> + >> + in->ops.dpi->disconnect(in, dssdev); >> +} >> + >> +static int td028ttec1_panel_enable(struct omap_dss_device *dssdev) >> +{ >> + struct panel_drv_data *ddata = to_panel_data(dssdev); >> + struct omap_dss_device *in = ddata->in; >> + int r; >> + >> + if (!omapdss_device_is_connected(dssdev)) >> + return -ENODEV; >> + >> + if (omapdss_device_is_enabled(dssdev)) >> + return 0; >> + >> + in->ops.dpi->set_data_lines(in, ddata->data_lines); >> + in->ops.dpi->set_timings(in, &ddata->videomode); >> + >> + r = in->ops.dpi->enable(in); >> + if (r) >> + return r; >> + >> + dev_dbg(dssdev->dev, "td028ttec1_panel_enable() - state %d\n", >> + dssdev->state); >> + >> + /* three times command zero */ >> + r |= jbt_reg_write_nodata(ddata, 0x00); >> + udelay(1000); > > These are big delays, and I guess the timing is not strict here, so > maybe some sleep variant would be better. Yes, good hint! We will look at it. And add some locks so that enabling and disabling can't interfere. > >> + r |= jbt_reg_write_nodata(ddata, 0x00); >> + udelay(1000); >> + r |= jbt_reg_write_nodata(ddata, 0x00); >> + udelay(1000); >> + >> + /* deep standby out */ >> + r |= jbt_reg_write(ddata, JBT_REG_POWER_ON_OFF, 0x17); >> + >> + /* RGB I/F on, RAM write off, QVGA through, SIGCON enable */ >> + r |= jbt_reg_write(ddata, JBT_REG_DISPLAY_MODE, 0x80); >> + >> + /* Quad mode off */ >> + r |= jbt_reg_write(ddata, JBT_REG_QUAD_RATE, 0x00); >> + >> + /* AVDD on, XVDD on */ >> + r |= jbt_reg_write(ddata, JBT_REG_POWER_ON_OFF, 0x16); >> + >> + /* Output control */ >> + r |= jbt_reg_write16(ddata, JBT_REG_OUTPUT_CONTROL, 0xfff9); >> + >> + /* Sleep mode off */ >> + r |= jbt_reg_write_nodata(ddata, JBT_REG_SLEEP_OUT); >> + >> + /* at this point we have like 50% grey */ >> + >> + /* initialize register set */ >> + r |= jbt_reg_write(ddata, JBT_REG_DISPLAY_MODE1, 0x01); >> + r |= jbt_reg_write(ddata, JBT_REG_DISPLAY_MODE2, 0x00); >> + r |= jbt_reg_write(ddata, JBT_REG_RGB_FORMAT, 0x60); >> + r |= jbt_reg_write(ddata, JBT_REG_DRIVE_SYSTEM, 0x10); >> + r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_OP, 0x56); >> + r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_MODE, 0x33); >> + r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_FREQ, 0x11); >> + r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_FREQ, 0x11); >> + r |= jbt_reg_write(ddata, JBT_REG_OPAMP_SYSCLK, 0x02); >> + r |= jbt_reg_write(ddata, JBT_REG_VSC_VOLTAGE, 0x2b); >> + r |= jbt_reg_write(ddata, JBT_REG_VCOM_VOLTAGE, 0x40); >> + r |= jbt_reg_write(ddata, JBT_REG_EXT_DISPL, 0x03); >> + r |= jbt_reg_write(ddata, JBT_REG_DCCLK_DCEV, 0x04); >> + /* >> + * default of 0x02 in JBT_REG_ASW_SLEW responsible for 72Hz requirement >> + * to avoid red / blue flicker >> + */ >> + r |= jbt_reg_write(ddata, JBT_REG_ASW_SLEW, 0x04); >> + r |= jbt_reg_write(ddata, JBT_REG_DUMMY_DISPLAY, 0x00); >> + >> + r |= jbt_reg_write(ddata, JBT_REG_SLEEP_OUT_FR_A, 0x11); >> + r |= jbt_reg_write(ddata, JBT_REG_SLEEP_OUT_FR_B, 0x11); >> + r |= jbt_reg_write(ddata, JBT_REG_SLEEP_OUT_FR_C, 0x11); >> + r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_D, 0x2040); >> + r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_E, 0x60c0); >> + r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_F, 0x1020); >> + r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_G, 0x60c0); >> + >> + r |= jbt_reg_write16(ddata, JBT_REG_GAMMA1_FINE_1, 0x5533); >> + r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_FINE_2, 0x00); >> + r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_INCLINATION, 0x00); >> + r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_BLUE_OFFSET, 0x00); >> + r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_BLUE_OFFSET, 0x00); >> + >> + r |= jbt_reg_write16(ddata, JBT_REG_HCLOCK_VGA, 0x1f0); >> + r |= jbt_reg_write(ddata, JBT_REG_BLANK_CONTROL, 0x02); >> + r |= jbt_reg_write16(ddata, JBT_REG_BLANK_TH_TV, 0x0804); >> + r |= jbt_reg_write16(ddata, JBT_REG_BLANK_TH_TV, 0x0804); >> + >> + r |= jbt_reg_write(ddata, JBT_REG_CKV_ON_OFF, 0x01); >> + r |= jbt_reg_write16(ddata, JBT_REG_CKV_1_2, 0x0000); >> + >> + r |= jbt_reg_write16(ddata, JBT_REG_OEV_TIMING, 0x0d0e); >> + r |= jbt_reg_write16(ddata, JBT_REG_ASW_TIMING_1, 0x11a4); >> + r |= jbt_reg_write(ddata, JBT_REG_ASW_TIMING_2, 0x0e); >> + >> + r |= jbt_reg_write_nodata(ddata, JBT_REG_DISPLAY_ON); >> + >> + dssdev->state = OMAP_DSS_DISPLAY_ACTIVE; >> + >> + return r; >> +} >> + >> +static void td028ttec1_panel_disable(struct omap_dss_device *dssdev) >> +{ >> + struct panel_drv_data *ddata = to_panel_data(dssdev); >> + struct omap_dss_device *in = ddata->in; >> + >> + if (!omapdss_device_is_enabled(dssdev)) >> + return; >> + >> + dev_dbg(dssdev->dev, "td028ttec1_panel_disable()\n"); >> + >> + in->ops.dpi->disable(in); >> + >> + jbt_reg_write_nodata(ddata, JBT_REG_DISPLAY_OFF); >> + jbt_reg_write16(ddata, JBT_REG_OUTPUT_CONTROL, 0x8002); >> + jbt_reg_write_nodata(ddata, JBT_REG_SLEEP_IN); >> + jbt_reg_write(ddata, JBT_REG_POWER_ON_OFF, 0x00); >> + >> + dssdev->state = OMAP_DSS_DISPLAY_DISABLED; >> +} >> + >> +static void td028ttec1_panel_set_timings(struct omap_dss_device *dssdev, >> + struct omap_video_timings *timings) >> +{ >> + struct panel_drv_data *ddata = to_panel_data(dssdev); >> + struct omap_dss_device *in = ddata->in; >> + >> + ddata->videomode = *timings; >> + dssdev->panel.timings = *timings; >> + >> + in->ops.dpi->set_timings(in, timings); >> +} >> + >> +static void td028ttec1_panel_get_timings(struct omap_dss_device *dssdev, >> + struct omap_video_timings *timings) >> +{ >> + struct panel_drv_data *ddata = to_panel_data(dssdev); >> + >> + *timings = ddata->videomode; >> +} >> + >> +static int td028ttec1_panel_check_timings(struct omap_dss_device *dssdev, >> + struct omap_video_timings *timings) >> +{ >> + struct panel_drv_data *ddata = to_panel_data(dssdev); >> + struct omap_dss_device *in = ddata->in; >> + >> + return in->ops.dpi->check_timings(in, timings); >> +} >> + >> +static struct omap_dss_driver td028ttec1_ops = { >> + .connect = td028ttec1_panel_connect, >> + .disconnect = td028ttec1_panel_disconnect, >> + >> + .enable = td028ttec1_panel_enable, >> + .disable = td028ttec1_panel_disable, >> + >> + .set_timings = td028ttec1_panel_set_timings, >> + .get_timings = td028ttec1_panel_get_timings, >> + .check_timings = td028ttec1_panel_check_timings, >> +}; >> + >> +static int td028ttec1_panel_probe_pdata(struct platform_device *pdev) >> +{ >> + const struct panel_tpo_td028tec1_platform_data *pdata; >> + struct panel_drv_data *ddata = platform_get_drvdata(pdev); >> + struct omap_dss_device *dssdev, *in; >> + >> + pdata = dev_get_platdata(&pdev->dev); >> + >> + in = omap_dss_find_output(pdata->source); >> + if (in == NULL) { >> + dev_err(&pdev->dev, "failed to find video source '%s'\n", >> + pdata->source); >> + return -EPROBE_DEFER; >> + } >> + >> + ddata->in = in; >> + >> + ddata->data_lines = pdata->data_lines; >> + >> + dssdev = &ddata->dssdev; >> + dssdev->name = pdata->name; >> + >> + ddata->cs_gpio = pdata->cs_gpio; >> + ddata->scl_gpio = pdata->scl_gpio; >> + ddata->din_gpio = pdata->din_gpio; >> + ddata->dout_gpio = pdata->dout_gpio; >> + >> + return 0; >> +} >> + >> +static int td028ttec1_panel_probe(struct platform_device *pdev) >> +{ >> + struct panel_drv_data *ddata; >> + struct omap_dss_device *dssdev; >> + int r; >> + >> + ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL); >> + if (ddata == NULL) >> + return -ENOMEM; >> + >> + platform_set_drvdata(pdev, ddata); >> + >> + if (dev_get_platdata(&pdev->dev)) { >> + r = td028ttec1_panel_probe_pdata(pdev); >> + if (r) >> + return r; >> + } else { >> + return -ENODEV; >> + } >> + >> + if (gpio_is_valid(ddata->cs_gpio)) { >> + r = devm_gpio_request_one(&pdev->dev, ddata->cs_gpio, >> + GPIOF_OUT_INIT_HIGH, "lcd cs"); >> + if (r) >> + goto err_gpio; >> + } >> + >> + if (gpio_is_valid(ddata->scl_gpio)) { >> + r = devm_gpio_request_one(&pdev->dev, ddata->scl_gpio, >> + GPIOF_OUT_INIT_HIGH, "lcd scl"); >> + if (r) >> + goto err_gpio; >> + } >> + >> + if (gpio_is_valid(ddata->dout_gpio)) { >> + r = devm_gpio_request_one(&pdev->dev, ddata->dout_gpio, >> + GPIOF_OUT_INIT_LOW, "lcd dout"); >> + if (r) >> + goto err_gpio; >> + } >> + >> + if (gpio_is_valid(ddata->din_gpio)) { >> + r = devm_gpio_request_one(&pdev->dev, ddata->din_gpio, >> + GPIOF_IN, "lcd din"); >> + if (r) >> + goto err_gpio; >> + } >> + >> + /* according to data sheet: wait 50ms (Tpos of LCM). However, 50ms >> + * seems unreliable with later LCM batches, increasing to 90ms */ >> + mdelay(90); > > What is this delay for? Usually sleeps/delays should be between two "HW > actions". Here there's nothing below this line that would count as such > an action. I guess that this delay is intended to power on the display first, then wait some time and after that delay enable the DSS clocks and signals and make the controller chip in the panel initialize. But this of course depends on the order the DSS components are probed and enabled and how power is controlled (we don't interrupt power at all). I think we can move this delay (sleep) to the beginning of td028ttec1_panel_enable() before /* three times command zero */ to make sure that a freshly powered display has enough time to do its internal reset and initializations before registers are programmed. > >> + ddata->videomode = td028ttec1_panel_timings; >> + >> + dssdev = &ddata->dssdev; >> + dssdev->dev = &pdev->dev; >> + dssdev->driver = &td028ttec1_ops; >> + dssdev->type = OMAP_DISPLAY_TYPE_DPI; >> + dssdev->owner = THIS_MODULE; >> + dssdev->panel.timings = ddata->videomode; >> + dssdev->phy.dpi.data_lines = ddata->data_lines; >> + >> + r = omapdss_register_display(dssdev); >> + if (r) { >> + dev_err(&pdev->dev, "Failed to register panel\n"); >> + goto err_reg; >> + } >> + >> + return 0; >> + >> +err_reg: >> +err_gpio: >> + omap_dss_put_device(ddata->in); >> + return r; >> +} >> + > > Tomi BR, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/10/13 12:34, Dr. H. Nikolaus Schaller wrote: > Hi Tomi, > > Am 10.10.2013 um 10:19 schrieb Tomi Valkeinen: > >> Hi, >> >> On 10/10/13 00:08, Marek Belisko wrote: >>> For communicating with driver is used gpio bitbanging because TD028 does >>> not have a standard compliant SPI interface. It is a 3-wire thing with >>> direction reversal. >> >> Isn't that SPI_3WIRE? > > Maybe - but we have no complete datasheet and I don't know an official spec of > a 3-wire SPI protocol to compare how 3-wire should work. Yep, and I know only what I read on wikipedia a few hours ago =). > And I think (but I may be wrong) that SPI_3WIRE is an optional feature of the SoC > specific SPI drivers and in my understanding the OMAP has no such driver: > > http://lxr.free-electrons.com/source/drivers/spi/spi-omap2-mcspi.c#L874 > > And spi-bitbang.c hasn't either. > > So I would prefer to leave this as an area for optimizations for future work. > Maybe we can add some more comments on this. Ok. Well, I guess it's not too bad. But it's really not something that should be in the panel driver (assuming it's "standard" 3-wire SPI). Some SoC could support 3-wire SPI, and then it'd be good to use the SoCs hardware for SPI communication. Having bitbanging hardcoded into the driver will prevent that. >>> + int cs_gpio; >>> + int scl_gpio; >>> + int din_gpio; >>> + int dout_gpio; >> >> Three wires, but four gpios? What am I missing here... > > We have wired up the 3-wire SPI interface of the display > to 4 wires of the McSPI interface because we initially thought > that it could work in 4 wire mode as well. > > The next step we did was to port the driver code from the > Openmoko Neo1973 U-Boot which used the gpio-bitbang > mechanism. > > Since it worked and is used only for enabling/disabling the > display and for no other purpose we never felt it important > to check or modify the code to use SPI. > > But you are right - we don't use the din_gpio really since > we never read registers from the chip. So 3 gpios could be > sufficient. > > Or we must handle the case that din_gpio == dout_gpio > in panel_probe to avoid duplicate use of the same gpio. The panel hardware has three wires, so the panel driver (if it does handle the bus by bitbanging) can only refer to three gpios. So either the bus details should be hidden by using the normal spi framework, or if the driver does the bitbanging, use the gpios as specified in the panel spec. The panel driver cannot contain any board specific stuff. >>> +static int jbt_spi_xfer(struct panel_drv_data *data, int wordnum, int bitlen) >> >> Hmm, if it's not SPI, maybe it shouldn't be called SPI? > > Yes, we can remove the _spi_ in this name. Or maybe use "spi3w" or such, just to make it clear it's not plain standard spi. Again, presuming it's really 3-wire spi =). >>> +static int td028ttec1_panel_probe(struct platform_device *pdev) >>> +{ >>> + struct panel_drv_data *ddata; >>> + struct omap_dss_device *dssdev; >>> + int r; >>> + >>> + ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL); >>> + if (ddata == NULL) >>> + return -ENOMEM; >>> + >>> + platform_set_drvdata(pdev, ddata); >>> + >>> + if (dev_get_platdata(&pdev->dev)) { >>> + r = td028ttec1_panel_probe_pdata(pdev); >>> + if (r) >>> + return r; >>> + } else { >>> + return -ENODEV; >>> + } >>> + >>> + if (gpio_is_valid(ddata->cs_gpio)) { >>> + r = devm_gpio_request_one(&pdev->dev, ddata->cs_gpio, >>> + GPIOF_OUT_INIT_HIGH, "lcd cs"); >>> + if (r) >>> + goto err_gpio; >>> + } >>> + >>> + if (gpio_is_valid(ddata->scl_gpio)) { >>> + r = devm_gpio_request_one(&pdev->dev, ddata->scl_gpio, >>> + GPIOF_OUT_INIT_HIGH, "lcd scl"); >>> + if (r) >>> + goto err_gpio; >>> + } >>> + >>> + if (gpio_is_valid(ddata->dout_gpio)) { >>> + r = devm_gpio_request_one(&pdev->dev, ddata->dout_gpio, >>> + GPIOF_OUT_INIT_LOW, "lcd dout"); >>> + if (r) >>> + goto err_gpio; >>> + } >>> + >>> + if (gpio_is_valid(ddata->din_gpio)) { >>> + r = devm_gpio_request_one(&pdev->dev, ddata->din_gpio, >>> + GPIOF_IN, "lcd din"); >>> + if (r) >>> + goto err_gpio; >>> + } >>> + >>> + /* according to data sheet: wait 50ms (Tpos of LCM). However, 50ms >>> + * seems unreliable with later LCM batches, increasing to 90ms */ >>> + mdelay(90); >> >> What is this delay for? Usually sleeps/delays should be between two "HW >> actions". Here there's nothing below this line that would count as such >> an action. > > I guess that this delay is intended to power on the display first, then wait some I'm not very comfortable with merging a driver, when the driver author guesses what parts of the driver do =). > time and after that delay enable the DSS clocks and signals and make the > controller chip in the panel initialize. I don't see the code before the delay doing any power up, it just sets the data bus gpios to certain state. Do those gpio initializations power up the panel? > But this of course depends on the order the DSS components are probed > and enabled and how power is controlled (we don't interrupt power at all). > > I think we can move this delay (sleep) to the beginning of > > td028ttec1_panel_enable() before /* three times command zero */ > > to make sure that a freshly powered display has enough time to do its > internal reset and initializations before registers are programmed. The probe function should not enable the panel (well, it can enable the panel, but only to read an ID or such and then disable it before exiting probe). So after probe, the panel should be as dead as possible, power disabled etc. All the powering up should happen in enable(). Tomi
Hi Tomi, Am 10.10.2013 um 13:10 schrieb Tomi Valkeinen: > On 10/10/13 12:34, Dr. H. Nikolaus Schaller wrote: >> Hi Tomi, >> >> Am 10.10.2013 um 10:19 schrieb Tomi Valkeinen: >> >>> Hi, >>> >>> On 10/10/13 00:08, Marek Belisko wrote: >>>> For communicating with driver is used gpio bitbanging because TD028 does >>>> not have a standard compliant SPI interface. It is a 3-wire thing with >>>> direction reversal. >>> >>> Isn't that SPI_3WIRE? >> >> Maybe - but we have no complete datasheet and I don't know an official spec of >> a 3-wire SPI protocol to compare how 3-wire should work. > > Yep, and I know only what I read on wikipedia a few hours ago =). > >> And I think (but I may be wrong) that SPI_3WIRE is an optional feature of the SoC >> specific SPI drivers and in my understanding the OMAP has no such driver: >> >> http://lxr.free-electrons.com/source/drivers/spi/spi-omap2-mcspi.c#L874 >> >> And spi-bitbang.c hasn't either. >> >> So I would prefer to leave this as an area for optimizations for future work. >> Maybe we can add some more comments on this. > > Ok. Well, I guess it's not too bad. But it's really not something that > should be in the panel driver (assuming it's "standard" 3-wire SPI). > Some SoC could support 3-wire SPI, and then it'd be good to use the SoCs > hardware for SPI communication. Having bitbanging hardcoded into the > driver will prevent that. Yes, I agree and I am willing to help if someone comes up with such a SoC. At the moment we have connected it to the OMAP3 only. I.e. I want not to do a lot of work for others who we just guess about that they might exist... > >>>> + int cs_gpio; >>>> + int scl_gpio; >>>> + int din_gpio; >>>> + int dout_gpio; >>> >>> Three wires, but four gpios? What am I missing here... >> >> We have wired up the 3-wire SPI interface of the display >> to 4 wires of the McSPI interface because we initially thought >> that it could work in 4 wire mode as well. >> >> The next step we did was to port the driver code from the >> Openmoko Neo1973 U-Boot which used the gpio-bitbang >> mechanism. >> >> Since it worked and is used only for enabling/disabling the >> display and for no other purpose we never felt it important >> to check or modify the code to use SPI. >> >> But you are right - we don't use the din_gpio really since >> we never read registers from the chip. So 3 gpios could be >> sufficient. >> >> Or we must handle the case that din_gpio == dout_gpio >> in panel_probe to avoid duplicate use of the same gpio. > > The panel hardware has three wires, so the panel driver (if it does > handle the bus by bitbanging) can only refer to three gpios. Hm. The panle hardware has 3 but the SoC (OMAP3) the driver is running on has 4. > So either > the bus details should be hidden by using the normal spi framework, or > if the driver does the bitbanging, use the gpios as specified in the > panel spec. The panel driver cannot contain any board specific stuff. The bitbang driver shown below can handle either 3 or 4 gpios (except for initialization). > >>>> +static int jbt_spi_xfer(struct panel_drv_data *data, int wordnum, int bitlen) >>> >>> Hmm, if it's not SPI, maybe it shouldn't be called SPI? >> >> Yes, we can remove the _spi_ in this name. > > Or maybe use "spi3w" or such, just to make it clear it's not plain > standard spi. Again, presuming it's really 3-wire spi =). > >>>> +static int td028ttec1_panel_probe(struct platform_device *pdev) >>>> +{ >>>> + struct panel_drv_data *ddata; >>>> + struct omap_dss_device *dssdev; >>>> + int r; >>>> + >>>> + ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL); >>>> + if (ddata == NULL) >>>> + return -ENOMEM; >>>> + >>>> + platform_set_drvdata(pdev, ddata); >>>> + >>>> + if (dev_get_platdata(&pdev->dev)) { >>>> + r = td028ttec1_panel_probe_pdata(pdev); >>>> + if (r) >>>> + return r; >>>> + } else { >>>> + return -ENODEV; >>>> + } >>>> + >>>> + if (gpio_is_valid(ddata->cs_gpio)) { >>>> + r = devm_gpio_request_one(&pdev->dev, ddata->cs_gpio, >>>> + GPIOF_OUT_INIT_HIGH, "lcd cs"); >>>> + if (r) >>>> + goto err_gpio; >>>> + } >>>> + >>>> + if (gpio_is_valid(ddata->scl_gpio)) { >>>> + r = devm_gpio_request_one(&pdev->dev, ddata->scl_gpio, >>>> + GPIOF_OUT_INIT_HIGH, "lcd scl"); >>>> + if (r) >>>> + goto err_gpio; >>>> + } >>>> + >>>> + if (gpio_is_valid(ddata->dout_gpio)) { >>>> + r = devm_gpio_request_one(&pdev->dev, ddata->dout_gpio, >>>> + GPIOF_OUT_INIT_LOW, "lcd dout"); >>>> + if (r) >>>> + goto err_gpio; >>>> + } >>>> + >>>> + if (gpio_is_valid(ddata->din_gpio)) { >>>> + r = devm_gpio_request_one(&pdev->dev, ddata->din_gpio, >>>> + GPIOF_IN, "lcd din"); >>>> + if (r) >>>> + goto err_gpio; >>>> + } >>>> + >>>> + /* according to data sheet: wait 50ms (Tpos of LCM). However, 50ms >>>> + * seems unreliable with later LCM batches, increasing to 90ms */ >>>> + mdelay(90); >>> >>> What is this delay for? Usually sleeps/delays should be between two "HW >>> actions". Here there's nothing below this line that would count as such >>> an action. >> >> I guess that this delay is intended to power on the display first, then wait some > > I'm not very comfortable with merging a driver, when the driver author > guesses what parts of the driver do =). Well, that is the problem with some badly documented chips. We have "inherited" that from the Openmoko project and I think those guys did have access to some more precise data sheets but we don't. You can't even find google results for this strange jbt chip. It works on all our GTA04 devices for quite a long time (using the old display API). We just recently upgraded to display-new. > >> time and after that delay enable the DSS clocks and signals and make the >> controller chip in the panel initialize. > > I don't see the code before the delay doing any power up, it just sets > the data bus gpios to certain state. Do those gpio initializations power > up the panel? No, and they don't need to. The enable_power code is not implemented because we have no dedicated regulator (but one shared and enabled early in the kernel). It should be quite straightforward to add it to enable/disable if we need it. We may want to change this in the next hardware revision, but on the other hand do not want to postpone upstreaming until we have that, because an upstreamed driver will help GTA04 users of today. > >> But this of course depends on the order the DSS components are probed >> and enabled and how power is controlled (we don't interrupt power at all). >> >> I think we can move this delay (sleep) to the beginning of >> >> td028ttec1_panel_enable() before /* three times command zero */ >> >> to make sure that a freshly powered display has enough time to do its >> internal reset and initializations before registers are programmed. > > The probe function should not enable the panel (well, it can enable the > panel, but only to read an ID or such and then disable it before exiting > probe). > > So after probe, the panel should be as dead as possible, power disabled > etc. All the powering up should happen in enable(). Yes, I agree. And threrefore the safety-delay should indeed be in enable(). BR, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/10/13 14:52, Dr. H. Nikolaus Schaller wrote: > Yes, I agree and I am willing to help if someone comes up with such a SoC. > At the moment we have connected it to the OMAP3 only. True, but even without that kind of SoC, SPI bitbanging should be handled by an SPI driver, not by the drivers that use the bus. > I.e. I want not to do a lot of work for others who we just guess about that they > might exist... Yep. It's fine for me, it's not that much extra code in the panel driver. >> The panel hardware has three wires, so the panel driver (if it does >> handle the bus by bitbanging) can only refer to three gpios. > > Hm. The panle hardware has 3 but the SoC (OMAP3) the driver > is running on has 4. Right, but this panel driver is a driver for the panel hardware. Not for the SoC, or the SoC+panel combination. So the panel driver must only use resources as seen by the panel hardware. >> So either >> the bus details should be hidden by using the normal spi framework, or >> if the driver does the bitbanging, use the gpios as specified in the >> panel spec. The panel driver cannot contain any board specific stuff. > > The bitbang driver shown below can handle either 3 or 4 gpios (except > for initialization). It's not a bitbang driver, it's a panel driver. And anyway, if I understood right, your use of 4 gpios was just a hack to try to make it look like a normal 4-wire SPI bus. What you really have is 3 wires, 3 gpios. I don't see any reason to use 4 gpios, as two of them are the same. Hmm, how does it work anyway. Did I understand it right, the panel's 'DIN' pin is connected to two gpios on the SoC, and one of those gpios is set as output, and the other as input? So the SoC is always pulling that line up or down, and the panel is also pulling it up or down when it's sending data. I'm no HW guy but that sounds quite bad =). I've never written or studied a bitbanging driver, but shouldn't there be just one gpio used on the SoC for DIN, and it would be set to either output or input mode, depending on if we are reading or writing? Tomi
On 10/10/2013 02:13 PM, Tomi Valkeinen wrote: > On 10/10/13 14:52, Dr. H. Nikolaus Schaller wrote: > >> Yes, I agree and I am willing to help if someone comes up with such a SoC. >> At the moment we have connected it to the OMAP3 only. > > True, but even without that kind of SoC, SPI bitbanging should be > handled by an SPI driver, not by the drivers that use the bus. > >> I.e. I want not to do a lot of work for others who we just guess about that they >> might exist... > > Yep. It's fine for me, it's not that much extra code in the panel driver. > >>> The panel hardware has three wires, so the panel driver (if it does >>> handle the bus by bitbanging) can only refer to three gpios. >> >> Hm. The panle hardware has 3 but the SoC (OMAP3) the driver >> is running on has 4. > > Right, but this panel driver is a driver for the panel hardware. Not for > the SoC, or the SoC+panel combination. So the panel driver must only use > resources as seen by the panel hardware. > >>> So either >>> the bus details should be hidden by using the normal spi framework, or >>> if the driver does the bitbanging, use the gpios as specified in the >>> panel spec. The panel driver cannot contain any board specific stuff. >> >> The bitbang driver shown below can handle either 3 or 4 gpios (except >> for initialization). > > It's not a bitbang driver, it's a panel driver. And anyway, if I > understood right, your use of 4 gpios was just a hack to try to make it > look like a normal 4-wire SPI bus. What you really have is 3 wires, 3 > gpios. I don't see any reason to use 4 gpios, as two of them are the same. > > Hmm, how does it work anyway. Did I understand it right, the panel's > 'DIN' pin is connected to two gpios on the SoC, and one of those gpios > is set as output, and the other as input? So the SoC is always pulling > that line up or down, and the panel is also pulling it up or down when > it's sending data. I'm no HW guy but that sounds quite bad =). > > I've never written or studied a bitbanging driver, but shouldn't there > be just one gpio used on the SoC for DIN, and it would be set to either > output or input mode, depending on if we are reading or writing? Back in the OpenMoko days we used the panel in normal 4-wire SPI mode with the GPIO bitbang SPI master. The bitbang code in this driver also looks like normal 4 wire. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 10.10.2013 um 14:26 schrieb Lars-Peter Clausen: > On 10/10/2013 02:13 PM, Tomi Valkeinen wrote: >> On 10/10/13 14:52, Dr. H. Nikolaus Schaller wrote: >> >>> Yes, I agree and I am willing to help if someone comes up with such a SoC. >>> At the moment we have connected it to the OMAP3 only. >> >> True, but even without that kind of SoC, SPI bitbanging should be >> handled by an SPI driver, not by the drivers that use the bus. >> >>> I.e. I want not to do a lot of work for others who we just guess about that they >>> might exist... >> >> Yep. It's fine for me, it's not that much extra code in the panel driver. >> >>>> The panel hardware has three wires, so the panel driver (if it does >>>> handle the bus by bitbanging) can only refer to three gpios. >>> >>> Hm. The panle hardware has 3 but the SoC (OMAP3) the driver >>> is running on has 4. >> >> Right, but this panel driver is a driver for the panel hardware. Not for >> the SoC, or the SoC+panel combination. So the panel driver must only use >> resources as seen by the panel hardware. >> >>>> So either >>>> the bus details should be hidden by using the normal spi framework, or >>>> if the driver does the bitbanging, use the gpios as specified in the >>>> panel spec. The panel driver cannot contain any board specific stuff. >>> >>> The bitbang driver shown below can handle either 3 or 4 gpios (except >>> for initialization). >> >> It's not a bitbang driver, it's a panel driver. And anyway, if I >> understood right, your use of 4 gpios was just a hack to try to make it >> look like a normal 4-wire SPI bus. What you really have is 3 wires, 3 >> gpios. I don't see any reason to use 4 gpios, as two of them are the same. There is a protection resistor in the one defined as "output" so that protocol errors don't have two outputs work against each other. >> >> Hmm, how does it work anyway. Did I understand it right, the panel's >> 'DIN' pin is connected to two gpios on the SoC, and one of those gpios >> is set as output, and the other as input? Yes. >> So the SoC is always pulling >> that line up or down, and the panel is also pulling it up or down when >> it's sending data. I'm no HW guy but that sounds quite bad =). Yes, that is the strange thing with this protocol. It does a direction reversal after some time. I.e. the panel switches its input to output and the SoC has to be fast enough to do the same. Therefore, we have one output GPIO (protected by a resistor) and a separate input GPIO. Sometimes interfacing hardware is indeed strange. >> >> I've never written or studied a bitbanging driver, but shouldn't there >> be just one gpio used on the SoC for DIN, and it would be set to either >> output or input mode, depending on if we are reading or writing? > > Back in the OpenMoko days we used the panel in normal 4-wire SPI mode with > the GPIO bitbang SPI master. The bitbang code in this driver also looks like > normal 4 wire. Thanks for this hint! Maybe using a standard 4-wire SPI simply works if we only write data and make sure the panel is not sending anything... I still hesitate to break working code because it needs quite some time to debug and we don't even know if it ever works, i.e. sends us to swampy ground... BR, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/10/2013 03:42 PM, Dr. H. Nikolaus Schaller wrote: > > Am 10.10.2013 um 14:26 schrieb Lars-Peter Clausen: > >> On 10/10/2013 02:13 PM, Tomi Valkeinen wrote: >>> On 10/10/13 14:52, Dr. H. Nikolaus Schaller wrote: >>> >>>> Yes, I agree and I am willing to help if someone comes up with such a SoC. >>>> At the moment we have connected it to the OMAP3 only. >>> >>> True, but even without that kind of SoC, SPI bitbanging should be >>> handled by an SPI driver, not by the drivers that use the bus. >>> >>>> I.e. I want not to do a lot of work for others who we just guess about that they >>>> might exist... >>> >>> Yep. It's fine for me, it's not that much extra code in the panel driver. >>> >>>>> The panel hardware has three wires, so the panel driver (if it does >>>>> handle the bus by bitbanging) can only refer to three gpios. >>>> >>>> Hm. The panle hardware has 3 but the SoC (OMAP3) the driver >>>> is running on has 4. >>> >>> Right, but this panel driver is a driver for the panel hardware. Not for >>> the SoC, or the SoC+panel combination. So the panel driver must only use >>> resources as seen by the panel hardware. >>> >>>>> So either >>>>> the bus details should be hidden by using the normal spi framework, or >>>>> if the driver does the bitbanging, use the gpios as specified in the >>>>> panel spec. The panel driver cannot contain any board specific stuff. >>>> >>>> The bitbang driver shown below can handle either 3 or 4 gpios (except >>>> for initialization). >>> >>> It's not a bitbang driver, it's a panel driver. And anyway, if I >>> understood right, your use of 4 gpios was just a hack to try to make it >>> look like a normal 4-wire SPI bus. What you really have is 3 wires, 3 >>> gpios. I don't see any reason to use 4 gpios, as two of them are the same. > > There is a protection resistor in the one defined as "output" so that > protocol errors don't have two outputs work against each other. > >>> >>> Hmm, how does it work anyway. Did I understand it right, the panel's >>> 'DIN' pin is connected to two gpios on the SoC, and one of those gpios >>> is set as output, and the other as input? > > Yes. > >>> So the SoC is always pulling >>> that line up or down, and the panel is also pulling it up or down when >>> it's sending data. I'm no HW guy but that sounds quite bad =). > > Yes, that is the strange thing with this protocol. It does a direction reversal > after some time. I.e. the panel switches its input to output and the SoC has > to be fast enough to do the same. Therefore, we have one output GPIO > (protected by a resistor) and a separate input GPIO. > > Sometimes interfacing hardware is indeed strange. > >>> >>> I've never written or studied a bitbanging driver, but shouldn't there >>> be just one gpio used on the SoC for DIN, and it would be set to either >>> output or input mode, depending on if we are reading or writing? >> >> Back in the OpenMoko days we used the panel in normal 4-wire SPI mode with >> the GPIO bitbang SPI master. The bitbang code in this driver also looks like >> normal 4 wire. > > > Thanks for this hint! > > Maybe using a standard 4-wire SPI simply works if we only write data and > make sure the panel is not sending anything... According to the datasheet the the panel as a dedicated dout pin. Maybe you did not connect it in your design, which means you won't be able to read any data from the panel at all. Also your custom bitbang code looks a bit strange: gpio_set_value(data->dout_gpio, 1); if (gpio_get_value(data->din_gpio) == 0) return 1; You set the state on the dout pin and then read the din pin, if both do not match you abort with an error. This suggest that, for whatever reason, you feed the dout pin back into the din pin in your design. Btw. this is also the only place where din is used in your driver. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/10/13 21:58, Lars-Peter Clausen wrote: > According to the datasheet the the panel as a dedicated dout pin. Maybe > you did not connect it in your design, which means you won't be able to > read any data from the panel at all. I don't see a dedicated dout in the datasheet... http://dl.wolfgang-draxinger.net/openmoko/doc/TD028TTEC1.pdf > Also your custom bitbang code looks a bit strange: > > gpio_set_value(data->dout_gpio, 1); > if (gpio_get_value(data->din_gpio) == 0) > return 1; > > You set the state on the dout pin and then read the din pin, if both do > not match you abort with an error. This suggest that, for whatever > reason, you feed the dout pin back into the din pin in your design. Btw. > this is also the only place where din is used in your driver. Yes, he said the single "Serial interface data input/output" pin is connected to both din and dout on the SoC. I guess the purpose of that gpio_get_value() is to ensure that the panel is not pulling the line when the SoC is writing to it. Not that I really understand how that can work, but I'm not a HW guy =). Tomi
On 10/11/2013 06:41 AM, Tomi Valkeinen wrote: > On 10/10/13 21:58, Lars-Peter Clausen wrote: > >> According to the datasheet the the panel as a dedicated dout pin. Maybe >> you did not connect it in your design, which means you won't be able to >> read any data from the panel at all. > > I don't see a dedicated dout in the datasheet... > http://dl.wolfgang-draxinger.net/openmoko/doc/TD028TTEC1.pdf Hm, ok, looks like the display controller used in the panel (the jbt6k74) has separate DIN and DOUT, but the panel only has one pin. But the datasheet for the panel is not exactly clear on whether it's DIN pin is both the DIN and DOUT pin from the controller or, just DIN and DOUT not being connected at all. > >> Also your custom bitbang code looks a bit strange: >> >> gpio_set_value(data->dout_gpio, 1); >> if (gpio_get_value(data->din_gpio) == 0) >> return 1; >> >> You set the state on the dout pin and then read the din pin, if both do >> not match you abort with an error. This suggest that, for whatever >> reason, you feed the dout pin back into the din pin in your design. Btw. >> this is also the only place where din is used in your driver. > > Yes, he said the single "Serial interface data input/output" pin is > connected to both din and dout on the SoC. I guess the purpose of that > gpio_get_value() is to ensure that the panel is not pulling the line > when the SoC is writing to it. Not that I really understand how that can > work, but I'm not a HW guy =). Hm, ok. I don't think the panel will ever actively drive the line. So in my opinion using either the McBSP SPI master or the GPIO bitbang SPI master should work just fine. You just wouldn't be able to read any register from the device. But the driver is not attempting to do this, so it should be fine. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi all, Am 11.10.2013 um 06:41 schrieb Tomi Valkeinen: > On 10/10/13 21:58, Lars-Peter Clausen wrote: > >> According to the datasheet the the panel as a dedicated dout pin. Maybe >> you did not connect it in your design, which means you won't be able to >> read any data from the panel at all. > > I don't see a dedicated dout in the datasheet... > http://dl.wolfgang-draxinger.net/openmoko/doc/TD028TTEC1.pdf > >> Also your custom bitbang code looks a bit strange: >> >> gpio_set_value(data->dout_gpio, 1); >> if (gpio_get_value(data->din_gpio) == 0) >> return 1; >> >> You set the state on the dout pin and then read the din pin, if both do >> not match you abort with an error. This suggest that, for whatever >> reason, Detecting hardware failure. If the panel is not connected or broken (short circuit) reading back may fail and this can help to detect a hardware failure. >> you feed the dout pin back into the din pin in your design. Btw. >> this is also the only place where din is used in your driver. Ok, I see the issue with that. It assumes this type of feedback which does not necessarily exist. I.e. if someone else doesn't have this feedback line the driver will not work. So a more general solution should work with any setup, even if no din gpio has been defined. I.e. do this test only if the din_gpio exists. The main reason that we don't use din_gpio elsewhere is that we have no jbt_read() function because we lack information what we can read from the panel controller chip - and have not seen a use-case for that. And thanks to your hint, I think either returning 1 is wrong (should be some -EIO or similar). Or the return r; at the end of td028ttec1_panel_enable is wrong (should be return rc ? -EIO : 0; ). We will check that. > Yes, he said the single "Serial interface data input/output" pin is > connected to both din and dout on the SoC. I have found two public links which may describe what we do: The panel data sheet [1], Section 9 describes the interface as "3 wire serial interface". It sometimes calls the data lines DIN and DOUT and describes them separately, i.e. like a 4 wire interface. So I guess the jbt6k chip has indeed 4 wires, but (as you said), din/dout are lines are tied together on the panel side to save (share) one wire in the flex cable. [2] shows in figure 2 an example how to connect a 3-wire interface to a 4-wire SPI SoC. This is the way we have done it. > I guess the purpose of that > gpio_get_value() is to ensure that the panel is not pulling the line > when the SoC is writing to it. Not that I really understand how that can > work, but I'm not a HW guy =). Finally, I have checked one detail which rules out to use a standard SPI driver for OMAP on our board. The reason is that we have a McBSP port of the OMAP3 and not a McSPI. So we have to run this "3 wire serial interface protocol" by bitbanging. IMHO, the most flexible way to hook up the panel (and driver) to arbitrary hardware is by using GPIOs and not having the panel driver restrict to a SPI interface. And having the bitbanging encapsulated in the driver itself (instead on relying on some spi-bitbang) makes it less dependent on changes somewhere else. I.e. the driver module is indeed a module. Marek is preparing an updated patch for further discussion. BR, Nikolaus [1]: http://dl.wolfgang-draxinger.net/openmoko/doc/TD028TTEC1.pdf [2]: http://www.totalphase.com/support/kb/10059/ -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Lars-Peter, ah, I didn't see your mail while writing mine - so some overlap. Am 11.10.2013 um 09:08 schrieb Lars-Peter Clausen: > On 10/11/2013 06:41 AM, Tomi Valkeinen wrote: >> On 10/10/13 21:58, Lars-Peter Clausen wrote: >> >>> According to the datasheet the the panel as a dedicated dout pin. Maybe >>> you did not connect it in your design, which means you won't be able to >>> read any data from the panel at all. >> >> I don't see a dedicated dout in the datasheet... >> http://dl.wolfgang-draxinger.net/openmoko/doc/TD028TTEC1.pdf > > Hm, ok, looks like the display controller used in the panel (the jbt6k74) has separate DIN and DOUT, but the panel only has one pin. Yes > But the datasheet for the panel is not exactly clear Yes > on whether it's DIN pin is both the DIN and DOUT pin from the controller or, just DIN and DOUT not being connected at all. I think they have a 4-wire controller in the panel and tied the lines together. This works if they properly control the data direction. There are some read commands where this direction reversal happens. > >> >>> Also your custom bitbang code looks a bit strange: >>> >>> gpio_set_value(data->dout_gpio, 1); >>> if (gpio_get_value(data->din_gpio) == 0) >>> return 1; >>> >>> You set the state on the dout pin and then read the din pin, if both do >>> not match you abort with an error. This suggest that, for whatever >>> reason, you feed the dout pin back into the din pin in your design. Btw. >>> this is also the only place where din is used in your driver. >> >> Yes, he said the single "Serial interface data input/output" pin is >> connected to both din and dout on the SoC. I guess the purpose of that >> gpio_get_value() is to ensure that the panel is not pulling the line >> when the SoC is writing to it. Not that I really understand how that can >> work, but I'm not a HW guy =). > > Hm, ok. I don't think the panel will ever actively drive the line. Well it will drive it if we issue some read command (but I have no information about these commands). > So in my opinion using either the McBSP SPI master or the GPIO bitbang SPI master should work just fine. You just wouldn't be able to read any register from the device. But the driver is not attempting to do this, so it should be fine. I am not sure if there is a SPI driver for a McBSP port [1]? And to make that work (reliably) and tested it might need a lot of work for us. At least I think such a change (e.g. setting up clock polarity etc.) is not done in some minutes. And the only feedback we have from the panel is "does not work"/"works". I.e. if we are not lucky that it works immediately we have no real means to debug. IMHO it also gives more flexibility to board designers to choose GPIOs instead of enforcing some SPI interface by the driver (and encapsulate this arguable protocol in the driver). Maybe some board has 3 spare GPIOs but neither McBSPs nor McSPIs available. BR, Nikolaus http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/053064.html -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/10/13 10:42, Dr. H. Nikolaus Schaller wrote: > I am not sure if there is a SPI driver for a McBSP port [1]? And to make that > work (reliably) and tested it might need a lot of work for us. At least I think > such a change (e.g. setting up clock polarity etc.) is not done in some minutes. > And the only feedback we have from the panel is "does not work"/"works". I.e. > if we are not lucky that it works immediately we have no real means to debug. > > IMHO it also gives more flexibility to board designers to choose GPIOs instead > of enforcing some SPI interface by the driver (and encapsulate this arguable > protocol in the driver). Maybe some board has 3 spare GPIOs but neither > McBSPs nor McSPIs available. This has been an interesting thread, I've learnt a lot =). I still think the panel driver should not handle this, but there should be a separate spi bitbang driver for it. I understand you're not enthusiastic going that way, as the current version works for you. However, when using DT, we need to think how to represent the hardware in the device tree data, and it has to be right from the beginning. That's why I won't allow representing this panel as having 4 gpios in the DT data, because that is not correct. The panel has 3 pins. But then, the panel does allow reading, which could be implemented using 4 gpios as you have done. This data should be in the spi-bitbang data, and the panel should just use the standard SPI framework. Using SPI framework does not mean you should use McBSP or McSPI. It's up to you how the 3-wire SPI is implemented on the SoC side, the panel would just work in all the cases. Tomi
Hi Tomi, On Fri, Oct 11, 2013 at 10:17 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > On 11/10/13 10:42, Dr. H. Nikolaus Schaller wrote: > >> I am not sure if there is a SPI driver for a McBSP port [1]? And to make that >> work (reliably) and tested it might need a lot of work for us. At least I think >> such a change (e.g. setting up clock polarity etc.) is not done in some minutes. >> And the only feedback we have from the panel is "does not work"/"works". I.e. >> if we are not lucky that it works immediately we have no real means to debug. >> >> IMHO it also gives more flexibility to board designers to choose GPIOs instead >> of enforcing some SPI interface by the driver (and encapsulate this arguable >> protocol in the driver). Maybe some board has 3 spare GPIOs but neither >> McBSPs nor McSPIs available. > > This has been an interesting thread, I've learnt a lot =). > > I still think the panel driver should not handle this, but there should > be a separate spi bitbang driver for it. > > I understand you're not enthusiastic going that way, as the current > version works for you. However, when using DT, we need to think how to > represent the hardware in the device tree data, and it has to be right > from the beginning. > > That's why I won't allow representing this panel as having 4 gpios in > the DT data, because that is not correct. The panel has 3 pins. But > then, the panel does allow reading, which could be implemented using 4 > gpios as you have done. This data should be in the spi-bitbang data, and > the panel should just use the standard SPI framework. I disagree. There are different drivers which pass in platform data gpios (encoder-tfp410.c or encoder-tpd12s015.c) and those must be covered by DT then. I cannot see problem why to have for td028 panel 3 or 4 gpios defined in DT. > > Using SPI framework does not mean you should use McBSP or McSPI. It's up > to you how the 3-wire SPI is implemented on the SoC side, the panel > would just work in all the cases. > > Tomi > > BR, marek
On 11/10/13 11:59, Belisko Marek wrote: >> That's why I won't allow representing this panel as having 4 gpios in >> the DT data, because that is not correct. The panel has 3 pins. But >> then, the panel does allow reading, which could be implemented using 4 >> gpios as you have done. This data should be in the spi-bitbang data, and >> the panel should just use the standard SPI framework. > I disagree. There are different drivers which pass in platform data > gpios (encoder-tfp410.c or encoder-tpd12s015.c) > and those must be covered by DT then. I cannot see problem why to have > for td028 panel 3 or 4 gpios defined in DT. Yes, they are plain GPIOs, and defined in the spec for the respective chip. There's no alternative to how they could be represented. Here, with the td028, the spec speaks of 3 pins for the serial bus. Not 4. So there cannot be 4 gpios defined in the panel's data, that just doesn't make sense. Additionally, what we have here are not "normal" gpios, but pins for a serial bus, 3-wire SPI. If the panel data specifies the gpios, then it's not possible to use real SPI hardware with the panel. Tomi
On 10/11/2013 10:59 AM, Belisko Marek wrote: > Hi Tomi, > > On Fri, Oct 11, 2013 at 10:17 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: >> On 11/10/13 10:42, Dr. H. Nikolaus Schaller wrote: >> >>> I am not sure if there is a SPI driver for a McBSP port [1]? And to make that >>> work (reliably) and tested it might need a lot of work for us. At least I think >>> such a change (e.g. setting up clock polarity etc.) is not done in some minutes. >>> And the only feedback we have from the panel is "does not work"/"works". I.e. >>> if we are not lucky that it works immediately we have no real means to debug. >>> >>> IMHO it also gives more flexibility to board designers to choose GPIOs instead >>> of enforcing some SPI interface by the driver (and encapsulate this arguable >>> protocol in the driver). Maybe some board has 3 spare GPIOs but neither >>> McBSPs nor McSPIs available. >> >> This has been an interesting thread, I've learnt a lot =). >> >> I still think the panel driver should not handle this, but there should >> be a separate spi bitbang driver for it. >> >> I understand you're not enthusiastic going that way, as the current >> version works for you. However, when using DT, we need to think how to >> represent the hardware in the device tree data, and it has to be right >> from the beginning. >> >> That's why I won't allow representing this panel as having 4 gpios in >> the DT data, because that is not correct. The panel has 3 pins. But >> then, the panel does allow reading, which could be implemented using 4 >> gpios as you have done. This data should be in the spi-bitbang data, and >> the panel should just use the standard SPI framework. > I disagree. There are different drivers which pass in platform data > gpios (encoder-tfp410.c or encoder-tpd12s015.c) > and those must be covered by DT then. I cannot see problem why to have > for td028 panel 3 or 4 gpios defined in DT. The problem is not representing it in the devicetree, but representing it correctly. This is a SPI slave device, hence it should be presented in the devicetree as a SPI slave device and not as a platform device with 4 GPIOs. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi all, Am 11.10.2013 um 11:06 schrieb Lars-Peter Clausen: > On 10/11/2013 10:59 AM, Belisko Marek wrote: >> Hi Tomi, >> >> On Fri, Oct 11, 2013 at 10:17 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: >>> On 11/10/13 10:42, Dr. H. Nikolaus Schaller wrote: >>> >>>> I am not sure if there is a SPI driver for a McBSP port [1]? And to make that >>>> work (reliably) and tested it might need a lot of work for us. At least I think >>>> such a change (e.g. setting up clock polarity etc.) is not done in some minutes. >>>> And the only feedback we have from the panel is "does not work"/"works". I.e. >>>> if we are not lucky that it works immediately we have no real means to debug. >>>> >>>> IMHO it also gives more flexibility to board designers to choose GPIOs instead >>>> of enforcing some SPI interface by the driver (and encapsulate this arguable >>>> protocol in the driver). Maybe some board has 3 spare GPIOs but neither >>>> McBSPs nor McSPIs available. >>> >>> This has been an interesting thread, I've learnt a lot =). >>> >>> I still think the panel driver should not handle this, but there should >>> be a separate spi bitbang driver for it. >>> >>> I understand you're not enthusiastic going that way, as the current >>> version works for you. However, when using DT, we need to think how to >>> represent the hardware in the device tree data, and it has to be right >>> from the beginning. >>> >>> That's why I won't allow representing this panel as having 4 gpios in >>> the DT data, because that is not correct. The panel has 3 pins. But >>> then, the panel does allow reading, which could be implemented using 4 >>> gpios as you have done. This data should be in the spi-bitbang data, and >>> the panel should just use the standard SPI framework. >> I disagree. There are different drivers which pass in platform data >> gpios (encoder-tfp410.c or encoder-tpd12s015.c) >> and those must be covered by DT then. I cannot see problem why to have >> for td028 panel 3 or 4 gpios defined in DT. > > The problem is not representing it in the devicetree, but representing it > correctly. This is a SPI slave device, hence it should be presented in the > devicetree as a SPI slave device and not as a platform device with 4 GPIOs. Hm. Is this a SPI or does it just look like one? Or is it some - otherwise unknown - "3 wire serial interface". Or is it a "3(+1) GPIO slave device"? I am still not sure about this. If we really want to do it correctly, we may have to write a driver for that special serial protocol as well. If it turns out that we can't mis-use and tweak it into a standard SPI driver with bit-bang backend. I simply fear that we get dependencies with the SPI subsystem and have to test, debug and maintain it. Maintaining the GPIO thing we currently have is easy. What would be against taking the GPIO approach first and then upgrade as soon as someone raises his/her finger that he/she wants to really interface this display differently and is not happy with the 3/4 GPIOs? Either they come up with a patch or contact the driver author (=me). BR, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/10/13 12:50, Dr. H. Nikolaus Schaller wrote: > Hm. Is this a SPI or does it just look like one? Or is it some - otherwise > unknown - "3 wire serial interface". Or is it a "3(+1) GPIO slave device"? > I am still not sure about this. Lars-Peter said "Back in the OpenMoko days we used the panel in normal 4-wire SPI mode with the GPIO bitbang SPI master." I don't know much about SPI, so I can't answer to that. If the serial bus is indeed not any kind of more or less standard SPI version, but really a custom bus for this controller, then the case is a bit unclear. > If we really want to do it correctly, we may have to write a driver for that > special serial protocol as well. If it turns out that we can't mis-use and tweak > it into a standard SPI driver with bit-bang backend. > > I simply fear that we get dependencies with the SPI subsystem and have > to test, debug and maintain it. Maintaining the GPIO thing we currently have > is easy. > > What would be against taking the GPIO approach first and then upgrade as soon > as someone raises his/her finger that he/she wants to really interface this display > differently and is not happy with the 3/4 GPIOs? Either they come up with a patch > or contact the driver author (=me). I don't have anything against that as long as we use only platform data. But DT data is not an in-kernel API, it's an external API. Once we define that the DT data for this panel is something, that's it, we should stick to it. Of course, we can build compatibility layers for old DT data, but I would avoid that if at all possible. If we now create the DT data with gpios, and the panel as platform device, it'd be rather nasty change to make it a child of an spi bus. (I presume, I have never made such a change). And, as the gpios and platform device approach is clearly wrong way to describe the hardware, I'm quite against using that description in the DT data. That said, one option is to describe the hardware correctly in the DT data, but have a platform device for the panel, with panel driver doing the bitbanging. In that case it is possible to update the system to use SPI framework if needed, without changing the DT data. However, I'm not sure how easy that would be. Tomi
Hi Tomi, Am 11.10.2013 um 12:09 schrieb Tomi Valkeinen: > On 11/10/13 12:50, Dr. H. Nikolaus Schaller wrote: > >> Hm. Is this a SPI or does it just look like one? Or is it some - otherwise >> unknown - "3 wire serial interface". Or is it a "3(+1) GPIO slave device"? >> I am still not sure about this. > > Lars-Peter said "Back in the OpenMoko days we used the panel in normal > 4-wire SPI mode with the GPIO bitbang SPI master." > > I don't know much about SPI, so I can't answer to that. If the serial > bus is indeed not any kind of more or less standard SPI version, but > really a custom bus for this controller, then the case is a bit unclear. I have thought over lunch time that it is worth to look into how the Openmoko did physically hook up the display and if parts of that code (it was for 2.6.26 or so and for a Samsung SoC) is understandable and reusable (e.g. hints how to set up the board file for a bitbang SPI on OMAP3 - we have never done this before). > >> If we really want to do it correctly, we may have to write a driver for that >> special serial protocol as well. If it turns out that we can't mis-use and tweak >> it into a standard SPI driver with bit-bang backend. >> >> I simply fear that we get dependencies with the SPI subsystem and have >> to test, debug and maintain it. Maintaining the GPIO thing we currently have >> is easy. >> >> What would be against taking the GPIO approach first and then upgrade as soon >> as someone raises his/her finger that he/she wants to really interface this display >> differently and is not happy with the 3/4 GPIOs? Either they come up with a patch >> or contact the driver author (=me). > > I don't have anything against that as long as we use only platform data. > > But DT data is not an in-kernel API, it's an external API. Once we > define that the DT data for this panel is something, that's it, we > should stick to it. Of course, we can build compatibility layers for old > DT data, but I would avoid that if at all possible. Ah, I see. I already think that using the DT makes such things not easier but more difficult - but I am not at all familiar with it. > If we now create the DT data with gpios, and the panel as platform > device, it'd be rather nasty change to make it a child of an spi bus. (I > presume, I have never made such a change). > > And, as the gpios and platform device approach is clearly wrong way to > describe the hardware, I'm quite against using that description in the > DT data. Since we are not familiar with DT yet, it is difficult to see the consequences of such a step. > That said, one option is to describe the hardware correctly in the DT > data, but have a platform device for the panel, with panel driver doing > the bitbanging. In that case it is possible to update the system to use > SPI framework if needed, without changing the DT data. However, I'm not > sure how easy that would be. Sounds quite complex, indeed. So our next step will be to look into the GTA02 SPI thing to get more knowlegde about thier solution. BR, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/video/omap2/displays-new/Kconfig b/drivers/video/omap2/displays-new/Kconfig index 6c90885..3f86432 100644 --- a/drivers/video/omap2/displays-new/Kconfig +++ b/drivers/video/omap2/displays-new/Kconfig @@ -56,6 +56,11 @@ config DISPLAY_PANEL_SHARP_LS037V7DW01 help LCD Panel used in TI's SDP3430 and EVM boards +config DISPLAY_PANEL_TPO_TD028TTEC1 + tristate "TPO TD028TTEC1 LCD Panel" + help + LCD panel used by Openmoko. + config DISPLAY_PANEL_TPO_TD043MTEA1 tristate "TPO TD043MTEA1 LCD Panel" depends on SPI @@ -70,4 +75,5 @@ config DISPLAY_PANEL_NEC_NL8048HL11 This NEC NL8048HL11 panel is TFT LCD used in the Zoom2/3/3630 sdp boards. + endmenu diff --git a/drivers/video/omap2/displays-new/Makefile b/drivers/video/omap2/displays-new/Makefile index 5aeb11b..0323a8a 100644 --- a/drivers/video/omap2/displays-new/Makefile +++ b/drivers/video/omap2/displays-new/Makefile @@ -8,5 +8,6 @@ obj-$(CONFIG_DISPLAY_PANEL_DSI_CM) += panel-dsi-cm.o obj-$(CONFIG_DISPLAY_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o obj-$(CONFIG_DISPLAY_PANEL_LGPHILIPS_LB035Q02) += panel-lgphilips-lb035q02.o obj-$(CONFIG_DISPLAY_PANEL_SHARP_LS037V7DW01) += panel-sharp-ls037v7dw01.o +obj-$(CONFIG_DISPLAY_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o obj-$(CONFIG_DISPLAY_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o obj-$(CONFIG_DISPLAY_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o diff --git a/drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c b/drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c new file mode 100644 index 0000000..b63586e --- /dev/null +++ b/drivers/video/omap2/displays-new/panel-tpo-td028ttec1.c @@ -0,0 +1,537 @@ +/* + * Toppoly TD028TTEC1 panel support + * + * Copyright (C) 2008 Nokia Corporation + * Author: Tomi Valkeinen <tomi.valkeinen@nokia.com> + * + * Neo 1973 code (jbt6k74.c): + * Copyright (C) 2006-2007 by OpenMoko, Inc. + * Author: Harald Welte <laforge@openmoko.org> + * + * Ported and adapted from Neo 1973 U-Boot by: + * H. Nikolaus Schaller <hns@goldelico.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + * + * 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, see <http://www.gnu.org/licenses/>. + */ + +#include <linux/module.h> +#include <linux/delay.h> +#include <linux/platform_device.h> +#include <linux/gpio.h> +#include <video/omapdss.h> +#include <video/omap-panel-data.h> + +struct panel_drv_data { + struct omap_dss_device dssdev; + struct omap_dss_device *in; + + int data_lines; + + struct omap_video_timings videomode; + + int cs_gpio; + int scl_gpio; + int din_gpio; + int dout_gpio; + + u_int16_t tx_buf[4]; +}; + +static struct omap_video_timings td028ttec1_panel_timings = { + .x_res = 480, + .y_res = 640, + .pixel_clock = 22153, + .hfp = 24, + .hsw = 8, + .hbp = 8, + .vfp = 4, + .vsw = 2, + .vbp = 2, + + .vsync_level = OMAPDSS_SIG_ACTIVE_LOW, + .hsync_level = OMAPDSS_SIG_ACTIVE_LOW, + + .data_pclk_edge = OMAPDSS_DRIVE_SIG_FALLING_EDGE, + .de_level = OMAPDSS_SIG_ACTIVE_HIGH, + .sync_pclk_edge = OMAPDSS_DRIVE_SIG_OPPOSITE_EDGES, +}; + +#define JBT_COMMAND 0x000 +#define JBT_DATA 0x100 + +/* 150uS minimum clock cycle, we have two of this plus our other + * instructions */ + +#define SPI_DELAY() udelay(200) + +static int jbt_spi_xfer(struct panel_drv_data *data, int wordnum, int bitlen) +{ + u_int16_t tmpdout = 0; + int i, j; + + gpio_set_value(data->cs_gpio, 0); + + for (i = 0; i < wordnum; i++) { + tmpdout = data->tx_buf[i]; + + for (j = 0; j < bitlen; j++) { + gpio_set_value(data->scl_gpio, 0); + if (tmpdout & (1 << (bitlen-1))) { + gpio_set_value(data->dout_gpio, 1); + if (gpio_get_value(data->din_gpio) == 0) + return 1; + } else { + gpio_set_value(data->dout_gpio, 0); + if (gpio_get_value(data->din_gpio) != 0) + return 1; + } + SPI_DELAY(); + gpio_set_value(data->scl_gpio, 1); + SPI_DELAY(); + tmpdout <<= 1; + } + } + + gpio_set_value(data->cs_gpio, 1); + + return 0; +} + +#define JBT_COMMAND 0x000 +#define JBT_DATA 0x100 + +int jbt_reg_write_nodata(struct panel_drv_data *ddata, u_int8_t reg) +{ + int rc; + + ddata->tx_buf[0] = JBT_COMMAND | reg; + + rc = jbt_spi_xfer(ddata, 1, 9); + if (rc) + dev_warn(ddata->dssdev.dev, "Failed to write reg: %x\n", reg); + + return rc; +} + +int jbt_reg_write(struct panel_drv_data *ddata, u_int8_t reg, u_int8_t data) +{ + int rc; + + ddata->tx_buf[0] = JBT_COMMAND | reg; + ddata->tx_buf[1] = JBT_DATA | data; + + rc = jbt_spi_xfer(ddata, 2, 9); + if (rc) + dev_warn(ddata->dssdev.dev, "Failed to write reg: %x\n", reg); + + return rc; +} + +int jbt_reg_write16(struct panel_drv_data *ddata, u_int8_t reg, u_int16_t data) +{ + int rc; + + ddata->tx_buf[0] = JBT_COMMAND | reg; + ddata->tx_buf[1] = JBT_DATA | (data >> 8); + ddata->tx_buf[2] = JBT_DATA | (data & 0xff); + + rc = jbt_spi_xfer(ddata, 3, 9); + if (rc) + dev_warn(ddata->dssdev.dev, "Failed to write reg: %x\n", reg); + + return rc; +} + +enum jbt_register { + JBT_REG_SLEEP_IN = 0x10, + JBT_REG_SLEEP_OUT = 0x11, + + JBT_REG_DISPLAY_OFF = 0x28, + JBT_REG_DISPLAY_ON = 0x29, + + JBT_REG_RGB_FORMAT = 0x3a, + JBT_REG_QUAD_RATE = 0x3b, + + JBT_REG_POWER_ON_OFF = 0xb0, + JBT_REG_BOOSTER_OP = 0xb1, + JBT_REG_BOOSTER_MODE = 0xb2, + JBT_REG_BOOSTER_FREQ = 0xb3, + JBT_REG_OPAMP_SYSCLK = 0xb4, + JBT_REG_VSC_VOLTAGE = 0xb5, + JBT_REG_VCOM_VOLTAGE = 0xb6, + JBT_REG_EXT_DISPL = 0xb7, + JBT_REG_OUTPUT_CONTROL = 0xb8, + JBT_REG_DCCLK_DCEV = 0xb9, + JBT_REG_DISPLAY_MODE1 = 0xba, + JBT_REG_DISPLAY_MODE2 = 0xbb, + JBT_REG_DISPLAY_MODE = 0xbc, + JBT_REG_ASW_SLEW = 0xbd, + JBT_REG_DUMMY_DISPLAY = 0xbe, + JBT_REG_DRIVE_SYSTEM = 0xbf, + + JBT_REG_SLEEP_OUT_FR_A = 0xc0, + JBT_REG_SLEEP_OUT_FR_B = 0xc1, + JBT_REG_SLEEP_OUT_FR_C = 0xc2, + JBT_REG_SLEEP_IN_LCCNT_D = 0xc3, + JBT_REG_SLEEP_IN_LCCNT_E = 0xc4, + JBT_REG_SLEEP_IN_LCCNT_F = 0xc5, + JBT_REG_SLEEP_IN_LCCNT_G = 0xc6, + + JBT_REG_GAMMA1_FINE_1 = 0xc7, + JBT_REG_GAMMA1_FINE_2 = 0xc8, + JBT_REG_GAMMA1_INCLINATION = 0xc9, + JBT_REG_GAMMA1_BLUE_OFFSET = 0xca, + + JBT_REG_BLANK_CONTROL = 0xcf, + JBT_REG_BLANK_TH_TV = 0xd0, + JBT_REG_CKV_ON_OFF = 0xd1, + JBT_REG_CKV_1_2 = 0xd2, + JBT_REG_OEV_TIMING = 0xd3, + JBT_REG_ASW_TIMING_1 = 0xd4, + JBT_REG_ASW_TIMING_2 = 0xd5, + + JBT_REG_HCLOCK_VGA = 0xec, + JBT_REG_HCLOCK_QVGA = 0xed, +}; + +#define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev) + +static int td028ttec1_panel_connect(struct omap_dss_device *dssdev) +{ + struct panel_drv_data *ddata = to_panel_data(dssdev); + struct omap_dss_device *in = ddata->in; + int r; + + if (omapdss_device_is_connected(dssdev)) + return 0; + + r = in->ops.dpi->connect(in, dssdev); + if (r) + return r; + + return 0; +} + +static void td028ttec1_panel_disconnect(struct omap_dss_device *dssdev) +{ + struct panel_drv_data *ddata = to_panel_data(dssdev); + struct omap_dss_device *in = ddata->in; + + if (!omapdss_device_is_connected(dssdev)) + return; + + in->ops.dpi->disconnect(in, dssdev); +} + +static int td028ttec1_panel_enable(struct omap_dss_device *dssdev) +{ + struct panel_drv_data *ddata = to_panel_data(dssdev); + struct omap_dss_device *in = ddata->in; + int r; + + if (!omapdss_device_is_connected(dssdev)) + return -ENODEV; + + if (omapdss_device_is_enabled(dssdev)) + return 0; + + in->ops.dpi->set_data_lines(in, ddata->data_lines); + in->ops.dpi->set_timings(in, &ddata->videomode); + + r = in->ops.dpi->enable(in); + if (r) + return r; + + dev_dbg(dssdev->dev, "td028ttec1_panel_enable() - state %d\n", + dssdev->state); + + /* three times command zero */ + r |= jbt_reg_write_nodata(ddata, 0x00); + udelay(1000); + r |= jbt_reg_write_nodata(ddata, 0x00); + udelay(1000); + r |= jbt_reg_write_nodata(ddata, 0x00); + udelay(1000); + + /* deep standby out */ + r |= jbt_reg_write(ddata, JBT_REG_POWER_ON_OFF, 0x17); + + /* RGB I/F on, RAM write off, QVGA through, SIGCON enable */ + r |= jbt_reg_write(ddata, JBT_REG_DISPLAY_MODE, 0x80); + + /* Quad mode off */ + r |= jbt_reg_write(ddata, JBT_REG_QUAD_RATE, 0x00); + + /* AVDD on, XVDD on */ + r |= jbt_reg_write(ddata, JBT_REG_POWER_ON_OFF, 0x16); + + /* Output control */ + r |= jbt_reg_write16(ddata, JBT_REG_OUTPUT_CONTROL, 0xfff9); + + /* Sleep mode off */ + r |= jbt_reg_write_nodata(ddata, JBT_REG_SLEEP_OUT); + + /* at this point we have like 50% grey */ + + /* initialize register set */ + r |= jbt_reg_write(ddata, JBT_REG_DISPLAY_MODE1, 0x01); + r |= jbt_reg_write(ddata, JBT_REG_DISPLAY_MODE2, 0x00); + r |= jbt_reg_write(ddata, JBT_REG_RGB_FORMAT, 0x60); + r |= jbt_reg_write(ddata, JBT_REG_DRIVE_SYSTEM, 0x10); + r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_OP, 0x56); + r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_MODE, 0x33); + r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_FREQ, 0x11); + r |= jbt_reg_write(ddata, JBT_REG_BOOSTER_FREQ, 0x11); + r |= jbt_reg_write(ddata, JBT_REG_OPAMP_SYSCLK, 0x02); + r |= jbt_reg_write(ddata, JBT_REG_VSC_VOLTAGE, 0x2b); + r |= jbt_reg_write(ddata, JBT_REG_VCOM_VOLTAGE, 0x40); + r |= jbt_reg_write(ddata, JBT_REG_EXT_DISPL, 0x03); + r |= jbt_reg_write(ddata, JBT_REG_DCCLK_DCEV, 0x04); + /* + * default of 0x02 in JBT_REG_ASW_SLEW responsible for 72Hz requirement + * to avoid red / blue flicker + */ + r |= jbt_reg_write(ddata, JBT_REG_ASW_SLEW, 0x04); + r |= jbt_reg_write(ddata, JBT_REG_DUMMY_DISPLAY, 0x00); + + r |= jbt_reg_write(ddata, JBT_REG_SLEEP_OUT_FR_A, 0x11); + r |= jbt_reg_write(ddata, JBT_REG_SLEEP_OUT_FR_B, 0x11); + r |= jbt_reg_write(ddata, JBT_REG_SLEEP_OUT_FR_C, 0x11); + r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_D, 0x2040); + r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_E, 0x60c0); + r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_F, 0x1020); + r |= jbt_reg_write16(ddata, JBT_REG_SLEEP_IN_LCCNT_G, 0x60c0); + + r |= jbt_reg_write16(ddata, JBT_REG_GAMMA1_FINE_1, 0x5533); + r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_FINE_2, 0x00); + r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_INCLINATION, 0x00); + r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_BLUE_OFFSET, 0x00); + r |= jbt_reg_write(ddata, JBT_REG_GAMMA1_BLUE_OFFSET, 0x00); + + r |= jbt_reg_write16(ddata, JBT_REG_HCLOCK_VGA, 0x1f0); + r |= jbt_reg_write(ddata, JBT_REG_BLANK_CONTROL, 0x02); + r |= jbt_reg_write16(ddata, JBT_REG_BLANK_TH_TV, 0x0804); + r |= jbt_reg_write16(ddata, JBT_REG_BLANK_TH_TV, 0x0804); + + r |= jbt_reg_write(ddata, JBT_REG_CKV_ON_OFF, 0x01); + r |= jbt_reg_write16(ddata, JBT_REG_CKV_1_2, 0x0000); + + r |= jbt_reg_write16(ddata, JBT_REG_OEV_TIMING, 0x0d0e); + r |= jbt_reg_write16(ddata, JBT_REG_ASW_TIMING_1, 0x11a4); + r |= jbt_reg_write(ddata, JBT_REG_ASW_TIMING_2, 0x0e); + + r |= jbt_reg_write_nodata(ddata, JBT_REG_DISPLAY_ON); + + dssdev->state = OMAP_DSS_DISPLAY_ACTIVE; + + return r; +} + +static void td028ttec1_panel_disable(struct omap_dss_device *dssdev) +{ + struct panel_drv_data *ddata = to_panel_data(dssdev); + struct omap_dss_device *in = ddata->in; + + if (!omapdss_device_is_enabled(dssdev)) + return; + + dev_dbg(dssdev->dev, "td028ttec1_panel_disable()\n"); + + in->ops.dpi->disable(in); + + jbt_reg_write_nodata(ddata, JBT_REG_DISPLAY_OFF); + jbt_reg_write16(ddata, JBT_REG_OUTPUT_CONTROL, 0x8002); + jbt_reg_write_nodata(ddata, JBT_REG_SLEEP_IN); + jbt_reg_write(ddata, JBT_REG_POWER_ON_OFF, 0x00); + + dssdev->state = OMAP_DSS_DISPLAY_DISABLED; +} + +static void td028ttec1_panel_set_timings(struct omap_dss_device *dssdev, + struct omap_video_timings *timings) +{ + struct panel_drv_data *ddata = to_panel_data(dssdev); + struct omap_dss_device *in = ddata->in; + + ddata->videomode = *timings; + dssdev->panel.timings = *timings; + + in->ops.dpi->set_timings(in, timings); +} + +static void td028ttec1_panel_get_timings(struct omap_dss_device *dssdev, + struct omap_video_timings *timings) +{ + struct panel_drv_data *ddata = to_panel_data(dssdev); + + *timings = ddata->videomode; +} + +static int td028ttec1_panel_check_timings(struct omap_dss_device *dssdev, + struct omap_video_timings *timings) +{ + struct panel_drv_data *ddata = to_panel_data(dssdev); + struct omap_dss_device *in = ddata->in; + + return in->ops.dpi->check_timings(in, timings); +} + +static struct omap_dss_driver td028ttec1_ops = { + .connect = td028ttec1_panel_connect, + .disconnect = td028ttec1_panel_disconnect, + + .enable = td028ttec1_panel_enable, + .disable = td028ttec1_panel_disable, + + .set_timings = td028ttec1_panel_set_timings, + .get_timings = td028ttec1_panel_get_timings, + .check_timings = td028ttec1_panel_check_timings, +}; + +static int td028ttec1_panel_probe_pdata(struct platform_device *pdev) +{ + const struct panel_tpo_td028tec1_platform_data *pdata; + struct panel_drv_data *ddata = platform_get_drvdata(pdev); + struct omap_dss_device *dssdev, *in; + + pdata = dev_get_platdata(&pdev->dev); + + in = omap_dss_find_output(pdata->source); + if (in == NULL) { + dev_err(&pdev->dev, "failed to find video source '%s'\n", + pdata->source); + return -EPROBE_DEFER; + } + + ddata->in = in; + + ddata->data_lines = pdata->data_lines; + + dssdev = &ddata->dssdev; + dssdev->name = pdata->name; + + ddata->cs_gpio = pdata->cs_gpio; + ddata->scl_gpio = pdata->scl_gpio; + ddata->din_gpio = pdata->din_gpio; + ddata->dout_gpio = pdata->dout_gpio; + + return 0; +} + +static int td028ttec1_panel_probe(struct platform_device *pdev) +{ + struct panel_drv_data *ddata; + struct omap_dss_device *dssdev; + int r; + + ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL); + if (ddata == NULL) + return -ENOMEM; + + platform_set_drvdata(pdev, ddata); + + if (dev_get_platdata(&pdev->dev)) { + r = td028ttec1_panel_probe_pdata(pdev); + if (r) + return r; + } else { + return -ENODEV; + } + + if (gpio_is_valid(ddata->cs_gpio)) { + r = devm_gpio_request_one(&pdev->dev, ddata->cs_gpio, + GPIOF_OUT_INIT_HIGH, "lcd cs"); + if (r) + goto err_gpio; + } + + if (gpio_is_valid(ddata->scl_gpio)) { + r = devm_gpio_request_one(&pdev->dev, ddata->scl_gpio, + GPIOF_OUT_INIT_HIGH, "lcd scl"); + if (r) + goto err_gpio; + } + + if (gpio_is_valid(ddata->dout_gpio)) { + r = devm_gpio_request_one(&pdev->dev, ddata->dout_gpio, + GPIOF_OUT_INIT_LOW, "lcd dout"); + if (r) + goto err_gpio; + } + + if (gpio_is_valid(ddata->din_gpio)) { + r = devm_gpio_request_one(&pdev->dev, ddata->din_gpio, + GPIOF_IN, "lcd din"); + if (r) + goto err_gpio; + } + + /* according to data sheet: wait 50ms (Tpos of LCM). However, 50ms + * seems unreliable with later LCM batches, increasing to 90ms */ + mdelay(90); + + ddata->videomode = td028ttec1_panel_timings; + + dssdev = &ddata->dssdev; + dssdev->dev = &pdev->dev; + dssdev->driver = &td028ttec1_ops; + dssdev->type = OMAP_DISPLAY_TYPE_DPI; + dssdev->owner = THIS_MODULE; + dssdev->panel.timings = ddata->videomode; + dssdev->phy.dpi.data_lines = ddata->data_lines; + + r = omapdss_register_display(dssdev); + if (r) { + dev_err(&pdev->dev, "Failed to register panel\n"); + goto err_reg; + } + + return 0; + +err_reg: +err_gpio: + omap_dss_put_device(ddata->in); + return r; +} + +static int __exit td028ttec1_panel_remove(struct platform_device *pdev) +{ + struct panel_drv_data *ddata = platform_get_drvdata(pdev); + struct omap_dss_device *dssdev = &ddata->dssdev; + struct omap_dss_device *in = ddata->in; + + omapdss_unregister_display(dssdev); + + td028ttec1_panel_disable(dssdev); + td028ttec1_panel_disconnect(dssdev); + + omap_dss_put_device(in); + + return 0; +} + +static struct platform_driver td028ttec1_driver = { + .probe = td028ttec1_panel_probe, + .remove = __exit_p(td028ttec1_panel_remove), + + .driver = { + .name = "panel-tpo-td028ttec1", + .owner = THIS_MODULE, + }, +}; + +module_platform_driver(td028ttec1_driver); + +MODULE_AUTHOR("H. Nikolaus Schaller <hns@goldelico.com>"); +MODULE_DESCRIPTION("Toppoly TD028TTEC1 panel driver"); +MODULE_LICENSE("GPL"); diff --git a/include/video/omap-panel-data.h b/include/video/omap-panel-data.h index f7ac8d9..02565ff 100644 --- a/include/video/omap-panel-data.h +++ b/include/video/omap-panel-data.h @@ -238,4 +238,26 @@ struct panel_nec_nl8048hl11_platform_data { int qvga_gpio; }; +/** + * panel-tpo-td028tec1 platform data + * @name: name for display netity + * @source: name of the display entity used as a video source + * @data_lines: number of DPI datalines + * @cs_gpio: CS gpio + * @scl_gpio: CLK gpio + * @din_gpio: input data gpio + * @dout_gpio: output data gpio + */ +struct panel_tpo_td028tec1_platform_data { + const char *name; + const char *source; + + int data_lines; + + int cs_gpio; + int scl_gpio; + int din_gpio; + int dout_gpio; +}; + #endif /* __OMAP_PANEL_DATA_H */