Re: [PATCH 3/3] drm/mediatek: mtk_crtc: Pick the connector route on every CRTC enable

[CK Hu (胡俊光)]

On Sat, 2026-10-10 at 20:42 -0400, zoan37 wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> mtk_crtc_update_output() only picks the output component of a CRTC
> with connector routes when connectors_changed is set in the commit that
> enables the CRTC. But connectors_changed only says that the connectors
> changed in that commit, not since the CRTC was last enabled. A connector
> can be attached to (or moved to) the CRTC while it is inactive, and the
> later commit that only sets ACTIVE then has active_changed but not
> connectors_changed:
> 
> - If the CRTC had never been enabled, the route slot is still NULL and
>   mtk_crtc_ddp_hw_init() dereferences it (mtk_ddp_comp_clk_enable()
>   and so on).
> - If the CRTC was driven through another route before, e.g. a CRTC
>   shared by an HDMI bridge on DSI and a DisplayPort output that is
>   switched off (DPMS) while the monitor moves from one to the other,
>   the pipeline is set up for the old output.
> 
> update_output() only runs from atomic_enable and the lookup is a short
> loop over the routes, so do it on every enable.
> 
> Fixes: 01389b324c97 ("drm/mediatek: Add connector dynamic selection capability")
> Assisted-by: LLM
> Signed-off-by: zoan37 <agentzoan@xxxxxxxxx>
> ---
> 
> Notes:
>     Testing: found by reading the code. The same change runs on the MT8189
>     Chromebook described in patch 1, where one routed CRTC drives either
>     HDMI or USB-C DisplayPort: DPMS off/on and suspend/resume with a DP
>     monitor attached come back. The sequence that fails without it
>     (connector attached or swapped while the CRTC is inactive, then only
>     ACTIVE set) was not reproduced on purpose. Compile-tested (W=1) on
>     next-20261008.
> 
>  drivers/gpu/drm/mediatek/mtk_crtc.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
> index 704931b369b6..0bfd17f68282 100644
> --- a/drivers/gpu/drm/mediatek/mtk_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
> @@ -695,9 +695,6 @@ static void mtk_crtc_update_output(struct drm_crtc *crtc,
>         struct mtk_drm_private *priv;
>         unsigned int encoder_mask = crtc_state->encoder_mask;
> 
> -       if (!crtc_state->connectors_changed)
> -               return;
> -

Agree this modification, but below deb_dbg() about connectors_changed should also removed.

Regards,
CK

>         if (!mtk_crtc->num_conn_routes)
>                 return;
> 
> --
> 2.43.0
>