Re: [PATCH v17 2/2] drm/bridge: Add Lontium LT7911EXC eDP to MIPI DSI bridge
From: Dmitry Baryshkov
Date: Tue Jul 28 2026 - 06:09:45 EST
On Wed, Jul 15, 2026 at 09:54:38AM +0800, syyang@xxxxxxxxxxx wrote:
> From: Sunyun Yang <syyang@xxxxxxxxxxx>
>
> Add support for the Lontium LT7911EXC bridge chip, which converts
> eDP input to MIPI DSI output using an internal firmware-controlled
> pipeline.
>
> The driver provides:
> - DRM bridge integration for eDP-to-DSI routing
> - MIPI DSI host interface for downstream panel attachment
> - Firmware upgrade mechanism over I2C (erase/program/verify)
> - GPIO-based reset and regulator management
>
> Display timing and MIPI DCS packet generation are handled by the chip
> firmware and are not configured by the driver.
>
> Signed-off-by: Sunyun Yang <syyang@xxxxxxxxxxx>
> ---
> +static void lt7911exc_reset(struct lt7911exc *lt7911exc)
> +{
> + /* Assert reset pin: logical 1 -> physical state low (Reset Active) */
Redundant comment.
> + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 1);
> + usleep_range(5000, 6000);
> +
> + /* Deassert reset pin: logical 0 -> physical state high (Run state) */
This one too.
> + gpiod_set_value_cansleep(lt7911exc->reset_gpio, 0);
> + msleep(400);
> +}
> +
[...]
> +static void lt7911exc_firmware_upgrade_work(struct work_struct *work)
> +{
> + struct lt7911exc *lt7911exc = container_of(work, struct lt7911exc, work);
> + struct device *dev = lt7911exc->dev;
> + const struct firmware *fw;
> + u8 *buffer;
> + size_t total_size = FW_SIZE - 4;
> + u32 crc32, version;
> + int ret;
> +
> + mutex_lock(<7911exc->upgrade_lock);
> +
> + scoped_guard(mutex, <7911exc->ocm_lock) {
> + if (lt7911exc->removed)
> + goto out_unlock_upgrade;
> + }
> +
> + ret = request_firmware(&fw, FW_FILE, dev);
> + if (ret) {
> + dev_err(dev, "failed to load '%s'\n", FW_FILE);
> + goto out_unlock_upgrade;
> + }
> +
> + if (fw->size > total_size) {
> + dev_err(dev, "firmware too large (%zu > %zu)\n", fw->size, total_size);
> + goto out_release_fw;
> + }
> +
> + buffer = kvmalloc(total_size, GFP_KERNEL);
> + if (!buffer)
> + goto out_release_fw;
> +
> + memset(buffer, 0xff, total_size);
> + memcpy(buffer, fw->data, fw->size);
memcpy(buffer, fw->data, fw->size);
memset(buffer + fw->size, 0xff, total_size - fw->size);
> + crc32 = cal_crc32_custom(buffer, total_size);
> + kvfree(buffer);
> +
> + lt7911exc_reset(lt7911exc);
> +
> + scoped_guard(mutex, <7911exc->ocm_lock) {
> + ret = lt7911exc_inside_mcu_stop(lt7911exc);
Move guard() into the function.
> + }
> + if (ret)
> + goto out_release_fw;
> +
> + ret = lt7911exc_block_erase(lt7911exc);
> + if (ret) {
> + dev_err(dev, "failed to block erase.\n");
> + goto out_mcu_run;
> + }
> +
> + ret = lt7911exc_write_data(lt7911exc, fw, 0);
> + if (ret < 0) {
> + dev_err(dev, "failed to write firmware data\n");
> + goto out_mcu_run;
> + }
> +
> + ret = lt7911exc_write_crc(lt7911exc, crc32, FW_SIZE - 4);
> + if (ret < 0) {
> + dev_err(dev, "failed to write firmware crc\n");
> + goto out_mcu_run;
> + }
> +
> + lt7911exc_reset(lt7911exc);
> +
> + ret = lt7911exc_upgrade_result(lt7911exc, crc32);
> + if (ret)
> + dev_err(dev, "firmware verification failed\n");
> +
> + /*
> + * Always restore MCU to running state: flash is already erased,
> + * leaving MCU stopped would render the chip completely unusable.
> + */
> + scoped_guard(mutex, <7911exc->ocm_lock) {
> + ret = lt7911exc_inside_mcu_run(lt7911exc);
Move guard() into the run function.
> + if (!ret) {
> + ret = lt7911exc_read_version(lt7911exc, &version);
Why is ihis being executed under the OCM lock?
> + if (!ret)
> + lt7911exc->fw_version = version;
> + }
> + }
> +
> + if (ret)
> + dev_err(dev, "failed to read version after upgrade\n");
> +
> + goto out_release_fw;
> +
> +out_mcu_run:
> + scoped_guard(mutex, <7911exc->ocm_lock)
> + lt7911exc_inside_mcu_run(lt7911exc);
> +
> +out_release_fw:
> + release_firmware(fw);
release_firmware() could have happened long ago, after copying the
buffer.
> +
> +out_unlock_upgrade:
> + scoped_guard(mutex, <7911exc->ocm_lock) {
> + if (!lt7911exc->removed)
> + lt7911exc->upgrade = false;
> + }
> +
> + /* Notify DRM to re-trigger modeset after firmware upgrade */
> + if (!lt7911exc->removed && lt7911exc->bridge.dev)
> + drm_kms_helper_hotplug_event(lt7911exc->bridge.dev);
drm_bridge_hpd_notify(), outside of the lock.
> +
> + mutex_unlock(<7911exc->upgrade_lock);
> +}
> +
> +static void lt7911exc_atomic_pre_enable(struct drm_bridge *bridge, struct drm_atomic_commit *state)
> +{
> + struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge);
> + struct device *dev = lt7911exc->dev;
> + int ret;
> +
> + guard(mutex)(<7911exc->ocm_lock);
> +
> + if (!lt7911exc->upgrade) {
Why? Can't you just use a lock to prevent DRM vs firmware upgrade races.
> + ret = regmap_write(lt7911exc->regmap, 0xe0b0, 0x01);
What will enable the output if bridge was upgrading during
atomic_pre_enable?
> + if (ret)
> + dev_err(dev, "failed to enable mipi output stream\n");
> + }
> +}
> +
> +static void lt7911exc_atomic_post_disable(struct drm_bridge *bridge,
> + struct drm_atomic_commit *state)
> +{
> + struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge);
> + struct device *dev = lt7911exc->dev;
> + int ret;
> +
> + guard(mutex)(<7911exc->ocm_lock);
> +
> + if (!lt7911exc->upgrade) {
> + ret = regmap_write(lt7911exc->regmap, 0xe0b0, 0x00);
> + if (ret)
> + dev_err(dev, "failed to disable mipi output stream\n");
> + }
> +}
> +
> +static int lt7911exc_bridge_attach(struct drm_bridge *bridge,
> + struct drm_encoder *encoder,
> + enum drm_bridge_attach_flags flags)
> +{
> + struct lt7911exc *lt7911exc = bridge_to_lt7911exc(bridge);
> +
> + if (!drm_core_check_feature(bridge->dev, DRIVER_ATOMIC)) {
> + dev_err(lt7911exc->dev, "needs atomic updates support\n");
> + return -ENOTSUPP;
> + }
I'd say, it's redundant.
> +
> + return drm_bridge_attach(encoder, lt7911exc->output_bridge, bridge, flags);
> +}
> +
> +static const struct drm_bridge_funcs lt7911exc_bridge_funcs = {
> + .attach = lt7911exc_bridge_attach,
> + .atomic_pre_enable = lt7911exc_atomic_pre_enable,
> + .atomic_post_disable = lt7911exc_atomic_post_disable,
> + .atomic_reset = drm_atomic_helper_bridge_reset,
> + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +};
> +
> +static int lt7911exc_dsi_host_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev)
> +{
> + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host);
> + struct drm_bridge *bridge;
> +
> + /* currently do not support connecting several DSI devices to the same host */
> + if (lt7911exc->output_bridge)
> + return -EBUSY;
> +
> + if (dev->lanes > 4) {
> + dev_err(lt7911exc->dev, "unsupported number of data lanes(%u)\n", dev->lanes);
> + return -EINVAL;
> + }
> +
> + bridge = devm_drm_of_get_bridge(lt7911exc->dev, host->dev->of_node, 1, 0);
> + if (IS_ERR(bridge)) {
> + dev_err(lt7911exc->dev, "failed to add DSI device\n");
> + return PTR_ERR(bridge);
> + }
> +
> + lt7911exc->output_bridge = bridge;
Follow what other drivers are doing. Please store the next bridge in
drm_bridge::next_bridge. It saves you from manual handling of its
refcounts.
> +
> + /*
> + * Force panel-first enable order: the DSI output stream (0xe0b0)
> + * must not be enabled before the panel is powered on.
> + * This override is safe because LT7911EXC's transfer() is a stub —
> + * panel DCS commands are silently discarded and panel init is done
Is it a hardware limitation or something which is not yet implemented in
the driver?
> + * by the chip's internal firmware. The panel driver's
> + * prepare_prev_first preference is therefore irrelevant here.
> + */
Do you have an actual issue with one of the panels? The overall logic
should be to let the panels decide whether it needs to setup anything
before the DSI output stream comes up. If there are any issues, they
should be fixed on the DSI host side.
> + lt7911exc->output_bridge->pre_enable_prev_first = false;
> +
> + drm_bridge_add(<7911exc->bridge);
> +
> + return 0;
> +}
> +
> +static int lt7911exc_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev)
> +{
> + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host);
> +
> + drm_bridge_remove(<7911exc->bridge);
> +
> + return 0;
> +}
> +
> +/*
> + * LT7911EXC's internal MCU owns the DSI link and handles all panel
> + * initialization. The host transfer() is a no-op sink: accept the
How does it know panel init commands? What about the panels which use
DCS commands to setup brightness?
> + * message and report it as sent so the panel driver does not abort
> + * its init sequence. Actual DSI transmission is done by chip firmware.
> + */
> +static ssize_t lt7911exc_dsi_host_transfer(struct mipi_dsi_host *host,
> + const struct mipi_dsi_msg *msg)
> +{
> + struct lt7911exc *lt7911exc = dsi_host_to_lt7911exc(host);
> +
> + if (msg->rx_len) {
> + dev_warn(lt7911exc->dev, "MIPI rx is not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + switch (msg->type) {
> + case MIPI_DSI_DCS_SHORT_WRITE:
> + case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
> + case MIPI_DSI_DCS_LONG_WRITE:
> + case MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM:
> + case MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM:
> + case MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM:
> + case MIPI_DSI_GENERIC_LONG_WRITE:
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + guard(mutex)(<7911exc->ocm_lock);
> +
> + if (lt7911exc->upgrade)
> + return -EBUSY;
> +
> + return msg->tx_len;
> +}
> +
> +static const struct mipi_dsi_host_ops lt7911exc_dsi_host_ops = {
> + .attach = lt7911exc_dsi_host_attach,
> + .detach = lt7911exc_dsi_host_detach,
> + .transfer = lt7911exc_dsi_host_transfer,
> +};
> +
--
With best wishes
Dmitry