Hi Maxime

On Tue, 25 May 2021 at 10:11, Maxime Ripard maxime@cerno.tech wrote:

In order to access the HDMI controller, we need to make sure the HSM clock is enabled. If we were to access it with the clock disabled, the CPU would completely hang, resulting in an hard crash.

Since we have different code path that would require it, let's move that clock enable / disable to runtime_pm that will take care of the reference counting for us.

This does change the order of clk_set_rate vs clk_prepare_enable, so the clock is already running during vc4_hdmi_encoder_pre_crtc_configure when the pixel rate is known. However the crtc and HDMI blocks won't be actively passing pixels at that point, so I don't see an issue with changing the rate. The clock manager block will sort out the rate change happily.

Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com

Fixes: 4f6e3d66ac52 ("drm/vc4: Add runtime PM support to the HDMI encoder driver") Signed-off-by: Maxime Ripard maxime@cerno.tech

---

drivers/gpu/drm/vc4/vc4_hdmi.c | 41 +++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index f9de8632a28b..867009a471e1 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -632,7 +632,6 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder, HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_ENABLE);

    clk_disable_unprepare(vc4_hdmi->pixel_bvb_clock);

•   clk_disable_unprepare(vc4_hdmi->hsm_clock);
    clk_disable_unprepare(vc4_hdmi->pixel_clock);
  
    ret = pm_runtime_put(&vc4_hdmi->pdev->dev);
  

@@ -943,13 +942,6 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, return; }

•   ret = clk_prepare_enable(vc4_hdmi->hsm_clock);
  

•   if (ret) {
  

•           DRM_ERROR("Failed to turn on HSM clock: %d\n", ret);
  

•           clk_disable_unprepare(vc4_hdmi->pixel_clock);
  

•           return;
  

•   }
  

•   vc4_hdmi_cec_update_clk_div(vc4_hdmi);
  
    if (pixel_rate > 297000000)
  

@@ -962,7 +954,6 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, ret = clk_set_min_rate(vc4_hdmi->pixel_bvb_clock, bvb_rate); if (ret) { DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret);

•           clk_disable_unprepare(vc4_hdmi->hsm_clock);
            clk_disable_unprepare(vc4_hdmi->pixel_clock);
            return;
    }
  

@@ -970,7 +961,6 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, ret = clk_prepare_enable(vc4_hdmi->pixel_bvb_clock); if (ret) { DRM_ERROR("Failed to turn on pixel bvb clock: %d\n", ret);

•           clk_disable_unprepare(vc4_hdmi->hsm_clock);
            clk_disable_unprepare(vc4_hdmi->pixel_clock);
            return;
    }
  

@@ -2097,6 +2087,29 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi) return 0; }

+#ifdef CONFIG_PM +static int vc4_hdmi_runtime_suspend(struct device *dev) +{

•   struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
  

•   clk_disable_unprepare(vc4_hdmi->hsm_clock);
  

•   return 0;
  

+}

+static int vc4_hdmi_runtime_resume(struct device *dev) +{

•   struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
  

•   int ret;
  

•   ret = clk_prepare_enable(vc4_hdmi->hsm_clock);
  

•   if (ret)
  

•           return ret;
  

•   return 0;
  

+} +#endif

static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) { const struct vc4_hdmi_variant *variant = of_device_get_match_data(dev); @@ -2353,11 +2366,19 @@ static const struct of_device_id vc4_hdmi_dt_match[] = { {} };

+static const struct dev_pm_ops vc4_hdmi_pm_ops = {

•   SET_RUNTIME_PM_OPS(vc4_hdmi_runtime_suspend,
  

•                      vc4_hdmi_runtime_resume,
  

•                      NULL)
  

+};

struct platform_driver vc4_hdmi_driver = { .probe = vc4_hdmi_dev_probe, .remove = vc4_hdmi_dev_remove, .driver = { .name = "vc4_hdmi", .of_match_table = vc4_hdmi_dt_match,

•           .pm = &vc4_hdmi_pm_ops,
    },
  

};

2.31.1

On Tue, 25 May 2021 at 10:11, Maxime Ripard maxime@cerno.tech wrote:

If the HPD GPIO is not available and drm_probe_ddc fails, we end up reading the HDMI_HOTPLUG register, but the controller might be powered off resulting in a CPU hang. Make sure we have the power domain and the HSM clock powered during the detect cycle to prevent the hang from happening.

Fixes: 4f6e3d66ac52 ("drm/vc4: Add runtime PM support to the HDMI encoder driver") Signed-off-by: Maxime Ripard maxime@cerno.tech

Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com

---

drivers/gpu/drm/vc4/vc4_hdmi.c | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 867009a471e1..4b6857467e58 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -166,6 +166,8 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector); bool connected = false;

•   WARN_ON(pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev));
  

•   if (vc4_hdmi->hpd_gpio) {
            if (gpio_get_value_cansleep(vc4_hdmi->hpd_gpio) ^
                vc4_hdmi->hpd_active_low)
  

@@ -187,10 +189,12 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) } }

•           pm_runtime_put(&vc4_hdmi->pdev->dev);
            return connector_status_connected;
    }
  
    cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
  

•   pm_runtime_put(&vc4_hdmi->pdev->dev);
    return connector_status_disconnected;
  

}

-- 2.31.1