diff mbox

[3/3] drm/i915: Set the digital port encoder personality during modeset

Message ID 1389113755-2021-3-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni Jan. 7, 2014, 4:55 p.m. UTC 
From: Damien Lespiau <damien.lespiau@intel.com>

The ->detect() vfunc of connectors is usually responsible for setting the
encoder type on intel_digital_ports when a hotplug event happens.

However we sometimes want to force a modeset on a specific connector:
  - the user can ask the SETCRTC ioctl to use a connector that isn't
    marked as connected (because we never received a hotplug event for it).
    This can be also used in tests to do a modeset without the need of a
    plugged-in monitor.
  - the command line video= option can be used to force modesets, eg.:
    video=HDMI-A-1:1024x768e

So, before we try to do anything with the DDI encoder as part of a modeset,
we need to ensure that the personality of the encoder matches the selected
connector.

v2: Made by Paulo:
  - Return false if we have more than one connector
  - WARN in case it's not eDP/DP/HDMI
  - Don't break error strings in pieces
  - Change the "status == connected" message from DRM_ERROR to
    DRM_DEBUG_KMS
  - Don't store+use the old encoder->type at compute_config
  - Add bugzilla reference
  - Add testcase reference
v3: Made by Paulo:
  - WARN in case "connectors > 1", since we have a patch that should
    catch this case earlier

Testcase: igt/kms_setmode/clone-exclusive-crtc
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=68463
Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 94 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 91 insertions(+), 3 deletions(-)

Comments

Lespiau, Damien Jan. 10, 2014, 6:13 p.m. UTC | #1 
On Tue, Jan 07, 2014 at 02:55:55PM -0200, Paulo Zanoni wrote:
> +static bool
> +intel_ddi_ensure_encoder_type(struct intel_encoder *encoder)

[...]

> +		if (connector->base.status == connector_status_connected) {
> +			DRM_DEBUG_KMS("Can't change DDI %c personality to %s, it has a connected %s device\n",
> +				      port_name(port),
> +				      intel_output_name(new_encoder_type),
> +				      intel_output_name(old_encoder_type));
> +			return false;
> +		}

Actually, this test should be:

                if (old_encoder_type != INTEL_OUTPUT_UNKNOWN &&
                    connector->base.status == connector_status_connected) {

Otherwise we miss the case I wanted the patch for in the first place: Forcing a
not-yet-sexed DDI connector (type == UNKNOWN) on the command line, say.
video=HDMI-A-1:1024x768e. In that case, we end up with type == UNKNOWN and
connector->base.status == connector_status_connected, but still want to force
the DDI personality to HDMI.

--
Damien
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 1488b28..4ec1665 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1512,18 +1512,106 @@  static void intel_ddi_destroy(struct drm_encoder *encoder)
 	intel_dp_encoder_destroy(encoder);
 }
 
+/*
+ * The ->detect() vfunc of connectors is usually responsible for setting the
+ * encoder type on intel_digital_ports when a hotplug event happens.
+ *
+ * However we sometimes want to force a modeset on a specific connector:
+ *   - the user can ask the SETCRTC ioctl to use a connector that isn't marked
+ *     as connected (because we never received a hotplug event for it).
+ *     This can be also used in tests to do a modeset without the need of a
+ *     plugged-in monitor.
+ *   - the command line video= option can be used to force modesets, eg.:
+ *     video=HDMI-A-1:1024x768e
+ *
+ * So, before we try to do anything with the DDI encoder as part of a modeset,
+ * we need to ensure that the personality of the encoder matches the selected
+ * connector.
+ */
+static bool
+intel_ddi_ensure_encoder_type(struct intel_encoder *encoder)
+{
+	struct drm_device *dev = encoder->base.dev;
+	struct intel_connector *connector;
+	int connectors = 0;
+
+	list_for_each_entry(connector, &dev->mode_config.connector_list,
+			    base.head) {
+		int connector_type, old_encoder_type, new_encoder_type;
+		int port;
+
+		if (connector->new_encoder != encoder)
+			continue;
+
+		connectors++;
+		if (WARN_ON(connectors > 1))
+			return false;
+
+		connector_type = connector->base.connector_type;
+		old_encoder_type = encoder->type;
+		switch (connector_type) {
+		case DRM_MODE_CONNECTOR_HDMIA:
+		case DRM_MODE_CONNECTOR_HDMIB:
+			new_encoder_type = INTEL_OUTPUT_HDMI;
+			break;
+		case DRM_MODE_CONNECTOR_DisplayPort:
+			new_encoder_type = INTEL_OUTPUT_DISPLAYPORT;
+			break;
+		case DRM_MODE_CONNECTOR_eDP:
+			continue;
+		default:
+			WARN(1, "DRM connector type %d\n", connector_type);
+			continue;
+		}
+
+		if (old_encoder_type == new_encoder_type)
+			continue;
+
+		port = intel_ddi_get_encoder_port(encoder);
+
+		if (old_encoder_type == INTEL_OUTPUT_EDP) {
+			DRM_ERROR("Can't change DDI %c personality to %s, it's an eDP DDI\n",
+				  port_name(port),
+				  intel_output_name(new_encoder_type));
+			return false;
+		}
+
+		if (connector->base.status == connector_status_connected) {
+			DRM_DEBUG_KMS("Can't change DDI %c personality to %s, it has a connected %s device\n",
+				      port_name(port),
+				      intel_output_name(new_encoder_type),
+				      intel_output_name(old_encoder_type));
+			return false;
+		}
+
+		DRM_DEBUG_KMS("Changing DDI %c from %s to %s\n",
+			      port_name(port),
+			      intel_output_name(old_encoder_type),
+			      intel_output_name(new_encoder_type));
+
+		encoder->type = new_encoder_type;
+	}
+
+	return true;
+}
+
 static bool intel_ddi_compute_config(struct intel_encoder *encoder,
 				     struct intel_crtc_config *pipe_config)
 {
-	int type = encoder->type;
 	int port = intel_ddi_get_encoder_port(encoder);
+	int ret;
+
+	ret = intel_ddi_ensure_encoder_type(encoder);
+	if (!ret)
+		return false;
 
-	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
+	WARN(encoder->type == INTEL_OUTPUT_UNKNOWN,
+	     "compute_config() on unknown output!\n");
 
 	if (port == PORT_A)
 		pipe_config->cpu_transcoder = TRANSCODER_EDP;
 
-	if (type == INTEL_OUTPUT_HDMI)
+	if (encoder->type == INTEL_OUTPUT_HDMI)
 		return intel_hdmi_compute_config(encoder, pipe_config);
 	else
 		return intel_dp_compute_config(encoder, pipe_config);