| Message ID | 20191127073109.9807-1-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/ast: Fix potential NULL-pointer read during atomic mode setting | expand |
On Wed, Nov 27, 2019 at 08:31:09AM +0100, Thomas Zimmermann wrote: > 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 > > The fix implements atomic_check for planes and rejects configurations > with an invisible primary plane. > > v2: > * do an atomic check for plane > * reject invisible primary planes > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Fixes: b48e1b6ffd28 ("drm/ast: Add CRTC helpers for atomic modesetting") > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Dave Airlie <airlied@redhat.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: "Y.C. Chen" <yc_chen@aspeedtech.com> > Cc: Sam Ravnborg <sam@ravnborg.org> > --- > drivers/gpu/drm/ast/ast_mode.c | 50 +++++++++++++++++++++++++++++++++- > 1 file changed, 49 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c > index 4725ec911a66..8e7bb8ce8130 100644 > --- a/drivers/gpu/drm/ast/ast_mode.c > +++ b/drivers/gpu/drm/ast/ast_mode.c > @@ -31,6 +31,7 @@ > #include <linux/export.h> > #include <linux/pci.h> > > +#include <drm/drm_atomic.h> > #include <drm/drm_atomic_helper.h> > #include <drm/drm_atomic_state_helper.h> > #include <drm/drm_crtc.h> > @@ -556,6 +557,31 @@ static const uint32_t ast_primary_plane_formats[] = { > int ast_primary_plane_helper_atomic_check(struct drm_plane *plane, > struct drm_plane_state *state) > { > + struct drm_crtc *crtc; > + struct drm_crtc_state *crtc_state; > + int ret; > + > + if (state->crtc) > + crtc = state->crtc; > + else if (plane->state && plane->state->crtc) > + crtc = plane->state->crtc; > + else > + return 0; /* disabling an already disabled plane */ > + > + crtc_state = drm_atomic_get_new_crtc_state(state->state, crtc); > + if (WARN_ON_ONCE(!crtc_state)) > + return -EINVAL; /* BUG: no new CRTC state allocated */ The above is a lot more complicated than necessary, see e.g. atmel_hlcdc_plane_atomic_check. And even that is too complicated, since crtc is set iff fb is set, you only need to check one of them. > + > + ret = drm_atomic_helper_check_plane_state(state, crtc_state, > + DRM_PLANE_HELPER_NO_SCALING, > + DRM_PLANE_HELPER_NO_SCALING, > + false, true); > + if (ret) > + return ret; > + > + if (!state->visible) > + return -EINVAL; /* primary plane must be visible */ > + > return 0; > } > > @@ -567,7 +593,7 @@ void ast_primary_plane_helper_atomic_update(struct drm_plane *plane, > struct drm_gem_vram_object *gbo; > s64 gpu_addr; > > - if (!crtc || !state->fb) > + if (!crtc || !state->fb || !state->visible) > return; > > gbo = drm_gem_vram_of_gem(state->fb->obj[0]); > @@ -660,6 +686,28 @@ ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane, > static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane, > struct drm_plane_state *state) > { > + struct drm_crtc *crtc; > + struct drm_crtc_state *crtc_state; > + int ret; > + > + if (state->crtc) > + crtc = state->crtc; > + else if (plane->state && plane->state->crtc) > + crtc = plane->state->crtc; > + else > + return 0; /* disabling an already disabled plane */ > + > + crtc_state = drm_atomic_get_new_crtc_state(state->state, crtc); > + if (WARN_ON_ONCE(!crtc_state)) > + return -EINVAL; /* BUG: no new CRTC state allocated */ > + > + ret = drm_atomic_helper_check_plane_state(state, crtc_state, > + DRM_PLANE_HELPER_NO_SCALING, > + DRM_PLANE_HELPER_NO_SCALING, > + false, true); Pretty sure you want your cursor to be positionable ... -Daniel > + if (ret) > + return ret; > + > return 0; > } > > -- > 2.23.0 >
Hi Daniel Am 27.11.19 um 11:49 schrieb Daniel Vetter: > On Wed, Nov 27, 2019 at 08:31:09AM +0100, Thomas Zimmermann wrote: >> 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 >> >> The fix implements atomic_check for planes and rejects configurations >> with an invisible primary plane. >> >> v2: >> * do an atomic check for plane >> * reject invisible primary planes >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> Fixes: b48e1b6ffd28 ("drm/ast: Add CRTC helpers for atomic modesetting") >> Cc: Gerd Hoffmann <kraxel@redhat.com> >> Cc: Dave Airlie <airlied@redhat.com> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> Cc: "Y.C. Chen" <yc_chen@aspeedtech.com> >> Cc: Sam Ravnborg <sam@ravnborg.org> >> --- >> drivers/gpu/drm/ast/ast_mode.c | 50 +++++++++++++++++++++++++++++++++- >> 1 file changed, 49 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c >> index 4725ec911a66..8e7bb8ce8130 100644 >> --- a/drivers/gpu/drm/ast/ast_mode.c >> +++ b/drivers/gpu/drm/ast/ast_mode.c >> @@ -31,6 +31,7 @@ >> #include <linux/export.h> >> #include <linux/pci.h> >> >> +#include <drm/drm_atomic.h> >> #include <drm/drm_atomic_helper.h> >> #include <drm/drm_atomic_state_helper.h> >> #include <drm/drm_crtc.h> >> @@ -556,6 +557,31 @@ static const uint32_t ast_primary_plane_formats[] = { >> int ast_primary_plane_helper_atomic_check(struct drm_plane *plane, >> struct drm_plane_state *state) >> { >> + struct drm_crtc *crtc; >> + struct drm_crtc_state *crtc_state; >> + int ret; >> + >> + if (state->crtc) >> + crtc = state->crtc; >> + else if (plane->state && plane->state->crtc) >> + crtc = plane->state->crtc; >> + else >> + return 0; /* disabling an already disabled plane */ >> + >> + crtc_state = drm_atomic_get_new_crtc_state(state->state, crtc); >> + if (WARN_ON_ONCE(!crtc_state)) >> + return -EINVAL; /* BUG: no new CRTC state allocated */ > > The above is a lot more complicated than necessary, see e.g. > atmel_hlcdc_plane_atomic_check. And even that is too complicated, since > crtc is set iff fb is set, you only need to check one of them. Thanks for your review. This comment sounds as if it's at the wrong position? Best regards Thomas > >> + >> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, >> + DRM_PLANE_HELPER_NO_SCALING, >> + DRM_PLANE_HELPER_NO_SCALING, >> + false, true); >> + if (ret) >> + return ret; >> + >> + if (!state->visible) >> + return -EINVAL; /* primary plane must be visible */ >> + >> return 0; >> } >> >> @@ -567,7 +593,7 @@ void ast_primary_plane_helper_atomic_update(struct drm_plane *plane, >> struct drm_gem_vram_object *gbo; >> s64 gpu_addr; >> >> - if (!crtc || !state->fb) >> + if (!crtc || !state->fb || !state->visible) >> return; >> >> gbo = drm_gem_vram_of_gem(state->fb->obj[0]); >> @@ -660,6 +686,28 @@ ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane, >> static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane, >> struct drm_plane_state *state) >> { >> + struct drm_crtc *crtc; >> + struct drm_crtc_state *crtc_state; >> + int ret; >> + >> + if (state->crtc) >> + crtc = state->crtc; >> + else if (plane->state && plane->state->crtc) >> + crtc = plane->state->crtc; >> + else >> + return 0; /* disabling an already disabled plane */ >> + >> + crtc_state = drm_atomic_get_new_crtc_state(state->state, crtc); >> + if (WARN_ON_ONCE(!crtc_state)) >> + return -EINVAL; /* BUG: no new CRTC state allocated */ >> + >> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, >> + DRM_PLANE_HELPER_NO_SCALING, >> + DRM_PLANE_HELPER_NO_SCALING, >> + false, true); > > Pretty sure you want your cursor to be positionable ... > -Daniel > >> + if (ret) >> + return ret; >> + >> return 0; >> } >> >> -- >> 2.23.0 >> >
On Wed, Nov 27, 2019 at 01:31:25PM +0100, Thomas Zimmermann wrote: > Hi Daniel > > Am 27.11.19 um 11:49 schrieb Daniel Vetter: > > On Wed, Nov 27, 2019 at 08:31:09AM +0100, Thomas Zimmermann wrote: > >> 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 > >> > >> The fix implements atomic_check for planes and rejects configurations > >> with an invisible primary plane. > >> > >> v2: > >> * do an atomic check for plane > >> * reject invisible primary planes > >> > >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > >> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> > >> Fixes: b48e1b6ffd28 ("drm/ast: Add CRTC helpers for atomic modesetting") > >> Cc: Gerd Hoffmann <kraxel@redhat.com> > >> Cc: Dave Airlie <airlied@redhat.com> > >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >> Cc: "Y.C. Chen" <yc_chen@aspeedtech.com> > >> Cc: Sam Ravnborg <sam@ravnborg.org> > >> --- > >> drivers/gpu/drm/ast/ast_mode.c | 50 +++++++++++++++++++++++++++++++++- > >> 1 file changed, 49 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c > >> index 4725ec911a66..8e7bb8ce8130 100644 > >> --- a/drivers/gpu/drm/ast/ast_mode.c > >> +++ b/drivers/gpu/drm/ast/ast_mode.c > >> @@ -31,6 +31,7 @@ > >> #include <linux/export.h> > >> #include <linux/pci.h> > >> > >> +#include <drm/drm_atomic.h> > >> #include <drm/drm_atomic_helper.h> > >> #include <drm/drm_atomic_state_helper.h> > >> #include <drm/drm_crtc.h> > >> @@ -556,6 +557,31 @@ static const uint32_t ast_primary_plane_formats[] = { > >> int ast_primary_plane_helper_atomic_check(struct drm_plane *plane, > >> struct drm_plane_state *state) > >> { > >> + struct drm_crtc *crtc; > >> + struct drm_crtc_state *crtc_state; > >> + int ret; > >> + > >> + if (state->crtc) > >> + crtc = state->crtc; > >> + else if (plane->state && plane->state->crtc) > >> + crtc = plane->state->crtc; > >> + else > >> + return 0; /* disabling an already disabled plane */ > >> + > >> + crtc_state = drm_atomic_get_new_crtc_state(state->state, crtc); > >> + if (WARN_ON_ONCE(!crtc_state)) > >> + return -EINVAL; /* BUG: no new CRTC state allocated */ > > > > The above is a lot more complicated than necessary, see e.g. > > atmel_hlcdc_plane_atomic_check. And even that is too complicated, since > > crtc is set iff fb is set, you only need to check one of them. > > Thanks for your review. This comment sounds as if it's at the wrong > position? Nope. Instead of the stuff above you have do this: if (!state->crtc) return 0; crtc_state = drm_atomic_get_existing_crtc_state(s->state, s->crtc); mode = &crtc_state->adjusted_mode; which is what atmel has, except the redundant fb check dropped. Same for the cursor version below too. -Daniel > > Best regards > Thomas > > > > >> + > >> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, > >> + DRM_PLANE_HELPER_NO_SCALING, > >> + DRM_PLANE_HELPER_NO_SCALING, > >> + false, true); > >> + if (ret) > >> + return ret; > >> + > >> + if (!state->visible) > >> + return -EINVAL; /* primary plane must be visible */ > >> + > >> return 0; > >> } > >> > >> @@ -567,7 +593,7 @@ void ast_primary_plane_helper_atomic_update(struct drm_plane *plane, > >> struct drm_gem_vram_object *gbo; > >> s64 gpu_addr; > >> > >> - if (!crtc || !state->fb) > >> + if (!crtc || !state->fb || !state->visible) > >> return; > >> > >> gbo = drm_gem_vram_of_gem(state->fb->obj[0]); > >> @@ -660,6 +686,28 @@ ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane, > >> static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane, > >> struct drm_plane_state *state) > >> { > >> + struct drm_crtc *crtc; > >> + struct drm_crtc_state *crtc_state; > >> + int ret; > >> + > >> + if (state->crtc) > >> + crtc = state->crtc; > >> + else if (plane->state && plane->state->crtc) > >> + crtc = plane->state->crtc; > >> + else > >> + return 0; /* disabling an already disabled plane */ > >> + > >> + crtc_state = drm_atomic_get_new_crtc_state(state->state, crtc); > >> + if (WARN_ON_ONCE(!crtc_state)) > >> + return -EINVAL; /* BUG: no new CRTC state allocated */ > >> + > >> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, > >> + DRM_PLANE_HELPER_NO_SCALING, > >> + DRM_PLANE_HELPER_NO_SCALING, > >> + false, true); > > > > Pretty sure you want your cursor to be positionable ... > > -Daniel > > > >> + if (ret) > >> + return ret; > >> + > >> return 0; > >> } > >> > >> -- > >> 2.23.0 > >> > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer >
Am 27.11.19 um 13:38 schrieb Daniel Vetter: > On Wed, Nov 27, 2019 at 01:31:25PM +0100, Thomas Zimmermann wrote: >> Hi Daniel >> >> Am 27.11.19 um 11:49 schrieb Daniel Vetter: >>> On Wed, Nov 27, 2019 at 08:31:09AM +0100, Thomas Zimmermann wrote: >>>> 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 >>>> >>>> The fix implements atomic_check for planes and rejects configurations >>>> with an invisible primary plane. >>>> >>>> v2: >>>> * do an atomic check for plane >>>> * reject invisible primary planes >>>> >>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> >>>> Fixes: b48e1b6ffd28 ("drm/ast: Add CRTC helpers for atomic modesetting") >>>> Cc: Gerd Hoffmann <kraxel@redhat.com> >>>> Cc: Dave Airlie <airlied@redhat.com> >>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>>> Cc: "Y.C. Chen" <yc_chen@aspeedtech.com> >>>> Cc: Sam Ravnborg <sam@ravnborg.org> >>>> --- >>>> drivers/gpu/drm/ast/ast_mode.c | 50 +++++++++++++++++++++++++++++++++- >>>> 1 file changed, 49 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c >>>> index 4725ec911a66..8e7bb8ce8130 100644 >>>> --- a/drivers/gpu/drm/ast/ast_mode.c >>>> +++ b/drivers/gpu/drm/ast/ast_mode.c >>>> @@ -31,6 +31,7 @@ >>>> #include <linux/export.h> >>>> #include <linux/pci.h> >>>> >>>> +#include <drm/drm_atomic.h> >>>> #include <drm/drm_atomic_helper.h> >>>> #include <drm/drm_atomic_state_helper.h> >>>> #include <drm/drm_crtc.h> >>>> @@ -556,6 +557,31 @@ static const uint32_t ast_primary_plane_formats[] = { >>>> int ast_primary_plane_helper_atomic_check(struct drm_plane *plane, >>>> struct drm_plane_state *state) >>>> { >>>> + struct drm_crtc *crtc; >>>> + struct drm_crtc_state *crtc_state; >>>> + int ret; >>>> + >>>> + if (state->crtc) >>>> + crtc = state->crtc; >>>> + else if (plane->state && plane->state->crtc) >>>> + crtc = plane->state->crtc; >>>> + else >>>> + return 0; /* disabling an already disabled plane */ >>>> + >>>> + crtc_state = drm_atomic_get_new_crtc_state(state->state, crtc); >>>> + if (WARN_ON_ONCE(!crtc_state)) >>>> + return -EINVAL; /* BUG: no new CRTC state allocated */ >>> >>> The above is a lot more complicated than necessary, see e.g. >>> atmel_hlcdc_plane_atomic_check. And even that is too complicated, since >>> crtc is set iff fb is set, you only need to check one of them. >> >> Thanks for your review. This comment sounds as if it's at the wrong >> position? > > Nope. Instead of the stuff above you have do this: > > if (!state->crtc) > return 0; > > crtc_state = drm_atomic_get_existing_crtc_state(s->state, s->crtc); > mode = &crtc_state->adjusted_mode; > > which is what atmel has, except the redundant fb check dropped. Same for > the cursor version below too. Thanks for clarifying. > -Daniel > >> >> Best regards >> Thomas >> >>> >>>> + >>>> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, >>>> + DRM_PLANE_HELPER_NO_SCALING, >>>> + DRM_PLANE_HELPER_NO_SCALING, >>>> + false, true); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + if (!state->visible) >>>> + return -EINVAL; /* primary plane must be visible */ >>>> + >>>> return 0; >>>> } >>>> >>>> @@ -567,7 +593,7 @@ void ast_primary_plane_helper_atomic_update(struct drm_plane *plane, >>>> struct drm_gem_vram_object *gbo; >>>> s64 gpu_addr; >>>> >>>> - if (!crtc || !state->fb) >>>> + if (!crtc || !state->fb || !state->visible) >>>> return; >>>> >>>> gbo = drm_gem_vram_of_gem(state->fb->obj[0]); >>>> @@ -660,6 +686,28 @@ ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane, >>>> static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane, >>>> struct drm_plane_state *state) >>>> { >>>> + struct drm_crtc *crtc; >>>> + struct drm_crtc_state *crtc_state; >>>> + int ret; >>>> + >>>> + if (state->crtc) >>>> + crtc = state->crtc; >>>> + else if (plane->state && plane->state->crtc) >>>> + crtc = plane->state->crtc; >>>> + else >>>> + return 0; /* disabling an already disabled plane */ >>>> + >>>> + crtc_state = drm_atomic_get_new_crtc_state(state->state, crtc); >>>> + if (WARN_ON_ONCE(!crtc_state)) >>>> + return -EINVAL; /* BUG: no new CRTC state allocated */ >>>> + >>>> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, >>>> + DRM_PLANE_HELPER_NO_SCALING, >>>> + DRM_PLANE_HELPER_NO_SCALING, >>>> + false, true); >>> >>> Pretty sure you want your cursor to be positionable ... >>> -Daniel >>> >>>> + if (ret) >>>> + return ret; >>>> + >>>> return 0; >>>> } >>>> >>>> -- >>>> 2.23.0 >>>> >>> >> >> -- >> Thomas Zimmermann >> Graphics Driver Developer >> SUSE Software Solutions Germany GmbH >> Maxfeldstr. 5, 90409 Nürnberg, Germany >> (HRB 36809, AG Nürnberg) >> Geschäftsführer: Felix Imendörffer >> > > > >
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 4725ec911a66..8e7bb8ce8130 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -31,6 +31,7 @@ #include <linux/export.h> #include <linux/pci.h> +#include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_atomic_state_helper.h> #include <drm/drm_crtc.h> @@ -556,6 +557,31 @@ static const uint32_t ast_primary_plane_formats[] = { int ast_primary_plane_helper_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) { + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + int ret; + + if (state->crtc) + crtc = state->crtc; + else if (plane->state && plane->state->crtc) + crtc = plane->state->crtc; + else + return 0; /* disabling an already disabled plane */ + + crtc_state = drm_atomic_get_new_crtc_state(state->state, crtc); + if (WARN_ON_ONCE(!crtc_state)) + return -EINVAL; /* BUG: no new CRTC state allocated */ + + ret = drm_atomic_helper_check_plane_state(state, crtc_state, + DRM_PLANE_HELPER_NO_SCALING, + DRM_PLANE_HELPER_NO_SCALING, + false, true); + if (ret) + return ret; + + if (!state->visible) + return -EINVAL; /* primary plane must be visible */ + return 0; } @@ -567,7 +593,7 @@ void ast_primary_plane_helper_atomic_update(struct drm_plane *plane, struct drm_gem_vram_object *gbo; s64 gpu_addr; - if (!crtc || !state->fb) + if (!crtc || !state->fb || !state->visible) return; gbo = drm_gem_vram_of_gem(state->fb->obj[0]); @@ -660,6 +686,28 @@ ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane, static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) { + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + int ret; + + if (state->crtc) + crtc = state->crtc; + else if (plane->state && plane->state->crtc) + crtc = plane->state->crtc; + else + return 0; /* disabling an already disabled plane */ + + crtc_state = drm_atomic_get_new_crtc_state(state->state, crtc); + if (WARN_ON_ONCE(!crtc_state)) + return -EINVAL; /* BUG: no new CRTC state allocated */ + + ret = drm_atomic_helper_check_plane_state(state, crtc_state, + DRM_PLANE_HELPER_NO_SCALING, + DRM_PLANE_HELPER_NO_SCALING, + false, true); + if (ret) + return ret; + return 0; }
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 The fix implements atomic_check for planes and rejects configurations with an invisible primary plane. v2: * do an atomic check for plane * reject invisible primary planes Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> Fixes: b48e1b6ffd28 ("drm/ast: Add CRTC helpers for atomic modesetting") Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Dave Airlie <airlied@redhat.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: "Y.C. Chen" <yc_chen@aspeedtech.com> Cc: Sam Ravnborg <sam@ravnborg.org> --- drivers/gpu/drm/ast/ast_mode.c | 50 +++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) -- 2.23.0