From 7119ad5f6b0ce222072b0c0b31c3b4ac87660afa Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Mon, 12 Jan 2015 21:10:12 +0100 Subject: [PATCH 1/6] drm: Drop superfluous "select VT_HW_CONSOLE_BINDING" commit 765d5b9c2b72f5b9 ("fbdev: fbcon: select VT_HW_CONSOLE_BINDING") made FRAMEBUFFER_CONSOLE always select VT_HW_CONSOLE_BINDING, but forgot to remove select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE from the individual drivers' sections that already did this before. Remove it, also from new drivers. Signed-off-by: Geert Uytterhoeven Signed-off-by: Daniel Vetter --- drivers/gpu/drm/exynos/Kconfig | 1 - drivers/gpu/drm/rockchip/Kconfig | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig index 7f9f6f9e9b7e..c072999b7e03 100644 --- a/drivers/gpu/drm/exynos/Kconfig +++ b/drivers/gpu/drm/exynos/Kconfig @@ -6,7 +6,6 @@ config DRM_EXYNOS select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT - select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE select VIDEOMODE_HELPERS help Choose this option if you have a Samsung SoC EXYNOS chipset. diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig index 0d87bf6ddd96..cb21e3821244 100644 --- a/drivers/gpu/drm/rockchip/Kconfig +++ b/drivers/gpu/drm/rockchip/Kconfig @@ -7,7 +7,6 @@ config DRM_ROCKCHIP select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT - select VT_HW_CONSOLE_BINDING if FRAMEBUFFER_CONSOLE select VIDEOMODE_HELPERS help Choose this option if you have a Rockchip soc chipset. From 01934c2a691882185b3021d437df13bcba07711d Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Fri, 19 Dec 2014 11:21:32 +0100 Subject: [PATCH 2/6] drm/fb-helper: Propagate errors from initial config failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make drm_fb_helper_initial_config() return an int rather than a bool so that the error can be properly propagated. While at it, update drivers to propagate errors further rather than just ignore them. v2: - cirrus: No cleanup is required, the top-level cirrus_driver_load() will do it as part of cirrus_driver_unload() in its cleanup path. Reported-by: Fengguang Wu Cc: David Airlie Cc: Daniel Vetter Cc: Patrik Jakobsson Cc: Rob Clark Cc: Tomi Valkeinen Cc: Alex Deucher Cc: Christian König Cc: Ben Skeggs Signed-off-by: Thierry Reding Reviewed-by: Alex Deucher Reviewed-by: Patrik Jakobsson Reviewed-by: Christian König [danvet: Squash in simplification patch from kbuild.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/ast/ast_fb.c | 21 +++++++++++++++------ drivers/gpu/drm/bochs/bochs_fbdev.c | 14 ++++++++++++-- drivers/gpu/drm/cirrus/cirrus_fbdev.c | 12 ++++++------ drivers/gpu/drm/drm_fb_helper.c | 2 +- drivers/gpu/drm/gma500/framebuffer.c | 22 ++++++++++++++++++---- drivers/gpu/drm/mgag200/mgag200_fb.c | 12 ++++++++++-- drivers/gpu/drm/msm/msm_fbdev.c | 10 ++++++++-- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 21 +++++++++++++++------ drivers/gpu/drm/omapdrm/omap_fbdev.c | 10 ++++++++-- drivers/gpu/drm/qxl/qxl_fb.c | 22 ++++++++++++++++------ drivers/gpu/drm/radeon/radeon_fb.c | 21 +++++++++++++++------ drivers/gpu/drm/udl/udl_fb.c | 22 +++++++++++++++------- include/drm/drm_fb_helper.h | 2 +- 13 files changed, 140 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c index 5c60ae524c45..ff68eefae273 100644 --- a/drivers/gpu/drm/ast/ast_fb.c +++ b/drivers/gpu/drm/ast/ast_fb.c @@ -335,18 +335,27 @@ int ast_fbdev_init(struct drm_device *dev) ret = drm_fb_helper_init(dev, &afbdev->helper, 1, 1); - if (ret) { - kfree(afbdev); - return ret; - } + if (ret) + goto free; - drm_fb_helper_single_add_all_connectors(&afbdev->helper); + ret = drm_fb_helper_single_add_all_connectors(&afbdev->helper); + if (ret) + goto fini; /* disable all the possible outputs/crtcs before entering KMS mode */ drm_helper_disable_unused_functions(dev); - drm_fb_helper_initial_config(&afbdev->helper, 32); + ret = drm_fb_helper_initial_config(&afbdev->helper, 32); + if (ret) + goto fini; + return 0; + +fini: + drm_fb_helper_fini(&afbdev->helper); +free: + kfree(afbdev); + return ret; } void ast_fbdev_fini(struct drm_device *dev) diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c b/drivers/gpu/drm/bochs/bochs_fbdev.c index 61dbf09dff5d..976d9798dc99 100644 --- a/drivers/gpu/drm/bochs/bochs_fbdev.c +++ b/drivers/gpu/drm/bochs/bochs_fbdev.c @@ -207,12 +207,22 @@ int bochs_fbdev_init(struct bochs_device *bochs) if (ret) return ret; - drm_fb_helper_single_add_all_connectors(&bochs->fb.helper); + ret = drm_fb_helper_single_add_all_connectors(&bochs->fb.helper); + if (ret) + goto fini; + drm_helper_disable_unused_functions(bochs->dev); - drm_fb_helper_initial_config(&bochs->fb.helper, 32); + + ret = drm_fb_helper_initial_config(&bochs->fb.helper, 32); + if (ret) + goto fini; bochs->fb.initialized = true; return 0; + +fini: + drm_fb_helper_fini(&bochs->fb.helper); + return ret; } void bochs_fbdev_fini(struct bochs_device *bochs) diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c index 502a89eb54b5..13ddf1c4bb8e 100644 --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c @@ -317,17 +317,17 @@ int cirrus_fbdev_init(struct cirrus_device *cdev) ret = drm_fb_helper_init(cdev->dev, &gfbdev->helper, cdev->num_crtc, CIRRUSFB_CONN_LIMIT); - if (ret) { - kfree(gfbdev); + if (ret) + return ret; + + ret = drm_fb_helper_single_add_all_connectors(&gfbdev->helper); + if (ret) return ret; - } - drm_fb_helper_single_add_all_connectors(&gfbdev->helper); /* disable all the possible outputs/crtcs before entering KMS mode */ drm_helper_disable_unused_functions(cdev->dev); - drm_fb_helper_initial_config(&gfbdev->helper, bpp_sel); - return 0; + return drm_fb_helper_initial_config(&gfbdev->helper, bpp_sel); } void cirrus_fbdev_fini(struct cirrus_device *cdev) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 52ce26d6b4fb..876f1ef0acd1 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1688,7 +1688,7 @@ out: * RETURNS: * Zero if everything went ok, nonzero otherwise. */ -bool drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel) +int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel) { struct drm_device *dev = fb_helper->dev; int count = 0; diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c index ddd90ddbc200..2d42ce6d3757 100644 --- a/drivers/gpu/drm/gma500/framebuffer.c +++ b/drivers/gpu/drm/gma500/framebuffer.c @@ -593,6 +593,7 @@ int psb_fbdev_init(struct drm_device *dev) { struct psb_fbdev *fbdev; struct drm_psb_private *dev_priv = dev->dev_private; + int ret; fbdev = kzalloc(sizeof(struct psb_fbdev), GFP_KERNEL); if (!fbdev) { @@ -604,16 +605,29 @@ int psb_fbdev_init(struct drm_device *dev) drm_fb_helper_prepare(dev, &fbdev->psb_fb_helper, &psb_fb_helper_funcs); - drm_fb_helper_init(dev, &fbdev->psb_fb_helper, dev_priv->ops->crtcs, - INTELFB_CONN_LIMIT); + ret = drm_fb_helper_init(dev, &fbdev->psb_fb_helper, + dev_priv->ops->crtcs, INTELFB_CONN_LIMIT); + if (ret) + goto free; - drm_fb_helper_single_add_all_connectors(&fbdev->psb_fb_helper); + ret = drm_fb_helper_single_add_all_connectors(&fbdev->psb_fb_helper); + if (ret) + goto fini; /* disable all the possible outputs/crtcs before entering KMS mode */ drm_helper_disable_unused_functions(dev); - drm_fb_helper_initial_config(&fbdev->psb_fb_helper, 32); + ret = drm_fb_helper_initial_config(&fbdev->psb_fb_helper, 32); + if (ret) + goto fini; + return 0; + +fini: + drm_fb_helper_fini(&fbdev->psb_fb_helper); +free: + kfree(fbdev); + return ret; } static void psb_fbdev_fini(struct drm_device *dev) diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c index 4415af3666ab..c36b8304042b 100644 --- a/drivers/gpu/drm/mgag200/mgag200_fb.c +++ b/drivers/gpu/drm/mgag200/mgag200_fb.c @@ -303,14 +303,22 @@ int mgag200_fbdev_init(struct mga_device *mdev) if (ret) return ret; - drm_fb_helper_single_add_all_connectors(&mfbdev->helper); + ret = drm_fb_helper_single_add_all_connectors(&mfbdev->helper); + if (ret) + goto fini; /* disable all the possible outputs/crtcs before entering KMS mode */ drm_helper_disable_unused_functions(mdev->dev); - drm_fb_helper_initial_config(&mfbdev->helper, bpp_sel); + ret = drm_fb_helper_initial_config(&mfbdev->helper, bpp_sel); + if (ret) + goto fini; return 0; + +fini: + drm_fb_helper_fini(&mfbdev->helper); + return ret; } void mgag200_fbdev_fini(struct mga_device *mdev) diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c index 1f3af13ccede..115b509a4a00 100644 --- a/drivers/gpu/drm/msm/msm_fbdev.c +++ b/drivers/gpu/drm/msm/msm_fbdev.c @@ -241,17 +241,23 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev) goto fail; } - drm_fb_helper_single_add_all_connectors(helper); + ret = drm_fb_helper_single_add_all_connectors(helper); + if (ret) + goto fini; /* disable all the possible outputs/crtcs before entering KMS mode */ drm_helper_disable_unused_functions(dev); - drm_fb_helper_initial_config(helper, 32); + ret = drm_fb_helper_initial_config(helper, 32); + if (ret) + goto fini; priv->fbdev = helper; return helper; +fini: + drm_fb_helper_fini(helper); fail: kfree(fbdev); return NULL; diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index 3ed12a8cfc91..5a7705dcd67e 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -539,12 +539,12 @@ nouveau_fbcon_init(struct drm_device *dev) ret = drm_fb_helper_init(dev, &fbcon->helper, dev->mode_config.num_crtc, 4); - if (ret) { - kfree(fbcon); - return ret; - } + if (ret) + goto free; - drm_fb_helper_single_add_all_connectors(&fbcon->helper); + ret = drm_fb_helper_single_add_all_connectors(&fbcon->helper); + if (ret) + goto fini; if (drm->device.info.ram_size <= 32 * 1024 * 1024) preferred_bpp = 8; @@ -557,8 +557,17 @@ nouveau_fbcon_init(struct drm_device *dev) /* disable all the possible outputs/crtcs before entering KMS mode */ drm_helper_disable_unused_functions(dev); - drm_fb_helper_initial_config(&fbcon->helper, preferred_bpp); + ret = drm_fb_helper_initial_config(&fbcon->helper, preferred_bpp); + if (ret) + goto fini; + return 0; + +fini: + drm_fb_helper_fini(&fbcon->helper); +free: + kfree(fbcon); + return ret; } void diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c index 8436c6857cda..d292d24b3a6e 100644 --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c @@ -334,17 +334,23 @@ struct drm_fb_helper *omap_fbdev_init(struct drm_device *dev) goto fail; } - drm_fb_helper_single_add_all_connectors(helper); + ret = drm_fb_helper_single_add_all_connectors(helper); + if (ret) + goto fini; /* disable all the possible outputs/crtcs before entering KMS mode */ drm_helper_disable_unused_functions(dev); - drm_fb_helper_initial_config(helper, 32); + ret = drm_fb_helper_initial_config(helper, 32); + if (ret) + goto fini; priv->fbdev = helper; return helper; +fini: + drm_fb_helper_fini(helper); fail: kfree(fbdev); return NULL; diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c index 3d7c1d00a424..f778c0e8ae3c 100644 --- a/drivers/gpu/drm/qxl/qxl_fb.c +++ b/drivers/gpu/drm/qxl/qxl_fb.c @@ -686,14 +686,24 @@ int qxl_fbdev_init(struct qxl_device *qdev) ret = drm_fb_helper_init(qdev->ddev, &qfbdev->helper, qxl_num_crtc /* num_crtc - QXL supports just 1 */, QXLFB_CONN_LIMIT); - if (ret) { - kfree(qfbdev); - return ret; - } + if (ret) + goto free; + + ret = drm_fb_helper_single_add_all_connectors(&qfbdev->helper); + if (ret) + goto fini; + + ret = drm_fb_helper_initial_config(&qfbdev->helper, bpp_sel); + if (ret) + goto fini; - drm_fb_helper_single_add_all_connectors(&qfbdev->helper); - drm_fb_helper_initial_config(&qfbdev->helper, bpp_sel); return 0; + +fini: + drm_fb_helper_fini(&qfbdev->helper); +free: + kfree(qfbdev); + return ret; } void qxl_fbdev_fini(struct qxl_device *qdev) diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c index 29b9220ec399..3000bc4c136b 100644 --- a/drivers/gpu/drm/radeon/radeon_fb.c +++ b/drivers/gpu/drm/radeon/radeon_fb.c @@ -390,18 +390,27 @@ int radeon_fbdev_init(struct radeon_device *rdev) ret = drm_fb_helper_init(rdev->ddev, &rfbdev->helper, rdev->num_crtc, RADEONFB_CONN_LIMIT); - if (ret) { - kfree(rfbdev); - return ret; - } + if (ret) + goto free; - drm_fb_helper_single_add_all_connectors(&rfbdev->helper); + ret = drm_fb_helper_single_add_all_connectors(&rfbdev->helper); + if (ret) + goto fini; /* disable all the possible outputs/crtcs before entering KMS mode */ drm_helper_disable_unused_functions(rdev->ddev); - drm_fb_helper_initial_config(&rfbdev->helper, bpp_sel); + ret = drm_fb_helper_initial_config(&rfbdev->helper, bpp_sel); + if (ret) + goto fini; + return 0; + +fini: + drm_fb_helper_fini(&rfbdev->helper); +free: + kfree(rfbdev); + return ret; } void radeon_fbdev_fini(struct radeon_device *rdev) diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index 8cbcb4589bd3..5fc16cecd3ba 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -589,19 +589,27 @@ int udl_fbdev_init(struct drm_device *dev) ret = drm_fb_helper_init(dev, &ufbdev->helper, 1, 1); - if (ret) { - kfree(ufbdev); - return ret; + if (ret) + goto free; - } - - drm_fb_helper_single_add_all_connectors(&ufbdev->helper); + ret = drm_fb_helper_single_add_all_connectors(&ufbdev->helper); + if (ret) + goto fini; /* disable all the possible outputs/crtcs before entering KMS mode */ drm_helper_disable_unused_functions(dev); - drm_fb_helper_initial_config(&ufbdev->helper, bpp_sel); + ret = drm_fb_helper_initial_config(&ufbdev->helper, bpp_sel); + if (ret) + goto fini; + return 0; + +fini: + drm_fb_helper_fini(&ufbdev->helper); +free: + kfree(ufbdev); + return ret; } void udl_fbdev_cleanup(struct drm_device *dev) diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index b597068103aa..21b944c456f6 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -125,7 +125,7 @@ void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch, int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info); int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper); -bool drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel); +int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel); int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper); int drm_fb_helper_debug_enter(struct fb_info *info); int drm_fb_helper_debug_leave(struct fb_info *info); From cdd1cf799bd24ac0a4184549601ae302267301c5 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 4 Dec 2014 21:03:25 +0000 Subject: [PATCH 3/6] drm: Make drm_read() more robust against multithreaded races The current implementation of drm_read() faces a number of issues: 1. Upon an error, it consumes the event which may lead to the client blocking. 2. Upon an error, it forgets about events already copied 3. If it fails to copy a single event with O_NONBLOCK it falls into a infinite loop of reporting EAGAIN. 3. There is a race between multiple waiters and blocking reads of the events list. Here, we inline drm_dequeue_event() into drm_read() so that we can take the spinlock around the list walking and event copying, and importantly reorder the error handling to avoid the issues above. Cc: Takashi Iwai Signed-off-by: Chris Wilson Reviewed-by: Takashi Iwai Testcase: igt/drm_read Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_fops.c | 91 ++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 0b9514b6cd64..076dd606b580 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -478,64 +478,59 @@ int drm_release(struct inode *inode, struct file *filp) } EXPORT_SYMBOL(drm_release); -static bool -drm_dequeue_event(struct drm_file *file_priv, - size_t total, size_t max, struct drm_pending_event **out) -{ - struct drm_device *dev = file_priv->minor->dev; - struct drm_pending_event *e; - unsigned long flags; - bool ret = false; - - spin_lock_irqsave(&dev->event_lock, flags); - - *out = NULL; - if (list_empty(&file_priv->event_list)) - goto out; - e = list_first_entry(&file_priv->event_list, - struct drm_pending_event, link); - if (e->event->length + total > max) - goto out; - - file_priv->event_space += e->event->length; - list_del(&e->link); - *out = e; - ret = true; - -out: - spin_unlock_irqrestore(&dev->event_lock, flags); - return ret; -} - ssize_t drm_read(struct file *filp, char __user *buffer, size_t count, loff_t *offset) { struct drm_file *file_priv = filp->private_data; - struct drm_pending_event *e; - size_t total; - ssize_t ret; + struct drm_device *dev = file_priv->minor->dev; + ssize_t ret = 0; - if ((filp->f_flags & O_NONBLOCK) == 0) { - ret = wait_event_interruptible(file_priv->event_wait, - !list_empty(&file_priv->event_list)); - if (ret < 0) - return ret; - } + if (!access_ok(VERIFY_WRITE, buffer, count)) + return -EFAULT; - total = 0; - while (drm_dequeue_event(file_priv, total, count, &e)) { - if (copy_to_user(buffer + total, - e->event, e->event->length)) { - total = -EFAULT; + spin_lock_irq(&dev->event_lock); + for (;;) { + if (list_empty(&file_priv->event_list)) { + if (ret) + break; + + if (filp->f_flags & O_NONBLOCK) { + ret = -EAGAIN; + break; + } + + spin_unlock_irq(&dev->event_lock); + ret = wait_event_interruptible(file_priv->event_wait, + !list_empty(&file_priv->event_list)); + spin_lock_irq(&dev->event_lock); + if (ret < 0) + break; + + ret = 0; + } else { + struct drm_pending_event *e; + + e = list_first_entry(&file_priv->event_list, + struct drm_pending_event, link); + if (e->event->length + ret > count) + break; + + if (__copy_to_user_inatomic(buffer + ret, + e->event, e->event->length)) { + if (ret == 0) + ret = -EFAULT; + break; + } + + file_priv->event_space += e->event->length; + ret += e->event->length; + list_del(&e->link); e->destroy(e); - break; } - - total += e->event->length; - e->destroy(e); } + spin_unlock_irq(&dev->event_lock); - return total ?: -EAGAIN; + return ret; } EXPORT_SYMBOL(drm_read); From 42c5814c9c9828a7fcbe25f69c5c8cbbf29ed957 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Mon, 12 Jan 2015 21:12:17 -0800 Subject: [PATCH 4/6] next: drm/atomic: Use copy_from_user to copy 64 bit data from user space Copying 64 bit data from user space using get_user is not supported on all architectures, and may result in the following build error. ERROR: "__get_user_bad" [drivers/gpu/drm/drm.ko] undefined! Avoid the problem by using copy_from_user. Fixes: d34f20d6e2f2 ("drm: Atomic modeset ioctl") Cc: Rob Clark Signed-off-by: Guenter Roeck Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 1e38dfc8e462..af3f3dfdb49f 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1259,7 +1259,9 @@ retry: goto fail; } - if (get_user(prop_value, prop_values_ptr + copied_props)) { + if (copy_from_user(&prop_value, + prop_values_ptr + copied_props, + sizeof(prop_value))) { ret = -EFAULT; goto fail; } From 162b6a57ac50eec236530a16c071ffa50e87362a Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 21 Jan 2015 08:45:21 +0100 Subject: [PATCH 5/6] drm/probe-helper: don't lose hotplug event There's a race window (small for hpd, 10s large for polled outputs) where userspace could sneak in with an unrelated connnector probe ioctl call and eat the hotplug event (since neither the hpd nor the poll code see a state change). To avoid this, check whether the connector state changes in all other ->detect calls (in the current helper code that's only probe_single) and if that's the case, fire off a hotplug event. Note that we can't directly call the hotplug event handler, since that expects that no locks are held (due to reentrancy with the fb code to update the kms console). Also, this requires that drivers using the probe_single helper function set up the poll work. All current drivers do that already, and with the reworked hpd handling there'll be no downside to unconditionally setting up the poll work any more. v2: Review from Rob Clark - Don't bail out of the output poll work immediately if it's disabled to make sure we deliver the delayed hoptplug events. Instead just jump to the tail. - Don't scheduel the work when it's not set up. Would be a driver bug since using probe helpers for anything dynamic without them initialized makes them all noops. Reviewed-by: Chris Wilson (v1) Cc: Rob Clark Reviewed-by: Rob Clark Tested-by: Rob Clark Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_probe_helper.c | 36 ++++++++++++++++++++++++++++-- include/drm/drm_crtc.h | 1 + 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 2fbdcca7ca9a..33bf550a1d3f 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -103,6 +103,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect int count = 0; int mode_flags = 0; bool verbose_prune = true; + enum drm_connector_status old_status; WARN_ON(!mutex_is_locked(&dev->mode_config.mutex)); @@ -121,7 +122,33 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect if (connector->funcs->force) connector->funcs->force(connector); } else { + old_status = connector->status; + connector->status = connector->funcs->detect(connector, true); + + /* + * Normally either the driver's hpd code or the poll loop should + * pick up any changes and fire the hotplug event. But if + * userspace sneaks in a probe, we might miss a change. Hence + * check here, and if anything changed start the hotplug code. + */ + if (old_status != connector->status) { + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n", + connector->base.id, + connector->name, + old_status, connector->status); + + /* + * The hotplug event code might call into the fb + * helpers, and so expects that we do not hold any + * locks. Fire up the poll struct instead, it will + * disable itself again. + */ + dev->mode_config.delayed_event = true; + if (dev->mode_config.poll_enabled) + schedule_delayed_work(&dev->mode_config.output_poll_work, + 0); + } } /* Re-enable polling in case the global poll config changed. */ @@ -274,10 +301,14 @@ static void output_poll_execute(struct work_struct *work) struct drm_device *dev = container_of(delayed_work, struct drm_device, mode_config.output_poll_work); struct drm_connector *connector; enum drm_connector_status old_status; - bool repoll = false, changed = false; + bool repoll = false, changed; + + /* Pick up any changes detected by the probe functions. */ + changed = dev->mode_config.delayed_event; + dev->mode_config.delayed_event = false; if (!drm_kms_helper_poll) - return; + goto out; mutex_lock(&dev->mode_config.mutex); list_for_each_entry(connector, &dev->mode_config.connector_list, head) { @@ -319,6 +350,7 @@ static void output_poll_execute(struct work_struct *work) mutex_unlock(&dev->mode_config.mutex); +out: if (changed) drm_kms_helper_hotplug_event(dev); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index f444263055c5..65da9fb939a7 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1089,6 +1089,7 @@ struct drm_mode_config { /* output poll support */ bool poll_enabled; bool poll_running; + bool delayed_event; struct delayed_work output_poll_work; /* pointers to standard properties */ From b7703726251191cd9f3ef3a80b2d9667901eec95 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 21 Jan 2015 08:45:22 +0100 Subject: [PATCH 6/6] drm/probe-helper: clamp unknown connector status in the poll work On some chipset we try to avoid possibly invasive output detection methods (like load detect which can cause flickering elsewhere) in the output poll work. Drivers could hence return unknown when a previous full ->detect call returned a different state. This change will generate a hotplug event, forcing userspace to do a full scan. This in turn updates the connector->status field so that we will _again_ get a state change when the hotplug work re-runs in 10 seconds. To avoid this ping-pong loop detect this situation and clamp the connector state to the old value. Patch is inspired by a patch from Knut Peterson. Knut's patch completely ignored connector state changes if either the old or new status was unknown, which seemed to be a bit too agressive to me. v2: Rebased onto the drm_probe_helper.c extraction. References: http://lists.freedesktop.org/archives/dri-devel/2012-August/025975.html Cc: Knut Petersen Cc: Alex Deucher Reviewed-by: Chris Wilson Acked-by: Alex Deucher Cc: Rob Clark Reviewed-by: Rob Clark Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_probe_helper.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 33bf550a1d3f..6591d48c1b9d 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -335,6 +335,24 @@ static void output_poll_execute(struct work_struct *work) if (old_status != connector->status) { const char *old, *new; + /* + * The poll work sets force=false when calling detect so + * that drivers can avoid to do disruptive tests (e.g. + * when load detect cycles could cause flickering on + * other, running displays). This bears the risk that we + * flip-flop between unknown here in the poll work and + * the real state when userspace forces a full detect + * call after receiving a hotplug event due to this + * change. + * + * Hence clamp an unknown detect status to the old + * value. + */ + if (connector->status == connector_status_unknown) { + connector->status = old_status; + continue; + } + old = drm_get_connector_status_name(old_status); new = drm_get_connector_status_name(connector->status);