ср, 30 вер. 2026 р. о 12:02 Thierry Reding <[email protected]> пише:
>
> On Wed, Sep 30, 2026 at 10:05:35AM +0300, Svyatoslav Ryhel wrote:
> > Add a driver for panels used in LG Optimus 2X P990. Both panels are 4"
> > WVGA MIPI DBI Type B linked to DRM encoder via RGB to DBI bridge.
> >
> > Signed-off-by: Svyatoslav Ryhel <[email protected]>
> > ---
> >  drivers/gpu/drm/panel/Kconfig                 |  14 +
> >  drivers/gpu/drm/panel/Makefile                |   1 +
> >  .../drm/panel/panel-hitachi-tx10d07vm0baa.c   | 396 ++++++++++++++++++
> >  3 files changed, 411 insertions(+)
> >  create mode 100644 drivers/gpu/drm/panel/panel-hitachi-tx10d07vm0baa.c
> >
> > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > index 747f47347521a..47962197a9c76 100644
> > --- a/drivers/gpu/drm/panel/Kconfig
> > +++ b/drivers/gpu/drm/panel/Kconfig
> > @@ -273,6 +273,20 @@ config DRM_PANEL_HIMAX_HX8394
> >
> >         If M is selected the module will be called panel-himax-hx8394.
> >
> > +config DRM_PANEL_HITACHI_TX10D07VM0BAA
> > +     tristate "Hitachi TX10D07VM0BAA and LG LH400WV3 MIPI DBI panels"
> > +     depends on OF
> > +     depends on BACKLIGHT_CLASS_DEVICE
> > +     select DRM_MIPI_DBI
> > +     select VIDEOMODE_HELPERS
> > +     help
> > +       Say Y here if you want to enable support for the HITACHI
> > +       TX10D07VM0BAA and LG LH400WV3 MIPI DBI panels found in the
> > +       LG Optimus 2X P990 smartphone.
> > +
> > +       To compile this driver as a module, choose M here: the module will
> > +       be called panel-hitachi-tx10d07vm0baa.
> > +
> >  config DRM_PANEL_HYDIS_HV101HD1
> >       tristate "Hydis HV101HD1 panel"
> >       depends on OF
> > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > index f2c9c80a218f0..414267a49d922 100644
> > --- a/drivers/gpu/drm/panel/Makefile
> > +++ b/drivers/gpu/drm/panel/Makefile
> > @@ -27,6 +27,7 @@ obj-$(CONFIG_DRM_PANEL_HIMAX_HX83112A) += 
> > panel-himax-hx83112a.o
> >  obj-$(CONFIG_DRM_PANEL_HIMAX_HX83112B) += panel-himax-hx83112b.o
> >  obj-$(CONFIG_DRM_PANEL_HIMAX_HX83121A) += panel-himax-hx83121a.o
> >  obj-$(CONFIG_DRM_PANEL_HIMAX_HX8394) += panel-himax-hx8394.o
> > +obj-$(CONFIG_DRM_PANEL_HITACHI_TX10D07VM0BAA) += 
> > panel-hitachi-tx10d07vm0baa.o
> >  obj-$(CONFIG_DRM_PANEL_HYDIS_HV101HD1) += panel-hydis-hv101hd1.o
> >  obj-$(CONFIG_DRM_PANEL_ILITEK_ILI7807S) += panel-ilitek-ili7807s.o
> >  obj-$(CONFIG_DRM_PANEL_ILITEK_ILI7836A) += panel-ilitek-ili7836a.o
> > diff --git a/drivers/gpu/drm/panel/panel-hitachi-tx10d07vm0baa.c 
> > b/drivers/gpu/drm/panel/panel-hitachi-tx10d07vm0baa.c
> > new file mode 100644
> > index 0000000000000..d2c3b63649288
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panel/panel-hitachi-tx10d07vm0baa.c
> > @@ -0,0 +1,396 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include <linux/array_size.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/media-bus-format.h>
> > +#include <linux/module.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/property.h>
> > +#include <linux/regulator/consumer.h>
> > +
> > +#include <video/mipi_display.h>
> > +
> > +#include <drm/drm_mipi_dbi.h>
> > +#include <drm/drm_modes.h>
> > +#include <drm/drm_of.h>
> > +#include <drm/drm_panel.h>
> > +#include <drm/drm_probe_helper.h>
> > +
> > +enum panel_dbi_id {
> > +     PANEL_DBI_NONE,
> > +     PANEL_DBI_TX10D07VM0BAA,
> > +     PANEL_DBI_LH400WV3,
> > +};
> > +
> > +static const struct regulator_bulk_data panel_dbi_supplies[] = {
> > +     { .supply = "avci" }, { .supply = "iovcc" },
> > +};
> > +
> > +struct panel_dbi {
> > +     struct drm_panel panel;
> > +     struct mipi_dbi *dbi;
> > +
> > +     struct regulator_bulk_data *supplies;
> > +     struct gpio_desc *reset_gpio;
> > +};
> > +
> > +static inline struct panel_dbi *to_panel_dbi(struct drm_panel *panel)
> > +{
> > +     return container_of(panel, struct panel_dbi, panel);
> > +}
> > +
> > +#define panel_dbi_command(priv, cmd, seq...) \
> > +({ \
> > +     const u8 d[] = { seq }; \
> > +     struct drm_panel *panel = &(priv)->panel; \
> > +     struct device *dev = panel->dev; \
> > +     int ret; \
> > +     ret = mipi_dbi_command_stackbuf((priv)->dbi, cmd, d, ARRAY_SIZE(d)); \
> > +     if (ret) \
> > +             dev_err_ratelimited(dev, "error %d when sending command 
> > %#02x\n", ret, cmd); \
> > +     ret; \
> > +})
> > +
>
> [...]
> > +static int panel_dbi_probe(struct platform_device *pdev)
> > +{
> > +     struct device *dev = &pdev->dev;
> > +     const struct drm_panel_funcs *panel_dbi_funcs;
> > +     struct panel_dbi *priv;
> > +     enum panel_dbi_id id;
> > +     int ret;
> > +
> > +     id = (uintptr_t)of_device_get_match_data(dev);
> > +
> > +     switch (id) {
> > +     case PANEL_DBI_TX10D07VM0BAA:
> > +             panel_dbi_funcs = &hitachi_tx10d07vm0baa_panel_funcs;
> > +             break;
> > +
> > +     case PANEL_DBI_LH400WV3:
> > +             panel_dbi_funcs = &lg_lh400wv3_panel_funcs;
> > +             break;
> > +
> > +     default:
> > +             return dev_err_probe(dev, -ENODEV, "Unknown device %d\n", id);
> > +     }
>
> This is a bit pointless. The only reason you need that default here is
> because you have an enum that is "none" but that PANEL_DBI_NONE is never
> even used.
>
> > +
> > +     priv = devm_drm_panel_alloc(dev, struct panel_dbi, panel,
> > +                                 panel_dbi_funcs, DRM_MODE_CONNECTOR_DPI);
> > +     if (IS_ERR(priv))
> > +             return PTR_ERR(priv);
> > +
> > +     ret = devm_regulator_bulk_get_const(dev, 
> > ARRAY_SIZE(panel_dbi_supplies),
> > +                                         panel_dbi_supplies, 
> > &priv->supplies);
> > +     if (ret)
> > +             return dev_err_probe(dev, ret, "Failed to get supplies\n");
> > +
> > +     priv->reset_gpio = devm_gpiod_get_optional(dev, "reset", 
> > GPIOD_OUT_HIGH);
> > +     if (IS_ERR(priv->reset_gpio))
> > +             return dev_err_probe(dev, PTR_ERR(priv->reset_gpio),
> > +                                  "Failed to get reset gpio\n");
> > +
> > +     ret = drm_panel_of_backlight(&priv->panel);
> > +     if (ret)
> > +             return dev_err_probe(dev, ret, "Failed to get backlight\n");
> > +
> > +     ret = devm_drm_panel_add(dev, &priv->panel);
> > +     if (ret)
> > +             return dev_err_probe(dev, ret, "Failed to add panel\n");
> > +
> > +     platform_set_drvdata(pdev, priv);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct of_device_id panel_dbi_of_match[] = {
> > +     { .compatible = "hit,tx10d07vm0baa", .data = (void 
> > *)PANEL_DBI_TX10D07VM0BAA },
> > +     { .compatible = "lg,lh400wv3-sd04", .data = (void 
> > *)PANEL_DBI_LH400WV3 },
>
> Why the detour through that PANEL_DB_* enum? You could just pass the
> panel funcs pointers directly via .data here.
>

Passing API/OPS via .data is discouraged.

> Also, looking at the enable/disable sequences these are in fact two
> different drivers, with the only commonality being that they happen to
> be used in the same device. Rolling them both into one driver seems a
> bit odd.

I did this to simplify maintainance. Both panels are used in the LG
Optimus 2X. My assumption is that LG switched one to another at some
point, hence they share same timings, controls and supplies, but
differ in en/disable sequence. Additionally, these are the the only
DBI Type B-only panels in the kernel, from what I can see.

> If you really want to avoid duplication, maybe they should go
> into some kind of "simple" or "generic" DBI driver.

DBI Type B is not well supported in the kernel.

>
> Thierry

Reply via email to