[PATCH v4 0/3] drm/bridge: ite-it66121: Fix display output for DVI monitors

[PATCH v4 0/3] drm/bridge: ite-it66121: Fix display output for DVI monitors

[Javier Martinez Canillas]

Display output does not work when connecting an AM625 BeaglePlay board to
a DVI monitor, because the DRM it66121 bridge driver assumes that the sink
type is always HDMI. This patch series fixes the issue.

Patch #1 reworks the driver to use the HDMI helpers instead of open coding
the AVI infoframes buffer management.

Patch #2 moves the .mode_set logic to the .atomic_enable handler.

Patch #3 finally fixes the mentioned issue by using the display information
to determine whether HDMI or DVI mode should be set.

This is a v4 of the series, that addresses issues pointed out by Maxime.

The patches were tested on both DVI and an HDMI monitors.

Changes in v4:
- Convert the driver to use the HDMI helpers (Maxime Ripard).
- Move .mode_set logic to .atomic_enable (Maxime Ripard).

Changes in v3:
- Move the HDMI/DVI mode set to the .atomic_enable handler (Maxime Ripard).

Changes in v2:
- Don't store the sink type in a per-commit bridge state (Maxime Ripard).

Javier Martinez Canillas (3):
  drm/bridge: ite-it66121: Switch to the HDMI connector helpers
  drm/bridge: ite-it66121: Move logic .mode_set setup to .atomic_enable
  drm/bridge: ite-it66121: Select HDMI or DVI mode based on sink type

 drivers/gpu/drm/bridge/Kconfig       |   2 +
 drivers/gpu/drm/bridge/ite-it66121.c | 198 ++++++++++++++++-----------
 2 files changed, 121 insertions(+), 79 deletions(-)

-- 
2.54.0

base-commit: 4c26e162947f91aa78ba57dd4fddd38fc80e7d60
branch: it66121-fix-dvi-mode-v4

[PATCH v4 1/3] drm/bridge: ite-it66121: Switch to the HDMI connector helpers

[Javier Martinez Canillas]

Instead of open coding the HDMI AVI Infoframes buffer management,
use the helpers provided by the HDMI connector framework.

Suggested-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

Changes in v4:
- New patch for v4

 drivers/gpu/drm/bridge/Kconfig       |   2 +
 drivers/gpu/drm/bridge/ite-it66121.c | 132 ++++++++++++++++-----------
 2 files changed, 83 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index f81b566c82a1..4a57d49b4c6d 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -205,6 +205,8 @@ config DRM_LONTIUM_LT8713SX
 config DRM_ITE_IT66121
 	tristate "ITE IT66121 HDMI bridge"
 	depends on OF
+	select DRM_DISPLAY_HDMI_STATE_HELPER
+	select DRM_DISPLAY_HELPER
 	select DRM_KMS_HELPER
 	select REGMAP_I2C
 	help
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index 19a027d75b61..947b7a0f0a45 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -20,6 +20,8 @@
 #include <linux/pinctrl/consumer.h>
 #include <linux/regulator/consumer.h>
 
+#include <drm/display/drm_hdmi_helper.h>
+#include <drm/display/drm_hdmi_state_helper.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_edid.h>
@@ -304,7 +306,6 @@ struct it66121_ctx {
 	struct i2c_client *client;
 	u32 bus_width;
 	struct mutex lock; /* Protects fields below and device registers */
-	struct hdmi_avi_infoframe hdmi_avi_infoframe;
 	struct {
 		struct platform_device *pdev;
 		u8 ch_enable;
@@ -727,6 +728,10 @@ static void it66121_bridge_enable(struct drm_bridge *bridge,
 	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
 
 	ctx->connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
+	if (!ctx->connector)
+		return;
+
+	drm_atomic_helper_connector_hdmi_update_infoframes(ctx->connector, state);
 
 	it66121_set_mute(ctx, false);
 }
@@ -764,40 +769,10 @@ void it66121_bridge_mode_set(struct drm_bridge *bridge,
 			     const struct drm_display_mode *mode,
 			     const struct drm_display_mode *adjusted_mode)
 {
-	u8 buf[HDMI_INFOFRAME_SIZE(AVI)];
 	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
-	int ret;
 
 	mutex_lock(&ctx->lock);
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&ctx->hdmi_avi_infoframe, ctx->connector,
-						       adjusted_mode);
-	if (ret) {
-		DRM_ERROR("Failed to setup AVI infoframe: %d\n", ret);
-		goto unlock;
-	}
-
-	ret = hdmi_avi_infoframe_pack(&ctx->hdmi_avi_infoframe, buf, sizeof(buf));
-	if (ret < 0) {
-		DRM_ERROR("Failed to pack infoframe: %d\n", ret);
-		goto unlock;
-	}
-
-	/* Write new AVI infoframe packet */
-	ret = regmap_bulk_write(ctx->regmap, IT66121_AVIINFO_DB1_REG,
-				&buf[HDMI_INFOFRAME_HEADER_SIZE],
-				HDMI_AVI_INFOFRAME_SIZE);
-	if (ret)
-		goto unlock;
-
-	if (regmap_write(ctx->regmap, IT66121_AVIINFO_CSUM_REG, buf[3]))
-		goto unlock;
-
-	/* Enable AVI infoframe */
-	if (regmap_write(ctx->regmap, IT66121_AVI_INFO_PKT_REG,
-			 IT66121_AVI_INFO_PKT_ON | IT66121_AVI_INFO_PKT_RPT))
-		goto unlock;
-
 	/* Set TX mode to HDMI */
 	if (regmap_write(ctx->regmap, IT66121_HDMI_MODE_REG, IT66121_HDMI_MODE_HDMI))
 		goto unlock;
@@ -825,24 +800,6 @@ void it66121_bridge_mode_set(struct drm_bridge *bridge,
 	mutex_unlock(&ctx->lock);
 }
 
-static enum drm_mode_status it66121_bridge_mode_valid(struct drm_bridge *bridge,
-						      const struct drm_display_info *info,
-						      const struct drm_display_mode *mode)
-{
-	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
-	unsigned long max_clock;
-
-	max_clock = (ctx->bus_width == 12) ? 74250 : 148500;
-
-	if (mode->clock > max_clock)
-		return MODE_CLOCK_HIGH;
-
-	if (mode->clock < 25000)
-		return MODE_CLOCK_LOW;
-
-	return MODE_OK;
-}
-
 static enum drm_connector_status
 it66121_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
 {
@@ -873,6 +830,72 @@ static void it66121_bridge_hpd_disable(struct drm_bridge *bridge)
 		dev_err(ctx->dev, "failed to disable HPD IRQ\n");
 }
 
+static enum drm_mode_status
+it66121_bridge_hdmi_tmds_char_rate_valid(const struct drm_bridge *bridge,
+					 const struct drm_display_mode *mode,
+					 unsigned long long tmds_rate)
+{
+	const struct it66121_ctx *ctx =
+		container_of(bridge, const struct it66121_ctx, bridge);
+	unsigned long max_clock;
+
+	max_clock = (ctx->bus_width == 12) ? 74250 : 148500;
+
+	if (mode->clock > max_clock)
+		return MODE_CLOCK_HIGH;
+
+	if (mode->clock < 25000)
+		return MODE_CLOCK_LOW;
+
+	return MODE_OK;
+}
+
+static int it66121_bridge_hdmi_clear_avi_infoframe(struct drm_bridge *bridge)
+{
+	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
+
+	return regmap_write(ctx->regmap, IT66121_AVI_INFO_PKT_REG, 0);
+}
+
+static int it66121_bridge_hdmi_clear_hdmi_infoframe(struct drm_bridge *bridge)
+{
+	return 0;
+}
+
+static int it66121_bridge_hdmi_write_avi_infoframe(struct drm_bridge *bridge,
+						   const u8 *buffer, size_t len)
+{
+	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
+	int ret;
+
+	mutex_lock(&ctx->lock);
+
+	/* Write new AVI infoframe packet */
+	ret = regmap_bulk_write(ctx->regmap, IT66121_AVIINFO_DB1_REG,
+				&buffer[HDMI_INFOFRAME_HEADER_SIZE],
+				HDMI_AVI_INFOFRAME_SIZE);
+	if (ret)
+		goto unlock;
+
+	ret = regmap_write(ctx->regmap, IT66121_AVIINFO_CSUM_REG, buffer[3]);
+	if (ret)
+		goto unlock;
+
+	/* Enable AVI infoframe */
+	ret = regmap_write(ctx->regmap, IT66121_AVI_INFO_PKT_REG,
+			   IT66121_AVI_INFO_PKT_ON | IT66121_AVI_INFO_PKT_RPT);
+
+unlock:
+	mutex_unlock(&ctx->lock);
+	return ret;
+}
+
+static int it66121_bridge_hdmi_write_hdmi_infoframe(struct drm_bridge *bridge,
+						    const u8 *buffer, size_t len)
+{
+	return 0;
+}
+
 static const struct drm_edid *it66121_bridge_edid_read(struct drm_bridge *bridge,
 						       struct drm_connector *connector)
 {
@@ -913,11 +936,15 @@ static const struct drm_bridge_funcs it66121_bridge_funcs = {
 	.atomic_disable = it66121_bridge_disable,
 	.atomic_check = it66121_bridge_check,
 	.mode_set = it66121_bridge_mode_set,
-	.mode_valid = it66121_bridge_mode_valid,
 	.detect = it66121_bridge_detect,
 	.edid_read = it66121_bridge_edid_read,
 	.hpd_enable = it66121_bridge_hpd_enable,
 	.hpd_disable = it66121_bridge_hpd_disable,
+	.hdmi_tmds_char_rate_valid = it66121_bridge_hdmi_tmds_char_rate_valid,
+	.hdmi_clear_avi_infoframe = it66121_bridge_hdmi_clear_avi_infoframe,
+	.hdmi_write_avi_infoframe = it66121_bridge_hdmi_write_avi_infoframe,
+	.hdmi_clear_hdmi_infoframe = it66121_bridge_hdmi_clear_hdmi_infoframe,
+	.hdmi_write_hdmi_infoframe = it66121_bridge_hdmi_write_hdmi_infoframe,
 };
 
 static irqreturn_t it66121_irq_threaded_handler(int irq, void *dev_id)
@@ -1588,7 +1615,10 @@ static int it66121_probe(struct i2c_client *client)
 
 	ctx->bridge.of_node = dev->of_node;
 	ctx->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
-	ctx->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID;
+	ctx->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID |
+			  DRM_BRIDGE_OP_HDMI;
+	ctx->bridge.vendor = "ITE";
+	ctx->bridge.product = "IT66121";
 	if (client->irq > 0) {
 		ctx->bridge.ops |= DRM_BRIDGE_OP_HPD;
 
-- 
2.54.0

[PATCH v4 2/3] drm/bridge: ite-it66121: Move logic .mode_set setup to .atomic_enable

[Javier Martinez Canillas]

Move the existing .mode_set logic to the .atomic_enable callback. The
former is deprecated and drivers are supposed to use the latter instead.

Suggested-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

Changes in v4:
- New patch for v4

 drivers/gpu/drm/bridge/ite-it66121.c | 79 +++++++++++++++-------------
 1 file changed, 42 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index 947b7a0f0a45..c8795247cfa8 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -654,6 +654,47 @@ static int it66121_bridge_attach(struct drm_bridge *bridge,
 	return 0;
 }
 
+static void it66121_set_mode(struct it66121_ctx *ctx,
+			     struct drm_atomic_commit *state)
+{
+	struct drm_connector *connector = ctx->connector;
+	const struct drm_crtc_state *crtc_state;
+	const struct drm_display_mode *mode;
+	struct drm_crtc *crtc;
+
+	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
+	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	mode = &crtc_state->adjusted_mode;
+
+	mutex_lock(&ctx->lock);
+
+	/* Set TX mode to HDMI */
+	if (regmap_write(ctx->regmap, IT66121_HDMI_MODE_REG, IT66121_HDMI_MODE_HDMI))
+		goto unlock;
+
+	if ((ctx->id == ID_IT66121 || ctx->id == ID_IT66122) &&
+	    regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
+			      IT66121_CLK_BANK_PWROFF_TXCLK,
+			      IT66121_CLK_BANK_PWROFF_TXCLK)) {
+		goto unlock;
+	}
+
+	if (it66121_configure_input(ctx))
+		goto unlock;
+
+	if (it66121_configure_afe(ctx, mode))
+		goto unlock;
+
+	if ((ctx->id == ID_IT66121 || ctx->id == ID_IT66122) &&
+	    regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
+			      IT66121_CLK_BANK_PWROFF_TXCLK, 0)) {
+		goto unlock;
+	}
+
+unlock:
+	mutex_unlock(&ctx->lock);
+}
+
 static int it66121_set_mute(struct it66121_ctx *ctx, bool mute)
 {
 	int ret;
@@ -733,6 +774,7 @@ static void it66121_bridge_enable(struct drm_bridge *bridge,
 
 	drm_atomic_helper_connector_hdmi_update_infoframes(ctx->connector, state);
 
+	it66121_set_mode(ctx, state);
 	it66121_set_mute(ctx, false);
 }
 
@@ -764,42 +806,6 @@ static int it66121_bridge_check(struct drm_bridge *bridge,
 	return 0;
 }
 
-static
-void it66121_bridge_mode_set(struct drm_bridge *bridge,
-			     const struct drm_display_mode *mode,
-			     const struct drm_display_mode *adjusted_mode)
-{
-	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
-
-	mutex_lock(&ctx->lock);
-
-	/* Set TX mode to HDMI */
-	if (regmap_write(ctx->regmap, IT66121_HDMI_MODE_REG, IT66121_HDMI_MODE_HDMI))
-		goto unlock;
-
-	if ((ctx->id == ID_IT66121 || ctx->id == ID_IT66122) &&
-	    regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
-			      IT66121_CLK_BANK_PWROFF_TXCLK,
-			      IT66121_CLK_BANK_PWROFF_TXCLK)) {
-		goto unlock;
-	}
-
-	if (it66121_configure_input(ctx))
-		goto unlock;
-
-	if (it66121_configure_afe(ctx, adjusted_mode))
-		goto unlock;
-
-	if ((ctx->id == ID_IT66121 || ctx->id == ID_IT66122) &&
-	    regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
-			      IT66121_CLK_BANK_PWROFF_TXCLK, 0)) {
-		goto unlock;
-	}
-
-unlock:
-	mutex_unlock(&ctx->lock);
-}
-
 static enum drm_connector_status
 it66121_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
 {
@@ -935,7 +941,6 @@ static const struct drm_bridge_funcs it66121_bridge_funcs = {
 	.atomic_enable = it66121_bridge_enable,
 	.atomic_disable = it66121_bridge_disable,
 	.atomic_check = it66121_bridge_check,
-	.mode_set = it66121_bridge_mode_set,
 	.detect = it66121_bridge_detect,
 	.edid_read = it66121_bridge_edid_read,
 	.hpd_enable = it66121_bridge_hpd_enable,
-- 
2.54.0

[PATCH v4 3/3] drm/bridge: ite-it66121: Select HDMI or DVI mode based on sink type

[Javier Martinez Canillas]

The driver unconditionally sets the transmission mode to HDMI, which leads
to display output not working with DVI monitors. Check connector's display
information sink type to identify the correct mode to configure the bridge.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

Changes in v4:
- Convert the driver to use the HDMI helpers (Maxime Ripard).
- Move .mode_set logic to .atomic_enable (Maxime Ripard).

Changes in v3:
- Move the HDMI/DVI mode set to the .atomic_enable handler (Maxime Ripard).

Changes in v2:
- Don't store the sink type in a per-commit bridge state (Maxime Ripard).

 drivers/gpu/drm/bridge/ite-it66121.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index c8795247cfa8..95ee98a4a8df 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -184,6 +184,7 @@
 
 #define IT66121_HDMI_MODE_REG			0xC0
 #define IT66121_HDMI_MODE_HDMI			BIT(0)
+#define IT66121_HDMI_MODE_DVI			0
 
 #define IT66121_SYS_STATUS_REG			0x0E
 #define IT66121_SYS_STATUS_ACTIVE_IRQ		BIT(7)
@@ -658,6 +659,7 @@ static void it66121_set_mode(struct it66121_ctx *ctx,
 			     struct drm_atomic_commit *state)
 {
 	struct drm_connector *connector = ctx->connector;
+	unsigned int tx_mode = IT66121_HDMI_MODE_HDMI;
 	const struct drm_crtc_state *crtc_state;
 	const struct drm_display_mode *mode;
 	struct drm_crtc *crtc;
@@ -666,10 +668,13 @@ static void it66121_set_mode(struct it66121_ctx *ctx,
 	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 	mode = &crtc_state->adjusted_mode;
 
+	if (!connector->display_info.is_hdmi)
+		tx_mode = IT66121_HDMI_MODE_DVI;
+
 	mutex_lock(&ctx->lock);
 
-	/* Set TX mode to HDMI */
-	if (regmap_write(ctx->regmap, IT66121_HDMI_MODE_REG, IT66121_HDMI_MODE_HDMI))
+	/* Set TX mode to HDMI or DVI */
+	if (regmap_write(ctx->regmap, IT66121_HDMI_MODE_REG, tx_mode))
 		goto unlock;
 
 	if ((ctx->id == ID_IT66121 || ctx->id == ID_IT66122) &&
-- 
2.54.0

Re: [PATCH v4 1/3] drm/bridge: ite-it66121: Switch to the HDMI connector helpers

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 6695 bytes --]

On Fri, May 15, 2026 at 11:02:09AM +0200, Javier Martinez Canillas wrote:
> Instead of open coding the HDMI AVI Infoframes buffer management,
> use the helpers provided by the HDMI connector framework.
> 
> Suggested-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
> Changes in v4:
> - New patch for v4
> 
>  drivers/gpu/drm/bridge/Kconfig       |   2 +
>  drivers/gpu/drm/bridge/ite-it66121.c | 132 ++++++++++++++++-----------
>  2 files changed, 83 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index f81b566c82a1..4a57d49b4c6d 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -205,6 +205,8 @@ config DRM_LONTIUM_LT8713SX
>  config DRM_ITE_IT66121
>  	tristate "ITE IT66121 HDMI bridge"
>  	depends on OF
> +	select DRM_DISPLAY_HDMI_STATE_HELPER
> +	select DRM_DISPLAY_HELPER
>  	select DRM_KMS_HELPER
>  	select REGMAP_I2C
>  	help
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index 19a027d75b61..947b7a0f0a45 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -20,6 +20,8 @@
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/regulator/consumer.h>
>  
> +#include <drm/display/drm_hdmi_helper.h>
> +#include <drm/display/drm_hdmi_state_helper.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_bridge.h>
>  #include <drm/drm_edid.h>
> @@ -304,7 +306,6 @@ struct it66121_ctx {
>  	struct i2c_client *client;
>  	u32 bus_width;
>  	struct mutex lock; /* Protects fields below and device registers */
> -	struct hdmi_avi_infoframe hdmi_avi_infoframe;
>  	struct {
>  		struct platform_device *pdev;
>  		u8 ch_enable;
> @@ -727,6 +728,10 @@ static void it66121_bridge_enable(struct drm_bridge *bridge,
>  	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
>  
>  	ctx->connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
> +	if (!ctx->connector)
> +		return;
> +
> +	drm_atomic_helper_connector_hdmi_update_infoframes(ctx->connector, state);
>  
>  	it66121_set_mute(ctx, false);
>  }
> @@ -764,40 +769,10 @@ void it66121_bridge_mode_set(struct drm_bridge *bridge,
>  			     const struct drm_display_mode *mode,
>  			     const struct drm_display_mode *adjusted_mode)
>  {
> -	u8 buf[HDMI_INFOFRAME_SIZE(AVI)];
>  	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
> -	int ret;
>  
>  	mutex_lock(&ctx->lock);
>  
> -	ret = drm_hdmi_avi_infoframe_from_display_mode(&ctx->hdmi_avi_infoframe, ctx->connector,
> -						       adjusted_mode);
> -	if (ret) {
> -		DRM_ERROR("Failed to setup AVI infoframe: %d\n", ret);
> -		goto unlock;
> -	}
> -
> -	ret = hdmi_avi_infoframe_pack(&ctx->hdmi_avi_infoframe, buf, sizeof(buf));
> -	if (ret < 0) {
> -		DRM_ERROR("Failed to pack infoframe: %d\n", ret);
> -		goto unlock;
> -	}
> -
> -	/* Write new AVI infoframe packet */
> -	ret = regmap_bulk_write(ctx->regmap, IT66121_AVIINFO_DB1_REG,
> -				&buf[HDMI_INFOFRAME_HEADER_SIZE],
> -				HDMI_AVI_INFOFRAME_SIZE);
> -	if (ret)
> -		goto unlock;
> -
> -	if (regmap_write(ctx->regmap, IT66121_AVIINFO_CSUM_REG, buf[3]))
> -		goto unlock;
> -
> -	/* Enable AVI infoframe */
> -	if (regmap_write(ctx->regmap, IT66121_AVI_INFO_PKT_REG,
> -			 IT66121_AVI_INFO_PKT_ON | IT66121_AVI_INFO_PKT_RPT))
> -		goto unlock;
> -
>  	/* Set TX mode to HDMI */
>  	if (regmap_write(ctx->regmap, IT66121_HDMI_MODE_REG, IT66121_HDMI_MODE_HDMI))
>  		goto unlock;
> @@ -825,24 +800,6 @@ void it66121_bridge_mode_set(struct drm_bridge *bridge,
>  	mutex_unlock(&ctx->lock);
>  }
>  
> -static enum drm_mode_status it66121_bridge_mode_valid(struct drm_bridge *bridge,
> -						      const struct drm_display_info *info,
> -						      const struct drm_display_mode *mode)
> -{
> -	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
> -	unsigned long max_clock;
> -
> -	max_clock = (ctx->bus_width == 12) ? 74250 : 148500;
> -
> -	if (mode->clock > max_clock)
> -		return MODE_CLOCK_HIGH;
> -
> -	if (mode->clock < 25000)
> -		return MODE_CLOCK_LOW;
> -
> -	return MODE_OK;
> -}
> -
>  static enum drm_connector_status
>  it66121_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
>  {
> @@ -873,6 +830,72 @@ static void it66121_bridge_hpd_disable(struct drm_bridge *bridge)
>  		dev_err(ctx->dev, "failed to disable HPD IRQ\n");
>  }
>  
> +static enum drm_mode_status
> +it66121_bridge_hdmi_tmds_char_rate_valid(const struct drm_bridge *bridge,
> +					 const struct drm_display_mode *mode,
> +					 unsigned long long tmds_rate)
> +{
> +	const struct it66121_ctx *ctx =
> +		container_of(bridge, const struct it66121_ctx, bridge);
> +	unsigned long max_clock;
> +
> +	max_clock = (ctx->bus_width == 12) ? 74250 : 148500;
> +
> +	if (mode->clock > max_clock)
> +		return MODE_CLOCK_HIGH;
> +
> +	if (mode->clock < 25000)
> +		return MODE_CLOCK_LOW;
> +
> +	return MODE_OK;
> +}
> +
> +static int it66121_bridge_hdmi_clear_avi_infoframe(struct drm_bridge *bridge)
> +{
> +	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
> +
> +	return regmap_write(ctx->regmap, IT66121_AVI_INFO_PKT_REG, 0);
> +}
> +
> +static int it66121_bridge_hdmi_clear_hdmi_infoframe(struct drm_bridge *bridge)
> +{
> +	return 0;
> +}
> +
> +static int it66121_bridge_hdmi_write_avi_infoframe(struct drm_bridge *bridge,
> +						   const u8 *buffer, size_t len)
> +{
> +	struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
> +	int ret;
> +
> +	mutex_lock(&ctx->lock);
> +
> +	/* Write new AVI infoframe packet */
> +	ret = regmap_bulk_write(ctx->regmap, IT66121_AVIINFO_DB1_REG,
> +				&buffer[HDMI_INFOFRAME_HEADER_SIZE],
> +				HDMI_AVI_INFOFRAME_SIZE);
> +	if (ret)
> +		goto unlock;
> +
> +	ret = regmap_write(ctx->regmap, IT66121_AVIINFO_CSUM_REG, buffer[3]);
> +	if (ret)
> +		goto unlock;
> +
> +	/* Enable AVI infoframe */
> +	ret = regmap_write(ctx->regmap, IT66121_AVI_INFO_PKT_REG,
> +			   IT66121_AVI_INFO_PKT_ON | IT66121_AVI_INFO_PKT_RPT);
> +
> +unlock:
> +	mutex_unlock(&ctx->lock);
> +	return ret;
> +}
> +
> +static int it66121_bridge_hdmi_write_hdmi_infoframe(struct drm_bridge *bridge,
> +						    const u8 *buffer, size_t len)
> +{
> +	return 0;
> +}
> +

When the code says it's mandatory, it means "it needs to be implemented" ;)

Looks good otherwise
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v4 2/3] drm/bridge: ite-it66121: Move logic .mode_set setup to .atomic_enable

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1558 bytes --]

On Fri, May 15, 2026 at 11:02:10AM +0200, Javier Martinez Canillas wrote:
> Move the existing .mode_set logic to the .atomic_enable callback. The
> former is deprecated and drivers are supposed to use the latter instead.
> 
> Suggested-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
> Changes in v4:
> - New patch for v4
> 
>  drivers/gpu/drm/bridge/ite-it66121.c | 79 +++++++++++++++-------------
>  1 file changed, 42 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index 947b7a0f0a45..c8795247cfa8 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -654,6 +654,47 @@ static int it66121_bridge_attach(struct drm_bridge *bridge,
>  	return 0;
>  }
>  
> +static void it66121_set_mode(struct it66121_ctx *ctx,
> +			     struct drm_atomic_commit *state)
> +{
> +	struct drm_connector *connector = ctx->connector;

We should drop this entirely now. It's available in atomic_enable, you should pass it as an
argument.

> +	const struct drm_crtc_state *crtc_state;
> +	const struct drm_display_mode *mode;
> +	struct drm_crtc *crtc;
> +
> +	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
> +	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);

drm_atomic_get_new_connector_state and drm_atomic_get_new_crtc_state can
return a NULL pointer. We should check, warn and return if it's the
case.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v4 3/3] drm/bridge: ite-it66121: Select HDMI or DVI mode based on sink type

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2264 bytes --]

On Fri, May 15, 2026 at 11:02:11AM +0200, Javier Martinez Canillas wrote:
> The driver unconditionally sets the transmission mode to HDMI, which leads
> to display output not working with DVI monitors. Check connector's display
> information sink type to identify the correct mode to configure the bridge.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
> Changes in v4:
> - Convert the driver to use the HDMI helpers (Maxime Ripard).
> - Move .mode_set logic to .atomic_enable (Maxime Ripard).
> 
> Changes in v3:
> - Move the HDMI/DVI mode set to the .atomic_enable handler (Maxime Ripard).
> 
> Changes in v2:
> - Don't store the sink type in a per-commit bridge state (Maxime Ripard).
> 
>  drivers/gpu/drm/bridge/ite-it66121.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index c8795247cfa8..95ee98a4a8df 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -184,6 +184,7 @@
>  
>  #define IT66121_HDMI_MODE_REG			0xC0
>  #define IT66121_HDMI_MODE_HDMI			BIT(0)
> +#define IT66121_HDMI_MODE_DVI			0
>  
>  #define IT66121_SYS_STATUS_REG			0x0E
>  #define IT66121_SYS_STATUS_ACTIVE_IRQ		BIT(7)
> @@ -658,6 +659,7 @@ static void it66121_set_mode(struct it66121_ctx *ctx,
>  			     struct drm_atomic_commit *state)
>  {
>  	struct drm_connector *connector = ctx->connector;
> +	unsigned int tx_mode = IT66121_HDMI_MODE_HDMI;
>  	const struct drm_crtc_state *crtc_state;
>  	const struct drm_display_mode *mode;
>  	struct drm_crtc *crtc;
> @@ -666,10 +668,13 @@ static void it66121_set_mode(struct it66121_ctx *ctx,
>  	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>  	mode = &crtc_state->adjusted_mode;
>  
> +	if (!connector->display_info.is_hdmi)
> +		tx_mode = IT66121_HDMI_MODE_DVI;
> +
>  	mutex_lock(&ctx->lock);
>  
> -	/* Set TX mode to HDMI */
> -	if (regmap_write(ctx->regmap, IT66121_HDMI_MODE_REG, IT66121_HDMI_MODE_HDMI))
> +	/* Set TX mode to HDMI or DVI */
> +	if (regmap_write(ctx->regmap, IT66121_HDMI_MODE_REG, tx_mode))

A ternary operator would probably be better here?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v4 1/3] drm/bridge: ite-it66121: Switch to the HDMI connector helpers

[Javier Martinez Canillas]

Maxime Ripard <mripard@kernel.org> writes:

Hello Maxime,

> On Fri, May 15, 2026 at 11:02:09AM +0200, Javier Martinez Canillas wrote:

[...]

>> +static int it66121_bridge_hdmi_write_hdmi_infoframe(struct drm_bridge *bridge,
>> +						    const u8 *buffer, size_t len)
>> +{
>> +	return 0;
>> +}
>> +
>
> When the code says it's mandatory, it means "it needs to be implemented" ;)
>

Fair. The driver currently is only implenting AVI infoframes, but I agree
that needs to implement these too. I'll tackle that in the next iteration.

> Looks good otherwise

Thanks again for all your feedback and patience reviewing this series.

> Maxime

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH v4 1/3] drm/bridge: ite-it66121: Switch to the HDMI connector helpers

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]

On Fri, May 15, 2026 at 11:02:09AM +0200, Javier Martinez Canillas wrote:
> +static enum drm_mode_status
> +it66121_bridge_hdmi_tmds_char_rate_valid(const struct drm_bridge *bridge,
> +					 const struct drm_display_mode *mode,
> +					 unsigned long long tmds_rate)
> +{
> +	const struct it66121_ctx *ctx =
> +		container_of(bridge, const struct it66121_ctx, bridge);
> +	unsigned long max_clock;
> +
> +	max_clock = (ctx->bus_width == 12) ? 74250 : 148500;
> +
> +	if (mode->clock > max_clock)
> +		return MODE_CLOCK_HIGH;
> +
> +	if (mode->clock < 25000)
> +		return MODE_CLOCK_LOW;
> +
> +	return MODE_OK;
> +}

You must not use mode->clock here, use the tdms_rate passed as an argument.

>  static irqreturn_t it66121_irq_threaded_handler(int irq, void *dev_id)
> @@ -1588,7 +1615,10 @@ static int it66121_probe(struct i2c_client *client)
>  
>  	ctx->bridge.of_node = dev->of_node;
>  	ctx->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
> -	ctx->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID;
> +	ctx->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID |
> +			  DRM_BRIDGE_OP_HDMI;
> +	ctx->bridge.vendor = "ITE";
> +	ctx->bridge.product = "IT66121";
>  	if (client->irq > 0) {
>  		ctx->bridge.ops |= DRM_BRIDGE_OP_HPD;

It's not really for today, but ideally we should move to the audio
helpers at some point too.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]