| Message ID | 20230405045611.28093-1-zack@kde.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/atomic-helper: Do not assume vblank is always present | expand |
Hello Zack, Thanks for your patch. Zack Rusin <zack@kde.org> writes: > From: Zack Rusin <zackr@vmware.com> > > Many drivers (in particular all of the virtualized drivers) do not > implement vblank handling. Assuming vblank is always present > leads to crashes. > > Fix the crashes by making sure the device supports vblank before using > it. > [...] > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index 299fa2a19a90..6438aeb1c65f 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -997,12 +997,16 @@ int drm_crtc_next_vblank_start(struct drm_crtc *crtc, ktime_t *vblanktime) > { > unsigned int pipe = drm_crtc_index(crtc); > struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe]; > - struct drm_display_mode *mode = &vblank->hwmode; > + struct drm_display_mode *mode; > u64 vblank_start; > > + if (!drm_dev_has_vblank(crtc->dev)) > + return -EINVAL; > + > if (!vblank->framedur_ns || !vblank->linedur_ns) > return -EINVAL; > > + mode = &vblank->hwmode; > if (!drm_crtc_get_last_vbltimestamp(crtc, vblanktime, false)) > return -EINVAL; > Rob already posted more or less the same fix: https://lore.kernel.org/lkml/CAF6AEGsdT95-uAKcv2+hdMLKd2xwfPeN+FMuDTRMc-Ps7WbRjw@mail.gmail.com/T/
On Wed, Apr 05, 2023 at 09:35:45AM +0200, Javier Martinez Canillas wrote: > > Hello Zack, > > Thanks for your patch. > > Zack Rusin <zack@kde.org> writes: > > > From: Zack Rusin <zackr@vmware.com> > > > > Many drivers (in particular all of the virtualized drivers) do not > > implement vblank handling. Assuming vblank is always present > > leads to crashes. > > > > Fix the crashes by making sure the device supports vblank before using > > it. > > > > [...] > > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > > index 299fa2a19a90..6438aeb1c65f 100644 > > --- a/drivers/gpu/drm/drm_vblank.c > > +++ b/drivers/gpu/drm/drm_vblank.c > > @@ -997,12 +997,16 @@ int drm_crtc_next_vblank_start(struct drm_crtc *crtc, ktime_t *vblanktime) > > { > > unsigned int pipe = drm_crtc_index(crtc); > > struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe]; > > - struct drm_display_mode *mode = &vblank->hwmode; > > + struct drm_display_mode *mode; > > u64 vblank_start; > > > > + if (!drm_dev_has_vblank(crtc->dev)) > > + return -EINVAL; > > + > > if (!vblank->framedur_ns || !vblank->linedur_ns) > > return -EINVAL; > > > > + mode = &vblank->hwmode; > > if (!drm_crtc_get_last_vbltimestamp(crtc, vblanktime, false)) > > return -EINVAL; > > > > Rob already posted more or less the same fix: > > https://lore.kernel.org/lkml/CAF6AEGsdT95-uAKcv2+hdMLKd2xwfPeN+FMuDTRMc-Ps7WbRjw@mail.gmail.com/T/ Yeah I pushed it to drm-misc-next yesterday, please check that works. Also note that we still (in some cases at least where) need another fix to handle the crtc on/off state transitions (but only for drivers which do have vblank), I'll send that out asap. -Daniel
From: Martin Krastev <krastevm@vmware.com> LGTM! Reviewed-by: Martin Krastev <krastevm@vmware.com> Regards, Martin On 5.04.23 г. 7:56 ч., Zack Rusin wrote: > From: Zack Rusin <zackr@vmware.com> > > Many drivers (in particular all of the virtualized drivers) do not > implement vblank handling. Assuming vblank is always present > leads to crashes. > > Fix the crashes by making sure the device supports vblank before using > it. > > Fixes crashes on boot, as in: > Oops: 0000 [#1] PREEMPT SMP PTI > CPU: 0 PID: 377 Comm: systemd-modules Not tainted 6.3.0-rc4-vmwgfx #1 > Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 11/12/2020 > RIP: 0010:drm_crtc_next_vblank_start+0x2c/0x80 [drm] > Code: 1e fa 0f 1f 44 00 00 8b 87 90 00 00 00 48 8b 17 55 48 8d 0c c0 48 89 e5 41 54 53 48 8d 1c 48 48 c1 e3 04 48 03 9a 40 01 00 00 <8b> 53 74 85 d2 74 3f 8b 4> > RSP: 0018:ffffb825004073c8 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 > RDX: ffff9b18cf8d0000 RSI: ffffb825004073e8 RDI: ffff9b18d0f40000 > RBP: ffffb825004073d8 R08: ffff9b18cf8d0000 R09: 0000000000000000 > R10: ffff9b18c57ef6e8 R11: ffff9b18c2d59e00 R12: 0000000000000000 > R13: ffff9b18cfa99280 R14: 0000000000000000 R15: ffff9b18cf8d0000 > FS: 00007f2b82538900(0000) GS:ffff9b19f7c00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000074 CR3: 00000001060a6003 CR4: 00000000003706f0 > Call Trace: > <TASK> > drm_atomic_helper_wait_for_fences+0x87/0x1e0 [drm_kms_helper] > drm_atomic_helper_commit+0xa1/0x160 [drm_kms_helper] > drm_atomic_commit+0x9a/0xd0 [drm] > ? __pfx___drm_printfn_info+0x10/0x10 [drm] > drm_client_modeset_commit_atomic+0x1e9/0x230 [drm] > drm_client_modeset_commit_locked+0x5f/0x160 [drm] > ? mutex_lock+0x17/0x50 > drm_client_modeset_commit+0x2b/0x50 [drm] > __drm_fb_helper_restore_fbdev_mode_unlocked+0xca/0x100 [drm_kms_helper] > drm_fb_helper_set_par+0x40/0x80 [drm_kms_helper] > fbcon_init+0x2c5/0x690 > visual_init+0xee/0x190 > do_bind_con_driver.isra.0+0x1ce/0x4b0 > do_take_over_console+0x136/0x220 > ? vprintk_default+0x21/0x30 > do_fbcon_takeover+0x78/0x130 > do_fb_registered+0x139/0x270 > fbcon_fb_registered+0x3b/0x90 > ? fb_add_videomode+0x81/0xf0 > register_framebuffer+0x22f/0x330 > __drm_fb_helper_initial_config_and_unlock+0x349/0x520 [drm_kms_helper] > ? kmalloc_large+0x25/0xc0 > drm_fb_helper_initial_config+0x3f/0x50 [drm_kms_helper] > drm_fbdev_generic_client_hotplug+0x73/0xc0 [drm_kms_helper] > drm_fbdev_generic_setup+0x99/0x170 [drm_kms_helper] > > Signed-off-by: Zack Rusin <zackr@vmware.com> > Fixes: d39e48ca80c0 ("drm/atomic-helper: Set fence deadline for vblank") > Cc: Rob Clark <robdclark@chromium.org> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/drm_vblank.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index 299fa2a19a90..6438aeb1c65f 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -997,12 +997,16 @@ int drm_crtc_next_vblank_start(struct drm_crtc *crtc, ktime_t *vblanktime) > { > unsigned int pipe = drm_crtc_index(crtc); > struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe]; > - struct drm_display_mode *mode = &vblank->hwmode; > + struct drm_display_mode *mode; > u64 vblank_start; > > + if (!drm_dev_has_vblank(crtc->dev)) > + return -EINVAL; > + > if (!vblank->framedur_ns || !vblank->linedur_ns) > return -EINVAL; > > + mode = &vblank->hwmode; > if (!drm_crtc_get_last_vbltimestamp(crtc, vblanktime, false)) > return -EINVAL; >
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 299fa2a19a90..6438aeb1c65f 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -997,12 +997,16 @@ int drm_crtc_next_vblank_start(struct drm_crtc *crtc, ktime_t *vblanktime) { unsigned int pipe = drm_crtc_index(crtc); struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe]; - struct drm_display_mode *mode = &vblank->hwmode; + struct drm_display_mode *mode; u64 vblank_start; + if (!drm_dev_has_vblank(crtc->dev)) + return -EINVAL; + if (!vblank->framedur_ns || !vblank->linedur_ns) return -EINVAL; + mode = &vblank->hwmode; if (!drm_crtc_get_last_vbltimestamp(crtc, vblanktime, false)) return -EINVAL;