• Kuogee Hsieh's avatar
    drm/msm/dp: add atomic_check to bridge ops · 3a661247
    Kuogee Hsieh authored
    DRM commit_tails() will disable downstream crtc/encoder/bridge if
    both disable crtc is required and crtc->active is set before pushing
    a new frame downstream.
    
    There is a rare case that user space display manager issue an extra
    screen update immediately followed by close DRM device while down
    stream display interface is disabled. This extra screen update will
    timeout due to the downstream interface is disabled but will cause
    crtc->active be set. Hence the followed commit_tails() called by
    drm_release() will pass the disable downstream crtc/encoder/bridge
    conditions checking even downstream interface is disabled.
    This cause the crash to happen at dp_bridge_disable() due to it trying
    to access the main link register to push the idle pattern out while main
    link clocks is disabled.
    
    This patch adds atomic_check to prevent the extra frame will not
    be pushed down if display interface is down so that crtc->active
    will not be set neither. This will fail the conditions checking
    of disabling down stream crtc/encoder/bridge which prevent
    drm_release() from calling dp_bridge_disable() so that crash
    at dp_bridge_disable() prevented.
    
    There is no protection in the DRM framework to check if the display
    pipeline has been already disabled before trying again. The only
    check is the crtc_state->active but this is controlled by usermode
    using UAPI. Hence if the usermode sets this and then crashes, the
    driver needs to protect against double disable.
    
    SError Interrupt on CPU7, code 0x00000000be000411 -- SError
    CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19
    Hardware name: Google Lazor (rev3 - 8) (DT)
    pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
    pc : __cmpxchg_case_acq_32+0x14/0x2c
    lr : do_raw_spin_lock+0xa4/0xdc
    sp : ffffffc01092b6a0
    x29: ffffffc01092b6a0 x28: 0000000000000028 x27: 0000000000000038
    x26: 0000000000000004 x25: ffffffd2973dce48 x24: 0000000000000000
    x23: 00000000ffffffff x22: 00000000ffffffff x21: ffffffd2978d0008
    x20: ffffffd2978d0008 x19: ffffff80ff759fc0 x18: 0000000000000000
    x17: 004800a501260460 x16: 0441043b04600438 x15: 04380000089807d0
    x14: 07b0089807800780 x13: 0000000000000000 x12: 0000000000000000
    x11: 0000000000000438 x10: 00000000000007d0 x9 : ffffffd2973e09e4
    x8 : ffffff8092d53300 x7 : ffffff808902e8b8 x6 : 0000000000000001
    x5 : ffffff808902e880 x4 : 0000000000000000 x3 : ffffff80ff759fc0
    x2 : 0000000000000001 x1 : 0000000000000000 x0 : ffffff80ff759fc0
    Kernel panic - not syncing: Asynchronous SError Interrupt
    CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19
    Hardware name: Google Lazor (rev3 - 8) (DT)
    Call trace:
     dump_backtrace.part.0+0xbc/0xe4
     show_stack+0x24/0x70
     dump_stack_lvl+0x68/0x84
     dump_stack+0x18/0x34
     panic+0x14c/0x32c
     nmi_panic+0x58/0x7c
     arm64_serror_panic+0x78/0x84
     do_serror+0x40/0x64
     el1h_64_error_handler+0x30/0x48
     el1h_64_error+0x68/0x6c
     __cmpxchg_case_acq_32+0x14/0x2c
     _raw_spin_lock_irqsave+0x38/0x4c
     lock_timer_base+0x40/0x78
     __mod_timer+0xf4/0x25c
     schedule_timeout+0xd4/0xfc
     __wait_for_common+0xac/0x140
     wait_for_completion_timeout+0x2c/0x54
     dp_ctrl_push_idle+0x40/0x88
     dp_bridge_disable+0x24/0x30
     drm_atomic_bridge_chain_disable+0x90/0xbc
     drm_atomic_helper_commit_modeset_disables+0x198/0x444
     msm_atomic_commit_tail+0x1d0/0x374
     commit_tail+0x80/0x108
     drm_atomic_helper_commit+0x118/0x11c
     drm_atomic_commit+0xb4/0xe0
     drm_client_modeset_commit_atomic+0x184/0x224
     drm_client_modeset_commit_locked+0x58/0x160
     drm_client_modeset_commit+0x3c/0x64
     __drm_fb_helper_restore_fbdev_mode_unlocked+0x98/0xac
     drm_fb_helper_set_par+0x74/0x80
     drm_fb_helper_hotplug_event+0xdc/0xe0
     __drm_fb_helper_restore_fbdev_mode_unlocked+0x7c/0xac
     drm_fb_helper_restore_fbdev_mode_unlocked+0x20/0x2c
     drm_fb_helper_lastclose+0x20/0x2c
     drm_lastclose+0x44/0x6c
     drm_release+0x88/0xd4
     __fput+0x104/0x220
     ____fput+0x1c/0x28
     task_work_run+0x8c/0x100
     do_exit+0x450/0x8d0
     do_group_exit+0x40/0xac
     __wake_up_parent+0x0/0x38
     invoke_syscall+0x84/0x11c
     el0_svc_common.constprop.0+0xb8/0xe4
     do_el0_svc+0x8c/0xb8
     el0_svc+0x2c/0x54
     el0t_64_sync_handler+0x120/0x1c0
     el0t_64_sync+0x190/0x194
    SMP: stopping secondary CPUs
    Kernel Offset: 0x128e800000 from 0xffffffc008000000
    PHYS_OFFSET: 0x80000000
    CPU features: 0x800,00c2a015,19801c82
    Memory Limit: none
    
    Changes in v2:
    -- add more commit text
    
    Changes in v3:
    -- add comments into dp_bridge_atomic_check()
    
    Changes in v4:
    -- rewording the comment into dp_bridge_atomic_check()
    
    Changes in v5:
    -- removed quote x at end of commit text
    
    Changes in v6:
    -- removed quote x at end of comment in dp_bridge_atomic_check()
    
    Fixes: 8a3b4c17 ("drm/msm/dp: employ bridge mechanism for display enable and disable")
    Reported-by: default avatarLeonard Lausen <leonard@lausen.nl>
    Suggested-by: default avatarRob Clark <robdclark@gmail.com>
    Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/17Signed-off-by: default avatarKuogee Hsieh <quic_khsieh@quicinc.com>
    Reviewed-by: default avatarAbhinav Kumar <quic_abhinavk@quicinc.com>
    Patchwork: https://patchwork.freedesktop.org/patch/505331/
    Link: https://lore.kernel.org/r/1664408211-25314-1-git-send-email-quic_khsieh@quicinc.comSigned-off-by: default avatarAbhinav Kumar <quic_abhinavk@quicinc.com>
    3a661247
dp_drm.c 4.94 KB