On 2022-09-12 18:08, Stefan Agner wrote: > On 2022-08-15 19:58, Greg Kroah-Hartman wrote: >> From: Neil Armstrong <narmstrong@xxxxxxxxxxxx> >> >> [ Upstream commit 0af5e0b41110e2da872030395231ab19c45be931 ] >> >> This implements the necessary change to no more use the embedded >> connector in dw-hdmi and use the dedicated bridge connector driver >> by passing DRM_BRIDGE_ATTACH_NO_CONNECTOR to the bridge attach call. >> >> The necessary connector properties are added to handle the same >> functionalities as the embedded dw-hdmi connector, i.e. the HDR >> metadata, the CEC notifier & other flags. >> >> 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. > > HDMI on ODROID-N2+ was working with v5.15.60, and stopped working with > v5.15.61. Reverting this commit (and two dependent refcount leak) to be > the culprit. Reverting just the refcount leaks is not enough to get HDMI > working, so I assume it is this commit. > > I haven't investigated much beyond that, maybe its simple a case of a > missing kernel configuration? DRM_DISPLAY_CONNECTOR is compiled, and the > module display_connector is loaded, so that part seemed to have worked. > > Any ideas welcome. > > FWIW, I track the issue in the HAOS tracker at > https://github.com/home-assistant/operating-system/issues/2120. It seems that backporting commit 7cd70656d128 ("drm/bridge: display-connector: implement bus fmts callbacks") fixes the problem without reverting this commit. @Greg, can we backport this commit as well? -- Stefan > > -- > Stefan > >> >> Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx> >> Acked-by: Sam Ravnborg <sam@xxxxxxxxxxxx> >> Acked-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> >> Link: >> https://patchwork.freedesktop.org/patch/msgid/20211020123947.2585572-5-narmstrong@xxxxxxxxxxxx >> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> >> --- >> drivers/gpu/drm/meson/Kconfig | 2 + >> drivers/gpu/drm/meson/meson_dw_hdmi.c | 1 + >> drivers/gpu/drm/meson/meson_encoder_hdmi.c | 81 +++++++++++++++++++++- >> 3 files changed, 82 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/meson/Kconfig b/drivers/gpu/drm/meson/Kconfig >> index 9f9281dd49f8..a4e1ed96e5e8 100644 >> --- a/drivers/gpu/drm/meson/Kconfig >> +++ b/drivers/gpu/drm/meson/Kconfig >> @@ -6,9 +6,11 @@ config DRM_MESON >> select DRM_KMS_HELPER >> select DRM_KMS_CMA_HELPER >> select DRM_GEM_CMA_HELPER >> + select DRM_DISPLAY_CONNECTOR >> select VIDEOMODE_HELPERS >> select REGMAP_MMIO >> select MESON_CANVAS >> + select CEC_CORE if CEC_NOTIFIER >> >> config DRM_MESON_DW_HDMI >> tristate "HDMI Synopsys Controller support for Amlogic Meson Display" >> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c >> b/drivers/gpu/drm/meson/meson_dw_hdmi.c >> index fb540a503efe..5cd2b2ebbbd3 100644 >> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c >> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c >> @@ -803,6 +803,7 @@ static int meson_dw_hdmi_bind(struct device *dev, >> struct device *master, >> dw_plat_data->input_bus_encoding = V4L2_YCBCR_ENC_709; >> dw_plat_data->ycbcr_420_allowed = true; >> dw_plat_data->disable_cec = true; >> + dw_plat_data->output_port = 1; >> >> if (dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-gxl-dw-hdmi") || >> dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-gxm-dw-hdmi") || >> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c >> b/drivers/gpu/drm/meson/meson_encoder_hdmi.c >> index db332fa4cd54..5e306de6f485 100644 >> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c >> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c >> @@ -14,8 +14,11 @@ >> #include <linux/regulator/consumer.h> >> #include <linux/reset.h> >> >> +#include <media/cec-notifier.h> >> + >> #include <drm/drm_atomic_helper.h> >> #include <drm/drm_bridge.h> >> +#include <drm/drm_bridge_connector.h> >> #include <drm/drm_device.h> >> #include <drm/drm_edid.h> >> #include <drm/drm_probe_helper.h> >> @@ -34,8 +37,10 @@ struct meson_encoder_hdmi { >> struct drm_encoder encoder; >> struct drm_bridge bridge; >> struct drm_bridge *next_bridge; >> + struct drm_connector *connector; >> struct meson_drm *priv; >> unsigned long output_bus_fmt; >> + struct cec_notifier *cec_notifier; >> }; >> >> #define bridge_to_meson_encoder_hdmi(x) \ >> @@ -50,6 +55,14 @@ static int meson_encoder_hdmi_attach(struct >> drm_bridge *bridge, >> &encoder_hdmi->bridge, flags); >> } >> >> +static void meson_encoder_hdmi_detach(struct drm_bridge *bridge) >> +{ >> + struct meson_encoder_hdmi *encoder_hdmi = >> bridge_to_meson_encoder_hdmi(bridge); >> + >> + cec_notifier_conn_unregister(encoder_hdmi->cec_notifier); >> + encoder_hdmi->cec_notifier = NULL; >> +} >> + >> static void meson_encoder_hdmi_set_vclk(struct meson_encoder_hdmi >> *encoder_hdmi, >> const struct drm_display_mode *mode) >> { >> @@ -298,9 +311,30 @@ static int meson_encoder_hdmi_atomic_check(struct >> drm_bridge *bridge, >> return 0; >> } >> >> +static void meson_encoder_hdmi_hpd_notify(struct drm_bridge *bridge, >> + enum drm_connector_status status) >> +{ >> + struct meson_encoder_hdmi *encoder_hdmi = >> bridge_to_meson_encoder_hdmi(bridge); >> + struct edid *edid; >> + >> + if (!encoder_hdmi->cec_notifier) >> + return; >> + >> + if (status == connector_status_connected) { >> + edid = drm_bridge_get_edid(encoder_hdmi->next_bridge, >> encoder_hdmi->connector); >> + if (!edid) >> + return; >> + >> + cec_notifier_set_phys_addr_from_edid(encoder_hdmi->cec_notifier, edid); >> + } else >> + cec_notifier_phys_addr_invalidate(encoder_hdmi->cec_notifier); >> +} >> + >> static const struct drm_bridge_funcs meson_encoder_hdmi_bridge_funcs = { >> .attach = meson_encoder_hdmi_attach, >> + .detach = meson_encoder_hdmi_detach, >> .mode_valid = meson_encoder_hdmi_mode_valid, >> + .hpd_notify = meson_encoder_hdmi_hpd_notify, >> .atomic_enable = meson_encoder_hdmi_atomic_enable, >> .atomic_disable = meson_encoder_hdmi_atomic_disable, >> .atomic_get_input_bus_fmts = meson_encoder_hdmi_get_inp_bus_fmts, >> @@ -313,6 +347,7 @@ static const struct drm_bridge_funcs >> meson_encoder_hdmi_bridge_funcs = { >> int meson_encoder_hdmi_init(struct meson_drm *priv) >> { >> struct meson_encoder_hdmi *meson_encoder_hdmi; >> + struct platform_device *pdev; >> struct device_node *remote; >> int ret; >> >> @@ -337,6 +372,7 @@ int meson_encoder_hdmi_init(struct meson_drm *priv) >> meson_encoder_hdmi->bridge.funcs = &meson_encoder_hdmi_bridge_funcs; >> meson_encoder_hdmi->bridge.of_node = priv->dev->of_node; >> meson_encoder_hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA; >> + meson_encoder_hdmi->bridge.interlace_allowed = true; >> >> drm_bridge_add(&meson_encoder_hdmi->bridge); >> >> @@ -353,17 +389,58 @@ int meson_encoder_hdmi_init(struct meson_drm *priv) >> meson_encoder_hdmi->encoder.possible_crtcs = BIT(0); >> >> /* Attach HDMI Encoder Bridge to Encoder */ >> - ret = drm_bridge_attach(&meson_encoder_hdmi->encoder, >> &meson_encoder_hdmi->bridge, NULL, 0); >> + ret = drm_bridge_attach(&meson_encoder_hdmi->encoder, >> &meson_encoder_hdmi->bridge, NULL, >> + DRM_BRIDGE_ATTACH_NO_CONNECTOR); >> if (ret) { >> dev_err(priv->dev, "Failed to attach bridge: %d\n", ret); >> return ret; >> } >> >> + /* Initialize & attach Bridge Connector */ >> + meson_encoder_hdmi->connector = drm_bridge_connector_init(priv->drm, >> + &meson_encoder_hdmi->encoder); >> + if (IS_ERR(meson_encoder_hdmi->connector)) { >> + dev_err(priv->dev, "Unable to create HDMI bridge connector\n"); >> + return PTR_ERR(meson_encoder_hdmi->connector); >> + } >> + drm_connector_attach_encoder(meson_encoder_hdmi->connector, >> + &meson_encoder_hdmi->encoder); >> + >> /* >> * We should have now in place: >> - * encoder->[hdmi encoder bridge]->[dw-hdmi bridge]->[dw-hdmi connector] >> + * encoder->[hdmi encoder bridge]->[dw-hdmi bridge]->[display >> connector bridge]->[display connector] >> */ >> >> + /* >> + * drm_connector_attach_max_bpc_property() requires the >> + * connector to have a state. >> + */ >> + drm_atomic_helper_connector_reset(meson_encoder_hdmi->connector); >> + >> + if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL) || >> + meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) || >> + meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) >> + drm_connector_attach_hdr_output_metadata_property(meson_encoder_hdmi->connector); >> + >> + drm_connector_attach_max_bpc_property(meson_encoder_hdmi->connector, 8, 8); >> + >> + /* Handle this here until handled by drm_bridge_connector_init() */ >> + meson_encoder_hdmi->connector->ycbcr_420_allowed = true; >> + >> + pdev = of_find_device_by_node(remote); >> + if (pdev) { >> + struct cec_connector_info conn_info; >> + struct cec_notifier *notifier; >> + >> + cec_fill_conn_info_from_drm(&conn_info, meson_encoder_hdmi->connector); >> + >> + notifier = cec_notifier_conn_register(&pdev->dev, NULL, &conn_info); >> + if (!notifier) >> + return -ENOMEM; >> + >> + meson_encoder_hdmi->cec_notifier = notifier; >> + } >> + >> dev_dbg(priv->dev, "HDMI encoder initialized\n"); >> >> return 0;
