Hello Maxime,
On 9/2/22 16:41, Maxime Ripard wrote:
> The VC4 HDMI controller driver relies on the HDMI codec ASoC driver. In
> order to set it up properly, in vc4_hdmi_audio_init(), our HDMI driver
> will register a device matching the HDMI codec driver, and then register
> an ASoC card using that codec.
>
> However, if vc4 is compiled as a module, chances are that the hdmi-codec
> driver will be too. In such a case, the module loader will have a very
> narrow window to load the module between the device registration and the
> card registration.
>
> If it fails to load the module in time, the card registration will fail
> with EPROBE_DEFER, and we'll abort the audio initialisation,
> unregistering the HDMI codec device in the process.
>
> The next time the bind callback will be run, it's likely that we end up
> missing that window again, effectively preventing vc4 to probe entirely.
>
> In order to prevent this, we can create a soft dependency of the vc4
> driver on the HDMI codec one so that we're sure the HDMI codec will be
> loaded before the VC4 module is, and thus we'll never end up in the
> previous situation.
>
> Fixes: 91e99e113929 ("drm/vc4: hdmi: Register HDMI codec")
> Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
> ---
> drivers/gpu/drm/vc4/vc4_drv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index ffbbb454c9e8..2027063fdc30 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -490,6 +490,7 @@ module_init(vc4_drm_register);
> module_exit(vc4_drm_unregister);
>
> MODULE_ALIAS("platform:vc4-drm");
> +MODULE_SOFTDEP("pre: snd-soc-hdmi-codec");
While this will solve the issue, I'm not a big fan of the MODULE_SOFTDEP()
macro to be honest. I always have the feeling that's fragile and something
that may not be updated if for example a module name changes in the future.
I wonder if instead the HDMI_CODEC_DRV_NAME platform device should not be
registered earlier and in a place that's not cleaned up in the probe error
path. For example in vc4_drm_register() instead of vc4_hdmi_audio_init().
Granted, that would register the platform device even when HDMI isn't used
which is disadvantage of doing it at vc4_hdmi_bind() time. But still, feel
that is more robust to rely on the Linux device model than module softdeps.
Having said that, I don't have a strong opinion and this patch is correct
and has been in the mailing list for a long time. So feel free to push it
if you think is the correct approach to solve this:
Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
