From patchwork Mon Apr 3 08:32:54 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 9658963 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 109CA60352 for ; Mon, 3 Apr 2017 08:33:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0319A28450 for ; Mon, 3 Apr 2017 08:33:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id EC2B52845B; Mon, 3 Apr 2017 08:33:49 +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 9972F28450 for ; Mon, 3 Apr 2017 08:33:49 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5AFA56E2F6; Mon, 3 Apr 2017 08:33:20 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wr0-x242.google.com (mail-wr0-x242.google.com [IPv6:2a00:1450:400c:c0c::242]) by gabe.freedesktop.org (Postfix) with ESMTPS id 892556E2E8 for ; Mon, 3 Apr 2017 08:33:18 +0000 (UTC) Received: by mail-wr0-x242.google.com with SMTP id w43so31336032wrb.1 for ; Mon, 03 Apr 2017 01:33:18 -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=fMLRlihgcXJj3X62YoAc/bLl1hKqbuppL9brpxPJ7y4=; b=Cxma7gOd8E+5sw4hytVk6VsAU2L/JNOPgBAtQ5WU8Q6XJLFEHoSq2MID6QtfE4Nm+8 6CR6Sx12cYXfab/wcOr2umyT0Hp1sQEDIi4uDLRL8+Y8Oo7klUuPE/SCFW7Y5JNnIw23 FubN9n7cMZtCUUG6DTu4HVwcT2PlWp26Bb5Nw= 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=fMLRlihgcXJj3X62YoAc/bLl1hKqbuppL9brpxPJ7y4=; b=HcwCkwVYFQ/+xQ0W2egQ+TSTpLG8/k6JNA1zko3VqWaUusui6SfNjEaVOxfia0NEmU /Pmn+oXiLghN73FgRA9O1+uFx9ySWaXpEINQUkNuA1leZmgLZiZ/L4KetPJ9+4OJeMXq SdZzLLw1tiP7r6zZmYlIwtQn71Hdh5ndHkmuRAm5nbdTO351zenBXjBCSl86UaaD6985 +WxyNl8UjRhSLC0GNPx8t4cU7te6SntVfDAqD/9vJHb6TZuJ5cXr6oiD4nwJa1kAR7u6 ccz8oJYa7V7ShuhxY2IbUD7ll3LuqdTwTje7LDqbAHBPsbPWA1640Uxk1Oqu9tWdnYkM u7Eg== X-Gm-Message-State: AFeK/H03IEIP5dLrCg7FSa0oEkitkelEEQoxI6caatGx6EH8vw3VkpLVdvtHcMqWAbl7aA== X-Received: by 10.28.13.65 with SMTP id 62mr8267618wmn.1.1491208397154; Mon, 03 Apr 2017 01:33:17 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:56b5:0:3dcb:681a:5b6e:8025]) by smtp.gmail.com with ESMTPSA id w10sm13326590wmw.14.2017.04.03.01.33.16 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 03 Apr 2017 01:33:16 -0700 (PDT) From: Daniel Vetter To: DRI Development Date: Mon, 3 Apr 2017 10:32:54 +0200 Message-Id: <20170403083304.9083-6-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170403083304.9083-1-daniel.vetter@ffwll.ch> References: <20170403083304.9083-1-daniel.vetter@ffwll.ch> Cc: Daniel Vetter , Intel Graphics Development , Daniel Vetter Subject: [Intel-gfx] [PATCH 05/15] drm: drop modeset_lock_all from drm_state_info 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 If we push the locks down we don't have to take them all at the same time. Aside: Making dump_info fully safe should be fairly simple, if we protect the ->state pointers with rcu. Simply putting a synchronize_rcu() into the drm_atomic_state free function should be all that's roughly needed. Well except we shouldn't block in there, so better to put that into a work_struct. But I've not set out to fix that little issue. Cc: Rob Clark Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 60 ++++++++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 345310213820..9afb14371ce0 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1676,22 +1676,8 @@ static void drm_atomic_print_state(const struct drm_atomic_state *state) drm_atomic_connector_print_state(&p, connector_state); } -/** - * drm_state_dump - dump entire device atomic state - * @dev: the drm device - * @p: where to print the state to - * - * Just for debugging. Drivers might want an option to dump state - * to dmesg in case of error irq's. (Hint, you probably want to - * ratelimit this!) - * - * The caller must drm_modeset_lock_all(), or if this is called - * from error irq handler, it should not be enabled by default. - * (Ie. if you are debugging errors you might not care that this - * is racey. But calling this without all modeset locks held is - * not inherently safe.) - */ -void drm_state_dump(struct drm_device *dev, struct drm_printer *p) +static void __drm_state_dump(struct drm_device *dev, struct drm_printer *p, + bool take_locks) { struct drm_mode_config *config = &dev->mode_config; struct drm_plane *plane; @@ -1702,17 +1688,51 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p) if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) return; - list_for_each_entry(plane, &config->plane_list, head) + list_for_each_entry(plane, &config->plane_list, head) { + if (take_locks) + drm_modeset_lock(&plane->mutex, NULL); drm_atomic_plane_print_state(p, plane->state); + if (take_locks) + drm_modeset_unlock(&plane->mutex); + } - list_for_each_entry(crtc, &config->crtc_list, head) + list_for_each_entry(crtc, &config->crtc_list, head) { + if (take_locks) + drm_modeset_lock(&crtc->mutex, NULL); drm_atomic_crtc_print_state(p, crtc->state); + if (take_locks) + drm_modeset_unlock(&crtc->mutex); + } drm_connector_list_iter_begin(dev, &conn_iter); + if (take_locks) + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); drm_for_each_connector_iter(connector, &conn_iter) drm_atomic_connector_print_state(p, connector->state); + if (take_locks) + drm_modeset_unlock(&dev->mode_config.connection_mutex); drm_connector_list_iter_end(&conn_iter); } + +/** + * drm_state_dump - dump entire device atomic state + * @dev: the drm device + * @p: where to print the state to + * + * Just for debugging. Drivers might want an option to dump state + * to dmesg in case of error irq's. (Hint, you probably want to + * ratelimit this!) + * + * The caller must drm_modeset_lock_all(), or if this is called + * from error irq handler, it should not be enabled by default. + * (Ie. if you are debugging errors you might not care that this + * is racey. But calling this without all modeset locks held is + * not inherently safe.) + */ +void drm_state_dump(struct drm_device *dev, struct drm_printer *p) +{ + __drm_state_dump(dev, p, false); +} EXPORT_SYMBOL(drm_state_dump); #ifdef CONFIG_DEBUG_FS @@ -1722,9 +1742,7 @@ static int drm_state_info(struct seq_file *m, void *data) struct drm_device *dev = node->minor->dev; struct drm_printer p = drm_seq_file_printer(m); - drm_modeset_lock_all(dev); - drm_state_dump(dev, &p); - drm_modeset_unlock_all(dev); + __drm_state_dump(dev, &p, true); return 0; }