Hi
>-----Original Message-----
>From: Dmitry Baryshkov <[email protected]>
>Sent: Saturday, March 14, 2026 10:09 AM
>To: Hermes Wu (吳佳宏) <[email protected]>
>Cc: Andrzej Hajda <[email protected]>; Neil Armstrong
><[email protected]>; Robert Foss <[email protected]>; Laurent Pinchart
><[email protected]>; Jonas Karlman <[email protected]>; Jernej
>Skrabec <[email protected]>; David Airlie <[email protected]>; Simona
>Vetter <[email protected]>; Maarten Lankhorst
><[email protected]>; Maxime Ripard <[email protected]>;
>Thomas Zimmermann <[email protected]>; Rob Herring <[email protected]>;
>Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>;
>Pet Weng (翁玉芬) <[email protected]>; Kenneth Hung (洪家倫)
><[email protected]>; [email protected];
>[email protected]; [email protected]
>Subject: Re: [PATCH v3 2/2] drm/bridge: Add ITE IT6162 MIPI DSI to HDMI bridge
>driver
>
>On Fri, Mar 13, 2026 at 02:16:01PM +0800, Hermes Wu via B4 Relay wrote:
>> From: Hermes Wu <[email protected]>
>>
>> Add support for the ITE IT6162 MIPI DSI to HDMI 2.0 bridge chip.
>> The IT6162 is an I2C-controlled bridge that supports the following
>> configurations:
>>
>> - Single MIPI DSI input: up to 4K @ 30Hz
>> - Dual MIPI DSI input (combined): up to 4K @ 60Hz
>>
>> The driver implements the DRM bridge and connector frameworks,
>> including mode setting, EDID retrieval, and HPD support.
>>
>> Also add a MAINTAINERS entry for the newly introduced ITE IT6162 MIPI
>> DSI to HDMI bridge driver, covering the driver source file and the
>> device tree binding document.
>>
>> Signed-off-by: Hermes Wu <[email protected]>
>> ---
>> Changes in v3:
>> * Fix OFFSET_VERSION_H register offset from 0x03 to 0x05
>> * Add MIPI_PORT_EN_MASK macro combining MIPI_PORT1_EN_MASK and
>> MIPI_PORT0_EN_MASK
>> * Rename HDCP enums: NO_HDCP -> HDCP_DISABLE, NO_HDCP_STATE ->
>> HDCP_STATE_IDLE,
>> AUTH_DONE -> HDCP_STATE_AUTH_DONE, AUTH_FAIL -> HDCP_STATE_AUTH_FAIL
>> * Rename it6162_infoblock_complete() to it6162_wait_command_complete()
>> * Rename it6162_infoblock_host_set() to it6162_infoblock_trigger()
>> * Remove it6162_infoblock_mipi_config() and it6162_infoblock_write_msg()
>> wrappers, inline into it6162_reset_init() and
>> it6162_mipi_set_video_timing() respectively
>> * Remove it6162_set_default_config() wrapper, inline into
>> it6162_reset_init()
>> * Fix typo: hdcp_encyption -> hdcp_encryption
>> * Fix typo: it6162_hdcp_read_infomation -> it6162_hdcp_read_information
>> * Remove dev_err_probe() usage outside of probe path
>> * Remove verbose success-path dev_info/dev_dbg logging throughout
>> * Replace __func__ usage in error messages with descriptive strings
>> * Fix double error printing in probe for it6162_init_pdata failure path
>> * Fix uninitialized variable warning: initialize cp_status to
>> DRM_MODE_CONTENT_PROTECTION_DESIRED at declaration; move
>> drm_hdcp_update_content_protection() inside the state-change block
>> * Fix audio sample width mapping: case 20 now maps to WORD_LENGTH_20BIT,
>> case 24 to WORD_LENGTH_24BIT
>> * Remove stray drm_dbg("it6162_bridge_atomic_disable") call
>> * Remove drm_dbg() calls from it6162_display_mode_to_settings()
>> * Drop unused struct it6162 * parameter from it6162_avi_to_video_setting()
>> and it6162_display_mode_to_settings()
>> * Fold it6162_set_default_config() body directly into it6162_reset_init(),
>> removing the wrapper
>> * Fix it6162_infoblock_request_data(): split command complete polling and
>> buffer status check into two steps; use wait_event_timeout() for
>> data_buf_sts since it is updated asynchronously by the interrupt
>> handler; add wait_queue_head_t data_buf_wait to struct it6162 and
>> wake_up() in interrupt handler
>> ---
>> MAINTAINERS | 7 +
>> drivers/gpu/drm/bridge/Kconfig | 17 +
>> drivers/gpu/drm/bridge/Makefile | 1 +
>> drivers/gpu/drm/bridge/ite-it6162.c | 1631
>> +++++++++++++++++++++++++++++++++++
>> 4 files changed, 1656 insertions(+)
>>
>> +
>> +static int it6162_bridge_hdmi_audio_prepare(struct drm_bridge *bridge,
>> + struct drm_connector *connector,
>> + struct hdmi_codec_daifmt *fmt,
>> + struct hdmi_codec_params *params) {
>> + struct it6162 *it6162 = bridge_to_it6162(bridge);
>> + struct it6162_audio config;
>> +
>> + it6162_audio_update_hw_params(it6162, &config, fmt, params);
>> + it6162_enable_audio(it6162, &config);
>
>You write the intermediate structure, use it and then forget it immediately.
>Can it be removed completely?
yes. it will remove in next patch
>> + return
>> drm_atomic_helper_connector_hdmi_update_audio_infoframe(connector,
>> +
>> ¶ms->cea);
>> +}
>> +
>> +static int it6162_bridge_hdmi_audio_startup(struct drm_bridge *bridge,
>> + struct drm_connector *connector) {
>> + return 0;
>> +}
>
>This is not necessary and can be dropped.
>
>> +
>> +static void it6162_bridge_hdmi_audio_shutdown(struct drm_bridge *bridge,
>> + struct drm_connector *connector) {
>> + struct it6162 *it6162 = bridge_to_it6162(bridge);
>
>drm_atomic_helper_connector_hdmi_clear_audio_infoframe() here.
>
>> +
>> + it6162_disable_audio(it6162);
>> +}
>> +
>> +static enum drm_mode_status
>> +it6162_hdmi_tmds_char_rate_valid(const struct drm_bridge *bridge,
>> + const struct drm_display_mode *mode,
>> + unsigned long long tmds_rate)
>> +{
>> + /*IT6162 hdmi supports HDMI2.0 600Mhz*/
>> + if (tmds_rate > 600000000)
>> + return MODE_CLOCK_HIGH;
>> +
>> + return MODE_OK;
>> +}
>> +
>> +static inline int
>> +it6162_write_infoframe(struct it6162 *it6162, const u8 *buffer,
>> +size_t len) {
>> + if (len > DATA_BUFFER_DEPTH)
>> + return -EINVAL;
>> +
>> + regmap_bulk_write(it6162->regmap, OFFSET_DATA_BUFFER, buffer, len);
>> + regmap_write(it6162->regmap, OFFSET_DATA_TYPE_IDX, len);
>> + it6162_infoblock_trigger(it6162, HOST_SET_CEA_INFOFRAME);
>
>Does the hardware automatically identify, which infoframe is it?
>
FW will check infoframe type in Header.
>> + return 0;
>> +}
>> +
>> +static inline int it6162_clear_infoframe(struct it6162 *it6162, u8
>> +type) {
>> + regmap_write(it6162->regmap, OFFSET_DATA_TYPE_IDX, 3);
>> + regmap_write(it6162->regmap, OFFSET_DATA_BUFFER, type);
>> + regmap_write(it6162->regmap, OFFSET_DATA_BUFFER + 1, 0x00);
>> + regmap_write(it6162->regmap, OFFSET_DATA_BUFFER + 2, 0x00);
>> + it6162_infoblock_trigger(it6162, HOST_SET_CEA_INFOFRAME);
>> + return 0;
>> +}
>> +
>> +static int
>> +it6162_bridge_hdmi_write_avi_infoframe(struct drm_bridge *bridge,
>> + const u8 *buffer, size_t len) {
>> + struct it6162 *it6162 = bridge_to_it6162(bridge);
>> +
>> + return it6162_write_infoframe(it6162, buffer, len); }
>> +
>> +static int
>> +it6162_bridge_hdmi_clear_avi_infoframe(struct drm_bridge *bridge) {
>> + struct it6162 *it6162 = bridge_to_it6162(bridge);
>> +
>> + return it6162_clear_infoframe(it6162, HDMI_INFOFRAME_TYPE_AVI); }
>> +
>> +static int
>> +it6162_bridge_hdmi_write_audio_infoframe(struct drm_bridge *bridge,
>> + const u8 *buffer, size_t len)
>> +{
>> + struct it6162 *it6162 = bridge_to_it6162(bridge);
>> +
>> + return it6162_write_infoframe(it6162, buffer, len); }
>> +
>> +static int
>> +it6162_bridge_hdmi_clear_audio_infoframe(struct drm_bridge *bridge) {
>> + struct it6162 *it6162 = bridge_to_it6162(bridge);
>> +
>> + return it6162_clear_infoframe(it6162, HDMI_INFOFRAME_TYPE_AUDIO); }
>> +
>> +static int
>> +it6162_bridge_hdmi_write_spd_infoframe(struct drm_bridge *bridge,
>> + const u8 *buffer, size_t len) {
>> + struct it6162 *it6162 = bridge_to_it6162(bridge);
>> +
>> + return it6162_write_infoframe(it6162, buffer, len); }
>> +
>> +static int
>> +it6162_bridge_hdmi_clear_spd_infoframe(struct drm_bridge *bridge) {
>> + struct it6162 *it6162 = bridge_to_it6162(bridge);
>> +
>> + return it6162_clear_infoframe(it6162, HDMI_INFOFRAME_TYPE_SPD); }
>> +
>> +static const struct drm_bridge_funcs it6162_bridge_funcs = {
>> + .attach = it6162_bridge_attach,
>> + .mode_valid = it6162_bridge_mode_valid,
>> + .detect = it6162_bridge_detect,
>> +
>> + .atomic_enable = it6162_bridge_atomic_enable,
>> + .atomic_disable = it6162_bridge_atomic_disable,
>> + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>> + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
>> + .atomic_reset = drm_atomic_helper_bridge_reset,
>> + .atomic_check = it6162_bridge_atomic_check,
>> +
>> + .edid_read = it6162_bridge_read_edid,
>> +
>> + .hdmi_clear_avi_infoframe = it6162_bridge_hdmi_clear_avi_infoframe,
>> + .hdmi_write_avi_infoframe = it6162_bridge_hdmi_write_avi_infoframe,
>> + .hdmi_clear_spd_infoframe = it6162_bridge_hdmi_clear_spd_infoframe,
>> + .hdmi_write_spd_infoframe = it6162_bridge_hdmi_write_spd_infoframe,
>> + .hdmi_clear_audio_infoframe = it6162_bridge_hdmi_clear_audio_infoframe,
>> + .hdmi_write_audio_infoframe =
>> +it6162_bridge_hdmi_write_audio_infoframe,
>
>No HDMI Infoframes support? That's really sad.
Is HDMI Infoframe means HDMI-VSIF and HDMI-HF-VSIF?
>
>> +
>> + .hdmi_tmds_char_rate_valid = it6162_hdmi_tmds_char_rate_valid,
>> + .hdmi_audio_prepare = it6162_bridge_hdmi_audio_prepare,
>> + .hdmi_audio_startup = it6162_bridge_hdmi_audio_startup,
>> + .hdmi_audio_shutdown = it6162_bridge_hdmi_audio_shutdown,
>> +};
>> +
>> +static int it6162_probe(struct i2c_client *client) {
>> + struct device *dev = &client->dev;
>> + struct device_node *np = dev->of_node;
>> + struct it6162 *it6162;
>> + int ret;
>> +
>> + it6162 = devm_drm_bridge_alloc(dev, struct it6162, bridge,
>> + &it6162_bridge_funcs);
>> + if (IS_ERR(it6162))
>> + return PTR_ERR(it6162);
>> +
>> + it6162->dev = dev;
>> +
>> + ret = it6162_of_get_dsi_host(it6162);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = it6162_i2c_regmap_init(client, it6162);
>> + if (ret != 0)
>> + return ret;
>> +
>> + ret = it6162_init_pdata(it6162);
>> + if (ret)
>> + return ret;
>> +
>> + it6162_config_default(it6162);
>> + it6162_parse_dt(it6162);
>> +
>> + if (it6162_detect_devices(it6162) < 0)
>> + return -ENODEV;
>> +
>> + if (!client->irq) {
>> + dev_err(dev, "Failed to get INTP IRQ");
>> + return -ENODEV;
>> + }
>> +
>> + ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>> + it6162_int_threaded_handler,
>> + IRQF_TRIGGER_LOW | IRQF_ONESHOT |
>> + IRQF_NO_AUTOEN,
>> + "it6162-intp", it6162);
>> + if (ret)
>> + return ret;
>> +
>> + INIT_DELAYED_WORK(&it6162->hdcp_work, it6162_hdcp_work);
>> + init_waitqueue_head(&it6162->data_buf_wait);
>> +
>> + mutex_init(&it6162->lock);
>> +
>> + it6162->bridge.of_node = np;
>> + it6162->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID |
>> + DRM_BRIDGE_OP_MODES;
>> +
>> + it6162->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
>> +
>> + it6162->bridge.vendor = "ITE";
>> + it6162->bridge.product = "IT6162";
>
>You missed to specify DRM_BRIDGE_OP_HDMI, so all HDMI callbacks will be
>ignored.
>
>> +
>> + if (it6162_of_get_audio(it6162)) {
>> + it6162->bridge.ops |= DRM_BRIDGE_OP_HDMI_AUDIO;
>> + it6162->bridge.hdmi_audio_dev = dev;
>> + it6162->bridge.hdmi_audio_max_i2s_playback_channels = 8;
>> + it6162->bridge.hdmi_audio_dai_port = 2;
>> + }
>> +
>> + devm_drm_bridge_add(dev, &it6162->bridge);
>> +
>> + return it6162_attach_dsi(it6162);
>> +}
>> +
>> +static void it6162_remove(struct i2c_client *client) {
>> + struct it6162 *it6162 = i2c_get_clientdata(client);
>> +
>> + disable_irq(client->irq);
>> + cancel_delayed_work_sync(&it6162->hdcp_work);
>> + mutex_destroy(&it6162->lock);
>> +}
>> +
>> +static const struct it6162_chip_info it6162_chip_info = {
>> + .chip_id = 0x616200,
>> + .version = 0x006500,
>> +};
>> +
>> +static const struct of_device_id it6162_dt_ids[] = {
>> + { .compatible = "ite,it6162", .data = &it6162_chip_info},
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, it6162_dt_ids);
>> +
>> +static const struct i2c_device_id it6162_i2c_ids[] = {
>> + { "it6162", 0 },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(i2c, it6162_i2c_ids);
>> +
>> +static struct i2c_driver it6162_driver = {
>> + .driver = {
>> + .name = "it6162",
>> + .of_match_table = it6162_dt_ids,
>> + },
>> + .probe = it6162_probe,
>> + .remove = it6162_remove,
>> + .id_table = it6162_i2c_ids,
>> +};
>> +module_i2c_driver(it6162_driver);
>> +
>> +MODULE_AUTHOR("Pet Weng <[email protected]>");
>> +MODULE_AUTHOR("Hermes Wu <[email protected]>");
>> +MODULE_DESCRIPTION("it6162 MIPI to HDMI driver");
>> +MODULE_LICENSE("GPL");
>>
>> --
>> 2.34.1
>>
>>
>
>--
>With best wishes
>Dmitry
>
BR,
Hermes
