On Mon, Oct 26, 2026 at 06:05:11PM +0800, Liu Ying wrote:
> CRTC(s) could still be running after the DRM device is unplugged by
> calling drm_dev_unplug(), because the CRTC disablement logic is
> protected and bypassed by the drm_dev_enter()/drm_dev_exit() pair.
> Hence, Pixel Engine's AXI clock use count(managed by Pixel Engine
> driver's runtime PM) and pixel clock use count could be inbalanced
> after removing and re-installing the driver module. To fix this,
> add a helper dc_crtc_disable_at_boot() and call it to properly
> disable all CRTCs before advertising DRM device to user-space by
> calling drm_dev_register().
>
> Fixes: 711a3b878366 ("drm/imx: Add i.MX8qxp Display Controller KMS")
> Reviewed-by: Frank Li <[email protected]>
> Signed-off-by: Liu Ying <[email protected]>
> --
> v7:
> - Check CRTC status to ensure that clocks were enabled by a previous boot
> before disabling them in dc_crtc_disable_at_boot(). (Sashiko AI bot)
> - Modify debug message in dc_crtc_disable_at_boot().
>
> v6:
> - Rebase onto the latest drm-misc-next and resolve conflicts.
>
> v3:
> - Collect Frank's R-b tag.
> ---
> drivers/gpu/drm/imx/dc/dc-crtc.c | 67
> ++++++++++++++++++++++++++++++++++------
> drivers/gpu/drm/imx/dc/dc-drv.c | 5 +++
> drivers/gpu/drm/imx/dc/dc-drv.h | 3 ++
> 3 files changed, 65 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/dc/dc-crtc.c
> b/drivers/gpu/drm/imx/dc/dc-crtc.c
> index 0d64186a9901..c7dbe5f500b4 100644
> --- a/drivers/gpu/drm/imx/dc/dc-crtc.c
> +++ b/drivers/gpu/drm/imx/dc/dc-crtc.c
> @@ -227,6 +227,7 @@ dc_crtc_atomic_enable(struct drm_crtc *crtc, struct
> drm_atomic_commit *state)
> struct drm_crtc_state *new_crtc_state =
> drm_atomic_get_new_crtc_state(state, crtc);
> struct drm_display_mode *adj = &new_crtc_state->adjusted_mode;
> + struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
> struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> enum dc_link_id cf_link;
> int idx, ret;
> @@ -287,29 +288,37 @@ dc_crtc_atomic_enable(struct drm_crtc *crtc, struct
> drm_atomic_commit *state)
>
> DC_CRTC_CHECK_FRAMEGEN_FIFO(dc_crtc->fg);
>
> + dc_drv_set_crtc_enable(dc_drm, drm_crtc_index(crtc));
> +
> drm_dev_exit(idx);
>
> out:
> dc_crtc_queue_state_event(new_crtc_state);
> }
>
> -static void
> -dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit
> *state)
> +static inline void __dc_crtc_disable_fg(struct drm_crtc *crtc)
> {
> - struct drm_crtc_state *new_crtc_state =
> - drm_atomic_get_new_crtc_state(state, crtc);
> - struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
> struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> - int idx;
> -
> - if (!drm_dev_enter(crtc->dev, &idx))
> - goto out;
>
> enable_irq(dc_crtc->irq_dec_seqcomplete);
> dc_fg_disable(dc_crtc->fg);
> DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(dec_seqcomplete_done);
> disable_irq(dc_crtc->irq_dec_seqcomplete);
> +}
>
> +static void
> +dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_commit
> *state)
> +{
> + struct drm_crtc_state *new_crtc_state =
> + drm_atomic_get_new_crtc_state(state, crtc);
> + struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
> + struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> + int idx;
> +
> + if (!drm_dev_enter(crtc->dev, &idx))
> + goto out;
> +
> + __dc_crtc_disable_fg(crtc);
> dc_fg_disable_clock(dc_crtc->fg);
>
> /* request pixel engine power-off as plane is off too */
> @@ -318,8 +327,9 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct
> drm_atomic_commit *state)
> /* request display engine power-off when CRTC is disabled */
> pm_runtime_put(dc_crtc->de->dev);
>
> - drm_dev_exit(idx);
> + dc_drv_set_crtc_disable(dc_drm, drm_crtc_index(crtc));
>
> + drm_dev_exit(idx);
> out:
> drm_crtc_vblank_off(crtc);
>
> @@ -331,6 +341,43 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct
> drm_atomic_commit *state)
> spin_unlock_irq(&crtc->dev->event_lock);
> }
>
> +void dc_crtc_disable_at_boot(struct drm_crtc *crtc)
Shouldn't this function return int in order to propagate a potential error to
dc_drm_bind()? For example, if pm_runtime_resume_and_get() fails, or frame
index isn't moving, wouldn't it be better for dc_drm_bind() to fail as well?
--
Thanks,
Laurentiu
