From 71d873cc99309ff68d2488d32094774c77537e50 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Mon, 2 Dec 2019 12:15:51 +0100 Subject: [PATCH] drm/ast: Move modesetting code to CRTC's atomic_flush() When enabling the CRTC after waking up from a power-saving mode, the primary plane's framebuffer might be NULL, which leads to a stack trace as shown below. [ 632.624608] BUG: kernel NULL pointer dereference, address: 0000000000000048 [ 632.624631] #PF: supervisor read access in kernel mode [ 632.624639] #PF: error_code(0x0000) - not-present page [ 632.624647] PGD 0 P4D 0 [ 632.624654] Oops: 0000 [#1] SMP PTI [ 632.624662] CPU: 0 PID: 2082 Comm: gnome-shell Tainted: G E 5.4.0-rc7-1-default+ #114 [ 632.624673] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN FIRE X2270 M2, BIOS 2.05 07/01/2010 [ 632.624689] RIP: 0010:ast_crtc_helper_atomic_enable+0x7d/0x680 [ast] [ 632.624698] Code: 48 8b 80 e0 02 00 00 4c 8b 60 10 31 c0 f3 48 ab 48 8b 83 78 04 00 00 4c 89 ef 48 8d 70 18 e8 9a e9 55 ce 48 8b 83 78 04 00 00 <49> 8b 7c 24 48 4c 89 ea 4c 8d 44 24 28 48 8d 4c 24 20 48 8d 70 18 [ 632.624718] RSP: 0018:ffffbe9ec123fa40 EFLAGS: 00010246 [ 632.624726] RAX: ffff95a13cfd3400 RBX: ffff95a13cf32000 RCX: 0000000000000000 [ 632.624735] RDX: 0000000000000000 RSI: ffff95a13cfd34e8 RDI: ffffbe9ec123fb40 [ 632.624744] RBP: ffffbe9ec123fb80 R08: 0000000000000000 R09: 0000000000000003 [ 632.624753] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 [ 632.624762] R13: ffffbe9ec123fa70 R14: ffff95a13beb7000 R15: ffff95a13cf32800 [ 632.624772] FS: 00007f6d2763e140(0000) GS:ffff95a134000000(0000) knlGS:0000000000000000 [ 632.624782] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 632.624790] CR2: 0000000000000048 CR3: 00000001192f8004 CR4: 00000000000206f0 [ 632.624800] Call Trace: [ 632.624811] ? __lock_acquire+0x409/0x7c0 [ 632.624830] drm_atomic_helper_commit_modeset_enables+0x1af/0x200 [ 632.624840] drm_atomic_helper_commit_tail+0x32/0x70 [ 632.624849] commit_tail+0xc7/0x110 [ 632.624857] drm_atomic_helper_commit+0x121/0x130 [ 632.624867] drm_atomic_connector_commit_dpms+0xd7/0x100 [ 632.624878] set_property_atomic+0xaf/0x110 [ 632.624890] drm_mode_obj_set_property_ioctl+0xbb/0x190 [ 632.624899] ? drm_mode_obj_find_prop_id+0x40/0x40 [ 632.624909] drm_ioctl_kernel+0x86/0xd0 [ 632.624918] drm_ioctl+0x1e4/0x36b [ 632.624925] ? drm_mode_obj_find_prop_id+0x40/0x40 [ 632.624939] do_vfs_ioctl+0x4bd/0x6e0 [ 632.624949] ksys_ioctl+0x5e/0x90 [ 632.624957] __x64_sys_ioctl+0x16/0x20 [ 632.624966] do_syscall_64+0x5a/0x220 [ 632.624976] entry_SYSCALL_64_after_hwframe+0x49/0xbe [ 632.624984] RIP: 0033:0x7f6d2b0de387 [ 632.624991] Code: 00 00 90 48 8b 05 f9 9a 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c9 9a 0c 00 f7 d8 64 89 01 48 [ 632.625011] RSP: 002b:00007fffb49def38 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [ 632.625021] RAX: ffffffffffffffda RBX: 00007fffb49def70 RCX: 00007f6d2b0de387 [ 632.625030] RDX: 00007fffb49def70 RSI: 00000000c01864ba RDI: 0000000000000009 [ 632.625040] RBP: 00000000c01864ba R08: 0000000000000000 R09: 00000000c0c0c0c0 [ 632.625049] R10: 0000000000000030 R11: 0000000000000246 R12: 000055bc367eb920 [ 632.625058] R13: 0000000000000009 R14: 0000000000000002 R15: 0000000000000000 [ 632.625071] Modules linked in: ebtable_filter(E) ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E) ip_tables(E) x_tables(E) af_packet(E) scsi_transport_iscsi(E) dmi_sysfs(E) msr(E) xfs(E) intel_powerclamp(E) coretemp(E) k) [ 632.625185] CR2: 0000000000000048 The STR is * start gdm and wait for it to switch off the display * wake up the display by pressing a key CRTC modesetting depends on the new state of the CRTC and the primary plane's framebuffer. The bugfix moves the modesetting code into the CRTC's atomic_flush() function, where it is protected from the plane's framebuffer being NULL. The CRTC's atomic-enable function, which is the modesetting's original location, still contains DPMS state handling. It's exactly the inverse of the atomic-disable function. v3: * protect modesetting from from fb == NULL v2: * do an atomic check for plane * reject invisible primary planes Signed-off-by: Thomas Zimmermann Acked-by: Gerd Hoffmann Fixes: b48e1b6ffd28 ("drm/ast: Add CRTC helpers for atomic modesetting") Cc: Gerd Hoffmann Cc: Dave Airlie Cc: Daniel Vetter Cc: "Y.C. Chen" Cc: Sam Ravnborg Link: https://patchwork.freedesktop.org/patch/msgid/20191202111557.15176-2-tzimmermann@suse.de --- drivers/gpu/drm/ast/ast_mode.c | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 4725ec911a66..33aca817b686 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -790,6 +790,8 @@ static void ast_crtc_helper_atomic_begin(struct drm_crtc *crtc, static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state) { + struct drm_device *dev = crtc->dev; + struct ast_private *ast = dev->dev_private; const struct drm_framebuffer *fb = crtc->primary->state->fb; struct drm_display_mode adjusted_mode; struct ast_vbios_mode_info vbios_mode; @@ -800,36 +802,18 @@ static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, if (!fb) return; - ast_set_color_reg(crtc, fb); - memset(&adjusted_mode, 0, sizeof(adjusted_mode)); drm_mode_copy(&adjusted_mode, &crtc->state->adjusted_mode); succ = ast_get_vbios_mode_info(fb, &crtc->state->adjusted_mode, &adjusted_mode, &vbios_mode); if (WARN_ON_ONCE(!succ)) - return; + return; /* BUG: didn't validate this in atomic_check() */ + ast_set_color_reg(crtc, fb); ast_set_vbios_color_reg(crtc, fb, &vbios_mode); -} -static void -ast_crtc_helper_atomic_enable(struct drm_crtc *crtc, - struct drm_crtc_state *old_crtc_state) -{ - struct drm_device *dev = crtc->dev; - struct ast_private *ast = crtc->dev->dev_private; - const struct drm_framebuffer *fb = crtc->primary->state->fb; - struct drm_display_mode adjusted_mode; - struct ast_vbios_mode_info vbios_mode; - bool succ; - - memset(&adjusted_mode, 0, sizeof(adjusted_mode)); - drm_mode_copy(&adjusted_mode, &crtc->state->adjusted_mode); - - succ = ast_get_vbios_mode_info(fb, &crtc->state->adjusted_mode, - &adjusted_mode, &vbios_mode); - if (WARN_ON_ONCE(!succ)) + if (!crtc->state->mode_changed) return; ast_set_vbios_mode_reg(crtc, &adjusted_mode, &vbios_mode); @@ -840,7 +824,12 @@ ast_crtc_helper_atomic_enable(struct drm_crtc *crtc, ast_set_crtthd_reg(crtc); ast_set_sync_reg(dev, &adjusted_mode, &vbios_mode); ast_set_dac_reg(crtc, &adjusted_mode, &vbios_mode); +} +static void +ast_crtc_helper_atomic_enable(struct drm_crtc *crtc, + struct drm_crtc_state *old_crtc_state) +{ ast_crtc_dpms(crtc, DRM_MODE_DPMS_ON); } -- 2.30.2