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

[Devarsh Thakkar <devarsht@ti.com>]

Hi Dmitry,

On 09/10/25 20:39, Devarsh Thakkar wrote:
> Hi Dmitry,
> 
> Thanks for the quick reviews.
> 
> On 09/10/25 01:48, Dmitry Baryshkov wrote:
>> On Tue, Oct 07, 2025 at 04:53:09PM +0530, 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 <devarsht@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-devarsht@ti.com/
>>> Link to V1:
>>> https://lore.kernel.org/all/20251003143642.4072918-1-devarsht@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);
>>> @@ -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);
>>>       }
>>
>> The EDID read / free can also be dropped, they don't serve any purpose
>> now.
>>
> 
> Sorry I did not understand this clearly.
> I have already removed drm_edid_connector_update from sii902x_get_modes 
> since now it is taken care inside sii902x_edid_read itself which is also 
> called by .get_modes (sii902x_get_modes).
> 
> We still need to read edid inside sii902x_get_modes as for the legacy 
> driver scenario which don't use DRM_BRIDGE_ATTACH_NO_CONNECTOR as in 
> those cases .get_modes will get called but bridge function .edid_read 
> won't be called. Also as the caller is supposed to free the edid 
> allocated so I have preserved drm_edid_free call in .get_modes as 
> drm_edid_connector_add_modes is allocating a new drm_edid pointer already.
> 
> Kindly let me know if there is a misunderstanding here.
> 

Just thought to check if we are good w.r.t above.
To summarize, as mentioned above, I think both sii902x_get_modes and 
sii902x_bridge_edid_read need to read edid. The former is called only 
for legacy driver scenario (and not for drivers using 
DRM_BRIDGE_ATTACH_NO_CONNECTOR) and the latter is called for the display 
drivers using DRM_BRIDGE_ATTACH_NO_CONNECTOR.

Regards
Devarsh

> Regards
> Devarsh
> 
>>> -    sii902x->sink_is_hdmi = connector->display_info.is_hdmi;
>>> -
>>>       return num;
>>>   }
>>> -- 
>>> 2.39.1
>>>
>>
>