From patchwork Wed Jul 29 10:51:41 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 6892021 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id D8B899F358 for ; Wed, 29 Jul 2015 10:51:58 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 08DA42077C for ; Wed, 29 Jul 2015 10:51:58 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 5F16520781 for ; Wed, 29 Jul 2015 10:51:55 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 101C96EBB9; Wed, 29 Jul 2015 03:51:53 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wi0-f172.google.com (mail-wi0-f172.google.com [209.85.212.172]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3E4266EBB9 for ; Wed, 29 Jul 2015 03:51:51 -0700 (PDT) Received: by wibxm9 with SMTP id xm9so195275329wib.0 for ; Wed, 29 Jul 2015 03:51:50 -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; bh=GtapNkE3otfSwFxx/+FhnK54OR9IZajXoXoeTrFpzX0=; b=JgoAvTLp6NBnnhv6Kh61nzgPREfQTRzNaFDixOqvGyQNc5e4CRE6tPNL7vKUjRKtcP 4NgGOr2ISk6aIg94n5FggkwjL4pxIsFuEEmvKi0ou/666ruOio1dOK6Pl0UMUXXhYPWe oGl/63yJqnvkwweYi5oqgpmMrfYprg6s0Rho8= 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; bh=GtapNkE3otfSwFxx/+FhnK54OR9IZajXoXoeTrFpzX0=; b=N+7ckSmZbGZV3sO3yuvVwOSu9MqBzB4Fa8fGr+3/a+1G9yeaZgXAMBc673cRcd2y4V I6g9hmbZsk4vQwq89/u3eqij7rIk+Gd31r3xnbfLooRITTefhTG39BhtczK0tVdQ6oXP oqDzuOI32kzC9HtV/OW0RBa4UbVog6WVJ/Vi/GeqOYf+3tkX8BteJKfxNiFVgHhQbfSf 2arjee0pyKRp6CddlA74W9oFfJuPNO0V7GqVI8CMH/4HpyNeIY7MmmAnE9jbaxh5ZByC 57mIRsFYFRPhs0qFu7E354umsCFbkHDQAU0YPzUF4M0OHL5f48nItWlrvZCbBy7os565 F3Mw== X-Gm-Message-State: ALoCoQnosmjpWdIRePJnxVlMz2DfR3Q95MLUqACrYsVKGRzn3GQ89i3pGNxfyhIvQUY6p5BNRt4y X-Received: by 10.180.7.130 with SMTP id j2mr16760845wia.29.1438167109919; Wed, 29 Jul 2015 03:51:49 -0700 (PDT) Received: from phenom.ffwll.local (212-51-149-109.fiber7.init7.net. [212.51.149.109]) by smtp.gmail.com with ESMTPSA id ec19sm23620050wic.0.2015.07.29.03.51.48 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 29 Jul 2015 03:51:48 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development , DRI Development Subject: [PATCH] drm/atomic: Paper over locking WARN in default_state_clear Date: Wed, 29 Jul 2015 12:51:41 +0200 Message-Id: <1438167101-20704-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.5.0 Cc: Daniel Vetter , Daniel Vetter X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,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 In commit 6f75cea66c8dd043ced282016b21a639af176642 Author: Daniel Vetter Date: Wed Nov 19 18:38:07 2014 +0100 drm/atomic: Only destroy connector states with connection mutex held I tried to fix races of atomic commits against connector hot-unplugging. The idea is to ensure lifetimes by holding the connection_mutex long enough. That works for synchronous commits, but not for async ones. For async atomic commit we really need to fix up connector lifetimes for real. But that's a much bigger task, so just add more duct-tape: For cleaning up connector states we currently don't need the connector itself. So NULL it out and remove the locking check. Of course that check was to protect the entire sequence, but the modeset itself should be save since currently DP MST hot-removal does a dpms-off. And that should synchronize with any outstanding async atomic commit. Or at least that's my hope, this is all a giant mess. Reported-by: Maarten Lankhorst Cc: Maarten Lankhorst Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 3efd91c0c6cb..434915448ea0 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -153,9 +153,15 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) if (!connector) continue; - WARN_ON(!drm_modeset_is_locked(&config->connection_mutex)); - - connector->funcs->atomic_destroy_state(connector, + /* + * FIXME: Async commits can race with connector unplugging and + * there's currently nothing that prevents cleanup up state for + * deleted connectors. As long as the callback doesn't look at + * the connector we'll be fine though, so make sure that's the + * case by setting all connector pointers to NULL. + */ + state->connector_states[i]->connector = NULL; + connector->funcs->atomic_destroy_state(NULL, state->connector_states[i]); state->connectors[i] = NULL; state->connector_states[i] = NULL;