On 02.11.2022 19:07, Dmitry Baryshkov wrote:
> Use drm_connector's helpers enable_hpd and disable_hpd to enable and
> disable HPD automatically by the means of drm_kms_helper_poll_*
> functions. As the drm_bridge_connector_enable_hpd() and
> drm_bridge_connector_disable_hpd() functions are now unused, replace
> them with stubs to ease driver migration.
>
> Enabling the HPD from drm_bridge_connector_init() can happen too early,
> before the driver is prepared to handle HPD events. As the
> drm_bridge_connector_enable_hpd() is empty anyway, drop this call
> anyway.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
This patch, merged to recent linux-next as commit 92d755d8f13b
("drm/bridge_connector: rely on drm_kms_helper_poll_* for HPD
enablement"), triggers the following warning on all my Amlogic Meson
based boards:
------------[ cut here ]------------
Hot plug detection already enabled
WARNING: CPU: 2 PID: 285 at drivers/gpu/drm/drm_bridge.c:1257
drm_bridge_hpd_enable+0x90/0xa0
Modules linked in: snd_soc_meson_axg_frddr snd_soc_meson_axg_fifo
dwmac_generic panfrost(+) reset_meson_audio_arb(+) drm_shmem_helper
meson_dw_hdmi(+) dw_hdmi rc_odroid drm_display_helper meson_ir(+)
gpu_sched meson_rng rng_core meson_gxbb_wdt crct10dif_ce dwmac_meson8b
stmmac_platform snd_soc_meson_axg_sound_card snd_soc_meson_card_utils
mdio_mux_meson_g12a stmmac meson_drm pwm_meson pcs_xpcs meson_canvas
snd_soc_meson_axg_tdm_interface rtc_meson_vrtc
snd_soc_meson_axg_tdm_formatter nvmem_meson_efuse display_connector
CPU: 2 PID: 285 Comm: systemd-udevd Not tainted 6.1.0-rc6+ #13236
Hardware name: Hardkernel ODROID-C4 (DT)
pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : drm_bridge_hpd_enable+0x90/0xa0
lr : drm_bridge_hpd_enable+0x90/0xa0
...
Call trace:
drm_bridge_hpd_enable+0x90/0xa0
_drm_bridge_connector_enable_hpd+0x24/0x34
drm_kms_helper_poll_enable.part.0+0x50/0xd0
drm_helper_probe_single_connector_modes+0x400/0x530
drm_client_modeset_probe+0x1ec/0xba0
__drm_fb_helper_initial_config_and_unlock+0x4c/0x630
drm_fb_helper_initial_config+0x50/0x60
drm_fbdev_client_hotplug+0xc4/0x150
drm_fbdev_generic_setup+0x88/0x150
meson_drv_bind_master+0x3d4/0x4a0 [meson_drm]
meson_drv_bind+0x14/0x20 [meson_drm]
try_to_bring_up_aggregate_device+0x164/0x1d0
__component_add+0xa8/0x174
component_add+0x14/0x20
meson_dw_hdmi_probe+0x1c/0x30 [meson_dw_hdmi]
platform_probe+0x68/0xe0
really_probe+0xbc/0x2e0
__driver_probe_device+0x78/0xe0
driver_probe_device+0x3c/0x160
__driver_attach+0x88/0x1a0
bus_for_each_dev+0x70/0xd0
driver_attach+0x24/0x30
bus_add_driver+0x154/0x204
driver_register+0x78/0x130
__platform_driver_register+0x28/0x34
meson_dw_hdmi_platform_driver_init+0x24/0x1000 [meson_dw_hdmi]
do_one_initcall+0x74/0x400
do_init_module+0x48/0x1d4
load_module+0x1818/0x1de0
__do_sys_finit_module+0xc0/0x114
__arm64_sys_finit_module+0x20/0x30
invoke_syscall+0x48/0x114
el0_svc_common.constprop.0+0x60/0x11c
do_el0_svc_compat+0x1c/0x40
el0_svc_compat+0x48/0xc0
el0t_32_sync_handler+0x90/0x140
el0t_32_sync+0x190/0x194
irq event stamp: 17422
hardirqs last enabled at (17421): [<ffff800009196058>]
__schedule+0x998/0xa14
hardirqs last disabled at (17422): [<ffff80000918e4a4>] el1_dbg+0x24/0x90
softirqs last enabled at (17404): [<ffff80000801046c>] _stext+0x46c/0x5d8
softirqs last disabled at (17383): [<ffff8000080166d0>]
____do_softirq+0x10/0x20
---[ end trace 0000000000000000 ]---
I looks that this change requires some additional changes/fixes in the
drivers/gpu/drm/meson/meson_dw_hdmi.c
> drivers/gpu/drm/drm_bridge_connector.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c
> index 1c7d936523df..0e13bc87a6ac 100644
> --- a/drivers/gpu/drm/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/drm_bridge_connector.c
> @@ -136,6 +136,11 @@ static void drm_bridge_connector_hpd_cb(void *cb_data,
> * This is typically used by display drivers in their resume handler.
> */
> void drm_bridge_connector_enable_hpd(struct drm_connector *connector)
> +{
> +}
> +EXPORT_SYMBOL_GPL(drm_bridge_connector_enable_hpd);
> +
> +static void _drm_bridge_connector_enable_hpd(struct drm_connector *connector)
> {
> struct drm_bridge_connector *bridge_connector =
> to_drm_bridge_connector(connector);
> @@ -145,7 +150,6 @@ void drm_bridge_connector_enable_hpd(struct drm_connector *connector)
> drm_bridge_hpd_enable(hpd, drm_bridge_connector_hpd_cb,
> bridge_connector);
> }
> -EXPORT_SYMBOL_GPL(drm_bridge_connector_enable_hpd);
>
> /**
> * drm_bridge_connector_disable_hpd - Disable hot-plug detection for the
> @@ -156,6 +160,11 @@ EXPORT_SYMBOL_GPL(drm_bridge_connector_enable_hpd);
> * This is typically used by display drivers in their suspend handler.
> */
> void drm_bridge_connector_disable_hpd(struct drm_connector *connector)
> +{
> +}
> +EXPORT_SYMBOL_GPL(drm_bridge_connector_disable_hpd);
> +
> +static void _drm_bridge_connector_disable_hpd(struct drm_connector *connector)
> {
> struct drm_bridge_connector *bridge_connector =
> to_drm_bridge_connector(connector);
> @@ -164,7 +173,6 @@ void drm_bridge_connector_disable_hpd(struct drm_connector *connector)
> if (hpd)
> drm_bridge_hpd_disable(hpd);
> }
> -EXPORT_SYMBOL_GPL(drm_bridge_connector_disable_hpd);
>
> /* -----------------------------------------------------------------------------
> * Bridge Connector Functions
> @@ -305,6 +313,8 @@ static int drm_bridge_connector_get_modes(struct drm_connector *connector)
> static const struct drm_connector_helper_funcs drm_bridge_connector_helper_funcs = {
> .get_modes = drm_bridge_connector_get_modes,
> /* No need for .mode_valid(), the bridges are checked by the core. */
> + .enable_hpd = _drm_bridge_connector_enable_hpd,
> + .disable_hpd = _drm_bridge_connector_disable_hpd,
> };
>
> /* -----------------------------------------------------------------------------
> @@ -387,10 +397,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> connector_type, ddc);
> drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs);
>
> - if (bridge_connector->bridge_hpd) {
> + if (bridge_connector->bridge_hpd)
> connector->polled = DRM_CONNECTOR_POLL_HPD;
> - drm_bridge_connector_enable_hpd(connector);
> - }
> else if (bridge_connector->bridge_detect)
> connector->polled = DRM_CONNECTOR_POLL_CONNECT
> | DRM_CONNECTOR_POLL_DISCONNECT;
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
