diff mbox series

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

Message ID 20241224014701.253490-1-marex@denx.de (mailing list archive)
State New
Headers show
Series [v2,1/3] drm/bridge: imx8mp-hdmi-tx: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR | expand

Commit Message

Marek Vasut Dec. 24, 2024, 1:46 a.m. UTC
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(+)

Comments

Dmitry Baryshkov Dec. 24, 2024, 4:21 a.m. UTC | #1
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.

>  
>  	hdmi->dw_hdmi = dw_hdmi_probe(pdev, plat_data);
>  	if (IS_ERR(hdmi->dw_hdmi))
> -- 
> 2.45.2
>
Marek Vasut Dec. 25, 2024, 8:40 p.m. UTC | #2
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 ?
Dmitry Baryshkov Dec. 26, 2024, 8:21 p.m. UTC | #3
On Wed, 25 Dec 2024 at 22:50, Marek Vasut <marex@denx.de> 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 ?

I'd say, not really. If you have to resend the series for whatever
reason, please mention that in the commit message.
If you don't it should be fine.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Liu Ying Dec. 30, 2024, 6:57 a.m. UTC | #4
On 12/24/2024, 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

Select it for DRM_IMX8MP_DW_HDMI_BRIDGE instead?
Furthermore, if yes, should it be even selected for DRM_DW_HDMI instead?

I'm not sure if this is needed to be selected though, since only DRM_MESON is
selecting it while CONFIG_DRM_DISPLAY_CONNECTOR is enabled in multiple
defconfig files(like arch/arm/configs/multi_v7_defconfig).

>  	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;

Dmitry's comment applies here.
https://lore.kernel.org/all/vvsj6ri2ke25nzocbq736yv7rphzma6pn3yk2uh7iu43zfe2sa@2fwye4k4w6he/

>  
>  	hdmi->dw_hdmi = dw_hdmi_probe(pdev, plat_data);
>  	if (IS_ERR(hdmi->dw_hdmi))
Liu Ying Dec. 30, 2024, 7:04 a.m. UTC | #5
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

Regards,
Liu Ying
Dmitry Baryshkov Dec. 30, 2024, 1:48 p.m. UTC | #6
On Mon, Dec 30, 2024 at 07:04:51AM +0000, 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

Indeed. I should be less trustful.

Unreviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Marek Vasut Dec. 30, 2024, 9:41 p.m. UTC | #7
On 12/30/24 7:57 AM, Liu Ying wrote:

[...]

>> 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
> 
> Select it for DRM_IMX8MP_DW_HDMI_BRIDGE instead?
> Furthermore, if yes, should it be even selected for DRM_DW_HDMI instead?
> 
> I'm not sure if this is needed to be selected though, since only DRM_MESON is
> selecting it while CONFIG_DRM_DISPLAY_CONNECTOR is enabled in multiple
> defconfig files(like arch/arm/configs/multi_v7_defconfig).

It is necessary to update existing .config .

[...]
Marek Vasut Dec. 30, 2024, 9:44 p.m. UTC | #8
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 ?
Dmitry Baryshkov Dec. 30, 2024, 11:48 p.m. UTC | #9
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.
Frieder Schrempf Jan. 13, 2025, 9:13 a.m. UTC | #10
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.
diff mbox series

Patch

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;
 
 	hdmi->dw_hdmi = dw_hdmi_probe(pdev, plat_data);
 	if (IS_ERR(hdmi->dw_hdmi))