From patchwork Tue Jul 4 15:18:23 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 9825233 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id CE68560237 for ; Tue, 4 Jul 2017 15:18:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BE1F324B48 for ; Tue, 4 Jul 2017 15:18:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B0FB627F3E; Tue, 4 Jul 2017 15:18:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 182FB24B48 for ; Tue, 4 Jul 2017 15:18:50 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5EF566E35D; Tue, 4 Jul 2017 15:18:45 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wm0-x241.google.com (mail-wm0-x241.google.com [IPv6:2a00:1450:400c:c09::241]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2B82A6E348 for ; Tue, 4 Jul 2017 15:18:44 +0000 (UTC) Received: by mail-wm0-x241.google.com with SMTP id j85so26982340wmj.0 for ; Tue, 04 Jul 2017 08:18:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=J2GPfnBK8Pa6qJMnHTSre5viNVo1DZu1G3xzjfBH/lY=; b=ZpjGyRGmyWMbYAyF46dPbV7pxqt+24k10WsI3oTMXiQuSqL13WImPt8zrLYeIbRXAR GTNh/3dX488nuPGsXz+mVO5e0WUDm+a9baa4NPmeJjLrwamHpvRWDsVe9vPXoRjrLalQ CZmW60Y8H14gRr814aV2T8xDbT9etBncVrm/w= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=J2GPfnBK8Pa6qJMnHTSre5viNVo1DZu1G3xzjfBH/lY=; b=b3CBWlQSeX1ZY3QuVAiAgYE4Il8Nbg7xU8x1lIbRpcGU+Ur5/1mykF1xc1j3EoliVF CEeEsqUP0ISsFUqWbFy9ZuStrnP6fizFyBN1o2qvOKmWWaQoIxBClqVdsHnIRSWm5Gsq VL8gs2oe49v00yH0OezViLUKaH0dNz7+CMeG3ISn/QcVCS+e4XlQsWIQHQxIaTitEwXC e/V3BWg4cmJN7lglSHDLHgcvUIKZOM/4y/Ofhg4MuBfkPuaJf+BwuPiDTQtpn78Fdx16 9eekyDMEb990SGoO5mOk5MfTVtTSzM33nTPH+k3Zp78IclVmoddBS4lTQI6eEx6Co4DI mG4w== X-Gm-Message-State: AKS2vOyHDS3JWbRuxvMugZxx8X4p8AWUD5fXlmsy0jKZiGDGjeQn1Qht m592NJJcfEdcrx0MClQ= X-Received: by 10.80.179.12 with SMTP id q12mr19087360edd.151.1499181522768; Tue, 04 Jul 2017 08:18:42 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:5640:0:960b:2678:e223:c1c6]) by smtp.gmail.com with ESMTPSA id 2sm23676308edt.36.2017.07.04.08.18.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 04 Jul 2017 08:18:41 -0700 (PDT) From: Daniel Vetter To: DRI Development Date: Tue, 4 Jul 2017 17:18:23 +0200 Message-Id: <20170704151833.17304-4-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.13.2 In-Reply-To: <20170704151833.17304-1-daniel.vetter@ffwll.ch> References: <20170704151833.17304-1-daniel.vetter@ffwll.ch> Cc: Daniel Vetter , Intel Graphics Development , Thierry Reding Subject: [Intel-gfx] [PATCH 03/13] drm/fb-helper: Add top-level lock X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP From: Thierry Reding Introduce a new top-level lock for the FB helper code. This will allow better locking granularity and avoid the need to abuse modeset locking for this purpose instead. This patch just adds the new lock everywhere we currently grab mode_config->mutex (explicitly, or through drm_modeset_lock_all). Follow-up patches will push the kms locking down into only the places that need it. v2: - use lockdep_assert_held - use drm_fb_helper_for_each_connector where possible - use the new top-level lock consistently, i.e. in all the places we're currently acquiring mode_config.mutex. - small polish to the kerneldoc Tested-by: John Stultz Signed-off-by: Thierry Reding (v1) Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_fb_helper.c | 47 +++++++++++++++++++++++++++++++++++++---- include/drm/drm_fb_helper.h | 19 ++++++++++++++++- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 5e73cc9f51e3..a0d4603857d8 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -119,7 +119,8 @@ static int __drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, if (!drm_fbdev_emulation) return 0; - WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex)); + lockdep_assert_held(&fb_helper->lock); + lockdep_assert_held(&fb_helper->dev->mode_config.mutex); count = fb_helper->connector_count + 1; @@ -150,11 +151,13 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, { int err; + mutex_lock(&fb_helper->lock); mutex_lock(&fb_helper->dev->mode_config.mutex); err = __drm_fb_helper_add_one_connector(fb_helper, connector); mutex_unlock(&fb_helper->dev->mode_config.mutex); + mutex_unlock(&fb_helper->lock); return err; } @@ -184,6 +187,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper) if (!drm_fbdev_emulation) return 0; + mutex_lock(&fb_helper->lock); mutex_lock(&dev->mode_config.mutex); drm_connector_list_iter_begin(dev, &conn_iter); drm_for_each_connector_iter(connector, &conn_iter) { @@ -207,6 +211,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper) out: drm_connector_list_iter_end(&conn_iter); mutex_unlock(&dev->mode_config.mutex); + mutex_unlock(&fb_helper->lock); return ret; } @@ -221,9 +226,9 @@ static int __drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, if (!drm_fbdev_emulation) return 0; - WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex)); + lockdep_assert_held(&fb_helper->lock); - for (i = 0; i < fb_helper->connector_count; i++) { + drm_fb_helper_for_each_connector(fb_helper, i) { if (fb_helper->connector_info[i]->connector == connector) break; } @@ -247,11 +252,13 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, { int err; + mutex_lock(&fb_helper->lock); mutex_lock(&fb_helper->dev->mode_config.mutex); err = __drm_fb_helper_remove_one_connector(fb_helper, connector); mutex_unlock(&fb_helper->dev->mode_config.mutex); + mutex_unlock(&fb_helper->lock); return err; } @@ -517,16 +524,21 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper) if (!drm_fbdev_emulation) return -ENODEV; + mutex_lock(&fb_helper->lock); drm_modeset_lock_all(dev); + ret = restore_fbdev_mode(fb_helper); do_delayed = fb_helper->delayed_hotplug; if (do_delayed) fb_helper->delayed_hotplug = false; + drm_modeset_unlock_all(dev); + mutex_unlock(&fb_helper->lock); if (do_delayed) drm_fb_helper_hotplug_event(fb_helper); + return ret; } EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked); @@ -576,11 +588,13 @@ static bool drm_fb_helper_force_kernel_mode(void) if (dev->switch_power_state == DRM_SWITCH_POWER_OFF) continue; + mutex_lock(&helper->lock); drm_modeset_lock_all(dev); ret = restore_fbdev_mode(helper); if (ret) error = true; drm_modeset_unlock_all(dev); + mutex_unlock(&helper->lock); } return error; } @@ -620,9 +634,11 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode) /* * For each CRTC in this fb, turn the connectors on/off. */ + mutex_lock(&fb_helper->lock); drm_modeset_lock_all(dev); if (!drm_fb_helper_is_bound(fb_helper)) { drm_modeset_unlock_all(dev); + mutex_unlock(&fb_helper->lock); return; } @@ -641,6 +657,7 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode) } } drm_modeset_unlock_all(dev); + mutex_unlock(&fb_helper->lock); } /** @@ -762,6 +779,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, INIT_WORK(&helper->resume_work, drm_fb_helper_resume_worker); INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work); helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; + mutex_init(&helper->lock); helper->funcs = funcs; helper->dev = dev; } @@ -927,6 +945,7 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper) } mutex_unlock(&kernel_fb_helper_lock); + mutex_destroy(&fb_helper->lock); drm_fb_helper_crtc_free(fb_helper); } @@ -1257,9 +1276,11 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info) if (oops_in_progress) return -EBUSY; + mutex_lock(&fb_helper->lock); drm_modeset_lock_all(dev); if (!drm_fb_helper_is_bound(fb_helper)) { drm_modeset_unlock_all(dev); + mutex_unlock(&fb_helper->lock); return -EBUSY; } @@ -1292,6 +1313,7 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info) } out: drm_modeset_unlock_all(dev); + mutex_unlock(&fb_helper->lock); return rc; } EXPORT_SYMBOL(drm_fb_helper_setcmap); @@ -1314,6 +1336,7 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, struct drm_crtc *crtc; int ret = 0; + mutex_lock(&fb_helper->lock); mutex_lock(&dev->mode_config.mutex); if (!drm_fb_helper_is_bound(fb_helper)) { ret = -EBUSY; @@ -1360,6 +1383,7 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, unlock: mutex_unlock(&dev->mode_config.mutex); + mutex_unlock(&fb_helper->lock); return ret; } EXPORT_SYMBOL(drm_fb_helper_ioctl); @@ -1589,9 +1613,11 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var, if (oops_in_progress) return -EBUSY; + mutex_lock(&fb_helper->lock); drm_modeset_lock_all(dev); if (!drm_fb_helper_is_bound(fb_helper)) { drm_modeset_unlock_all(dev); + mutex_unlock(&fb_helper->lock); return -EBUSY; } @@ -1600,6 +1626,7 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var, else ret = pan_display_legacy(var, info); drm_modeset_unlock_all(dev); + mutex_unlock(&fb_helper->lock); return ret; } @@ -2266,6 +2293,7 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper, DRM_DEBUG_KMS("No connectors reported connected with modes\n"); /* prevent concurrent modification of connector_count by hotplug */ + lockdep_assert_held(&fb_helper->lock); lockdep_assert_held(&fb_helper->dev->mode_config.mutex); crtcs = kcalloc(fb_helper->connector_count, @@ -2392,12 +2420,14 @@ int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel) if (!drm_fbdev_emulation) return 0; + mutex_lock(&fb_helper->lock); mutex_lock(&dev->mode_config.mutex); drm_setup_crtcs(fb_helper, dev->mode_config.max_width, dev->mode_config.max_height); ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel); mutex_unlock(&dev->mode_config.mutex); + mutex_unlock(&fb_helper->lock); if (ret) return ret; @@ -2445,25 +2475,34 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config); int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) { struct drm_device *dev = fb_helper->dev; + int err = 0; if (!drm_fbdev_emulation) return 0; + mutex_lock(&fb_helper->lock); mutex_lock(&dev->mode_config.mutex); + if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) { fb_helper->delayed_hotplug = true; mutex_unlock(&dev->mode_config.mutex); - return 0; + goto unlock; } + DRM_DEBUG_KMS("\n"); drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height); mutex_unlock(&dev->mode_config.mutex); + mutex_unlock(&fb_helper->lock); drm_fb_helper_set_par(fb_helper->fbdev); return 0; + +unlock: + mutex_unlock(&fb_helper->lock); + return err; } EXPORT_SYMBOL(drm_fb_helper_hotplug_event); diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 119e5e4609c7..ea170b96e88d 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -169,7 +169,6 @@ struct drm_fb_helper_connector { * @crtc_info: per-CRTC helper state (mode, x/y offset, etc) * @connector_count: number of connected connectors * @connector_info_alloc_count: size of connector_info - * @connector_info: array of per-connector information * @funcs: driver callbacks for fb helper * @fbdev: emulated fbdev device info struct * @pseudo_palette: fake palette of 16 colors @@ -191,6 +190,12 @@ struct drm_fb_helper { struct drm_fb_helper_crtc *crtc_info; int connector_count; int connector_info_alloc_count; + /** + * @connector_info: + * + * Array of per-connector information. Do not iterate directly, but use + * drm_fb_helper_for_each_connector. + */ struct drm_fb_helper_connector **connector_info; const struct drm_fb_helper_funcs *funcs; struct fb_info *fbdev; @@ -201,6 +206,18 @@ struct drm_fb_helper { struct work_struct resume_work; /** + * @lock: + * + * Top-level FBDEV helper lock. This protects all internal data + * structures and lists, such as @connector_info and @crtc_info. + * + * FIXME: fbdev emulation locking is a mess and long term we want to + * protect all helper internal state with this lock as well as reduce + * core KMS locking as much as possible. + */ + struct mutex lock; + + /** * @kernel_fb_list: * * Entry on the global kernel_fb_helper_list, used for kgdb entry/exit.