Hi Maxime
On Tue, 25 May 2021 at 10:11, Maxime Ripard <maxime@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> Fixes: 4f6e3d66ac52 ("drm/vc4: Add runtime PM support to the HDMI encoder driver")
> Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
> ---
> 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
>
