Hi Joseph,
On 08/04/2025, Joseph Guo wrote:
> Waveshare touchscreen consists of a DPI panel and a driver board.
> The waveshare driver board consists of ICN6211 and a MCU to
> convert DSI to DPI and control the backlight.
> This driver treats the MCU and ICN6211 board as a whole unit.
> It can support all resolution waveshare DSI2DPI based panel,
> the timing table should come from 'panel-dpi' panel in the device tree.
>
> Signed-off-by: Joseph Guo <qijian....@nxp.com>
For next version, you may add:
Suggested-by: Liu Ying <victor....@nxp.com>
> ---
> drivers/gpu/drm/bridge/Kconfig | 11 ++
> drivers/gpu/drm/bridge/Makefile | 1 +
> drivers/gpu/drm/bridge/waveshare-dsi.c | 210
> +++++++++++++++++++++++++++++++++
> 3 files changed, 222 insertions(+)
This patch doesn't apply to drm-misc-next cleanly. I see conflicts in Kconfig
and Makefile. It seems that you generate the patch series based on NXP down
stream kernel instead of the upstream kernel.
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index
> cb3b797fcea1c73e83c9187fef6582296b340305..26fec25c61ed7d950c094e0224f1196946079485
> 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -472,4 +472,15 @@ config DRM_ITE_IT6161
> help
> ITE IT6161 bridge chip driver.
>
> +config DRM_WAVESHARE_BRIDGE
Sort the config names alphabetically.
> + tristate "Waveshare DSI bridge"
depends on BACKLIGHT_CLASS_DEVICE
> + depends on OF
> + select DRM_PANEL_BRIDGE
> + select DRM_KMS_HELPER
> + select DRM_MIPI_DSI
> + select REGMAP_I2C
> + help
> + Driver for waveshare DSI to DPI bridge board.
> + Please say Y if you have such hardware
> +
> endmenu
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index
> d1db90688a150fdc3a5fd40acebe740798c452b0..3caa4d8f71675804328aa5a51ec67b2587938621
> 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -48,3 +48,4 @@ obj-$(CONFIG_DRM_ITE_IT6263) += it6263.o
> obj-$(CONFIG_DRM_ITE_IT6161) += it6161.o
> obj-$(CONFIG_DRM_SEC_MIPI_DSIM) += sec-dsim.o
> obj-$(CONFIG_DRM_NXP_SEIKO_43WVFIG) += nxp-seiko-43wvfig.o
> +obj-$(CONFIG_DRM_WAVESHARE_BRIDGE) += waveshare-dsi.o
Sort the config names alphabetically with the best effort.
> diff --git a/drivers/gpu/drm/bridge/waveshare-dsi.c
> b/drivers/gpu/drm/bridge/waveshare-dsi.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..efb3a2fc501b5725b02f49862526d1704a3a4b7b
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/waveshare-dsi.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2025 NXP
Nit: Drop a space between * and C.
> + * Based on panel-raspberrypi-touchscreen by Broadcom
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_graph.h>
> +#include <linux/regmap.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_print.h>
> +
> +struct ws_bridge {
> + struct drm_bridge bridge;
> + struct drm_bridge *next_bridge;
> + struct backlight_device *backlight;
> + struct device *dev;
> + struct regmap *reg_map;
> +};
> +
> +static const struct regmap_config ws_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = 0xff,
> + .disable_debugfs = true,
drivers/gpu/drm/bridge/waveshare-dsi.c:34:10: error: ‘const struct
regmap_config’ has no member named ‘disable_debugfs’
34 | .disable_debugfs = true,
| ^~~~~~~~~~~~~~~
> +};
> +
> +static struct ws_bridge *bridge_to_ws_bridge(struct drm_bridge *bridge)
> +{
> + return container_of(bridge, struct ws_bridge, bridge);
> +}
> +
> +static int ws_bridge_attach_dsi(struct ws_bridge *ws)
> +{
> + struct device_node *dsi_host_node;
> + struct mipi_dsi_host *host;
> + struct mipi_dsi_device *dsi;
> + const struct mipi_dsi_device_info info = {
> + .type = "ws-bridge",
> + .channel = 0,
> + .node = NULL,
> + };
> + struct device *dev = ws->dev;
> + int ret;
Nit: Sort these variables in reverse Christmas tree fashion.
> +
> + dsi_host_node = of_graph_get_remote_node(dev->of_node, 0, 0);
> + if (!dsi_host_node) {
> + dev_err(dev, "Failed to get remote port\n");
> + return -ENODEV;
> + }
> +
> + host = of_find_mipi_dsi_host_by_node(dsi_host_node);
> +
Nit: Drop this blank line.
> + of_node_put(dsi_host_node);
> + if (!host)
> + return dev_err_probe(dev, -EPROBE_DEFER, "Failed to find
> dsi_host\n");
> +
> + dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
> +
Ditto.
> + if (IS_ERR(dsi))
> + return dev_err_probe(dev, PTR_ERR(dsi), "Failed to create dsi
> device\n");
> +
> + dsi->mode_flags = MIPI_DSI_MODE_VIDEO_HSE | MIPI_DSI_MODE_VIDEO |
> + MIPI_DSI_CLOCK_NON_CONTINUOUS;
> + dsi->format = MIPI_DSI_FMT_RGB888;
> + dsi->lanes = 2;
> +
> + ret = devm_mipi_dsi_attach(dev, dsi);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Failed to attach dsi to
> host\n");
> +
> + return 0;
> +}
> +
> +static int ws_bridge_bridge_attach(struct drm_bridge *bridge,
> + enum drm_bridge_attach_flags flags)
drivers/gpu/drm/bridge/waveshare-dsi.c:117:19: error: initialization of ‘int
(*)(struct drm_bridge *, struct drm_encoder *, enum drm_bridge_attach_flags)’
from incompatible pointer type ‘int (*)(struct drm_bridge *, enum
drm_bridge_attach_flags)’ [-Werror=incompatible-pointer-types]
117 | .attach = ws_bridge_bridge_attach,
| ^~~~~~~~~~~~~~~~~~~~~~~
> +{
> + struct ws_bridge *ws = bridge_to_ws_bridge(bridge);
> + int ret;
> +
> + ret = ws_bridge_attach_dsi(ws);
> + if (ret)
> + return ret;
> +
> + return drm_bridge_attach(ws->bridge.encoder, ws->next_bridge,
> + &ws->bridge, flags);
> +}
> +
> +static void ws_bridge_bridge_enable(struct drm_bridge *bridge)
> +{
> + struct ws_bridge *ws = bridge_to_ws_bridge(bridge);
> +
> + regmap_write(ws->reg_map, 0xad, 0x01);
> + backlight_enable(ws->backlight);
> +}
> +
> +static void ws_bridge_bridge_disable(struct drm_bridge *bridge)
> +{
> + struct ws_bridge *ws = bridge_to_ws_bridge(bridge);
> +
> + backlight_disable(ws->backlight);
> + regmap_write(ws->reg_map, 0xad, 0x00);
> +}
> +
> +static const struct drm_bridge_funcs ws_bridge_bridge_funcs = {
> + .enable = ws_bridge_bridge_enable,
> + .disable = ws_bridge_bridge_disable,
> + .attach = ws_bridge_bridge_attach,
> +};
> +
> +static int ws_bridge_bl_update_status(struct backlight_device *bl)
> +{
> + struct ws_bridge *ws = bl_get_data(bl);
> +
> + regmap_write(ws->reg_map, 0xab, 0xff - backlight_get_brightness(bl));
> + regmap_write(ws->reg_map, 0xaa, 0x01);
> +
> + return 0;
> +}
> +
> +static const struct backlight_ops ws_bridge_bl_ops = {
> + .update_status = ws_bridge_bl_update_status,
> +};
> +
> +static struct backlight_device *ws_bridge_create_backlight(struct ws_bridge
> *ws)
> +{
> + struct device *dev = ws->dev;
> + const struct backlight_properties props = {
> + .type = BACKLIGHT_RAW,
> + .brightness = 255,
> + .max_brightness = 255,
> + };
Nit: Sort these variables in reverse Christmas tree fashion.
> +
> + return devm_backlight_device_register(dev, dev_name(dev), dev, ws,
> + &ws_bridge_bl_ops, &props);
> +}
> +
> +static int ws_bridge_probe(struct i2c_client *i2c)
> +{
> + struct device *dev = &i2c->dev;
> + struct ws_bridge *ws;
> + struct drm_panel *panel;
> + int ret;
> + struct backlight_device *backlight;
Ditto.
> +
> + ws = devm_kzalloc(dev, sizeof(*ws), GFP_KERNEL);
Recently upstream bridge drivers were changed to use devm_drm_bridge_alloc()
to allocate the main structure which contains the DRM bridge member.
devm_kzalloc() is no more allowed to be used.
> + if (!ws)
> + return -ENOMEM;
> +
> + ws->dev = dev;
> +
> + ws->reg_map = devm_regmap_init_i2c(i2c, &ws_regmap_config);
> + if (IS_ERR(ws->reg_map))
> + return dev_err_probe(dev, PTR_ERR(ws->reg_map), "Failed to
> allocate regmap\n");
> +
> + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, -1, &panel, NULL);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to find remote panel\n");
> +
> + ws->next_bridge = devm_drm_panel_bridge_add(dev, panel);
> + if (IS_ERR(ws->next_bridge))
> + return PTR_ERR(ws->next_bridge);
> +
> + ws->backlight = ws_bridge_create_backlight(ws);
> + if (IS_ERR(backlight)) {
> + ret = PTR_ERR(backlight);
> + dev_err(dev, "Failed to create backlight: %d\n", ret);
> + return ret;
> + }
> +
> + regmap_write(ws->reg_map, 0xc0, 0x01);
> + regmap_write(ws->reg_map, 0xc2, 0x01);
> + regmap_write(ws->reg_map, 0xac, 0x01);
> +
> + ws->bridge.funcs = &ws_bridge_bridge_funcs;
> + ws->bridge.type = DRM_MODE_CONNECTOR_DPI;
> + ws->bridge.of_node = dev->of_node;
> + devm_drm_bridge_add(dev, &ws->bridge);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ws_bridge_of_ids[] = {
> + {.compatible = "waveshare,dsi2dpi",},
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(of, ws_bridge_of_ids);
> +
> +static struct i2c_driver ws_bridge_driver = {
> + .driver = {
> + .name = "ws_dsi2dpi",
> + .of_match_table = ws_bridge_of_ids,
> + },
> + .probe = ws_bridge_probe,
> +};
> +module_i2c_driver(ws_bridge_driver);
> +
> +MODULE_AUTHOR("Joseph Guo <qijian....@nxp.com>");
> +MODULE_DESCRIPTION("Waveshare DSI2DPI bridge driver");
> +MODULE_LICENSE("GPL");
>
--
Regards,
Liu Ying
