This adds device tree bindings for the Samsung LMS397KF04 RGB DPI display panel.
Cc: devicetree@vger.kernel.org Signed-off-by: Linus Walleij linus.walleij@linaro.org --- .../display/panel/samsung,lms397kf04.yaml | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,lms397kf04.yaml
diff --git a/Documentation/devicetree/bindings/display/panel/samsung,lms397kf04.yaml b/Documentation/devicetree/bindings/display/panel/samsung,lms397kf04.yaml new file mode 100644 index 000000000000..4cb75a5f2e3a --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/samsung,lms397kf04.yaml @@ -0,0 +1,74 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/samsung,lms397kf04.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Samsung LMS397KF04 display panel + +description: The datasheet claims this is based around a display controller + named DB7430 with a separate backlight controller. + +maintainers: + - Linus Walleij linus.walleij@linaro.org + +allOf: + - $ref: panel-common.yaml# + +properties: + compatible: + const: samsung,lms397kf04 + + reg: true + + reset-gpios: true + + vci-supply: + description: regulator that supplies the VCI analog voltage + usually around 3.0 V + + vccio-supply: + description: regulator that supplies the VCCIO voltage usually + around 1.8 V + + backlight: true + + spi-max-frequency: + $ref: /schemas/types.yaml#/definitions/uint32 + description: inherited as a SPI client node, the datasheet specifies + maximum 300 ns minimum cycle which gives around 3 MHz max frequency + maximum: 3000000 + + port: true + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + panel@0 { + compatible = "samsung,lms397kf04"; + spi-max-frequency = <3000000>; + reg = <0>; + vci-supply = <&lcd_3v0_reg>; + vccio-supply = <&lcd_1v8_reg>; + reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>; + backlight = <&ktd259>; + + port { + panel_in: endpoint { + remote-endpoint = <&display_out>; + }; + }; + }; + }; + +...
This adds a new driver for the Samsung LMS397KF04 DPI display controlled over SPI.
Signed-off-by: Linus Walleij linus.walleij@linaro.org --- MAINTAINERS | 7 + drivers/gpu/drm/panel/Kconfig | 8 + drivers/gpu/drm/panel/Makefile | 1 + .../gpu/drm/panel/panel-samsung-lms397kf04.c | 421 ++++++++++++++++++ 4 files changed, 437 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-samsung-lms397kf04.c
diff --git a/MAINTAINERS b/MAINTAINERS index d92f85ca831d..6d32432adebe 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5731,6 +5731,13 @@ S: Maintained F: Documentation/devicetree/bindings/display/panel/raydium,rm67191.yaml F: drivers/gpu/drm/panel/panel-raydium-rm67191.c
+DRM DRIVER FOR SAMSUNG LMS397KF04 PANELS +M: Linus Walleij linus.walleij@linaro.org +S: Maintained +T: git git://anongit.freedesktop.org/drm/drm-misc +F: Documentation/devicetree/bindings/display/panel/samsung,lms397kf04.yaml +F: drivers/gpu/drm/panel/panel-samsung-lms397kf04.c + DRM DRIVER FOR SITRONIX ST7703 PANELS M: Guido Günther agx@sigxcpu.org R: Purism Kernel Team kernel@puri.sm diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 4894913936e9..8e24a92ad9ce 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -348,6 +348,14 @@ config DRM_PANEL_SAMSUNG_S6D16D0 depends on DRM_MIPI_DSI select VIDEOMODE_HELPERS
+config DRM_PANEL_SAMSUNG_LMS397KF04 + tristate "Samsung LMS397KF04 DPI panel" + depends on OF && SPI && GPIOLIB + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y here if you want to enable support for the Samsung + LMS397KF04 480x800 DPI panel. + config DRM_PANEL_SAMSUNG_S6E3HA2 tristate "Samsung S6E3HA2 DSI video mode panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index cae4d976c069..de23d00937db 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -33,6 +33,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o +obj-$(CONFIG_DRM_PANEL_SAMSUNG_LMS397KF04) += panel-samsung-lms397kf04.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o diff --git a/drivers/gpu/drm/panel/panel-samsung-lms397kf04.c b/drivers/gpu/drm/panel/panel-samsung-lms397kf04.c new file mode 100644 index 000000000000..41290cadc351 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-samsung-lms397kf04.c @@ -0,0 +1,421 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Panel driver for the Samsung LMS397KF04 480x800 DPI RGB panel. + * According to the data sheet the display controller is called DB7430 + * Linus Walleij linus.walleij@linaro.org + */ +#include <drm/drm_modes.h> +#include <drm/drm_panel.h> + +#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/media-bus-format.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regulator/consumer.h> +#include <linux/spi/spi.h> + +#include <video/mipi_display.h> + +#define LMS397_MANUFACTURER_CMD 0xb0 +#define LMS397_UNKNOWN_B4 0xb4 +#define LMS397_USER_SELECT 0xb5 +#define LMS397_UNKNOWN_B7 0xb7 +#define LMS397_UNKNOWN_B8 0xb8 +#define LMS397_PANEL_DRIVING 0xc0 +#define LMS397_SOURCE_CONTROL 0xc1 +#define LMS397_GATE_INTERFACE 0xc4 +#define LMS397_DISPLAY_H_TIMING 0xc5 +#define LMS397_RGB_SYNC_OPTION 0xc6 +#define LMS397_GAMMA_SET_RED 0xc8 +#define LMS397_GAMMA_SET_GREEN 0xc9 +#define LMS397_GAMMA_SET_BLUE 0xca +#define LMS397_BIAS_CURRENT_CTRL 0xd1 +#define LMS397_DDV_CTRL 0xd2 +#define LMS397_GAMMA_CTRL_REF 0xd3 +#define LMS397_UNKNOWN_D4 0xd4 +#define LMS397_DCDC_CTRL 0xd5 +#define LMS397_VCL_CTRL 0xd6 +#define LMS397_UNKNOWN_F8 0xf8 +#define LMS397_UNKNOWN_FC 0xfc + +#define DATA_MASK 0x100 + +/** + * struct lms397kf04 - state container for the LMS397kf04 panel + */ +struct lms397kf04 { + /** + * @dev: the container device + */ + struct device *dev; + /** + * @spi: the corresponding SPI device + */ + struct spi_device *spi; + /** + * @panel: the DRM panel instance for this device + */ + struct drm_panel panel; + /** + * @width: the width of this panel in mm + */ + u32 width; + /** + * @height: the height of this panel in mm + */ + u32 height; + /** + * @reset: reset GPIO line + */ + struct gpio_desc *reset; + /** + * @regulators: VCCIO and VIO supply regulators + */ + struct regulator_bulk_data regulators[2]; +}; + +static const struct drm_display_mode lms397kf04_mode = { + /* + * 31 ns period min (htotal*vtotal*vrefresh)/1000 + * gives a Vrefresh of ~71 Hz. + */ + .clock = 32258, + .hdisplay = 480, + .hsync_start = 480 + 10, + .hsync_end = 480 + 10 + 4, + .htotal = 480 + 10 + 4 + 40, + .vdisplay = 800, + .vsync_start = 800 + 6, + .vsync_end = 800 + 6 + 1, + .vtotal = 800 + 6 + 1 + 7, + .width_mm = 53, + .height_mm = 87, + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC, +}; + +static inline struct lms397kf04 *to_lms397kf04(struct drm_panel *panel) +{ + return container_of(panel, struct lms397kf04, panel); +} + +static int lms397kf04_write_word(struct lms397kf04 *lms, u16 data) +{ + /* SPI buffers are always in CPU order */ + return spi_write(lms->spi, &data, 2); +} + +static int lms397kf04_dcs_write(struct lms397kf04 *lms, const u8 *data, size_t len) +{ + int ret; + + dev_dbg(lms->dev, "SPI writing dcs seq: %*ph\n", (int)len, data); + + /* + * This sends 9 bits with the first bit (bit 8) set to 0 + * This indicates that this is a command. Anything after the + * command is data. + */ + ret = lms397kf04_write_word(lms, *data); + + while (!ret && --len) { + ++data; + /* This sends 9 bits with the first bit (bit 8) set to 1 */ + ret = lms397kf04_write_word(lms, *data | DATA_MASK); + } + + if (ret) { + dev_err(lms->dev, "SPI error %d writing dcs seq: %*ph\n", ret, + (int)len, data); + } + + return ret; +} + +#define lms397kf04_dcs_write_seq_static(ctx, seq ...) \ + ({ \ + static const u8 d[] = { seq }; \ + lms397kf04_dcs_write(ctx, d, ARRAY_SIZE(d)); \ + }) + +static int lms397kf04_power_on(struct lms397kf04 *lms) +{ + int ret; + + /* Power up */ + ret = regulator_bulk_enable(ARRAY_SIZE(lms->regulators), + lms->regulators); + if (ret) { + dev_err(lms->dev, "failed to enable regulators: %d\n", ret); + return ret; + } + msleep(50); + + /* Assert reset >=1 ms */ + gpiod_set_value_cansleep(lms->reset, 1); + msleep(1); + /* De-assert reset */ + gpiod_set_value_cansleep(lms->reset, 0); + /* Wait >= 10 ms */ + msleep(10); + dev_info(lms->dev, "de-asserted RESET\n"); + + /* + * This is set to 0x0a (RGB/BGR order + horizontal flip) in order + * to make the display behave normally. If this is not set the displays + * normal output behaviour is horizontally flipped and BGR ordered. Do + * it twice because the first message doesn't always "take". + */ + lms397kf04_dcs_write_seq_static(lms, MIPI_DCS_SET_ADDRESS_MODE, 0x0a); + lms397kf04_dcs_write_seq_static(lms, MIPI_DCS_SET_ADDRESS_MODE, 0x0a); + /* Called "Access protection off" in vendor code */ + lms397kf04_dcs_write_seq_static(lms, LMS397_MANUFACTURER_CMD, 0x00); + lms397kf04_dcs_write_seq_static(lms, LMS397_PANEL_DRIVING, 0x28, 0x08); + lms397kf04_dcs_write_seq_static(lms, LMS397_SOURCE_CONTROL, + 0x01, 0x30, 0x15, 0x05, 0x22); + lms397kf04_dcs_write_seq_static(lms, LMS397_GATE_INTERFACE, + 0x10, 0x01, 0x00); + lms397kf04_dcs_write_seq_static(lms, LMS397_DISPLAY_H_TIMING, + 0x06, 0x55, 0x03, 0x07, 0x0b, + 0x33, 0x00, 0x01, 0x03); + /* + * 0x00 in datasheet 0x01 in vendor code 0x00, it seems 0x01 means + * DE active high and 0x00 means DE active low. + */ + lms397kf04_dcs_write_seq_static(lms, LMS397_RGB_SYNC_OPTION, 0x01); + lms397kf04_dcs_write_seq_static(lms, LMS397_GAMMA_SET_RED, + /* R positive gamma */ 0x00, + 0x0A, 0x31, 0x3B, 0x4E, 0x58, 0x59, 0x5B, 0x58, 0x5E, 0x62, + 0x60, 0x61, 0x5E, 0x62, 0x55, 0x55, 0x7F, 0x08, + /* R negative gamma */ 0x00, + 0x0A, 0x31, 0x3B, 0x4E, 0x58, 0x59, 0x5B, 0x58, 0x5E, 0x62, + 0x60, 0x61, 0x5E, 0x62, 0x55, 0x55, 0x7F, 0x08); + lms397kf04_dcs_write_seq_static(lms, LMS397_GAMMA_SET_GREEN, + /* G positive gamma */ 0x00, + 0x25, 0x15, 0x28, 0x3D, 0x4A, 0x48, 0x4C, 0x4A, 0x52, 0x59, + 0x59, 0x5B, 0x56, 0x60, 0x5D, 0x55, 0x7F, 0x0A, + /* G negative gamma */ 0x00, + 0x25, 0x15, 0x28, 0x3D, 0x4A, 0x48, 0x4C, 0x4A, 0x52, 0x59, + 0x59, 0x5B, 0x56, 0x60, 0x5D, 0x55, 0x7F, 0x0A); + lms397kf04_dcs_write_seq_static(lms, LMS397_GAMMA_SET_BLUE, + /* B positive gamma */ 0x00, + 0x48, 0x10, 0x1F, 0x2F, 0x35, 0x38, 0x3D, 0x3C, 0x45, 0x4D, + 0x4E, 0x52, 0x51, 0x60, 0x7F, 0x7E, 0x7F, 0x0C, + /* B negative gamma */ 0x00, + 0x48, 0x10, 0x1F, 0x2F, 0x35, 0x38, 0x3D, 0x3C, 0x45, 0x4D, + 0x4E, 0x52, 0x51, 0x60, 0x7F, 0x7E, 0x7F, 0x0C); + lms397kf04_dcs_write_seq_static(lms, LMS397_BIAS_CURRENT_CTRL, + 0x33, 0x13); + lms397kf04_dcs_write_seq_static(lms, LMS397_DDV_CTRL, + 0x11, 0x00, 0x00); + lms397kf04_dcs_write_seq_static(lms, LMS397_GAMMA_CTRL_REF, + 0x50, 0x50); + lms397kf04_dcs_write_seq_static(lms, LMS397_DCDC_CTRL, + 0x2f, 0x11, 0x1e, 0x46); + lms397kf04_dcs_write_seq_static(lms, LMS397_VCL_CTRL, + 0x11, 0x0a); + + return 0; +} + +static void lms397kf04_power_off(struct lms397kf04 *lms) +{ + /* Go into RESET and disable regulators */ + gpiod_set_value_cansleep(lms->reset, 1); + regulator_bulk_disable(ARRAY_SIZE(lms->regulators), + lms->regulators); +} + +static int lms397kf04_unprepare(struct drm_panel *panel) +{ + struct lms397kf04 *lms = to_lms397kf04(panel); + + lms397kf04_power_off(lms); + + return 0; +} + +static int lms397kf04_disable(struct drm_panel *panel) +{ + struct lms397kf04 *lms = to_lms397kf04(panel); + + lms397kf04_dcs_write_seq_static(lms, MIPI_DCS_SET_DISPLAY_OFF); + msleep(25); + lms397kf04_dcs_write_seq_static(lms, MIPI_DCS_ENTER_SLEEP_MODE); + msleep(120); + + return 0; +} + +static int lms397kf04_prepare(struct drm_panel *panel) +{ + struct lms397kf04 *lms = to_lms397kf04(panel); + int ret; + + ret = lms397kf04_power_on(lms); + if (ret) + return ret; + + return 0; +} + +static int lms397kf04_enable(struct drm_panel *panel) +{ + struct lms397kf04 *lms = to_lms397kf04(panel); + + /* Exit sleep mode */ + lms397kf04_dcs_write_seq_static(lms, MIPI_DCS_EXIT_SLEEP_MODE); + msleep(20); + + /* NVM (non-volatile memory) load sequence */ + lms397kf04_dcs_write_seq_static(lms, LMS397_UNKNOWN_D4, + 0x52, 0x5e); + lms397kf04_dcs_write_seq_static(lms, LMS397_UNKNOWN_F8, + 0x01, 0xf5, 0xf2, 0x71, 0x44); + lms397kf04_dcs_write_seq_static(lms, LMS397_UNKNOWN_FC, + 0x00, 0x08); + msleep(150); + + /* CABC turn on sequence (BC = backlight control) */ + lms397kf04_dcs_write_seq_static(lms, LMS397_UNKNOWN_B4, + 0x0f, 0x00, 0x50); + lms397kf04_dcs_write_seq_static(lms, LMS397_USER_SELECT, 0x80); + lms397kf04_dcs_write_seq_static(lms, LMS397_UNKNOWN_B7, 0x24); + lms397kf04_dcs_write_seq_static(lms, LMS397_UNKNOWN_B8, 0x01); + + /* Turn on display */ + lms397kf04_dcs_write_seq_static(lms, MIPI_DCS_SET_DISPLAY_ON); + + /* Update brightness */ + + return 0; +} + +/** + * lms397kf04_get_modes() - return the mode + * @panel: the panel to get the mode for + * @connector: reference to the central DRM connector control structure + */ +static int lms397kf04_get_modes(struct drm_panel *panel, + struct drm_connector *connector) +{ + struct lms397kf04 *lms = to_lms397kf04(panel); + struct drm_display_mode *mode; + static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; + + mode = drm_mode_duplicate(connector->dev, &lms397kf04_mode); + if (!mode) { + dev_err(lms->dev, "failed to add mode\n"); + return -ENOMEM; + } + + connector->display_info.width_mm = mode->width_mm; + connector->display_info.height_mm = mode->height_mm; + connector->display_info.bus_flags = + DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE; + drm_display_info_set_bus_formats(&connector->display_info, + &bus_format, 1); + + drm_mode_set_name(mode); + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; + + drm_mode_probed_add(connector, mode); + + return 1; +} + +static const struct drm_panel_funcs lms397kf04_drm_funcs = { + .disable = lms397kf04_disable, + .unprepare = lms397kf04_unprepare, + .prepare = lms397kf04_prepare, + .enable = lms397kf04_enable, + .get_modes = lms397kf04_get_modes, +}; + +static int lms397kf04_probe(struct spi_device *spi) +{ + struct device *dev = &spi->dev; + struct lms397kf04 *lms; + int ret; + + lms = devm_kzalloc(dev, sizeof(*lms), GFP_KERNEL); + if (!lms) + return -ENOMEM; + lms->dev = dev; + + /* + * VCI is the analog voltage supply + * VCCIO is the digital I/O voltage supply + */ + lms->regulators[0].supply = "vci"; + lms->regulators[1].supply = "vccio"; + ret = devm_regulator_bulk_get(dev, + ARRAY_SIZE(lms->regulators), + lms->regulators); + if (ret) + return dev_err_probe(dev, ret, "failed to get regulators\n"); + + /* This asserts the RESET signal, putting the display into reset */ + lms->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(lms->reset)) { + dev_err(dev, "no RESET GPIO\n"); + return -ENODEV; + } + + spi->bits_per_word = 9; + /* Preserve e.g. SPI_3WIRE setting */ + spi->mode |= SPI_MODE_3; + ret = spi_setup(spi); + if (ret < 0) { + dev_err(dev, "spi setup failed.\n"); + return ret; + } + lms->spi = spi; + + drm_panel_init(&lms->panel, dev, &lms397kf04_drm_funcs, + DRM_MODE_CONNECTOR_DPI); + + /* FIXME: if no external backlight, use internal backlight */ + ret = drm_panel_of_backlight(&lms->panel); + if (ret) { + dev_info(dev, "failed to add backlight\n"); + return ret; + } + + spi_set_drvdata(spi, lms); + + drm_panel_add(&lms->panel); + dev_info(dev, "added panel\n"); + + return 0; +} + +static int lms397kf04_remove(struct spi_device *spi) +{ + struct lms397kf04 *lms = spi_get_drvdata(spi); + + drm_panel_remove(&lms->panel); + return 0; +} + +static const struct of_device_id lms397kf04_match[] = { + { .compatible = "samsung,lms397kf04", }, + {}, +}; +MODULE_DEVICE_TABLE(of, lms397kf04_match); + +static struct spi_driver lms397kf04_driver = { + .probe = lms397kf04_probe, + .remove = lms397kf04_remove, + .driver = { + .name = "lms397kf04-panel", + .of_match_table = lms397kf04_match, + }, +}; +module_spi_driver(lms397kf04_driver); + +MODULE_AUTHOR("Linus Walleij linus.walleij@linaro.org"); +MODULE_DESCRIPTION("Samsung LMS397KF04 panel driver"); +MODULE_LICENSE("GPL v2");
Hi,
On Thu, Apr 29, 2021 at 8:40 AM Linus Walleij <linus.walleij@linaro.org wrote:
@@ -33,6 +33,7 @@ obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o +obj-$(CONFIG_DRM_PANEL_SAMSUNG_LMS397KF04) += panel-samsung-lms397kf04.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
nit: sort ordering is off by one. D comes before M.
diff --git a/drivers/gpu/drm/panel/panel-samsung-lms397kf04.c b/drivers/gpu/drm/panel/panel-samsung-lms397kf04.c new file mode 100644 index 000000000000..41290cadc351 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-samsung-lms397kf04.c @@ -0,0 +1,421 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Panel driver for the Samsung LMS397KF04 480x800 DPI RGB panel.
- According to the data sheet the display controller is called DB7430
- Linus Walleij linus.walleij@linaro.org
- */
+#include <drm/drm_modes.h> +#include <drm/drm_panel.h>
+#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/media-bus-format.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regulator/consumer.h> +#include <linux/spi/spi.h>
+#include <video/mipi_display.h>
+#define LMS397_MANUFACTURER_CMD 0xb0 +#define LMS397_UNKNOWN_B4 0xb4 +#define LMS397_USER_SELECT 0xb5 +#define LMS397_UNKNOWN_B7 0xb7 +#define LMS397_UNKNOWN_B8 0xb8 +#define LMS397_PANEL_DRIVING 0xc0 +#define LMS397_SOURCE_CONTROL 0xc1 +#define LMS397_GATE_INTERFACE 0xc4 +#define LMS397_DISPLAY_H_TIMING 0xc5 +#define LMS397_RGB_SYNC_OPTION 0xc6 +#define LMS397_GAMMA_SET_RED 0xc8 +#define LMS397_GAMMA_SET_GREEN 0xc9 +#define LMS397_GAMMA_SET_BLUE 0xca +#define LMS397_BIAS_CURRENT_CTRL 0xd1 +#define LMS397_DDV_CTRL 0xd2 +#define LMS397_GAMMA_CTRL_REF 0xd3 +#define LMS397_UNKNOWN_D4 0xd4 +#define LMS397_DCDC_CTRL 0xd5 +#define LMS397_VCL_CTRL 0xd6 +#define LMS397_UNKNOWN_F8 0xf8 +#define LMS397_UNKNOWN_FC 0xfc
I managed to dig up a copy of the DCS spec. It says that all these 0xb0 - 0xfc are specific to each manufacturer, so that makes sense that they're all defined in this file instead of somewhere common. ...but it also says that, at least the way they're intended to be used, these commands are all supposed to be used only in the factory and then disabled. I guess maybe the manufacturer of this panel ignored that and requires these things to be configured each time the panel is powered up?
Also: should these #defines have a "DB7430" prefix instead of "LMS397" prefix? Presumably all of these commands are defined by the display controller and the only "LMS397" specific stuff is what you need to program into them for your display. I'm trying to think forward to when the second panel comes along that also has a DB7430 display controller.
+#define DATA_MASK 0x100
+/**
- struct lms397kf04 - state container for the LMS397kf04 panel
- */
+struct lms397kf04 {
/*** @dev: the container device*/struct device *dev;
optional nit: these are all small descriptions. Why not move them all to the single-line format, like:
/** @dev: the container device */
/*** @spi: the corresponding SPI device*/struct spi_device *spi;/*** @panel: the DRM panel instance for this device*/struct drm_panel panel;/*** @width: the width of this panel in mm*/u32 width;/*** @height: the height of this panel in mm*/u32 height;/*** @reset: reset GPIO line*/struct gpio_desc *reset;/*** @regulators: VCCIO and VIO supply regulators*/struct regulator_bulk_data regulators[2];+};
+static const struct drm_display_mode lms397kf04_mode = {
/** 31 ns period min (htotal*vtotal*vrefresh)/1000* gives a Vrefresh of ~71 Hz.*/.clock = 32258,.hdisplay = 480,.hsync_start = 480 + 10,.hsync_end = 480 + 10 + 4,.htotal = 480 + 10 + 4 + 40,.vdisplay = 800,.vsync_start = 800 + 6,.vsync_end = 800 + 6 + 1,.vtotal = 800 + 6 + 1 + 7,.width_mm = 53,.height_mm = 87,.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,+};
+static inline struct lms397kf04 *to_lms397kf04(struct drm_panel *panel) +{
return container_of(panel, struct lms397kf04, panel);+}
+static int lms397kf04_write_word(struct lms397kf04 *lms, u16 data) +{
/* SPI buffers are always in CPU order */return spi_write(lms->spi, &data, 2);+}
+static int lms397kf04_dcs_write(struct lms397kf04 *lms, const u8 *data, size_t len) +{
int ret;dev_dbg(lms->dev, "SPI writing dcs seq: %*ph\n", (int)len, data);/** This sends 9 bits with the first bit (bit 8) set to 0* This indicates that this is a command. Anything after the* command is data.*/ret = lms397kf04_write_word(lms, *data);while (!ret && --len) {++data;/* This sends 9 bits with the first bit (bit 8) set to 1 */ret = lms397kf04_write_word(lms, *data | DATA_MASK);}if (ret) {dev_err(lms->dev, "SPI error %d writing dcs seq: %*ph\n", ret,(int)len, data);}return ret;
So I've never looked at MIPI stuff at all before. ...but, as far as I can tell your panel is implementing MIPI DBI Type C Option 1 (3-line SPI). It feels like you should be using the functions for dealing with MIPI DBI, like mipi_dbi_spi_init(), mipi_dbi_command(). If I'm reading the code correctly, I think that will have the benefit of making your panel more flexible in terms of the capabilities of the SPI controller. It seems to handle the case when the SPI controller doesn't support 9-bits-per-word transfers, for instance.
If you have a good reason for not using the MIPI DBI functions then let me know and I'll look over your functions more closely.
+}
+#define lms397kf04_dcs_write_seq_static(ctx, seq ...) \
({ \static const u8 d[] = { seq }; \lms397kf04_dcs_write(ctx, d, ARRAY_SIZE(d)); \})+static int lms397kf04_power_on(struct lms397kf04 *lms) +{
int ret;/* Power up */ret = regulator_bulk_enable(ARRAY_SIZE(lms->regulators),lms->regulators);if (ret) {dev_err(lms->dev, "failed to enable regulators: %d\n", ret);return ret;}msleep(50);/* Assert reset >=1 ms */gpiod_set_value_cansleep(lms->reset, 1);msleep(1);
I dunno if it's still conventional wisdom, but "Documentation/timers/timers-howto.rst" still says that you should be using usleep() for such short sleeps.
/* De-assert reset */gpiod_set_value_cansleep(lms->reset, 0);/* Wait >= 10 ms */msleep(10);dev_info(lms->dev, "de-asserted RESET\n");
Can this be something less than dev_info()? dev_dbg() maybe?
/** This is set to 0x0a (RGB/BGR order + horizontal flip) in order* to make the display behave normally. If this is not set the displays* normal output behaviour is horizontally flipped and BGR ordered. Do* it twice because the first message doesn't always "take".*/lms397kf04_dcs_write_seq_static(lms, MIPI_DCS_SET_ADDRESS_MODE, 0x0a);lms397kf04_dcs_write_seq_static(lms, MIPI_DCS_SET_ADDRESS_MODE, 0x0a);
This actually looks like a somewhat standard property, but I guess exactly how it's interpreted is somewhat subject to the panel. Seems fine to just hardcode something (like you're doing) for now and someone can later figure out how to make it fancier.
Also: should we be error-checking lms397kf04_dcs_write_seq_static() return values in this function? spi_write() can fail...
/* Called "Access protection off" in vendor code */lms397kf04_dcs_write_seq_static(lms, LMS397_MANUFACTURER_CMD, 0x00);
Should you change the #define to LMS397_ACCESS_PROTECTION or something, then?
I don't have the vendor code, but I would suspect that "MANUFACTURER_CMD" is 0xb0 because the MIPI DCS document describes 0xb0 as the ID of the first custom manufacturer-specific command, but really all the commands 0xb0 - 0xff are manufacturer-specific commands.
lms397kf04_dcs_write_seq_static(lms, LMS397_PANEL_DRIVING, 0x28, 0x08);lms397kf04_dcs_write_seq_static(lms, LMS397_SOURCE_CONTROL,0x01, 0x30, 0x15, 0x05, 0x22);lms397kf04_dcs_write_seq_static(lms, LMS397_GATE_INTERFACE,0x10, 0x01, 0x00);lms397kf04_dcs_write_seq_static(lms, LMS397_DISPLAY_H_TIMING,0x06, 0x55, 0x03, 0x07, 0x0b,0x33, 0x00, 0x01, 0x03);/** 0x00 in datasheet 0x01 in vendor code 0x00, it seems 0x01 means* DE active high and 0x00 means DE active low.*/lms397kf04_dcs_write_seq_static(lms, LMS397_RGB_SYNC_OPTION, 0x01);lms397kf04_dcs_write_seq_static(lms, LMS397_GAMMA_SET_RED,/* R positive gamma */ 0x00,0x0A, 0x31, 0x3B, 0x4E, 0x58, 0x59, 0x5B, 0x58, 0x5E, 0x62,0x60, 0x61, 0x5E, 0x62, 0x55, 0x55, 0x7F, 0x08,/* R negative gamma */ 0x00,0x0A, 0x31, 0x3B, 0x4E, 0x58, 0x59, 0x5B, 0x58, 0x5E, 0x62,0x60, 0x61, 0x5E, 0x62, 0x55, 0x55, 0x7F, 0x08);lms397kf04_dcs_write_seq_static(lms, LMS397_GAMMA_SET_GREEN,/* G positive gamma */ 0x00,0x25, 0x15, 0x28, 0x3D, 0x4A, 0x48, 0x4C, 0x4A, 0x52, 0x59,0x59, 0x5B, 0x56, 0x60, 0x5D, 0x55, 0x7F, 0x0A,/* G negative gamma */ 0x00,0x25, 0x15, 0x28, 0x3D, 0x4A, 0x48, 0x4C, 0x4A, 0x52, 0x59,0x59, 0x5B, 0x56, 0x60, 0x5D, 0x55, 0x7F, 0x0A);lms397kf04_dcs_write_seq_static(lms, LMS397_GAMMA_SET_BLUE,/* B positive gamma */ 0x00,0x48, 0x10, 0x1F, 0x2F, 0x35, 0x38, 0x3D, 0x3C, 0x45, 0x4D,0x4E, 0x52, 0x51, 0x60, 0x7F, 0x7E, 0x7F, 0x0C,/* B negative gamma */ 0x00,0x48, 0x10, 0x1F, 0x2F, 0x35, 0x38, 0x3D, 0x3C, 0x45, 0x4D,0x4E, 0x52, 0x51, 0x60, 0x7F, 0x7E, 0x7F, 0x0C);lms397kf04_dcs_write_seq_static(lms, LMS397_BIAS_CURRENT_CTRL,0x33, 0x13);lms397kf04_dcs_write_seq_static(lms, LMS397_DDV_CTRL,0x11, 0x00, 0x00);lms397kf04_dcs_write_seq_static(lms, LMS397_GAMMA_CTRL_REF,0x50, 0x50);lms397kf04_dcs_write_seq_static(lms, LMS397_DCDC_CTRL,0x2f, 0x11, 0x1e, 0x46);lms397kf04_dcs_write_seq_static(lms, LMS397_VCL_CTRL,0x11, 0x0a);return 0;+}
+static void lms397kf04_power_off(struct lms397kf04 *lms) +{
/* Go into RESET and disable regulators */gpiod_set_value_cansleep(lms->reset, 1);regulator_bulk_disable(ARRAY_SIZE(lms->regulators),lms->regulators);
In theory regulator_bulk_disable() could fail. Since unprepare() also has an "error" return maybe return it?
+}
+static int lms397kf04_unprepare(struct drm_panel *panel) +{
struct lms397kf04 *lms = to_lms397kf04(panel);lms397kf04_power_off(lms);return 0;+}
It could be shorter. Assuming you add a return value to lms397kf04_power_off():
static int lms397kf04_unprepare(struct drm_panel *panel) { return lms397kf04_power_off(to_lms397kf04(panel)); }
+static int lms397kf04_disable(struct drm_panel *panel) +{
struct lms397kf04 *lms = to_lms397kf04(panel);lms397kf04_dcs_write_seq_static(lms, MIPI_DCS_SET_DISPLAY_OFF);msleep(25);lms397kf04_dcs_write_seq_static(lms, MIPI_DCS_ENTER_SLEEP_MODE);msleep(120);return 0;+}
+static int lms397kf04_prepare(struct drm_panel *panel) +{
struct lms397kf04 *lms = to_lms397kf04(panel);int ret;ret = lms397kf04_power_on(lms);if (ret)return ret;return 0;
It could be shorter:
static int lms397kf04_prepare(struct drm_panel *panel) { return lms397kf04_power_on(to_lms397kf04(panel)); }
+}
+static int lms397kf04_enable(struct drm_panel *panel) +{
struct lms397kf04 *lms = to_lms397kf04(panel);/* Exit sleep mode */lms397kf04_dcs_write_seq_static(lms, MIPI_DCS_EXIT_SLEEP_MODE);msleep(20);/* NVM (non-volatile memory) load sequence */lms397kf04_dcs_write_seq_static(lms, LMS397_UNKNOWN_D4,0x52, 0x5e);lms397kf04_dcs_write_seq_static(lms, LMS397_UNKNOWN_F8,0x01, 0xf5, 0xf2, 0x71, 0x44);lms397kf04_dcs_write_seq_static(lms, LMS397_UNKNOWN_FC,0x00, 0x08);msleep(150);
I'll believe you if you tell me that it's correct, but something feels odd to me. As I mentioned above, the MIPI DCS specs say that it's expected that most of the configuration that you're doing in lms397kf04_power_on() would happen at manufacturer time and then be "locked" so you don't need to do it again.
I could sorta believe that maybe some panels wouldn't have any non-volatile storage and that would explain why you need to program this stuff on every enable. ...but now the above comment says that it's loading stuff from non-volatile memory.
Are you sure that all of the magic config commands you have in lms397kf04_power_on() are actually needed / doing anything? Could they be leftover from some example code and they're not actually needed anymore?
/* CABC turn on sequence (BC = backlight control) */
I would expect that the "CA" would mean "Content Adaptive". As I understand it CABC saves power by dimming the backlight on the screen (or maybe part of the screen) depending on the contents. I think it's supposed to be a good power savings for watching full screen movies where it's common that nothing is using "full white".
lms397kf04_dcs_write_seq_static(lms, LMS397_UNKNOWN_B4,0x0f, 0x00, 0x50);lms397kf04_dcs_write_seq_static(lms, LMS397_USER_SELECT, 0x80);lms397kf04_dcs_write_seq_static(lms, LMS397_UNKNOWN_B7, 0x24);lms397kf04_dcs_write_seq_static(lms, LMS397_UNKNOWN_B8, 0x01);/* Turn on display */lms397kf04_dcs_write_seq_static(lms, MIPI_DCS_SET_DISPLAY_ON);/* Update brightness */
Was there supposed to be something after "Update brightness"?
return 0;+}
+/**
- lms397kf04_get_modes() - return the mode
- @panel: the panel to get the mode for
- @connector: reference to the central DRM connector control structure
- */
+static int lms397kf04_get_modes(struct drm_panel *panel,
struct drm_connector *connector)
nit: second line in function declaration has odd indentation.
+{
struct lms397kf04 *lms = to_lms397kf04(panel);struct drm_display_mode *mode;static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;mode = drm_mode_duplicate(connector->dev, &lms397kf04_mode);if (!mode) {dev_err(lms->dev, "failed to add mode\n");return -ENOMEM;}connector->display_info.width_mm = mode->width_mm;connector->display_info.height_mm = mode->height_mm;connector->display_info.bus_flags =DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;drm_display_info_set_bus_formats(&connector->display_info,&bus_format, 1);
panel-simple also sets the bpc in the "display_info". Do you need to?
drm_mode_set_name(mode);mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;drm_mode_probed_add(connector, mode);return 1;+}
+static const struct drm_panel_funcs lms397kf04_drm_funcs = {
.disable = lms397kf04_disable,.unprepare = lms397kf04_unprepare,.prepare = lms397kf04_prepare,.enable = lms397kf04_enable,.get_modes = lms397kf04_get_modes,+};
+static int lms397kf04_probe(struct spi_device *spi) +{
struct device *dev = &spi->dev;struct lms397kf04 *lms;int ret;lms = devm_kzalloc(dev, sizeof(*lms), GFP_KERNEL);if (!lms)return -ENOMEM;lms->dev = dev;/** VCI is the analog voltage supply* VCCIO is the digital I/O voltage supply*/lms->regulators[0].supply = "vci";lms->regulators[1].supply = "vccio";ret = devm_regulator_bulk_get(dev,ARRAY_SIZE(lms->regulators),lms->regulators);if (ret)return dev_err_probe(dev, ret, "failed to get regulators\n");/* This asserts the RESET signal, putting the display into reset */lms->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);if (IS_ERR(lms->reset)) {dev_err(dev, "no RESET GPIO\n");return -ENODEV;
nit: include the error code in the error message? Here and in other error messages in probe.
Also, you're forcing the error to -ENODEV. What if devm_gpiod_get() returned -EPROBE_DEFER? That will cause a problem, right? Why not just return the error code?
...oh, or more broadly use the nifty dev_err_probe() function that you use above!
}spi->bits_per_word = 9;/* Preserve e.g. SPI_3WIRE setting */spi->mode |= SPI_MODE_3;ret = spi_setup(spi);if (ret < 0) {dev_err(dev, "spi setup failed.\n");return ret;}lms->spi = spi;drm_panel_init(&lms->panel, dev, &lms397kf04_drm_funcs,DRM_MODE_CONNECTOR_DPI);/* FIXME: if no external backlight, use internal backlight */ret = drm_panel_of_backlight(&lms->panel);if (ret) {dev_info(dev, "failed to add backlight\n");
dev_err() ?
return ret;}spi_set_drvdata(spi, lms);drm_panel_add(&lms->panel);dev_info(dev, "added panel\n");
Change to dev_dbg() ?
-Doug
Hi Doug,
just sent out a v2 of this! Hope you can look at it.
Some comment on comments:
On Thu, Apr 29, 2021 at 10:21 PM Doug Anderson dianders@chromium.org wrote:
(...)
+#define LMS397_UNKNOWN_F8 0xf8 +#define LMS397_UNKNOWN_FC 0xfc
I managed to dig up a copy of the DCS spec. It says that all these 0xb0 - 0xfc are specific to each manufacturer, so that makes sense that they're all defined in this file instead of somewhere common. ...but it also says that, at least the way they're intended to be used, these commands are all supposed to be used only in the factory and then disabled. I guess maybe the manufacturer of this panel ignored that and requires these things to be configured each time the panel is powered up?
(...)
I'll believe you if you tell me that it's correct, but something feels odd to me. As I mentioned above, the MIPI DCS specs say that it's expected that most of the configuration that you're doing in lms397kf04_power_on() would happen at manufacturer time and then be "locked" so you don't need to do it again.
I could sorta believe that maybe some panels wouldn't have any non-volatile storage and that would explain why you need to program this stuff on every enable. ...but now the above comment says that it's loading stuff from non-volatile memory.
Are you sure that all of the magic config commands you have in lms397kf04_power_on() are actually needed / doing anything? Could they be leftover from some example code and they're not actually needed anymore?
Yes it is absolutely necessary.
This happens for a bit, see for example drivers/gpu/drm/panel/panel-novatek-nt35510.c
Why manufacturers (Samsung) do this I do not know for sure. Could be either:
1. The flash memory is an optional external component so they save money on the BOM like this 2. Programming the flash in production takes too much time so they save manufacturing cost like this 3. Time-to-market, product was stressed out the door without time to set up the proper flashing on the production line 4. The panel actually has some settings from production, but oops they are wrong and now they can't be fixed.
Case 4 corresponds to e.g. all ACPI quirks we have because abstracting things away by giving stuff BIOS is so good in theory and fails so much in practice because bugs.
So I've never looked at MIPI stuff at all before. ...but, as far as I can tell your panel is implementing MIPI DBI Type C Option 1 (3-line SPI). It feels like you should be using the functions for dealing with MIPI DBI, like mipi_dbi_spi_init(), mipi_dbi_command(). If I'm reading the code correctly, I think that will have the benefit of making your panel more flexible in terms of the capabilities of the SPI controller. It seems to handle the case when the SPI controller doesn't support 9-bits-per-word transfers, for instance.
If you have a good reason for not using the MIPI DBI functions then let me know and I'll look over your functions more closely.
MIPI DBI nominally requires an extra (GPIO) line called D/C (data/command) and this display for sure does not have it. So it would be "type c1" (no extra GPIO).
I will test and see if I can get DBI working... if I do I will respin v2 to v3.
Yours, Linus Walleij
On Tue, 06 Apr 2021 01:47:12 +0200, Linus Walleij wrote:
This adds device tree bindings for the Samsung LMS397KF04 RGB DPI display panel.
Cc: devicetree@vger.kernel.org Signed-off-by: Linus Walleij linus.walleij@linaro.org
.../display/panel/samsung,lms397kf04.yaml | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,lms397kf04.yaml
Reviewed-by: Rob Herring robh@kernel.org
On Tue, Apr 6, 2021 at 1:47 AM Linus Walleij linus.walleij@linaro.org wrote:
This adds device tree bindings for the Samsung LMS397KF04 RGB DPI display panel.
Cc: devicetree@vger.kernel.org Signed-off-by: Linus Walleij linus.walleij@linaro.org
Someone on DRM misc up to review and/or apply these two patches?
Doug? I bet you have some patch(es) you can toss back at me to review in return.
Yours, Linus Walleij
Hi,
On Thu, Apr 29, 2021 at 7:34 AM Linus Walleij linus.walleij@linaro.org wrote:
On Tue, Apr 6, 2021 at 1:47 AM Linus Walleij linus.walleij@linaro.org wrote:
This adds device tree bindings for the Samsung LMS397KF04 RGB DPI display panel.
Cc: devicetree@vger.kernel.org Signed-off-by: Linus Walleij linus.walleij@linaro.org
Someone on DRM misc up to review and/or apply these two patches?
Doug? I bet you have some patch(es) you can toss back at me to review in return.
I know next to nothing about MIPI panels, but I gave it my best shot. Please take my comments with a grain of salt since I'm still a bit noobie.
I think I already probably owed you a review anyway for previous reviews you did for me. I do have my big 20-part series out there and I certainly won't object to more review but most of the patches are close to having enough review at the moment. For some of the simple (dare I say "trivial"?) patches Bjorn was nice enough to provide review. For the panel patches I got an extra review from Sean Paul. I think there are only one or two patches in the series that have no review at the moment because I had to spin them for Bjorn's feedback and he hasn't gotten back to it yet.
-Doug
Hi,
On Thu, Apr 29, 2021 at 8:39 AM Linus Walleij linus.walleij@linaro.org wrote:
This adds device tree bindings for the Samsung LMS397KF04 RGB DPI display panel.
Cc: devicetree@vger.kernel.org Signed-off-by: Linus Walleij linus.walleij@linaro.org Reviewed-by: Rob Herring robh@kernel.org
.../display/panel/samsung,lms397kf04.yaml | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+)
Probably not needed since you have Rob Herring's review, but:
Reviewed-by: Douglas Anderson dianders@chromium.org
dri-devel@lists.freedesktop.org
-
Doug Anderson -
Linus Walleij -
Rob Herring