| Message ID | 20221230113155.3430142-3-javierm@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Add PinePhone Pro display support | expand |
Hi Javier, On Fri, Dec 30, 2022 at 12:31:52PM +0100, Javier Martinez Canillas wrote: > From: Kamil Trzciński <ayufan@ayufan.eu> > > The driver is for panels based on the Himax HX8394 controller, such as the > HannStar HSD060BHW4 720x1440 TFT LCD panel that uses a MIPI-DSI interface. I see you've removed debug printks from enable/disable/prepare/unprepare hooks. Have you tested the driver thoroughly with various DRM apps, with DPM/suspend/resume, etc.? The dw-mipi-dsi driver does some unorthodox things[1], that can lead to unbalanced calls to these functions in some situations, and that's why all these printks were there. To ensure the driver hooks are called correctly, while preparing the code for upstreaming. This lead to broken display in some situations during suspend/resume. https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L868 Also, have you checked the clocks are actually configured correctly by the rk3399 cru driver? I have a lot of trouble with that, too. clk driver sometimes selects the fractional clock, but does not give it the necessary >20x difference between input/output clock rates. You'll only notice if you measure clock rates directly, by looking at actual refresh rate, by using some testing DRM app. Clock subsystem sometimes shuffles things around if you switch VOPs and use big VOP for mipi-dsi display, instead of the default small VOP. I'll test this patchset in a few days against purely mainline code, but I'm pretty sure looking at the modes you use, that this will not work on some Pinephone Pro's, and will cause display corruption when you fix your clock setup, so that CRU actually outputs 74.25MHz as requested by the mode. (Which can be fixed by this patch https://github.com/megous/linux/commit/f7ee16f12ee8a44ee2472f2967ca27768106e00f) regards, o. > Signed-off-by: Kamil Trzciński <ayufan@ayufan.eu> > Co-developed-by: Ondrej Jirman <megi@xff.cz> > Signed-off-by: Ondrej Jirman <megi@xff.cz> > Co-developed-by: Javier Martinez Canillas <javierm@redhat.com> > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > --- > > Changes in v4: > - Add Tom Fitzhenry's Tested-by tag. > > Changes in v3: > - Add Sam Ravnborg's reviwed-by tag. > - Move driver patch after one introducing the DT binding (Sam Ravnborg). > > Changes in v2: > - Add year to driver's copyright notice (Sam Ravnborg) > - Remove unused <video/display_timing.h> header include (Sam Ravnborg). > - Use mipi_dsi_dcs_write_seq() helper and drop custom macro (Sam Ravnborg). > - Drop unnecessary info messages and move useful one to debug (Sam Ravnborg). > > drivers/gpu/drm/panel/Kconfig | 12 + > drivers/gpu/drm/panel/Makefile | 1 + > drivers/gpu/drm/panel/panel-himax-hx8394.c | 446 +++++++++++++++++++++ > 3 files changed, 459 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-himax-hx8394.c > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > index 737edcdf9eef..7ee9c83f09a7 100644 > --- a/drivers/gpu/drm/panel/Kconfig > +++ b/drivers/gpu/drm/panel/Kconfig > @@ -154,6 +154,18 @@ config DRM_PANEL_FEIYANG_FY07024DI26A30D > Say Y if you want to enable support for panels based on the > Feiyang FY07024DI26A30-D MIPI-DSI interface. > > +config DRM_PANEL_HIMAX_HX8394 > + tristate "HIMAX HX8394 MIPI-DSI LCD panels" > + depends on OF > + depends on DRM_MIPI_DSI > + depends on BACKLIGHT_CLASS_DEVICE > + help > + Say Y if you want to enable support for panels based on the > + Himax HX8394 controller, such as the HannStar HSD060BHW4 > + 720x1440 TFT LCD panel that uses a MIPI-DSI interface. > + > + If M is selected the module will be called panel-himax-hx8394. > + > config DRM_PANEL_ILITEK_IL9322 > tristate "Ilitek ILI9322 320x240 QVGA panels" > depends on OF && SPI > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile > index f8f9d9f6a307..84c01adafd4c 100644 > --- a/drivers/gpu/drm/panel/Makefile > +++ b/drivers/gpu/drm/panel/Makefile > @@ -13,6 +13,7 @@ obj-$(CONFIG_DRM_PANEL_EBBG_FT8719) += panel-ebbg-ft8719.o > obj-$(CONFIG_DRM_PANEL_ELIDA_KD35T133) += panel-elida-kd35t133.o > obj-$(CONFIG_DRM_PANEL_FEIXIN_K101_IM2BA02) += panel-feixin-k101-im2ba02.o > obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d.o > +obj-$(CONFIG_DRM_PANEL_HIMAX_HX8394) += panel-himax-hx8394.o > obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o > obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += panel-ilitek-ili9341.o > obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o > diff --git a/drivers/gpu/drm/panel/panel-himax-hx8394.c b/drivers/gpu/drm/panel/panel-himax-hx8394.c > new file mode 100644 > index 000000000000..fc7a2c299f8d > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-himax-hx8394.c > @@ -0,0 +1,446 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver for panels based on Himax HX8394 controller, such as: > + * > + * - HannStar HSD060BHW4 5.99" MIPI-DSI panel > + * > + * Copyright (C) 2021 Kamil Trzciński > + * > + * Based on drivers/gpu/drm/panel/panel-sitronix-st7703.c > + * Copyright (C) Purism SPC 2019 > + */ > + > +#include <linux/delay.h> > +#include <linux/gpio/consumer.h> > +#include <linux/media-bus-format.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/regulator/consumer.h> > + > +#include <video/mipi_display.h> > + > +#include <drm/drm_mipi_dsi.h> > +#include <drm/drm_modes.h> > +#include <drm/drm_panel.h> > + > +#define DRV_NAME "panel-himax-hx8394" > + > +/* Manufacturer specific commands sent via DSI, listed in HX8394-F datasheet */ > +#define HX8394_CMD_SETSEQUENCE 0xb0 > +#define HX8394_CMD_SETPOWER 0xb1 > +#define HX8394_CMD_SETDISP 0xb2 > +#define HX8394_CMD_SETCYC 0xb4 > +#define HX8394_CMD_SETVCOM 0xb6 > +#define HX8394_CMD_SETTE 0xb7 > +#define HX8394_CMD_SETSENSOR 0xb8 > +#define HX8394_CMD_SETEXTC 0xb9 > +#define HX8394_CMD_SETMIPI 0xba > +#define HX8394_CMD_SETOTP 0xbb > +#define HX8394_CMD_SETREGBANK 0xbd > +#define HX8394_CMD_UNKNOWN1 0xc0 > +#define HX8394_CMD_SETDGCLUT 0xc1 > +#define HX8394_CMD_SETID 0xc3 > +#define HX8394_CMD_SETDDB 0xc4 > +#define HX8394_CMD_UNKNOWN2 0xc6 > +#define HX8394_CMD_SETCABC 0xc9 > +#define HX8394_CMD_SETCABCGAIN 0xca > +#define HX8394_CMD_SETPANEL 0xcc > +#define HX8394_CMD_SETOFFSET 0xd2 > +#define HX8394_CMD_SETGIP0 0xd3 > +#define HX8394_CMD_UNKNOWN3 0xd4 > +#define HX8394_CMD_SETGIP1 0xd5 > +#define HX8394_CMD_SETGIP2 0xd6 > +#define HX8394_CMD_SETGPO 0xd6 > +#define HX8394_CMD_SETSCALING 0xdd > +#define HX8394_CMD_SETIDLE 0xdf > +#define HX8394_CMD_SETGAMMA 0xe0 > +#define HX8394_CMD_SETCHEMODE_DYN 0xe4 > +#define HX8394_CMD_SETCHE 0xe5 > +#define HX8394_CMD_SETCESEL 0xe6 > +#define HX8394_CMD_SET_SP_CMD 0xe9 > +#define HX8394_CMD_SETREADINDEX 0xfe > +#define HX8394_CMD_GETSPIREAD 0xff > + > +struct hx8394 { > + struct device *dev; > + struct drm_panel panel; > + struct gpio_desc *reset_gpio; > + struct regulator *vcc; > + struct regulator *iovcc; > + bool prepared; > + > + const struct hx8394_panel_desc *desc; > +}; > + > +struct hx8394_panel_desc { > + const struct drm_display_mode *mode; > + unsigned int lanes; > + unsigned long mode_flags; > + enum mipi_dsi_pixel_format format; > + int (*init_sequence)(struct hx8394 *ctx); > +}; > + > +static inline struct hx8394 *panel_to_hx8394(struct drm_panel *panel) > +{ > + return container_of(panel, struct hx8394, panel); > +} > + > +static int hsd060bhw4_init_sequence(struct hx8394 *ctx) > +{ > + struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); > + > + /* 5.19.8 SETEXTC: Set extension command (B9h) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETEXTC, > + 0xff, 0x83, 0x94); > + > + /* 5.19.2 SETPOWER: Set power (B1h) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETPOWER, > + 0x48, 0x11, 0x71, 0x09, 0x32, 0x24, 0x71, 0x31, 0x55, 0x30); > + > + /* 5.19.9 SETMIPI: Set MIPI control (BAh) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETMIPI, > + 0x63, 0x03, 0x68, 0x6b, 0xb2, 0xc0); > + > + /* 5.19.3 SETDISP: Set display related register (B2h) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETDISP, > + 0x00, 0x80, 0x78, 0x0c, 0x07); > + > + /* 5.19.4 SETCYC: Set display waveform cycles (B4h) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETCYC, > + 0x12, 0x63, 0x12, 0x63, 0x12, 0x63, 0x01, 0x0c, 0x7c, 0x55, > + 0x00, 0x3f, 0x12, 0x6b, 0x12, 0x6b, 0x12, 0x6b, 0x01, 0x0c, > + 0x7c); > + > + /* 5.19.19 SETGIP0: Set GIP Option0 (D3h) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETGIP0, > + 0x00, 0x00, 0x00, 0x00, 0x3c, 0x1c, 0x00, 0x00, 0x32, 0x10, > + 0x09, 0x00, 0x09, 0x32, 0x15, 0xad, 0x05, 0xad, 0x32, 0x00, > + 0x00, 0x00, 0x00, 0x37, 0x03, 0x0b, 0x0b, 0x37, 0x00, 0x00, > + 0x00, 0x0c, 0x40); > + > + /* 5.19.20 Set GIP Option1 (D5h) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETGIP1, > + 0x19, 0x19, 0x18, 0x18, 0x1b, 0x1b, 0x1a, 0x1a, 0x00, 0x01, > + 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x20, 0x21, 0x18, 0x18, > + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, > + 0x24, 0x25, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, > + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18); > + > + /* 5.19.21 Set GIP Option2 (D6h) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETGIP2, > + 0x18, 0x18, 0x19, 0x19, 0x1b, 0x1b, 0x1a, 0x1a, 0x07, 0x06, > + 0x05, 0x04, 0x03, 0x02, 0x01, 0x00, 0x25, 0x24, 0x18, 0x18, > + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, > + 0x21, 0x20, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, > + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18); > + > + /* 5.19.25 SETGAMMA: Set gamma curve related setting (E0h) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETGAMMA, > + 0x00, 0x04, 0x0c, 0x12, 0x14, 0x18, 0x1a, 0x18, 0x31, 0x3f, > + 0x4d, 0x4c, 0x54, 0x65, 0x6b, 0x70, 0x7f, 0x82, 0x7e, 0x8a, > + 0x99, 0x4a, 0x48, 0x49, 0x4b, 0x4a, 0x4c, 0x4b, 0x7f, 0x00, > + 0x04, 0x0c, 0x11, 0x13, 0x17, 0x1a, 0x18, 0x31, > + 0x3f, 0x4d, 0x4c, 0x54, 0x65, 0x6b, 0x70, 0x7f, > + 0x82, 0x7e, 0x8a, 0x99, 0x4a, 0x48, 0x49, 0x4b, > + 0x4a, 0x4c, 0x4b, 0x7f); > + > + /* 5.19.17 SETPANEL (CCh) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETPANEL, 0x0b); > + > + /* Unknown command, not listed in the HX8394-F datasheet */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_UNKNOWN1, 0x1f, 0x31); > + > + /* 5.19.5 SETVCOM: Set VCOM voltage (B6h) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETVCOM, > + 0x7d, 0x7d); > + > + /* Unknown command, not listed in the HX8394-F datasheet */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_UNKNOWN3, > + 0x02); > + > + /* 5.19.11 Set register bank (BDh) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETREGBANK, 0x01); > + > + /* 5.19.2 SETPOWER: Set power (B1h) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETPOWER, 0x00); > + > + /* 5.19.11 Set register bank (BDh) */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETREGBANK, 0x00); > + > + /* Unknown command, not listed in the HX8394-F datasheet */ > + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_UNKNOWN3, > + 0xed); > + > + return 0; > +} > + > +static const struct drm_display_mode hsd060bhw4_mode = { > + .hdisplay = 720, > + .hsync_start = 720 + 40, > + .hsync_end = 720 + 40 + 46, > + .htotal = 720 + 40 + 46 + 40, > + .vdisplay = 1440, > + .vsync_start = 1440 + 9, > + .vsync_end = 1440 + 9 + 7, > + .vtotal = 1440 + 9 + 7 + 7, > + .clock = 74250, > + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, > + .width_mm = 68, > + .height_mm = 136, > +}; > + > +static const struct hx8394_panel_desc hsd060bhw4_desc = { > + .mode = &hsd060bhw4_mode, > + .lanes = 4, > + .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST, > + .format = MIPI_DSI_FMT_RGB888, > + .init_sequence = hsd060bhw4_init_sequence, > +}; > + > +static int hx8394_enable(struct drm_panel *panel) > +{ > + struct hx8394 *ctx = panel_to_hx8394(panel); > + struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); > + int ret; > + > + ret = ctx->desc->init_sequence(ctx); > + if (ret) { > + dev_err(ctx->dev, "Panel init sequence failed: %d\n", ret); > + return ret; > + } > + > + ret = mipi_dsi_dcs_exit_sleep_mode(dsi); > + if (ret) { > + dev_err(ctx->dev, "Failed to exit sleep mode: %d\n", ret); > + return ret; > + } > + > + /* Panel is operational 120 msec after reset */ > + msleep(120); > + > + ret = mipi_dsi_dcs_set_display_on(dsi); > + if (ret) { > + dev_err(ctx->dev, "Failed to turn on the display: %d\n", ret); > + goto sleep_in; > + } > + > + return 0; > + > +sleep_in: > + /* This will probably fail, but let's try orderly power off anyway. */ > + ret = mipi_dsi_dcs_enter_sleep_mode(dsi); > + if (!ret) > + msleep(50); > + > + return ret; > +} > + > +static int hx8394_disable(struct drm_panel *panel) > +{ > + struct hx8394 *ctx = panel_to_hx8394(panel); > + struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); > + int ret; > + > + ret = mipi_dsi_dcs_enter_sleep_mode(dsi); > + if (ret) { > + dev_err(ctx->dev, "Failed to enter sleep mode: %d\n", ret); > + return ret; > + } > + > + msleep(50); /* about 3 frames */ > + > + return 0; > +} > + > +static int hx8394_unprepare(struct drm_panel *panel) > +{ > + struct hx8394 *ctx = panel_to_hx8394(panel); > + > + if (!ctx->prepared) > + return 0; > + > + gpiod_set_value_cansleep(ctx->reset_gpio, 1); > + > + regulator_disable(ctx->iovcc); > + regulator_disable(ctx->vcc); > + > + ctx->prepared = false; > + > + return 0; > +} > + > +static int hx8394_prepare(struct drm_panel *panel) > +{ > + struct hx8394 *ctx = panel_to_hx8394(panel); > + int ret; > + > + if (ctx->prepared) > + return 0; > + > + gpiod_set_value_cansleep(ctx->reset_gpio, 1); > + > + ret = regulator_enable(ctx->vcc); > + if (ret) { > + dev_err(ctx->dev, "Failed to enable vcc supply: %d\n", ret); > + return ret; > + } > + > + ret = regulator_enable(ctx->iovcc); > + if (ret) { > + dev_err(ctx->dev, "Failed to enable iovcc supply: %d\n", ret); > + goto disable_vcc; > + } > + > + gpiod_set_value_cansleep(ctx->reset_gpio, 0); > + > + msleep(180); > + > + ctx->prepared = true; > + > + return 0; > + > +disable_vcc: > + gpiod_set_value_cansleep(ctx->reset_gpio, 1); > + regulator_disable(ctx->vcc); > + return ret; > +} > + > +static int hx8394_get_modes(struct drm_panel *panel, > + struct drm_connector *connector) > +{ > + struct hx8394 *ctx = panel_to_hx8394(panel); > + struct drm_display_mode *mode; > + > + mode = drm_mode_duplicate(connector->dev, ctx->desc->mode); > + if (!mode) { > + dev_err(ctx->dev, "Failed to add mode %ux%u@%u\n", > + ctx->desc->mode->hdisplay, ctx->desc->mode->vdisplay, > + drm_mode_vrefresh(ctx->desc->mode)); > + return -ENOMEM; > + } > + > + drm_mode_set_name(mode); > + > + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > + connector->display_info.width_mm = mode->width_mm; > + connector->display_info.height_mm = mode->height_mm; > + drm_mode_probed_add(connector, mode); > + > + return 1; > +} > + > +static const struct drm_panel_funcs hx8394_drm_funcs = { > + .disable = hx8394_disable, > + .unprepare = hx8394_unprepare, > + .prepare = hx8394_prepare, > + .enable = hx8394_enable, > + .get_modes = hx8394_get_modes, > +}; > + > +static int hx8394_probe(struct mipi_dsi_device *dsi) > +{ > + struct device *dev = &dsi->dev; > + struct hx8394 *ctx; > + int ret; > + > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + > + ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(ctx->reset_gpio)) > + return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio), > + "Failed to get reset gpio\n"); > + > + mipi_dsi_set_drvdata(dsi, ctx); > + > + ctx->dev = dev; > + ctx->desc = of_device_get_match_data(dev); > + > + dsi->mode_flags = ctx->desc->mode_flags; > + dsi->format = ctx->desc->format; > + dsi->lanes = ctx->desc->lanes; > + > + ctx->vcc = devm_regulator_get(dev, "vcc"); > + if (IS_ERR(ctx->vcc)) > + return dev_err_probe(dev, PTR_ERR(ctx->vcc), > + "Failed to request vcc regulator\n"); > + > + ctx->iovcc = devm_regulator_get(dev, "iovcc"); > + if (IS_ERR(ctx->iovcc)) > + return dev_err_probe(dev, PTR_ERR(ctx->iovcc), > + "Failed to request iovcc regulator\n"); > + > + drm_panel_init(&ctx->panel, dev, &hx8394_drm_funcs, > + DRM_MODE_CONNECTOR_DSI); > + > + ret = drm_panel_of_backlight(&ctx->panel); > + if (ret) > + return ret; > + > + drm_panel_add(&ctx->panel); > + > + ret = mipi_dsi_attach(dsi); > + if (ret < 0) { > + dev_err_probe(dev, ret, "mipi_dsi_attach failed\n"); > + drm_panel_remove(&ctx->panel); > + return ret; > + } > + > + dev_dbg(dev, "%ux%u@%u %ubpp dsi %udl - ready\n", > + ctx->desc->mode->hdisplay, ctx->desc->mode->vdisplay, > + drm_mode_vrefresh(ctx->desc->mode), > + mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes); > + > + return 0; > +} > + > +static void hx8394_shutdown(struct mipi_dsi_device *dsi) > +{ > + struct hx8394 *ctx = mipi_dsi_get_drvdata(dsi); > + int ret; > + > + ret = drm_panel_disable(&ctx->panel); > + if (ret < 0) > + dev_err(&dsi->dev, "Failed to disable panel: %d\n", ret); > + > + ret = drm_panel_unprepare(&ctx->panel); > + if (ret < 0) > + dev_err(&dsi->dev, "Failed to unprepare panel: %d\n", ret); > +} > + > +static void hx8394_remove(struct mipi_dsi_device *dsi) > +{ > + struct hx8394 *ctx = mipi_dsi_get_drvdata(dsi); > + int ret; > + > + hx8394_shutdown(dsi); > + > + ret = mipi_dsi_detach(dsi); > + if (ret < 0) > + dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret); > + > + drm_panel_remove(&ctx->panel); > +} > + > +static const struct of_device_id hx8394_of_match[] = { > + { .compatible = "hannstar,hsd060bhw4", .data = &hsd060bhw4_desc }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, hx8394_of_match); > + > +static struct mipi_dsi_driver hx8394_driver = { > + .probe = hx8394_probe, > + .remove = hx8394_remove, > + .shutdown = hx8394_shutdown, > + .driver = { > + .name = DRV_NAME, > + .of_match_table = hx8394_of_match, > + }, > +}; > +module_mipi_dsi_driver(hx8394_driver); > + > +MODULE_AUTHOR("Kamil Trzciński <ayufan@ayufan.eu>"); > +MODULE_DESCRIPTION("DRM driver for Himax HX8394 based MIPI DSI panels"); > +MODULE_LICENSE("GPL"); > -- > 2.38.1 >
Hello Ondřej, Thanks a lot for your comments. On 12/30/22 16:40, Ondřej Jirman wrote: > Hi Javier, > > On Fri, Dec 30, 2022 at 12:31:52PM +0100, Javier Martinez Canillas wrote: >> From: Kamil Trzciński <ayufan@ayufan.eu> >> >> The driver is for panels based on the Himax HX8394 controller, such as the >> HannStar HSD060BHW4 720x1440 TFT LCD panel that uses a MIPI-DSI interface. > > I see you've removed debug printks from enable/disable/prepare/unprepare Yes, because as you said were debug printks. Feel free to propose adding the debug printks if you consider useful for normal usage and not just for devel purposes. > hooks. Have you tested the driver thoroughly with various DRM apps, > with DPM/suspend/resume, etc.? > I did not. I wasn't expecting suspend and resume to work on the PPP given its support is quite minimal currently. > The dw-mipi-dsi driver does some unorthodox things[1], that can lead to unbalanced > calls to these functions in some situations, and that's why all these printks > were there. To ensure the driver hooks are called correctly, while preparing > the code for upstreaming. This lead to broken display in some situations during > suspend/resume. > > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L868 > This needs to be fixed in the dw-mipi-dsi driver then. But at least we will get a panel-himax-hx8394 driver in mainline to avoid having to use downstream trees for development and testing. > Also, have you checked the clocks are actually configured correctly by the > rk3399 cru driver? I have a lot of trouble with that, too. clk driver sometimes > selects the fractional clock, but does not give it the necessary >20x difference > between input/output clock rates. You'll only notice if you measure clock rates > directly, by looking at actual refresh rate, by using some testing DRM app. > Clock subsystem sometimes shuffles things around if you switch VOPs and use big > VOP for mipi-dsi display, instead of the default small VOP. > I have not. Just verified that the display was working on my PPP and could start a mutter wayland session. We could fix the clock configuration as follow-up IMO. > I'll test this patchset in a few days against purely mainline code, but I'm > pretty sure looking at the modes you use, that this will not work on some > Pinephone Pro's, and will cause display corruption when you fix your clock > setup, so that CRU actually outputs 74.25MHz as requested by the mode. (Which > can be fixed by this patch https://github.com/megous/linux/commit/f7ee16f12ee8a44ee2472f2967ca27768106e00f) > As mentioned, I prefer to make the support incremental. First having the panel driver and then we can fix any remaining issue as follow-up patch series.
Hello Javier, On Sat, Dec 31, 2022 at 04:15:24PM +0100, Javier Martinez Canillas wrote: > Hello Ondřej, > > Thanks a lot for your comments. > > On 12/30/22 16:40, Ondřej Jirman wrote: > > Hi Javier, > > > > On Fri, Dec 30, 2022 at 12:31:52PM +0100, Javier Martinez Canillas wrote: > >> From: Kamil Trzciński <ayufan@ayufan.eu> > >> > >> The driver is for panels based on the Himax HX8394 controller, such as the > >> HannStar HSD060BHW4 720x1440 TFT LCD panel that uses a MIPI-DSI interface. > > > > I see you've removed debug printks from enable/disable/prepare/unprepare > > Yes, because as you said were debug printks. Feel free to propose adding the > debug printks if you consider useful for normal usage and not just for devel > purposes. I already did, and used them do debug and fix the issues. This submission just doesn't include the fixes. > > hooks. Have you tested the driver thoroughly with various DRM apps, > > with DPM/suspend/resume, etc.? > > > > I did not. I wasn't expecting suspend and resume to work on the PPP given its > support is quite minimal currently. System suspend/resume works and is used by distributions. Display blanking is also used by normal distros, even if you don't use system suspend/resume. > > The dw-mipi-dsi driver does some unorthodox things[1], that can lead to unbalanced > > calls to these functions in some situations, and that's why all these printks > > were there. To ensure the driver hooks are called correctly, while preparing > > the code for upstreaming. This lead to broken display in some situations during > > suspend/resume. > > > > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L868 > > > > This needs to be fixed in the dw-mipi-dsi driver then. But at least we will get > a panel-himax-hx8394 driver in mainline to avoid having to use downstream trees > for development and testing. The only thing this driver is supposed to do is to power up (+configure) and power down the display, the rest is boilerplate. Powercycling via suspend/resume and/or other means (like disabling the crtc via DRM API), has to be tested, to verify the driver can at least do a power down/up cycle and not just initial powerup. > > Also, have you checked the clocks are actually configured correctly by the > > rk3399 cru driver? I have a lot of trouble with that, too. clk driver sometimes > > selects the fractional clock, but does not give it the necessary >20x difference > > between input/output clock rates. You'll only notice if you measure clock rates > > directly, by looking at actual refresh rate, by using some testing DRM app. > > Clock subsystem sometimes shuffles things around if you switch VOPs and use big > > VOP for mipi-dsi display, instead of the default small VOP. > > > > I have not. Just verified that the display was working on my PPP and could start > a mutter wayland session. We could fix the clock configuration as follow-up IMO. The display output will be broken after you fix the assigned-clocks in DT to expected values (use GPLL parent, to make the HW generate the exact pixel clock defined in the display mode). So this needs to be dealt with now, not later. The driver issues are all known at this time and have fixes available, unlike a year ago: - panel mode not working with actual clock rate it requests (severe image corruption on some pinephone pro's) - no display output after suspend/resume cycle or a blanking/unblanking cycle So if you're submitting a known broken code, at least mention the issues in code comments, so that people that will inevitably hit the bugs will not spend large amount of time hunting for the cause again, when the issue and causes are known already. Just figuring out the image corruption took more than a year since it was discovered. Better not inflict that on others. regards, o. > > I'll test this patchset in a few days against purely mainline code, but I'm > > pretty sure looking at the modes you use, that this will not work on some > > Pinephone Pro's, and will cause display corruption when you fix your clock > > setup, so that CRU actually outputs 74.25MHz as requested by the mode. (Which > > can be fixed by this patch https://github.com/megous/linux/commit/f7ee16f12ee8a44ee2472f2967ca27768106e00f) > > > > As mentioned, I prefer to make the support incremental. First having the panel > driver and then we can fix any remaining issue as follow-up patch series. > > -- > Best regards, > > Javier Martinez Canillas > Core Platforms > Red Hat >
Hello Ondřej, On 1/2/23 11:59, Ondřej Jirman wrote: [...] >> Yes, because as you said were debug printks. Feel free to propose adding the >> debug printks if you consider useful for normal usage and not just for devel >> purposes. > > I already did, and used them do debug and fix the issues. This submission just > doesn't include the fixes. > I missed the fixes, I think that cherry-picked and squashed from your tree before you added commit f19ce7bb7d72 ("arm64: dts: rk3399-pinephone-pro: Use unused GPLL for VOPs DCLK") at least. >>> hooks. Have you tested the driver thoroughly with various DRM apps, >>> with DPM/suspend/resume, etc.? >>> >> >> I did not. I wasn't expecting suspend and resume to work on the PPP given its >> support is quite minimal currently. > > System suspend/resume works and is used by distributions. Display blanking is > also used by normal distros, even if you don't use system suspend/resume. > I know but my point was that the PPP mainline support isn't ready to be used as a daily driver in practice. So I didn't consider susped/resume or display blank as a requirement to upstream an initial support for the panel driver. [...] >>> Also, have you checked the clocks are actually configured correctly by the >>> rk3399 cru driver? I have a lot of trouble with that, too. clk driver sometimes >>> selects the fractional clock, but does not give it the necessary >20x difference >>> between input/output clock rates. You'll only notice if you measure clock rates >>> directly, by looking at actual refresh rate, by using some testing DRM app. >>> Clock subsystem sometimes shuffles things around if you switch VOPs and use big >>> VOP for mipi-dsi display, instead of the default small VOP. >>> >> >> I have not. Just verified that the display was working on my PPP and could start >> a mutter wayland session. We could fix the clock configuration as follow-up IMO. > > The display output will be broken after you fix the assigned-clocks in DT to > expected values (use GPLL parent, to make the HW generate the exact pixel clock > defined in the display mode). So this needs to be dealt with now, not later. > > > The driver issues are all known at this time and have fixes available, unlike > a year ago: > My goal was to have some initial support in mainline even if there could be some issues. IMO it is better to use upstream as a baseline and attempt to support the PPP incrementally. But since you are aware of the issues and know what are the available fixes, I'll let you continue with the effort and take care of the patches. Hopefully there may be things that will be helpful, such as the binding schema patch and the collected tags. I can also take care of pushing the DRM bits to the drm-misc-next tree once you feel that those are ready to get merged.
On Mon, Jan 02, 2023 at 02:51:42PM +0100, Javier Martinez Canillas wrote: > Hello Ondřej, > > [...] > > My goal was to have some initial support in mainline even if there could be some > issues. IMO it is better to use upstream as a baseline and attempt to support the > PPP incrementally. > > But since you are aware of the issues and know what are the available fixes, I'll > let you continue with the effort and take care of the patches. Hopefully there may > be things that will be helpful, such as the binding schema patch and the collected > tags. I can also take care of pushing the DRM bits to the drm-misc-next tree once > you feel that those are ready to get merged. Ok. The panel driver itself works fine with some changes in other DRM drivers. In fact, it will not need any changes, assuming the to be proposed fixes to dw-mipi-dsi will pass, too. So I don't have many objections against this driver itself. I'm not sure I should be giving reviewed-by to driver I co-wrote. :) Anyway, I checked it again, and only issue I found was that shutdown callback tries to disable the panel even if it may already be disabled, which will lead to unbalanced calls to regulator_disable functions, which may produce some needless warnings on shutdown/reboot. So if you want to commit this driver now, go ahead. DT will need one more round. As you say, the overall usable support for Pinephone Pro in mainline is still way off into the future, so I agree it's not necessary to get hung up on these issues. I can do a DT revision + add in the other suggested DRM patches, so that there's at least a searchable public record of the remaining issues. kind regards, o. > -- > Best regards, > > Javier Martinez Canillas > Core Platforms > Red Hat >
On 1/2/23 16:20, Ondřej Jirman wrote: > On Mon, Jan 02, 2023 at 02:51:42PM +0100, Javier Martinez Canillas wrote: >> Hello Ondřej, >> >> [...] >> >> My goal was to have some initial support in mainline even if there could be some >> issues. IMO it is better to use upstream as a baseline and attempt to support the >> PPP incrementally. >> >> But since you are aware of the issues and know what are the available fixes, I'll >> let you continue with the effort and take care of the patches. Hopefully there may >> be things that will be helpful, such as the binding schema patch and the collected >> tags. I can also take care of pushing the DRM bits to the drm-misc-next tree once >> you feel that those are ready to get merged. > > Ok. The panel driver itself works fine with some changes in other DRM drivers. > In fact, it will not need any changes, assuming the to be proposed fixes to > dw-mipi-dsi will pass, too. So I don't have many objections against this driver > itself. > Exactly, that is what I was trying to say. Awesome that you agree with that. > I'm not sure I should be giving reviewed-by to driver I co-wrote. :) Anyway, Indeed :) > I checked it again, and only issue I found was that shutdown callback tries > to disable the panel even if it may already be disabled, which will lead to > unbalanced calls to regulator_disable functions, which may produce some needless > warnings on shutdown/reboot. > > So if you want to commit this driver now, go ahead. DT will need one more round. > > As you say, the overall usable support for Pinephone Pro in mainline is still > way off into the future, so I agree it's not necessary to get hung up on these > issues. I can do a DT revision + add in the other suggested DRM patches, so that > there's at least a searchable public record of the remaining issues. > Perfect, sounds like a plan. I'll re-spin a v5 that only includes the panel patches then and drop the DTS. Thanks again for your feedback and comments!
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 737edcdf9eef..7ee9c83f09a7 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -154,6 +154,18 @@ config DRM_PANEL_FEIYANG_FY07024DI26A30D Say Y if you want to enable support for panels based on the Feiyang FY07024DI26A30-D MIPI-DSI interface. +config DRM_PANEL_HIMAX_HX8394 + tristate "HIMAX HX8394 MIPI-DSI LCD panels" + depends on OF + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y if you want to enable support for panels based on the + Himax HX8394 controller, such as the HannStar HSD060BHW4 + 720x1440 TFT LCD panel that uses a MIPI-DSI interface. + + If M is selected the module will be called panel-himax-hx8394. + config DRM_PANEL_ILITEK_IL9322 tristate "Ilitek ILI9322 320x240 QVGA panels" depends on OF && SPI diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index f8f9d9f6a307..84c01adafd4c 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -13,6 +13,7 @@ obj-$(CONFIG_DRM_PANEL_EBBG_FT8719) += panel-ebbg-ft8719.o obj-$(CONFIG_DRM_PANEL_ELIDA_KD35T133) += panel-elida-kd35t133.o obj-$(CONFIG_DRM_PANEL_FEIXIN_K101_IM2BA02) += panel-feixin-k101-im2ba02.o obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d.o +obj-$(CONFIG_DRM_PANEL_HIMAX_HX8394) += panel-himax-hx8394.o obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += panel-ilitek-ili9341.o obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o diff --git a/drivers/gpu/drm/panel/panel-himax-hx8394.c b/drivers/gpu/drm/panel/panel-himax-hx8394.c new file mode 100644 index 000000000000..fc7a2c299f8d --- /dev/null +++ b/drivers/gpu/drm/panel/panel-himax-hx8394.c @@ -0,0 +1,446 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for panels based on Himax HX8394 controller, such as: + * + * - HannStar HSD060BHW4 5.99" MIPI-DSI panel + * + * Copyright (C) 2021 Kamil Trzciński + * + * Based on drivers/gpu/drm/panel/panel-sitronix-st7703.c + * Copyright (C) Purism SPC 2019 + */ + +#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/media-bus-format.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/regulator/consumer.h> + +#include <video/mipi_display.h> + +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_modes.h> +#include <drm/drm_panel.h> + +#define DRV_NAME "panel-himax-hx8394" + +/* Manufacturer specific commands sent via DSI, listed in HX8394-F datasheet */ +#define HX8394_CMD_SETSEQUENCE 0xb0 +#define HX8394_CMD_SETPOWER 0xb1 +#define HX8394_CMD_SETDISP 0xb2 +#define HX8394_CMD_SETCYC 0xb4 +#define HX8394_CMD_SETVCOM 0xb6 +#define HX8394_CMD_SETTE 0xb7 +#define HX8394_CMD_SETSENSOR 0xb8 +#define HX8394_CMD_SETEXTC 0xb9 +#define HX8394_CMD_SETMIPI 0xba +#define HX8394_CMD_SETOTP 0xbb +#define HX8394_CMD_SETREGBANK 0xbd +#define HX8394_CMD_UNKNOWN1 0xc0 +#define HX8394_CMD_SETDGCLUT 0xc1 +#define HX8394_CMD_SETID 0xc3 +#define HX8394_CMD_SETDDB 0xc4 +#define HX8394_CMD_UNKNOWN2 0xc6 +#define HX8394_CMD_SETCABC 0xc9 +#define HX8394_CMD_SETCABCGAIN 0xca +#define HX8394_CMD_SETPANEL 0xcc +#define HX8394_CMD_SETOFFSET 0xd2 +#define HX8394_CMD_SETGIP0 0xd3 +#define HX8394_CMD_UNKNOWN3 0xd4 +#define HX8394_CMD_SETGIP1 0xd5 +#define HX8394_CMD_SETGIP2 0xd6 +#define HX8394_CMD_SETGPO 0xd6 +#define HX8394_CMD_SETSCALING 0xdd +#define HX8394_CMD_SETIDLE 0xdf +#define HX8394_CMD_SETGAMMA 0xe0 +#define HX8394_CMD_SETCHEMODE_DYN 0xe4 +#define HX8394_CMD_SETCHE 0xe5 +#define HX8394_CMD_SETCESEL 0xe6 +#define HX8394_CMD_SET_SP_CMD 0xe9 +#define HX8394_CMD_SETREADINDEX 0xfe +#define HX8394_CMD_GETSPIREAD 0xff + +struct hx8394 { + struct device *dev; + struct drm_panel panel; + struct gpio_desc *reset_gpio; + struct regulator *vcc; + struct regulator *iovcc; + bool prepared; + + const struct hx8394_panel_desc *desc; +}; + +struct hx8394_panel_desc { + const struct drm_display_mode *mode; + unsigned int lanes; + unsigned long mode_flags; + enum mipi_dsi_pixel_format format; + int (*init_sequence)(struct hx8394 *ctx); +}; + +static inline struct hx8394 *panel_to_hx8394(struct drm_panel *panel) +{ + return container_of(panel, struct hx8394, panel); +} + +static int hsd060bhw4_init_sequence(struct hx8394 *ctx) +{ + struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); + + /* 5.19.8 SETEXTC: Set extension command (B9h) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETEXTC, + 0xff, 0x83, 0x94); + + /* 5.19.2 SETPOWER: Set power (B1h) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETPOWER, + 0x48, 0x11, 0x71, 0x09, 0x32, 0x24, 0x71, 0x31, 0x55, 0x30); + + /* 5.19.9 SETMIPI: Set MIPI control (BAh) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETMIPI, + 0x63, 0x03, 0x68, 0x6b, 0xb2, 0xc0); + + /* 5.19.3 SETDISP: Set display related register (B2h) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETDISP, + 0x00, 0x80, 0x78, 0x0c, 0x07); + + /* 5.19.4 SETCYC: Set display waveform cycles (B4h) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETCYC, + 0x12, 0x63, 0x12, 0x63, 0x12, 0x63, 0x01, 0x0c, 0x7c, 0x55, + 0x00, 0x3f, 0x12, 0x6b, 0x12, 0x6b, 0x12, 0x6b, 0x01, 0x0c, + 0x7c); + + /* 5.19.19 SETGIP0: Set GIP Option0 (D3h) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETGIP0, + 0x00, 0x00, 0x00, 0x00, 0x3c, 0x1c, 0x00, 0x00, 0x32, 0x10, + 0x09, 0x00, 0x09, 0x32, 0x15, 0xad, 0x05, 0xad, 0x32, 0x00, + 0x00, 0x00, 0x00, 0x37, 0x03, 0x0b, 0x0b, 0x37, 0x00, 0x00, + 0x00, 0x0c, 0x40); + + /* 5.19.20 Set GIP Option1 (D5h) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETGIP1, + 0x19, 0x19, 0x18, 0x18, 0x1b, 0x1b, 0x1a, 0x1a, 0x00, 0x01, + 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x20, 0x21, 0x18, 0x18, + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, + 0x24, 0x25, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18); + + /* 5.19.21 Set GIP Option2 (D6h) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETGIP2, + 0x18, 0x18, 0x19, 0x19, 0x1b, 0x1b, 0x1a, 0x1a, 0x07, 0x06, + 0x05, 0x04, 0x03, 0x02, 0x01, 0x00, 0x25, 0x24, 0x18, 0x18, + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, + 0x21, 0x20, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, + 0x18, 0x18, 0x18, 0x18, 0x18, 0x18); + + /* 5.19.25 SETGAMMA: Set gamma curve related setting (E0h) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETGAMMA, + 0x00, 0x04, 0x0c, 0x12, 0x14, 0x18, 0x1a, 0x18, 0x31, 0x3f, + 0x4d, 0x4c, 0x54, 0x65, 0x6b, 0x70, 0x7f, 0x82, 0x7e, 0x8a, + 0x99, 0x4a, 0x48, 0x49, 0x4b, 0x4a, 0x4c, 0x4b, 0x7f, 0x00, + 0x04, 0x0c, 0x11, 0x13, 0x17, 0x1a, 0x18, 0x31, + 0x3f, 0x4d, 0x4c, 0x54, 0x65, 0x6b, 0x70, 0x7f, + 0x82, 0x7e, 0x8a, 0x99, 0x4a, 0x48, 0x49, 0x4b, + 0x4a, 0x4c, 0x4b, 0x7f); + + /* 5.19.17 SETPANEL (CCh) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETPANEL, 0x0b); + + /* Unknown command, not listed in the HX8394-F datasheet */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_UNKNOWN1, 0x1f, 0x31); + + /* 5.19.5 SETVCOM: Set VCOM voltage (B6h) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETVCOM, + 0x7d, 0x7d); + + /* Unknown command, not listed in the HX8394-F datasheet */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_UNKNOWN3, + 0x02); + + /* 5.19.11 Set register bank (BDh) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETREGBANK, 0x01); + + /* 5.19.2 SETPOWER: Set power (B1h) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETPOWER, 0x00); + + /* 5.19.11 Set register bank (BDh) */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_SETREGBANK, 0x00); + + /* Unknown command, not listed in the HX8394-F datasheet */ + mipi_dsi_dcs_write_seq(dsi, HX8394_CMD_UNKNOWN3, + 0xed); + + return 0; +} + +static const struct drm_display_mode hsd060bhw4_mode = { + .hdisplay = 720, + .hsync_start = 720 + 40, + .hsync_end = 720 + 40 + 46, + .htotal = 720 + 40 + 46 + 40, + .vdisplay = 1440, + .vsync_start = 1440 + 9, + .vsync_end = 1440 + 9 + 7, + .vtotal = 1440 + 9 + 7 + 7, + .clock = 74250, + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, + .width_mm = 68, + .height_mm = 136, +}; + +static const struct hx8394_panel_desc hsd060bhw4_desc = { + .mode = &hsd060bhw4_mode, + .lanes = 4, + .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST, + .format = MIPI_DSI_FMT_RGB888, + .init_sequence = hsd060bhw4_init_sequence, +}; + +static int hx8394_enable(struct drm_panel *panel) +{ + struct hx8394 *ctx = panel_to_hx8394(panel); + struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); + int ret; + + ret = ctx->desc->init_sequence(ctx); + if (ret) { + dev_err(ctx->dev, "Panel init sequence failed: %d\n", ret); + return ret; + } + + ret = mipi_dsi_dcs_exit_sleep_mode(dsi); + if (ret) { + dev_err(ctx->dev, "Failed to exit sleep mode: %d\n", ret); + return ret; + } + + /* Panel is operational 120 msec after reset */ + msleep(120); + + ret = mipi_dsi_dcs_set_display_on(dsi); + if (ret) { + dev_err(ctx->dev, "Failed to turn on the display: %d\n", ret); + goto sleep_in; + } + + return 0; + +sleep_in: + /* This will probably fail, but let's try orderly power off anyway. */ + ret = mipi_dsi_dcs_enter_sleep_mode(dsi); + if (!ret) + msleep(50); + + return ret; +} + +static int hx8394_disable(struct drm_panel *panel) +{ + struct hx8394 *ctx = panel_to_hx8394(panel); + struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); + int ret; + + ret = mipi_dsi_dcs_enter_sleep_mode(dsi); + if (ret) { + dev_err(ctx->dev, "Failed to enter sleep mode: %d\n", ret); + return ret; + } + + msleep(50); /* about 3 frames */ + + return 0; +} + +static int hx8394_unprepare(struct drm_panel *panel) +{ + struct hx8394 *ctx = panel_to_hx8394(panel); + + if (!ctx->prepared) + return 0; + + gpiod_set_value_cansleep(ctx->reset_gpio, 1); + + regulator_disable(ctx->iovcc); + regulator_disable(ctx->vcc); + + ctx->prepared = false; + + return 0; +} + +static int hx8394_prepare(struct drm_panel *panel) +{ + struct hx8394 *ctx = panel_to_hx8394(panel); + int ret; + + if (ctx->prepared) + return 0; + + gpiod_set_value_cansleep(ctx->reset_gpio, 1); + + ret = regulator_enable(ctx->vcc); + if (ret) { + dev_err(ctx->dev, "Failed to enable vcc supply: %d\n", ret); + return ret; + } + + ret = regulator_enable(ctx->iovcc); + if (ret) { + dev_err(ctx->dev, "Failed to enable iovcc supply: %d\n", ret); + goto disable_vcc; + } + + gpiod_set_value_cansleep(ctx->reset_gpio, 0); + + msleep(180); + + ctx->prepared = true; + + return 0; + +disable_vcc: + gpiod_set_value_cansleep(ctx->reset_gpio, 1); + regulator_disable(ctx->vcc); + return ret; +} + +static int hx8394_get_modes(struct drm_panel *panel, + struct drm_connector *connector) +{ + struct hx8394 *ctx = panel_to_hx8394(panel); + struct drm_display_mode *mode; + + mode = drm_mode_duplicate(connector->dev, ctx->desc->mode); + if (!mode) { + dev_err(ctx->dev, "Failed to add mode %ux%u@%u\n", + ctx->desc->mode->hdisplay, ctx->desc->mode->vdisplay, + drm_mode_vrefresh(ctx->desc->mode)); + return -ENOMEM; + } + + drm_mode_set_name(mode); + + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; + connector->display_info.width_mm = mode->width_mm; + connector->display_info.height_mm = mode->height_mm; + drm_mode_probed_add(connector, mode); + + return 1; +} + +static const struct drm_panel_funcs hx8394_drm_funcs = { + .disable = hx8394_disable, + .unprepare = hx8394_unprepare, + .prepare = hx8394_prepare, + .enable = hx8394_enable, + .get_modes = hx8394_get_modes, +}; + +static int hx8394_probe(struct mipi_dsi_device *dsi) +{ + struct device *dev = &dsi->dev; + struct hx8394 *ctx; + int ret; + + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + + ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(ctx->reset_gpio)) + return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio), + "Failed to get reset gpio\n"); + + mipi_dsi_set_drvdata(dsi, ctx); + + ctx->dev = dev; + ctx->desc = of_device_get_match_data(dev); + + dsi->mode_flags = ctx->desc->mode_flags; + dsi->format = ctx->desc->format; + dsi->lanes = ctx->desc->lanes; + + ctx->vcc = devm_regulator_get(dev, "vcc"); + if (IS_ERR(ctx->vcc)) + return dev_err_probe(dev, PTR_ERR(ctx->vcc), + "Failed to request vcc regulator\n"); + + ctx->iovcc = devm_regulator_get(dev, "iovcc"); + if (IS_ERR(ctx->iovcc)) + return dev_err_probe(dev, PTR_ERR(ctx->iovcc), + "Failed to request iovcc regulator\n"); + + drm_panel_init(&ctx->panel, dev, &hx8394_drm_funcs, + DRM_MODE_CONNECTOR_DSI); + + ret = drm_panel_of_backlight(&ctx->panel); + if (ret) + return ret; + + drm_panel_add(&ctx->panel); + + ret = mipi_dsi_attach(dsi); + if (ret < 0) { + dev_err_probe(dev, ret, "mipi_dsi_attach failed\n"); + drm_panel_remove(&ctx->panel); + return ret; + } + + dev_dbg(dev, "%ux%u@%u %ubpp dsi %udl - ready\n", + ctx->desc->mode->hdisplay, ctx->desc->mode->vdisplay, + drm_mode_vrefresh(ctx->desc->mode), + mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes); + + return 0; +} + +static void hx8394_shutdown(struct mipi_dsi_device *dsi) +{ + struct hx8394 *ctx = mipi_dsi_get_drvdata(dsi); + int ret; + + ret = drm_panel_disable(&ctx->panel); + if (ret < 0) + dev_err(&dsi->dev, "Failed to disable panel: %d\n", ret); + + ret = drm_panel_unprepare(&ctx->panel); + if (ret < 0) + dev_err(&dsi->dev, "Failed to unprepare panel: %d\n", ret); +} + +static void hx8394_remove(struct mipi_dsi_device *dsi) +{ + struct hx8394 *ctx = mipi_dsi_get_drvdata(dsi); + int ret; + + hx8394_shutdown(dsi); + + ret = mipi_dsi_detach(dsi); + if (ret < 0) + dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret); + + drm_panel_remove(&ctx->panel); +} + +static const struct of_device_id hx8394_of_match[] = { + { .compatible = "hannstar,hsd060bhw4", .data = &hsd060bhw4_desc }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, hx8394_of_match); + +static struct mipi_dsi_driver hx8394_driver = { + .probe = hx8394_probe, + .remove = hx8394_remove, + .shutdown = hx8394_shutdown, + .driver = { + .name = DRV_NAME, + .of_match_table = hx8394_of_match, + }, +}; +module_mipi_dsi_driver(hx8394_driver); + +MODULE_AUTHOR("Kamil Trzciński <ayufan@ayufan.eu>"); +MODULE_DESCRIPTION("DRM driver for Himax HX8394 based MIPI DSI panels"); +MODULE_LICENSE("GPL");