Re: [PATCH v4] drm/bridge: sii902x: Fix HDMI detection with DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

Hi,

On 30/10/2025 17:16, Devarsh Thakkar wrote:
> The sii902x driver was caching HDMI detection state in a sink_is_hdmi field
> and checking it in mode_set() to determine whether to set HDMI or DVI
> output mode. This approach had two problems:
> 
> 1. With DRM_BRIDGE_ATTACH_NO_CONNECTOR (used by modern display drivers like
> TIDSS), the bridge's get_modes() is never called. Instead, the
> drm_bridge_connector helper calls the bridge's edid_read() and updates the
> connector itself. This meant sink_is_hdmi was never populated, causing the
> driver to default to DVI mode and breaking HDMI audio.
> 
> 2. The mode_set() callback doesn't receive atomic state or connector
> pointer, making it impossible to check connector->display_info.is_hdmi
> directly at that point.
> 
> Fix this by moving the HDMI vs DVI decision from mode_set() to
> atomic_enable(), where we can access the connector via
> drm_atomic_get_new_connector_for_encoder(). This works for both connector
> models:
> 
> - With DRM_BRIDGE_ATTACH_NO_CONNECTOR: Returns the drm_bridge_connector
>   created by the display driver, which has already been updated by the
> helper's call to drm_edid_connector_update()
> 
> - Without DRM_BRIDGE_ATTACH_NO_CONNECTOR (legacy): Returns the connector
>   embedded in sii902x struct, which gets updated by the bridge's own
> get_modes()
> 
> Fixes: 3de47e1309c2 ("drm/bridge: sii902x: use display info is_hdmi")
> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
> ---
> V4: Shift HDMI detection to atomic_enable() and remove sink_is_hdmi caching
> V3: Use drm_edid_connector_update without edid NULL check
> V2: Use drm_edid_connector_update to detect HDMI
> 
> Link to V3:
> https://lore.kernel.org/all/20251007112309.1103811-1-devarsht@ti.com/
> Link to V2:
> https://lore.kernel.org/all/20251006150714.3144368-1-devarsht@ti.com/
> Link to V1:
> https://lore.kernel.org/all/20251003143642.4072918-1-devarsht@ti.com/
> 
>  drivers/gpu/drm/bridge/sii902x.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index d537b1d036fb..1f0aba28ad1e 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -179,7 +179,6 @@ struct sii902x {
>  	struct drm_connector connector;
>  	struct gpio_desc *reset_gpio;
>  	struct i2c_mux_core *i2cmux;
> -	bool sink_is_hdmi;
>  	u32 bus_width;
>  
>  	/*
> @@ -315,8 +314,6 @@ static int sii902x_get_modes(struct drm_connector *connector)
>  		drm_edid_free(drm_edid);
>  	}
>  
> -	sii902x->sink_is_hdmi = connector->display_info.is_hdmi;
> -
>  	return num;
>  }
>  
> @@ -342,9 +339,17 @@ static void sii902x_bridge_atomic_enable(struct drm_bridge *bridge,
>  					 struct drm_atomic_state *state)
>  {
>  	struct sii902x *sii902x = bridge_to_sii902x(bridge);
> +	struct drm_connector *connector;
> +	u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
> +
> +	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
> +	if (connector && connector->display_info.is_hdmi)
> +		output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
>  
>  	mutex_lock(&sii902x->mutex);
>  
> +	regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
> +			   SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
>  	regmap_update_bits(sii902x->regmap, SII902X_PWR_STATE_CTRL,
>  			   SII902X_AVI_POWER_STATE_MSK,
>  			   SII902X_AVI_POWER_STATE_D(0));
> @@ -359,16 +364,12 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
>  				    const struct drm_display_mode *adj)
>  {
>  	struct sii902x *sii902x = bridge_to_sii902x(bridge);
> -	u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
>  	struct regmap *regmap = sii902x->regmap;
>  	u8 buf[HDMI_INFOFRAME_SIZE(AVI)];
>  	struct hdmi_avi_infoframe frame;
>  	u16 pixel_clock_10kHz = adj->clock / 10;
>  	int ret;
>  
> -	if (sii902x->sink_is_hdmi)
> -		output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
> -
>  	buf[0] = pixel_clock_10kHz & 0xff;
>  	buf[1] = pixel_clock_10kHz >> 8;
>  	buf[2] = drm_mode_vrefresh(adj);
> @@ -384,11 +385,6 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
>  
>  	mutex_lock(&sii902x->mutex);
>  
> -	ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
> -				 SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
> -	if (ret)
> -		goto out;
> -
>  	ret = regmap_bulk_write(regmap, SII902X_TPI_VIDEO_DATA, buf, 10);
>  	if (ret)
>  		goto out;

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

 Tomi