public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: [PATCH v3.1 04/13] drm/i915: Convert connector checking to atomic, v3.
Date: Thu, 6 Aug 2015 13:49:22 +0200	[thread overview]
Message-ID: <55C349C2.4040909@linux.intel.com> (raw)
In-Reply-To: <1438771031-23227-5-git-send-email-maarten.lankhorst@linux.intel.com>

Right now dpms callbacks can still fiddle with the connector state,
but it can only turn connectors off.

This is remediated by only checking crtc->state->active when the
connector is active, and ignore crtc->state->active when the
connector is off.

connectors_active is no longer checked, and will be removed later
in this series together with dpms.

Another check for !encoder->crtc is performed by check_encoder_state
too, so it can be removed.

Changes since v1:
- Add commit message.
- rename state to old_state.
- Move deletion of mst_port check to mst patch.
Changes since v2:
- Fix a null pointer dereference on MST now hw readout is fixed.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
---
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 77b4da7e698c..9baddc16b285 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6360,38 +6360,36 @@ static void intel_encoder_dpms(struct intel_encoder *encoder, int mode)
  * internal consistency). */
 static void intel_connector_check_state(struct intel_connector *connector)
 {
+	struct drm_crtc *crtc = connector->base.state->crtc;
+
+	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
+		      connector->base.base.id,
+		      connector->base.name);
+
 	if (connector->get_hw_state(connector)) {
-		struct intel_encoder *encoder = connector->encoder;
-		struct drm_crtc *crtc;
-		bool encoder_enabled;
-		enum pipe pipe;
+		struct drm_encoder *encoder = &connector->encoder->base;
+		struct drm_connector_state *conn_state = connector->base.state;
 
-		DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
-			      connector->base.base.id,
-			      connector->base.name);
+		I915_STATE_WARN(!crtc,
+			 "connector enabled without attached crtc\n");
 
-		I915_STATE_WARN(connector->base.dpms == DRM_MODE_DPMS_OFF,
-		     "wrong connector dpms state\n");
-		I915_STATE_WARN(connector->base.encoder != &encoder->base,
-		     "active connector not linked to encoder\n");
+		if (!crtc)
+			return;
 
-		if (encoder) {
-			I915_STATE_WARN(!encoder->connectors_active,
-			     "encoder->connectors_active not set\n");
+		I915_STATE_WARN(!crtc->state->active,
+		      "connector is active, but attached crtc isn't\n");
 
-			encoder_enabled = encoder->get_hw_state(encoder, &pipe);
-			I915_STATE_WARN(!encoder_enabled, "encoder not enabled\n");
-			if (I915_STATE_WARN_ON(!encoder->base.crtc))
-				return;
+		if (!encoder)
+			return;
 
-			crtc = encoder->base.crtc;
+		I915_STATE_WARN(conn_state->best_encoder != encoder,
+			"atomic encoder doesn't match attached encoder\n");
 
-			I915_STATE_WARN(!crtc->state->enable,
-					"crtc not enabled\n");
-			I915_STATE_WARN(!to_intel_crtc(crtc)->active, "crtc not active\n");
-			I915_STATE_WARN(pipe != to_intel_crtc(crtc)->pipe,
-			     "encoder active on the wrong pipe\n");
-		}
+		I915_STATE_WARN(conn_state->crtc != encoder->crtc,
+			"attached encoder crtc differs from connector crtc\n");
+	} else {
+		I915_STATE_WARN(!crtc && connector->base.state->best_encoder,
+			"best encoder set without crtc!\n");
 	}
 }
 
@@ -12699,20 +12697,23 @@ static void check_wm_state(struct drm_device *dev)
 }
 
 static void
-check_connector_state(struct drm_device *dev)
+check_connector_state(struct drm_device *dev,
+		      struct drm_atomic_state *old_state)
 {
-	struct intel_connector *connector;
+	struct drm_connector_state *old_conn_state;
+	struct drm_connector *connector;
+	int i;
 
-	for_each_intel_connector(dev, connector) {
-		struct drm_encoder *encoder = connector->base.encoder;
-		struct drm_connector_state *state = connector->base.state;
+	for_each_connector_in_state(old_state, connector, old_conn_state, i) {
+		struct drm_encoder *encoder = connector->encoder;
+		struct drm_connector_state *state = connector->state;
 
 		/* This also checks the encoder/connector hw state with the
 		 * ->get_hw_state callbacks. */
-		intel_connector_check_state(connector);
+		intel_connector_check_state(to_intel_connector(connector));
 
 		I915_STATE_WARN(state->best_encoder != encoder,
-		     "connector's staged encoder doesn't match current encoder\n");
+		     "connector's atomic encoder doesn't match legacy encoder\n");
 	}
 }
 
@@ -12903,7 +12904,7 @@ intel_modeset_check_state(struct drm_device *dev,
 			  struct drm_atomic_state *old_state)
 {
 	check_wm_state(dev);
-	check_connector_state(dev);
+	check_connector_state(dev, old_state);
 	check_encoder_state(dev);
 	check_crtc_state(dev);
 	check_shared_dpll_state(dev);

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2015-08-06 11:49 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-05 10:36 [PATCH v3 00/13] DPMS updates and atomic state checking Maarten Lankhorst
2015-08-05 10:36 ` [PATCH v3 01/13] drm/i915: Make the force_thru workaround atomic, v2 Maarten Lankhorst
2015-08-05 14:03   ` Daniel Vetter
2015-08-05 10:37 ` [PATCH v3 02/13] drm/i915: Validate the state after an atomic modeset only, and pass the state Maarten Lankhorst
2015-08-05 10:37 ` [PATCH v3 03/13] drm/i915: Update atomic state when removing mst connector Maarten Lankhorst
2015-08-06 11:47   ` [PATCH v3.1 1/3] drm/i915: Fix broken mst get_hw_state Maarten Lankhorst
2015-08-06 11:47     ` [PATCH v3.1 2/3] drm/i915: Update atomic state when removing mst connector, v3 Maarten Lankhorst
2015-08-06 12:30       ` Sivakumar Thulasimani
2015-08-06 11:47     ` [PATCH v3.1 3/3] drm/i915: Don't try to remove MST cleanly when force removed Maarten Lankhorst
2015-08-06 13:01       ` Daniel Vetter
2015-08-06 13:51         ` Maarten Lankhorst
2015-08-06 15:45           ` Daniel Vetter
2015-08-06 12:59     ` [PATCH v3.1 1/3] drm/i915: Fix broken mst get_hw_state Daniel Vetter
2015-08-06 13:37       ` Maarten Lankhorst
2015-08-06 15:58         ` Daniel Vetter
2015-08-05 10:37 ` [PATCH v3 04/13] drm/i915: Convert connector checking to atomic, v2 Maarten Lankhorst
2015-08-06 11:49   ` Maarten Lankhorst [this message]
2015-08-05 10:37 ` [PATCH v3 05/13] drm/i915: Remove some unneeded checks from check_crtc_state Maarten Lankhorst
2015-08-05 10:37 ` [PATCH v3 06/13] drm/i915: Remove connectors_active from state checking Maarten Lankhorst
2015-08-05 10:37 ` [PATCH v3 07/13] drm/i915: Make crtc checking use the atomic state, v2 Maarten Lankhorst
2015-08-05 10:37 ` [PATCH v3 08/13] drm/i915: Get rid of dpms handling Maarten Lankhorst
2015-08-05 10:37 ` [PATCH v3 09/13] drm/i915: Remove connectors_active from sanitization, v2 Maarten Lankhorst
2015-08-05 10:37 ` [PATCH v3 10/13] drm/i915: Remove connectors_active from intel_dp.c, v2 Maarten Lankhorst
2015-08-05 10:37 ` [PATCH v3 11/13] drm/i915: Remove connectors_active Maarten Lankhorst
2015-08-05 10:37 ` [PATCH v3 12/13] drm/i915: Only update mode related state if a modeset happened Maarten Lankhorst
2015-08-06 13:12   ` Daniel Vetter
2015-08-06 14:06     ` Maarten Lankhorst
2015-08-06 16:01       ` Daniel Vetter
2015-08-05 10:37 ` [PATCH v3 13/13] drm/i915: Handle return value in intel_pin_and_fence_fb_obj, v2 Maarten Lankhorst
2015-08-11 22:17   ` shuang.he
2015-08-06 13:13 ` [PATCH v3 00/13] DPMS updates and atomic state checking 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=55C349C2.4040909@linux.intel.com \
    --to=maarten.lankhorst@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /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