From: Jon Hunter <jonathanh@nvidia.com>
To: Thierry Reding <thierry.reding@gmail.com>,
David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
Stephen Warren <swarren@wwwdotorg.org>,
Alexandre Courbot <gnurou@gmail.com>
Cc: <dri-devel@lists.freedesktop.org>, <linux-tegra@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, Jon Hunter <jonathanh@nvidia.com>
Subject: [PATCH V2] drm/tegra: Fix crash caused by reference count imbalance
Date: Wed, 18 May 2016 12:11:29 +0100 [thread overview]
Message-ID: <1463569889-14632-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 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.
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>
---
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..a49bb006182d 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 (WARN_ON(!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;
}
--
2.1.4
next reply other threads:[~2016-05-18 11:11 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-18 11:11 Jon Hunter [this message]
2016-05-18 12:17 ` [PATCH V2] drm/tegra: Fix crash caused by reference count imbalance Daniel Vetter
2016-05-18 14:12 ` Thierry Reding
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=1463569889-14632-1-git-send-email-jonathanh@nvidia.com \
--to=jonathanh@nvidia.com \
--cc=airlied@linux.ie \
--cc=daniel@ffwll.ch \
--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