On Tue, Dec 2, 2025 at 10:03 PM Linus Walleij <[email protected]> wrote:
>
> commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
> "drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
> caused a series of regressions in all panels that send
> DSI commands in their .prepare() and .unprepare()
> callbacks when used with the Rockchip driver.
>
> As the CRTC is no longer online at bridge_pre_enable()
> and gone at brige_post_disable() which maps to the panel
> bridge .prepare()/.unprepare() callbacks, any CRTC that
> enable/disable the DSI transmitter in it's enable/disable
> callbacks will be unable to send any DSI commands in the
> .prepare() and .unprepare() callbacks.
>
> However the Rockchip driver definitely need the CRTC to be
> enabled during .prepare()/.unprepare().
>
> Solve this by implementing a custom commit tail function
> in the Rockchip driver that always enables the CRTC first
> and disables it last, using the newly exported helpers.
>
> This patch is an edited carbon-copy of the same patch to
> the ST-Ericsson MCDE driver.
>
> Link: 
> https://lore.kernel.org/all/caamcf8di8sc_xvzanzq9suiuf-ayvg2yjhx2dwmvvcnff3p...@mail.gmail.com/
> Reported-by: Aradhya Bhatia <[email protected]>
> Reported-by: Vicente Bergas <[email protected]>
> Signed-off-by: Linus Walleij <[email protected]>
> ---
> Rockchip people: can you please test this patch (along
> with patch 1 of course).

Hi Linus,
i've applied all 4 patches from the V6 patch series on top of v6.18
and tested on the rk3399-gru-kevin platform.
It indeed fixes the reported issue.

Tested-by: Vicente Bergas <[email protected]>

Regards,
  Vicente.

> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 50 
> +++++++++++++++++++++++++++++-
>  1 file changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index 2f469d370021..63e50ea00920 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -24,8 +24,56 @@ static const struct drm_framebuffer_funcs 
> rockchip_drm_fb_funcs = {
>         .dirty         = drm_atomic_helper_dirtyfb,
>  };
>
> +/*
> + * This commit tail explicitly copies and changes the behaviour of
> + * the related core DRM atomic helper instead of trying to make
> + * the core helpers overly generic.
> + */
> +static void rockchip_drm_atomic_commit_tail(struct drm_atomic_state *state)
> +{
> +       struct drm_device *dev = state->dev;
> +
> +       /*
> +        * Variant of drm_atomic_helper_commit_modeset_disables()
> +        * that will disable and post-disable all bridges BEFORE
> +        * disabling the CRTC.
> +        */
> +       drm_atomic_helper_commit_encoder_bridge_disable(dev, state);
> +       drm_atomic_helper_commit_encoder_bridge_post_disable(dev, state);
> +       drm_atomic_helper_commit_crtc_disable(dev, state);
> +       drm_atomic_helper_update_legacy_modeset_state(dev, state);
> +       drm_atomic_helper_calc_timestamping_constants(state);
> +       drm_atomic_helper_commit_crtc_set_mode(dev, state);
> +
> +       /*
> +        * Variant of drm_atomic_helper_commit_modeset_enables()
> +        * that will enable the CRTC BEFORE pre-enabling and
> +        * enabling the bridges.
> +        */
> +       drm_atomic_helper_commit_crtc_enable(dev, state);
> +       drm_atomic_helper_commit_encoder_bridge_pre_enable(dev, state);
> +       drm_atomic_helper_commit_encoder_bridge_enable(dev, state);
> +       drm_atomic_helper_commit_writebacks(dev, state);
> +
> +       drm_atomic_helper_commit_planes(dev, state,
> +                                       DRM_PLANE_COMMIT_ACTIVE_ONLY);
> +
> +       drm_atomic_helper_fake_vblank(state);
> +
> +       drm_atomic_helper_commit_hw_done(state);
> +
> +       drm_atomic_helper_wait_for_vblanks(dev, state);
> +
> +       drm_atomic_helper_cleanup_planes(dev, state);
> +}
> +
>  static const struct drm_mode_config_helper_funcs 
> rockchip_mode_config_helpers = {
> -       .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
> +       /*
> +        * Using this function is necessary to commit atomic updates
> +        * that need the CRTC to be enabled before a commit, as is
> +        * the case with e.g. DSI displays.
> +        */
> +       .atomic_commit_tail = rockchip_drm_atomic_commit_tail,
>  };
>
>  static struct drm_framebuffer *
>
> --
> 2.51.1
>

Reply via email to