<div style="line-height:1.7;color:#000000;font-size:14px;font-family:Arial"><div id="spnEditorContent"><p style="margin: 0;">Hi Johan,</p></div><pre>At 2024-07-03 02:23:24, "Johan Jonker" <jbx6244@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 <zhengyang@rock-chips.com>
>Signed-off-by: Johan Jonker <jbx6244@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
<div></div><div><br></div><div>.........</div><div><br></div>>   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)
<div>>+                return -EPROBE_DEFER;</div><div><br></div><div>Move  EPROBE_DEFER early still does not avoid the issue I mentioned in V7.</div><div><br></div><div>For the component based driver mode in drm，the probe of last subcomponent（component_add（）） will</div><div>trigger the bind callback of all subcomponent， any   -EPROBE_DEFER  return in the bind callback will lead</div><div>the  EPROBE_DEFER of the last subcomponent .probe.</div><div>For one example on rk3066:</div><div>LCDC0-->HDMI</div><div>LCDC1-->RGB-->panel</div><div>lcdc0(vop_probe)--->hdmi_bind-->audio register-->hdmi_codec_probe success-->lcdc1(vop_bind)--></div><div>rockchip_rgb_init(defer by somehow,maybe panel register failed)--> lcdc0(vop_probe) return   EPROBE_DEFER.</div><div><br></div><div>As there is on success probe(hdmi_codec_probe) during the process, this will trigger a new deferred probe, see:</div><div>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;
}</div><div><br></div><div>So the potentia of infinite loop porbe rise。</div><div><br></div><div>I think one possible solutiion is register the extra dirvers（hdmi audio，cec，hdcp）at the</div><div>encoder->funcs->late_register hook。</div><div>the late_register is called at the very last of drm_dev_register,  all the subcompent must</div><div>bind success if we finally run to this step.</div><div><br></div><div><br></div><div><br></div>>+
>    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
>
</pre></div>