All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: don't leak dp_connector at intel_ddi_init
@ 2013-10-09 16:52 Paulo Zanoni
  2013-10-09 17:25 ` Daniel Vetter
  0 siblings, 1 reply; 2+ messages in thread
From: Paulo Zanoni @ 2013-10-09 16:52 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Regression introduced by:
    commit 311a20949f047a70935d6591010f42336f5402e7
    Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
        drm/i915: don't init DP or HDMI when not supported by DDI port

Since the commit above it is possible to have a DDI encoder that has
the HDMI connector but not the DP connector (in case the port doesn't
support DP). In this case, we must properly free the DP connector.

We just leak this once, so it's not a big deal.

Reported by kmemleak.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 62 +++++++++++++++++++++++++++-------------
 1 file changed, 42 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 6d335f8..31f4fe2 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1345,6 +1345,41 @@ static const struct drm_encoder_funcs intel_ddi_funcs = {
 	.destroy = intel_ddi_destroy,
 };
 
+static struct intel_connector *
+intel_ddi_init_dp_connector(struct intel_digital_port *intel_dig_port)
+{
+	struct intel_connector *connector;
+	enum port port = intel_dig_port->port;
+
+	connector = kzalloc(sizeof(*connector), GFP_KERNEL);
+	if (!connector)
+		return NULL;
+
+	intel_dig_port->dp.output_reg = DDI_BUF_CTL(port);
+	if (!intel_dp_init_connector(intel_dig_port, connector)) {
+		kfree(connector);
+		return NULL;
+	}
+
+	return connector;
+}
+
+static struct intel_connector *
+intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port)
+{
+	struct intel_connector *connector;
+	enum port port = intel_dig_port->port;
+
+	connector = kzalloc(sizeof(*connector), GFP_KERNEL);
+	if (!connector)
+		return NULL;
+
+	intel_dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port);
+	intel_hdmi_init_connector(intel_dig_port, connector);
+
+	return connector;
+}
+
 void intel_ddi_init(struct drm_device *dev, enum port port)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1369,12 +1404,6 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 	if (!intel_dig_port)
 		return;
 
-	dp_connector = kzalloc(sizeof(*dp_connector), GFP_KERNEL);
-	if (!dp_connector) {
-		kfree(intel_dig_port);
-		return;
-	}
-
 	intel_encoder = &intel_dig_port->base;
 	encoder = &intel_encoder->base;
 
@@ -1394,29 +1423,22 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
 					  (DDI_BUF_PORT_REVERSAL |
 					   DDI_A_4_LANES);
-	intel_dig_port->dp.output_reg = DDI_BUF_CTL(port);
 
 	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
 	intel_encoder->crtc_mask =  (1 << 0) | (1 << 1) | (1 << 2);
 	intel_encoder->cloneable = false;
 	intel_encoder->hot_plug = intel_ddi_hot_plug;
 
-	if (init_dp && !intel_dp_init_connector(intel_dig_port, dp_connector)) {
-		drm_encoder_cleanup(encoder);
-		kfree(intel_dig_port);
-		kfree(dp_connector);
-		return;
-	}
+	if (init_dp)
+		dp_connector = intel_ddi_init_dp_connector(intel_dig_port);
 
 	/* In theory we don't need the encoder->type check, but leave it just in
 	 * case we have some really bad VBTs... */
-	if (intel_encoder->type != INTEL_OUTPUT_EDP && init_hdmi) {
-		hdmi_connector = kzalloc(sizeof(*hdmi_connector),
-					 GFP_KERNEL);
-		if (!hdmi_connector)
-			return;
+	if (intel_encoder->type != INTEL_OUTPUT_EDP && init_hdmi)
+		hdmi_connector = intel_ddi_init_hdmi_connector(intel_dig_port);
 
-		intel_dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port);
-		intel_hdmi_init_connector(intel_dig_port, hdmi_connector);
+	if (!dp_connector && !hdmi_connector) {
+		drm_encoder_cleanup(encoder);
+		kfree(intel_dig_port);
 	}
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] drm/i915: don't leak dp_connector at intel_ddi_init
  2013-10-09 16:52 [PATCH] drm/i915: don't leak dp_connector at intel_ddi_init Paulo Zanoni
@ 2013-10-09 17:25 ` Daniel Vetter
  0 siblings, 0 replies; 2+ messages in thread
From: Daniel Vetter @ 2013-10-09 17:25 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni

On Wed, Oct 09, 2013 at 01:52:36PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Regression introduced by:
>     commit 311a20949f047a70935d6591010f42336f5402e7
>     Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
>         drm/i915: don't init DP or HDMI when not supported by DDI port
> 
> Since the commit above it is possible to have a DDI encoder that has
> the HDMI connector but not the DP connector (in case the port doesn't
> support DP). In this case, we must properly free the DP connector.
> 
> We just leak this once, so it's not a big deal.
> 
> Reported by kmemleak.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-10-09 17:24 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-09 16:52 [PATCH] drm/i915: don't leak dp_connector at intel_ddi_init Paulo Zanoni
2013-10-09 17:25 ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.