Hi Johan,
At 2024-07-03 02:23:24, "Johan Jonker" <jbx6...@gmail.com> wrote:
>Add sound support to the RK3066 HDMI driver.
>The HDMI TX audio source is connected to I2S_8CH.
>
>Signed-off-by: Zheng Yang <zhengy...@rock-chips.com>
>Signed-off-by: Johan Jonker <jbx6...@gmail.com>
>---
>
>Changed V8:
> return -EPROBE_DEFER as early as possible
> move rk3066_hdmi_audio_codec_init() function after rk3066_hdmi_register()
> restyle
>
>Changed V7:
> rebase
>---
> drivers/gpu/drm/rockchip/Kconfig | 2 +
> drivers/gpu/drm/rockchip/rk3066_hdmi.c | 296 +++++++++++++++++++++++--
> 2 files changed, 283 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/gpu/drm/rockchip/Kconfig
>b/drivers/gpu/drm/rockchip/Kconfig
>index 1bf3e2829cd0..a32ee558408c 100644
>--- a/drivers/gpu/drm/rockchip/Kconfig
>+++ b/drivers/gpu/drm/rockchip/Kconfig
>@@ -102,6 +102,8 @@ config ROCKCHIP_RGB
> config ROCKCHIP_RK3066_HDMI
> bool "Rockchip specific extensions for RK3066 HDMI"
> depends on DRM_ROCKCHIP
>+ select SND_SOC_HDMI_CODEC if SND_SOC
.........
> drm_encoder_helper_add(encoder, &rk3066_hdmi_encoder_helper_funcs);
> drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
>@@ -740,6 +988,7 @@ static int rk3066_hdmi_bind(struct device *dev, struct
>device *master,
> {
> struct platform_device *pdev = to_platform_device(dev);
> struct drm_device *drm = data;
>+ struct drm_encoder *encoder;
> struct rk3066_hdmi *hdmi;
> int irq;
> int ret;
>@@ -748,8 +997,21 @@ static int rk3066_hdmi_bind(struct device *dev, struct
>device *master,
> if (!hdmi)
> return -ENOMEM;
>
>+ encoder = &hdmi->encoder.encoder;
>+
>+ encoder->possible_crtcs =
>+ drm_of_find_possible_crtcs(drm, dev->of_node);
>+
>+ /*
>+ * If we failed to find the CRTC(s) which this encoder is
>+ * supposed to be connected to, it's because the CRTC has
>+ * not been registered yet. Defer probing, and hope that
>+ * the required CRTC is added later.
>+ */
>+ if (encoder->possible_crtcs == 0)
>+ return -EPROBE_DEFER;
Move EPROBE_DEFER early still does not avoid the issue I mentioned in V7.
For the component based driver mode in drm，the probe of last
subcomponent（component_add（）） will
trigger the bind callback of all subcomponent， any -EPROBE_DEFER return in
the bind callback will lead
the EPROBE_DEFER of the last subcomponent .probe.
For one example on rk3066:
LCDC0-->HDMI
LCDC1-->RGB-->panel
lcdc0(vop_probe)--->hdmi_bind-->audio register-->hdmi_codec_probe
success-->lcdc1(vop_bind)-->
rockchip_rgb_init(defer by somehow,maybe panel register failed)-->
lcdc0(vop_probe) return EPROBE_DEFER.
As there is on success probe(hdmi_codec_probe) during the process, this will
trigger a new deferred probe, see:
static int driver_probe_device(struct device_driver *drv, struct device *dev)
{
int trigger_count = atomic_read(&deferred_trigger_count);
int ret;
atomic_inc(&probe_count);
ret = __driver_probe_device(drv, dev);
if (ret == -EPROBE_DEFER || ret == EPROBE_DEFER) {
driver_deferred_probe_add(dev);
/*
* Did a trigger occur while probing? Need to re-trigger if yes
*/
if (trigger_count != atomic_read(&deferred_trigger_count) &&
!defer_all_probes)
driver_deferred_probe_trigger();
}
atomic_dec(&probe_count);
wake_up_all(&probe_waitqueue);
return ret;
}
So the potentia of infinite loop porbe rise。
I think one possible solutiion is register the extra dirvers（hdmi
audio，cec，hdcp）at the
encoder->funcs->late_register hook。
the late_register is called at the very last of drm_dev_register, all the
subcompent must
bind success if we finally run to this step.
>+
> hdmi->dev = dev;
>- hdmi->drm_dev = drm;
> hdmi->regs = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(hdmi->regs))
> return PTR_ERR(hdmi->regs);
>@@ -800,6 +1062,8 @@ static int rk3066_hdmi_bind(struct device *dev, struct
>device *master,
> if (ret)
> goto err_disable_i2c;
>
>+ rk3066_hdmi_audio_codec_init(hdmi, dev);
>+
> dev_set_drvdata(dev, hdmi);
>
> ret = devm_request_threaded_irq(dev, irq, rk3066_hdmi_hardirq,
>@@ -813,6 +1077,7 @@ static int rk3066_hdmi_bind(struct device *dev, struct
>device *master,
> return 0;
>
> err_cleanup_hdmi:
>+ platform_device_unregister(hdmi->audio_pdev);
> hdmi->connector.funcs->destroy(&hdmi->connector);
> hdmi->encoder.encoder.funcs->destroy(&hdmi->encoder.encoder);
> err_disable_i2c:
>@@ -828,6 +1093,7 @@ static void rk3066_hdmi_unbind(struct device *dev, struct
>device *master,
> {
> struct rk3066_hdmi *hdmi = dev_get_drvdata(dev);
>
>+ platform_device_unregister(hdmi->audio_pdev);
> hdmi->connector.funcs->destroy(&hdmi->connector);
> hdmi->encoder.encoder.funcs->destroy(&hdmi->encoder.encoder);
>
>--
>2.39.2
>
