Re: [PATCH] fbdev: Remove udlfb driver
On Tue, 1 Dec 2020, Mikulas Patocka wrote: > When I try to run Xorg (from Debian 9) with the kernel 5.10-rc6, it > doesn't work at all, I get this crash: I tried to rux Xorg on another machine (with up-to-date Debian ports) and it didn't crash on unplug. Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbdev: Remove udlfb driver
On Mon, 30 Nov 2020, Daniel Vetter wrote: > On Mon, Nov 30, 2020 at 01:39:17PM -0500, Mikulas Patocka wrote: > > > > > > On Mon, 30 Nov 2020, Daniel Vetter wrote: > > > > > On Mon, Nov 30, 2020 at 09:31:15AM -0500, Mikulas Patocka wrote: > > > > > > > > The framebuffer driver supports programs running full-screen directly > > > > on > > > > the framebuffer console, such as web browser "links -g", image viewer > > > > "fbi", postscript+pdf viewer "fbgs", ZX Spectrum emulator "fuse-sdl", > > > > movie player "mplayer -vo fbdev". The DRM driver doesn't run them. > > > > > > Hm this should in general work on drm drivers. Without that it's clear the > > > switch-over isn't really ready yet. > > > > I fixed it with this patch two years ago: > > https://lists.freedesktop.org/archives/dri-devel/2018-June/179023.html > > > > But the patch never went through and the fb_defio feature was removed in > > the kernel 5.6 (commit d0c4fc5a4814e431c15272935c8dc973c18073aa). > > The generic fbdev emulation still has a defio implementation. We could try > to make it more efficient maybe, but it should be all there. The module > option disappeared since you now should always get it by default. Yes, it's there. I've tried to run links on the current DRM UDL driver and it displays correctly - except, that the page list gets sometimes corrupted and a crash happens. > > Without fb_defio, the only other possibility how to update the screen is > > the ioctl DRM_IOCTL_MODE_DIRTYFB. But this ioctl requires master mode, so > > user programs like "links -g" can't issue it. > > Now I'm confused, I thought you wanted to use fbdev /dev/fb* nodes? Those > should support defio. And if you want your applications to use drm > modesetting natively, they have to be drm master anyway. You can't use the > DIRTYFB ioctl to upload fbdev contents. So I'm a bit confused. > -Daniel Yes. I was confused about it too. You can't use DRM ioctls on /dev/fb. Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbdev: Remove udlfb driver
On Tue, 1 Dec 2020, Thomas Zimmermann wrote: > Hi > > Am 30.11.20 um 19:39 schrieb Mikulas Patocka: > > > > > > On Mon, 30 Nov 2020, Daniel Vetter wrote: > > > > > On Mon, Nov 30, 2020 at 09:31:15AM -0500, Mikulas Patocka wrote: > > > > > > > > The framebuffer driver supports programs running full-screen directly on > > > > the framebuffer console, such as web browser "links -g", image viewer > > > > "fbi", postscript+pdf viewer "fbgs", ZX Spectrum emulator "fuse-sdl", > > > > movie player "mplayer -vo fbdev". The DRM driver doesn't run them. > > > > > > Hm this should in general work on drm drivers. Without that it's clear the > > > switch-over isn't really ready yet. > > > > I fixed it with this patch two years ago: > > https://lists.freedesktop.org/archives/dri-devel/2018-June/179023.html > > > > But the patch never went through and the fb_defio feature was removed in > > the kernel 5.6 (commit d0c4fc5a4814e431c15272935c8dc973c18073aa). > > > > > > Without fb_defio, the only other possibility how to update the screen is > > the ioctl DRM_IOCTL_MODE_DIRTYFB. But this ioctl requires master mode, so > > user programs like "links -g" can't issue it. > > That's confusing. DIRTYFB is only for DRM. Yes, you're right. > And why can links not run as DRM master mode? If it renders to the terminal, > it should act like a composer. In that case it almost certainly wants master > status. > > Best regards > Thomas How can a userspace program acquire master mode without being suid? Is there some "Hello World!" program that shows how to use DRM? I'm not an expert in DRM, but if there were some tutorial+documentation, I could consider porting "links" to it. Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbdev: Remove udlfb driver
On Tue, 1 Dec 2020, Thomas Zimmermann wrote: > Hi > > Am 30.11.20 um 15:31 schrieb Mikulas Patocka: > > > > > > On Mon, 30 Nov 2020, Thomas Zimmermann wrote: > > > > > Udlfb has been superseded by DRM's udl. The DRM driver is better by > > > any means and actively maintained. Remove udlfb. > > > > Hi > > > > I am using udlfb and it's definitely better than the DRM driver. The DRM > > driver will crash the kernel if you unplug the device while Xorg is > > running. The framebuffer driver doesn't crash in this case. (I have a cat > > and the cat sometimes unplugs cables and I don't want to reboot the system > > because of it :-) > > What's the exact STR here? Just open the /dev/fb* and pull the cable. > > Do I need a cat? :) When I run links -g on the DRM driver, it works at first, but after some unplug/plug I got this: [ 282.174963] general protection fault, probably for non-canonical address 0xdead00f8: [#1] PREEMPT SMP [ 282.177439] CPU: 9 PID: 3377 Comm: links Not tainted 5.10.0-rc6 #1 [ 282.178843] Hardware name: empty empty/S3992-E, BIOS 'V1.06 ' 06/09/2009 [ 282.180325] RIP: 0010:fb_deferred_io_mkwrite+0x7c/0x100 [fb] [ 282.181865] Code: ff 83 e2 01 48 0f 44 c3 f0 48 0f ba 28 00 0f 82 83 00 00 00 49 8b 45 28 49 39 c4 48 8d 50 f8 74 5d 48 39 d3 74 1c 48 8b 4b 20 <48> 39 4a 20 77 4e 48 8b 42 08 4c 39 e0 48 8d 50 f8 74 41 48 39 d3 [ 282.185341] RSP: :888348293de8 EFLAGS: 00010216 [ 282.187166] RAX: dead0100 RBX: ea0004182980 RCX: [ 282.189071] RDX: dead00f8 RSI: 0801 RDI: a045c048 [ 282.191021] RBP: 888106af1c00 R08: 5fc6128b R09: 3626d41e [ 282.193006] R10: 0008 R11: 88a04fffc000 R12: a045c068 [ 282.195015] R13: a045c040 R14: a045c048 R15: fff03fff [ 282.197077] FS: 7fa64cbf4a00() GS:88a00fcc() knlGS: [ 282.199293] CS: 0010 DS: ES: CR0: 80050033 [ 282.201484] CR2: 7fa63f272008 CR3: 00105b2e CR4: 06e0 [ 282.203728] Call Trace: [ 282.205927] do_page_mkwrite+0x56/0xc0 [ 282.208125] ? __do_fault+0x2d/0xe0 [ 282.210257] handle_mm_fault+0xafc/0x15e0 [ 282.212350] exc_page_fault+0x1ac/0x4a0 [ 282.214408] ? asm_exc_page_fault+0x8/0x30 [ 282.216473] asm_exc_page_fault+0x1e/0x30 [ 282.218462] RIP: 0033:0x7fa64641f3a8 [ 282.220469] Code: c3 48 81 fa 00 08 00 00 77 a8 48 83 fa 40 77 16 f3 0f 7f 07 f3 0f 7f 47 10 f3 0f 7f 44 17 f0 f3 0f 7f 44 17 e0 c3 48 8d 4f 40 0f 7f 07 48 83 e1 c0 f3 0f 7f 44 17 f0 f3 0f 7f 47 10 f3 0f 7f [ 282.224730] RSP: 002b:7ffe6da66ba8 EFLAGS: 00010206 [ 282.226881] RAX: 7fa63f272000 RBX: 7fa64cbf4990 RCX: 7fa63f272040 [ 282.229094] RDX: 003f4800 RSI: RDI: 7fa63f272000 [ 282.231339] RBP: R08: R09: [ 282.233614] R10: 0008 R11: 0246 R12: 0f00 [ 282.235927] R13: 003f4800 R14: 003f4800 R15: 0005 [ 282.238262] Modules linked in: mousedev hid_generic usbhid hid iptable_mangle ipt_REJECT nf_reject_ipv4 tun xt_MASQUERADE iptable_nat xt_tcpudp iscsi_target_mod target_core_pscsi target_core_file ip6table_filter ip6_tables iptable_filter ip_tables x_tables af_packet target_core_mod cpufreq_powersave cpufreq_ondemand cpufreq_userspace configfs cpufreq_conservative autofs4 binfmt_misc bridge stp llc snd_usb_audio snd_hwdep snd_usbmidi_lib snd_rawmidi snd_pcm snd_timer snd soundcore dmi_sysfs nf_nat_ftp nf_conntrack_ftp nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 lm85 hwmon_vid udl drm_kms_helper cfbfillrect syscopyarea cfbimgblt sysfillrect sysimgblt fb_sys_fops cfbcopyarea fb font fbdev drm drm_panel_orientation_quirks backlight ipv6 ohci_pci amd64_edac_mod edac_mce_amd kvm_amd kvm irqbypass tg3 pcspkr ptp pps_core ehci_pci ohci_hcd ehci_hcd libphy usbcore k10temp hwmon e100 usb_common mii i2c_piix4 pata_serverworks rtc_cmos floppy acpi_cpufreq button processor spadfs evdev [ 282.257235] ---[ end trace d7968a41c1fbf717 ]--- [ 282.267115] RIP: 0010:fb_deferred_io_mkwrite+0x7c/0x100 [fb] [ 282.270350] Code: ff 83 e2 01 48 0f 44 c3 f0 48 0f ba 28 00 0f 82 83 00 00 00 49 8b 45 28 49 39 c4 48 8d 50 f8 74 5d 48 39 d3 74 1c 48 8b 4b 20 <48> 39 4a 20 77 4e 48 8b 42 08 4c 39 e0 48 8d 50 f8 74 41 48 39 d3 [ 282.276893] RSP: :888348293de8 EFLAGS: 00010216 [ 282.280202] RAX: dead0100 RBX: ea0004182980 RCX: [ 282.283564] RDX: dead00f8 RSI: 0801 RDI: a045c048 [ 282.286965] RBP: 888106af1c00 R08: 5fc6128b R09: 3626d41e [ 282.290628] R10: 0008 R11: 88a04fffc000 R12: a045c068 [ 282.294039] R13: a045c040 R14:
Re: [PATCH] fbdev: Remove udlfb driver
On Mon, 30 Nov 2020, Daniel Vetter wrote: > On Mon, Nov 30, 2020 at 09:31:15AM -0500, Mikulas Patocka wrote: > > > > The framebuffer driver supports programs running full-screen directly on > > the framebuffer console, such as web browser "links -g", image viewer > > "fbi", postscript+pdf viewer "fbgs", ZX Spectrum emulator "fuse-sdl", > > movie player "mplayer -vo fbdev". The DRM driver doesn't run them. > > Hm this should in general work on drm drivers. Without that it's clear the > switch-over isn't really ready yet. I fixed it with this patch two years ago: https://lists.freedesktop.org/archives/dri-devel/2018-June/179023.html But the patch never went through and the fb_defio feature was removed in the kernel 5.6 (commit d0c4fc5a4814e431c15272935c8dc973c18073aa). Without fb_defio, the only other possibility how to update the screen is the ioctl DRM_IOCTL_MODE_DIRTYFB. But this ioctl requires master mode, so user programs like "links -g" can't issue it. Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbdev: Remove udlfb driver
On Mon, 30 Nov 2020, Thomas Zimmermann wrote: > Udlfb has been superseded by DRM's udl. The DRM driver is better by > any means and actively maintained. Remove udlfb. Hi I am using udlfb and it's definitely better than the DRM driver. The DRM driver will crash the kernel if you unplug the device while Xorg is running. The framebuffer driver doesn't crash in this case. (I have a cat and the cat sometimes unplugs cables and I don't want to reboot the system because of it :-) The framebuffer driver is faster, it keeps back buffer and updates only data that differ between the front and back buffer. The DRM driver doesn't have such optimization, it will update everything in a given rectangle - this increases USB traffic and makes video playback more jerky. The framebuffer driver supports programs running full-screen directly on the framebuffer console, such as web browser "links -g", image viewer "fbi", postscript+pdf viewer "fbgs", ZX Spectrum emulator "fuse-sdl", movie player "mplayer -vo fbdev". The DRM driver doesn't run them. If you seach for someone to maintain the framebuffer driver, I can do it. Mikulas > Signed-off-by: Thomas Zimmermann > --- > CREDITS |5 + > Documentation/fb/index.rst |1 - > Documentation/fb/udlfb.rst | 162 --- > MAINTAINERS |9 - > drivers/video/fbdev/Kconfig | 17 +- > drivers/video/fbdev/Makefile |1 - > drivers/video/fbdev/udlfb.c | 1994 -- > 7 files changed, 6 insertions(+), 2183 deletions(-) > delete mode 100644 Documentation/fb/udlfb.rst > delete mode 100644 drivers/video/fbdev/udlfb.c ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbdev: fbcon: Fix unregister crash when more than one framebuffer
On Mon, 10 Dec 2018, Noralf Trønnes wrote:
> When unregistering fbdev using unregister_framebuffer(), any bound
> console will unbind automatically. This is working fine if this is the
> only framebuffer, resulting in a switch to the dummy console. However if
> there is a fb0 and I unregister fb1 having a bound console, I eventually
> get a crash. The fastest way for me to trigger the crash is to do a
> reboot, resulting in this splat:
>
> [ 76.478825] WARNING: CPU: 0 PID: 527 at linux/kernel/workqueue.c:1442
> __queue_work+0x2d4/0x41c
> [ 76.478849] Modules linked in: raspberrypi_hwmon gpio_backlight backlight
> bcm2835_rng rng_core [last unloaded: tinydrm]
> [ 76.478916] CPU: 0 PID: 527 Comm: systemd-udevd Not tainted 4.20.0-rc4+ #4
> [ 76.478933] Hardware name: BCM2835
> [ 76.478949] Backtrace:
> [ 76.478995] [] (dump_backtrace) from []
> (show_stack+0x20/0x24)
> [ 76.479022] r6: r5:c0bc73be r4: r3:6fb5bf81
> [ 76.479060] [] (show_stack) from []
> (dump_stack+0x20/0x28)
> [ 76.479102] [] (dump_stack) from [] (__warn+0xec/0x12c)
> [ 76.479134] [] (__warn) from []
> (warn_slowpath_null+0x4c/0x58)
> [ 76.479165] r9:c0eb6944 r8:0001 r7:c0e927f8 r6:c0bc73be r5:05a2
> r4:c0139e84
> [ 76.479197] [] (warn_slowpath_null) from []
> (__queue_work+0x2d4/0x41c)
> [ 76.479222] r6:d7666a00 r5:c0e918ee r4:dbc4e700
> [ 76.479251] [] (__queue_work) from []
> (queue_work_on+0x60/0x88)
> [ 76.479281] r10:c0496bf8 r9:0100 r8:c0e92ae0 r7:0001 r6:d9403700
> r5:d7666a00
> [ 76.479298] r4:2113
> [ 76.479348] [] (queue_work_on) from []
> (cursor_timer_handler+0x30/0x54)
> [ 76.479374] r7:d8a8fabc r6:c0e08088 r5:d8afdc5c r4:d8a8fabc
> [ 76.479413] [] (cursor_timer_handler) from []
> (call_timer_fn+0x100/0x230)
> [ 76.479435] r4:c0e9192f r3:d758a340
> [ 76.479465] [] (call_timer_fn) from []
> (expire_timers+0x10c/0x12c)
> [ 76.479495] r10:4000 r9:c0e9192f r8:c0e92ae0 r7:d8afdccc r6:c0e19280
> r5:c0496bf8
> [ 76.479513] r4:d8a8fabc
> [ 76.479541] [] (expire_timers) from []
> (run_timer_softirq+0xa8/0x184)
> [ 76.479570] r9:0001 r8:c0e19280 r7: r6:c0e08088 r5:c0e1a3e0
> r4:c0e19280
> [ 76.479603] [] (run_timer_softirq) from []
> (__do_softirq+0x1ac/0x3fc)
> [ 76.479632] r10:c0e91680 r9:d8afc020 r8:000a r7:0100 r6:0001
> r5:0002
> [ 76.479650] r4:c0eb65ec
> [ 76.479686] [] (__do_softirq) from []
> (irq_exit+0xe8/0x168)
> [ 76.479716] r10:d8d1a9b0 r9:d8afc000 r8:0001 r7:d949c000 r6:
> r5:c0e8b3f0
> [ 76.479734] r4:
> [ 76.479764] [] (irq_exit) from []
> (__handle_domain_irq+0x94/0xb0)
> [ 76.479793] [] (__handle_domain_irq) from []
> (bcm2835_handle_irq+0x3c/0x48)
> [ 76.479823] r8:d8afdebc r7:d8afddfc r6: r5:c0e089f8 r4:d8afddc8
> r3:d8afddc8
> [ 76.479851] [] (bcm2835_handle_irq) from []
> (__irq_svc+0x70/0x98)
>
> The problem is in the console rebinding in fbcon_fb_unbind(). It uses the
> virtual console index as the new framebuffer index to bind the console(s)
> to. The correct way is to use the con2fb_map lookup table to find the
> framebuffer index.
>
> Fixes: cfafca8067c6 ("fbdev: fbcon: console unregistration from
> unregister_framebuffer")
> Signed-off-by: Noralf Trønnes
> Reviewed-by: Mikulas Patocka
> ---
>
> Mikulas,
> If you have forgotten about this, here's where you gave your r-b:
> https://lists.freedesktop.org/archives/dri-devel/2018-July/182728.html
>
> drivers/video/fbdev/core/fbcon.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c
> b/drivers/video/fbdev/core/fbcon.c
> index 8958ccc8b1ac..8976190b6c1f 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -3064,7 +3064,7 @@ static int fbcon_fb_unbind(int idx)
> for (i = first_fb_vc; i <= last_fb_vc; i++) {
> if (con2fb_map[i] != idx &&
> con2fb_map[i] != -1) {
> - new_idx = i;
> + new_idx = con2fb_map[i];
> break;
> }
> }
> --
> 2.19.2
Acked-by: Mikulas Patocka
Mikulas___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Sleeping from invalid context in udlfb
On Thu, 2 Aug 2018, David Airlie wrote:
>
> I'm pretty sure udlkms handles this already.
>
> Dave.
But it crashes on unplug :-)
Mikulas
> On Wed, Aug 1, 2018 at 11:34 PM, Mikulas Patocka wrote:
>
>
> On Wed, 1 Aug 2018, Geert Uytterhoeven wrote:
>
> > Hi Mikulas,
> >
> > On Wed, Aug 1, 2018 at 12:59 PM Mikulas Patocka
> wrote:
> > > On Wed, 1 Aug 2018, Geert Uytterhoeven wrote:
> > > > On Tue, Jul 31, 2018 at 5:23 PM Mikulas Patocka
> wrote:
> > > > > BTW when using the udlfb driver as a console, I've got this
> warning.
> > > > > vt_console_print takes a spinlock and then calls the
> framebuffer driver
> > > > > that sleeps.
> > > > >
> > > > > The question is - whose fault is this? Could the console code
> somehow be
> > > > > told to print characters without holding a spinlock? Or does it
> mean that
> > > > > framebuffer drivers can't sleep?
> > > > >
> > > > > udlfb communicates through USB, so the sleeping is inevitable.
> > > > >
> > > > > Mikulas
> > > > >
> > > > >
> > > > > BUG: sleeping function called from invalid context at
> mm/slab.h:421
> > > > > in_atomic(): 1, irqs_disabled(): 0, pid: 430, name: kworker/2:3
> > > > > 6 locks held by kworker/2:3/430:
> > > > > #0: 1301127e ( (wq_completion)"events"){} , at:
> process_one_work+0x17c/0x3a8
> > > > > #1: beacc951 (
> (work_completion)(&(>init_framebuffer_work)->work)){} , at:
> process_one_work+0x17c/0x3a8
> > > > > #2: a402f826 ( registration_lock){} , at:
> register_framebuffer+0x28/0x2c0 [fb]
> > > > > #3: 21cbe902 ( console_lock){} , at:
> register_framebuffer+0x258/0x2c0 [fb]
> > > > > #4: 96d51735 ( console_owner){} , at:
> console_unlock+0x174/0x500
> > > > > #5: faa7f206 ( printing_lock){} , at:
> vt_console_print+0x60/0x3a0
> > > > > Preemption disabled at: []
> vt_console_print+0x60/0x3a0
> > > > > CPU: 2 PID: 430 Comm: kworker/2:3 Not tainted 4.17.10-debug #3
> > > > > Hardware name: Marvell Armada 8040 MacchiatoBin/Armada 8040
> MacchiatoBin, BIOS EDK II Jul 30 2018
> > > > > Workqueue: events dlfb_init_framebuffer_work [udlfb]
> > > > > Call trace:
> > > > > dump_backtrace+0x0/0x150
> > > > > show_stack+0x14/0x20
> > > > > dump_stack+0x8c/0xac
> > > > > ___might_sleep+0x140/0x170
> > > > > __might_sleep+0x50/0x88
> > > > > __kmalloc+0x1b0/0x270
> > > > > xhci_urb_enqueue+0xa8/0x460 [xhci_hcd]
> > > > > usb_hcd_submit_urb+0xc0/0x998 [usbcore]
> > > > > usb_submit_urb+0x1e0/0x518 [usbcore]
> > > > > dlfb_submit_urb+0x38/0x98 [udlfb]
> > > > > dlfb_handle_damage.isra.4+0x1e0/0x210 [udlfb]
> > > > > dlfb_ops_imageblit+0x28/0x38 [udlfb]
> > > > > soft_cursor+0x15c/0x1d8 [fb]
> > > > > bit_cursor+0x324/0x510 [fb]
> > > > > fbcon_cursor+0x144/0x1a0 [fb]
> > > > > hide_cursor+0x38/0xa0
> > > > > vt_console_print+0x334/0x3a0
> > > > > console_unlock+0x274/0x500
> > > > > register_framebuffer+0x22c/0x2c0 [fb]
> > > > > dlfb_init_framebuffer_work+0x1ec/0x2fc [udlfb]
> > > > > process_one_work+0x1e8/0x3a8
> > > > > worker_thread+0x44/0x418
> > > > > kthread+0x11c/0x120
> > > > > ret_from_fork+0x10/0x18
> > > >
> > > > This is sort of expected: you cannot do USB transfers from
> printk().
> > > >
> > > > Gr{oetje,eeting}s,
> > > >
> > > > Geert
> > >
> > > So, should there be a framebuffer flag that prevents the console
> from
> > > binding to it?
> > >
> > > If I start the kernel with "console=ttyS0,115200", it doesn't try
> to bind
> > > to the udlfb driver, but if I start it without this flag, it does
> and
> > > crashes :-(
> >
> > Your frame buffer driver should offload tasks that may sleep to e.g. a
> > workqueue.
> >
> > Gr{oetje,eeting}s,
> >
> > Geert
>
> I can try to do this - but - taking a spinlock and copying 8MB framebuffer
> would damage scheduling latency even for PCI framebuffer drivers.
>
> Mikulas
>
>
>
> ___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] udlfb: fix NULL pointer dereference in dlfb_usb_probe()
On Fri, 9 Nov 2018, Alexey Khoroshilov wrote:
> If memory allocation for dlfb fails, error handling code
> unconditionally dereference NULL pointer.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov
> Fixes: 68a958a915ca ("udlfb: handle unplug properly")
Reviewed-by: Mikulas Patocka
> ---
> drivers/video/fbdev/udlfb.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c
> index 070026a7e55a..9643fe7c8349 100644
> --- a/drivers/video/fbdev/udlfb.c
> +++ b/drivers/video/fbdev/udlfb.c
> @@ -1590,7 +1590,7 @@ static int dlfb_usb_probe(struct usb_interface *intf,
> int i;
> const struct device_attribute *attr;
> struct dlfb_data *dlfb;
> - struct fb_info *info;
> + struct fb_info *info = NULL;
> int retval = -ENOMEM;
> struct usb_device *usbdev = interface_to_usbdev(intf);
>
> @@ -1701,8 +1701,8 @@ static int dlfb_usb_probe(struct usb_interface *intf,
> return 0;
>
> error:
> - if (dlfb->info) {
> - dlfb_ops_destroy(dlfb->info);
> + if (info) {
> + dlfb_ops_destroy(info);
> } else if (dlfb) {
> usb_put_dev(dlfb->udev);
> kfree(dlfb);
> --
> 2.7.4
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm udl fixes
On Tue, 4 Sep 2018, Daniel Vetter wrote: > With kms you need logind or someone like that who orchestrates the vt > switching and makes sure you can read/write other people's stuff. BTW. I'm just wondering how is this 'master mode' security working at all. The user start Xserver under the user's UID and the Xserver asks logind to set master mode on the DRM file descriptor. There are plenty of ways how the user can steal a file descriptor from the Xserver that is running under the same UID - for example: - setting LD_PRELOAD to inject a library into the Xserver - calling ptrace on the Xserver process - opening /proc/`pidof Xorg`/fd When one of the user's processes has a handle in 'master mode', any other user's process can steal it. So what does these 'master mode' restrictions really protect against? Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm udl fixes
On Tue, 4 Sep 2018, Daniel Vetter wrote: > On Tue, Sep 4, 2018 at 7:04 PM, Mikulas Patocka wrote: > > > > > > On Tue, 4 Sep 2018, Daniel Vetter wrote: > > > >> On Tue, Sep 4, 2018 at 1:41 AM, Dave Airlie wrote: > >> >> > >> >> I've seen that you dropped this patch: > >> >> https://patchwork.kernel.org/patch/10445393/ > >> >> > >> >> Is that patch correct or incorrect? In case it is incorrect, do you have > >> >> an idea how should fbdefio be used properly on KMS drivers? > >> > > >> > I suppose I was wondering what use fbdefio really has, modern code > >> > should be using KMS interface and the KMS dirty handling should be > >> > able to cover those cases. > > > > I use fbdefio with the links 2 web browser and with fbi and fbgs image > > viewers. > > > > I tried to look into modifying the links browser to work without fbdefio - > > but the DRM_IOCTL_MODE_DIRTYFB ioctl requires master mode, which means > > that it is unuseable by an unprivileged process. If you suggest how should > > console applications use the framebuffer without fbdefio, I can implement > > it. > > Well with the console there's really no security. Either you disallow > touching fbdev, or everyone can read whatever they want, even if > they're not the currently active vt. So security just doesn't exist > with fbdev. I added myself to the video group, so that I can open /dev/fb0 and run graphics console applications. But I still can't set master mode, so I can't issue DRM_IOCTL_MODE_DIRTYFB to update the screen when fbdefio is turned off. > With kms you need logind or someone like that who orchestrates the vt > switching and makes sure you can read/write other people's stuff. The Links browser registers two signals via vt_mode.acqsig and vt_mode.relsig and repaints the screen or stops painting when they are received. If it should do something else - then what? It would be nice if the kms developers provided a simple graphics console application that describes how to use it. Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm udl fixes
On Tue, 4 Sep 2018, Daniel Vetter wrote: > On Tue, Sep 4, 2018 at 1:41 AM, Dave Airlie wrote: > >> > >> I've seen that you dropped this patch: > >> https://patchwork.kernel.org/patch/10445393/ > >> > >> Is that patch correct or incorrect? In case it is incorrect, do you have > >> an idea how should fbdefio be used properly on KMS drivers? > > > > I suppose I was wondering what use fbdefio really has, modern code > > should be using KMS interface and the KMS dirty handling should be > > able to cover those cases. I use fbdefio with the links 2 web browser and with fbi and fbgs image viewers. I tried to look into modifying the links browser to work without fbdefio - but the DRM_IOCTL_MODE_DIRTYFB ioctl requires master mode, which means that it is unuseable by an unprivileged process. If you suggest how should console applications use the framebuffer without fbdefio, I can implement it. > > I don't really like the maintainability decrease moving to in-driver > > page handling causes vs using shmem. > > I think tinydrm has the same problem of shmem+fbdefio, could be worth > it to chat with Noralf. Adding him. Perhaps we could allocate tiny structures that point to pages and construct the list from them - but it would require rewriting all the fbdefio drivers, and I wonder if anyone has all the hardware that uses fbdefio to test it. Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm udl fixes
On Tue, 31 Jul 2018, Dave Airlie wrote: > Pull request to myself just so it's logged and linked in right place, > but this is a set of Mikulas's udl kms patches I've looked over and am > happy with. > > Dave. I've seen that you dropped this patch: https://patchwork.kernel.org/patch/10445393/ Is that patch correct or incorrect? In case it is incorrect, do you have an idea how should fbdefio be used properly on KMS drivers? Mikulas > The following changes since commit acb1872577b346bd15ab3a3f8dff780d6cca4b70: > > Linux 4.18-rc7 (2018-07-29 14:44:52 -0700) > > are available in the Git repository at: > > git://people.freedesktop.org/~airlied/linux drm-udl-next > > for you to fetch changes up to 90991209837ab619555a46a97a88dead7a960d2d: > > udl-kms: dont spam the syslog with debug messages (2018-07-31 08:11:12 > +1000) > > ------------ > Mikulas Patocka (7): > udl-kms: change down_interruptible to down > udl-kms: handle allocation failure > udl-kms: fix crash due to uninitialized memory > udl-kms: avoid division > udl-kms: avoid prefetch > udl-kms: use spin_lock_irq instead of spin_lock_irqsave > udl-kms: dont spam the syslog with debug messages > > drivers/gpu/drm/udl/udl_drv.h | 2 +- > drivers/gpu/drm/udl/udl_fb.c | 23 ++- > drivers/gpu/drm/udl/udl_main.c | 45 +++-- > drivers/gpu/drm/udl/udl_modeset.c | 7 +++--- > drivers/gpu/drm/udl/udl_transfer.c | 46 > +- > 5 files changed, 60 insertions(+), 63 deletions(-) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] mach64: fix image corruption due to reading accelerator registers
On Mon, 27 Aug 2018, Ville Syrjälä wrote:
> On Sat, Aug 25, 2018 at 03:51:52PM -0400, Mikulas Patocka wrote:
> > Reading the registers without waiting for engine idle returns
> > unpredictable values. These unpredictable values result in display
> > corruption - if atyfb_imageblit reads the content of DP_PIX_WIDTH with the
> > bit DP_HOST_TRIPLE_EN set (from previous invocation), the driver would
> > never ever clear the bit, resulting in display corruption.
> >
> > We don't want to wait for idle because it would degrade performance, so
> > this patch modifies the driver so that it never reads accelerator
> > registers.
> >
> > HOST_CNTL doesn't have to be read, we can just write it with
> > HOST_BYTE_ALIGN because no other part of the driver cares if
> > HOST_BYTE_ALIGN is set.
> >
> > DP_PIX_WIDTH is written in the functions atyfb_copyarea and atyfb_fillrect
> > with the default value and in atyfb_imageblit with the value set according
> > to the source image data.
> >
> > Signed-off-by: Mikulas Patocka
> > Cc: sta...@vger.kernel.org
> >
> > ---
> > drivers/video/fbdev/aty/mach64_accel.c | 22 +-
> > 1 file changed, 9 insertions(+), 13 deletions(-)
> >
> > Index: linux-stable/drivers/video/fbdev/aty/mach64_accel.c
> > ===
> > --- linux-stable.orig/drivers/video/fbdev/aty/mach64_accel.c
> > 2018-08-24 19:51:34.0 +0200
> > +++ linux-stable/drivers/video/fbdev/aty/mach64_accel.c 2018-08-24
> > 20:28:55.0 +0200
> > @@ -127,7 +127,7 @@ void aty_init_engine(struct atyfb_par *p
> >
> > /* set host attributes */
> > wait_for_fifo(13, par);
> > - aty_st_le32(HOST_CNTL, 0, par);
> > + aty_st_le32(HOST_CNTL, HOST_BYTE_ALIGN, par);
> >
> > /* set pattern attributes */
> > aty_st_le32(PAT_REG0, 0, par);
> > @@ -233,7 +233,8 @@ void atyfb_copyarea(struct fb_info *info
> > rotation = rotation24bpp(dx, direction);
> > }
> >
> > - wait_for_fifo(4, par);
> > + wait_for_fifo(5, par);
> > + aty_st_le32(DP_PIX_WIDTH, par->crtc.dp_pix_width, par);
> > aty_st_le32(DP_SRC, FRGD_SRC_BLIT, par);
> > aty_st_le32(SRC_Y_X, (sx << 16) | sy, par);
> > aty_st_le32(SRC_HEIGHT1_WIDTH1, (width << 16) | area->height, par);
> > @@ -269,7 +270,8 @@ void atyfb_fillrect(struct fb_info *info
> > rotation = rotation24bpp(dx, DST_X_LEFT_TO_RIGHT);
> > }
> >
> > - wait_for_fifo(3, par);
> > + wait_for_fifo(4, par);
> > + aty_st_le32(DP_PIX_WIDTH, par->crtc.dp_pix_width, par);
> > aty_st_le32(DP_FRGD_CLR, color, par);
> > aty_st_le32(DP_SRC,
> > BKGD_SRC_BKGD_CLR | FRGD_SRC_FRGD_CLR | MONO_SRC_ONE,
> > @@ -284,7 +286,7 @@ void atyfb_imageblit(struct fb_info *inf
> > {
> > struct atyfb_par *par = (struct atyfb_par *) info->par;
> > u32 src_bytes, dx = image->dx, dy = image->dy, width = image->width;
> > - u32 pix_width_save, pix_width, host_cntl, rotation = 0, src, mix;
> > + u32 pix_width, rotation = 0, src, mix;
> >
> > if (par->asleep)
> > return;
> > @@ -296,8 +298,7 @@ void atyfb_imageblit(struct fb_info *inf
> > return;
> > }
> >
> > - pix_width = pix_width_save = aty_ld_le32(DP_PIX_WIDTH, par);
> > - host_cntl = aty_ld_le32(HOST_CNTL, par) | HOST_BYTE_ALIGN;
> > + pix_width = par->crtc.dp_pix_width;
> >
> > switch (image->depth) {
> > case 1:
> > @@ -370,12 +371,11 @@ void atyfb_imageblit(struct fb_info *inf
> > mix = FRGD_MIX_D_XOR_S | BKGD_MIX_D;
> > }
> >
> > - wait_for_fifo(6, par);
> > - aty_st_le32(DP_WRITE_MASK, 0x, par);
>
> Looks like init_engine() sets this one for us. So dropping should be ok.
>
> > + wait_for_fifo(5, par);
> > aty_st_le32(DP_PIX_WIDTH, pix_width, par);
> > aty_st_le32(DP_MIX, mix, par);
> > aty_st_le32(DP_SRC, src, par);
> > - aty_st_le32(HOST_CNTL, host_cntl, par);
> > + aty_st_le32(HOST_CNTL, HOST_BYTE_ALIGN, par);
>
> Presumably we could drop this as well since it never changes.
I tried to drop this HOST_CNTL write and it didn't work. It seems that the
register is written implicitly by some of the other commands.
Mikulas
> Either way
> Reviewed-by: Ville Syrjälä
>
> > aty_st_le32(DST_CNTL, DST_Y_TOP_TO_BOTTOM | DST_X_LEFT_TO_RIGHT |
> > rotation, par);
> >
> > draw_rect(dx, dy, width, image->height, par);
> > @@ -424,8 +424,4 @@ void atyfb_imageblit(struct fb_info *inf
> > aty_st_le32(HOST_DATA0, get_unaligned_le32(pbitmap),
> > par);
> > }
> > }
> > -
> > - /* restore pix_width */
> > - wait_for_fifo(1, par);
> > - aty_st_le32(DP_PIX_WIDTH, pix_width_save, par);
> > }
>
> --
> Ville Syrjälä
> syrj...@sci.fi
> http://www.sci.fi/~syrjala/
> ___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] mach64: fix console corruption in 24bpp mode
On Fri, 17 Aug 2018, Mikulas Patocka wrote:
> There's console font corruption when using the mach64 driver in 24bpp
> mode.
>
> In 24bpp mode, the mach64 accelerator is set up for 8-bpp mode (with
> horizontal width and stride multiplied by 3). In this mode, the
> accelerator can't even possibly support color expansion. Consquently, we
> have to use an unaccelerated function cfb_imageblit for color expansion.
>
> Signed-off-by: Mikulas Patocka
> Cc: sta...@vger.kernel.org
Self-nack for this patch.
I've sent further patches that fix the display corruption without
disabling the accelerator.
Mikulas
> ---
> drivers/video/fbdev/aty/mach64_accel.c |3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: linux-stable/drivers/video/fbdev/aty/mach64_accel.c
> ===
> --- linux-stable.orig/drivers/video/fbdev/aty/mach64_accel.c 2018-04-20
> 18:11:01.0 +0200
> +++ linux-stable/drivers/video/fbdev/aty/mach64_accel.c 2018-08-13
> 17:37:04.0 +0200
> @@ -291,7 +291,8 @@ void atyfb_imageblit(struct fb_info *inf
> if (!image->width || !image->height)
> return;
> if (!par->accel_flags ||
> - (image->depth != 1 && info->var.bits_per_pixel != image->depth)) {
> + (image->depth != 1 && info->var.bits_per_pixel != image->depth) ||
> + (image->depth == 1 && info->var.bits_per_pixel == 24)) {
> cfb_imageblit(info, image);
> return;
> }
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] mach64: optimize wait_for_fifo
This is a simple optimization for fifo waiting that improves scrolling
performance by 5%. If the queue has more free entries that what we
consume, we can skip the costly register read next time.
Signed-off-by: Mikulas Patocka
---
drivers/video/fbdev/aty/atyfb.h| 12
drivers/video/fbdev/aty/mach64_accel.c |4 +++-
2 files changed, 11 insertions(+), 5 deletions(-)
Index: linux-stable/drivers/video/fbdev/aty/atyfb.h
===
--- linux-stable.orig/drivers/video/fbdev/aty/atyfb.h 2018-08-25
21:49:16.0 +0200
+++ linux-stable/drivers/video/fbdev/aty/atyfb.h2018-08-25
21:52:51.0 +0200
@@ -147,6 +147,7 @@ struct atyfb_par {
u16 pci_id;
u32 accel_flags;
int blitter_may_be_busy;
+ unsigned fifo_space;
int asleep;
int lock_blank;
unsigned long res_start;
@@ -346,10 +347,13 @@ extern int aty_init_cursor(struct fb_inf
* Hardware acceleration
*/
-static inline void wait_for_fifo(u16 entries, const struct atyfb_par *par)
+static inline void wait_for_fifo(u16 entries, struct atyfb_par *par)
{
- while ((aty_ld_le32(FIFO_STAT, par) & 0x) >
- ((u32) (0x8000 >> entries)));
+ unsigned fifo_space = par->fifo_space;
+ while (entries > fifo_space) {
+ fifo_space = 16 - fls(aty_ld_le32(FIFO_STAT, par) & 0x);
+ }
+ par->fifo_space = fifo_space - entries;
}
static inline void wait_for_idle(struct atyfb_par *par)
@@ -359,7 +363,7 @@ static inline void wait_for_idle(struct
par->blitter_may_be_busy = 0;
}
-extern void aty_reset_engine(const struct atyfb_par *par);
+extern void aty_reset_engine(struct atyfb_par *par);
extern void aty_init_engine(struct atyfb_par *par, struct fb_info *info);
void atyfb_copyarea(struct fb_info *info, const struct fb_copyarea *area);
Index: linux-stable/drivers/video/fbdev/aty/mach64_accel.c
===
--- linux-stable.orig/drivers/video/fbdev/aty/mach64_accel.c2018-08-25
21:49:16.0 +0200
+++ linux-stable/drivers/video/fbdev/aty/mach64_accel.c 2018-08-25
21:49:16.0 +0200
@@ -37,7 +37,7 @@ static u32 rotation24bpp(u32 dx, u32 dir
return ((rotation << 8) | DST_24_ROTATION_ENABLE);
}
-void aty_reset_engine(const struct atyfb_par *par)
+void aty_reset_engine(struct atyfb_par *par)
{
/* reset engine */
aty_st_le32(GEN_TEST_CNTL,
@@ -50,6 +50,8 @@ void aty_reset_engine(const struct atyfb
/* HOST errors */
aty_st_le32(BUS_CNTL,
aty_ld_le32(BUS_CNTL, par) | BUS_HOST_ERR_ACK |
BUS_FIFO_ERR_ACK, par);
+
+ par->fifo_space = 0;
}
static void reset_GTC_3D_engine(const struct atyfb_par *par)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] mach64: fix image corruption due to reading accelerator registers
Reading the registers without waiting for engine idle returns
unpredictable values. These unpredictable values result in display
corruption - if atyfb_imageblit reads the content of DP_PIX_WIDTH with the
bit DP_HOST_TRIPLE_EN set (from previous invocation), the driver would
never ever clear the bit, resulting in display corruption.
We don't want to wait for idle because it would degrade performance, so
this patch modifies the driver so that it never reads accelerator
registers.
HOST_CNTL doesn't have to be read, we can just write it with
HOST_BYTE_ALIGN because no other part of the driver cares if
HOST_BYTE_ALIGN is set.
DP_PIX_WIDTH is written in the functions atyfb_copyarea and atyfb_fillrect
with the default value and in atyfb_imageblit with the value set according
to the source image data.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/aty/mach64_accel.c | 22 +-
1 file changed, 9 insertions(+), 13 deletions(-)
Index: linux-stable/drivers/video/fbdev/aty/mach64_accel.c
===
--- linux-stable.orig/drivers/video/fbdev/aty/mach64_accel.c2018-08-24
19:51:34.0 +0200
+++ linux-stable/drivers/video/fbdev/aty/mach64_accel.c 2018-08-24
20:28:55.0 +0200
@@ -127,7 +127,7 @@ void aty_init_engine(struct atyfb_par *p
/* set host attributes */
wait_for_fifo(13, par);
- aty_st_le32(HOST_CNTL, 0, par);
+ aty_st_le32(HOST_CNTL, HOST_BYTE_ALIGN, par);
/* set pattern attributes */
aty_st_le32(PAT_REG0, 0, par);
@@ -233,7 +233,8 @@ void atyfb_copyarea(struct fb_info *info
rotation = rotation24bpp(dx, direction);
}
- wait_for_fifo(4, par);
+ wait_for_fifo(5, par);
+ aty_st_le32(DP_PIX_WIDTH, par->crtc.dp_pix_width, par);
aty_st_le32(DP_SRC, FRGD_SRC_BLIT, par);
aty_st_le32(SRC_Y_X, (sx << 16) | sy, par);
aty_st_le32(SRC_HEIGHT1_WIDTH1, (width << 16) | area->height, par);
@@ -269,7 +270,8 @@ void atyfb_fillrect(struct fb_info *info
rotation = rotation24bpp(dx, DST_X_LEFT_TO_RIGHT);
}
- wait_for_fifo(3, par);
+ wait_for_fifo(4, par);
+ aty_st_le32(DP_PIX_WIDTH, par->crtc.dp_pix_width, par);
aty_st_le32(DP_FRGD_CLR, color, par);
aty_st_le32(DP_SRC,
BKGD_SRC_BKGD_CLR | FRGD_SRC_FRGD_CLR | MONO_SRC_ONE,
@@ -284,7 +286,7 @@ void atyfb_imageblit(struct fb_info *inf
{
struct atyfb_par *par = (struct atyfb_par *) info->par;
u32 src_bytes, dx = image->dx, dy = image->dy, width = image->width;
- u32 pix_width_save, pix_width, host_cntl, rotation = 0, src, mix;
+ u32 pix_width, rotation = 0, src, mix;
if (par->asleep)
return;
@@ -296,8 +298,7 @@ void atyfb_imageblit(struct fb_info *inf
return;
}
- pix_width = pix_width_save = aty_ld_le32(DP_PIX_WIDTH, par);
- host_cntl = aty_ld_le32(HOST_CNTL, par) | HOST_BYTE_ALIGN;
+ pix_width = par->crtc.dp_pix_width;
switch (image->depth) {
case 1:
@@ -370,12 +371,11 @@ void atyfb_imageblit(struct fb_info *inf
mix = FRGD_MIX_D_XOR_S | BKGD_MIX_D;
}
- wait_for_fifo(6, par);
- aty_st_le32(DP_WRITE_MASK, 0x, par);
+ wait_for_fifo(5, par);
aty_st_le32(DP_PIX_WIDTH, pix_width, par);
aty_st_le32(DP_MIX, mix, par);
aty_st_le32(DP_SRC, src, par);
- aty_st_le32(HOST_CNTL, host_cntl, par);
+ aty_st_le32(HOST_CNTL, HOST_BYTE_ALIGN, par);
aty_st_le32(DST_CNTL, DST_Y_TOP_TO_BOTTOM | DST_X_LEFT_TO_RIGHT |
rotation, par);
draw_rect(dx, dy, width, image->height, par);
@@ -424,8 +424,4 @@ void atyfb_imageblit(struct fb_info *inf
aty_st_le32(HOST_DATA0, get_unaligned_le32(pbitmap),
par);
}
}
-
- /* restore pix_width */
- wait_for_fifo(1, par);
- aty_st_le32(DP_PIX_WIDTH, pix_width_save, par);
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] mach64: fix display corruption on big endian machines
The code for manual bit triple is not endian-clean. It builds the variable
"hostdword" using byte accesses, therefore we must read the variable with
"le32_to_cpu".
The patch also enables (hardware or software) bit triple only if the image
is monochrome (image->depth). If we want to blit full-color image, we
shouldn't use the triple code.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/aty/mach64_accel.c |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Index: linux-stable/drivers/video/fbdev/aty/mach64_accel.c
===
--- linux-stable.orig/drivers/video/fbdev/aty/mach64_accel.c2018-08-24
17:31:21.0 +0200
+++ linux-stable/drivers/video/fbdev/aty/mach64_accel.c 2018-08-24
19:12:40.0 +0200
@@ -345,7 +345,7 @@ void atyfb_imageblit(struct fb_info *inf
* since Rage 3D IIc we have DP_HOST_TRIPLE_EN bit
* this hwaccelerated triple has an issue with not aligned data
*/
- if (M64_HAS(HW_TRIPLE) && image->width % 8 == 0)
+ if (image->depth == 1 && M64_HAS(HW_TRIPLE) && image->width % 8
== 0)
pix_width |= DP_HOST_TRIPLE_EN;
}
@@ -382,7 +382,7 @@ void atyfb_imageblit(struct fb_info *inf
src_bytes = (((image->width * image->depth) + 7) / 8) * image->height;
/* manual triple each pixel */
- if (info->var.bits_per_pixel == 24 && !(pix_width & DP_HOST_TRIPLE_EN))
{
+ if (image->depth == 1 && info->var.bits_per_pixel == 24 && !(pix_width
& DP_HOST_TRIPLE_EN)) {
int inbit, outbit, mult24, byte_id_in_dword, width;
u8 *pbitmapin = (u8*)image->data, *pbitmapout;
u32 hostdword;
@@ -415,7 +415,7 @@ void atyfb_imageblit(struct fb_info *inf
}
}
wait_for_fifo(1, par);
- aty_st_le32(HOST_DATA0, hostdword, par);
+ aty_st_le32(HOST_DATA0, le32_to_cpu(hostdword), par);
}
} else {
u32 *pbitmap, dwords = (src_bytes + 3) / 4;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] mach64: fix console corruption in 24bpp mode
On Sun, 19 Aug 2018, Ville Syrjälä wrote: > On Fri, Aug 17, 2018 at 03:15:52PM -0400, Mikulas Patocka wrote: > > There's console font corruption when using the mach64 driver in 24bpp > > mode. > > > > In 24bpp mode, the mach64 accelerator is set up for 8-bpp mode (with > > horizontal width and stride multiplied by 3). In this mode, the > > accelerator can't even possibly support color expansion. Consquently, we > > have to use an unaccelerated function cfb_imageblit for color expansion. > > Hmm. I would think it should work just fine since we feed in each bit > three times and the hw 24bpp rotate thing should take care of selecting > the right component. > > - if (M64_HAS(HW_TRIPLE) && image->width % 8 == 0) > + if (M64_HAS(HW_TRIPLE) && width % 8 == 0) > pix_width |= DP_HOST_TRIPLE_EN; > perhaps? This doesn't have any effect (width is image->width * 3 and multiplication by 3 doesn't affect divisibility by 8). But I have looked at it deeper and I realized that we could fix the screen corruption and use the accelerator in 24bpp mode. I'll send the patches in following emails. There's endianity problem in the code that does manual bit triple in atyfb_imageblit (I am using this driver on sparc64). After fixing this, some of the corruption is fixed, but not all of it (there's still corruption when the screen is scrolled). Another bug is that the function atyfb_imageblit is reading the accelerator registers without waiting for the idle engine - so it reads some indetermediate value of some previous command. I reworked the accelerator functions so that they don't read the registers at all. After fixing these two bugs, I don't observe any corruption at all. I also noticed that loading the bitmap is inefficient - for every word written, the driver reads the fifo status register and these reads are slow and cannot be queued. I improved the function wait_for_fifo, so that if there's more space than we need, it remembers the surplus and it doesn't have to read the register next time. Mikulas___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] mach64: detect the dot clock divider correctly on sparc
On Sun, 19 Aug 2018, Ville Syrjälä wrote: > On Fri, Aug 17, 2018 at 03:19:37PM -0400, Mikulas Patocka wrote: > > On Sun Ultra 5, it happens that the dot clock is not set up properly for > > some videomodes. For example, if we set the videomode "r1024x768x60" in > > the firmware, Linux would incorrectly set a videomode with refresh rate > > 180Hz when booting (suprisingly, my LCD monitor can display it, although > > display quality is very low). > > > > The reason is this: Older mach64 cards set the divider in the register > > VCLK_POST_DIV. The register has four 2-bit fields (the field that is > > actually used is specified in the lowest two bits of the register > > CLOCK_CNTL). The 2 bits select divider "1, 2, 4, 8". On newer mach64 cards, > > there's another bit added - the top four bits of PLL_EXT_CNTL extend the > > divider selection, so we have possible dividers "1, 2, 4, 8, 3, 5, 6, 12". > > The Linux driver clears the top four bits of PLL_EXT_CNTL and never sets > > them, so it can work regardless if the card supports them. However, the > > sparc64 firmware may set these extended dividers during boot - and the > > mach64 driver detects incorrect dot clock in this case. > > > > This patch makes the driver read the additional divider bit from > > PLL_EXT_CNTL and calculate the initial refresh rate properly. > > > > Signed-off-by: Mikulas Patocka > > Cc: sta...@vger.kernel.org > > > > --- > > drivers/video/fbdev/aty/atyfb.h |3 ++- > > drivers/video/fbdev/aty/atyfb_base.c |7 --- > > drivers/video/fbdev/aty/mach64_ct.c | 10 +- > > 3 files changed, 11 insertions(+), 9 deletions(-) > > > > Index: linux-stable/drivers/video/fbdev/aty/atyfb.h > > === > > --- linux-stable.orig/drivers/video/fbdev/aty/atyfb.h 2018-08-13 > > 21:12:11.0 +0200 > > +++ linux-stable/drivers/video/fbdev/aty/atyfb.h2018-08-13 > > 21:17:14.0 +0200 > > @@ -333,6 +333,8 @@ extern const struct aty_pll_ops aty_pll_ > > extern void aty_set_pll_ct(const struct fb_info *info, const union aty_pll > > *pll); > > extern u8 aty_ld_pll_ct(int offset, const struct atyfb_par *par); > > > > +extern const u8 aty_postdividers[8]; > > + > > > > /* > > * Hardware cursor support > > @@ -359,7 +361,6 @@ static inline void wait_for_idle(struct > > > > extern void aty_reset_engine(const struct atyfb_par *par); > > extern void aty_init_engine(struct atyfb_par *par, struct fb_info *info); > > -extern u8 aty_ld_pll_ct(int offset, const struct atyfb_par *par); > > > > void atyfb_copyarea(struct fb_info *info, const struct fb_copyarea *area); > > void atyfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect); > > Index: linux-stable/drivers/video/fbdev/aty/atyfb_base.c > > === > > --- linux-stable.orig/drivers/video/fbdev/aty/atyfb_base.c 2018-08-13 > > 21:12:11.0 +0200 > > +++ linux-stable/drivers/video/fbdev/aty/atyfb_base.c 2018-08-13 > > 21:22:23.0 +0200 > > @@ -3087,17 +3087,18 @@ static int atyfb_setup_sparc(struct pci_ > > /* > > * PLL Reference Divider M: > > */ > > - M = pll_regs[2]; > > + M = pll_regs[PLL_REF_DIV]; > > > > /* > > * PLL Feedback Divider N (Dependent on CLOCK_CNTL): > > */ > > - N = pll_regs[7 + (clock_cntl & 3)]; > > + N = pll_regs[VCLK0_FB_DIV + (clock_cntl & 3)]; > > > > /* > > * PLL Post Divider P (Dependent on CLOCK_CNTL): > > */ > > - P = 1 << (pll_regs[6] >> ((clock_cntl & 3) << 1)); > > + P = aty_postdividers[((pll_regs[VCLK_POST_DIV] >> ((clock_cntl > > & 3) << 1)) & 3) | > > +((pll_regs[PLL_EXT_CNTL] >> (2 + > > (clock_cntl & 3))) & 4)]; > > > > /* > > * PLL Divider Q: > > Index: linux-stable/drivers/video/fbdev/aty/mach64_ct.c > > === > > --- linux-stable.orig/drivers/video/fbdev/aty/mach64_ct.c 2018-08-13 > > 21:12:11.0 +0200 > > +++ linux-stable/drivers/video/fbdev/aty/mach64_ct.c2018-08-13 > > 21:35:32.0 +0200 > >
[PATCH] atyfb: fix debugging printks
This patch fixes the debugging printks. Use pr_cont, so that the lines are
not broken up. Use printk when starting a new line (a long string of
pr_cont's without any printks causes missing characters in the console
output on sparc).
Signed-off-by: Mikulas Patocka
---
drivers/video/fbdev/aty/atyfb_base.c | 36 +++
1 file changed, 20 insertions(+), 16 deletions(-)
Index: linux-stable/drivers/video/fbdev/aty/atyfb_base.c
===
--- linux-stable.orig/drivers/video/fbdev/aty/atyfb_base.c 2018-08-15
19:45:56.0 +0200
+++ linux-stable/drivers/video/fbdev/aty/atyfb_base.c 2018-08-15
20:46:52.0 +0200
@@ -1480,24 +1480,28 @@ static int atyfb_set_par(struct fb_info
base = 0x2000;
printk("debug atyfb: Mach64 non-shadow register values:");
for (i = 0; i < 256; i = i+4) {
- if (i % 16 == 0)
- printk("\ndebug atyfb: 0x%04X: ", base + i);
- printk(" %08X", aty_ld_le32(i, par));
+ if (i % 16 == 0) {
+ pr_cont("\n");
+ printk("debug atyfb: 0x%04X: ", base + i);
+ }
+ pr_cont(" %08X", aty_ld_le32(i, par));
}
- printk("\n\n");
+ pr_cont("\n\n");
#ifdef CONFIG_FB_ATY_CT
/* PLL registers */
base = 0x00;
printk("debug atyfb: Mach64 PLL register values:");
for (i = 0; i < 64; i++) {
- if (i % 16 == 0)
- printk("\ndebug atyfb: 0x%02X: ", base + i);
+ if (i % 16 == 0) {
+ pr_cont("\n");
+ printk("debug atyfb: 0x%02X: ", base + i);
+ }
if (i % 4 == 0)
- printk(" ");
- printk("%02X", aty_ld_pll_ct(i, par));
+ pr_cont(" ");
+ pr_cont("%02X", aty_ld_pll_ct(i, par));
}
- printk("\n\n");
+ pr_cont("\n\n");
#endif /* CONFIG_FB_ATY_CT */
#ifdef CONFIG_FB_ATY_GENERIC_LCD
@@ -1509,19 +1513,19 @@ static int atyfb_set_par(struct fb_info
for (i = 0; i <= POWER_MANAGEMENT; i++) {
if (i == EXT_VERT_STRETCH)
continue;
- printk("\ndebug atyfb: 0x%04X: ",
+ pr_cont("\ndebug atyfb: 0x%04X: ",
lt_lcd_regs[i]);
- printk(" %08X", aty_ld_lcd(i, par));
+ pr_cont(" %08X", aty_ld_lcd(i, par));
}
} else {
for (i = 0; i < 64; i++) {
if (i % 4 == 0)
- printk("\ndebug atyfb: 0x%02X: ",
+ pr_cont("\ndebug atyfb: 0x%02X: ",
base + i);
- printk(" %08X", aty_ld_lcd(i, par));
+ pr_cont(" %08X", aty_ld_lcd(i, par));
}
}
- printk("\n\n");
+ pr_cont("\n\n");
}
#endif /* CONFIG_FB_ATY_GENERIC_LCD */
}
@@ -2597,8 +2601,8 @@ static int aty_init(struct fb_info *info
aty_ld_le32(DSP_ON_OFF, par),
aty_ld_le32(CLOCK_CNTL, par));
for (i = 0; i < 40; i++)
- printk(" %02x", aty_ld_pll_ct(i, par));
- printk("\n");
+ pr_cont(" %02x", aty_ld_pll_ct(i, par));
+ pr_cont("\n");
}
#endif
if (par->pll_ops->init_pll)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] mach64: detect the dot clock divider correctly on sparc
On Sun Ultra 5, it happens that the dot clock is not set up properly for
some videomodes. For example, if we set the videomode "r1024x768x60" in
the firmware, Linux would incorrectly set a videomode with refresh rate
180Hz when booting (suprisingly, my LCD monitor can display it, although
display quality is very low).
The reason is this: Older mach64 cards set the divider in the register
VCLK_POST_DIV. The register has four 2-bit fields (the field that is
actually used is specified in the lowest two bits of the register
CLOCK_CNTL). The 2 bits select divider "1, 2, 4, 8". On newer mach64 cards,
there's another bit added - the top four bits of PLL_EXT_CNTL extend the
divider selection, so we have possible dividers "1, 2, 4, 8, 3, 5, 6, 12".
The Linux driver clears the top four bits of PLL_EXT_CNTL and never sets
them, so it can work regardless if the card supports them. However, the
sparc64 firmware may set these extended dividers during boot - and the
mach64 driver detects incorrect dot clock in this case.
This patch makes the driver read the additional divider bit from
PLL_EXT_CNTL and calculate the initial refresh rate properly.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/aty/atyfb.h |3 ++-
drivers/video/fbdev/aty/atyfb_base.c |7 ---
drivers/video/fbdev/aty/mach64_ct.c | 10 +-
3 files changed, 11 insertions(+), 9 deletions(-)
Index: linux-stable/drivers/video/fbdev/aty/atyfb.h
===
--- linux-stable.orig/drivers/video/fbdev/aty/atyfb.h 2018-08-13
21:12:11.0 +0200
+++ linux-stable/drivers/video/fbdev/aty/atyfb.h2018-08-13
21:17:14.0 +0200
@@ -333,6 +333,8 @@ extern const struct aty_pll_ops aty_pll_
extern void aty_set_pll_ct(const struct fb_info *info, const union aty_pll
*pll);
extern u8 aty_ld_pll_ct(int offset, const struct atyfb_par *par);
+extern const u8 aty_postdividers[8];
+
/*
* Hardware cursor support
@@ -359,7 +361,6 @@ static inline void wait_for_idle(struct
extern void aty_reset_engine(const struct atyfb_par *par);
extern void aty_init_engine(struct atyfb_par *par, struct fb_info *info);
-extern u8 aty_ld_pll_ct(int offset, const struct atyfb_par *par);
void atyfb_copyarea(struct fb_info *info, const struct fb_copyarea *area);
void atyfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect);
Index: linux-stable/drivers/video/fbdev/aty/atyfb_base.c
===
--- linux-stable.orig/drivers/video/fbdev/aty/atyfb_base.c 2018-08-13
21:12:11.0 +0200
+++ linux-stable/drivers/video/fbdev/aty/atyfb_base.c 2018-08-13
21:22:23.0 +0200
@@ -3087,17 +3087,18 @@ static int atyfb_setup_sparc(struct pci_
/*
* PLL Reference Divider M:
*/
- M = pll_regs[2];
+ M = pll_regs[PLL_REF_DIV];
/*
* PLL Feedback Divider N (Dependent on CLOCK_CNTL):
*/
- N = pll_regs[7 + (clock_cntl & 3)];
+ N = pll_regs[VCLK0_FB_DIV + (clock_cntl & 3)];
/*
* PLL Post Divider P (Dependent on CLOCK_CNTL):
*/
- P = 1 << (pll_regs[6] >> ((clock_cntl & 3) << 1));
+ P = aty_postdividers[((pll_regs[VCLK_POST_DIV] >> ((clock_cntl
& 3) << 1)) & 3) |
+((pll_regs[PLL_EXT_CNTL] >> (2 +
(clock_cntl & 3))) & 4)];
/*
* PLL Divider Q:
Index: linux-stable/drivers/video/fbdev/aty/mach64_ct.c
===
--- linux-stable.orig/drivers/video/fbdev/aty/mach64_ct.c 2018-08-13
21:12:11.0 +0200
+++ linux-stable/drivers/video/fbdev/aty/mach64_ct.c2018-08-13
21:35:32.0 +0200
@@ -115,7 +115,7 @@ static void aty_st_pll_ct(int offset, u8
*/
#define Maximum_DSP_PRECISION 7
-static u8 postdividers[] = {1,2,4,8,3};
+const u8 aty_postdividers[8] = {1,2,4,8,3,5,6,12};
static int aty_dsp_gt(const struct fb_info *info, u32 bpp, struct pll_ct *pll)
{
@@ -222,7 +222,7 @@ static int aty_valid_pll_ct(const struct
pll->vclk_post_div += (q < 64*8);
pll->vclk_post_div += (q < 32*8);
}
- pll->vclk_post_div_real = postdividers[pll->vclk_post_div];
+ pll->vclk_post_div_real = aty_postdividers[pll->vclk_post_div];
//pll->vclk_post_div <<= 6;
pll->vclk_fb_div = q * pll->vclk_post_div_real / 8;
pllvclk = (100 * 2 * pll->vclk_fb_div) /
@@ -513,7 +513,7 @@ static int aty_init_pll_ct(const struct
u8 mclk_fb_div, pll_ext_cntl;
pll->ct.pll_ref_div = a
[PATCH] mach64: fix console corruption in 24bpp mode
There's console font corruption when using the mach64 driver in 24bpp
mode.
In 24bpp mode, the mach64 accelerator is set up for 8-bpp mode (with
horizontal width and stride multiplied by 3). In this mode, the
accelerator can't even possibly support color expansion. Consquently, we
have to use an unaccelerated function cfb_imageblit for color expansion.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/aty/mach64_accel.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux-stable/drivers/video/fbdev/aty/mach64_accel.c
===
--- linux-stable.orig/drivers/video/fbdev/aty/mach64_accel.c2018-04-20
18:11:01.0 +0200
+++ linux-stable/drivers/video/fbdev/aty/mach64_accel.c 2018-08-13
17:37:04.0 +0200
@@ -291,7 +291,8 @@ void atyfb_imageblit(struct fb_info *inf
if (!image->width || !image->height)
return;
if (!par->accel_flags ||
- (image->depth != 1 && info->var.bits_per_pixel != image->depth)) {
+ (image->depth != 1 && info->var.bits_per_pixel != image->depth) ||
+ (image->depth == 1 && info->var.bits_per_pixel == 24)) {
cfb_imageblit(info, image);
return;
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Sleeping from invalid context in udlfb
On Wed, 1 Aug 2018, Geert Uytterhoeven wrote:
> Hi Mikulas,
>
> On Wed, Aug 1, 2018 at 12:59 PM Mikulas Patocka wrote:
> > On Wed, 1 Aug 2018, Geert Uytterhoeven wrote:
> > > On Tue, Jul 31, 2018 at 5:23 PM Mikulas Patocka
> > > wrote:
> > > > BTW when using the udlfb driver as a console, I've got this warning.
> > > > vt_console_print takes a spinlock and then calls the framebuffer driver
> > > > that sleeps.
> > > >
> > > > The question is - whose fault is this? Could the console code somehow be
> > > > told to print characters without holding a spinlock? Or does it mean
> > > > that
> > > > framebuffer drivers can't sleep?
> > > >
> > > > udlfb communicates through USB, so the sleeping is inevitable.
> > > >
> > > > Mikulas
> > > >
> > > >
> > > > BUG: sleeping function called from invalid context at mm/slab.h:421
> > > > in_atomic(): 1, irqs_disabled(): 0, pid: 430, name: kworker/2:3
> > > > 6 locks held by kworker/2:3/430:
> > > > #0: 1301127e ( (wq_completion)"events"){} , at:
> > > > process_one_work+0x17c/0x3a8
> > > > #1: beacc951 (
> > > > (work_completion)(&(>init_framebuffer_work)->work)){} , at:
> > > > process_one_work+0x17c/0x3a8
> > > > #2: a402f826 ( registration_lock){} , at:
> > > > register_framebuffer+0x28/0x2c0 [fb]
> > > > #3: 21cbe902 ( console_lock){} , at:
> > > > register_framebuffer+0x258/0x2c0 [fb]
> > > > #4: 96d51735 ( console_owner){} , at:
> > > > console_unlock+0x174/0x500
> > > > #5: faa7f206 ( printing_lock){} , at:
> > > > vt_console_print+0x60/0x3a0
> > > > Preemption disabled at: [] vt_console_print+0x60/0x3a0
> > > > CPU: 2 PID: 430 Comm: kworker/2:3 Not tainted 4.17.10-debug #3
> > > > Hardware name: Marvell Armada 8040 MacchiatoBin/Armada 8040
> > > > MacchiatoBin, BIOS EDK II Jul 30 2018
> > > > Workqueue: events dlfb_init_framebuffer_work [udlfb]
> > > > Call trace:
> > > > dump_backtrace+0x0/0x150
> > > > show_stack+0x14/0x20
> > > > dump_stack+0x8c/0xac
> > > > ___might_sleep+0x140/0x170
> > > > __might_sleep+0x50/0x88
> > > > __kmalloc+0x1b0/0x270
> > > > xhci_urb_enqueue+0xa8/0x460 [xhci_hcd]
> > > > usb_hcd_submit_urb+0xc0/0x998 [usbcore]
> > > > usb_submit_urb+0x1e0/0x518 [usbcore]
> > > > dlfb_submit_urb+0x38/0x98 [udlfb]
> > > > dlfb_handle_damage.isra.4+0x1e0/0x210 [udlfb]
> > > > dlfb_ops_imageblit+0x28/0x38 [udlfb]
> > > > soft_cursor+0x15c/0x1d8 [fb]
> > > > bit_cursor+0x324/0x510 [fb]
> > > > fbcon_cursor+0x144/0x1a0 [fb]
> > > > hide_cursor+0x38/0xa0
> > > > vt_console_print+0x334/0x3a0
> > > > console_unlock+0x274/0x500
> > > > register_framebuffer+0x22c/0x2c0 [fb]
> > > > dlfb_init_framebuffer_work+0x1ec/0x2fc [udlfb]
> > > > process_one_work+0x1e8/0x3a8
> > > > worker_thread+0x44/0x418
> > > > kthread+0x11c/0x120
> > > > ret_from_fork+0x10/0x18
> > >
> > > This is sort of expected: you cannot do USB transfers from printk().
> > >
> > > Gr{oetje,eeting}s,
> > >
> > > Geert
> >
> > So, should there be a framebuffer flag that prevents the console from
> > binding to it?
> >
> > If I start the kernel with "console=ttyS0,115200", it doesn't try to bind
> > to the udlfb driver, but if I start it without this flag, it does and
> > crashes :-(
>
> Your frame buffer driver should offload tasks that may sleep to e.g. a
> workqueue.
>
> Gr{oetje,eeting}s,
>
> Geert
I can try to do this - but - taking a spinlock and copying 8MB framebuffer
would damage scheduling latency even for PCI framebuffer drivers.
Mikulas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Sleeping from invalid context in udlfb
On Wed, 1 Aug 2018, Geert Uytterhoeven wrote:
> Hi Mikulas,
>
> On Tue, Jul 31, 2018 at 5:23 PM Mikulas Patocka wrote:
> > BTW when using the udlfb driver as a console, I've got this warning.
> > vt_console_print takes a spinlock and then calls the framebuffer driver
> > that sleeps.
> >
> > The question is - whose fault is this? Could the console code somehow be
> > told to print characters without holding a spinlock? Or does it mean that
> > framebuffer drivers can't sleep?
> >
> > udlfb communicates through USB, so the sleeping is inevitable.
> >
> > Mikulas
> >
> >
> > BUG: sleeping function called from invalid context at mm/slab.h:421
> > in_atomic(): 1, irqs_disabled(): 0, pid: 430, name: kworker/2:3
> > 6 locks held by kworker/2:3/430:
> > #0: 1301127e ( (wq_completion)"events"){} , at:
> > process_one_work+0x17c/0x3a8
> > #1: beacc951 (
> > (work_completion)(&(>init_framebuffer_work)->work)){} , at:
> > process_one_work+0x17c/0x3a8
> > #2: a402f826 ( registration_lock){} , at:
> > register_framebuffer+0x28/0x2c0 [fb]
> > #3: 21cbe902 ( console_lock){} , at:
> > register_framebuffer+0x258/0x2c0 [fb]
> > #4: 96d51735 ( console_owner){} , at:
> > console_unlock+0x174/0x500
> > #5: faa7f206 ( printing_lock){} , at:
> > vt_console_print+0x60/0x3a0
> > Preemption disabled at: [] vt_console_print+0x60/0x3a0
> > CPU: 2 PID: 430 Comm: kworker/2:3 Not tainted 4.17.10-debug #3
> > Hardware name: Marvell Armada 8040 MacchiatoBin/Armada 8040 MacchiatoBin,
> > BIOS EDK II Jul 30 2018
> > Workqueue: events dlfb_init_framebuffer_work [udlfb]
> > Call trace:
> > dump_backtrace+0x0/0x150
> > show_stack+0x14/0x20
> > dump_stack+0x8c/0xac
> > ___might_sleep+0x140/0x170
> > __might_sleep+0x50/0x88
> > __kmalloc+0x1b0/0x270
> > xhci_urb_enqueue+0xa8/0x460 [xhci_hcd]
> > usb_hcd_submit_urb+0xc0/0x998 [usbcore]
> > usb_submit_urb+0x1e0/0x518 [usbcore]
> > dlfb_submit_urb+0x38/0x98 [udlfb]
> > dlfb_handle_damage.isra.4+0x1e0/0x210 [udlfb]
> > dlfb_ops_imageblit+0x28/0x38 [udlfb]
> > soft_cursor+0x15c/0x1d8 [fb]
> > bit_cursor+0x324/0x510 [fb]
> > fbcon_cursor+0x144/0x1a0 [fb]
> > hide_cursor+0x38/0xa0
> > vt_console_print+0x334/0x3a0
> > console_unlock+0x274/0x500
> > register_framebuffer+0x22c/0x2c0 [fb]
> > dlfb_init_framebuffer_work+0x1ec/0x2fc [udlfb]
> > process_one_work+0x1e8/0x3a8
> > worker_thread+0x44/0x418
> > kthread+0x11c/0x120
> > ret_from_fork+0x10/0x18
>
> This is sort of expected: you cannot do USB transfers from printk().
>
> Gr{oetje,eeting}s,
>
> Geert
So, should there be a framebuffer flag that prevents the console from
binding to it?
If I start the kernel with "console=ttyS0,115200", it doesn't try to bind
to the udlfb driver, but if I start it without this flag, it does and
crashes :-(
Mikulas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Sleeping from invalid context in udlfb
BTW when using the udlfb driver as a console, I've got this warning.
vt_console_print takes a spinlock and then calls the framebuffer driver
that sleeps.
The question is - whose fault is this? Could the console code somehow be
told to print characters without holding a spinlock? Or does it mean that
framebuffer drivers can't sleep?
udlfb communicates through USB, so the sleeping is inevitable.
Mikulas
BUG: sleeping function called from invalid context at mm/slab.h:421
in_atomic(): 1, irqs_disabled(): 0, pid: 430, name: kworker/2:3
6 locks held by kworker/2:3/430:
#0: 1301127e ( (wq_completion)"events"){} , at:
process_one_work+0x17c/0x3a8
#1: beacc951 (
(work_completion)(&(>init_framebuffer_work)->work)){} , at:
process_one_work+0x17c/0x3a8
#2: a402f826 ( registration_lock){} , at:
register_framebuffer+0x28/0x2c0 [fb]
#3: 21cbe902 ( console_lock){} , at:
register_framebuffer+0x258/0x2c0 [fb]
#4: 96d51735 ( console_owner){} , at: console_unlock+0x174/0x500
#5: faa7f206 ( printing_lock){} , at: vt_console_print+0x60/0x3a0
Preemption disabled at: [] vt_console_print+0x60/0x3a0
CPU: 2 PID: 430 Comm: kworker/2:3 Not tainted 4.17.10-debug #3
Hardware name: Marvell Armada 8040 MacchiatoBin/Armada 8040 MacchiatoBin, BIOS
EDK II Jul 30 2018
Workqueue: events dlfb_init_framebuffer_work [udlfb]
Call trace:
dump_backtrace+0x0/0x150
show_stack+0x14/0x20
dump_stack+0x8c/0xac
___might_sleep+0x140/0x170
__might_sleep+0x50/0x88
__kmalloc+0x1b0/0x270
xhci_urb_enqueue+0xa8/0x460 [xhci_hcd]
usb_hcd_submit_urb+0xc0/0x998 [usbcore]
usb_submit_urb+0x1e0/0x518 [usbcore]
dlfb_submit_urb+0x38/0x98 [udlfb]
dlfb_handle_damage.isra.4+0x1e0/0x210 [udlfb]
dlfb_ops_imageblit+0x28/0x38 [udlfb]
soft_cursor+0x15c/0x1d8 [fb]
bit_cursor+0x324/0x510 [fb]
fbcon_cursor+0x144/0x1a0 [fb]
hide_cursor+0x38/0xa0
vt_console_print+0x334/0x3a0
console_unlock+0x274/0x500
register_framebuffer+0x22c/0x2c0 [fb]
dlfb_init_framebuffer_work+0x1ec/0x2fc [udlfb]
process_one_work+0x1e8/0x3a8
worker_thread+0x44/0x418
kthread+0x11c/0x120
ret_from_fork+0x10/0x18
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] udlfb: handle unplug properly
The udlfb driver maintained an open count and cleaned up itself when the
count reached zero. But the console is also counted in the reference count
- so, if the user unplugged the device, the open count would not drop to
zero and the driver stayed loaded with console attached. If the user
re-plugged the adapter, it would create a device /dev/fb1, show green
screen and the access to the console would be lost.
The framebuffer subsystem has reference counting on its own - in order to
fix the unplug bug, we rely the framebuffer reference counting. When the
user unplugs the adapter, we call unregister_framebuffer unconditionally.
unregister_framebuffer will unbind the console, wait until all users stop
using the framebuffer and then call the fb_destroy method. The fb_destroy
cleans up the USB driver.
This patch makes the following changes:
* Drop dlfb->kref and rely on implicit framebuffer reference counting
instead.
* dlfb_usb_disconnect calls unregister_framebuffer, the rest of driver
cleanup is done in the function dlfb_ops_destroy. dlfb_ops_destroy will
be called by the framebuffer subsystem when no processes have the
framebuffer open or mapped.
* We don't use workqueue during initialization, but initialize directly
from dlfb_usb_probe. The workqueue could race with dlfb_usb_disconnect
and this racing would produce various kinds of memory corruption.
* We use usb_get_dev and usb_put_dev to make sure that the USB subsystem
doesn't free the device under us.
Signed-off-by: Mikulas Patocka
---
drivers/video/fbdev/udlfb.c | 141 +++-
include/video/udlfb.h |3
2 files changed, 37 insertions(+), 107 deletions(-)
Index: linux-4.17.10/drivers/video/fbdev/udlfb.c
===
--- linux-4.17.10.orig/drivers/video/fbdev/udlfb.c 2018-07-27
11:41:12.0 +0200
+++ linux-4.17.10/drivers/video/fbdev/udlfb.c 2018-07-27 14:44:47.0
+0200
@@ -916,8 +916,6 @@ static int dlfb_ops_open(struct fb_info
dlfb->fb_count++;
- kref_get(>kref);
-
if (fb_defio && (info->fbdefio == NULL)) {
/* enable defio at last moment if not disabled by client */
@@ -940,14 +938,17 @@ static int dlfb_ops_open(struct fb_info
return 0;
}
-/*
- * Called when all client interfaces to start transactions have been disabled,
- * and all references to our device instance (dlfb_data) are released.
- * Every transaction must have a reference, so we know are fully spun down
- */
-static void dlfb_free(struct kref *kref)
+static void dlfb_ops_destroy(struct fb_info *info)
{
- struct dlfb_data *dlfb = container_of(kref, struct dlfb_data, kref);
+ struct dlfb_data *dlfb = info->par;
+
+ if (info->cmap.len != 0)
+ fb_dealloc_cmap(>cmap);
+ if (info->monspecs.modedb)
+ fb_destroy_modedb(info->monspecs.modedb);
+ vfree(info->screen_base);
+
+ fb_destroy_modelist(>modelist);
while (!list_empty(>deferred_free)) {
struct dlfb_deferred_free *d =
list_entry(dlfb->deferred_free.next, struct dlfb_deferred_free, list);
@@ -957,40 +958,13 @@ static void dlfb_free(struct kref *kref)
}
vfree(dlfb->backing_buffer);
kfree(dlfb->edid);
+ usb_put_dev(dlfb->udev);
kfree(dlfb);
-}
-
-static void dlfb_free_framebuffer(struct dlfb_data *dlfb)
-{
- struct fb_info *info = dlfb->info;
-
- if (info) {
- unregister_framebuffer(info);
-
- if (info->cmap.len != 0)
- fb_dealloc_cmap(>cmap);
- if (info->monspecs.modedb)
- fb_destroy_modedb(info->monspecs.modedb);
- vfree(info->screen_base);
-
- fb_destroy_modelist(>modelist);
-
- dlfb->info = NULL;
-
- /* Assume info structure is freed after this point */
- framebuffer_release(info);
- }
- /* ref taken in probe() as part of registering framebfufer */
- kref_put(>kref, dlfb_free);
+ /* Assume info structure is freed after this point */
+ framebuffer_release(info);
}
-static void dlfb_free_framebuffer_work(struct work_struct *work)
-{
- struct dlfb_data *dlfb = container_of(work, struct dlfb_data,
-free_framebuffer_work.work);
- dlfb_free_framebuffer(dlfb);
-}
/*
* Assumes caller is holding info->lock mutex (for open and release at least)
*/
@@ -1000,10 +974,6 @@ static int dlfb_ops_release(struct fb_in
dlfb->fb_count--;
- /* We can't free fb_info here - fbmem will touch it when we return */
- if (dlfb->virtualized && (dlfb->fb_count == 0))
- schedule_delayed_work(>free_framebuffer_work, HZ);
-
if ((dlfb->
Re: [PATCH] fb: fix lost console when the user unplugs a USB adapter
On Wed, 25 Jul 2018, Bartlomiej Zolnierkiewicz wrote:
> > Can unregister_framebuffer() be called when /dev/fb0 is open as a file
> > handle and/or mapped to some process?
>
> It should be OK.
>
> > > Moreover the dlfb <-> fb_info locking scheme seems to be reversed
> > > (+racy) as it is dlfb that should control lifetime of fb_info, then
> > > in dlfb_free() we should just call framebuffer_release() etc.
> >
> > How should in your opinion framebuffer destruction work?
> >
> > Should the driver count the number of users and call
> > unregister_framebuffer() when it drops to zero?
> >
> > Or should the driver call unregister_framebuffer() unconditionally when
> > the device is unplugged and destroy the device in the "fb_destroy"
> > callback? (fb_destroy seems to be called by the framebuffer subsystem when
> > the open count reaches zero)
>
> The driver should call unregister_framebuffer() unconditionally in
> dlfb_usb_disconnect() (instead of calling unlink_framebuffer()).
I reworked the udlfb driver to call unregister_framebuffer unconditionally
and destroy the device from the fb_destroy method and it works very well.
I tried to unplug it at various times and it didn't result in any crashes
or warnings.
I'll send the patch in next email.
> Anyway it seems that this would require major reworking of the driver and
> I think that it would be better to put efforts into fixing udl-kms driver
> instead. For now I have queued your patch (with __unregister_framebuffer()
> change to keep the old behavior for non-udlfb drivers) for v4.19 (patch
> attached at the end of this mail).
Could someone describe what is the architecturely proper way to unload a
KMS driver?
udl_usb_disconnect calls these functions
drm_kms_helper_poll_disable(dev);
udl_fbdev_unplug(dev);
udl_drop_usb(dev);
drm_dev_unplug(dev);
- and if crashes if the device is unplugged while Xserver is running.
And it results in a warning "WARNING: CPU: 0 PID: 21685 at
drivers/gpu/drm/drm_mode_config.c:473 drm_mode_config_cleanup+0x294/0x2b8
[drm]" if the device is unplugged while only console is in use.
> > I tested matrox PCI framebuffer driver on an old computer - and it suffers
> > from the same problem as udlfb. The matrox driver keeps the open count and
> > destroys itself when the open count reaches zero - but the console that is
> > bound to the driver prevents the open count from reaching zero - so if I
> > unbind the PCI device in sysfs, it does nothing and the console is still
> > active and works.
>
> matroxfb is a not a good reference driver as it also defers the call to
> unregister_framebuffer() when the device is unplugged:
>
> static void matroxfb_remove(struct matrox_fb_info *minfo, int dummy)
> {
> ...
> minfo->dead = 1;
> if (minfo->usecount) {
> /* destroy it later */
> --> return;
> }
> matroxfb_unregister_device(minfo);
> unregister_framebuffer(>fbcon);
> ...
> }
I think that for PCI framebuffer drivers, there's no correct way how to
unload them correctly - so the framebuffer subsystem should prevent PCI
unbind.
If the user unbinds the device, then what?
- either you destroy the framebuffer immediatelly and you'll get crashes
if it is used by some programs
- or you delay destroying the framebuffer - but then, the unbound device
may be re-bound to a different instance of the driver (or a different
driver) and these two instances would clash accessing the videoram and
regsters simultaneously
BTW. only 5 framebuffer drivers currently use the fb_destroy - so most of
them are destroyed improperly.
Mikulas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fb: fix lost console when the user unplugs a USB adapter
On Wed, 4 Jul 2018, Noralf Trønnes wrote:
> AFAIU calling unregister_framebuffer() with open fd's is just fine as
> long as fb_info with buffers stay intact. All it does is to remove the
> fbX from userspace. Cleanup can be done in fb_ops->fb_destroy.
>
> I have been working on generic fbdev emulation for DRM [1] and I did a
> test now to see what would happen if I did unbind the driver from the
> device. It worked as expected if I didn't have another fbdev present,
> but if there is an fb0 and I remove fb1 with a console on it, I would
> sometimes get crashes, often with a call to cursor_timer_handler() in
> the traceback.
>
> I think there's index mixup in fbcon_fb_unbind(), at least this change
> seems to solve the immediate problem:
>
> diff --git a/drivers/video/fbdev/core/fbcon.c
> b/drivers/video/fbdev/core/fbcon.c
> index 5fb156bdcf4e..271b9b988b73 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -3066,7 +3072,7 @@ static int fbcon_fb_unbind(int idx)
> for (i = first_fb_vc; i <= last_fb_vc; i++) {
> if (con2fb_map[i] != idx &&
> con2fb_map[i] != -1) {
> - new_idx = i;
> + new_idx = con2fb_map[i];
> break;
> }
> }
Reviewed-by: Mikulas Patocka
Yes - that's a good point. i is virtual console index and it is assigned
to new_idx and new_idx is interpreted as a framebuffer device index.
> I haven't got time to follow up on this now, but making sure DRM generic
> fbdev emulation device unplug works is on my TODO.
>
> BTW, I believe the udl drm driver should be able to use the generic fbdev
> emulation if it had a drm_driver->gem_prime_vmap hook. It uses a shadow
> buffer which would also make fbdev mmap work for udl. (shmem buffers and
> fbdev deferred I/O doesn't work together since they both use
> page->lru/mapping)
I have sent patch for this:
https://patchwork.kernel.org/patch/10445393/
Mikulas___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fb: fix lost console when the user unplugs a USB adapter
On Wed, 4 Jul 2018, Bartlomiej Zolnierkiewicz wrote: > On Tuesday, July 03, 2018 01:18:57 PM Mikulas Patocka wrote: > > > > On Tue, 3 Jul 2018, Bartlomiej Zolnierkiewicz wrote: > > > > > Hi, > > > > > > On Sunday, June 03, 2018 11:46:29 AM Mikulas Patocka wrote: > > > > I have a USB display adapter using the udlfb driver and I use it on an > > > > ARM > > > > board that doesn't have any graphics card. When I plug the adapter in, > > > > the > > > > console is properly displayed, however when I unplug and re-plug the > > > > adapter, the console is not displayed and I can't access it until I > > > > reboot > > > > the board. > > > > > > > > The reason is this: > > > > When the adapter is unplugged, dlfb_usb_disconnect calls > > > > unlink_framebuffer, then it waits until the reference count drops to > > > > zero > > > > and then it deallocates the framebuffer. However, the console that is > > > > attached to the framebuffer device keeps the reference count non-zero, > > > > so > > > > the framebuffer device is never destroyed. When the USB adapter is > > > > plugged > > > > again, it creates a new device /dev/fb1 and the console is not attached > > > > to > > > > it. > > > > > > > > This patch fixes the bug by unbinding the console from > > > > unlink_framebuffer. > > > > The code to unbind the console is moved from do_unregister_framebuffer > > > > to > > > > a function unbind_console. When the console is unbound, the reference > > > > count drops to zero and the udlfb driver frees the framebuffer. When the > > > > adapter is plugged back, a new framebuffer is created and the console is > > > > attached to it. > > > > > > > > Signed-off-by: Mikulas Patocka > > > > Cc: sta...@vger.kernel.org > > > > > > After this change unbind_console() will be called twice in the standard > > > framebuffer unregister path: > > > > > > - first time, directly by do_unregister_framebuffer() > > > > > > - second time, indirectly by > > > do_unregister_framebuffer()->unlink_framebuffer() > > > > > > This doesn't look correctly. > > > > unbind_console calls the FB_EVENT_FB_UNBIND notifier, FB_EVENT_FB_UNBIND > > goes to the function fbcon_fb_unbind and fbcon_fb_unbind checks if the > > console is bound to the framebuffer for which unbind is requested. So a > > double call won't cause any trouble. > > Even if it works okay currently it is not a best design to send duplicate > events - especially since this can be easily avoided (for non-udlfb users) > by: > > - renaming "vanilla" unlink_framebuffer() to __unlink_framebuffer() > > - converting do_unregister_framebuffer() to use __unlink_framebuffer() > > - adding "new" unlink_framebuffer() that will also call unbind_console() > > > > Also why can't udlfb just use unregister_framebuffer() like all other > > > drivers (it uses unlink_framebuffer() and it is the only user of this > > > helper)? > > > > It uses unregister_framebuffer() - but - unregister_framebuffer() may only > > be called when the open count of the framebuffer is zero. So, the udlfb > > driver waits until the open count drops to zero and then calls > > unregister_framebuffer(). > > > > But the console subsystem keeps the framebuffer open - which means that if > > user use unplugs the USB adapter, the open count won't drop to zero > > (because the console is bound to it) - which means that > > unregister_framebuffer() will not be called. > > Is it a really the console subsystem and not the user-space keeping > /dev/fb0 (with console binded to fb0) opened after the USB device > vanishes? Yes - I unplugged the adapter without Xserver running and without any other framebuffer application running - the console keeps it open. > After re-plugging the USB device /dev/fb0 stays and /dev/fb1 > appears, right? The file /dev/fb0 is deleted (because dlfb_usb_disconnect calls unlink_framebuffer), but it is kept in the kernel. When I re-plug the adapter, /dev/fb1 is created but the console is still bound to /dev/fb0. When the adapter is re-plugged, it shows just a green screen. > I also mean that unregister_framebuffer() should be called instead > unlink_framebuffer(), not additionally some time later as it is done > currently. Can unregister_framebuffer()
Re: [PATCH] fb: fix lost console when the user unplugs a USB adapter
On Wed, 4 Jul 2018, Daniel Vetter wrote: > On Sun, Jun 03, 2018 at 11:46:29AM -0400, Mikulas Patocka wrote: > > I have a USB display adapter using the udlfb driver and I use it on an ARM > > board that doesn't have any graphics card. When I plug the adapter in, the > > console is properly displayed, however when I unplug and re-plug the > > adapter, the console is not displayed and I can't access it until I reboot > > the board. > > > > The reason is this: > > When the adapter is unplugged, dlfb_usb_disconnect calls > > unlink_framebuffer, then it waits until the reference count drops to zero > > and then it deallocates the framebuffer. However, the console that is > > attached to the framebuffer device keeps the reference count non-zero, so > > the framebuffer device is never destroyed. When the USB adapter is plugged > > again, it creates a new device /dev/fb1 and the console is not attached to > > it. > > > > This patch fixes the bug by unbinding the console from unlink_framebuffer. > > The code to unbind the console is moved from do_unregister_framebuffer to > > a function unbind_console. When the console is unbound, the reference > > count drops to zero and the udlfb driver frees the framebuffer. When the > > adapter is plugged back, a new framebuffer is created and the console is > > attached to it. > > > > Signed-off-by: Mikulas Patocka > > Cc: sta...@vger.kernel.org > > Does this work correctly with the udl drm driver and the drm fbdev > emulation? If yes I'm not sure what the value is in fixing up the uldfb > driver really ... > > Same for all the uldfb fixes in your other series. If the 2 drivers are > on feature parity I'd just go ahead and remove the uldfb one. > -Daniel The udl drm driver is worse than udlfb with regard to unplug. The udl drm driver destroys the device on USB unplug no matter if someone is using it or not. If you unplug the device while Xserver is running or while some console framebuffer application is running, you get a crash. The udl fb driver correctly waits until all users close the device before destroying it. Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fb: fix lost console when the user unplugs a USB adapter
On Tue, 3 Jul 2018, Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
> On Sunday, June 03, 2018 11:46:29 AM Mikulas Patocka wrote:
> > I have a USB display adapter using the udlfb driver and I use it on an ARM
> > board that doesn't have any graphics card. When I plug the adapter in, the
> > console is properly displayed, however when I unplug and re-plug the
> > adapter, the console is not displayed and I can't access it until I reboot
> > the board.
> >
> > The reason is this:
> > When the adapter is unplugged, dlfb_usb_disconnect calls
> > unlink_framebuffer, then it waits until the reference count drops to zero
> > and then it deallocates the framebuffer. However, the console that is
> > attached to the framebuffer device keeps the reference count non-zero, so
> > the framebuffer device is never destroyed. When the USB adapter is plugged
> > again, it creates a new device /dev/fb1 and the console is not attached to
> > it.
> >
> > This patch fixes the bug by unbinding the console from unlink_framebuffer.
> > The code to unbind the console is moved from do_unregister_framebuffer to
> > a function unbind_console. When the console is unbound, the reference
> > count drops to zero and the udlfb driver frees the framebuffer. When the
> > adapter is plugged back, a new framebuffer is created and the console is
> > attached to it.
> >
> > Signed-off-by: Mikulas Patocka
> > Cc: sta...@vger.kernel.org
>
> After this change unbind_console() will be called twice in the standard
> framebuffer unregister path:
>
> - first time, directly by do_unregister_framebuffer()
>
> - second time, indirectly by do_unregister_framebuffer()->unlink_framebuffer()
>
> This doesn't look correctly.
unbind_console calls the FB_EVENT_FB_UNBIND notifier, FB_EVENT_FB_UNBIND
goes to the function fbcon_fb_unbind and fbcon_fb_unbind checks if the
console is bound to the framebuffer for which unbind is requested. So a
double call won't cause any trouble.
> Also why can't udlfb just use unregister_framebuffer() like all other
> drivers (it uses unlink_framebuffer() and it is the only user of this
> helper)?
It uses unregister_framebuffer() - but - unregister_framebuffer() may only
be called when the open count of the framebuffer is zero. So, the udlfb
driver waits until the open count drops to zero and then calls
unregister_framebuffer().
But the console subsystem keeps the framebuffer open - which means that if
user use unplugs the USB adapter, the open count won't drop to zero
(because the console is bound to it) - which means that
unregister_framebuffer() will not be called.
You must unbind the console before calling unregister_framebuffer(). The
PCI framebuffer drivers don't have this problem because the user is not
expected to just unplug the PCI card while it is being used by the
console.
Mikulas
> > ---
> > drivers/video/fbdev/core/fbmem.c | 21 +
> > 1 file changed, 17 insertions(+), 4 deletions(-)
> >
> > Index: linux-4.16.12/drivers/video/fbdev/core/fbmem.c
> > ===
> > --- linux-4.16.12.orig/drivers/video/fbdev/core/fbmem.c 2018-05-26
> > 06:13:20.0 +0200
> > +++ linux-4.16.12/drivers/video/fbdev/core/fbmem.c 2018-05-26
> > 06:13:20.0 +0200
> > @@ -1805,12 +1805,12 @@ static int do_register_framebuffer(struc
> > return 0;
> > }
> >
> > -static int do_unregister_framebuffer(struct fb_info *fb_info)
> > +static int unbind_console(struct fb_info *fb_info)
> > {
> > struct fb_event event;
> > - int i, ret = 0;
> > + int ret;
> > + int i = fb_info->node;
> >
> > - i = fb_info->node;
> > if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)
> > return -EINVAL;
> >
> > @@ -1825,6 +1825,16 @@ static int do_unregister_framebuffer(str
> > unlock_fb_info(fb_info);
> > console_unlock();
> >
> > + return ret;
> > +}
> > +
> > +static int do_unregister_framebuffer(struct fb_info *fb_info)
> > +{
> > + struct fb_event event;
> > + int ret;
> > +
> > + ret = unbind_console(fb_info);
> > +
> > if (ret)
> > return -EINVAL;
> >
> > @@ -1835,7 +1845,7 @@ static int do_unregister_framebuffer(str
> > (fb_info->pixmap.flags & FB_PIXMAP_DEFAULT))
> > kfree(fb_info->pixmap.addr);
> > fb_destroy_modelist(_info->modelist);
> > - registered_fb[i] = NULL;
> > + regist
Re: [PATCH 18/21] udlfb: allow reallocating the framebuffer
On Mon, 4 Jun 2018, kbuild test robot wrote:
> Hi Mikulas,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on drm/drm-next]
> [also build test WARNING on v4.17-rc7 next-20180601]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Mikulas-Patocka/USB-DisplayLink-patches/20180603-233013
What is it really complaining about? That URL shows 404 Not Found and this
email has no warnings at all.
> base: git://people.freedesktop.org/~airlied/linux.git drm-next
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
>
> vim +1198 drivers/video/fbdev/udlfb.c
>
> 1171
> 1172/*
> 1173 * Assumes >lock held by caller
> 1174 * Assumes no active clients have framebuffer open
> 1175 */
> 1176static int dlfb_realloc_framebuffer(struct dlfb_data *dlfb,
> struct fb_info *info, u32 new_len)
> 1177{
> 1178u32 old_len = info->fix.smem_len;
> > 1179unsigned char *old_fb = info->screen_base;
> 1180unsigned char *new_fb;
> 1181unsigned char *new_back = NULL;
> 1182
> 1183new_len = PAGE_ALIGN(new_len);
> 1184
> 1185if (new_len > old_len) {
> 1186/*
> 1187 * Alloc system memory for virtual framebuffer
> 1188 */
> 1189new_fb = vmalloc(new_len);
> 1190if (!new_fb) {
> 1191dev_err(info->dev, "Virtual framebuffer
> alloc failed\n");
> 1192return -ENOMEM;
> 1193}
> 1194memset(new_fb, 0xff, new_len);
> 1195
> 1196if (info->screen_base) {
> 1197memcpy(new_fb, old_fb, old_len);
> > 1198dlfb_deferred_vfree(dlfb,
> > info->screen_base);
> 1199}
> 1200
> 1201info->screen_base = new_fb;
> 1202info->fix.smem_len = new_len;
> 1203info->fix.smem_start = (unsigned long) new_fb;
> 1204info->flags = udlfb_info_flags;
> 1205
> 1206/*
> 1207 * Second framebuffer copy to mirror the
> framebuffer state
> 1208 * on the physical USB device. We can function
> without this.
> 1209 * But with imperfect damage info we may send
> pixels over USB
> 1210 * that were, in fact, unchanged - wasting
> limited USB bandwidth
> 1211 */
> 1212if (shadow)
> 1213new_back = vzalloc(new_len);
> 1214if (!new_back)
> 1215dev_info(info->dev,
> 1216 "No shadow/backing buffer
> allocated\n");
> 1217else {
> 1218dlfb_deferred_vfree(dlfb,
> dlfb->backing_buffer);
> 1219dlfb->backing_buffer = new_back;
> 1220}
> 1221}
> 1222return 0;
> 1223}
> 1224
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/21] udl-kms: avoid prefetch
On Wed, 6 Jun 2018, Alexey Brodkin wrote: > Hi Mikulas, > > On Tue, 2018-06-05 at 11:30 -0400, Mikulas Patocka wrote: > > > > On Tue, 5 Jun 2018, Alexey Brodkin wrote: > > > > > Hi Mikulas, > > > > > > On Sun, 2018-06-03 at 16:41 +0200, Mikulas Patocka wrote: > > > > Modern processors can detect linear memory accesses and prefetch data > > > > automatically, so there's no need to use prefetch. > > > > > > Not each and every CPU that's capable of running Linux has prefetch > > > functionality :) > > > > > > Still read-on... > > > > > > > Signed-off-by: Mikulas Patocka > > > > > > > > --- > > > > drivers/gpu/drm/udl/udl_transfer.c |7 --- > > > > 1 file changed, 7 deletions(-) > > > > > > > > Index: linux-4.16.12/drivers/gpu/drm/udl/udl_transfer.c > > > > === > > > > --- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_transfer.c > > > > 2018-05-31 14:48:12.0 +0200 > > > > +++ linux-4.16.12/drivers/gpu/drm/udl/udl_transfer.c2018-05-31 > > > > 14:48:12.0 +0200 > > > > @@ -13,7 +13,6 @@ > > > > #include > > > > #include > > > > #include > > > > -#include > > > > #include > > > > > > > > #include > > > > @@ -51,9 +50,6 @@ static int udl_trim_hline(const u8 *bbac > > > > int start = width; > > > > int end = width; > > > > > > > > - prefetch((void *) front); > > > > - prefetch((void *) back); > > > > > > AFAIK prefetcher fetches new data according to a known history... i.e. > > > based on previously > > > used pattern we'll trying to get the next batch of data. > > > > > > But the code above is in the very beginning of the data processing > > > routine where > > > prefetcher doesn't yet have any history to know what and where to > > > prefetch. > > > > > > So I'd say this particular usage is good. > > > At least those prefetches shouldn't hurt because typically it > > > would be just 1 instruction if those exist or nothing if CPU/compiler > > > doesn't > > > support it. > > > > See this post > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lwn.net_Articles_444336_=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=lqdeeSSEes0GFDDl656e > > ViXO7breS55ytWkhpk5R81I=a5RaqJtvajFkM1hL7bOKD5jV7cpFfTvG2Y1cYCdBPd0=w0W8wFtAgENp8TE6RzdPGhdKRasJc_otIn08V0EkgrY= > > where they measured that > > prefetch hurts performance. Prefetch shouldn't be used unless you have a > > proof that it improves performance. > > > > The problem is that the prefetch instruction causes stalls in the pipeline > > when it encounters TLB miss and the automatic prefetcher doesn't. > > Wow, thanks for the link. > I didn't know about that subtle issue with prefetch instructions on ARM and > x86. > > So OK in case of UDL these prefetches anyways make not not much sense I guess > and there's > something worse still, see what I've got from WandBoard Quad running kmscube > [1] application > with help of perf utility: > --->8- > # Overhead Command Shared ObjectSymbol > # ... ... > > # > 92.93% kmscube [kernel.kallsyms][k] udl_render_hline > 2.51% kmscube [kernel.kallsyms][k] __divsi3 > 0.33% kmscube [kernel.kallsyms][k] _raw_spin_unlock_irqrestore > 0.22% kmscube [kernel.kallsyms][k] lock_acquire > 0.19% kmscube [kernel.kallsyms][k] _raw_spin_unlock_irq > 0.17% kmscube [kernel.kallsyms][k] udl_handle_damage > 0.12% kmscube [kernel.kallsyms][k] v7_dma_clean_range > 0.11% kmscube [kernel.kallsyms][k] l2c210_clean_range > 0.06% kmscube [kernel.kallsyms][k] __memzero > --->8- > > That said it's not even USB 2.0 which is a bottle-neck but > computations in the udl_render_hline(). > > > [1] https://cgit.freedesktop.org/mesa/kmscube/ > > -Alexey Try this patch http://people.redhat.com/~mpatocka/patches/kernel/udl/udlkms-avoid-division.patch It is doing a lot of divisions - and WandBoard has Cortex-A9, that doesn't have division instruction. BTW. the framebuffer UDL driver (not the modesetting driver) has performance counters in sysfs. Their location depends on the system, you can find them with find /sys -name "*metrics*" The file "metrics_reset" resets the counters, so you can measure if the prefetch instructions improve performance or not. Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/21] USB DisplayLink patches
On Tue, 5 Jun 2018, Alexey Brodkin wrote: > Hi Mikulas, > > On Sun, 2018-06-03 at 16:40 +0200, Mikulas Patocka wrote: > > Hi > > > > Here I'm sending bug fixes and performance improvements for the USB > > DisplayLink framebuffer and modesetting drivers for this merge window. > > For such a long series it would be very nice to post a link to your git > tree so people may play with your changes easily. > > Could you please prepare one? > > -Alexey I don't use git to manage my patches, I use quilt. I uploaded the patches from quilt here: http://people.redhat.com/~mpatocka/patches/kernel/udl/series.html Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/21] udl-kms: avoid prefetch
On Tue, 5 Jun 2018, Alexey Brodkin wrote: > Hi Mikulas, > > On Sun, 2018-06-03 at 16:41 +0200, Mikulas Patocka wrote: > > Modern processors can detect linear memory accesses and prefetch data > > automatically, so there's no need to use prefetch. > > Not each and every CPU that's capable of running Linux has prefetch > functionality :) > > Still read-on... > > > Signed-off-by: Mikulas Patocka > > > > --- > > drivers/gpu/drm/udl/udl_transfer.c |7 --- > > 1 file changed, 7 deletions(-) > > > > Index: linux-4.16.12/drivers/gpu/drm/udl/udl_transfer.c > > === > > --- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_transfer.c 2018-05-31 > > 14:48:12.0 +0200 > > +++ linux-4.16.12/drivers/gpu/drm/udl/udl_transfer.c2018-05-31 > > 14:48:12.0 +0200 > > @@ -13,7 +13,6 @@ > > #include > > #include > > #include > > -#include > > #include > > > > #include > > @@ -51,9 +50,6 @@ static int udl_trim_hline(const u8 *bbac > > int start = width; > > int end = width; > > > > - prefetch((void *) front); > > - prefetch((void *) back); > > AFAIK prefetcher fetches new data according to a known history... i.e. based > on previously > used pattern we'll trying to get the next batch of data. > > But the code above is in the very beginning of the data processing routine > where > prefetcher doesn't yet have any history to know what and where to prefetch. > > So I'd say this particular usage is good. > At least those prefetches shouldn't hurt because typically it > would be just 1 instruction if those exist or nothing if CPU/compiler doesn't > support it. See this post https://lwn.net/Articles/444336/ where they measured that prefetch hurts performance. Prefetch shouldn't be used unless you have a proof that it improves performance. The problem is that the prefetch instruction causes stalls in the pipeline when it encounters TLB miss and the automatic prefetcher doesn't. Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/21] USB DisplayLink patches
On Mon, 4 Jun 2018, Dave Airlie wrote: > On 4 June 2018 at 00:40, Mikulas Patocka wrote: > > Hi > > > > Here I'm sending bug fixes and performance improvements for the USB > > DisplayLink framebuffer and modesetting drivers for this merge window. > > > > Hi, > > You probably want to split these up into separate series for the kms and fbdev > drivers. > > Otherwise at least for drm you've missed this merge window, since it > closes around rc6 of the previous kernel, Could you apply at least the fbdefio patches (without them, fbdefio is unusable due to crashes) and the display corruption of the last line (because most people will hit it)? > did you use git send-email > for these patches, at least some of them viewed funny on my phone, I used the command "quilt mail". I use quilt, not git, for management of my patches. > I'll try and look over the kms ones soon. Do you have any numbers for > improvements to the kms ones? > > Dave. I measured performance improvement on the framebuffer patches. The kms driver already performs well, there's not much to do. I'd like to as you if you could review the patch "udl-kms: fix a linked-list corruption when using fbdefio" - for me it fixes the crashes, but I am not expert in modesetting drivers and I don't know if some other part of the kernel assumes that the framebuffer pages must be allocated with drm_gem_get_pages. BTW. When I unplug the USB adapter while using the modesetting driver, I get this warning. Do you have an idea how to fix it? WARNING: CPU: 0 PID: 61 at drivers/gpu/drm/drm_mode_config.c:439 drm_mode_config_cleanup+0x250/0x2b8 [drm] Modules linked in: udlfb hid_generic usbhid hid tun bridge stp llc autofs4 binfmt_misc ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables ipt_REJECT nf_reject_ipv4 xt_conntrack xt_multiport iptable_filter iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 xt_nat xt_tcpudp iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 ip_tables x_tables pppoe pppox af_packet ppp_generic slhc udl drm_kms_helper cfbfillrect cfbimgblt cfbcopyarea drm drm_panel_orientation_quirks syscopyarea sysfillrect sysimgblt fb_sys_fops fb font snd_usb_audio snd_hwdep snd_usbmidi_lib snd_rawmidi snd_pcm snd_timer snd soundcore nf_nat_ftp nf_conntrack_ftp nf_nat nf_conntrack sd_mod ipv6 aes_ce_blk crypto_simd cryptd aes_ce_cipher crc32_ce ghash_ce gf128mul aes_arm64 sha2_ce sha256_arm64 sha1_ce xhci_plat_hcd xhci_hcd sha1_generic usbcore usb_common ahci_platform libahci_platform libahci mvpp2 unix CPU: 0 PID: 61 Comm: kworker/0:2 Not tainted 4.17.0-rc7 #1 Hardware name: Marvell 8040 MACCHIATOBin (DT) Workqueue: usb_hub_wq hub_event [usbcore] pstate: 8005 (Nzcv daif -PAN -UAO) pc : drm_mode_config_cleanup+0x250/0x2b8 [drm] lr : drm_mode_config_cleanup+0x88/0x2b8 [drm] sp : ffc13a643920 x29: ffc13a643920 x28: ffc13a63ac00 x27: ffc1380962c0 x26: ffc11b95f898 x25: ff8000afd1d8 x24: ffc11b95f800 x23: 0060 x22: ff8000afd240 x21: ffc11b95eb38 x20: ffc11b95e800 x19: ffc11b95eb30 x18: ffc11b95ea7c x17: 007fa3591b60 x16: ff80081dc178 x15: ffc11b95ea78 x14: x13: ffc12b3fc000 x12: ffc12b3fc028 x11: ffc12b3fc119 x10: 001f x9 : 0028 x8 : ff8000a84000 x7 : x6 : 0001 x5 : 0002 x4 : 0001 x3 : 0002 x2 : 002f x1 : ffc11b95eaf8 x0 : ffc11b978818 Call trace: drm_mode_config_cleanup+0x250/0x2b8 [drm] udl_modeset_cleanup+0xc/0x18 [udl] udl_driver_unload+0x30/0x50 [udl] drm_dev_unregister+0x3c/0xe8 [drm] drm_dev_unplug+0x18/0x70 [drm] udl_usb_disconnect+0x30/0x40 [udl] usb_unbind_interface+0x6c/0x290 [usbcore] device_release_driver_internal+0x170/0x200 device_release_driver+0x14/0x20 bus_remove_device+0x118/0x128 device_del+0x110/0x308 usb_disable_device+0x8c/0x1f8 [usbcore] usb_disconnect+0xb4/0x218 [usbcore] usb_disconnect+0x9c/0x218 [usbcore] usb_disconnect+0x9c/0x218 [usbcore] hub_event+0xf20/0x1020 [usbcore] process_one_work+0x1c8/0x310 worker_thread+0x44/0x450 kthread+0x118/0x120 ret_from_fork+0x10/0x18 ---[ end trace 978a27ff198f1268 ]--- [drm:drm_mode_config_cleanup [drm]] *ERROR* connector DVI-I-1 leaked! Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] fb: fix lost console when the user unplugs a USB adapter
I have a USB display adapter using the udlfb driver and I use it on an ARM
board that doesn't have any graphics card. When I plug the adapter in, the
console is properly displayed, however when I unplug and re-plug the
adapter, the console is not displayed and I can't access it until I reboot
the board.
The reason is this:
When the adapter is unplugged, dlfb_usb_disconnect calls
unlink_framebuffer, then it waits until the reference count drops to zero
and then it deallocates the framebuffer. However, the console that is
attached to the framebuffer device keeps the reference count non-zero, so
the framebuffer device is never destroyed. When the USB adapter is plugged
again, it creates a new device /dev/fb1 and the console is not attached to
it.
This patch fixes the bug by unbinding the console from unlink_framebuffer.
The code to unbind the console is moved from do_unregister_framebuffer to
a function unbind_console. When the console is unbound, the reference
count drops to zero and the udlfb driver frees the framebuffer. When the
adapter is plugged back, a new framebuffer is created and the console is
attached to it.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/core/fbmem.c | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
Index: linux-4.16.12/drivers/video/fbdev/core/fbmem.c
===
--- linux-4.16.12.orig/drivers/video/fbdev/core/fbmem.c 2018-05-26
06:13:20.0 +0200
+++ linux-4.16.12/drivers/video/fbdev/core/fbmem.c 2018-05-26
06:13:20.0 +0200
@@ -1805,12 +1805,12 @@ static int do_register_framebuffer(struc
return 0;
}
-static int do_unregister_framebuffer(struct fb_info *fb_info)
+static int unbind_console(struct fb_info *fb_info)
{
struct fb_event event;
- int i, ret = 0;
+ int ret;
+ int i = fb_info->node;
- i = fb_info->node;
if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)
return -EINVAL;
@@ -1825,6 +1825,16 @@ static int do_unregister_framebuffer(str
unlock_fb_info(fb_info);
console_unlock();
+ return ret;
+}
+
+static int do_unregister_framebuffer(struct fb_info *fb_info)
+{
+ struct fb_event event;
+ int ret;
+
+ ret = unbind_console(fb_info);
+
if (ret)
return -EINVAL;
@@ -1835,7 +1845,7 @@ static int do_unregister_framebuffer(str
(fb_info->pixmap.flags & FB_PIXMAP_DEFAULT))
kfree(fb_info->pixmap.addr);
fb_destroy_modelist(_info->modelist);
- registered_fb[i] = NULL;
+ registered_fb[fb_info->node] = NULL;
num_registered_fb--;
fb_cleanup_device(fb_info);
event.info = fb_info;
@@ -1860,6 +1870,9 @@ int unlink_framebuffer(struct fb_info *f
device_destroy(fb_class, MKDEV(FB_MAJOR, i));
fb_info->dev = NULL;
}
+
+ unbind_console(fb_info);
+
return 0;
}
EXPORT_SYMBOL(unlink_framebuffer);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/21] udl-kms: dont spam the syslog with debug messages
The udl kms driver writes messages to the syslog whenever some application
opens or closes /dev/fb0 and whenever the user switches between the
Xserver and the console.
This patch changes the priority of these messages to debug.
Signed-off-by: Mikulas Patocka
---
drivers/gpu/drm/udl/udl_fb.c |6 +++---
drivers/gpu/drm/udl/udl_modeset.c |2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
Index: linux-4.17-rc7/drivers/gpu/drm/udl/udl_fb.c
===
--- linux-4.17-rc7.orig/drivers/gpu/drm/udl/udl_fb.c2018-06-03
13:17:58.0 +0200
+++ linux-4.17-rc7/drivers/gpu/drm/udl/udl_fb.c 2018-06-03 13:42:48.0
+0200
@@ -179,7 +179,7 @@ static int udl_fb_mmap(struct fb_info *i
pos = (unsigned long)info->fix.smem_start + offset;
- pr_notice("mmap() framebuffer addr:%lu size:%lu\n",
+ pr_debug("mmap() framebuffer addr:%lu size:%lu\n",
pos, size);
/* We don't want the framebuffer to be mapped encrypted */
@@ -237,7 +237,7 @@ static int udl_fb_open(struct fb_info *i
}
#endif
- pr_notice("open /dev/fb%d user=%d fb_info=%p count=%d\n",
+ pr_debug("open /dev/fb%d user=%d fb_info=%p count=%d\n",
info->node, user, info, ufbdev->fb_count);
return 0;
@@ -262,7 +262,7 @@ static int udl_fb_release(struct fb_info
}
#endif
- pr_warn("released /dev/fb%d user=%d count=%d\n",
+ pr_debug("released /dev/fb%d user=%d count=%d\n",
info->node, user, ufbdev->fb_count);
return 0;
Index: linux-4.17-rc7/drivers/gpu/drm/udl/udl_modeset.c
===
--- linux-4.17-rc7.orig/drivers/gpu/drm/udl/udl_modeset.c 2018-06-03
13:17:58.0 +0200
+++ linux-4.17-rc7/drivers/gpu/drm/udl/udl_modeset.c2018-06-03
13:41:56.0 +0200
@@ -243,7 +243,7 @@ static int udl_crtc_write_mode_to_hw(str
memcpy(buf, udl->mode_buf, udl->mode_buf_len);
retval = udl_submit_urb(dev, urb, udl->mode_buf_len);
- DRM_INFO("write mode info %d\n", udl->mode_buf_len);
+ DRM_DEBUG("write mode info %d\n", udl->mode_buf_len);
return retval;
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 12/21] udlfb: fix display corruption of the last line
The displaylink hardware has such a peculiarity that it doesn't render a
command until next command is received. This produces occasional
corruption, such as when setting 22x11 font on the console, only the first
line of the cursor will be blinking if the cursor is located at some
specific columns.
When we end up with a repeating pixel, the driver has a bug that it leaves
one uninitialized byte after the command (and this byte is enough to flush
the command and render it - thus it fixes the screen corruption), however
whe we end up with a non-repeating pixel, there is no byte appended and
this results in temporary screen corruption.
This patch fixes the screen corruption by always appending a byte 0xAF at
the end of URB. It also removes the uninitialized byte.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/udlfb.c | 30 --
1 file changed, 20 insertions(+), 10 deletions(-)
Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
===
--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-05-31
14:43:13.0 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-05-31 14:46:03.0
+0200
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include "edid.h"
@@ -450,17 +451,17 @@ static void dlfb_compress_hline(
raw_pixels_count_byte = cmd++; /* we'll know this later */
raw_pixel_start = pixel;
- cmd_pixel_end = pixel + min(MAX_CMD_PIXELS + 1,
- min((int)(pixel_end - pixel),
- (int)(cmd_buffer_end - cmd) / BPP));
+ cmd_pixel_end = pixel + min3(MAX_CMD_PIXELS + 1UL,
+ (unsigned long)(pixel_end - pixel),
+ (unsigned long)(cmd_buffer_end - 1 -
cmd) / BPP);
- prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * BPP);
+ prefetch_range((void *) pixel, (u8 *)cmd_pixel_end - (u8
*)pixel);
while (pixel < cmd_pixel_end) {
const uint16_t * const repeating_pixel = pixel;
- *cmd++ = *pixel >> 8;
- *cmd++ = *pixel;
+ put_unaligned_be16(*pixel, cmd);
+ cmd += 2;
pixel++;
if (unlikely((pixel < cmd_pixel_end) &&
@@ -486,13 +487,16 @@ static void dlfb_compress_hline(
if (pixel > raw_pixel_start) {
/* finalize last RAW span */
*raw_pixels_count_byte = (pixel-raw_pixel_start) & 0xFF;
+ } else {
+ /* undo unused byte */
+ cmd--;
}
*cmd_pixels_count_byte = (pixel - cmd_pixel_start) & 0xFF;
- dev_addr += (pixel - cmd_pixel_start) * BPP;
+ dev_addr += (u8 *)pixel - (u8 *)cmd_pixel_start;
}
- if (cmd_buffer_end <= MIN_RLX_CMD_BYTES + cmd) {
+ if (cmd_buffer_end - MIN_RLX_CMD_BYTES <= cmd) {
/* Fill leftover bytes with no-ops */
if (cmd_buffer_end > cmd)
memset(cmd, 0xAF, cmd_buffer_end - cmd);
@@ -610,8 +614,11 @@ static int dlfb_handle_damage(struct dlf
}
if (cmd > (char *) urb->transfer_buffer) {
+ int len;
+ if (cmd < (char *) urb->transfer_buffer +
urb->transfer_buffer_length)
+ *cmd++ = 0xAF;
/* Send partial buffer remaining before exiting */
- int len = cmd - (char *) urb->transfer_buffer;
+ len = cmd - (char *) urb->transfer_buffer;
ret = dlfb_submit_urb(dlfb, urb, len);
bytes_sent += len;
} else
@@ -735,8 +742,11 @@ static void dlfb_dpy_deferred_io(struct
}
if (cmd > (char *) urb->transfer_buffer) {
+ int len;
+ if (cmd < (char *) urb->transfer_buffer +
urb->transfer_buffer_length)
+ *cmd++ = 0xAF;
/* Send partial buffer remaining before exiting */
- int len = cmd - (char *) urb->transfer_buffer;
+ len = cmd - (char *) urb->transfer_buffer;
dlfb_submit_urb(dlfb, urb, len);
bytes_sent += len;
} else
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 19/21] udlfb: optimization - test the backing buffer
Currently, the udlfb driver only tests for identical bytes at the
beginning or at the end of a page and renders anything between the first
and last mismatching pixel. But pages are not the same as lines, so this
is quite suboptimal - if there is something modified at the beginning of a
page and at the end of a page, the whole page is rendered, even if most of
the page is not modified.
This patch makes it test for identical pixels at the beginning and end of
each rendering command. This patch improves identical byte detection by
41% when playing video in a window.
This patch also fixes a possible screen corruption if the user is writing
to the framebuffer while dlfb_render_hline is in progress - the pixel data
that is copied to the backbuffer with memcpy may be different from the
pixel data that is actually rendered to the hardware (because the content
of the framebuffer may change between memcpy and the rendering command).
We must make sure that we copy exactly the same pixel as the pixel that is
being rendered.
Signed-off-by: Mikulas Patocka
---
drivers/video/fbdev/udlfb.c | 45 +---
1 file changed, 34 insertions(+), 11 deletions(-)
Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
===
--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-05-31
14:51:43.0 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-05-31 14:51:43.0
+0200
@@ -431,7 +431,9 @@ static void dlfb_compress_hline(
const uint16_t *const pixel_end,
uint32_t *device_address_ptr,
uint8_t **command_buffer_ptr,
- const uint8_t *const cmd_buffer_end)
+ const uint8_t *const cmd_buffer_end,
+ unsigned long back_buffer_offset,
+ int *ident_ptr)
{
const uint16_t *pixel = *pixel_start_ptr;
uint32_t dev_addr = *device_address_ptr;
@@ -444,6 +446,14 @@ static void dlfb_compress_hline(
const uint16_t *raw_pixel_start = NULL;
const uint16_t *cmd_pixel_start, *cmd_pixel_end = NULL;
+ if (back_buffer_offset &&
+ *pixel == *(u16 *)((u8 *)pixel + back_buffer_offset)) {
+ pixel++;
+ dev_addr += BPP;
+ (*ident_ptr)++;
+ continue;
+ }
+
prefetchw((void *) cmd); /* pull in one cache line at least */
*cmd++ = 0xAF;
@@ -462,25 +472,37 @@ static void dlfb_compress_hline(
(unsigned long)(pixel_end - pixel),
(unsigned long)(cmd_buffer_end - 1 -
cmd) / BPP);
+ if (back_buffer_offset) {
+ /* note: the framebuffer may change under us, so we
must test for underflow */
+ while (cmd_pixel_end - 1 > pixel &&
+ *(cmd_pixel_end - 1) == *(u16 *)((u8
*)(cmd_pixel_end - 1) + back_buffer_offset))
+ cmd_pixel_end--;
+ }
+
prefetch_range((void *) pixel, (u8 *)cmd_pixel_end - (u8
*)pixel);
while (pixel < cmd_pixel_end) {
const uint16_t * const repeating_pixel = pixel;
+ u16 pixel_value = *pixel;
- put_unaligned_be16(*pixel, cmd);
+ put_unaligned_be16(pixel_value, cmd);
+ if (back_buffer_offset)
+ *(u16 *)((u8 *)pixel + back_buffer_offset) =
pixel_value;
cmd += 2;
pixel++;
if (unlikely((pixel < cmd_pixel_end) &&
-(*pixel == *repeating_pixel))) {
+(*pixel == pixel_value))) {
/* go back and fill in raw pixel count */
*raw_pixels_count_byte = ((repeating_pixel -
raw_pixel_start) + 1) & 0xFF;
- while ((pixel < cmd_pixel_end)
- && (*pixel == *repeating_pixel)) {
- pixel++;
- }
+ do {
+ if (back_buffer_offset)
+ *(u16 *)((u8 *)pixel +
back_buffer_offset) = pixel_value;
+ pixel++;
+ } while ((pixel < cmd_pixel_end) &&
+(*pixel == pixel_value));
/* immediately after raw data is repeat byte */
*cmd++ = ((pixel - repeating_pixel) - 1) & 0xFF;
@@ -531,6 +553,7 @@ s
[PATCH 13/21] udlfb: dont switch if we are switching to the same videomode
The udlfb driver reprograms the hardware everytime the user switches the
console, that makes quite unusable when working on the console.
This patch makes the driver remember the videomode we are in and avoid
reprogramming the hardware if we switch to the same videomode.
We mask the "activate" field and the "FB_VMODE_SMOOTH_XPAN" flag when
comparing the videomode, because they cause spurious switches when
switching to and from the Xserver.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/udlfb.c | 18 --
include/video/udlfb.h |1 +
2 files changed, 17 insertions(+), 2 deletions(-)
Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
===
--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-05-31
14:49:52.0 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-05-31 14:49:52.0
+0200
@@ -1041,10 +1041,24 @@ static int dlfb_ops_set_par(struct fb_in
int result;
u16 *pix_framebuffer;
int i;
+ struct fb_var_screeninfo fvs;
+
+ /* clear the activate field because it causes spurious miscompares */
+ fvs = info->var;
+ fvs.activate = 0;
+ fvs.vmode &= ~FB_VMODE_SMOOTH_XPAN;
+
+ if (!memcmp(>current_mode, , sizeof(struct
fb_var_screeninfo)))
+ return 0;
result = dlfb_set_video_mode(dlfb, >var);
- if ((result == 0) && (dlfb->fb_count == 0)) {
+ if (result)
+ return result;
+
+ dlfb->current_mode = fvs;
+
+ if (dlfb->fb_count == 0) {
/* paint greenscreen */
@@ -1056,7 +1070,7 @@ static int dlfb_ops_set_par(struct fb_in
info->screen_base);
}
- return result;
+ return 0;
}
/* To fonzi the jukebox (e.g. make blanking changes take effect) */
Index: linux-4.17-rc7/include/video/udlfb.h
===
--- linux-4.17-rc7.orig/include/video/udlfb.h 2018-05-31 14:49:52.0
+0200
+++ linux-4.17-rc7/include/video/udlfb.h2018-05-31 14:49:52.0
+0200
@@ -56,6 +56,7 @@ struct dlfb_data {
atomic_t bytes_identical; /* saved effort with backbuffer comparison */
atomic_t bytes_sent; /* to usb, after compression including overhead */
atomic_t cpu_kcycles_used; /* transpired during pixel processing */
+ struct fb_var_screeninfo current_mode;
};
#define NR_USB_REQUEST_I2C_SUB_IO 0x02
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 21/21] udlfb: use spin_lock_irq instead of spin_lock_irqsave
spin_lock_irqsave and spin_unlock_irqrestore is inteded to be called from
a context where it is unknown if interrupts are enabled or disabled (such
as interrupt handlers). From a process context, we should call
spin_lock_irq and spin_unlock_irq, that avoids the costly pushf and popf
instructions.
Signed-off-by: Mikulas Patocka
---
drivers/video/fbdev/udlfb.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
===
--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-06-03
13:17:46.0 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-06-03 13:17:46.0
+0200
@@ -1855,18 +1855,17 @@ static void dlfb_free_urb_list(struct dl
struct list_head *node;
struct urb_node *unode;
struct urb *urb;
- unsigned long flags;
/* keep waiting and freeing, until we've got 'em all */
while (count--) {
down(>urbs.limit_sem);
- spin_lock_irqsave(>urbs.lock, flags);
+ spin_lock_irq(>urbs.lock);
node = dlfb->urbs.list.next; /* have reserved one with sem */
list_del_init(node);
- spin_unlock_irqrestore(>urbs.lock, flags);
+ spin_unlock_irq(>urbs.lock);
unode = list_entry(node, struct urb_node, entry);
urb = unode->urb;
@@ -1944,7 +1943,6 @@ static struct urb *dlfb_get_urb(struct d
int ret;
struct list_head *entry;
struct urb_node *unode;
- unsigned long flags;
/* Wait for an in-flight buffer to complete and get re-queued */
ret = down_timeout(>urbs.limit_sem, GET_URB_TIMEOUT);
@@ -1956,14 +1954,14 @@ static struct urb *dlfb_get_urb(struct d
return NULL;
}
- spin_lock_irqsave(>urbs.lock, flags);
+ spin_lock_irq(>urbs.lock);
BUG_ON(list_empty(>urbs.list)); /* reserved one with limit_sem */
entry = dlfb->urbs.list.next;
list_del_init(entry);
dlfb->urbs.available--;
- spin_unlock_irqrestore(>urbs.lock, flags);
+ spin_unlock_irq(>urbs.lock);
unode = list_entry(entry, struct urb_node, entry);
return unode->urb;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 14/21] udlfb: make a local copy of fb_ops
The defio subsystem overwrites the method fb_osp->mmap. That method is
stored in module's static data - and that means that if we have multiple
diplaylink adapters, they will over write each other's method.
In order to avoid interference between multiple adapters, we copy the
fb_ops structure to a device-local memory.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/udlfb.c |3 ++-
include/video/udlfb.h |1 +
2 files changed, 3 insertions(+), 1 deletion(-)
Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
===
--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-06-03
13:17:33.0 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-06-03 13:17:33.0
+0200
@@ -1665,7 +1665,8 @@ static void dlfb_init_framebuffer_work(s
dlfb->info = info;
info->par = dlfb;
info->pseudo_palette = dlfb->pseudo_palette;
- info->fbops = _ops;
+ dlfb->ops = dlfb_ops;
+ info->fbops = >ops;
retval = fb_alloc_cmap(>cmap, 256, 0);
if (retval < 0) {
Index: linux-4.17-rc7/include/video/udlfb.h
===
--- linux-4.17-rc7.orig/include/video/udlfb.h 2018-06-03 13:17:33.0
+0200
+++ linux-4.17-rc7/include/video/udlfb.h2018-06-03 13:17:33.0
+0200
@@ -51,6 +51,7 @@ struct dlfb_data {
int base8;
u32 pseudo_palette[256];
int blank_mode; /*one of FB_BLANK_ */
+ struct fb_ops ops;
/* blit-only rendering path metrics, exposed through sysfs */
atomic_t bytes_rendered; /* raw pixel-bytes driver asked to render */
atomic_t bytes_identical; /* saved effort with backbuffer comparison */
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 05/21] udl-kms: fix a linked-list corruption when using fbdefio
The udl driver crashes when fbdefio is used. The crash can be reproduced
with this sequence:
1. echo 1 >/sys/module/udl/parameters/fb_defio
2. run some program that maps the framebuffer, such as 'links -g' or 'fbi'
3. allocate memory to the point where the machine starts swapping
The reason for the crash is that udl_gem_get_pages calls drm_gem_get_pages
and drm_gem_get_pages allocates the pages using shmem_read_mapping_page.
The shmem pages are kept on the memory management lists using the
page->lru entry.
However, fbdefio reuses the page->lru entry for the list of pages that
were modified, so the memory management lists are corrupted and the
machine crashes when vmscan starts to scan memory.
I fixed this crash by allocating pages with "alloc_page" instead. The
pages allocated with "alloc_page" have page->lru unused, and thus the
system doesn't crash when fbdefio uses it.
Unable to handle kernel paging request at virtual address dead0200
Mem abort info:
ESR = 0x9644
Exception class = DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
Data abort info:
ISV = 0, ISS = 0x0044
CM = 0, WnR = 1
[dead0200] address between user and kernel address ranges
Internal error: Oops: 9644 [#1] PREEMPT SMP
Modules linked in: ip6table_filter ip6_tables iptable_filter ip_tables x_tables
af_packet autofs4 udl drm_kms_helper cfbfillrect syscopyarea cfbimgblt
sysfillrect sysimgblt fb_sys_fops cfbcopyarea fb font drm
drm_panel_orientation_quirks mousedev hid_generic usbhid hid binfmt_misc
snd_usb_audio snd_hwdep snd_usbmidi_lib snd_rawmidi snd_pcm snd_timer snd
soundcore ipv6 aes_ce_blk crypto_simd cryptd aes_ce_cipher crc32_ce ghash_ce
gf128mul aes_arm64 sha2_ce sha256_arm64 sha1_ce sha1_generic xhci_plat_hcd
xhci_hcd sd_mod usbcore usb_common mvpp2 unix
CPU: 0 PID: 39 Comm: kswapd0 Not tainted 4.16.12 #3
Hardware name: Marvell 8040 MACHIATOBin (DT)
pstate: 0085 (nzcv daIf -PAN -UAO)
pc : isolate_lru_pages.isra.16+0x23c/0x2b0
lr : isolate_lru_pages.isra.16+0x104/0x2b0
sp : ffc13a897ac0
x29: ffc13a897ac0 x28: 0003
x27: 0003 x26: 0004
x25: ff80087e84a0 x24: ffc13a897b68
x23: ffbf04cefc60 x22: ffc13a897e44
x21: 0009 x20: ffc13a897c00
x19: ffc13a897b40 x18: ffbf04d39000
x17: fff8 x16: ffbf
x15: 0006 x14:
x13: 01aa x12: 4004001c
x11: 0001 x10: 0001
x9 : 000ee3ac x8 : 04a0
x7 : ff80087e84a0 x6 :
x5 : x4 : 0002
x3 : ff80087e84a0 x2 : 0200
x1 : dead0200 x0 : 4004001c
Process kswapd0 (pid: 39, stack limit = 0x97f25571)
Call trace:
isolate_lru_pages.isra.16+0x23c/0x2b0
shrink_inactive_list+0xe4/0x3b0
shrink_node_memcg.constprop.19+0x374/0x630
shrink_node+0x64/0x1c8
kswapd+0x340/0x568
kthread+0x118/0x120
ret_from_fork+0x10/0x18
Code: d2804002 f85e02e0 f85e02ec f9000461 (f923)
---[ end trace f9f3ad3856cb2ef3 ]---
note: kswapd0[39] exited with preempt_count 1
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/udl/udl_gem.c | 31 +++
1 file changed, 27 insertions(+), 4 deletions(-)
Index: linux-4.16.12/drivers/gpu/drm/udl/udl_gem.c
===
--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_gem.c2018-01-10
09:31:23.0 +0100
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_gem.c 2018-05-29 17:46:10.0
+0200
@@ -130,28 +130,51 @@ int udl_gem_fault(struct vm_fault *vmf)
int udl_gem_get_pages(struct udl_gem_object *obj)
{
struct page **pages;
+ int npages, i;
if (obj->pages)
return 0;
- pages = drm_gem_get_pages(>base);
- if (IS_ERR(pages))
- return PTR_ERR(pages);
+ npages = obj->base.size >> PAGE_SHIFT;
+
+ pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
+ if (!pages)
+ return -ENOMEM;
+
+ for (i = 0; i < npages; i++) {
+ struct page *p = alloc_page(GFP_KERNEL | __GFP_ZERO);
+ if (!p)
+ goto fail;
+ pages[i] = p;
+ }
obj->pages = pages;
return 0;
+
+fail:
+ while (i--)
+ put_page(pages[i]);
+ kvfree(pages);
+ return -ENOMEM;
}
void udl_gem_put_pages(struct udl_gem_object *obj)
{
+ int npages, i;
+
if (obj->base.import_attach) {
kvfree(obj->pages);
obj->pages = NULL;
return;
}
- drm_gem_put_pages(>base, obj->pages, false, false);
+ npages = obj->base.size >> PAGE_SHIFT;
+
+ for (i = 0; i < npages; i++)
+
[PATCH 16/21] udlfb: handle allocation failure
Allocations larger than PAGE_ALLOC_COSTLY_ORDER are unreliable and they
may fail anytime. This patch fixes the udlfb driver so that when a large
alloactions fails, it tries to do multiple smaller allocations.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/udlfb.c | 26 +-
1 file changed, 17 insertions(+), 9 deletions(-)
Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
===
--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-06-03
13:17:38.0 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-06-03 13:17:38.0
+0200
@@ -1843,17 +1843,22 @@ static void dlfb_free_urb_list(struct dl
static int dlfb_alloc_urb_list(struct dlfb_data *dlfb, int count, size_t size)
{
- int i = 0;
struct urb *urb;
struct urb_node *unode;
char *buf;
+ size_t wanted_size = count * size;
spin_lock_init(>urbs.lock);
+retry:
dlfb->urbs.size = size;
INIT_LIST_HEAD(>urbs.list);
- while (i < count) {
+ sema_init(>urbs.limit_sem, 0);
+ dlfb->urbs.count = 0;
+ dlfb->urbs.available = 0;
+
+ while (dlfb->urbs.count * size < wanted_size) {
unode = kzalloc(sizeof(*unode), GFP_KERNEL);
if (!unode)
break;
@@ -1866,11 +1871,16 @@ static int dlfb_alloc_urb_list(struct dl
}
unode->urb = urb;
- buf = usb_alloc_coherent(dlfb->udev, MAX_TRANSFER, GFP_KERNEL,
+ buf = usb_alloc_coherent(dlfb->udev, size, GFP_KERNEL,
>transfer_dma);
if (!buf) {
kfree(unode);
usb_free_urb(urb);
+ if (size > PAGE_SIZE) {
+ size /= 2;
+ dlfb_free_urb_list(dlfb);
+ goto retry;
+ }
break;
}
@@ -1881,14 +1891,12 @@ static int dlfb_alloc_urb_list(struct dl
list_add_tail(>entry, >urbs.list);
- i++;
+ up(>urbs.limit_sem);
+ dlfb->urbs.count++;
+ dlfb->urbs.available++;
}
- sema_init(>urbs.limit_sem, i);
- dlfb->urbs.count = i;
- dlfb->urbs.available = i;
-
- return i;
+ return dlfb->urbs.count;
}
static struct urb *dlfb_get_urb(struct dlfb_data *dlfb)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 15/21] udlfb: set optimal write delay
The default delay 5 jiffies is too much when the kernel is compiled with
HZ=100 - it results in jumpy cursor in Xwindow.
In order to find out the optimal delay, I benchmarked the driver on
1280x720x30fps video. I found out that with HZ=1000, 10ms is acceptable,
but with HZ=250 or HZ=300, we need 4ms, so that the video is played
without any frame skips.
This patch changes the delay to this value.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
include/video/udlfb.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-4.17-rc7/include/video/udlfb.h
===
--- linux-4.17-rc7.orig/include/video/udlfb.h 2018-06-03 13:17:37.0
+0200
+++ linux-4.17-rc7/include/video/udlfb.h2018-06-03 13:17:37.0
+0200
@@ -88,7 +88,7 @@ struct dlfb_data {
#define MIN_RAW_PIX_BYTES 2
#define MIN_RAW_CMD_BYTES (RAW_HEADER_BYTES + MIN_RAW_PIX_BYTES)
-#define DL_DEFIO_WRITE_DELAY5 /* fb_deferred_io.delay in jiffies */
+#define DL_DEFIO_WRITE_DELAYmsecs_to_jiffies(HZ <= 300 ? 4 : 10) /*
optimal value for 720p video */
#define DL_DEFIO_WRITE_DISABLE (HZ*60) /* "disable" with long delay */
/* remove these once align.h patch is taken into kernel */
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 00/21] USB DisplayLink patches
Hi Here I'm sending bug fixes and performance improvements for the USB DisplayLink framebuffer and modesetting drivers for this merge window. Mikulas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 02/21] udl-kms: change down_interruptible to down
If we leave urbs around, it causes not only leak, but also memory
corruption. This patch fixes the function udl_free_urb_list, so that it
always waits for all urbs that are in progress.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/udl/udl_main.c |7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
Index: linux-4.16.12/drivers/gpu/drm/udl/udl_main.c
===
--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_main.c 2018-05-31
10:23:42.0 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_main.c2018-05-31
10:28:11.0 +0200
@@ -170,18 +170,13 @@ static void udl_free_urb_list(struct drm
struct list_head *node;
struct urb_node *unode;
struct urb *urb;
- int ret;
unsigned long flags;
DRM_DEBUG("Waiting for completes and freeing all render urbs\n");
/* keep waiting and freeing, until we've got 'em all */
while (count--) {
-
- /* Getting interrupted means a leak, but ok at shutdown*/
- ret = down_interruptible(>urbs.limit_sem);
- if (ret)
- break;
+ down(>urbs.limit_sem);
spin_lock_irqsave(>urbs.lock, flags);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/21] udl-kms: avoid division
Division is slow, so it shouldn't be done by the pixel generating code.
The driver supports only 2 or 4 bytes per pixel, so we can replace
division with a shift.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/udl/udl_drv.h |2 -
drivers/gpu/drm/udl/udl_fb.c | 15 --
drivers/gpu/drm/udl/udl_transfer.c | 39 ++---
3 files changed, 30 insertions(+), 26 deletions(-)
Index: linux-4.17-rc7/drivers/gpu/drm/udl/udl_drv.h
===
--- linux-4.17-rc7.orig/drivers/gpu/drm/udl/udl_drv.h 2018-06-03
13:15:01.0 +0200
+++ linux-4.17-rc7/drivers/gpu/drm/udl/udl_drv.h2018-06-03
13:15:01.0 +0200
@@ -110,7 +110,7 @@ udl_fb_user_fb_create(struct drm_device
struct drm_file *file,
const struct drm_mode_fb_cmd2 *mode_cmd);
-int udl_render_hline(struct drm_device *dev, int bpp, struct urb **urb_ptr,
+int udl_render_hline(struct drm_device *dev, int log_bpp, struct urb **urb_ptr,
const char *front, char **urb_buf_ptr,
u32 byte_offset, u32 device_byte_offset, u32 byte_width,
int *ident_ptr, int *sent_ptr);
Index: linux-4.17-rc7/drivers/gpu/drm/udl/udl_fb.c
===
--- linux-4.17-rc7.orig/drivers/gpu/drm/udl/udl_fb.c2018-06-03
13:15:01.0 +0200
+++ linux-4.17-rc7/drivers/gpu/drm/udl/udl_fb.c 2018-06-03 13:15:01.0
+0200
@@ -91,7 +91,10 @@ int udl_handle_damage(struct udl_framebu
int bytes_identical = 0;
struct urb *urb;
int aligned_x;
- int bpp = fb->base.format->cpp[0];
+ int log_bpp;
+
+ BUG_ON(!is_power_of_2(fb->base.format->cpp[0]));
+ log_bpp = __ffs(fb->base.format->cpp[0]);
if (!fb->active_16)
return 0;
@@ -126,12 +129,12 @@ int udl_handle_damage(struct udl_framebu
for (i = y; i < y + height ; i++) {
const int line_offset = fb->base.pitches[0] * i;
- const int byte_offset = line_offset + (x * bpp);
- const int dev_byte_offset = (fb->base.width * bpp * i) + (x *
bpp);
- if (udl_render_hline(dev, bpp, ,
+ const int byte_offset = line_offset + (x << log_bpp);
+ const int dev_byte_offset = (fb->base.width * i + x) << log_bpp;
+ if (udl_render_hline(dev, log_bpp, ,
(char *) fb->obj->vmapping,
, byte_offset, dev_byte_offset,
-width * bpp,
+width << log_bpp,
_identical, _sent))
goto error;
}
@@ -150,7 +153,7 @@ int udl_handle_damage(struct udl_framebu
error:
atomic_add(bytes_sent, >bytes_sent);
atomic_add(bytes_identical, >bytes_identical);
- atomic_add(width*height*bpp, >bytes_rendered);
+ atomic_add((width * height) << log_bpp, >bytes_rendered);
end_cycles = get_cycles();
atomic_add(((unsigned int) ((end_cycles - start_cycles)
>> 10)), /* Kcycles */
Index: linux-4.17-rc7/drivers/gpu/drm/udl/udl_transfer.c
===
--- linux-4.17-rc7.orig/drivers/gpu/drm/udl/udl_transfer.c 2018-06-03
13:15:01.0 +0200
+++ linux-4.17-rc7/drivers/gpu/drm/udl/udl_transfer.c 2018-06-03
13:15:01.0 +0200
@@ -83,12 +83,12 @@ static inline u16 pixel32_to_be16(const
((pixel >> 8) & 0xf800));
}
-static inline u16 get_pixel_val16(const uint8_t *pixel, int bpp)
+static inline u16 get_pixel_val16(const uint8_t *pixel, int log_bpp)
{
- u16 pixel_val16 = 0;
- if (bpp == 2)
+ u16 pixel_val16;
+ if (log_bpp == 1)
pixel_val16 = *(const uint16_t *)pixel;
- else if (bpp == 4)
+ else
pixel_val16 = pixel32_to_be16(*(const uint32_t *)pixel);
return pixel_val16;
}
@@ -125,8 +125,9 @@ static void udl_compress_hline16(
const u8 *const pixel_end,
uint32_t *device_address_ptr,
uint8_t **command_buffer_ptr,
- const uint8_t *const cmd_buffer_end, int bpp)
+ const uint8_t *const cmd_buffer_end, int log_bpp)
{
+ const int bpp = 1 << log_bpp;
const u8 *pixel = *pixel_start_ptr;
uint32_t dev_addr = *device_address_ptr;
uint8_t *cmd = *command_buffer_ptr;
@@ -153,12 +154,12 @@ static void udl_compress_hline16(
raw_pixels_count_byte = cmd++; /* we'll know this later */
raw_pixel_start = pixel;
- cmd_pixel_end = pixel + min3(MAX_CM
[PATCH 03/21] udl-kms: handle allocation failure
Allocations larger than PAGE_ALLOC_COSTLY_ORDER are unreliable and they
may fail anytime. This patch fixes the udl kms driver so that when a large
alloactions fails, it tries to do multiple smaller allocations.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/udl/udl_main.c | 28 ++--
1 file changed, 18 insertions(+), 10 deletions(-)
Index: linux-4.16.12/drivers/gpu/drm/udl/udl_main.c
===
--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_main.c 2018-05-31
11:16:15.0 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_main.c2018-05-31
11:16:15.0 +0200
@@ -200,17 +200,22 @@ static void udl_free_urb_list(struct drm
static int udl_alloc_urb_list(struct drm_device *dev, int count, size_t size)
{
struct udl_device *udl = dev->dev_private;
- int i = 0;
struct urb *urb;
struct urb_node *unode;
char *buf;
+ size_t wanted_size = count * size;
spin_lock_init(>urbs.lock);
+retry:
udl->urbs.size = size;
INIT_LIST_HEAD(>urbs.list);
- while (i < count) {
+ sema_init(>urbs.limit_sem, 0);
+ udl->urbs.count = 0;
+ udl->urbs.available = 0;
+
+ while (udl->urbs.count * size < wanted_size) {
unode = kzalloc(sizeof(struct urb_node), GFP_KERNEL);
if (!unode)
break;
@@ -226,11 +231,16 @@ static int udl_alloc_urb_list(struct drm
}
unode->urb = urb;
- buf = usb_alloc_coherent(udl->udev, MAX_TRANSFER, GFP_KERNEL,
+ buf = usb_alloc_coherent(udl->udev, size, GFP_KERNEL,
>transfer_dma);
if (!buf) {
kfree(unode);
usb_free_urb(urb);
+ if (size > PAGE_SIZE) {
+ size /= 2;
+ udl_free_urb_list(dev);
+ goto retry;
+ }
break;
}
@@ -241,16 +251,14 @@ static int udl_alloc_urb_list(struct drm
list_add_tail(>entry, >urbs.list);
- i++;
+ up(>urbs.limit_sem);
+ udl->urbs.count++;
+ udl->urbs.available++;
}
- sema_init(>urbs.limit_sem, i);
- udl->urbs.count = i;
- udl->urbs.available = i;
-
- DRM_DEBUG("allocated %d %d byte urbs\n", i, (int) size);
+ DRM_DEBUG("allocated %d %d byte urbs\n", udl->urbs.count, (int) size);
- return i;
+ return udl->urbs.count;
}
struct urb *udl_get_urb(struct drm_device *dev)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 17/21] udlfb: set line_length in dlfb_ops_set_par
Set the variable "line_length" in the function dlfb_ops_set_par. Without
this, we get garbage if we select different videomode with fbset.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/udlfb.c |1 +
1 file changed, 1 insertion(+)
Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
===
--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-05-31
14:50:00.0 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-05-31 14:50:00.0
+0200
@@ -1057,6 +1057,7 @@ static int dlfb_ops_set_par(struct fb_in
return result;
dlfb->current_mode = fvs;
+ info->fix.line_length = info->var.xres * (info->var.bits_per_pixel / 8);
if (dlfb->fb_count == 0) {
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 08/21] udl-kms: avoid prefetch
Modern processors can detect linear memory accesses and prefetch data
automatically, so there's no need to use prefetch.
Signed-off-by: Mikulas Patocka
---
drivers/gpu/drm/udl/udl_transfer.c |7 ---
1 file changed, 7 deletions(-)
Index: linux-4.16.12/drivers/gpu/drm/udl/udl_transfer.c
===
--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_transfer.c 2018-05-31
14:48:12.0 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_transfer.c2018-05-31
14:48:12.0 +0200
@@ -13,7 +13,6 @@
#include
#include
#include
-#include
#include
#include
@@ -51,9 +50,6 @@ static int udl_trim_hline(const u8 *bbac
int start = width;
int end = width;
- prefetch((void *) front);
- prefetch((void *) back);
-
for (j = 0; j < width; j++) {
if (back[j] != front[j]) {
start = j;
@@ -140,8 +136,6 @@ static void udl_compress_hline16(
const u8 *cmd_pixel_start, *cmd_pixel_end = NULL;
uint16_t pixel_val16;
- prefetchw((void *) cmd); /* pull in one cache line at least */
-
*cmd++ = 0xaf;
*cmd++ = 0x6b;
*cmd++ = (uint8_t) ((dev_addr >> 16) & 0xFF);
@@ -158,7 +152,6 @@ static void udl_compress_hline16(
(unsigned long)(pixel_end - pixel) >>
log_bpp,
(unsigned long)(cmd_buffer_end - 1 -
cmd) / 2) << log_bpp);
- prefetch_range((void *) pixel, cmd_pixel_end - pixel);
pixel_val16 = get_pixel_val16(pixel, log_bpp);
while (pixel < cmd_pixel_end) {
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 06/21] udl-kms: make a local copy of fb_ops
The defio subsystem overwrites the method fb_osp->mmap. That method is
stored in module's static data - and that means that if we have multiple
diplaylink adapters, they will over write each other's method.
In order to avoid interference between multiple adapters, we copy the
fb_ops structure to a device-local memory.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/udl/udl_fb.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Index: linux-4.17-rc7/drivers/gpu/drm/udl/udl_fb.c
===
--- linux-4.17-rc7.orig/drivers/gpu/drm/udl/udl_fb.c2018-06-03
13:05:20.0 +0200
+++ linux-4.17-rc7/drivers/gpu/drm/udl/udl_fb.c 2018-06-03 13:08:03.0
+0200
@@ -34,6 +34,7 @@ module_param(fb_defio, int, S_IWUSR | S_
struct udl_fbdev {
struct drm_fb_helper helper;
struct udl_framebuffer ufb;
+ struct fb_ops fb_ops;
int fb_count;
};
@@ -405,7 +406,8 @@ static int udlfb_create(struct drm_fb_he
info->fix.smem_len = size;
info->fix.smem_start = (unsigned long)ufbdev->ufb.obj->vmapping;
- info->fbops = _ops;
+ ufbdev->fb_ops = udlfb_ops;
+ info->fbops = >fb_ops;
drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
drm_fb_helper_fill_var(info, >helper, sizes->fb_width,
sizes->fb_height);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 20/21] udlfb: avoid prefetch
Modern processors can detect linear memory accesses and prefetch data
automatically, so there's no need to use prefetch.
Signed-off-by: Mikulas Patocka
---
drivers/video/fbdev/udlfb.c |8
1 file changed, 8 deletions(-)
Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
===
--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-05-31
12:48:35.0 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-05-31 12:48:35.0
+0200
@@ -25,7 +25,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -375,9 +374,6 @@ static int dlfb_trim_hline(const u8 *bba
int start = width;
int end = width;
- prefetch((void *) front);
- prefetch((void *) back);
-
for (j = 0; j < width; j++) {
if (back[j] != front[j]) {
start = j;
@@ -454,8 +450,6 @@ static void dlfb_compress_hline(
continue;
}
- prefetchw((void *) cmd); /* pull in one cache line at least */
-
*cmd++ = 0xAF;
*cmd++ = 0x6B;
*cmd++ = dev_addr >> 16;
@@ -479,8 +473,6 @@ static void dlfb_compress_hline(
cmd_pixel_end--;
}
- prefetch_range((void *) pixel, (u8 *)cmd_pixel_end - (u8
*)pixel);
-
while (pixel < cmd_pixel_end) {
const uint16_t * const repeating_pixel = pixel;
u16 pixel_value = *pixel;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 09/21] udl-kms: use spin_lock_irq instead of spin_lock_irqsave
spin_lock_irqsave and spin_unlock_irqrestore is inteded to be called from
a context where it is unknown if interrupts are enabled or disabled (such
as interrupt handlers). From a process context, we should call
spin_lock_irq and spin_unlock_irq, that avoids the costly pushf and popf
instructions.
Signed-off-by: Mikulas Patocka
---
drivers/gpu/drm/udl/udl_main.c| 10 --
drivers/gpu/drm/udl/udl_modeset.c |5 ++---
2 files changed, 6 insertions(+), 9 deletions(-)
Index: linux-4.16.12/drivers/gpu/drm/udl/udl_main.c
===
--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_main.c 2018-05-31
11:17:01.0 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_main.c2018-05-31
11:17:01.0 +0200
@@ -170,7 +170,6 @@ static void udl_free_urb_list(struct drm
struct list_head *node;
struct urb_node *unode;
struct urb *urb;
- unsigned long flags;
DRM_DEBUG("Waiting for completes and freeing all render urbs\n");
@@ -178,12 +177,12 @@ static void udl_free_urb_list(struct drm
while (count--) {
down(>urbs.limit_sem);
- spin_lock_irqsave(>urbs.lock, flags);
+ spin_lock_irq(>urbs.lock);
node = udl->urbs.list.next; /* have reserved one with sem */
list_del_init(node);
- spin_unlock_irqrestore(>urbs.lock, flags);
+ spin_unlock_irq(>urbs.lock);
unode = list_entry(node, struct urb_node, entry);
urb = unode->urb;
@@ -268,7 +267,6 @@ struct urb *udl_get_urb(struct drm_devic
struct list_head *entry;
struct urb_node *unode;
struct urb *urb = NULL;
- unsigned long flags;
/* Wait for an in-flight buffer to complete and get re-queued */
ret = down_timeout(>urbs.limit_sem, GET_URB_TIMEOUT);
@@ -279,14 +277,14 @@ struct urb *udl_get_urb(struct drm_devic
goto error;
}
- spin_lock_irqsave(>urbs.lock, flags);
+ spin_lock_irq(>urbs.lock);
BUG_ON(list_empty(>urbs.list)); /* reserved one with limit_sem */
entry = udl->urbs.list.next;
list_del_init(entry);
udl->urbs.available--;
- spin_unlock_irqrestore(>urbs.lock, flags);
+ spin_unlock_irq(>urbs.lock);
unode = list_entry(entry, struct urb_node, entry);
urb = unode->urb;
Index: linux-4.16.12/drivers/gpu/drm/udl/udl_modeset.c
===
--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_modeset.c2018-05-31
11:17:01.0 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_modeset.c 2018-05-31
11:17:01.0 +0200
@@ -366,7 +366,6 @@ static int udl_crtc_page_flip(struct drm
{
struct udl_framebuffer *ufb = to_udl_fb(fb);
struct drm_device *dev = crtc->dev;
- unsigned long flags;
struct drm_framebuffer *old_fb = crtc->primary->fb;
if (old_fb) {
@@ -377,10 +376,10 @@ static int udl_crtc_page_flip(struct drm
udl_handle_damage(ufb, 0, 0, fb->width, fb->height);
- spin_lock_irqsave(>event_lock, flags);
+ spin_lock_irq(>event_lock);
if (event)
drm_crtc_send_vblank_event(crtc, event);
- spin_unlock_irqrestore(>event_lock, flags);
+ spin_unlock_irq(>event_lock);
crtc->primary->fb = fb;
return 0;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 18/21] udlfb: allow reallocating the framebuffer
This patch changes udlfb so that it may reallocate the framebuffer when
setting higher-resolution mode. If we boot the system without monitor
attached, udlfb creates a framebuffer with the size 800x600. This patch
makes it possible to select higher videomode with the fbset command when
a monitor is attached.
Note that there is no reliable way to prevent the system from touching the
old framebuffer, so we must not free it. We add it to the list
dlfb->deferred_free and free it when the driver is unloaded.
Signed-off-by: Mikulas Patocka
---
drivers/video/fbdev/udlfb.c | 70 +---
include/video/udlfb.h |1
2 files changed, 48 insertions(+), 23 deletions(-)
Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
===
--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-06-03
13:17:41.0 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-06-03 13:17:41.0
+0200
@@ -73,6 +73,13 @@ static bool fb_defio = 1; /* Detect mma
static bool shadow = 1; /* Optionally disable shadow framebuffer */
static int pixel_limit; /* Optionally force a pixel resolution limit */
+struct dlfb_deferred_free {
+ struct list_head list;
+ void *mem;
+};
+
+static int dlfb_realloc_framebuffer(struct dlfb_data *dlfb, struct fb_info
*info, u32 new_len);
+
/* dlfb keeps a list of urbs for efficient bulk transfers */
static void dlfb_urb_completion(struct urb *urb);
static struct urb *dlfb_get_urb(struct dlfb_data *dlfb);
@@ -927,6 +934,12 @@ static void dlfb_free(struct kref *kref)
{
struct dlfb_data *dlfb = container_of(kref, struct dlfb_data, kref);
+ while (!list_empty(>deferred_free)) {
+ struct dlfb_deferred_free *d =
list_entry(dlfb->deferred_free.next, struct dlfb_deferred_free, list);
+ list_del(>list);
+ vfree(d->mem);
+ kfree(d);
+ }
vfree(dlfb->backing_buffer);
kfree(dlfb->edid);
kfree(dlfb);
@@ -1020,10 +1033,6 @@ static int dlfb_ops_check_var(struct fb_
struct fb_videomode mode;
struct dlfb_data *dlfb = info->par;
- /* TODO: support dynamically changing framebuffer size */
- if ((var->xres * var->yres * 2) > info->fix.smem_len)
- return -EINVAL;
-
/* set device-specific elements of var unrelated to mode */
dlfb_var_color_format(var);
@@ -1042,6 +1051,7 @@ static int dlfb_ops_set_par(struct fb_in
u16 *pix_framebuffer;
int i;
struct fb_var_screeninfo fvs;
+ u32 line_length = info->var.xres * (info->var.bits_per_pixel / 8);
/* clear the activate field because it causes spurious miscompares */
fvs = info->var;
@@ -1051,13 +1061,17 @@ static int dlfb_ops_set_par(struct fb_in
if (!memcmp(>current_mode, , sizeof(struct
fb_var_screeninfo)))
return 0;
+ result = dlfb_realloc_framebuffer(dlfb, info, info->var.yres *
line_length);
+ if (result)
+ return result;
+
result = dlfb_set_video_mode(dlfb, >var);
if (result)
return result;
dlfb->current_mode = fvs;
- info->fix.line_length = info->var.xres * (info->var.bits_per_pixel / 8);
+ info->fix.line_length = line_length;
if (dlfb->fb_count == 0) {
@@ -1066,11 +1080,11 @@ static int dlfb_ops_set_par(struct fb_in
pix_framebuffer = (u16 *) info->screen_base;
for (i = 0; i < info->fix.smem_len / 2; i++)
pix_framebuffer[i] = 0x37e6;
-
- dlfb_handle_damage(dlfb, 0, 0, info->var.xres, info->var.yres,
- info->screen_base);
}
+ dlfb_handle_damage(dlfb, 0, 0, info->var.xres, info->var.yres,
+ info->screen_base);
+
return 0;
}
@@ -1146,21 +1160,29 @@ static struct fb_ops dlfb_ops = {
};
+static void dlfb_deferred_vfree(struct dlfb_data *dlfb, void *mem)
+{
+ struct dlfb_deferred_free *d = kmalloc(sizeof(struct
dlfb_deferred_free), GFP_KERNEL);
+ if (!d)
+ return;
+ d->mem = mem;
+ list_add(>list, >deferred_free);
+}
+
/*
* Assumes >lock held by caller
* Assumes no active clients have framebuffer open
*/
-static int dlfb_realloc_framebuffer(struct dlfb_data *dlfb, struct fb_info
*info)
+static int dlfb_realloc_framebuffer(struct dlfb_data *dlfb, struct fb_info
*info, u32 new_len)
{
- int old_len = info->fix.smem_len;
- int new_len;
+ u32 old_len = info->fix.smem_len;
unsigned char *old_fb = info->screen_base;
unsigned char *new_fb;
unsigned char *new_back = NULL;
- new_len = info->fix.line_length * info->
[PATCH 04/21] udl-kms: fix crash due to uninitialized memory
We must use kzalloc when allocating the fb_deferred_io structure.
Otherwise, the field first_io is undefined and it causes a crash.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/udl/udl_fb.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-4.16.12/drivers/gpu/drm/udl/udl_fb.c
===
--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_fb.c 2018-05-29
17:55:39.0 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_fb.c 2018-05-29 17:55:39.0
+0200
@@ -221,7 +221,7 @@ static int udl_fb_open(struct fb_info *i
struct fb_deferred_io *fbdefio;
- fbdefio = kmalloc(sizeof(struct fb_deferred_io), GFP_KERNEL);
+ fbdefio = kzalloc(sizeof(struct fb_deferred_io), GFP_KERNEL);
if (fbdefio) {
fbdefio->delay = DL_DEFIO_WRITE_DELAY;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 01/21] udl-kms: fix display corruption of the last line
The displaylink hardware has such a peculiarity that it doesn't render a
command until next command is received. This produces occasional
corruption, such as when setting 22x11 font on the console, only the first
line of the cursor will be blinking if the cursor is located at some
specific columns.
When we end up with a repeating pixel, the driver has a bug that it leaves
one uninitialized byte after the command (and this byte is enough to flush
the command and render it - thus it fixes the screen corruption), however
whe we end up with a non-repeating pixel, there is no byte appended and
this results in temporary screen corruption.
This patch fixes the screen corruption by always appending a byte 0xAF at
the end of URB. It also removes the uninitialized byte.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/udl/udl_fb.c |5 -
drivers/gpu/drm/udl/udl_transfer.c | 11 +++
2 files changed, 11 insertions(+), 5 deletions(-)
Index: linux-4.16.12/drivers/gpu/drm/udl/udl_transfer.c
===
--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_transfer.c 2018-05-31
14:47:07.0 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_transfer.c2018-05-31
14:53:31.0 +0200
@@ -153,11 +153,11 @@ static void udl_compress_hline16(
raw_pixels_count_byte = cmd++; /* we'll know this later */
raw_pixel_start = pixel;
- cmd_pixel_end = pixel + (min(MAX_CMD_PIXELS + 1,
- min((int)(pixel_end - pixel) / bpp,
- (int)(cmd_buffer_end - cmd) / 2))) * bpp;
+ cmd_pixel_end = pixel + min3(MAX_CMD_PIXELS + 1UL,
+ (unsigned long)(pixel_end - pixel) /
bpp,
+ (unsigned long)(cmd_buffer_end - 1 -
cmd) / 2) * bpp;
- prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * bpp);
+ prefetch_range((void *) pixel, cmd_pixel_end - pixel);
pixel_val16 = get_pixel_val16(pixel, bpp);
while (pixel < cmd_pixel_end) {
@@ -193,6 +193,9 @@ static void udl_compress_hline16(
if (pixel > raw_pixel_start) {
/* finalize last RAW span */
*raw_pixels_count_byte = ((pixel-raw_pixel_start) /
bpp) & 0xFF;
+ } else {
+ /* undo unused byte */
+ cmd--;
}
*cmd_pixels_count_byte = ((pixel - cmd_pixel_start) / bpp) &
0xFF;
Index: linux-4.16.12/drivers/gpu/drm/udl/udl_fb.c
===
--- linux-4.16.12.orig/drivers/gpu/drm/udl/udl_fb.c 2018-05-31
14:47:07.0 +0200
+++ linux-4.16.12/drivers/gpu/drm/udl/udl_fb.c 2018-05-31 14:53:32.0
+0200
@@ -137,7 +137,10 @@ int udl_handle_damage(struct udl_framebu
if (cmd > (char *) urb->transfer_buffer) {
/* Send partial buffer remaining before exiting */
- int len = cmd - (char *) urb->transfer_buffer;
+ int len;
+ if (cmd < (char *) urb->transfer_buffer +
urb->transfer_buffer_length)
+ *cmd++ = 0xAF;
+ len = cmd - (char *) urb->transfer_buffer;
ret = udl_submit_urb(dev, urb, len);
bytes_sent += len;
} else
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 11/21] udlfb: fix semaphore value leak
I observed that the performance of the udl fb driver degrades over time.
On a freshly booted machine, it takes 6 seconds to do "ls -la /usr/bin";
after some time of use, the same operation takes 14 seconds.
The reason is that the value of "limit_sem" decays over time.
The udl driver uses a semaphore "limit_set" to specify how many free urbs
are there on dlfb->urbs.list. If the count is zero, the "down" operation
will sleep until some urbs are added to the freelist.
In order to avoid some hypothetical deadlock, the driver will not call
"up" immediatelly, but it will offload it to a workqueue. The problem is
that if we call "schedule_delayed_work" on the same work item multiple
times, the work item may only be executed once.
This is happening:
* some urb completes
* dlfb_urb_completion adds it to the free list
* dlfb_urb_completion calls schedule_delayed_work to schedule the function
dlfb_release_urb_work to increase the semaphore count
* as the urb is on the free list, some other task grabs it and submits it
* the submitted urb completes, dlfb_urb_completion is called again
* dlfb_urb_completion calls schedule_delayed_work, but the work is already
scheduled, so it does nothing
* finally, dlfb_release_urb_work is called, it increases the semaphore
count by 1, although it should increase it by 2
So, the semaphore count is decreasing over time, and this causes gradual
performance degradation.
Note that in the current kernel, the "up" function may be called from
interrupt and it may race with the "down" function called by another
thread, so we don't have to offload the call of "up" to a workqueue at
all. This patch removes the workqueue code. The patch also changes
"down_interruptible" to "down" in dlfb_free_urb_list, so that we will
clean up the driver properly even if a signal arrives.
With this patch, the performance of udlfb no longer degrades.
Signed-off-by: Mikulas Patocka
Cc: sta...@vger.kernel.org
---
drivers/video/fbdev/udlfb.c | 27 ++-
include/video/udlfb.h |1 -
2 files changed, 2 insertions(+), 26 deletions(-)
Index: linux-4.17-rc7/drivers/video/fbdev/udlfb.c
===
--- linux-4.17-rc7.orig/drivers/video/fbdev/udlfb.c 2018-05-31
12:31:04.0 +0200
+++ linux-4.17-rc7/drivers/video/fbdev/udlfb.c 2018-05-31 12:31:04.0
+0200
@@ -922,14 +922,6 @@ static void dlfb_free(struct kref *kref)
kfree(dlfb);
}
-static void dlfb_release_urb_work(struct work_struct *work)
-{
- struct urb_node *unode = container_of(work, struct urb_node,
- release_urb_work.work);
-
- up(>dlfb->urbs.limit_sem);
-}
-
static void dlfb_free_framebuffer(struct dlfb_data *dlfb)
{
struct fb_info *info = dlfb->info;
@@ -1789,14 +1781,7 @@ static void dlfb_urb_completion(struct u
dlfb->urbs.available++;
spin_unlock_irqrestore(>urbs.lock, flags);
- /*
-* When using fb_defio, we deadlock if up() is called
-* while another is waiting. So queue to another process.
-*/
- if (fb_defio)
- schedule_delayed_work(>release_urb_work, 0);
- else
- up(>urbs.limit_sem);
+ up(>urbs.limit_sem);
}
static void dlfb_free_urb_list(struct dlfb_data *dlfb)
@@ -1805,16 +1790,11 @@ static void dlfb_free_urb_list(struct dl
struct list_head *node;
struct urb_node *unode;
struct urb *urb;
- int ret;
unsigned long flags;
/* keep waiting and freeing, until we've got 'em all */
while (count--) {
-
- /* Getting interrupted means a leak, but ok at disconnect */
- ret = down_interruptible(>urbs.limit_sem);
- if (ret)
- break;
+ down(>urbs.limit_sem);
spin_lock_irqsave(>urbs.lock, flags);
@@ -1854,9 +1834,6 @@ static int dlfb_alloc_urb_list(struct dl
break;
unode->dlfb = dlfb;
- INIT_DELAYED_WORK(>release_urb_work,
- dlfb_release_urb_work);
-
urb = usb_alloc_urb(0, GFP_KERNEL);
if (!urb) {
kfree(unode);
Index: linux-4.17-rc7/include/video/udlfb.h
===
--- linux-4.17-rc7.orig/include/video/udlfb.h 2018-05-31 12:31:04.0
+0200
+++ linux-4.17-rc7/include/video/udlfb.h2018-05-31 12:31:04.0
+0200
@@ -20,7 +20,6 @@ struct dloarea {
struct urb_node {
struct list_head entry;
struct dlfb_data *dlfb;
- struct delayed_work release_urb_work;
struct urb *urb;
};
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/nouveau/fbcon: fix font width not divisible by 8
The patch f045f459d925 ("drm/nouveau/fbcon: fix out-of-bounds memory accesses")
tries to fix some out of memory accesses. Unfortunatelly, the patch breaks the
display when using fonts with width that is not divisiable by 8.
The monochrome bitmap for each character is stored in memory by lines from top
to bottom. Each line is padded to a full byte.
For example, for 22x11 font, each line is padded to 16 bits, so each
character is consuming 44 bytes total, that is 11 32-bit words. The patch
f045f459d925 changed the logic to "dsize = ALIGN(image->width *
image->height, 32) >> 5", that is just 8 words - this is incorrect and it
causes display corruption.
This patch adds the necesary padding of lines to 8 bytes.
This patch should be backported to stable kernels where f045f459d925 was
backported.
Signed-off-by: Mikulas Patocka
Fixes: f045f459d925 ("drm/nouveau/fbcon: fix out-of-bounds memory accesses")
Cc: stable at vger.kernel.org
Index: linux-2.6/drivers/gpu/drm/nouveau/nvc0_fbcon.c
===
--- linux-2.6.orig/drivers/gpu/drm/nouveau/nvc0_fbcon.c
+++ linux-2.6/drivers/gpu/drm/nouveau/nvc0_fbcon.c
@@ -125,7 +125,7 @@ nvc0_fbcon_imageblit(struct fb_info *inf
OUT_RING (chan, 0);
OUT_RING (chan, image->dy);
- dwords = ALIGN(image->width * image->height, 32) >> 5;
+ dwords = ALIGN(ALIGN(image->width, 8) * image->height, 32) >> 5;
while (dwords) {
int push = dwords > 2047 ? 2047 : dwords;
Index: linux-2.6/drivers/gpu/drm/nouveau/nv04_fbcon.c
===
--- linux-2.6.orig/drivers/gpu/drm/nouveau/nv04_fbcon.c
+++ linux-2.6/drivers/gpu/drm/nouveau/nv04_fbcon.c
@@ -107,11 +107,11 @@ nv04_fbcon_imageblit(struct fb_info *inf
((image->dx + image->width) & 0x));
OUT_RING(chan, bg);
OUT_RING(chan, fg);
- OUT_RING(chan, (image->height << 16) | image->width);
+ OUT_RING(chan, (image->height << 16) | ALIGN(image->width, 8));
OUT_RING(chan, (image->height << 16) | image->width);
OUT_RING(chan, (image->dy << 16) | (image->dx & 0x));
- dsize = ALIGN(image->width * image->height, 32) >> 5;
+ dsize = ALIGN(ALIGN(image->width, 8) * image->height, 32) >> 5;
while (dsize) {
int iter_len = dsize > 128 ? 128 : dsize;
Index: linux-2.6/drivers/gpu/drm/nouveau/nv50_fbcon.c
===
--- linux-2.6.orig/drivers/gpu/drm/nouveau/nv50_fbcon.c
+++ linux-2.6/drivers/gpu/drm/nouveau/nv50_fbcon.c
@@ -125,7 +125,7 @@ nv50_fbcon_imageblit(struct fb_info *inf
OUT_RING(chan, 0);
OUT_RING(chan, image->dy);
- dwords = ALIGN(image->width * image->height, 32) >> 5;
+ dwords = ALIGN(ALIGN(image->width, 8) * image->height, 32) >> 5;
while (dwords) {
int push = dwords > 2047 ? 2047 : dwords;
3.14-rc7 crashes in drm ([PATCH] a crash in mga_driver_irq_uninstall)
On Mon, 24 Mar 2014, Daniel Vetter wrote: > >> Like I've said the entire teardown sequence for legacy drm drivers is > >> terminally busted, so the only hope we have is to reapply this missing > >> duct-tape which made your X crash. But if that itself isn't a regression > >> there's no way to fix the current drm/mga driver without a complete > >> rewrite as a new-style kernel modesetting driver. > >> -Daniel > > > > If someone understands the locking issues I pointed out above, it could be > > easy to fix. > > The locking issue isn't your problem, the real issue is that putting a > irq_uninstall into core code will break all the new (properly working) > drivers. And you can't really fix this in mga itself since the > lifetime rules of the register mappings are totally broken. It's a > fundamental misdesign of the legacy drm driver architecture and the > _only_ way to fix this bug for real is to rewrite this all. Which was > done for all the still used drivers like i915, radeon, nouveau, ... > -Daniel When I tried Radeon AGP card with the KMS driver, it lacked the possibility to set video mode with fbset and the framebuffer console was very slow because it wasn't accelerated. So, Radeon with the new driver is much less useable than Matrox. Did I misconfigure something? Or, is console acceleration and modesetting deliberately unsupported in KMS drivers? Mikulas
3.14-rc7 crashes in drm ([PATCH] a crash in mga_driver_irq_uninstall)
On Mon, 24 Mar 2014, Daniel Vetter wrote: > On Mon, Mar 24, 2014 at 01:17:12PM -0400, Mikulas Patocka wrote: > > > > > Hmm, given that Mikulas in > > > > > https://lkml.org/lkml/2014/2/26/537 > > > > > offered a diff of linux-3.13.5 files, it truly seems (shock! ack! > > > > > noo!) > > > > > that that indeed may have been a regression at <= 3.13 proper even > > > > > (which may pose interesting questions about the level of testing > > > > > coverage > > > > > we still enjoy [not!?] in this hardware area). > > > > That patch drops a mutex, so it is not correct. There is mutex resursion - > > we need to uninstall the irq in drm_master_destroy, because here we are > > committed to destroy the device. But the routine that uninstalls the irq > > takes struct_mutex, which is already held in drm_master_destroy. > > > > I suppose that the person who maintains drm reworks the patch so that it's > > correct: > > > > - could we use a different mutex to protect the irq in drm_irq.c? Or > > possibly no mutex at all and use cmpxchg to manipulate the variable > > dev->irq_enabled? - this seems like the best solution. But I am not sure > > if the code in drm_irq.c somehow depends on the facts that other parts of > > the drm subsystem take struct_mutex. > > > > - could we pass a new argument to drm_irq_uninstall that tells it not to > > take the mutex? drm_master_destroy would set this argument to 1. > > drm_master_destroy is mostly called with struct_mutex held, but there may > > be places in vmwgfx_drv.c where drm_master_put (which calls > > drm_master_destroy) may be called without struct_mutex held. > > > > Is it true that drm_master_destroy can be called without struct_mutex > > held? I don't know. > > > > > > I think drm maintainer should sort out the above issues and modify the > > patch accordingly. > > > > > -> All hell breaks loose if Xorg dies and takes all it's mappings with it > > > (in master_destroy, since the Xorg /dev fd is the master) and leaves the > > > driver hanging in the air if there's an interrupt still pending (or > > > anything else fwiw). > > > > For me that crash happened when xorg exited with a fatal error too. > > Is this fatal error itself a regression or have you seen that on older > kernels, too? In my case that Xorg error was not kernel-related at all. It happened because of unknown symbol because I used mga_dri.so from Debian 6 in Debian 7 (mga_dri.so isn't shipped in Debian 7 anymore). I can still play quake with that old mga_dri.so, although in some other scenario it causes failure because of unknown symbol. I should probably recompile mga_dri on my own. > Like I've said the entire teardown sequence for legacy drm drivers is > terminally busted, so the only hope we have is to reapply this missing > duct-tape which made your X crash. But if that itself isn't a regression > there's no way to fix the current drm/mga driver without a complete > rewrite as a new-style kernel modesetting driver. > -Daniel If someone understands the locking issues I pointed out above, it could be easy to fix. Mikulas
3.14-rc7 crashes in drm ([PATCH] a crash in mga_driver_irq_uninstall)
On Mon, 24 Mar 2014, Daniel Vetter wrote: > On Mon, Mar 24, 2014 at 07:45:47AM +1000, Dave Airlie wrote: > > On Mon, Mar 24, 2014 at 7:27 AM, Andreas Mohr wrote: > > > On Sun, Mar 23, 2014 at 09:39:16AM -0700, Linus Torvalds wrote: > > >> On Sun, Mar 23, 2014 at 5:15 AM, Andreas Mohr wrote: > > >> > > > >> > which did end up flawless on 3.12.0-rc2+, too > > >> > (but failed to improve the issue on 3.14.0-rc7+). > > >> > > > >> > So, for all intents and purposes, drm infrastructure seems unavoidably > > >> > (neither dri disable nor libdrm upgrade helps) affected. > > >> > Does anyone know which change caused that issue? > > >> > (I'm asking because bisect here would be relatively painful). > > >> > > >> So 3.12-rc2 works. Does 3.13 work? Is this a regression in the current > > >> 3.14 rc only, or did it happen already in the previous release? > > > > > > Hmm, given that Mikulas in > > > https://lkml.org/lkml/2014/2/26/537 > > > offered a diff of linux-3.13.5 files, it truly seems (shock! ack! noo!) > > > that that indeed may have been a regression at <= 3.13 proper even > > > (which may pose interesting questions about the level of testing coverage > > > we still enjoy [not!?] in this hardware area). That patch drops a mutex, so it is not correct. There is mutex resursion - we need to uninstall the irq in drm_master_destroy, because here we are committed to destroy the device. But the routine that uninstalls the irq takes struct_mutex, which is already held in drm_master_destroy. I suppose that the person who maintains drm reworks the patch so that it's correct: - could we use a different mutex to protect the irq in drm_irq.c? Or possibly no mutex at all and use cmpxchg to manipulate the variable dev->irq_enabled? - this seems like the best solution. But I am not sure if the code in drm_irq.c somehow depends on the facts that other parts of the drm subsystem take struct_mutex. - could we pass a new argument to drm_irq_uninstall that tells it not to take the mutex? drm_master_destroy would set this argument to 1. drm_master_destroy is mostly called with struct_mutex held, but there may be places in vmwgfx_drv.c where drm_master_put (which calls drm_master_destroy) may be called without struct_mutex held. Is it true that drm_master_destroy can be called without struct_mutex held? I don't know. I think drm maintainer should sort out the above issues and modify the patch accordingly. > > > Oh well, seems I'll have to prepare/build 3.13 now... > > > > It's > 15 year old hardware, so yes I believe we have close to 0 > > testing coverage on it outside of distros, > > > > I'm not even sure I have one anymore, I might be able to test an MGA in one > > box. > > I haven't done a full read of all the related code, but this smells like a > similar bug I've hit all over the place in the i810 driver (another one of > those undead drm drivers of yonders). Ingredients: > > 1) Xorg creates a drm mapping of the register space. > 2) Xorg tells the hw-specific drm which drm mapping has the hw registers, > and the driver uses that. Iirc this has been done as some form of OS > abstraction. Also note that these mappings aren't refcounted, so the first > guy to call drm_rmmap wins. > > -> All hell breaks loose if Xorg dies and takes all it's mappings with it > (in master_destroy, since the Xorg /dev fd is the master) and leaves the > driver hanging in the air if there's an interrupt still pending (or > anything else fwiw). For me that crash happened when xorg exited with a fatal error too. Mikulas
[PATCH] a crash in mga_driver_irq_uninstall
Hi I'm getting a reproducible crash in kernel MGA DRM driver. The crash happens in the following way: drm_release is called drm_release calls drm_master_put(_priv->master); drm_master_put drops a reference and calls drm_master_destroy drm_master_destroy calls drm_rmmap_locked to unmap the card's mmio space drm_release continues to execute drm_release calls drm_lastclose drm_lastclose calls drm_irq_uninstall drm_irq_uninstall calls dev->driver->irq_uninstall (mga_driver_irq_uninstall) mga_driver_irq_uninstall performs MGA_WRITE(MGA_IEN, 0), it crashes because the mmio range was unmapped The result are these two crashes - one in mga_driver_irq_uninstall and the other in mga_driver_irq_handler: [44272.019284] BUG: unable to handle kernel paging request at e4831e1c [44272.021000] IP: [] mga_driver_irq_uninstall+0x18/0x28 [mga] [44272.021000] *pde = 1e7ba067 *pte = [44272.021000] Oops: 0002 [#1] [44272.021000] Modules linked in: mga drm hid_generic usbhid hid ipv6 cpufreq_stats cpufreq_conservative cpufreq_powersave cpufreq_ondemand cpufreq_userspace plip spadfs hpfs nls_cp852 msdos fat matroxfb_base matroxfb_g450 matroxfb_accel cfbfillrect cfbimgblt cfbcopyarea matroxfb_DAC1064 g450_pll matroxfb_misc floppy dm_crypt snd_usb_audio snd_usbmidi_lib snd_hwdep snd_seq_midi snd_seq_midi_event snd_rawmidi snd_pcm snd_page_alloc snd_seq snd_seq_device snd_timer snd soundcore powernow_k6 pcspkr evdev ehci_pci via686a i2c_viapro e1000 i2c_core uhci_hcd ehci_hcd via_agp usbcore usb_common parport_pc agpgart parport dm_mod ext4 crc16 jbd2 mbcache sata_promise unix [44272.021000] CPU: 0 PID: 3140 Comm: Xorg Not tainted 3.13.5 #5 [44272.021000] Hardware name: System Manufacturer Product Name/VA-503A, BIOS 4.51 PG 08/02/00 [44272.021000] task: c043ce80 ti: de662000 task.ti: de662000 [44272.021000] EIP: 0060:[] EFLAGS: 00213286 CPU: 0 [44272.021000] EIP is at mga_driver_irq_uninstall+0x18/0x28 [mga] [44272.021000] EAX: de87fc00 EBX: de87fc00 ECX: e483 EDX: [44272.021000] ESI: 0001 EDI: 0001 EBP: 00203202 ESP: de663e58 [44272.021000] DS: 007b ES: 007b FS: GS: SS: 0068 [44272.021000] CR0: 8005003b CR2: e4831e1c CR3: 1e666000 CR4: 0090 [44272.021000] Stack: [44272.021000] e47d17ae e47f0064 e47e6ca2 e47f0079 000f 0187fc00 c0096e40 [44272.021000] de87fc00 0001 de87fc00 e481a277 0001 e481ee1f e481eaa7 e481ee1d [44272.021000] de7a6880 00150012 e47d6b2d 00017915 e47d6b2d df401d00 c0096e40 de87fc00 [44272.021000] Call Trace: [44272.021000] [] ? drm_irq_uninstall+0xae/0x180 [drm] [44272.021000] [] ? mga_do_cleanup_dma+0x237/0x300 [mga] [44272.021000] [] ? drm_ht_remove+0x2d/0x40 [drm] [44272.021000] [] ? drm_ht_remove+0x2d/0x40 [drm] [44272.021000] [] ? drm_lastclose+0x3e/0x180 [drm] [44272.021000] [] ? drm_release+0x37b/0x520 [drm] [44272.021000] [] ? __fput+0x72/0x1c0 [44272.021000] [] ? task_work_run+0x79/0xa0 [44272.021000] [] ? do_exit+0x18f/0x740 [44272.021000] [] ? vfs_writev+0x2c/0x60 [44272.021000] [] ? SyS_writev+0x4a/0xc0 [44272.021000] [] ? do_group_exit+0x26/0x60 [44272.021000] [] ? SyS_exit_group+0x11/0x20 [44272.021000] [] ? syscall_call+0x7/0xb [44272.021000] Code: 1e 00 00 b0 00 5b c3 8d b6 00 00 00 00 8d bf 00 00 00 00 8b 90 d8 00 00 00 85 d2 74 1b 8b 92 dc 00 00 00 8b 4a 10 ba 00 00 00 00 <89> 91 1c 1e 00 00 c6 80 80 00 00 00 00 c3 00 00 ba 00 f9 81 e4 [44272.021000] EIP: [] mga_driver_irq_uninstall+0x18/0x28 [mga] SS:ESP 0068:de663e58 [44272.021000] CR2: e4831e1c [44272.021000] ---[ end trace 68cd6cc5ef884eb2 ]--- [44272.021000] Fixing recursive fault but reboot is needed! [44272.651150] BUG: unable to handle kernel paging request at e4831e14 [44272.654217] IP: [] mga_driver_irq_handler+0x14/0xe0 [mga] [44272.654217] *pde = 1e7ba067 *pte = [44272.654217] Oops: [#2] [44272.654217] Modules linked in: mga drm hid_generic usbhid hid ipv6 cpufreq_stats cpufreq_conservative cpufreq_powersave cpufreq_ondemand cpufreq_userspace plip spadfs hpfs nls_cp852 msdos fat matroxfb_base matroxfb_g450 matroxfb_accel cfbfillrect cfbimgblt cfbcopyarea matroxfb_DAC1064 g450_pll matroxfb_misc floppy dm_crypt snd_usb_audio snd_usbmidi_lib snd_hwdep snd_seq_midi snd_seq_midi_event snd_rawmidi snd_pcm snd_page_alloc snd_seq snd_seq_device snd_timer snd soundcore powernow_k6 pcspkr evdev ehci_pci via686a i2c_viapro e1000 i2c_core uhci_hcd ehci_hcd via_agp usbcore usb_common parport_pc agpgart parport dm_mod ext4 crc16 jbd2 mbcache sata_promise unix [44272.654217] CPU: 0 PID: 0 Comm: swapper Tainted: G D 3.13.5 #5 [44272.654217] Hardware name: System Manufacturer Product Name/VA-503A, BIOS 4.51 PG 08/02/00 [44272.654217] task: c132f500 ti: df406000 task.ti: c1324000 [44272.654217] EIP: 0060:[] EFLAGS: 00210082 CPU: 0 [44272.654217] EIP is at mga_driver_irq_handler+0x14/0xe0 [mga] [44272.654217] EAX: de87fc00 EBX: df640a00 ECX: e0860803 EDX: e483 [44272.654217] ESI: 0080 EDI:
