Hi,
On 18/06/2025 15:06, Marek Szyprowski wrote:
> Commit c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain pre-enable
> and post-disable") changed the call sequence to the CRTC enable/disable
> and bridge pre_enable/post_disable methods, so those bridge methods are
> now called when CRTC is not yet enabled.
>
> This causes a lockup observed on Samsung Peach-Pit/Pi Chromebooks. The
> source of this lockup is a call to fimd_dp_clock_enable() function, when
> FIMD device is not yet runtime resumed. It worked before the mentioned
> commit only because the CRTC implemented by the FIMD driver was always
> enabled what guaranteed the FIMD device to be runtime resumed.
>
> This patch adds runtime PM guards to the fimd_dp_clock_enable() function
> to enable its proper operation also when the CRTC implemented by FIMD is
> not yet enabled.
>
> Fixes: 196e059a8a6a ("drm/exynos: convert clock_enable crtc callback to
> pipeline clock")
> Signed-off-by: Marek Szyprowski <m.szyprow...@samsung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index c394cc702d7d..205c238cc73a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -187,6 +187,7 @@ struct fimd_context {
> u32 i80ifcon;
> bool i80_if;
> bool suspended;
> + bool dp_clk_enabled;
> wait_queue_head_t wait_vsync_queue;
> atomic_t wait_vsync_event;
> atomic_t win_updated;
> @@ -1047,7 +1048,18 @@ static void fimd_dp_clock_enable(struct exynos_drm_clk
> *clk, bool enable)
> struct fimd_context *ctx = container_of(clk, struct fimd_context,
> dp_clk);
> u32 val = enable ? DP_MIE_CLK_DP_ENABLE : DP_MIE_CLK_DISABLE;
> +
> + if (enable == ctx->dp_clk_enabled)
> + return;
Does this happen, i.e. is this function called multiple times with
enable set? If so, do you rather need ref counting here? Otherwise the
first fimd_dp_clock_enable(enable=false) call with disable the clock,
instead of the last (assuming the enable/disable calls are matched on
the caller side).
> +
> + if (enable)
> + pm_runtime_resume_and_get(ctx->dev);
> +
> + ctx->dp_clk_enabled = enable;
> writel(val, ctx->regs + DP_MIE_CLKCON);
> +
> + if (!enable)
> + pm_runtime_put(ctx->dev);
> }
>
> static const struct exynos_drm_crtc_ops fimd_crtc_ops = {
Tomi
