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

[Dmitry Baryshkov]

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 at denx.de>
> > > > > ---
> > > > > Cc: Andrzej Hajda <andrzej.hajda at intel.com>
> > > > > Cc: David Airlie <airlied at gmail.com>
> > > > > Cc: Fabio Estevam <festevam at gmail.com>
> > > > > Cc: Jernej Skrabec <jernej.skrabec at gmail.com>
> > > > > Cc: Jonas Karlman <jonas at kwiboo.se>
> > > > > Cc: Laurent Pinchart <Laurent.pinchart at ideasonboard.com>
> > > > > Cc: Liu Ying <victor.liu at nxp.com>
> > > > > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > > > > Cc: Maxime Ripard <mripard at kernel.org>
> > > > > Cc: Neil Armstrong <neil.armstrong at linaro.org>
> > > > > Cc: Pengutronix Kernel Team <kernel at pengutronix.de>
> > > > > Cc: Robert Foss <rfoss at kernel.org>
> > > > > Cc: Sascha Hauer <s.hauer at pengutronix.de>
> > > > > Cc: Shawn Guo <shawnguo at kernel.org>
> > > > > Cc: Simona Vetter <simona at ffwll.ch>
> > > > > Cc: Stefan Agner <stefan at agner.ch>
> > > > > Cc: Thomas Zimmermann <tzimmermann at suse.de>
> > > > > Cc: dri-devel at lists.freedesktop.org
> > > > > Cc: imx at lists.linux.dev
> > > > > Cc: linux-arm-kernel at 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.

-- 
With best wishes
Dmitry