Re: [PATCH v2 1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[Frieder Schrempf <frieder.schrempf@kontron.de>]

On 31.12.24 12:48 AM, Dmitry Baryshkov wrote:
> On Mon, Dec 30, 2024 at 10:44:25PM +0100, Marek Vasut wrote:
>> On 12/30/24 8:04 AM, Ying Liu wrote:
>>> On 12/26/2024, Marek Vasut wrote:
>>>> On 12/24/24 5:21 AM, Dmitry Baryshkov wrote:
>>>>> On Tue, Dec 24, 2024 at 02:46:14AM +0100, Marek Vasut wrote:
>>>>>> The dw-hdmi output_port is set to 1 in order to look for a connector
>>>>>> next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR
>>>> working.
>>>>>> The output_port set to 1 makes the DW HDMI driver core look up the
>>>>>> next bridge in DT, where the next bridge is often the hdmi-connector .
>>>>>>
>>>>>> Similar to 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge
>>>> DRM_BRIDGE_ATTACH_NO_CONNECTOR")
>>>>>>
>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>> ---
>>>>>> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
>>>>>> Cc: David Airlie <airlied@gmail.com>
>>>>>> Cc: Fabio Estevam <festevam@gmail.com>
>>>>>> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
>>>>>> Cc: Jonas Karlman <jonas@kwiboo.se>
>>>>>> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
>>>>>> Cc: Liu Ying <victor.liu@nxp.com>
>>>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>>> Cc: Maxime Ripard <mripard@kernel.org>
>>>>>> Cc: Neil Armstrong <neil.armstrong@linaro.org>
>>>>>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
>>>>>> Cc: Robert Foss <rfoss@kernel.org>
>>>>>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>>>>>> Cc: Shawn Guo <shawnguo@kernel.org>
>>>>>> Cc: Simona Vetter <simona@ffwll.ch>
>>>>>> Cc: Stefan Agner <stefan@agner.ch>
>>>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>>>>> Cc: dri-devel@lists.freedesktop.org
>>>>>> Cc: imx@lists.linux.dev
>>>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>>>> ---
>>>>>> V2: No change
>>>>>> ---
>>>>>>    drivers/gpu/drm/bridge/imx/Kconfig          | 1 +
>>>>>>    drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 1 +
>>>>>>    2 files changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig
>>>> b/drivers/gpu/drm/bridge/imx/Kconfig
>>>>>> index 9a480c6abb856..d8e9fbf75edbb 100644
>>>>>> --- a/drivers/gpu/drm/bridge/imx/Kconfig
>>>>>> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
>>>>>> @@ -27,6 +27,7 @@ config DRM_IMX8MP_DW_HDMI_BRIDGE
>>>>>>    config DRM_IMX8MP_HDMI_PVI
>>>>>>    	tristate "Freescale i.MX8MP HDMI PVI bridge support"
>>>>>>    	depends on OF
>>>>>> +	select DRM_DISPLAY_CONNECTOR
>>>>>>    	help
>>>>>>    	  Choose this to enable support for the internal HDMI TX Parallel
>>>>>>    	  Video Interface found on the Freescale i.MX8MP SoC.
>>>>>> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>>>> b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>>>>>> index 1e7a789ec2890..4ebae5ad072ad 100644
>>>>>> --- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>>>>>> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
>>>>>> @@ -101,6 +101,7 @@ static int imx8mp_dw_hdmi_probe(struct
>>>> platform_device *pdev)
>>>>>>    	plat_data->phy_name = "SAMSUNG HDMI TX PHY";
>>>>>>    	plat_data->priv_data = hdmi;
>>>>>>    	plat_data->phy_force_vendor = true;
>>>>>> +	plat_data->output_port = 1;
>>>>>
>>>>> Quoting my feedback to a similar Liu's patch:
>>>>>
>>>>> This will break compatibility with older DT files, which don't have
>>>>> output port. I think you need to add output_port_optional flag to
>>>>> dw_hdmi_plat_data and still return 0 from dw_hdmi_parse_dt() if the flag
>>>>> is set, but there is no remote node.
>>>> Looking at the upstream imx8mp*dts , the oldest commit which adds HDMI
>>>> support is commit:
>>>>
>>>> 3e67a1ddd56d ("arm64: dts: imx8mp: Enable HDMI on
>>>> TQMa8MPxL/MBa8MPxL")
>>>>
>>>> That already contains the HDMI connector node. Every follow up addition
>>>> of HDMI to another device has been a copy of the same commit, with
>>>> connector, so I think it is safe to say, no upstream DT is going to be
>>>> broken by this change. Do we care about hypothetical downstream DTs
>>>> which may be missing the connector ?
>>>
>>> These have no HDMI connector nodes:
>>> arch/arm64/boot/dts/freescale/imx8mp-aristainetos3a-som-v1.dtsi
>>> arch/arm64/boot/dts/freescale/imx8mp-kontron-bl-osm-s.dts
>>> arch/arm64/boot/dts/freescale/imx8mp-kontron-smarc-eval-carrier.dts
>>> arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
>> Huh, I missed those, thanks.
>>
>> Would it be OK with you to fix those DTs up and add the missing connector,
>> rather than introduce some optional port workaround for them ?
> 
> I can't say for iMX8 particularly, but usually we try to keep backwards
> compatibility, as DT can be coming from device vendors. So, I'd say, we
> need both, the fixed DTS and the workaround.

FWIW, personally for the Kontron devicetrees mentioned above, I'm okay
with adding the connector node and break compatibility. Those
devicetrees have been added recently and I forgot to include the
connector node.

Of course I can only speak for myself and for Kontron, not for others.