All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/vc4: crtc: Fix runtime_pm reference counting
@ 2022-02-03 10:20 Maxime Ripard
  2022-02-17 16:18 ` Javier Martinez Canillas
  2022-02-17 16:34 ` (subset) " Maxime Ripard
  0 siblings, 2 replies; 4+ messages in thread
From: Maxime Ripard @ 2022-02-03 10:20 UTC (permalink / raw)
  To: dri-devel
  Cc: Dom Cobley, Tim Gover, Dave Stevenson, David Airlie,
	Maxime Ripard, Thomas Zimmermann, Daniel Vetter, Phil Elwell

At boot on the BCM2711, if the HDMI controllers are running, the CRTC
driver will disable itself and its associated HDMI controller to work
around a hardware bug that would leave some pixels stuck in a FIFO.

In order to avoid that issue, we need to run some operations in lockstep
between the CRTC and HDMI controller, and we need to make sure the HDMI
controller will be powered properly.

However, since we haven't enabled it through KMS, the runtime_pm state
is off at this point so we need to make sure the device is powered
through pm_runtime_resume_and_get, and once the operations are complete,
we call pm_runtime_put.

However, the HDMI controller will do that itself in its
post_crtc_powerdown, which means we'll end up calling pm_runtime_put for
a single pm_runtime_get, throwing the reference counting off. Let's
remove the pm_runtime_put call in the CRTC code in order to have the
proper counting.

Fixes: bca10db67bda ("drm/vc4: crtc: Make sure the HDMI controller is powered when disabling")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 287dbc89ad64..799aaf8c1abf 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -525,9 +525,11 @@ int vc4_crtc_disable_at_boot(struct drm_crtc *crtc)
 	if (ret)
 		return ret;
 
-	ret = pm_runtime_put(&vc4_hdmi->pdev->dev);
-	if (ret)
-		return ret;
+	/*
+	 * post_crtc_powerdown will have called pm_runtime_put, so we
+	 * don't need it here otherwise we'll get the reference counting
+	 * wrong.
+	 */
 
 	return 0;
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/vc4: crtc: Fix runtime_pm reference counting
  2022-02-03 10:20 [PATCH] drm/vc4: crtc: Fix runtime_pm reference counting Maxime Ripard
@ 2022-02-17 16:18 ` Javier Martinez Canillas
  2022-02-17 16:34 ` (subset) " Maxime Ripard
  1 sibling, 0 replies; 4+ messages in thread
From: Javier Martinez Canillas @ 2022-02-17 16:18 UTC (permalink / raw)
  To: Maxime Ripard, dri-devel
  Cc: Dom Cobley, Tim Gover, Dave Stevenson, David Airlie,
	Thomas Zimmermann, Daniel Vetter, Phil Elwell

Hello Maxime,

On 2/3/22 11:20, Maxime Ripard wrote:
> At boot on the BCM2711, if the HDMI controllers are running, the CRTC
> driver will disable itself and its associated HDMI controller to work
> around a hardware bug that would leave some pixels stuck in a FIFO.
> 
> In order to avoid that issue, we need to run some operations in lockstep
> between the CRTC and HDMI controller, and we need to make sure the HDMI
> controller will be powered properly.
> 
> However, since we haven't enabled it through KMS, the runtime_pm state
> is off at this point so we need to make sure the device is powered
> through pm_runtime_resume_and_get, and once the operations are complete,
> we call pm_runtime_put.
> 
> However, the HDMI controller will do that itself in its
> post_crtc_powerdown, which means we'll end up calling pm_runtime_put for
> a single pm_runtime_get, throwing the reference counting off. Let's
> remove the pm_runtime_put call in the CRTC code in order to have the
> proper counting.
> 
> Fixes: bca10db67bda ("drm/vc4: crtc: Make sure the HDMI controller is powered when disabling")
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/gpu/drm/vc4/vc4_crtc.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 287dbc89ad64..799aaf8c1abf 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -525,9 +525,11 @@ int vc4_crtc_disable_at_boot(struct drm_crtc *crtc)
>  	if (ret)
>  		return ret;
>  
> -	ret = pm_runtime_put(&vc4_hdmi->pdev->dev);
> -	if (ret)
> -		return ret;
> +	/*
> +	 * post_crtc_powerdown will have called pm_runtime_put, so we
> +	 * don't need it here otherwise we'll get the reference counting
> +	 * wrong.
> +	 */
>

I'm not familiar with the BCM2711 SoC nor its HDMI controller but your
commit message clearly explain the issue and is nice to have a comment
here, to avoid someone trying to do a pm_runtime_put() again.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: (subset) [PATCH] drm/vc4: crtc: Fix runtime_pm reference counting
  2022-02-03 10:20 [PATCH] drm/vc4: crtc: Fix runtime_pm reference counting Maxime Ripard
  2022-02-17 16:18 ` Javier Martinez Canillas
@ 2022-02-17 16:34 ` Maxime Ripard
  1 sibling, 0 replies; 4+ messages in thread
From: Maxime Ripard @ 2022-02-17 16:34 UTC (permalink / raw)
  To: Maxime Ripard, dri-devel
  Cc: Dom Cobley, Tim Gover, Dave Stevenson, David Airlie,
	Thomas Zimmermann, Daniel Vetter, Phil Elwell

On Thu, 3 Feb 2022 11:20:03 +0100, Maxime Ripard wrote:
> At boot on the BCM2711, if the HDMI controllers are running, the CRTC
> driver will disable itself and its associated HDMI controller to work
> around a hardware bug that would leave some pixels stuck in a FIFO.
> 
> In order to avoid that issue, we need to run some operations in lockstep
> between the CRTC and HDMI controller, and we need to make sure the HDMI
> controller will be powered properly.
> 
> [...]

Applied to drm/drm-misc (drm-misc-fixes).

Thanks!
Maxime

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/vc4: crtc: Fix runtime_pm reference counting
@ 2022-02-13 22:28 Stefan Wahren
  0 siblings, 0 replies; 4+ messages in thread
From: Stefan Wahren @ 2022-02-13 22:28 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Tim Gover, Dave Stevenson, David Airlie, Daniel Vetter,
	dri-devel, Phil Elwell

Hi Maxime,

i applied your patch on top of Linux 5.17-rc4 and it fixes the tones of
warnings after boot (see below). So you can have my

Tested-by: Stefan Wahren <stefan.wahren@i2se.com>

[    8.341814] ------------[ cut here ]------------
[    8.341820] WARNING: CPU: 1 PID: 7 at
drivers/gpu/drm/vc4/vc4_hdmi_regs.h:417
vc4_hdmi_write_infoframe+0x494/0x69c [vc4]
[    8.341861] Modules linked in: snd_soc_hdmi_codec brcmfmac
sha256_generic libsha256 sha256_arm cfg80211 brcmutil hci_uart btbcm
bluetooth vc4 ecdh_generic ecc libaes raspberrypi_hwmon snd_soc_core
ac97_bus snd_pcm_dmaengine snd_pcm snd_timer bcm2711_thermal
crc32_arm_ce snd genet soundcore drm_cma_helper mdio_bcm_unimac nvmem_rmem
[    8.341925] CPU: 1 PID: 7 Comm: kworker/u8:0 Not tainted 5.17.0-rc4 #21
[    8.341930] Hardware name: BCM2711
[    8.341933] Workqueue: events_unbound deferred_probe_work_func
[    8.341953]  unwind_backtrace from show_stack+0x10/0x14
[    8.341964]  show_stack from dump_stack_lvl+0x40/0x4c
[    8.341974]  dump_stack_lvl from __warn+0x14c/0x150
[    8.341983]  __warn from warn_slowpath_fmt+0xac/0xb4
[    8.341989]  warn_slowpath_fmt from
vc4_hdmi_write_infoframe+0x494/0x69c [vc4]
[    8.342026]  vc4_hdmi_write_infoframe [vc4] from
vc4_hdmi_encoder_post_crtc_enable+0x13ec/0x1910 [vc4]
[    8.342072]  vc4_hdmi_encoder_post_crtc_enable [vc4] from
drm_atomic_helper_commit_modeset_enables+0xa8/0x290
[    8.342104]  drm_atomic_helper_commit_modeset_enables from
vc4_atomic_commit_tail+0x230/0x700 [vc4]
[    8.342137]  vc4_atomic_commit_tail [vc4] from commit_tail+0xa4/0x194
[    8.342169]  commit_tail from drm_atomic_helper_commit+0x134/0x158
[    8.342178]  drm_atomic_helper_commit from
drm_client_modeset_commit_atomic+0x208/0x230
[    8.342192]  drm_client_modeset_commit_atomic from
drm_client_modeset_commit_locked+0x44/0x188
[    8.342206]  drm_client_modeset_commit_locked from
drm_client_modeset_commit+0x24/0x40
[    8.342214]  drm_client_modeset_commit from
__drm_fb_helper_restore_fbdev_mode_unlocked+0x60/0xc4
[    8.342225]  __drm_fb_helper_restore_fbdev_mode_unlocked from
drm_fb_helper_set_par+0x38/0x64
[    8.342233]  drm_fb_helper_set_par from fbcon_init+0x2f8/0x49c
[    8.342243]  fbcon_init from visual_init+0xbc/0x104
[    8.342253]  visual_init from do_bind_con_driver+0x1f0/0x3fc
[    8.342260]  do_bind_con_driver from do_take_over_console+0x84/0x1e0
[    8.342269]  do_take_over_console from do_fbcon_takeover+0x74/0xcc
[    8.342276]  do_fbcon_takeover from register_framebuffer+0x208/0x2dc
[    8.342285]  register_framebuffer from
__drm_fb_helper_initial_config_and_unlock+0x3f0/0x5bc
[    8.342293]  __drm_fb_helper_initial_config_and_unlock from
drm_fbdev_client_hotplug+0x118/0x1a8
[    8.342301]  drm_fbdev_client_hotplug from
drm_fbdev_generic_setup+0xa4/0x1a8
[    8.342308]  drm_fbdev_generic_setup from vc4_drm_bind+0x1b0/0x1b4 [vc4]
[    8.342336]  vc4_drm_bind [vc4] from try_to_bring_up_master+0x15c/0x1b4
[    8.342362]  try_to_bring_up_master from
component_master_add_with_match+0xc4/0xf8
[    8.342369]  component_master_add_with_match from
vc4_platform_drm_probe+0xa8/0xcc [vc4]
[    8.342396]  vc4_platform_drm_probe [vc4] from platform_probe+0x5c/0xb8
[    8.342425]  platform_probe from really_probe+0xc0/0x2fc
[    8.342430]  really_probe from __driver_probe_device+0x84/0xe4
[    8.342435]  __driver_probe_device from driver_probe_device+0x34/0xd4
[    8.342440]  driver_probe_device from __device_attach_driver+0x88/0xb4
[    8.342445]  __device_attach_driver from bus_for_each_drv+0x54/0xb4
[    8.342453]  bus_for_each_drv from __device_attach+0xdc/0x148
[    8.342460]  __device_attach from bus_probe_device+0x84/0x8c
[    8.342466]  bus_probe_device from deferred_probe_work_func+0x7c/0xa8
[    8.342475]  deferred_probe_work_func from process_one_work+0x228/0x524
[    8.342486]  process_one_work from worker_thread+0x40/0x56c
[    8.342493]  worker_thread from kthread+0xd8/0xf4
[    8.342501]  kthread from ret_from_fork+0x14/0x1c
[    8.342508] Exception stack(0xc20c3fb0 to 0xc20c3ff8)
[    8.342513] 3fa0:                                     00000000
00000000 00000000 00000000
[    8.342518] 3fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[    8.342523] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    8.342527] ---[ end trace 0000000000000000 ]---

...


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-02-17 16:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-03 10:20 [PATCH] drm/vc4: crtc: Fix runtime_pm reference counting Maxime Ripard
2022-02-17 16:18 ` Javier Martinez Canillas
2022-02-17 16:34 ` (subset) " Maxime Ripard
2022-02-13 22:28 Stefan Wahren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.