Re: [v3,3/7] drm/bridge_connector: rely on drm_kms_helper_poll_* for HPD enablement

Re: [v3,3/7] drm/bridge_connector: rely on drm_kms_helper_poll_* for HPD enablement

[Marek Szyprowski]

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@linaro.org>
> ---

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


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [v3,3/7] drm/bridge_connector: rely on drm_kms_helper_poll_* for HPD enablement

[Neil Armstrong]

On 11/01/2023 13:41, Marek Szyprowski wrote:
> 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@linaro.org>
>> ---
> 
> 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

Weird because _drm_bridge_connector_enable_hpd() has been removed with
4c00ac500d0e ("drm/bridge_connector: drop drm_bridge_connector_en/disable_hpd()")


>    drm_kms_helper_poll_enable.part.0+0x50/0xd0

It seems drm_kms_helper_poll_enable is called again by
drm_helper_probe_single_connector_modes() because poll_running isn't set.

Can you try with this change:
===============><===========================================
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 7973f2589ced..5d60d4fd868f 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -245,7 +245,7 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
  	struct drm_connector_list_iter conn_iter;
  	unsigned long delay = DRM_OUTPUT_POLL_PERIOD;

-	if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
+	if (!dev->mode_config.poll_init || !drm_kms_helper_poll || dev->mode_config.poll_enabled)
  		return;

  	drm_connector_list_iter_begin(dev, &conn_iter);
@@ -279,6 +279,8 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)

  	if (poll)
  		schedule_delayed_work(&dev->mode_config.output_poll_work, delay);
+
+	dev->mode_config.poll_enabled = true;
  }
  EXPORT_SYMBOL(drm_kms_helper_poll_enable);

@@ -561,7 +563,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
  		 * disable itself again.
  		 */
  		dev->mode_config.delayed_event = true;
-		if (dev->mode_config.poll_enabled)
+		if (dev->mode_config.poll_init)
  			schedule_delayed_work(&dev->mode_config.output_poll_work,
  					      0);
  	}
@@ -703,7 +705,7 @@ static void output_poll_execute(struct work_struct *work)
  	bool repoll = false, changed;
  	u64 old_epoch_counter;

-	if (!dev->mode_config.poll_enabled)
+	if (!dev->mode_config.poll_init)
  		return;

  	/* Pick up any changes detected by the probe functions. */
@@ -813,11 +815,11 @@ static void drm_kms_helper_poll_disable_fini(struct drm_device *dev, bool fini)
  	struct drm_connector *connector;
  	struct drm_connector_list_iter conn_iter;

-	if (!dev->mode_config.poll_enabled)
+	if (!dev->mode_config.poll_init || !dev->mode_config.poll_enabled)
  		return;

  	if (fini)
-		dev->mode_config.poll_enabled = false;
+		dev->mode_config.poll_init = false;

  	drm_connector_list_iter_begin(dev, &conn_iter);
  	drm_for_each_connector_iter(connector, &conn_iter) {
@@ -830,6 +832,8 @@ static void drm_kms_helper_poll_disable_fini(struct drm_device *dev, bool fini)
  	drm_connector_list_iter_end(&conn_iter);

  	cancel_delayed_work_sync(&dev->mode_config.output_poll_work);
+
+	dev->mode_config.poll_enabled = false;
  }

  /**
@@ -874,7 +878,7 @@ EXPORT_SYMBOL(drm_kms_helper_poll_disable);
  void drm_kms_helper_poll_init(struct drm_device *dev)
  {
  	INIT_DELAYED_WORK(&dev->mode_config.output_poll_work, output_poll_execute);
-	dev->mode_config.poll_enabled = true;
+	dev->mode_config.poll_init = true;

  	drm_kms_helper_poll_enable(dev);
  }
@@ -1003,7 +1007,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
  	struct drm_connector_list_iter conn_iter;
  	int changed = 0;

-	if (!dev->mode_config.poll_enabled)
+	if (!dev->mode_config.poll_init)
  		return false;

  	mutex_lock(&dev->mode_config.mutex);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index e5b053001d22..86c5f66f1a6c 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -345,6 +345,7 @@ struct drm_mode_config_funcs {
   * @max_width: maximum fb pixel width on this device
   * @max_height: maximum fb pixel height on this device
   * @funcs: core driver provided mode setting functions
+ * @poll_init: track polling initialization for this device
   * @poll_enabled: track polling support for this device
   * @poll_running: track polling status for this device
   * @delayed_event: track delayed poll uevent deliver for this device
@@ -543,6 +544,7 @@ struct drm_mode_config {
  	const struct drm_mode_config_funcs *funcs;

  	/* output poll support */
+	bool poll_init;
  	bool poll_enabled;
  	bool poll_running;
  	bool delayed_event;
===============><===========================================

Neil

>    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


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [v3,3/7] drm/bridge_connector: rely on drm_kms_helper_poll_* for HPD enablement

[Dmitry Baryshkov]

On 12/01/2023 11:35, Neil Armstrong wrote:
> On 11/01/2023 13:41, Marek Szyprowski wrote:
>> 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@linaro.org>
>>> ---
>>
>> 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
> 
> Weird because _drm_bridge_connector_enable_hpd() has been removed with
> 4c00ac500d0e ("drm/bridge_connector: drop 
> drm_bridge_connector_en/disable_hpd()")
> 
> 
>>    drm_kms_helper_poll_enable.part.0+0x50/0xd0
> 
> It seems drm_kms_helper_poll_enable is called again by
> drm_helper_probe_single_connector_modes() because poll_running isn't set.

Yes, I'm currently reworking poll_running handling. I hope to post a 
patch today.

-- 
With best wishes
Dmitry


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [v3,3/7] drm/bridge_connector: rely on drm_kms_helper_poll_* for HPD enablement

[Marek Szyprowski]

Hi Neil,

On 12.01.2023 10:35, Neil Armstrong wrote:
> On 11/01/2023 13:41, Marek Szyprowski wrote:
>> 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@linaro.org>
>>> ---
>>
>> 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
>
> Weird because _drm_bridge_connector_enable_hpd() has been removed with
> 4c00ac500d0e ("drm/bridge_connector: drop 
> drm_bridge_connector_en/disable_hpd()")

The above log has been captured at commit 92d755d8f13b during bisecting, 
so 4c00ac500d0e is not applied yet. Maybe I should have mention that. 
The issue on top of linux-next is similar.

>
>>  drm_kms_helper_poll_enable.part.0+0x50/0xd0
>
> It seems drm_kms_helper_poll_enable is called again by
> drm_helper_probe_single_connector_modes() because poll_running isn't set.
>
> Can you try with this change:

This fixes the issue on top of linux next-20230112. Thanks! Feel free to 
add if you plan to submit it as a formal patch:

Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

 > ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic