All of lore.kernel.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 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.