Re: [PATCH 7/8] drm/bridge: imx8mp-hdmi-tx: add an hdmi-connector when missing using a DT overlay at boot time

[Liu Ying <victor.liu@nxp.com>]

Hi Luca,

On Tue, Mar 31, 2026 at 12:54:30PM +0200, Luca Ceresoli wrote:
> Hello Liu,
> 
> On Tue Mar 31, 2026 at 5:03 AM CEST, Liu Ying wrote:
>> Hi Luca,
>>
>> On Mon, Mar 30, 2026 at 05:47:23PM +0200, Luca Ceresoli wrote:
>>> Hello Liu,
>>>
>>> On Mon Mar 30, 2026 at 5:02 AM CEST, Liu Ying wrote:
>>
>> [...]
>>
>>>>>>> +	fixup-hdmi-connector {
>>>>>>> +		compatible = "hdmi-connector";
>>>>>>> +		label = "HDMI";
>>>>>>> +		type = "a";
>>>>>>
>>>>>> What if a board uses another type?
>>>>>
>>>>> For boards affected by this patch, currently the connector is created by
>>>>> dw_hdmi_connector_create() which hardcodes type A [0], so there would be no
>>>>> difference.
>>>>
>>>> Yes, that's from driver's PoV.  However, userspace may get the type
>>>> from /sys/firmware/devicetree/base/fixup-hdmi-connector/type and use it
>>>> to do something.
>>>
>>> I'd say this is incorrect, the device tree is not an API for that. The
>>> connector type might be known to the driver by other means (ACPI, DP MST,
>>> whatever). So I think this is a non-problem.
>>
>> I just feel that it's not great to report potentially wrong type to users
>> through the above sys node ...
>>
>>>
>>> If userspace needs to know the connector type, that should come from the
>>> ioctl (DRM_IOCTL_MODE_GETCONNECTOR perhaps).
>>>
>>>> Maybe, that's trivial.
>>>
>>> Not sure I got what you mean here, sorry. What are you referring to?
>>
>> ... with the above potentially wrong type being said, I think maybe this
>> drawback is not a big deal and could be ignored.  Sorry for not being
>> clear in my last reply.
> 
> Ah, clear now. No problem!
> 
>>>>> Boards with a different connector should describe the connector in the
>>>>> device tree, if they need to instantiate the exact type.
>>>
>>> I think this is the only valid solution. It's very easy to do, nothing new
>>> to invent.
>>>
>>> Maybe on top of that we could add a warning when the overlay is applied,
>>> e.g. "imx8mp-hdmi-tx used without a connector described in device tree;
>>> adding a type A connector as a fallback; please add a valid description to
>>> your device tree".
>>
>> I'd say this doesn't sound a bad idea but I hope the message is clear and
>> short.
> 
> What about:
> 
>   Connector description not found in device tree, please add one. Falling back to Type A.

Maybe:
Please add a hdmi-connector DT node for imx8mp-hdmi-tx. A fixup node in type a is added for now.

> 
>>> Maybe pointing to a TODO entry in the documentation.
>>
>> To parameterize the HDMI connector type?  If so, I'm okay with that.
> 
> I was meaning a TODO entry to suggest people to add a connector description
> to the dts. E.g., expanding on the above suggested warning:
> 
>   Connector description not found in device tree, please add one. See https://docs.kernel.org/gpu/todo.html#<...>
> 
> And of course adding a TODO entry describing what one needs to do (add an
> hdmi-connector node and link it to port@1 of the hdmi-tx).
> 
> The drawback of the TODO is that items in todo.rst are supposed to be
> removed eventually when done in the code, but this one cannot be removed
> until some kernels printing the above logging message will be around,
> i.e. potentially for decades.

Not a big fan of adding a TODO entry, because those DT blobs without a
hdmi-connector node could be out there forever, meaning the added TODO
entry can never be removed.

> 
> So maybe the simplest solution is just the first warning message + a
> comment in the code right before the warning line, so it easily found with
> grep or a web search by who sees the warning.

+1.

> 
> Luca
> 
> --
> Luca Ceresoli, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com/

-- 
Regards,
Liu Ying