From patchwork Wed May 18 15:37:36 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jon Hunter X-Patchwork-Id: 623628 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3r8yyD6Gh3z9sBM for ; Thu, 19 May 2016 01:37:56 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752921AbcERPhz (ORCPT ); Wed, 18 May 2016 11:37:55 -0400 Received: from hqemgate15.nvidia.com ([216.228.121.64]:14810 "EHLO hqemgate15.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751895AbcERPhy (ORCPT ); Wed, 18 May 2016 11:37:54 -0400 Received: from hqnvupgp08.nvidia.com (Not Verified[216.228.121.13]) by hqemgate15.nvidia.com id ; Wed, 18 May 2016 08:37:50 -0700 Received: from HQMAIL101.nvidia.com ([172.20.12.94]) by hqnvupgp08.nvidia.com (PGP Universal service); Wed, 18 May 2016 08:36:40 -0700 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Wed, 18 May 2016 08:36:40 -0700 Received: from HQMAIL103.nvidia.com (172.20.187.11) by HQMAIL101.nvidia.com (172.20.187.10) with Microsoft SMTP Server (TLS) id 15.0.1130.7; Wed, 18 May 2016 15:37:53 +0000 Received: from hqnvemgw02.nvidia.com (172.16.227.111) by HQMAIL103.nvidia.com (172.20.187.11) with Microsoft SMTP Server id 15.0.1130.7 via Frontend Transport; Wed, 18 May 2016 15:37:53 +0000 Received: from jonathanh-lm.nvidia.com (Not Verified[10.21.132.103]) by hqnvemgw02.nvidia.com with Trustwave SEG (v7, 5, 5, 8150) id ; Wed, 18 May 2016 08:37:53 -0700 From: Jon Hunter To: Thierry Reding , David Airlie , Daniel Vetter , Stephen Warren , Alexandre Courbot CC: , , , Jon Hunter Subject: [PATCH V3] drm/tegra: Fix crash caused by reference count imbalance Date: Wed, 18 May 2016 16:37:36 +0100 Message-ID: <1463585856-16606-1-git-send-email-jonathanh@nvidia.com> X-Mailer: git-send-email 2.1.4 X-NVConfidentiality: public MIME-Version: 1.0 Sender: linux-tegra-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-tegra@vger.kernel.org Commit d2307dea14a4 ("drm/atomic: use connector references (v3)") added reference counting for DRM connectors and this caused a crash when exercising system suspend on Tegra114 Dalmore. The Tegra DSI driver implements a Tegra specific function, tegra_dsi_connector_duplicate_state(), to duplicate the connector state and destroys the state using the generic helper function, drm_atomic_helper_connector_destroy_state(). Following commit d2307dea14a4 ("drm/atomic: use connector references (v3)") there is now an imbalance in the connector reference count because the Tegra function to duplicate state does not take a reference when duplicating the state information. However, the generic helper function to destroy the state information assumes a reference has been taken and during system suspend, when the connector state is destroyed, this leads to a crash because we attempt to put the reference for an object that has already been freed. Fix this by calling __drm_atomic_helper_connector_duplicate_state() from tegra_dsi_connector_duplicate_state() to ensure that we take a reference on a connector if crtc is set. Note that this will also copy the connector state a 2nd time, but this should be harmless. By fixing tegra_dsi_connector_duplicate_state() to take a reference, although a crash was no longer seen, it was then observed that after each system suspend-resume cycle, the reference would be one greater than before the suspend-resume cycle. Following commit d2307dea14a4 ("drm/atomic: use connector references (v3)"), it was found that we also need to put the reference when calling the function tegra_dsi_connector_reset() before freeing the state. Fix this by updating tegra_dsi_connector_reset() to call the function __drm_atomic_helper_connector_destroy_state() in order to put the reference for the connector. Fixes: d2307dea14a4 ("drm/atomic: use connector references (v3)") Signed-off-by: Jon Hunter Reviewed-by: Daniel Vetter Acked-by: Thierry Reding --- V3 changes: - Dropped WARN_ON V2 changes: - Updated to next-20160518 - Replaced open coding of call to drm_connector_reference() with __drm_atomic_helper_connector_duplicate_state() per Daniel's feedback. drivers/gpu/drm/tegra/dsi.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c index 44e102799195..d1239ebc190f 100644 --- a/drivers/gpu/drm/tegra/dsi.c +++ b/drivers/gpu/drm/tegra/dsi.c @@ -745,13 +745,17 @@ static void tegra_dsi_soft_reset(struct tegra_dsi *dsi) static void tegra_dsi_connector_reset(struct drm_connector *connector) { - struct tegra_dsi_state *state = - kzalloc(sizeof(*state), GFP_KERNEL); + struct tegra_dsi_state *state = kzalloc(sizeof(*state), GFP_KERNEL); - if (state) { + if (!state) + return; + + if (connector->state) { + __drm_atomic_helper_connector_destroy_state(connector->state); kfree(connector->state); - __drm_atomic_helper_connector_reset(connector, &state->base); } + + __drm_atomic_helper_connector_reset(connector, &state->base); } static struct drm_connector_state * @@ -764,6 +768,9 @@ tegra_dsi_connector_duplicate_state(struct drm_connector *connector) if (!copy) return NULL; + __drm_atomic_helper_connector_duplicate_state(connector, + ©->base); + return ©->base; }