From fa4270d8e0257b4b76f11baa2866f4313d29aaf5 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 30 Sep 2015 19:21:34 +0300 Subject: drm: Don't zero vblank timestamps from the irq handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we couldn't get a high precisions vblank timestamp, we currently store a zeroed timestamp instead and assume the next vblank irq to get us something better. This makes sense when trying to update the timestamp from eg. vblank enable. But if we do this from the vblank irq we will never get a vblank timestamp unless we high precision timestamps are available and succeeded. This break weston for instance on drivers lacking high precision timestamps. To fix this, zero the timestamp only when not called from vbl irq. When called from the irq, we still want the timestamp, even if not perfect. This fixes a regression from 4dfd64862ff852df drm: Use vblank timestamps to guesstimate how many vblanks were missed Cc: Mario Kleiner Cc: Thierry Reding Reported-by: Thierry Reding Signed-off-by: Ville Syrjälä Tested-by: Thierry Reding Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index ed2394e1720b..6bff6d3e570e 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -232,10 +232,11 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, /* * Only reinitialize corresponding vblank timestamp if high-precision query - * available and didn't fail. Otherwise reinitialize delayed at next vblank - * interrupt and assign 0 for now, to mark the vblanktimestamp as invalid. + * available and didn't fail, or we were called from the vblank interrupt. + * Otherwise reinitialize delayed at next vblank interrupt and assign 0 + * for now, to mark the vblanktimestamp as invalid. */ - if (!rc) + if (!rc && (flags & DRM_CALLED_FROM_VBLIRQ) == 0) t_vblank = (struct timeval) {0, 0}; store_vblank(dev, pipe, diff, &t_vblank, cur_vblank); -- cgit From 88e72717c2de4181d8a6de1b04315953ad2bebdf Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Thu, 24 Sep 2015 18:35:31 +0200 Subject: drm/irq: Use unsigned int pipe in public API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This continues the pattern started in commit cc1ef118fc09 ("drm/irq: Make pipe unsigned and name consistent"). This is applied to the public APIs and driver callbacks, so pretty much all drivers need to be updated to match the new prototypes. Cc: Christian König Cc: Alex Deucher Cc: Russell King Cc: Inki Dae Cc: Jianwei Wang Cc: Alison Wang Cc: Patrik Jakobsson Cc: Daniel Vetter Cc: Jani Nikula Cc: Philipp Zabel Cc: David Airlie Cc: Rob Clark Cc: Ben Skeggs Cc: Tomi Valkeinen Cc: Laurent Pinchart Cc: Mark Yao Cc: Benjamin Gaignard Cc: Vincent Abriou Cc: Thomas Hellstrom Signed-off-by: Thierry Reding Reviewed-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 6bff6d3e570e..0659d9956b7c 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -877,7 +877,7 @@ drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, * Returns: * The software vblank counter. */ -u32 drm_vblank_count(struct drm_device *dev, int pipe) +u32 drm_vblank_count(struct drm_device *dev, unsigned int pipe) { struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; -- cgit From b44f84081b8db1b5830cbd30280ba1109cc1a084 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 30 Sep 2015 16:46:48 +0300 Subject: drm: Stop using drm_vblank_count() as the hw frame counter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drm_vblank_count() returns the software counter. We should not pretend it's the hw counter since we use the hw counter to figuere out what the software counter value should be. So instead provide a new function drm_vblank_no_hw_counter() for drivers that don't have a real hw counter. The new function simply returns 0, which is about the only thing it can do. Cc: Vincent Abriou Cc: Thierry Reding Signed-off-by: Ville Syrjälä Reviewed-by: Vincent Abriou [danvet: s/int pipe/unsigned int pipe/ to follow Thierry's interface change.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 0659d9956b7c..7bdf247b9681 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1798,3 +1798,20 @@ bool drm_crtc_handle_vblank(struct drm_crtc *crtc) return drm_handle_vblank(crtc->dev, drm_crtc_index(crtc)); } EXPORT_SYMBOL(drm_crtc_handle_vblank); + +/** + * drm_vblank_no_hw_counter - "No hw counter" implementation of .get_vblank_counter() + * @dev: DRM device + * @pipe: CRTC for which to read the counter + * + * Drivers can plug this into the .get_vblank_counter() function if + * there is no useable hardware frame counter available. + * + * Returns: + * 0 + */ +u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe) +{ + return 0; +} +EXPORT_SYMBOL(drm_vblank_no_hw_counter); -- cgit From 44844892cb94c4a6a550c0e7bfa9c667f213ee21 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Fri, 9 Oct 2015 22:57:36 +0300 Subject: drm: Don't use '\' for string literal concatenation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit String literals get concatenated just fine on their own, no need to use '\'. Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 7bdf247b9681..bc2e7c69509d 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1272,8 +1272,8 @@ void drm_vblank_off(struct drm_device *dev, unsigned int pipe) list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) { if (e->pipe != pipe) continue; - DRM_DEBUG("Sending premature vblank event on disable: \ - wanted %d, current %d\n", + DRM_DEBUG("Sending premature vblank event on disable: " + "wanted %d, current %d\n", e->event.sequence, seq); list_del(&e->base.link); drm_vblank_put(dev, pipe); -- cgit From 235fabe09b46469adad2c9e4cb0563758155187c Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Fri, 9 Oct 2015 22:57:37 +0300 Subject: drm: Add DRM_DEBUG_VBL() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new debug class for _verbose_ debug message from the vblank code. That is message we spew out potentially for every vblank interrupt. Thierry already got annoyed at the spew, and now I managed to lock up my box with these debug prints (seems serial console + a few debug prints every vblank aren't a good combination). Or should I maybe call it DRM_DEBUG_IRQ? Cc: Thierry Reding Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index bc2e7c69509d..eba6337f5860 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -213,17 +213,17 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, diff = DIV_ROUND_CLOSEST_ULL(diff_ns, framedur_ns); if (diff == 0 && flags & DRM_CALLED_FROM_VBLIRQ) - DRM_DEBUG("crtc %u: Redundant vblirq ignored." - " diff_ns = %lld, framedur_ns = %d)\n", - pipe, (long long) diff_ns, framedur_ns); + DRM_DEBUG_VBL("crtc %u: Redundant vblirq ignored." + " diff_ns = %lld, framedur_ns = %d)\n", + pipe, (long long) diff_ns, framedur_ns); } else { /* some kind of default for drivers w/o accurate vbl timestamping */ diff = (flags & DRM_CALLED_FROM_VBLIRQ) != 0; } - DRM_DEBUG("updating vblank count on crtc %u:" - " current=%u, diff=%u, hw=%u hw_last=%u\n", - pipe, vblank->count, diff, cur_vblank, vblank->last); + DRM_DEBUG_VBL("updating vblank count on crtc %u:" + " current=%u, diff=%u, hw=%u hw_last=%u\n", + pipe, vblank->count, diff, cur_vblank, vblank->last); if (diff == 0) { WARN_ON_ONCE(cur_vblank != vblank->last); @@ -800,11 +800,11 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, etime = ktime_sub_ns(etime, delta_ns); *vblank_time = ktime_to_timeval(etime); - DRM_DEBUG("crtc %u : v 0x%x p(%d,%d)@ %ld.%ld -> %ld.%ld [e %d us, %d rep]\n", - pipe, vbl_status, hpos, vpos, - (long)tv_etime.tv_sec, (long)tv_etime.tv_usec, - (long)vblank_time->tv_sec, (long)vblank_time->tv_usec, - duration_ns/1000, i); + DRM_DEBUG_VBL("crtc %u : v 0x%x p(%d,%d)@ %ld.%ld -> %ld.%ld [e %d us, %d rep]\n", + pipe, vbl_status, hpos, vpos, + (long)tv_etime.tv_sec, (long)tv_etime.tv_usec, + (long)vblank_time->tv_sec, (long)vblank_time->tv_usec, + duration_ns/1000, i); return ret; } -- cgit From 0c545ac4815657e0b062344c690ea35a11eeaec8 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Thu, 12 Nov 2015 14:34:18 +0200 Subject: drm: Don't oops in drm_calc_timestamping_constants() if drm_vblank_init() wasn't called MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seems the crtc helpers call drm_calc_timestamping_constants() unconditionally even if the driver didn't initialize vblank support by calling drm_vblank_init(). That used to be OK since the constants were stored under drm_crtc. However I broke this with commit eba1f35dfe14 ("drm: Move timestamping constants into drm_vblank_crtc") when I moved the constants to live inside the drm_vblank_crtc struct instead. If drm_vblank_init() isn't called, we don't allocate these structures, and so drm_calc_timestamping_constants() will oops. Fix it by adding a check into drm_calc_timestamping_constants() to see if vblank support was initialized at all. And to keep in line with other such checks, also toss in a check and warn for the case where vblank support was initialized, but the wrong number of crtcs was specified. Fixes the following sort of oops: BUG: unable to handle kernel NULL pointer dereference at 00000000000000b0 IP: [] drm_calc_timestamping_constants+0x86/0x130 [drm] PGD 0 Oops: 0002 [#1] SMP Modules linked in: sr_mod cdrom mgag200(+) i2c_algo_bit drm_kms_helper ahci syscopyarea sysfillrect sysimgblt libahci fb_sys_fops bnx2x ttm tg3(+) mdio drm ptp sd_mod libata i2c_core pps_core libcrc32c hpsa dm_mirror dm_region_hash dm_log dm_mod CPU: 0 PID: 418 Comm: kworker/0:2 Not tainted 4.3.0+ #1 Hardware name: HP ProLiant DL380 Gen9, BIOS P89 06/09/2015 Workqueue: events work_for_cpu_fn task: ffff88046ca95500 ti: ffff88007830c000 task.ti: ffff88007830c000 RIP: 0010:[] [] drm_calc_timestamping_constants+0x86/0x130 [drm] RSP: 0018:ffff88007830f4e8 EFLAGS: 00010246 RAX: 0000000000fe4c00 RBX: ffff88006a849160 RCX: 0000000000000540 RDX: 0000000000000000 RSI: 000000000000fde8 RDI: ffff88006a849000 RBP: ffff88007830f518 R08: ffff88007830c000 R09: 00000001b87e3712 R10: 00000000000050c4 R11: 0000000000000000 R12: 0000000000fe4c00 R13: ffff88006a849000 R14: 0000000000000000 R15: 000000000000fde8 FS: 0000000000000000(0000) GS:ffff88046f800000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00000000000000b0 CR3: 00000000019d6000 CR4: 00000000001406f0 Stack: ffff88007830f518 ffff88006a849000 ffff880c69b90340 ffff880c69b90000 ffff880c69b90348 ffff880c69b90340 ffff88007830f748 ffffffffa042f7e7 ffff88006a849090 0000000000000000 ffff88006a849160 0000000000000000 Call Trace: [] drm_crtc_helper_set_mode+0x3d7/0x4b0 [drm_kms_helper] [] drm_crtc_helper_set_config+0x8d4/0xb10 [drm_kms_helper] [] drm_mode_set_config_internal+0x64/0x100 [drm] [] drm_fb_helper_pan_display+0xa2/0x280 [drm_kms_helper] [] fb_pan_display+0xbb/0x170 [] bit_update_start+0x20/0x50 [] fbcon_switch+0x39b/0x590 [] redraw_screen+0x1a0/0x240 [] do_bind_con_driver+0x2ee/0x310 [] do_take_over_console+0x141/0x1b0 [] do_fbcon_takeover+0x57/0xb0 [] fbcon_event_notify+0x60b/0x750 [] notifier_call_chain+0x49/0x70 [] __blocking_notifier_call_chain+0x4d/0x70 [] blocking_notifier_call_chain+0x16/0x20 [] fb_notifier_call_chain+0x1b/0x20 [] register_framebuffer+0x1f1/0x330 [] drm_fb_helper_initial_config+0x27a/0x3d0 [drm_kms_helper] [] mgag200_fbdev_init+0xdd/0xf0 [mgag200] [] mgag200_modeset_init+0x176/0x1e0 [mgag200] [] mgag200_driver_load+0x3f9/0x580 [mgag200] [] drm_dev_register+0xa7/0xb0 [drm] [] drm_get_pci_dev+0x8f/0x1e0 [drm] [] mga_pci_probe+0x9b/0xc0 [mgag200] [] local_pci_probe+0x45/0xa0 [] work_for_cpu_fn+0x14/0x20 [] process_one_work+0x14c/0x3c0 [] worker_thread+0x244/0x470 [] ? __schedule+0x2aa/0x760 [] ? rescuer_thread+0x310/0x310 [] kthread+0xd8/0xf0 [] ? kthread_park+0x60/0x60 [] ret_from_fork+0x3f/0x70 [] ? kthread_park+0x60/0x60 Code: f6 31 d2 41 89 c2 8b 83 b4 00 00 00 0f af c1 48 98 48 69 c0 40 42 0f 00 48 f7 f6 f6 43 74 10 41 89 c4 75 26 f6 05 9a 6f 03 00 01 <45> 89 96 b0 00 00 00 45 89 a6 ac 00 00 00 75 35 48 83 c4 08 5b RIP [] drm_calc_timestamping_constants+0x86/0x130 [drm] RSP CR2: 00000000000000b0 Cc: Jeff Moyer Reported-by: Jeff Moyer References: http://lists.freedesktop.org/archives/dri-devel/2015-November/094217.html Fixes: eba1f35dfe14 ("drm: Move timestamping constants into drm_vblank_crtc") Signed-off-by: Ville Syrjälä Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_irq.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index eba6337f5860..2151ea551d3b 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -616,10 +616,18 @@ int drm_control(struct drm_device *dev, void *data, void drm_calc_timestamping_constants(struct drm_crtc *crtc, const struct drm_display_mode *mode) { - struct drm_vblank_crtc *vblank = &crtc->dev->vblank[drm_crtc_index(crtc)]; + struct drm_device *dev = crtc->dev; + unsigned int pipe = drm_crtc_index(crtc); + struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; int linedur_ns = 0, framedur_ns = 0; int dotclock = mode->crtc_clock; + if (!dev->num_crtcs) + return; + + if (WARN_ON(pipe >= dev->num_crtcs)) + return; + /* Valid dotclock? */ if (dotclock > 0) { int frame_size = mode->crtc_htotal * mode->crtc_vtotal; -- cgit