[PATCH] drm/vc4: fix infinite EPROBE_DEFER loop

Gabriel Dalimonte posted 1 patch 7 months, 3 weeks ago
There is a newer version of this series
drivers/gpu/drm/vc4/vc4_hdmi.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
[PATCH] drm/vc4: fix infinite EPROBE_DEFER loop
Posted by Gabriel Dalimonte 7 months, 3 weeks ago
`vc4_hdmi_audio_init` calls `devm_snd_dmaengine_pcm_register` which may
return EPROBE_DEFER. Calling `drm_connector_hdmi_audio_init` adds a
child device. The driver model docs[1] state that adding a child device
prior to returning EPROBE_DEFER may result in an infinite loop.

[1] https://www.kernel.org/doc/html/v6.14/driver-api/driver-model/driver.html

Signed-off-by: Gabriel Dalimonte <gabriel.dalimonte@gmail.com>
---
Starting with v6.14, my Raspberry Pi 4B on the mainline kernel started seeing
the vc4 driver looping during probe with:

vc4-drm gpu: bound fe400000.hvs (ops vc4_hvs_ops [vc4])
Registered IR keymap rc-cec
rc rc0: vc4-hdmi-0 as /devices/platform/soc/fef00700.hdmi/rc/rc0
input: vc4-hdmi-0 as /devices/platform/soc/fef00700.hdmi/rc/rc0/input3503
vc4_hdmi fef00700.hdmi: Could not register PCM component: -517

repeating several times per second.

From my understanding, this happens due to the mainline kernel missing the
patches to support audio portion of the HDMI interface. In this case, or
other cases where the sound subsystem can't create a device, it returns
-517 (EPROBE_DEFER). All of this is consistent with what I experienced prior
to 6.14 as well. However, prior to 6.14 it did not try to probe infinitely.

Bisecting 6.13 -> 6.14, it looks like
9640f1437a88d8c617ff5523f1f9dc8c3ff29121 [1] moved HDMI audio connector
initialization from audio vc4 audio initialization to vc4 connector
initialization. If my understanding is correct, this change causes a child
device to be added before EPROBE_DEFER is returned and queues the device probe
to happen when a new device is added, which happens immediately because the
audio child device was added earlier in the probe. 

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9640f1437a88d8c617ff5523f1f9dc8c3ff29121
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index a29a6ef266f9a5952af53030a9a2d313e2ecdfce..163d092bd973bb3dfc5ea61187ec5fdf4f4f6029 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -560,12 +560,6 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
 	if (ret)
 		return ret;
 
-	ret = drm_connector_hdmi_audio_init(connector, dev->dev,
-					    &vc4_hdmi_audio_funcs,
-					    8, false, -1);
-	if (ret)
-		return ret;
-
 	drm_connector_helper_add(connector, &vc4_hdmi_connector_helper_funcs);
 
 	/*
@@ -2291,6 +2285,12 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
 		return ret;
 	}
 
+	ret = drm_connector_hdmi_audio_init(&vc4_hdmi->connector, dev,
+					    &vc4_hdmi_audio_funcs, 8, false,
+					    -1);
+	if (ret)
+		return ret;
+
 	dai_link->cpus		= &vc4_hdmi->audio.cpu;
 	dai_link->codecs	= &vc4_hdmi->audio.codec;
 	dai_link->platforms	= &vc4_hdmi->audio.platform;

---
base-commit: b60301774a8fe6c30b14a95104ec099290a2e904
change-id: 20250426-vc4-audio-inf-probe-f67a8aa2a180

Best regards,
-- 
Gabriel Dalimonte <gabriel.dalimonte@gmail.com>
Re: [PATCH] drm/vc4: fix infinite EPROBE_DEFER loop
Posted by Dave Stevenson 7 months, 3 weeks ago
Hi Gabriel

On Sat, 26 Apr 2025 at 07:23, Gabriel Dalimonte
<gabriel.dalimonte@gmail.com> wrote:
>
> `vc4_hdmi_audio_init` calls `devm_snd_dmaengine_pcm_register` which may
> return EPROBE_DEFER. Calling `drm_connector_hdmi_audio_init` adds a
> child device. The driver model docs[1] state that adding a child device
> prior to returning EPROBE_DEFER may result in an infinite loop.
>
> [1] https://www.kernel.org/doc/html/v6.14/driver-api/driver-model/driver.html
>
> Signed-off-by: Gabriel Dalimonte <gabriel.dalimonte@gmail.com>
> ---
> Starting with v6.14, my Raspberry Pi 4B on the mainline kernel started seeing
> the vc4 driver looping during probe with:
>
> vc4-drm gpu: bound fe400000.hvs (ops vc4_hvs_ops [vc4])
> Registered IR keymap rc-cec
> rc rc0: vc4-hdmi-0 as /devices/platform/soc/fef00700.hdmi/rc/rc0
> input: vc4-hdmi-0 as /devices/platform/soc/fef00700.hdmi/rc/rc0/input3503
> vc4_hdmi fef00700.hdmi: Could not register PCM component: -517
>
> repeating several times per second.
>
> From my understanding, this happens due to the mainline kernel missing the
> patches to support audio portion of the HDMI interface. In this case, or
> other cases where the sound subsystem can't create a device, it returns
> -517 (EPROBE_DEFER). All of this is consistent with what I experienced prior
> to 6.14 as well. However, prior to 6.14 it did not try to probe infinitely.

Mainline should have all the bits for HDMI audio on Pi4.
It doesn't have the bits for Pi5 as it needs the newer DMA controller.

> Bisecting 6.13 -> 6.14, it looks like
> 9640f1437a88d8c617ff5523f1f9dc8c3ff29121 [1] moved HDMI audio connector
> initialization from audio vc4 audio initialization to vc4 connector
> initialization. If my understanding is correct, this change causes a child
> device to be added before EPROBE_DEFER is returned and queues the device probe
> to happen when a new device is added, which happens immediately because the
> audio child device was added earlier in the probe.

cc Dmitry as the author of that patch.

However I don't see an issue with moving the init back to vc4_hdmi_audio_init.
I'm not an expert on the sequencing of things around the audio side
though, so I wonder if Dmitry or Maxime could comment.

The patch could do with a Fixes: tag if 9640f1437a88 if it is
definitely the commit that breaks things.

Thanks
  Dave

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9640f1437a88d8c617ff5523f1f9dc8c3ff29121
> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index a29a6ef266f9a5952af53030a9a2d313e2ecdfce..163d092bd973bb3dfc5ea61187ec5fdf4f4f6029 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -560,12 +560,6 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
>         if (ret)
>                 return ret;
>
> -       ret = drm_connector_hdmi_audio_init(connector, dev->dev,
> -                                           &vc4_hdmi_audio_funcs,
> -                                           8, false, -1);
> -       if (ret)
> -               return ret;
> -
>         drm_connector_helper_add(connector, &vc4_hdmi_connector_helper_funcs);
>
>         /*
> @@ -2291,6 +2285,12 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
>                 return ret;
>         }
>
> +       ret = drm_connector_hdmi_audio_init(&vc4_hdmi->connector, dev,
> +                                           &vc4_hdmi_audio_funcs, 8, false,
> +                                           -1);
> +       if (ret)
> +               return ret;
> +
>         dai_link->cpus          = &vc4_hdmi->audio.cpu;
>         dai_link->codecs        = &vc4_hdmi->audio.codec;
>         dai_link->platforms     = &vc4_hdmi->audio.platform;
>
> ---
> base-commit: b60301774a8fe6c30b14a95104ec099290a2e904
> change-id: 20250426-vc4-audio-inf-probe-f67a8aa2a180
>
> Best regards,
> --
> Gabriel Dalimonte <gabriel.dalimonte@gmail.com>
>