public inbox for linux-tegra@vger.kernel.org
 help / color / mirror / Atom feed
From: Jon Hunter <jonathanh@nvidia.com>
To: Thierry Reding <thierry.reding@gmail.com>,
	David Airlie <airlied@linux.ie>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Alexandre Courbot <gnurou@gmail.com>
Cc: linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org,
	Jon Hunter <jonathanh@nvidia.com>
Subject: [PATCH] drm/tegra: Fix crash caused by reference count imbalance
Date: Tue, 17 May 2016 17:27:15 +0100	[thread overview]
Message-ID: <1463502435-29217-1-git-send-email-jonathanh@nvidia.com> (raw)

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 aligning tegra_dsi_connector_duplicate_state() with commit
d2307dea14a4 ("drm/atomic: use connector references (v3)"), so that we
take a reference on a connector if crtc is set.

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.

Finally, add a warning if allocating memory for the state information
fails in tegra_dsi_connector_reset().

Fixes: d2307dea14a4 ("drm/atomic: use connector references (v3)")

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/gpu/drm/tegra/dsi.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index 44e102799195..68aaa4c33cd8 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -745,13 +745,18 @@ 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 (WARN_ON(!state))
+		return;
+
+	if (connector->state) {
+		__drm_atomic_helper_connector_destroy_state(connector,
+							    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 +769,9 @@ tegra_dsi_connector_duplicate_state(struct drm_connector *connector)
 	if (!copy)
 		return NULL;
 
+	if (copy->base.crtc)
+		drm_connector_reference(connector);
+
 	return &copy->base;
 }
 
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

             reply	other threads:[~2016-05-17 16:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-17 16:27 Jon Hunter [this message]
2016-05-17 16:46 ` [PATCH] drm/tegra: Fix crash caused by reference count imbalance Daniel Vetter
     [not found]   ` <20160517164632.GT27098-dv86pmgwkMBes7Z6vYuT8azUEOm+Xw19@public.gmane.org>
2016-05-17 17:29     ` Jon Hunter
     [not found]       ` <573B54F2.4010300-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-05-17 17:36         ` Daniel Vetter
     [not found]           ` <CAKMK7uHNKPfKAQ49rboWSiXe_OrxK6On6ZrFrtSaCwmU7VfQGA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-05-18  9:18             ` Jon Hunter
     [not found]               ` <573C337C.3030009-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2016-05-18 10:26                 ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1463502435-29217-1-git-send-email-jonathanh@nvidia.com \
    --to=jonathanh@nvidia.com \
    --cc=airlied@linux.ie \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gnurou@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=swarren@wwwdotorg.org \
    --cc=thierry.reding@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox