From patchwork Thu Mar 20 13:01:22 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 3865721 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 2DE05BF540 for ; Thu, 20 Mar 2014 13:01:38 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 061D220221 for ; Thu, 20 Mar 2014 13:01:37 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 172652022A for ; Thu, 20 Mar 2014 13:01:36 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AD24F6E060; Thu, 20 Mar 2014 06:01:35 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ee0-f49.google.com (mail-ee0-f49.google.com [74.125.83.49]) by gabe.freedesktop.org (Postfix) with ESMTP id AA4436E061 for ; Thu, 20 Mar 2014 06:01:33 -0700 (PDT) Received: by mail-ee0-f49.google.com with SMTP id c41so625521eek.22 for ; Thu, 20 Mar 2014 06:01:32 -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=/7DRwVhrKA6APPBjNbVEnjxnKRPc6UNawXfS+6TeUtg=; b=MFkCvDrYxhPqdiunGwEivUr1ey5tpmhovO/dd1I4BTRnrgPLyvbLfF95bvd+W1A9vg m3TpBt9+qXBkLoCPsgR0zO7HVHVp5Rv+A0Sm2BFwMnkyyl7Q6zLwTnThsRL9RjxN3hqw tDoJit6Yz5NiGXRYppp0gg0XhVvFSgDZ4+QMs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=/7DRwVhrKA6APPBjNbVEnjxnKRPc6UNawXfS+6TeUtg=; b=ZZVYBd8NChqMg1dDweYW7Q3wyOhdLjnsnWQDc6zg/awWvZIw3wpplMqglDpxhJG6nn O13bvplaCEVrYAFADJ2tdbLbHJiEtU7tgbjRCIFiJUz2ioWYSDGT4xRGdsQ+BL0rBXRo HKNkmcFSyjpen1XpyzWn2EGPiGOXrcu8DQaFO9QWFatMxBZyNSXca9Pn7sKLP45WrDJw iMGW8+Ypwc/7Vde7NSM8JEuscqUX8MCXKV2J4GZ6EUnVLj/bvKjlcU6ZApPo5xDcVFm7 t5kgMDoUvuQ/bxxskO3OHUyQUvjqmGTUD3RMgKbpIBMZnqxL6YeforybcXCkdsd9iAKc JhQA== X-Gm-Message-State: ALoCoQn2+nQt8ANpKeoR2HFbCOZdqii5hGGHMBB9sPlE7Bbk5zmIKlMHO+Le2UETJRb9Zde8RqBj X-Received: by 10.14.7.137 with SMTP id 9mr421486eep.114.1395320492798; Thu, 20 Mar 2014 06:01:32 -0700 (PDT) Received: from aaron.ffwll.local (84-73-67-144.dclient.hispeed.ch. [84.73.67.144]) by mx.google.com with ESMTPSA id cb5sm4068715eeb.18.2014.03.20.06.01.31 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 20 Mar 2014 06:01:31 -0700 (PDT) From: Daniel Vetter To: DRI Development Date: Thu, 20 Mar 2014 14:01:22 +0100 Message-Id: <1395320482-3962-2-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.8.4.rc3 In-Reply-To: <1395320482-3962-1-git-send-email-daniel.vetter@ffwll.ch> References: <1395320482-3962-1-git-send-email-daniel.vetter@ffwll.ch> Cc: Daniel Vetter , Intel Graphics Development Subject: [Intel-gfx] [PATCH 2/2] drm/fb-helper: improve drm_fb_helper_initial_config locking X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.15 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-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The locking in drm_fb_helper_initial_config is a bit troublesome for a few reasons: - We can't just wrap the entire function up into modeset locks since the fbdev registration might call down into fbcon code, which then through our ->set_par implementation needs to be able to grab all modeset locks. So we'd have a neat deadlock. - This implies though that all current callers don't hold any modeset locks by necessity, so we have free reign to grab any modeset locks we need to grab. - The private state of the fbdev helper doesn't need any protection through locks, since once we have the fbdev registered it is mostly invariant or protected through the modeset locking in ->set_par and other callbacks. We can fully rely on driver having non-racy setup sequences here. For the initial config computation we actually may not grab locks since drivers which provide their own magic sauce (like i915) might need to grab locks themselves. - We should grab locks though when we probe outputs. Currently there's not much risk, but already now userspace could start poking at sysfs files and so probe concurrently. I expect that in the future driver init will be much more async, and since probing is really time-consuming this is a prime candidate. - We must not hold any crtc->mutex locks while calling probe functions since those might need to lock a crtc for e.g. load detection. i915 is such a driver. Also it's the probing calls which hit upon piles of new locking asserts I've recently added in commit 62ff94a5492175759546f8bc61383189d6b49122 Author: Daniel Vetter Date: Thu Jan 23 22:18:47 2014 +0100 drm/crtc-helper: remove LOCKING from kerneldoc and commit 63951385052f7974155fa38f962f0f4e9847f90a Author: Daniel Vetter Date: Thu Jan 23 15:14:15 2014 +0100 drm/doc: Repleace LOCKING kerneldoc sections in drm_modes.c Hence the right fix is to grab the mode_config mutex, but only that and only right around the probe calls. It seems to be sufficient to shut up all the locking WARNINGs I see on i915 and nouveau in drm_fb_helper_initial_config. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_fb_helper.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 87876198801d..16f271e21b9c 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1536,9 +1536,11 @@ bool drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel) drm_fb_helper_parse_command_line(fb_helper); + mutex_lock(&dev->mode_config.mutex); count = drm_fb_helper_probe_connector_modes(fb_helper, dev->mode_config.max_width, dev->mode_config.max_height); + mutex_unlock(&dev->mode_config.mutex); /* * we shouldn't end up with no modes here. */