Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752027AbbBVLra (ORCPT ); Sun, 22 Feb 2015 06:47:30 -0500 Received: from mail-we0-f171.google.com ([74.125.82.171]:39301 "EHLO mail-we0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751796AbbBVLr2 (ORCPT ); Sun, 22 Feb 2015 06:47:28 -0500 Date: Sun, 22 Feb 2015 12:49:03 +0100 From: Daniel Vetter To: Aaron Plattner Cc: David Airlie , Daniel Vetter , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/udl: add enable/disable_vblank stubs Message-ID: <20150222114903.GP24485@phenom.ffwll.local> Mail-Followup-To: Aaron Plattner , David Airlie , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <1424047959-23594-1-git-send-email-aplattner@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1424047959-23594-1-git-send-email-aplattner@nvidia.com> X-Operating-System: Linux phenom 3.16-2-amd64 User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6536 Lines: 136 On Sun, Feb 15, 2015 at 04:52:39PM -0800, Aaron Plattner wrote: > vblank_disable_and_save calls the driver's disable_vblank hook unconditionally, > which crashes the udl driver since it doesn't implement it. Fix this by adding > stub implementations of these functions identical to the qxl ones. > > usb 5-1: USB disconnect, device number 2 > BUG: unable to handle kernel NULL pointer dereference at (null) > IP: [< (null)>] (null) > PGD 3bc23d067 PUD 3bc0fc067 PMD 0 > Oops: 0010 [#1] PREEMPT SMP > Modules linked in: udlfb fb_sys_fops nvidia(PO) udl drm_kms_helper drm > syscopyarea sysfillrect sysimgblt netconsole cfg80211 > joydev mousedev nls_iso8859_1 nls_cp437 vfat fat coretemp > intel_rapl eeepc_wmi asus_wmi sparse_keymap led_class > rfkill hwmon iosf_mbi x86_pkg_temp_thermal intel_powerclamp > iTCO_wdt iTCO_vendor_support evdev kvm crct10dif_pclmul > crc32_pclmul ghash_clmulni_intel aesni_intel mac_hid > tpm_infineon aes_x86_64 lrw gf128mul e1000e glue_helper > ablk_helper cryptd mxm_wmi tpm_tis processor psmouse ptp > serio_raw tpm snd_hda_codec_hdmi i2c_i801 i2c_core lpc_ich > pps_core pcspkr snd_hda_codec_realtek snd_hda_codec_generic > mei_me mei snd_hda_intel snd_hda_controller snd_hda_codec > snd_hwdep snd_pcm snd_timer snd ie31200_edac edac_core > soundcore thermal shpchp battery video wmi button fan > sch_fq_codel nfs lockd grace sunrpc fscache btrfs xor > raid6_pq hid_generic usbhid hid sd_mod atkbd libps2 ahci > libahci crc32c_intel xhci_pci libata ehci_pci xhci_hcd > ehci_hcd scsi_mod usbcore usb_common i8042 serio [last > unloaded: drm] > CPU: 2 PID: 2044 Comm: Xorg.bin Tainted: P O 3.19.0-1-agp #1 > Hardware name: System manufacturer System Product Name/P8Z77-V, BIOS 2104 08/13/2013 > task: ffff8803f99d27c0 ti: ffff8803bc1c4000 task.ti: ffff8803bc1c4000 > RIP: 0010:[<0000000000000000>] [< (null)>] (null) > RSP: 0018:ffff8803bc1c7d00 EFLAGS: 00010046 > RAX: ffffffffa06d0140 RBX: 0000000000000000 RCX: 0000000000000000 > RDX: 000000000000cee6 RSI: 0000000000000000 RDI: ffff8804037c3000 > RBP: ffff8803bc1c7d88 R08: 000000000047b69a R09: 0000000000000000 > R10: ffffffffa06de187 R11: 0000000000003246 R12: ffff880403d74540 > R13: 0000000000000003 R14: ffff8804037c3000 R15: ffff8803bc32eac8 > FS: 00007f7f4753a900(0000) GS:ffff88041ec80000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000000 CR3: 00000003bc240000 CR4: 00000000001407e0 > Stack: > ffffffffa06e06da ffff8803bc1c7d38 0000000000000000 0000000000000082 > ffff8804037c3188 ffff8803bc1c7d40 0000000000000282 ffff8803bc1c7d68 > 0000000000000106 000000000000cee6 00000000f69756fa ffff880403d74580 > Call Trace: > [] ? vblank_disable_and_save+0x9a/0x200 [drm] > [] drm_vblank_cleanup+0x65/0xb0 [drm] > [] udl_driver_unload+0x18/0x50 [udl] > [] drm_dev_unregister+0x2d/0xb0 [drm] > [] drm_put_dev+0x27/0x70 [drm] > [] drm_release+0x347/0x520 [drm] > [] __fput+0x9f/0x200 > [] ____fput+0xe/0x10 > [] task_work_run+0xb7/0xf0 > [] do_notify_resume+0x95/0xa0 > [] int_signal+0x12/0x17 > Code: Bad RIP value. > RIP [< (null)>] (null) > RSP > CR2: 0000000000000000 > ---[ end trace e0b184ca053571af ]--- > > Reported-by: Thomas Meyer > Link: http://lists.freedesktop.org/archives/dri-devel/2014-December/073652.html > Signed-off-by: Aaron Plattner > --- > Daniel, it looks like your change "[5/5] drm/irq: Don't call > ->get_vblank_counter directly from irq_uninstall/cleanup" masks the immediate > problem, but it's not clear to me whether that's just because I didn't manage to > trigger any of the new vblank stuff, or whether your change really fixes it. It > does seem like these vblank functions are intended to be called unconditionally. On a quick look udl vblank handling seems to be 100% cargo-cult. It never calls handle_vblank, which means there's never going to be a useful vblank timestamp/counter value, but it also doesn't use the -1 trick for drm_send_vblank_event used to signal that just grabbing a cpu ts is all that's possible. I think the right fix here is to ditch both the vblank_init and cleanup and use -1 in drm_send_vblank_event. -Daniel > > drivers/gpu/drm/udl/udl_drv.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c > index d5728ec85254..5bacb556b0f5 100644 > --- a/drivers/gpu/drm/udl/udl_drv.c > +++ b/drivers/gpu/drm/udl/udl_drv.c > @@ -16,6 +16,20 @@ static int udl_driver_set_busid(struct drm_device *d, struct drm_master *m) > return 0; > } > > +static u32 udl_noop_get_vblank_counter(struct drm_device *dev, int crtc) > +{ > + return dev->vblank[crtc].count.counter; > +} > + > +static int udl_noop_enable_vblank(struct drm_device *dev, int crtc) > +{ > + return 0; > +} > + > +static void udl_noop_disable_vblank(struct drm_device *dev, int crtc) > +{ > +} > + > static const struct vm_operations_struct udl_gem_vm_ops = { > .fault = udl_gem_fault, > .open = drm_gem_vm_open, > @@ -42,6 +56,10 @@ static struct drm_driver driver = { > .unload = udl_driver_unload, > .set_busid = udl_driver_set_busid, > > + .get_vblank_counter = udl_noop_get_vblank_counter, > + .enable_vblank = udl_noop_enable_vblank, > + .disable_vblank = udl_noop_disable_vblank, > + > /* gem hooks */ > .gem_free_object = udl_gem_free_object, > .gem_vm_ops = &udl_gem_vm_ops, > -- > 2.3.0 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/