[09/11] drm/i915: check connector->encoder before using it.

Message ID	1400640904-16847-10-git-send-email-airlied@gmail.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <intel-gfx-bounces@lists.freedesktop.org> From: Dave Airlie <airlied@gmail.com> To: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org Date: Wed, 21 May 2014 12:55:02 +1000 Message-Id: <1400640904-16847-10-git-send-email-airlied@gmail.com> In-Reply-To: <1400640904-16847-1-git-send-email-airlied@gmail.com> References: <1400640904-16847-1-git-send-email-airlied@gmail.com> Subject: [Intel-gfx] [PATCH 09/11] drm/i915: check connector->encoder before using it. Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Message ID

1400640904-16847-10-git-send-email-airlied@gmail.com (mailing list archive)

State

New, archived

Headers

From: Dave Airlie <airlied@gmail.com>
To: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Date: Wed, 21 May 2014 12:55:02 +1000
Message-Id: <1400640904-16847-10-git-send-email-airlied@gmail.com>
In-Reply-To: <1400640904-16847-1-git-send-email-airlied@gmail.com>
References: <1400640904-16847-1-git-send-email-airlied@gmail.com>
Subject: [Intel-gfx] [PATCH 09/11] drm/i915: check connector->encoder before
	using it.
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Commit Message

Dave Airlie May 21, 2014, 2:55 a.m. UTC

From: Dave Airlie <airlied@redhat.com>

DP MST will need connectors that aren't connected to specific
encoders, add some checks in advance to avoid oopses.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  | 16 +++++++++-------
 drivers/gpu/drm/i915/i915_irq.c      |  4 ++++
 drivers/gpu/drm/i915/intel_display.c | 25 ++++++++++++++-----------
 3 files changed, 27 insertions(+), 18 deletions(-)

Comments

Todd Previte June 17, 2014, 3:23 p.m. UTC | #1

Looks good.

Reviewed-by: Todd Previte <tprevite@gmail.com>

> Dave Airlie <mailto:airlied@gmail.com>
> Tuesday, May 20, 2014 7:55 PM
> From: Dave Airlie <airlied@redhat.com>
>
> DP MST will need connectors that aren't connected to specific
> encoders, add some checks in advance to avoid oopses.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 16 +++++++++-------
> drivers/gpu/drm/i915/i915_irq.c | 4 ++++
> drivers/gpu/drm/i915/intel_display.c | 25 ++++++++++++++-----------
> 3 files changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 1e83ae4..88e944f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2279,13 +2279,15 @@ static void intel_connector_info(struct 
> seq_file *m,
> seq_printf(m, "\tCEA rev: %d\n",
> connector->display_info.cea_rev);
> }
> - if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
> - intel_encoder->type == INTEL_OUTPUT_EDP)
> - intel_dp_info(m, intel_connector);
> - else if (intel_encoder->type == INTEL_OUTPUT_HDMI)
> - intel_hdmi_info(m, intel_connector);
> - else if (intel_encoder->type == INTEL_OUTPUT_LVDS)
> - intel_lvds_info(m, intel_connector);
> + if (intel_encoder) {
> + if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
> + intel_encoder->type == INTEL_OUTPUT_EDP)
> + intel_dp_info(m, intel_connector);
> + else if (intel_encoder->type == INTEL_OUTPUT_HDMI)
> + intel_hdmi_info(m, intel_connector);
> + else if (intel_encoder->type == INTEL_OUTPUT_LVDS)
> + intel_lvds_info(m, intel_connector);
> + }
>
> seq_printf(m, "\tmodes:\n");
> list_for_each_entry(mode, &connector->modes, head)
> diff --git a/drivers/gpu/drm/i915/i915_irq.c 
> b/drivers/gpu/drm/i915/i915_irq.c
> index afa5519..5852dee 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1016,6 +1016,8 @@ static void i915_hotplug_work_func(struct 
> work_struct *work)
> dev_priv->hpd_event_bits = 0;
> list_for_each_entry(connector, &mode_config->connector_list, head) {
> intel_connector = to_intel_connector(connector);
> + if (!intel_connector->encoder)
> + continue;
> intel_encoder = intel_connector->encoder;
> if (intel_encoder->hpd_pin > HPD_NONE &&
> dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == 
> HPD_MARK_DISABLED &&
> @@ -1046,6 +1048,8 @@ static void i915_hotplug_work_func(struct 
> work_struct *work)
>
> list_for_each_entry(connector, &mode_config->connector_list, head) {
> intel_connector = to_intel_connector(connector);
> + if (!intel_connector->encoder)
> + continue;
> intel_encoder = intel_connector->encoder;
> if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
> if (intel_encoder->hot_plug)
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index b39d036..75b2aaf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4600,20 +4600,23 @@ static void intel_connector_check_state(struct 
> intel_connector *connector)
> "wrong connector dpms state\n");
> WARN(connector->base.encoder != &encoder->base,
> "active connector not linked to encoder\n");
> - WARN(!encoder->connectors_active,
> - "encoder->connectors_active not set\n");
>
> - encoder_enabled = encoder->get_hw_state(encoder, &pipe);
> - WARN(!encoder_enabled, "encoder not enabled\n");
> - if (WARN_ON(!encoder->base.crtc))
> - return;
> + if (encoder) {
> + WARN(!encoder->connectors_active,
> + "encoder->connectors_active not set\n");
> +
> + encoder_enabled = encoder->get_hw_state(encoder, &pipe);
> + WARN(!encoder_enabled, "encoder not enabled\n");
> + if (WARN_ON(!encoder->base.crtc))
> + return;
>
> - crtc = encoder->base.crtc;
> + crtc = encoder->base.crtc;
>
> - WARN(!crtc->enabled, "crtc not enabled\n");
> - WARN(!to_intel_crtc(crtc)->active, "crtc not active\n");
> - WARN(pipe != to_intel_crtc(crtc)->pipe,
> - "encoder active on the wrong pipe\n");
> + WARN(!crtc->enabled, "crtc not enabled\n");
> + WARN(!to_intel_crtc(crtc)->active, "crtc not active\n");
> + WARN(pipe != to_intel_crtc(crtc)->pipe,
> + "encoder active on the wrong pipe\n");
> + }
> }
> }
>
> Dave Airlie <mailto:airlied@gmail.com>
> Tuesday, May 20, 2014 7:54 PM
> Hey,
>
> So this set is pretty close to what I think we should be merging 
> initially,
>
> Since the last set, it makes fbcon and suspend/resume work a lot better,
>
> I've also fixed a couple of bugs in -intel that make things work a lot
> better.
>
> I've bashed on this a bit using kms-flip from intel-gpu-tools, hacked
> to add 3 monitor support.
>
> It still generates a fair few i915 state checker backtraces, and some
> of them are fairly hard to work out, it might be we should just tone
> down the state checker for encoders/connectors with no actual hw backing
> them.
>
> Dave.
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 1e83ae4..88e944f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2279,13 +2279,15 @@  static void intel_connector_info(struct seq_file *m,
 		seq_printf(m, "\tCEA rev: %d\n",
 			   connector->display_info.cea_rev);
 	}
-	if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
-	    intel_encoder->type == INTEL_OUTPUT_EDP)
-		intel_dp_info(m, intel_connector);
-	else if (intel_encoder->type == INTEL_OUTPUT_HDMI)
-		intel_hdmi_info(m, intel_connector);
-	else if (intel_encoder->type == INTEL_OUTPUT_LVDS)
-		intel_lvds_info(m, intel_connector);
+	if (intel_encoder) {
+		if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
+		    intel_encoder->type == INTEL_OUTPUT_EDP)
+			intel_dp_info(m, intel_connector);
+		else if (intel_encoder->type == INTEL_OUTPUT_HDMI)
+			intel_hdmi_info(m, intel_connector);
+		else if (intel_encoder->type == INTEL_OUTPUT_LVDS)
+			intel_lvds_info(m, intel_connector);
+	}
 
 	seq_printf(m, "\tmodes:\n");
 	list_for_each_entry(mode, &connector->modes, head)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index afa5519..5852dee 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1016,6 +1016,8 @@  static void i915_hotplug_work_func(struct work_struct *work)
 	dev_priv->hpd_event_bits = 0;
 	list_for_each_entry(connector, &mode_config->connector_list, head) {
 		intel_connector = to_intel_connector(connector);
+		if (!intel_connector->encoder)
+			continue;
 		intel_encoder = intel_connector->encoder;
 		if (intel_encoder->hpd_pin > HPD_NONE &&
 		    dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_MARK_DISABLED &&
@@ -1046,6 +1048,8 @@  static void i915_hotplug_work_func(struct work_struct *work)
 
 	list_for_each_entry(connector, &mode_config->connector_list, head) {
 		intel_connector = to_intel_connector(connector);
+		if (!intel_connector->encoder)
+			continue;
 		intel_encoder = intel_connector->encoder;
 		if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
 			if (intel_encoder->hot_plug)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b39d036..75b2aaf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4600,20 +4600,23 @@  static void intel_connector_check_state(struct intel_connector *connector)
 		     "wrong connector dpms state\n");
 		WARN(connector->base.encoder != &encoder->base,
 		     "active connector not linked to encoder\n");
-		WARN(!encoder->connectors_active,
-		     "encoder->connectors_active not set\n");
 
-		encoder_enabled = encoder->get_hw_state(encoder, &pipe);
-		WARN(!encoder_enabled, "encoder not enabled\n");
-		if (WARN_ON(!encoder->base.crtc))
-			return;
+		if (encoder) {
+			WARN(!encoder->connectors_active,
+			     "encoder->connectors_active not set\n");
+
+			encoder_enabled = encoder->get_hw_state(encoder, &pipe);
+			WARN(!encoder_enabled, "encoder not enabled\n");
+			if (WARN_ON(!encoder->base.crtc))
+				return;
 
-		crtc = encoder->base.crtc;
+			crtc = encoder->base.crtc;
 
-		WARN(!crtc->enabled, "crtc not enabled\n");
-		WARN(!to_intel_crtc(crtc)->active, "crtc not active\n");
-		WARN(pipe != to_intel_crtc(crtc)->pipe,
-		     "encoder active on the wrong pipe\n");
+			WARN(!crtc->enabled, "crtc not enabled\n");
+			WARN(!to_intel_crtc(crtc)->active, "crtc not active\n");
+			WARN(pipe != to_intel_crtc(crtc)->pipe,
+			     "encoder active on the wrong pipe\n");
+		}
 	}
 }

[09/11] drm/i915: check connector->encoder before using it.

Commit Message

Comments

Patch