| Message ID | 20190707181937.6250-21-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/omap: Replace custom display drivers with drm_bridge and drm_panel | expand |
Hi Laurent. I had assumed this driver would look like the other Topology driver, but they differ a lot. So it makes sense to have different drivers. This driver implements suspend/resume. But the correct way would be to implment prepare/unprepare. The power_on(), power_off() functions would then be embedded in the prepare(), unprepare() functions as there would be only one user. See additional comments in the following. Sam On Sun, Jul 07, 2019 at 09:19:01PM +0300, Laurent Pinchart wrote: > This panel is used on the OMAP3 Pandora. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/panel/Kconfig | 7 + > drivers/gpu/drm/panel/Makefile | 1 + > drivers/gpu/drm/panel/panel-tpo-td043mtea1.c | 510 +++++++++++++++++++ > 3 files changed, 518 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-tpo-td043mtea1.c > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > index b7099d211061..8f3660c73044 100644 > --- a/drivers/gpu/drm/panel/Kconfig > +++ b/drivers/gpu/drm/panel/Kconfig > @@ -312,6 +312,13 @@ config DRM_PANEL_TPO_TD028TTEC1 > Say Y here if you want to enable support for TPO TD028TTEC1 480x640 > 2.8" panel. > > +config DRM_PANEL_TPO_TD043MTEA1 > + tristate "TPO TD043MTEA1 panel driver" Spell out TPO? > + depends on GPIOLIB && OF && REGULATOR && SPI > + help > + Say Y here if you want to enable support for TPO TD043MTEA1 800x480 > + 4.3" panel. Maybe tell it is used on OMAP3 Pandora > + > config DRM_PANEL_TPO_TPG110 > tristate "TPO TPG 800x400 panel" > depends on OF && SPI && GPIOLIB > diff --git a/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c > new file mode 100644 > index 000000000000..6b17e47582b8 > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c > @@ -0,0 +1,510 @@ > +// SPDX-License-Identifier: GPL-2.0+ Just noticed, this is a different license than the others. But I guess this comes from the original file. > +/* > + * Toppology TD043MTEA1 Panel Driver So I actually asked Google this time - the correct spelling is "Toppoly". See: http://www.innolux.com/Pages/EN/AboutUs/Company_Overview_EN.html > +struct td043mtea1_device { > + struct drm_panel panel; > + > + struct spi_device *spi; > + struct regulator *vcc_reg; > + struct gpio_desc *reset_gpio; > + > + unsigned int mode; > + u16 gamma[12]; > + bool vmirror; > + bool powered_on; This flag will not be needed when prepare(), unprepare() are used. > + bool spi_suspended; > + bool power_on_resume; > +}; > + > + > +static void td043mtea1_write_gamma(struct td043mtea1_device *lcd) > +{ > + const u16 *gamma = lcd->gamma; > + unsigned int i; > + u8 val; > + > + /* gamma bits [9:8] */ > + for (val = i = 0; i < 4; i++) > + val |= (gamma[i] & 0x300) >> ((i + 1) * 2); > + td043mtea1_write(lcd, 0x11, val); > + > + for (val = i = 0; i < 4; i++) > + val |= (gamma[i+4] & 0x300) >> ((i + 1) * 2); Spaces around operators. > + td043mtea1_write(lcd, 0x12, val); > + > + for (val = i = 0; i < 4; i++) > + val |= (gamma[i+8] & 0x300) >> ((i + 1) * 2); Same here. > + td043mtea1_write(lcd, 0x13, val); > + > + /* gamma bits [7:0] */ > + for (val = i = 0; i < 12; i++) > + td043mtea1_write(lcd, 0x14 + i, gamma[i] & 0xff); > +} This function (td043mtea1_write_gamma()) fails to check the result of the write operations. Maybe on purpose. But looks strange we do it in some places but not all. > + > +static int td043mtea1_write_mirror(struct td043mtea1_device *lcd) > +{ > + u8 reg4 = TPO_R04_NFLIP_H | TPO_R04_NFLIP_V | > + TPO_R04_CP_CLK_FREQ_1H | TPO_R04_VGL_FREQ_1H; > + if (lcd->vmirror) > + reg4 &= ~TPO_R04_NFLIP_V; > + > + return td043mtea1_write(lcd, 4, reg4); > +} Add a: #define TPO_R4 4 And then use it here? Looks bad that the register number is hardcoded. Same goes for several other calls to the write() function. > + > +static int td043mtea1_power_on(struct td043mtea1_device *lcd) > +{ > + int ret; > + > + if (lcd->powered_on) > + return 0; > + > + ret = regulator_enable(lcd->vcc_reg); > + if (ret < 0) > + return ret; > + > + /* Wait for the panel to stabilize. */ > + msleep(160); > + > + gpiod_set_value(lcd->reset_gpio, 0); > + > + td043mtea1_write(lcd, 2, TPO_R02_MODE(lcd->mode) | TPO_R02_NCLK_RISING); > + td043mtea1_write(lcd, 3, TPO_R03_VAL_NORMAL); > + td043mtea1_write(lcd, 0x20, 0xf0); > + td043mtea1_write(lcd, 0x21, 0xf0); > + td043mtea1_write_mirror(lcd); > + td043mtea1_write_gamma(lcd); > + > + lcd->powered_on = true; > + > + return 0; > +} The above should be part of prepare() > + > +static void td043mtea1_power_off(struct td043mtea1_device *lcd) > +{ > + if (!lcd->powered_on) > + return; > + > + td043mtea1_write(lcd, 3, TPO_R03_VAL_STANDBY | TPO_R03_EN_PWM); > + > + gpiod_set_value(lcd->reset_gpio, 1); > + > + /* wait for at least 2 vsyncs before cutting off power */ > + msleep(50); > + > + td043mtea1_write(lcd, 3, TPO_R03_VAL_STANDBY); > + > + regulator_disable(lcd->vcc_reg); > + > + lcd->powered_on = false; > +} The above should be part of unprepare() > + > +/* ----------------------------------------------------------------------------- > + * sysfs > + */ > + > +static ssize_t vmirror_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > + > + return snprintf(buf, PAGE_SIZE, "%d\n", lcd->vmirror); > +} > + > +static ssize_t vmirror_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > + int val; > + int ret; > + > + ret = kstrtoint(buf, 0, &val); > + if (ret < 0) > + return ret; > + > + lcd->vmirror = !!val; > + > + ret = td043mtea1_write_mirror(lcd); > + if (ret < 0) > + return ret; > + > + return count; > +} > + > +static ssize_t mode_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > + > + return snprintf(buf, PAGE_SIZE, "%d\n", lcd->mode); > +} > + > +static ssize_t mode_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > + long val; > + int ret; > + > + ret = kstrtol(buf, 0, &val); > + if (ret != 0 || val & ~7) > + return -EINVAL; > + > + lcd->mode = val; > + > + val |= TPO_R02_NCLK_RISING; > + td043mtea1_write(lcd, 2, val); > + > + return count; > +} > + > +static ssize_t gamma_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > + ssize_t len = 0; > + unsigned int i; > + int ret; > + > + for (i = 0; i < ARRAY_SIZE(lcd->gamma); i++) { > + ret = snprintf(buf + len, PAGE_SIZE - len, "%u ", > + lcd->gamma[i]); > + if (ret < 0) > + return ret; > + len += ret; > + } > + buf[len - 1] = '\n'; > + > + return len; > +} > + > +static ssize_t gamma_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > + unsigned int g[12]; > + unsigned int i; > + int ret; > + > + ret = sscanf(buf, "%u %u %u %u %u %u %u %u %u %u %u %u", > + &g[0], &g[1], &g[2], &g[3], &g[4], &g[5], > + &g[6], &g[7], &g[8], &g[9], &g[10], &g[11]); > + if (ret != 12) > + return -EINVAL; > + > + for (i = 0; i < 12; i++) > + lcd->gamma[i] = g[i]; > + > + td043mtea1_write_gamma(lcd); > + > + return count; > +} > + > +static DEVICE_ATTR_RW(vmirror); > +static DEVICE_ATTR_RW(mode); > +static DEVICE_ATTR_RW(gamma); > + > +static struct attribute *td043mtea1_attrs[] = { > + &dev_attr_vmirror.attr, > + &dev_attr_mode.attr, > + &dev_attr_gamma.attr, > + NULL, > +}; > + > +static const struct attribute_group td043mtea1_attr_group = { > + .attrs = td043mtea1_attrs, > +}; I see what is done with mirror, mode and gamma - but the question is if they are really needed? And if needed, is it the right way to configure the panel? This is likely questiosn that are not easy to answer definitive, so best to keep this as it was before. > + > +/* ----------------------------------------------------------------------------- > + * Bridge Operations > + */ Panel operations, not bridge operations? > + > +static int td043mtea1_disable(struct drm_panel *panel) > +{ > + struct td043mtea1_device *lcd = to_td043mtea1_device(panel); > + > + if (!lcd->spi_suspended) > + td043mtea1_power_off(lcd); > + > + return 0; > +} > + > +static int td043mtea1_enable(struct drm_panel *panel) > +{ > + struct td043mtea1_device *lcd = to_td043mtea1_device(panel); > + int ret; > + > + /* > + * If we are resuming from system suspend, SPI might not be enabled > + * yet, so we'll program the LCD from SPI PM resume callback. > + */ > + if (lcd->spi_suspended) > + return 0; I do not recall this is needed in other panel drivers, so look at what other spi based panels do here. I think this is something that today is not required. > + > + ret = td043mtea1_power_on(lcd); > + if (ret) { > + dev_err(&lcd->spi->dev, "%s: power on failed (%d)\n", > + __func__, ret); > + return ret; > + } > + > + return 0; > +} > + > +static const struct drm_display_mode td043mtea1_mode = { > + .clock = 36000, > + .hdisplay = 800, > + .hsync_start = 800 + 68, > + .hsync_end = 800 + 68 + 1, > + .htotal = 800 + 68 + 1 + 214, > + .vdisplay = 480, > + .vsync_start = 480 + 39, > + .vsync_end = 480 + 39 + 1, > + .vtotal = 480 + 39 + 1 + 34, > + .vrefresh = 60, > + .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED, > + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, > +}; height_mm, width_mm > + > +static int td043mtea1_get_modes(struct drm_panel *panel) > +{ > + struct drm_connector *connector = panel->connector; > + struct drm_display_mode *mode; > + > + mode = drm_mode_duplicate(panel->drm, &td043mtea1_mode); > + if (!mode) > + return -ENOMEM; > + > + drm_mode_set_name(mode); > + drm_mode_probed_add(connector, mode); > + > + connector->display_info.width_mm = 94; > + connector->display_info.height_mm = 56; > + /* > + * FIXME: According to the datasheet sync signals are sampled on the > + * rising edge of the clock, but the code running on the OMAP3 Pandora > + * indicates sampling on the falling edge. This should be tested on a > + * real device. > + */ > + connector->display_info.bus_flags = DRM_BUS_FLAG_DE_HIGH > + | DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE > + | DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE; > + > + return 1; > +} > + > +static const struct drm_panel_funcs td043mtea1_funcs = { > + .disable = td043mtea1_disable, > + .enable = td043mtea1_enable, > + .get_modes = td043mtea1_get_modes, > +}; > + > +/* ----------------------------------------------------------------------------- > + * Power Management, Probe and Remove > + */ > + > +#ifdef CONFIG_PM_SLEEP > +static int td043mtea1_suspend(struct device *dev) > +{ > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > + > + if (lcd->powered_on) { > + td043mtea1_power_off(lcd); > + lcd->powered_on = true; > + } > + > + lcd->spi_suspended = true; > + > + return 0; > +} > + > +static int td043mtea1_resume(struct device *dev) > +{ > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > + int ret; > + > + lcd->spi_suspended = false; > + > + if (lcd->powered_on) { > + lcd->powered_on = false; > + ret = td043mtea1_power_on(lcd); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static SIMPLE_DEV_PM_OPS(td043mtea1_pm_ops, td043mtea1_suspend, > + td043mtea1_resume); > +#endif > + > +static int td043mtea1_probe(struct spi_device *spi) > +{ > + struct td043mtea1_device *lcd; > + int ret; > + > + lcd = devm_kzalloc(&spi->dev, sizeof(*lcd), GFP_KERNEL); > + if (lcd == NULL) > + return -ENOMEM; > + > + spi_set_drvdata(spi, lcd); > + lcd->spi = spi; > + lcd->mode = TPO_R02_MODE_800x480; > + memcpy(lcd->gamma, td043mtea1_def_gamma, sizeof(lcd->gamma)); > + > + lcd->vcc_reg = devm_regulator_get(&spi->dev, "vcc"); > + if (IS_ERR(lcd->vcc_reg)) { > + dev_err(&spi->dev, "failed to get VCC regulator\n"); > + return PTR_ERR(lcd->vcc_reg); > + } > + > + lcd->reset_gpio = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(lcd->reset_gpio)) { > + dev_err(&spi->dev, "failed to get reset GPIO\n"); > + return PTR_ERR(lcd->reset_gpio); > + } > + > + spi->bits_per_word = 16; > + spi->mode = SPI_MODE_0; > + > + ret = spi_setup(spi); > + if (ret < 0) { > + dev_err(&spi->dev, "failed to setup SPI: %d\n", ret); > + return ret; > + } > + > + ret = sysfs_create_group(&spi->dev.kobj, &td043mtea1_attr_group); > + if (ret < 0) { > + dev_err(&spi->dev, "failed to create sysfs files\n"); > + return ret; > + } > + > + drm_panel_init(&lcd->panel); > + lcd->panel.dev = &lcd->spi->dev; > + lcd->panel.funcs = &td043mtea1_funcs; > + > + ret = drm_panel_add(&lcd->panel); > + if (ret < 0) { > + sysfs_remove_group(&spi->dev.kobj, &td043mtea1_attr_group); > + return ret; > + } > + > + return 0; > +} > + > +static int td043mtea1_remove(struct spi_device *spi) > +{ > + struct td043mtea1_device *lcd = spi_get_drvdata(spi); > + > + drm_panel_remove(&lcd->panel); > + td043mtea1_disable(&lcd->panel); > + > + sysfs_remove_group(&spi->dev.kobj, &td043mtea1_attr_group); > + > + return 0; > +} > + > +static const struct of_device_id td043mtea1_of_match[] = { > + { .compatible = "tpo,td043mtea1", }, > + {}, { /* sentinel */ },
Hi Sam, On Wed, Jul 10, 2019 at 03:09:17PM +0200, Sam Ravnborg wrote: > Hi Laurent. > > I had assumed this driver would look like the other Topology driver, but > they differ a lot. So it makes sense to have different drivers. > > This driver implements suspend/resume. > But the correct way would be to implment prepare/unprepare. As for the NEC panel, I'm not opposed to this change, but I have no hardware to test it, so I would prefer if it was later done on top. > The power_on(), power_off() functions would then be embedded in > the prepare(), unprepare() functions as there would be only one user. > > See additional comments in the following. > > On Sun, Jul 07, 2019 at 09:19:01PM +0300, Laurent Pinchart wrote: > > This panel is used on the OMAP3 Pandora. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > drivers/gpu/drm/panel/Kconfig | 7 + > > drivers/gpu/drm/panel/Makefile | 1 + > > drivers/gpu/drm/panel/panel-tpo-td043mtea1.c | 510 +++++++++++++++++++ > > 3 files changed, 518 insertions(+) > > create mode 100644 drivers/gpu/drm/panel/panel-tpo-td043mtea1.c > > > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > > index b7099d211061..8f3660c73044 100644 > > --- a/drivers/gpu/drm/panel/Kconfig > > +++ b/drivers/gpu/drm/panel/Kconfig > > @@ -312,6 +312,13 @@ config DRM_PANEL_TPO_TD028TTEC1 > > Say Y here if you want to enable support for TPO TD028TTEC1 480x640 > > 2.8" panel. > > > > +config DRM_PANEL_TPO_TD043MTEA1 > > + tristate "TPO TD043MTEA1 panel driver" > > Spell out TPO? > > > + depends on GPIOLIB && OF && REGULATOR && SPI > > + help > > + Say Y here if you want to enable support for TPO TD043MTEA1 800x480 > > + 4.3" panel. > > Maybe tell it is used on OMAP3 Pandora > > > + > > config DRM_PANEL_TPO_TPG110 > > tristate "TPO TPG 800x400 panel" > > depends on OF && SPI && GPIOLIB > > diff --git a/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c > > new file mode 100644 > > index 000000000000..6b17e47582b8 > > --- /dev/null > > +++ b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c > > @@ -0,0 +1,510 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > Just noticed, this is a different license than the others. > But I guess this comes from the original file. Correct. > > +/* > > + * Toppology TD043MTEA1 Panel Driver > > So I actually asked Google this time - the correct spelling is > "Toppoly". > See: http://www.innolux.com/Pages/EN/AboutUs/Company_Overview_EN.html Fixed (and in the previous patch too). > > +struct td043mtea1_device { > > + struct drm_panel panel; > > + > > + struct spi_device *spi; > > + struct regulator *vcc_reg; > > + struct gpio_desc *reset_gpio; > > + > > + unsigned int mode; > > + u16 gamma[12]; > > + bool vmirror; > > + bool powered_on; > > This flag will not be needed when prepare(), unprepare() are used. > > > + bool spi_suspended; > > + bool power_on_resume; > > +}; > > + > > + > > > +static void td043mtea1_write_gamma(struct td043mtea1_device *lcd) > > +{ > > + const u16 *gamma = lcd->gamma; > > + unsigned int i; > > + u8 val; > > + > > + /* gamma bits [9:8] */ > > + for (val = i = 0; i < 4; i++) > > + val |= (gamma[i] & 0x300) >> ((i + 1) * 2); > > + td043mtea1_write(lcd, 0x11, val); > > + > > + for (val = i = 0; i < 4; i++) > > + val |= (gamma[i+4] & 0x300) >> ((i + 1) * 2); > > Spaces around operators. > > > + td043mtea1_write(lcd, 0x12, val); > > + > > + for (val = i = 0; i < 4; i++) > > + val |= (gamma[i+8] & 0x300) >> ((i + 1) * 2); > > Same here. > > > + td043mtea1_write(lcd, 0x13, val); > > + > > + /* gamma bits [7:0] */ > > + for (val = i = 0; i < 12; i++) > > + td043mtea1_write(lcd, 0x14 + i, gamma[i] & 0xff); > > +} > > This function (td043mtea1_write_gamma()) fails to check the result of > the write operations. Maybe on purpose. But looks strange we do it in > some places but not all. The return value is only checked in td043mtea1_write_mirror(). I think this should be fixed eventually, but again, no hardware to test :-( I'm this a bit reluctant to make intrusive changes. > > + > > +static int td043mtea1_write_mirror(struct td043mtea1_device *lcd) > > +{ > > + u8 reg4 = TPO_R04_NFLIP_H | TPO_R04_NFLIP_V | > > + TPO_R04_CP_CLK_FREQ_1H | TPO_R04_VGL_FREQ_1H; > > + if (lcd->vmirror) > > + reg4 &= ~TPO_R04_NFLIP_V; > > + > > + return td043mtea1_write(lcd, 4, reg4); > > +} > > Add a: > #define TPO_R4 4 > And then use it here? > Looks bad that the register number is hardcoded. > > Same goes for several other calls to the write() function. Without any idea of what those registers are for (and without the panel documentation), I think TPO_R4 is actually less readable than 4. > > + > > +static int td043mtea1_power_on(struct td043mtea1_device *lcd) > > +{ > > + int ret; > > + > > + if (lcd->powered_on) > > + return 0; > > + > > + ret = regulator_enable(lcd->vcc_reg); > > + if (ret < 0) > > + return ret; > > + > > + /* Wait for the panel to stabilize. */ > > + msleep(160); > > + > > + gpiod_set_value(lcd->reset_gpio, 0); > > + > > + td043mtea1_write(lcd, 2, TPO_R02_MODE(lcd->mode) | TPO_R02_NCLK_RISING); > > + td043mtea1_write(lcd, 3, TPO_R03_VAL_NORMAL); > > + td043mtea1_write(lcd, 0x20, 0xf0); > > + td043mtea1_write(lcd, 0x21, 0xf0); > > + td043mtea1_write_mirror(lcd); > > + td043mtea1_write_gamma(lcd); > > + > > + lcd->powered_on = true; > > + > > + return 0; > > +} > > The above should be part of prepare() > > > + > > +static void td043mtea1_power_off(struct td043mtea1_device *lcd) > > +{ > > + if (!lcd->powered_on) > > + return; > > + > > + td043mtea1_write(lcd, 3, TPO_R03_VAL_STANDBY | TPO_R03_EN_PWM); > > + > > + gpiod_set_value(lcd->reset_gpio, 1); > > + > > + /* wait for at least 2 vsyncs before cutting off power */ > > + msleep(50); > > + > > + td043mtea1_write(lcd, 3, TPO_R03_VAL_STANDBY); > > + > > + regulator_disable(lcd->vcc_reg); > > + > > + lcd->powered_on = false; > > +} > > The above should be part of unprepare() > > > + > > +/* ----------------------------------------------------------------------------- > > + * sysfs > > + */ > > + > > +static ssize_t vmirror_show(struct device *dev, struct device_attribute *attr, > > + char *buf) > > +{ > > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > > + > > + return snprintf(buf, PAGE_SIZE, "%d\n", lcd->vmirror); > > +} > > + > > +static ssize_t vmirror_store(struct device *dev, struct device_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > > + int val; > > + int ret; > > + > > + ret = kstrtoint(buf, 0, &val); > > + if (ret < 0) > > + return ret; > > + > > + lcd->vmirror = !!val; > > + > > + ret = td043mtea1_write_mirror(lcd); > > + if (ret < 0) > > + return ret; > > + > > + return count; > > +} > > + > > +static ssize_t mode_show(struct device *dev, struct device_attribute *attr, > > + char *buf) > > +{ > > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > > + > > + return snprintf(buf, PAGE_SIZE, "%d\n", lcd->mode); > > +} > > + > > +static ssize_t mode_store(struct device *dev, struct device_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > > + long val; > > + int ret; > > + > > + ret = kstrtol(buf, 0, &val); > > + if (ret != 0 || val & ~7) > > + return -EINVAL; > > + > > + lcd->mode = val; > > + > > + val |= TPO_R02_NCLK_RISING; > > + td043mtea1_write(lcd, 2, val); > > + > > + return count; > > +} > > + > > +static ssize_t gamma_show(struct device *dev, struct device_attribute *attr, > > + char *buf) > > +{ > > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > > + ssize_t len = 0; > > + unsigned int i; > > + int ret; > > + > > + for (i = 0; i < ARRAY_SIZE(lcd->gamma); i++) { > > + ret = snprintf(buf + len, PAGE_SIZE - len, "%u ", > > + lcd->gamma[i]); > > + if (ret < 0) > > + return ret; > > + len += ret; > > + } > > + buf[len - 1] = '\n'; > > + > > + return len; > > +} > > + > > +static ssize_t gamma_store(struct device *dev, struct device_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > > + unsigned int g[12]; > > + unsigned int i; > > + int ret; > > + > > + ret = sscanf(buf, "%u %u %u %u %u %u %u %u %u %u %u %u", > > + &g[0], &g[1], &g[2], &g[3], &g[4], &g[5], > > + &g[6], &g[7], &g[8], &g[9], &g[10], &g[11]); > > + if (ret != 12) > > + return -EINVAL; > > + > > + for (i = 0; i < 12; i++) > > + lcd->gamma[i] = g[i]; > > + > > + td043mtea1_write_gamma(lcd); > > + > > + return count; > > +} > > + > > +static DEVICE_ATTR_RW(vmirror); > > +static DEVICE_ATTR_RW(mode); > > +static DEVICE_ATTR_RW(gamma); > > + > > +static struct attribute *td043mtea1_attrs[] = { > > + &dev_attr_vmirror.attr, > > + &dev_attr_mode.attr, > > + &dev_attr_gamma.attr, > > + NULL, > > +}; > > + > > +static const struct attribute_group td043mtea1_attr_group = { > > + .attrs = td043mtea1_attrs, > > +}; > > I see what is done with mirror, mode and gamma - but the question is if > they are really needed? > And if needed, is it the right way to configure the panel? > This is likely questiosn that are not easy to answer definitive, so best > to keep this as it was before. I'm afraid I have no idea how (and if) those are used :-S > > + > > +/* ----------------------------------------------------------------------------- > > + * Bridge Operations > > + */ > > Panel operations, not bridge operations? > > > + > > +static int td043mtea1_disable(struct drm_panel *panel) > > +{ > > + struct td043mtea1_device *lcd = to_td043mtea1_device(panel); > > + > > + if (!lcd->spi_suspended) > > + td043mtea1_power_off(lcd); > > + > > + return 0; > > +} > > + > > +static int td043mtea1_enable(struct drm_panel *panel) > > +{ > > + struct td043mtea1_device *lcd = to_td043mtea1_device(panel); > > + int ret; > > + > > + /* > > + * If we are resuming from system suspend, SPI might not be enabled > > + * yet, so we'll program the LCD from SPI PM resume callback. > > + */ > > + if (lcd->spi_suspended) > > + return 0; > > I do not recall this is needed in other panel drivers, so look at what > other spi based panels do here. > I think this is something that today is not required. The problem here is that the display controller may be resumed before the SPI bus. Has that been solved somewhere in core code ? > > + > > + ret = td043mtea1_power_on(lcd); > > + if (ret) { > > + dev_err(&lcd->spi->dev, "%s: power on failed (%d)\n", > > + __func__, ret); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static const struct drm_display_mode td043mtea1_mode = { > > + .clock = 36000, > > + .hdisplay = 800, > > + .hsync_start = 800 + 68, > > + .hsync_end = 800 + 68 + 1, > > + .htotal = 800 + 68 + 1 + 214, > > + .vdisplay = 480, > > + .vsync_start = 480 + 39, > > + .vsync_end = 480 + 39 + 1, > > + .vtotal = 480 + 39 + 1 + 34, > > + .vrefresh = 60, > > + .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED, > > + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, > > +}; > > height_mm, width_mm > > > + > > +static int td043mtea1_get_modes(struct drm_panel *panel) > > +{ > > + struct drm_connector *connector = panel->connector; > > + struct drm_display_mode *mode; > > + > > + mode = drm_mode_duplicate(panel->drm, &td043mtea1_mode); > > + if (!mode) > > + return -ENOMEM; > > + > > + drm_mode_set_name(mode); > > + drm_mode_probed_add(connector, mode); > > + > > + connector->display_info.width_mm = 94; > > + connector->display_info.height_mm = 56; > > + /* > > + * FIXME: According to the datasheet sync signals are sampled on the > > + * rising edge of the clock, but the code running on the OMAP3 Pandora > > + * indicates sampling on the falling edge. This should be tested on a > > + * real device. > > + */ > > + connector->display_info.bus_flags = DRM_BUS_FLAG_DE_HIGH > > + | DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE > > + | DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE; > > + > > + return 1; > > +} > > + > > +static const struct drm_panel_funcs td043mtea1_funcs = { > > + .disable = td043mtea1_disable, > > + .enable = td043mtea1_enable, > > + .get_modes = td043mtea1_get_modes, > > +}; > > + > > +/* ----------------------------------------------------------------------------- > > + * Power Management, Probe and Remove > > + */ > > + > > +#ifdef CONFIG_PM_SLEEP > > +static int td043mtea1_suspend(struct device *dev) > > +{ > > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > > + > > + if (lcd->powered_on) { > > + td043mtea1_power_off(lcd); > > + lcd->powered_on = true; > > + } > > + > > + lcd->spi_suspended = true; > > + > > + return 0; > > +} > > + > > +static int td043mtea1_resume(struct device *dev) > > +{ > > + struct td043mtea1_device *lcd = dev_get_drvdata(dev); > > + int ret; > > + > > + lcd->spi_suspended = false; > > + > > + if (lcd->powered_on) { > > + lcd->powered_on = false; > > + ret = td043mtea1_power_on(lcd); > > + if (ret) > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static SIMPLE_DEV_PM_OPS(td043mtea1_pm_ops, td043mtea1_suspend, > > + td043mtea1_resume); > > +#endif > > + > > +static int td043mtea1_probe(struct spi_device *spi) > > +{ > > + struct td043mtea1_device *lcd; > > + int ret; > > + > > + lcd = devm_kzalloc(&spi->dev, sizeof(*lcd), GFP_KERNEL); > > + if (lcd == NULL) > > + return -ENOMEM; > > + > > + spi_set_drvdata(spi, lcd); > > + lcd->spi = spi; > > + lcd->mode = TPO_R02_MODE_800x480; > > + memcpy(lcd->gamma, td043mtea1_def_gamma, sizeof(lcd->gamma)); > > + > > + lcd->vcc_reg = devm_regulator_get(&spi->dev, "vcc"); > > + if (IS_ERR(lcd->vcc_reg)) { > > + dev_err(&spi->dev, "failed to get VCC regulator\n"); > > + return PTR_ERR(lcd->vcc_reg); > > + } > > + > > + lcd->reset_gpio = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_HIGH); > > + if (IS_ERR(lcd->reset_gpio)) { > > + dev_err(&spi->dev, "failed to get reset GPIO\n"); > > + return PTR_ERR(lcd->reset_gpio); > > + } > > + > > + spi->bits_per_word = 16; > > + spi->mode = SPI_MODE_0; > > + > > + ret = spi_setup(spi); > > + if (ret < 0) { > > + dev_err(&spi->dev, "failed to setup SPI: %d\n", ret); > > + return ret; > > + } > > + > > + ret = sysfs_create_group(&spi->dev.kobj, &td043mtea1_attr_group); > > + if (ret < 0) { > > + dev_err(&spi->dev, "failed to create sysfs files\n"); > > + return ret; > > + } > > + > > + drm_panel_init(&lcd->panel); > > + lcd->panel.dev = &lcd->spi->dev; > > + lcd->panel.funcs = &td043mtea1_funcs; > > + > > + ret = drm_panel_add(&lcd->panel); > > + if (ret < 0) { > > + sysfs_remove_group(&spi->dev.kobj, &td043mtea1_attr_group); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int td043mtea1_remove(struct spi_device *spi) > > +{ > > + struct td043mtea1_device *lcd = spi_get_drvdata(spi); > > + > > + drm_panel_remove(&lcd->panel); > > + td043mtea1_disable(&lcd->panel); > > + > > + sysfs_remove_group(&spi->dev.kobj, &td043mtea1_attr_group); > > + > > + return 0; > > +} > > + > > +static const struct of_device_id td043mtea1_of_match[] = { > > + { .compatible = "tpo,td043mtea1", }, > > + {}, > > { /* sentinel */ },
Hi Laurent. > > > +static int td043mtea1_disable(struct drm_panel *panel) > > > +{ > > > + struct td043mtea1_device *lcd = to_td043mtea1_device(panel); > > > + > > > + if (!lcd->spi_suspended) > > > + td043mtea1_power_off(lcd); > > > + > > > + return 0; > > > +} > > > + > > > +static int td043mtea1_enable(struct drm_panel *panel) > > > +{ > > > + struct td043mtea1_device *lcd = to_td043mtea1_device(panel); > > > + int ret; > > > + > > > + /* > > > + * If we are resuming from system suspend, SPI might not be enabled > > > + * yet, so we'll program the LCD from SPI PM resume callback. > > > + */ > > > + if (lcd->spi_suspended) > > > + return 0; > > > > I do not recall this is needed in other panel drivers, so look at what > > other spi based panels do here. > > I think this is something that today is not required. > > The problem here is that the display controller may be resumed before > the SPI bus. Has that been solved somewhere in core code ? I dunno. So the conclusion is to keep it as is, and any change will wait until someone with HW can step up. As for all your other feedback to this and the other panel drivers they did not trigger any repsonse from me. Looks forward for next iteration of this nice set of patches. Can we maybe get the panel drivers in before some of the infrastructure work? I know the users then may come a bit later, but I think thats OK. Sam
Hi Sam, On Fri, Aug 09, 2019 at 03:33:08PM +0200, Sam Ravnborg wrote: > Hi Laurent. > > > > > +static int td043mtea1_disable(struct drm_panel *panel) > > > > +{ > > > > + struct td043mtea1_device *lcd = to_td043mtea1_device(panel); > > > > + > > > > + if (!lcd->spi_suspended) > > > > + td043mtea1_power_off(lcd); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int td043mtea1_enable(struct drm_panel *panel) > > > > +{ > > > > + struct td043mtea1_device *lcd = to_td043mtea1_device(panel); > > > > + int ret; > > > > + > > > > + /* > > > > + * If we are resuming from system suspend, SPI might not be enabled > > > > + * yet, so we'll program the LCD from SPI PM resume callback. > > > > + */ > > > > + if (lcd->spi_suspended) > > > > + return 0; > > > > > > I do not recall this is needed in other panel drivers, so look at what > > > other spi based panels do here. > > > I think this is something that today is not required. > > > > The problem here is that the display controller may be resumed before > > the SPI bus. Has that been solved somewhere in core code ? > > I dunno. So the conclusion is to keep it as is, and any change > will wait until someone with HW can step up. Great, thanks. > As for all your other feedback to this and the other panel drivers > they did not trigger any repsonse from me. > > Looks forward for next iteration of this nice set of patches. > Can we maybe get the panel drivers in before some of the infrastructure > work? > I know the users then may come a bit later, but I think thats OK. Sure. I'll post the next version soon.
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index b7099d211061..8f3660c73044 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -312,6 +312,13 @@ config DRM_PANEL_TPO_TD028TTEC1 Say Y here if you want to enable support for TPO TD028TTEC1 480x640 2.8" panel. +config DRM_PANEL_TPO_TD043MTEA1 + tristate "TPO TD043MTEA1 panel driver" + depends on GPIOLIB && OF && REGULATOR && SPI + help + Say Y here if you want to enable support for TPO TD043MTEA1 800x480 + 4.3" panel. + config DRM_PANEL_TPO_TPG110 tristate "TPO TPG 800x400 panel" depends on OF && SPI && GPIOLIB diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index a1d4de64c0b1..bf76fcea7d22 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -33,5 +33,6 @@ obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) += panel-sitronix-st7701.o obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o obj-$(CONFIG_DRM_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o +obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o diff --git a/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c new file mode 100644 index 000000000000..6b17e47582b8 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c @@ -0,0 +1,510 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Toppology TD043MTEA1 Panel Driver + * + * Copyright (C) 2019 Texas Instruments Incorporated + * + * Based on the omapdrm-specific panel-tpo-td043mtea1 driver + * + * Author: Gražvydas Ignotas <notasas@gmail.com> + */ + +#include <linux/delay.h> +#include <linux/module.h> +#include <linux/regulator/consumer.h> +#include <linux/spi/spi.h> + +#include <drm/drm_connector.h> +#include <drm/drm_modes.h> +#include <drm/drm_panel.h> + +#define TPO_R02_MODE(x) ((x) & 7) +#define TPO_R02_MODE_800x480 7 +#define TPO_R02_NCLK_RISING BIT(3) +#define TPO_R02_HSYNC_HIGH BIT(4) +#define TPO_R02_VSYNC_HIGH BIT(5) + +#define TPO_R03_NSTANDBY BIT(0) +#define TPO_R03_EN_CP_CLK BIT(1) +#define TPO_R03_EN_VGL_PUMP BIT(2) +#define TPO_R03_EN_PWM BIT(3) +#define TPO_R03_DRIVING_CAP_100 BIT(4) +#define TPO_R03_EN_PRE_CHARGE BIT(6) +#define TPO_R03_SOFTWARE_CTL BIT(7) + +#define TPO_R04_NFLIP_H BIT(0) +#define TPO_R04_NFLIP_V BIT(1) +#define TPO_R04_CP_CLK_FREQ_1H BIT(2) +#define TPO_R04_VGL_FREQ_1H BIT(4) + +#define TPO_R03_VAL_NORMAL \ + (TPO_R03_NSTANDBY | TPO_R03_EN_CP_CLK | TPO_R03_EN_VGL_PUMP | \ + TPO_R03_EN_PWM | TPO_R03_DRIVING_CAP_100 | TPO_R03_EN_PRE_CHARGE | \ + TPO_R03_SOFTWARE_CTL) + +#define TPO_R03_VAL_STANDBY \ + (TPO_R03_DRIVING_CAP_100 | TPO_R03_EN_PRE_CHARGE | \ + TPO_R03_SOFTWARE_CTL) + +static const u16 td043mtea1_def_gamma[12] = { + 105, 315, 381, 431, 490, 537, 579, 686, 780, 837, 880, 1023 +}; + +struct td043mtea1_device { + struct drm_panel panel; + + struct spi_device *spi; + struct regulator *vcc_reg; + struct gpio_desc *reset_gpio; + + unsigned int mode; + u16 gamma[12]; + bool vmirror; + bool powered_on; + bool spi_suspended; + bool power_on_resume; +}; + +#define to_td043mtea1_device(p) container_of(p, struct td043mtea1_device, panel) + +/* ----------------------------------------------------------------------------- + * Hardware Access + */ + +static int td043mtea1_write(struct td043mtea1_device *lcd, u8 addr, u8 value) +{ + struct spi_message msg; + struct spi_transfer xfer; + u16 data; + int ret; + + spi_message_init(&msg); + + memset(&xfer, 0, sizeof(xfer)); + + data = ((u16)addr << 10) | (1 << 8) | value; + xfer.tx_buf = &data; + xfer.bits_per_word = 16; + xfer.len = 2; + spi_message_add_tail(&xfer, &msg); + + ret = spi_sync(lcd->spi, &msg); + if (ret < 0) + dev_warn(&lcd->spi->dev, "failed to write to LCD reg (%d)\n", + ret); + + return ret; +} + +static void td043mtea1_write_gamma(struct td043mtea1_device *lcd) +{ + const u16 *gamma = lcd->gamma; + unsigned int i; + u8 val; + + /* gamma bits [9:8] */ + for (val = i = 0; i < 4; i++) + val |= (gamma[i] & 0x300) >> ((i + 1) * 2); + td043mtea1_write(lcd, 0x11, val); + + for (val = i = 0; i < 4; i++) + val |= (gamma[i+4] & 0x300) >> ((i + 1) * 2); + td043mtea1_write(lcd, 0x12, val); + + for (val = i = 0; i < 4; i++) + val |= (gamma[i+8] & 0x300) >> ((i + 1) * 2); + td043mtea1_write(lcd, 0x13, val); + + /* gamma bits [7:0] */ + for (val = i = 0; i < 12; i++) + td043mtea1_write(lcd, 0x14 + i, gamma[i] & 0xff); +} + +static int td043mtea1_write_mirror(struct td043mtea1_device *lcd) +{ + u8 reg4 = TPO_R04_NFLIP_H | TPO_R04_NFLIP_V | + TPO_R04_CP_CLK_FREQ_1H | TPO_R04_VGL_FREQ_1H; + if (lcd->vmirror) + reg4 &= ~TPO_R04_NFLIP_V; + + return td043mtea1_write(lcd, 4, reg4); +} + +static int td043mtea1_power_on(struct td043mtea1_device *lcd) +{ + int ret; + + if (lcd->powered_on) + return 0; + + ret = regulator_enable(lcd->vcc_reg); + if (ret < 0) + return ret; + + /* Wait for the panel to stabilize. */ + msleep(160); + + gpiod_set_value(lcd->reset_gpio, 0); + + td043mtea1_write(lcd, 2, TPO_R02_MODE(lcd->mode) | TPO_R02_NCLK_RISING); + td043mtea1_write(lcd, 3, TPO_R03_VAL_NORMAL); + td043mtea1_write(lcd, 0x20, 0xf0); + td043mtea1_write(lcd, 0x21, 0xf0); + td043mtea1_write_mirror(lcd); + td043mtea1_write_gamma(lcd); + + lcd->powered_on = true; + + return 0; +} + +static void td043mtea1_power_off(struct td043mtea1_device *lcd) +{ + if (!lcd->powered_on) + return; + + td043mtea1_write(lcd, 3, TPO_R03_VAL_STANDBY | TPO_R03_EN_PWM); + + gpiod_set_value(lcd->reset_gpio, 1); + + /* wait for at least 2 vsyncs before cutting off power */ + msleep(50); + + td043mtea1_write(lcd, 3, TPO_R03_VAL_STANDBY); + + regulator_disable(lcd->vcc_reg); + + lcd->powered_on = false; +} + +/* ----------------------------------------------------------------------------- + * sysfs + */ + +static ssize_t vmirror_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct td043mtea1_device *lcd = dev_get_drvdata(dev); + + return snprintf(buf, PAGE_SIZE, "%d\n", lcd->vmirror); +} + +static ssize_t vmirror_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct td043mtea1_device *lcd = dev_get_drvdata(dev); + int val; + int ret; + + ret = kstrtoint(buf, 0, &val); + if (ret < 0) + return ret; + + lcd->vmirror = !!val; + + ret = td043mtea1_write_mirror(lcd); + if (ret < 0) + return ret; + + return count; +} + +static ssize_t mode_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct td043mtea1_device *lcd = dev_get_drvdata(dev); + + return snprintf(buf, PAGE_SIZE, "%d\n", lcd->mode); +} + +static ssize_t mode_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct td043mtea1_device *lcd = dev_get_drvdata(dev); + long val; + int ret; + + ret = kstrtol(buf, 0, &val); + if (ret != 0 || val & ~7) + return -EINVAL; + + lcd->mode = val; + + val |= TPO_R02_NCLK_RISING; + td043mtea1_write(lcd, 2, val); + + return count; +} + +static ssize_t gamma_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct td043mtea1_device *lcd = dev_get_drvdata(dev); + ssize_t len = 0; + unsigned int i; + int ret; + + for (i = 0; i < ARRAY_SIZE(lcd->gamma); i++) { + ret = snprintf(buf + len, PAGE_SIZE - len, "%u ", + lcd->gamma[i]); + if (ret < 0) + return ret; + len += ret; + } + buf[len - 1] = '\n'; + + return len; +} + +static ssize_t gamma_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct td043mtea1_device *lcd = dev_get_drvdata(dev); + unsigned int g[12]; + unsigned int i; + int ret; + + ret = sscanf(buf, "%u %u %u %u %u %u %u %u %u %u %u %u", + &g[0], &g[1], &g[2], &g[3], &g[4], &g[5], + &g[6], &g[7], &g[8], &g[9], &g[10], &g[11]); + if (ret != 12) + return -EINVAL; + + for (i = 0; i < 12; i++) + lcd->gamma[i] = g[i]; + + td043mtea1_write_gamma(lcd); + + return count; +} + +static DEVICE_ATTR_RW(vmirror); +static DEVICE_ATTR_RW(mode); +static DEVICE_ATTR_RW(gamma); + +static struct attribute *td043mtea1_attrs[] = { + &dev_attr_vmirror.attr, + &dev_attr_mode.attr, + &dev_attr_gamma.attr, + NULL, +}; + +static const struct attribute_group td043mtea1_attr_group = { + .attrs = td043mtea1_attrs, +}; + +/* ----------------------------------------------------------------------------- + * Bridge Operations + */ + +static int td043mtea1_disable(struct drm_panel *panel) +{ + struct td043mtea1_device *lcd = to_td043mtea1_device(panel); + + if (!lcd->spi_suspended) + td043mtea1_power_off(lcd); + + return 0; +} + +static int td043mtea1_enable(struct drm_panel *panel) +{ + struct td043mtea1_device *lcd = to_td043mtea1_device(panel); + int ret; + + /* + * If we are resuming from system suspend, SPI might not be enabled + * yet, so we'll program the LCD from SPI PM resume callback. + */ + if (lcd->spi_suspended) + return 0; + + ret = td043mtea1_power_on(lcd); + if (ret) { + dev_err(&lcd->spi->dev, "%s: power on failed (%d)\n", + __func__, ret); + return ret; + } + + return 0; +} + +static const struct drm_display_mode td043mtea1_mode = { + .clock = 36000, + .hdisplay = 800, + .hsync_start = 800 + 68, + .hsync_end = 800 + 68 + 1, + .htotal = 800 + 68 + 1 + 214, + .vdisplay = 480, + .vsync_start = 480 + 39, + .vsync_end = 480 + 39 + 1, + .vtotal = 480 + 39 + 1 + 34, + .vrefresh = 60, + .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED, + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, +}; + +static int td043mtea1_get_modes(struct drm_panel *panel) +{ + struct drm_connector *connector = panel->connector; + struct drm_display_mode *mode; + + mode = drm_mode_duplicate(panel->drm, &td043mtea1_mode); + if (!mode) + return -ENOMEM; + + drm_mode_set_name(mode); + drm_mode_probed_add(connector, mode); + + connector->display_info.width_mm = 94; + connector->display_info.height_mm = 56; + /* + * FIXME: According to the datasheet sync signals are sampled on the + * rising edge of the clock, but the code running on the OMAP3 Pandora + * indicates sampling on the falling edge. This should be tested on a + * real device. + */ + connector->display_info.bus_flags = DRM_BUS_FLAG_DE_HIGH + | DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE + | DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE; + + return 1; +} + +static const struct drm_panel_funcs td043mtea1_funcs = { + .disable = td043mtea1_disable, + .enable = td043mtea1_enable, + .get_modes = td043mtea1_get_modes, +}; + +/* ----------------------------------------------------------------------------- + * Power Management, Probe and Remove + */ + +#ifdef CONFIG_PM_SLEEP +static int td043mtea1_suspend(struct device *dev) +{ + struct td043mtea1_device *lcd = dev_get_drvdata(dev); + + if (lcd->powered_on) { + td043mtea1_power_off(lcd); + lcd->powered_on = true; + } + + lcd->spi_suspended = true; + + return 0; +} + +static int td043mtea1_resume(struct device *dev) +{ + struct td043mtea1_device *lcd = dev_get_drvdata(dev); + int ret; + + lcd->spi_suspended = false; + + if (lcd->powered_on) { + lcd->powered_on = false; + ret = td043mtea1_power_on(lcd); + if (ret) + return ret; + } + + return 0; +} + +static SIMPLE_DEV_PM_OPS(td043mtea1_pm_ops, td043mtea1_suspend, + td043mtea1_resume); +#endif + +static int td043mtea1_probe(struct spi_device *spi) +{ + struct td043mtea1_device *lcd; + int ret; + + lcd = devm_kzalloc(&spi->dev, sizeof(*lcd), GFP_KERNEL); + if (lcd == NULL) + return -ENOMEM; + + spi_set_drvdata(spi, lcd); + lcd->spi = spi; + lcd->mode = TPO_R02_MODE_800x480; + memcpy(lcd->gamma, td043mtea1_def_gamma, sizeof(lcd->gamma)); + + lcd->vcc_reg = devm_regulator_get(&spi->dev, "vcc"); + if (IS_ERR(lcd->vcc_reg)) { + dev_err(&spi->dev, "failed to get VCC regulator\n"); + return PTR_ERR(lcd->vcc_reg); + } + + lcd->reset_gpio = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(lcd->reset_gpio)) { + dev_err(&spi->dev, "failed to get reset GPIO\n"); + return PTR_ERR(lcd->reset_gpio); + } + + spi->bits_per_word = 16; + spi->mode = SPI_MODE_0; + + ret = spi_setup(spi); + if (ret < 0) { + dev_err(&spi->dev, "failed to setup SPI: %d\n", ret); + return ret; + } + + ret = sysfs_create_group(&spi->dev.kobj, &td043mtea1_attr_group); + if (ret < 0) { + dev_err(&spi->dev, "failed to create sysfs files\n"); + return ret; + } + + drm_panel_init(&lcd->panel); + lcd->panel.dev = &lcd->spi->dev; + lcd->panel.funcs = &td043mtea1_funcs; + + ret = drm_panel_add(&lcd->panel); + if (ret < 0) { + sysfs_remove_group(&spi->dev.kobj, &td043mtea1_attr_group); + return ret; + } + + return 0; +} + +static int td043mtea1_remove(struct spi_device *spi) +{ + struct td043mtea1_device *lcd = spi_get_drvdata(spi); + + drm_panel_remove(&lcd->panel); + td043mtea1_disable(&lcd->panel); + + sysfs_remove_group(&spi->dev.kobj, &td043mtea1_attr_group); + + return 0; +} + +static const struct of_device_id td043mtea1_of_match[] = { + { .compatible = "tpo,td043mtea1", }, + {}, +}; + +MODULE_DEVICE_TABLE(of, td043mtea1_of_match); + +static struct spi_driver td043mtea1_driver = { + .probe = td043mtea1_probe, + .remove = td043mtea1_remove, + .driver = { + .name = "panel-tpo-td043mtea1", +#ifdef CONFIG_PM_SLEEP + .pm = &td043mtea1_pm_ops, +#endif + .of_match_table = td043mtea1_of_match, + }, +}; + +module_spi_driver(td043mtea1_driver); + +MODULE_ALIAS("spi:tpo,td043mtea1"); +MODULE_AUTHOR("Gražvydas Ignotas <notasas@gmail.com>"); +MODULE_DESCRIPTION("TPO TD043MTEA1 Panel Driver"); +MODULE_LICENSE("GPL");
This panel is used on the OMAP3 Pandora. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/gpu/drm/panel/Kconfig | 7 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-tpo-td043mtea1.c | 510 +++++++++++++++++++ 3 files changed, 518 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-tpo-td043mtea1.c