From patchwork Thu May 12 14:57:00 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: cpaul@redhat.com X-Patchwork-Id: 9082531 Return-Path: X-Original-To: patchwork-intel-gfx@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 DA5489F372 for ; Thu, 12 May 2016 14:57:14 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 01D2820251 for ; Thu, 12 May 2016 14:57:14 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 1FD9A20221 for ; Thu, 12 May 2016 14:57:13 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 64BA06E8D9; Thu, 12 May 2016 14:57:12 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by gabe.freedesktop.org (Postfix) with ESMTPS id 94DFC6E8D7 for ; Thu, 12 May 2016 14:57:07 +0000 (UTC) Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4800885538; Thu, 12 May 2016 14:57:07 +0000 (UTC) Received: from ecstaticemu.bos.redhat.com (dhcp-25-142.bos.redhat.com [10.18.25.142]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4CEv4E1030233; Thu, 12 May 2016 10:57:06 -0400 From: Lyude To: Daniel Vetter , intel-gfx@lists.freedesktop.org Date: Thu, 12 May 2016 10:57:00 -0400 Message-Id: <1463065021-18280-4-git-send-email-cpaul@redhat.com> In-Reply-To: <1463065021-18280-1-git-send-email-cpaul@redhat.com> References: <1463065021-18280-1-git-send-email-cpaul@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 12 May 2016 14:57:07 +0000 (UTC) Subject: [Intel-gfx] [PATCH 3/4] drm/i915: Discard previous atomic state on resume if connectors change 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-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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 If an MST device is disconnected while the machine is suspended, the number of connectors will change as well after we call intel_dp_mst_resume(). This means that any previous atomic state we had before suspending is no longer valid, since it'll still be pointing to missing connectors. We need to check for this before committing the state, otherwise we'll kernel panic on resume whenever if any MST display was disconnected before we started resuming: BUG: unable to handle kernel NULL pointer dereference at 0000000000000008 IP: [] drm_atomic_helper_check_modeset+0x29f/0xb40 [drm_kms_helper] Call Trace: [] intel_atomic_check+0x34/0x1180 [i915] [] ? mark_held_locks+0x6f/0xa0 [] ? trace_hardirqs_on_caller+0x129/0x1b0 [] drm_atomic_check_only+0x192/0x620 [drm] [] ? pci_pm_thaw+0x21/0x90 [] drm_atomic_commit+0x17/0x60 [drm] [] intel_display_resume+0xbd/0x160 [i915] [] ? pci_pm_thaw+0x90/0x90 [] i915_drm_resume+0xd8/0x160 [i915] [] i915_pm_resume+0x25/0x30 [i915] [] pci_pm_resume+0x64/0xa0 [] dpm_run_callback+0x90/0x190 [] device_resume+0xd5/0x1f0 [] async_resume+0x1d/0x50 [] async_run_entry_fn+0x48/0x150 [] process_one_work+0x1e9/0x5c0 [] ? process_one_work+0x166/0x5c0 [] worker_thread+0x48/0x4e0 [] ? process_one_work+0x5c0/0x5c0 [] kthread+0xe4/0x100 [] ret_from_fork+0x22/0x50 [] ? kthread_create_on_node+0x200/0x200 Changes since v1: - Move drm_atomic_state_free() call down so we're holding the appropriate locks when destroying the atomic state Changes since v2: - Check that state != NULL before we start accessing it's members This fix is only required for 4.6 and below. David Airlie's patchseries for 4.7 to add connector reference counting provides a more proper fix for this. Upstream fix: b164d31f50b2923a7a92c2a40cb46973a6ba8c36 Cc: stable@vger.kernel.org Signed-off-by: Lyude --- drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 182f849..e49a313 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15959,6 +15959,18 @@ void intel_display_resume(struct drm_device *dev) retry: ret = drm_modeset_lock_all_ctx(dev, &ctx); + /* + * With MST, the number of connectors can change between suspend and + * resume, which means that the state we want to restore might now be + * impossible to use since it'll be pointing to non-existant + * connectors. + */ + if (ret == 0 && state && + state->num_connector != dev->mode_config.num_connector) { + drm_atomic_state_free(state); + state = NULL; + } + if (ret == 0 && !setup) { setup = true;