Hi Neil.
Looks good but a few comments in the following that needs some attention.
Sam
On Mon, Nov 23, 2020 at 03:33:54PM +0100, Neil Armstrong wrote:
> This add support for the Khadas TS050 1080x1920 5" LCD DSI panel designed to work
> with the Khadas Edge-V, Captain, VIM3 and VIM3L Single Board Computers.
> It provides a MIPI DSI interface to the host, a built-in LED backlight
> and touch controller.
>
> The init values was taken from the vendor source tree, comments were added to the
> know values but most of the init table is undocumented.
>
> Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/panel/Kconfig | 11 +
> drivers/gpu/drm/panel/Makefile | 1 +
> drivers/gpu/drm/panel/panel-khadas-ts050.c | 876 +++++++++++++++++++++
> 3 files changed, 888 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-khadas-ts050.c
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index b4e021ea30f9..8fec45b2ce02 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -145,6 +145,17 @@ config DRM_PANEL_JDI_LT070ME05000
> The panel has a 1200(RGB)×1920 (WUXGA) resolution and uses
> 24 bit per pixel.
>
> +config DRM_PANEL_KHADAS_TS050
> + tristate "Khadas TS050 panel"
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> + Say Y here if you want to enable support for Khadas TS050 TFT-LCD
> + panel module. The panel has a 1080x1920 resolution and uses
> + 24 bit RGB per pixel. It provides a MIPI DSI interface to
> + the host, a built-in LED backlight and touch controller.
> +
> config DRM_PANEL_KINGDISPLAY_KD097D04
> tristate "Kingdisplay kd097d04 panel"
> depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index ebbf488c7eac..03496695e03f 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
> obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
> obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
> obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
> +obj-$(CONFIG_DRM_PANEL_KHADAS_TS050) += panel-khadas-ts050.o
> obj-$(CONFIG_DRM_PANEL_KINGDISPLAY_KD097D04) += panel-kingdisplay-kd097d04.o
> obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
> obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
> diff --git a/drivers/gpu/drm/panel/panel-khadas-ts050.c b/drivers/gpu/drm/panel/panel-khadas-ts050.c
> new file mode 100644
> index 000000000000..856fcad69306
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-khadas-ts050.c
> @@ -0,0 +1,876 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 BayLibre, SAS
> + * Author: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <video/mipi_display.h>
> +
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_print.h>
Panels do not use DRM_ or drm_ for logging, so this include is not
needed.
> +
> +struct khadas_ts050_panel {
> + struct drm_panel base;
> + struct mipi_dsi_device *link;
> +
> + struct regulator *supply;
> + struct gpio_desc *reset_gpio;
> + struct gpio_desc *enable_gpio;
> +
> + bool prepared;
> + bool enabled;
> +};
> +
> +struct khadas_ts050_panel_cmd {
> + u8 cmd;
> + u8 data;
> +};
> +
> +/* Only the CMD1 User Command set is documented */
> +static const struct khadas_ts050_panel_cmd init_code[] = {
> + /* Select Unknown CMD Page (Undocumented) */
> + {0xff, 0xee},
...
> + {0xd4, 0x04}, /* RGBMIPICTRL: VSYNC front porch = 4 */
> +};
> +
> +static inline
> +struct khadas_ts050_panel *to_khadas_ts050_panel(struct drm_panel *panel)
> +{
> + return container_of(panel, struct khadas_ts050_panel, base);
> +}
> +
> +static int khadas_ts050_panel_prepare(struct drm_panel *panel)
> +{
> + struct khadas_ts050_panel *khadas_ts050 = to_khadas_ts050_panel(panel);
> + int err, regulator_err;
> + unsigned int i;
> +
> + if (khadas_ts050->prepared)
> + return 0;
> +
> + gpiod_set_value_cansleep(khadas_ts050->enable_gpio, 0);
> +
> + err = regulator_enable(khadas_ts050->supply);
> + if (err < 0)
> + return err;
> +
> + gpiod_set_value_cansleep(khadas_ts050->enable_gpio, 1);
> +
> + msleep(60);
> +
> + gpiod_set_value_cansleep(khadas_ts050->reset_gpio, 1);
> +
> + usleep_range(10000, 11000);
> +
> + gpiod_set_value_cansleep(khadas_ts050->reset_gpio, 0);
> +
> + /* Select CMD2 page 4 (Undocumented) */
> + mipi_dsi_dcs_write(khadas_ts050->link, 0xff, (u8[]){ 0x05 }, 1);
> +
> + /* Reload CMD1: Don't reload default value to register */
> + mipi_dsi_dcs_write(khadas_ts050->link, 0xfb, (u8[]){ 0x01 }, 1);
> +
> + mipi_dsi_dcs_write(khadas_ts050->link, 0xc5, (u8[]){ 0x01 }, 1);
> +
> + msleep(100);
> +
> + for (i = 0; i < ARRAY_SIZE(init_code); i++) {
> + err = mipi_dsi_dcs_write(khadas_ts050->link,
> + init_code[i].cmd,
> + &init_code[i].data, 1);
> + if (err < 0) {
> + dev_err(panel->dev, "failed write cmds: %d\n", err);
> + goto poweroff;
> + }
> + }
> +
> + err = mipi_dsi_dcs_exit_sleep_mode(khadas_ts050->link);
> + if (err < 0) {
> + dev_err(panel->dev, "failed to exit sleep mode: %d\n", err);
> + goto poweroff;
> + }
> +
> + msleep(120);
> +
> + /* Select CMD1 */
> + mipi_dsi_dcs_write(khadas_ts050->link, 0xff, (u8[]){ 0x00 }, 1);
> +
> + err = mipi_dsi_dcs_set_tear_on(khadas_ts050->link,
> + MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> + if (err < 0) {
> + dev_err(panel->dev, "failed to set tear on: %d\n", err);
> + goto poweroff;
> + }
> +
> +
> + err = mipi_dsi_dcs_set_display_on(khadas_ts050->link);
> + if (err < 0) {
> + dev_err(panel->dev, "failed to set display on: %d\n", err);
> + goto poweroff;
> + }
> +
> + usleep_range(10000, 11000);
> +
> + khadas_ts050->prepared = true;
> +
> + return 0;
> +
> +poweroff:
> + gpiod_set_value_cansleep(khadas_ts050->enable_gpio, 0);
gpiod_set_value_cansleep(khadas_ts050->reset_gpio, 1);
??
So panel is left in reset mode.
> +
> + regulator_err = regulator_disable(khadas_ts050->supply);
> + if (regulator_err)
> + dev_err(panel->dev, "failed to disable regulator: %d\n", regulator_err);
This check is not really of any value and can be dropped.
> +
> + return err;
> +}
> +
> +static int khadas_ts050_panel_unprepare(struct drm_panel *panel)
> +{
> + struct khadas_ts050_panel *khadas_ts050 = to_khadas_ts050_panel(panel);
> + int err;
> +
> + if (!khadas_ts050->prepared)
> + return 0;
> +
> + err = mipi_dsi_dcs_enter_sleep_mode(khadas_ts050->link);
> + if (err < 0) {
> + dev_err(panel->dev, "failed to enter sleep mode: %d\n", err);
> + return err;
Hmm, even if we fail here or below the panel is no longer prepared.
I think it would be better to set prepared to false unconditionally.
Many panel drivers get this wrong.
> + }
> +
> + msleep(150);
> +
> + gpiod_set_value_cansleep(khadas_ts050->enable_gpio, 0);
> +
> + err = regulator_disable(khadas_ts050->supply);
> + if (err < 0)
> + return err;
> +
> + khadas_ts050->prepared = false;
> +
> + return 0;
> +}
> +
> +static int khadas_ts050_panel_enable(struct drm_panel *panel)
> +{
> + struct khadas_ts050_panel *khadas_ts050 = to_khadas_ts050_panel(panel);
> +
> + if (khadas_ts050->enabled)
> + return 0;
No need for the check - just set it to true.
> +
> + khadas_ts050->enabled = true;
> +
> + return 0;
> +}
> +
> +static int khadas_ts050_panel_disable(struct drm_panel *panel)
> +{
> + struct khadas_ts050_panel *khadas_ts050 = to_khadas_ts050_panel(panel);
> + int err;
> +
> + if (!khadas_ts050->enabled)
> + return 0;
> +
> + err = mipi_dsi_dcs_set_display_off(khadas_ts050->link);
> + if (err < 0)
> + dev_err(panel->dev, "failed to set display off: %d\n", err);
> +
> + usleep_range(10000, 11000);
> +
> + khadas_ts050->enabled = false;
> +
> + return 0;
> +}
> +
> +static const struct drm_display_mode default_mode = {
> + .clock = 120000,
> + .hdisplay = 1088,
> + .hsync_start = 1088 + 104,
> + .hsync_end = 1088 + 104 + 4,
> + .htotal = 1088 + 104 + 4 + 127,
> + .vdisplay = 1920,
> + .vsync_start = 1920 + 4,
> + .vsync_end = 1920 + 4 + 2,
> + .vtotal = 1920 + 4 + 2 + 3,
> + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> +};
> +
> +static int khadas_ts050_panel_get_modes(struct drm_panel *panel,
> + struct drm_connector *connector)
> +{
> + struct drm_display_mode *mode;
> +
> + mode = drm_mode_duplicate(connector->dev, &default_mode);
> + if (!mode) {
> + dev_err(panel->dev, "failed to add mode %ux%u@%u\n",
> + default_mode.hdisplay, default_mode.vdisplay,
> + drm_mode_vrefresh(&default_mode));
> + return -ENOMEM;
> + }
> +
> + drm_mode_set_name(mode);
> +
> + drm_mode_probed_add(connector, mode);
> +
> + connector->display_info.width_mm = 64;
> + connector->display_info.height_mm = 118;
> + connector->display_info.bpc = 8;
> +
> + return 1;
> +}
> +
> +static const struct drm_panel_funcs khadas_ts050_panel_funcs = {
> + .prepare = khadas_ts050_panel_prepare,
> + .unprepare = khadas_ts050_panel_unprepare,
> + .enable = khadas_ts050_panel_enable,
> + .disable = khadas_ts050_panel_disable,
> + .get_modes = khadas_ts050_panel_get_modes,
> +};
> +
> +static const struct of_device_id khadas_ts050_of_match[] = {
> + { .compatible = "khadas,ts050", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, khadas_ts050_of_match);
> +
> +static int khadas_ts050_panel_add(struct khadas_ts050_panel *khadas_ts050)
> +{
> + struct device *dev = &khadas_ts050->link->dev;
> + int err;
> +
> + khadas_ts050->supply = devm_regulator_get(dev, "power");
> + if (IS_ERR(khadas_ts050->supply))
> + return PTR_ERR(khadas_ts050->supply);
Use return dev_err_probbe(dev, PTR_ERR(khadas_ts050->supply), "failed to get power supply");
> +
> + khadas_ts050->reset_gpio = devm_gpiod_get(dev, "reset",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(khadas_ts050->reset_gpio)) {
> + err = PTR_ERR(khadas_ts050->reset_gpio);
> + dev_dbg(dev, "failed to get reset gpio: %d\n", err);
> + return err;
Likewise - use return dev_err_probe(...);
> +
> + khadas_ts050->enable_gpio = devm_gpiod_get(dev, "enable",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(khadas_ts050->enable_gpio)) {
> + err = PTR_ERR(khadas_ts050->enable_gpio);
> + dev_dbg(dev, "failed to get enable gpio: %d\n", err);
> + return err;
> + }
dev_err_probe()
> +
> + drm_panel_init(&khadas_ts050->base, &khadas_ts050->link->dev,
> + &khadas_ts050_panel_funcs, DRM_MODE_CONNECTOR_DSI);
> +
> + err = drm_panel_of_backlight(&khadas_ts050->base);
> + if (err)
> + return err;
> +
> + drm_panel_add(&khadas_ts050->base);
> +
> + return 0;
> +}
> +
> +static int khadas_ts050_panel_probe(struct mipi_dsi_device *dsi)
> +{
> + struct khadas_ts050_panel *khadas_ts050;
> + int err;
> +
> + dsi->lanes = 4;
> + dsi->format = MIPI_DSI_FMT_RGB888;
> + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> + MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_EOT_PACKET;
> +
> + khadas_ts050 = devm_kzalloc(&dsi->dev, sizeof(*khadas_ts050),
> + GFP_KERNEL);
> + if (!khadas_ts050)
> + return -ENOMEM;
> +
> + mipi_dsi_set_drvdata(dsi, khadas_ts050);
> + khadas_ts050->link = dsi;
> +
> + err = khadas_ts050_panel_add(khadas_ts050);
> + if (err < 0)
> + return err;
> +
> + return mipi_dsi_attach(dsi);
> +}
> +
> +static int khadas_ts050_panel_remove(struct mipi_dsi_device *dsi)
> +{
> + struct khadas_ts050_panel *khadas_ts050 = mipi_dsi_get_drvdata(dsi);
> + int err;
> +
> + err = mipi_dsi_detach(dsi);
> + if (err < 0)
> + dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", err);
> +
> + drm_panel_remove(&khadas_ts050->base);
> + drm_panel_disable(&khadas_ts050->base);
> + drm_panel_unprepare(&khadas_ts050->base);
> +
> + return 0;
> +}
> +
> +static void khadas_ts050_panel_shutdown(struct mipi_dsi_device *dsi)
> +{
> + struct khadas_ts050_panel *khadas_ts050 = mipi_dsi_get_drvdata(dsi);
> +
> + drm_panel_disable(&khadas_ts050->base);
> + drm_panel_unprepare(&khadas_ts050->base);
> +}
> +
> +static struct mipi_dsi_driver khadas_ts050_panel_driver = {
> + .driver = {
> + .name = "panel-khadas-ts050",
> + .of_match_table = khadas_ts050_of_match,
> + },
> + .probe = khadas_ts050_panel_probe,
> + .remove = khadas_ts050_panel_remove,
> + .shutdown = khadas_ts050_panel_shutdown,
> +};
> +module_mipi_dsi_driver(khadas_ts050_panel_driver);
> +
> +MODULE_AUTHOR("Neil Armstrong <narmstrong@xxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("Khadas TS050 panel driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
