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

[Javier Martinez Canillas]

Maxime Ripard <mripard@xxxxxxxxxx> writes:

Hello Maxime!

> Hi,
>
> On Sun, May 10, 2026 at 09:14:49PM +0200, Javier Martinez Canillas wrote:
>> Currently, the driver assumes that the connector sink type is always HDMI
>> and configures the IT66121 bridge appropriately. But configuring in this
>> mode and enabling the transmission of AVI infoframe packets can cause DVI
>> monitors to fail parsing the video signal.
>> 
>> To prevent this, store the connector display information sink type in the
>> bridge atomic state and use it to decide whether the bridge should be set
>> in HDMI or DVI mode, and if the AVI infoframes packets should be sent.
>> 
>> Assisted-by: Cursor:claude-4.6-opus
>> Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
>> ---
>> 
>>  drivers/gpu/drm/bridge/ite-it66121.c | 68 ++++++++++++++++++----------
>>  1 file changed, 44 insertions(+), 24 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
>> index a203c94a27e5..99088277d170 100644
>> --- a/drivers/gpu/drm/bridge/ite-it66121.c
>> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
>> @@ -322,6 +322,7 @@ static inline struct it66121_ctx *bridge_to_it66121_ctx(struct drm_bridge *bridg
>>  
>>  struct it66121_bridge_state {
>>  	struct drm_bridge_state base;
>> +	bool sink_is_hdmi;
>>  };
>>  
>>  static inline struct it66121_bridge_state *
>> @@ -790,6 +791,14 @@ static int it66121_bridge_check(struct drm_bridge *bridge,
>>  				struct drm_connector_state *conn_state)
>>  {
>>  	struct it66121_ctx *ctx = bridge_to_it66121_ctx(bridge);
>> +	struct it66121_bridge_state *state =
>> +		to_it66121_bridge_state(bridge_state);
>> +
>> +	/* Default to HDMI to preserve legacy behavior. */
>> +	state->sink_is_hdmi = true;
>> +
>> +	if (conn_state && conn_state->connector)
>> +		state->sink_is_hdmi = conn_state->connector->display_info.is_hdmi;
>
> It's really not clear to me why you need to have that in the bridge
> state. What's wrong with using drm_display_info.is_hdmi directly?
>

I wrongly thought that couldn't get that info from it66121_bridge_mode_set()
so I thought about making it a property of the bridge atomic state.

But I see now that the connector is already stored in struct it66121_ctx *ctx
so the fix indeed becomes even more trivial.

> If you really want to rework the driver though, switch to HDMI state
> helpers would fix all of it :)
>

Sure, I will do. But I think that is still worth to land a minimal fix and
then do the switch to the HDMI state helpers as a follow up. So what do you
think of the following patch?

(Note that I dropped the Assisted-by tag this time since v2 has been manually
written by me, with no assistance from an LLM).