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

[Devarsh Thakkar]

Hi Tomi,

On 22/10/25 12:19, Tomi Valkeinen wrote:
Hi,

Thanks for the review.

On 07/10/2025 14:23, Devarsh Thakkar wrote:
The SII902x HDMI bridge driver wasn't working properly with drivers that
use the newer bridge connector architecture with the
DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, like TIDSS.  This caused HDMI audio to
fail since the driver wasn't properly setting the sink_is_hdmi flag when
the bridge was attached without a connector since .get_modes() is never
called in this case. Fix it by setting sink_is_hdmi flag when reading
the EDID block itself.

Fixes: 3de47e1309c2 ("drm/bridge: sii902x: use display info is_hdmi")
Signed-off-by: Devarsh Thakkar <devar...@ti.com>
---
V3: Use drm_edid_connector_update without edid NULL check
V2: Use drm_edid_connector_update to detect HDMI

Link to V2:
https://lore.kernel.org/all/20251006150714.3144368-1-devar...@ti.com/
Link to V1:
https://lore.kernel.org/all/20251003143642.4072918-1-devar...@ti.com/

  drivers/gpu/drm/bridge/sii902x.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index d537b1d036fb..bb613d4c281f 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -296,6 +296,8 @@ static const struct drm_edid *sii902x_edid_read(struct 
sii902x *sii902x,
        mutex_lock(&sii902x->mutex);
  
  	drm_edid = drm_edid_read_ddc(connector, sii902x->i2cmux->adapter[0]);
+       drm_edid_connector_update(connector, drm_edid);
+       sii902x->sink_is_hdmi = connector->display_info.is_hdmi;
  
  	mutex_unlock(&sii902x->mutex);

I'm not sure if this is right. If I'm not mistaken, the
drm_bridge_connector.c calls sii902x_edid_read, and then does a
drm_edid_connector_update(). So drm_edid_connector_update() will be
called twice, and also, I don't think the bridge should be updating
connector's data when using DRM_BRIDGE_ATTACH_NO_CONNECTOR.


Thanks for catching this!. I missed out to catch this while addressing 
previous review comment.

Maybe either drop sink_is_hdmi, and just look at
connector->display_info.is_hdmi, or if that doesn't work for some
reason, just get the "is-hdmi" flag from the edid data directly.


Yes I plan to drop sink_is_hdmi flag altogether and directly use 
connector->display_info.is_hdmi in v4.

I think we should be able to use 
drm_atomic_get_new_connector_for_encoder in .atomic_enable to get the 
connector->display_info.is_hdmi and this should work for both the legacy 
and DISPLAY_BRIDGE_ATTACH_NO_CONNECTOR scenarios and set the output mode 
accordingly. I will try out with this approach.

Kindly let me know if any concerns with above approach.

Regards
Devarsh

  Tomi

@@ -309,14 +311,11 @@ static int sii902x_get_modes(struct drm_connector 
*connector)
        int num = 0;
  
  	drm_edid = sii902x_edid_read(sii902x, connector);
-       drm_edid_connector_update(connector, drm_edid);
        if (drm_edid) {
                num = drm_edid_connector_add_modes(connector);
                drm_edid_free(drm_edid);
        }
  
-	sii902x->sink_is_hdmi = connector->display_info.is_hdmi;
-
        return num;
  }