[PATCH] mode: Retrieve only the current information for a Connector

[PATCH] mode: Retrieve only the current information for a Connector

[Chris Wilson]

Add a new API that allows the caller to skip any forced probing, which
may require slow i2c to a remote display, and only report the currently
active mode and encoder for a Connector. This is often the information
of interest and is much, much faster than re-retrieving the link status
and EDIDs, e.g. if the caller only wishes to count the number of active
outputs.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
C: David Herrmann <dh.herrmann@googlemail.com>
---
 tests/modeprint/modeprint.c | 18 ++++++++-
 xf86drmMode.c               | 98 +++++++++++++++++++++++++++++++++++++++++++++
 xf86drmMode.h               | 17 +++++++-
 3 files changed, 129 insertions(+), 4 deletions(-)

diff --git a/tests/modeprint/modeprint.c b/tests/modeprint/modeprint.c
index 6f0d039..514d3ba 100644
--- a/tests/modeprint/modeprint.c
+++ b/tests/modeprint/modeprint.c
@@ -43,6 +43,7 @@
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
 
+int current;
 int connectors;
 int full_props;
 int edid;
@@ -272,7 +273,7 @@ int printRes(int fd, drmModeResPtr res)
 
 	if (connectors) {
 		for (i = 0; i < res->count_connectors; i++) {
-			connector = drmModeGetConnector(fd, res->connectors[i]);
+			connector = (current ? drmModeGetConnectorCurrent : drmModeGetConnector) (fd, res->connectors[i]);
 
 			if (!connector)
 				printf("Could not get connector %i\n", res->connectors[i]);
@@ -331,6 +332,7 @@ int printRes(int fd, drmModeResPtr res)
 
 void args(int argc, char **argv)
 {
+	int defaults = 1;
 	int i;
 
 	fbs = 0;
@@ -341,32 +343,41 @@ void args(int argc, char **argv)
 	full_modes = 0;
 	full_props = 0;
 	connectors = 0;
+	current = 0;
 
 	module_name = argv[1];
 
 	for (i = 2; i < argc; i++) {
 		if (strcmp(argv[i], "-fb") == 0) {
 			fbs = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-crtcs") == 0) {
 			crtcs = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-cons") == 0) {
 			connectors = 1;
 			modes = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-modes") == 0) {
 			connectors = 1;
 			modes = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-full") == 0) {
 			connectors = 1;
 			modes = 1;
 			full_modes = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-props") == 0) {
 			connectors = 1;
 			full_props = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-edids") == 0) {
 			connectors = 1;
 			edid = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-encoders") == 0) {
 			encoders = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-v") == 0) {
 			fbs = 1;
 			edid = 1;
@@ -376,10 +387,13 @@ void args(int argc, char **argv)
 			full_modes = 1;
 			full_props = 1;
 			connectors = 1;
+			defaults = 0;
+		} else if (strcmp(argv[i], "-current") == 0) {
+			current = 1;
 		}
 	}
 
-	if (argc == 2) {
+	if (defaults) {
 		fbs = 1;
 		edid = 1;
 		crtcs = 1;
diff --git a/xf86drmMode.c b/xf86drmMode.c
index 9ea8fe7..5544e5b 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -572,6 +572,104 @@ err_allocs:
 	return r;
 }
 
+drmModeConnectorPtr drmModeGetConnectorCurrent(int fd, uint32_t connector_id)
+{
+	struct drm_mode_modeinfo mode;
+	struct drm_mode_get_encoder enc;
+	struct drm_mode_get_connector conn;
+	unsigned int num_props = 0;
+	drmModeConnectorPtr r;
+
+	memclear(conn);
+	conn.connector_id = connector_id;
+	do {
+		if (conn.count_props) {
+			drmFree(U642VOID(conn.props_ptr));
+			drmFree(U642VOID(conn.prop_values_ptr));
+
+			conn.props_ptr = VOID2U64(drmMalloc(conn.count_props*sizeof(uint32_t)));
+			if (!conn.props_ptr)
+				goto err;
+
+			conn.prop_values_ptr = VOID2U64(drmMalloc(conn.count_props*sizeof(uint64_t)));
+			if (!conn.prop_values_ptr)
+				goto err;
+
+			num_props = conn.count_props;
+		}
+
+		conn.count_modes = 1; /* skip detect */
+		conn.modes_ptr = (uintptr_t)&mode;
+		conn.count_encoders = 0;
+
+		if (drmIoctl(fd, DRM_IOCTL_MODE_GETCONNECTOR, &conn))
+			goto err;
+	} while (conn.count_props != num_props);
+
+	/* Retrieve the current mode */
+	memclear(mode);
+	memclear(enc);
+	if (conn.encoder_id) {
+		struct drm_mode_crtc crtc;
+
+		enc.encoder_id = conn.encoder_id;
+		if (drmIoctl(fd, DRM_IOCTL_MODE_GETENCODER, &enc))
+			goto err;
+
+		memclear(crtc);
+		crtc.crtc_id = enc.crtc_id;
+		if (drmIoctl(fd, DRM_IOCTL_MODE_GETCRTC, &crtc))
+			goto err;
+
+		if (crtc.mode_valid)
+			mode = crtc.mode;
+	}
+
+	if(!(r = drmMalloc(sizeof(*r))))
+		goto err;
+
+	memset(r, 0, sizeof(*r));
+	r->connector_id = conn.connector_id;
+	r->encoder_id   = conn.encoder_id;
+	r->connection   = conn.connection;
+	r->mmWidth      = conn.mm_width;
+	r->mmHeight     = conn.mm_height;
+	/* convert subpixel from kernel to userspace */
+	r->subpixel     = conn.subpixel + 1;
+
+	if (mode.clock) {
+		r->count_modes = 1;
+		r->modes       = drmAllocCpy(&mode, 1, sizeof(mode));
+		if (!r->modes)
+			goto err_copy;
+	}
+
+	if (conn.encoder_id) {
+		r->count_encoders = 1;
+		r->encoders       = drmAllocCpy(&enc, 1, sizeof(enc));
+		if (!r->encoders)
+			goto err_copy;
+	}
+
+	r->count_props = conn.count_props;
+	r->props       = U642VOID(conn.props_ptr);
+	r->prop_values = U642VOID(conn.prop_values_ptr);
+
+	r->connector_type  = conn.connector_type;
+	r->connector_type_id = conn.connector_type_id;
+
+	return r;
+
+err_copy:
+	drmFree(r->modes);
+	drmFree(r->encoders);
+	drmFree(r);
+err:
+	drmFree(U642VOID(conn.props_ptr));
+	drmFree(U642VOID(conn.prop_values_ptr));
+	return 0;
+}
+
 int drmModeAttachMode(int fd, uint32_t connector_id, drmModeModeInfoPtr mode_info)
 {
 	struct drm_mode_mode_cmd res;
diff --git a/xf86drmMode.h b/xf86drmMode.h
index 856a6bb..278da48 100644
--- a/xf86drmMode.h
+++ b/xf86drmMode.h
@@ -422,10 +422,23 @@ drmModeEncoderPtr drmModeGetEncoder(int fd, uint32_t encoder_id);
  */
 
 /**
- * Retrive information about the connector connectorId.
+ * Retrieve all information about the connector connectorId. This will do a
+ * forced probe on the connector to retrieve remote information such as EDIDs
+ * from the display device.
  */
 extern drmModeConnectorPtr drmModeGetConnector(int fd,
-		uint32_t connectorId);
+					       uint32_t connectorId);
+
+/**
+ * Retrieve current information, i.e the currently active mode and encoder,
+ * about the connector connectorId. This will not do any probing on the
+ * connector or remote device, and only reports what is currently known.
+ * For the complete set of modes and encoders associated with the connector
+ * use drmModeGetConnector() which will do a probe to determine any display
+ * link changes first.
+ */
+extern drmModeConnectorPtr drmModeGetConnectorCurrent(int fd,
+						      uint32_t connector_id);
 
 /**
  * Attaches the given mode to an connector.
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] mode: Retrieve only the current information for a Connector

[Chris Wilson]

Add a new API that allows the caller to skip any forced probing, which
may require slow i2c to a remote display, and only report the currently
active mode and encoder for a Connector. This is often the information
of interest and is much, much faster than re-retrieving the link status
and EDIDs, e.g. if the caller only wishes to count the number of active
outputs.

v2: Fix error path to avoid double free after a failed GETCONNECTOR
ioctl.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: David Herrmann <dh.herrmann@googlemail.com>
---
 tests/modeprint/modeprint.c | 18 ++++++++-
 xf86drmMode.c               | 97 +++++++++++++++++++++++++++++++++++++++++++++
 xf86drmMode.h               | 17 +++++++-
 3 files changed, 128 insertions(+), 4 deletions(-)

diff --git a/tests/modeprint/modeprint.c b/tests/modeprint/modeprint.c
index 6f0d039..514d3ba 100644
--- a/tests/modeprint/modeprint.c
+++ b/tests/modeprint/modeprint.c
@@ -43,6 +43,7 @@
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
 
+int current;
 int connectors;
 int full_props;
 int edid;
@@ -272,7 +273,7 @@ int printRes(int fd, drmModeResPtr res)
 
 	if (connectors) {
 		for (i = 0; i < res->count_connectors; i++) {
-			connector = drmModeGetConnector(fd, res->connectors[i]);
+			connector = (current ? drmModeGetConnectorCurrent : drmModeGetConnector) (fd, res->connectors[i]);
 
 			if (!connector)
 				printf("Could not get connector %i\n", res->connectors[i]);
@@ -331,6 +332,7 @@ int printRes(int fd, drmModeResPtr res)
 
 void args(int argc, char **argv)
 {
+	int defaults = 1;
 	int i;
 
 	fbs = 0;
@@ -341,32 +343,41 @@ void args(int argc, char **argv)
 	full_modes = 0;
 	full_props = 0;
 	connectors = 0;
+	current = 0;
 
 	module_name = argv[1];
 
 	for (i = 2; i < argc; i++) {
 		if (strcmp(argv[i], "-fb") == 0) {
 			fbs = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-crtcs") == 0) {
 			crtcs = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-cons") == 0) {
 			connectors = 1;
 			modes = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-modes") == 0) {
 			connectors = 1;
 			modes = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-full") == 0) {
 			connectors = 1;
 			modes = 1;
 			full_modes = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-props") == 0) {
 			connectors = 1;
 			full_props = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-edids") == 0) {
 			connectors = 1;
 			edid = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-encoders") == 0) {
 			encoders = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-v") == 0) {
 			fbs = 1;
 			edid = 1;
@@ -376,10 +387,13 @@ void args(int argc, char **argv)
 			full_modes = 1;
 			full_props = 1;
 			connectors = 1;
+			defaults = 0;
+		} else if (strcmp(argv[i], "-current") == 0) {
+			current = 1;
 		}
 	}
 
-	if (argc == 2) {
+	if (defaults) {
 		fbs = 1;
 		edid = 1;
 		crtcs = 1;
diff --git a/xf86drmMode.c b/xf86drmMode.c
index 9ea8fe7..4433dc7 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -572,6 +572,103 @@ err_allocs:
 	return r;
 }
 
+drmModeConnectorPtr drmModeGetConnectorCurrent(int fd, uint32_t connector_id)
+{
+	struct drm_mode_modeinfo mode;
+	struct drm_mode_get_encoder enc;
+	struct drm_mode_get_connector conn;
+	unsigned int num_props = 0;
+	drmModeConnectorPtr r;
+
+	memclear(conn);
+	conn.connector_id = connector_id;
+	do {
+		if (conn.count_props) {
+			drmFree(U642VOID(conn.props_ptr));
+			conn.props_ptr = VOID2U64(drmMalloc(conn.count_props*sizeof(uint32_t)));
+			if (!conn.props_ptr)
+				goto err;
+
+			drmFree(U642VOID(conn.prop_values_ptr));
+			conn.prop_values_ptr = VOID2U64(drmMalloc(conn.count_props*sizeof(uint64_t)));
+			if (!conn.prop_values_ptr)
+				goto err;
+
+			num_props = conn.count_props;
+		}
+
+		conn.count_modes = 1; /* skip detect */
+		conn.modes_ptr = (uintptr_t)&mode;
+		conn.count_encoders = 0;
+
+		if (drmIoctl(fd, DRM_IOCTL_MODE_GETCONNECTOR, &conn))
+			goto err;
+	} while (conn.count_props != num_props);
+
+	/* Retrieve the current mode */
+	memclear(mode);
+	memclear(enc);
+	if (conn.encoder_id) {
+		struct drm_mode_crtc crtc;
+
+		enc.encoder_id = conn.encoder_id;
+		if (drmIoctl(fd, DRM_IOCTL_MODE_GETENCODER, &enc))
+			goto err;
+
+		memclear(crtc);
+		crtc.crtc_id = enc.crtc_id;
+		if (drmIoctl(fd, DRM_IOCTL_MODE_GETCRTC, &crtc))
+			goto err;
+
+		if (crtc.mode_valid)
+			mode = crtc.mode;
+	}
+
+	if(!(r = drmMalloc(sizeof(*r))))
+		goto err;
+
+	memset(r, 0, sizeof(*r));
+	r->connector_id = conn.connector_id;
+	r->encoder_id   = conn.encoder_id;
+	r->connection   = conn.connection;
+	r->mmWidth      = conn.mm_width;
+	r->mmHeight     = conn.mm_height;
+	/* convert subpixel from kernel to userspace */
+	r->subpixel     = conn.subpixel + 1;
+
+	if (mode.clock) {
+		r->count_modes = 1;
+		r->modes       = drmAllocCpy(&mode, 1, sizeof(mode));
+		if (!r->modes)
+			goto err_copy;
+	}
+
+	if (conn.encoder_id) {
+		r->count_encoders = 1;
+		r->encoders       = drmAllocCpy(&enc, 1, sizeof(enc));
+		if (!r->encoders)
+			goto err_copy;
+	}
+
+	r->count_props = conn.count_props;
+	r->props       = U642VOID(conn.props_ptr);
+	r->prop_values = U642VOID(conn.prop_values_ptr);
+
+	r->connector_type  = conn.connector_type;
+	r->connector_type_id = conn.connector_type_id;
+
+	return r;
+
+err_copy:
+	drmFree(r->modes);
+	drmFree(r->encoders);
+	drmFree(r);
+err:
+	drmFree(U642VOID(conn.props_ptr));
+	drmFree(U642VOID(conn.prop_values_ptr));
+	return 0;
+}
+
 int drmModeAttachMode(int fd, uint32_t connector_id, drmModeModeInfoPtr mode_info)
 {
 	struct drm_mode_mode_cmd res;
diff --git a/xf86drmMode.h b/xf86drmMode.h
index 856a6bb..278da48 100644
--- a/xf86drmMode.h
+++ b/xf86drmMode.h
@@ -422,10 +422,23 @@ drmModeEncoderPtr drmModeGetEncoder(int fd, uint32_t encoder_id);
  */
 
 /**
- * Retrive information about the connector connectorId.
+ * Retrieve all information about the connector connectorId. This will do a
+ * forced probe on the connector to retrieve remote information such as EDIDs
+ * from the display device.
  */
 extern drmModeConnectorPtr drmModeGetConnector(int fd,
-		uint32_t connectorId);
+					       uint32_t connectorId);
+
+/**
+ * Retrieve current information, i.e the currently active mode and encoder,
+ * about the connector connectorId. This will not do any probing on the
+ * connector or remote device, and only reports what is currently known.
+ * For the complete set of modes and encoders associated with the connector
+ * use drmModeGetConnector() which will do a probe to determine any display
+ * link changes first.
+ */
+extern drmModeConnectorPtr drmModeGetConnectorCurrent(int fd,
+						      uint32_t connector_id);
 
 /**
  * Attaches the given mode to an connector.
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] mode: Retrieve only the current information for a Connector

[Daniel Vetter]

On Wed, Mar 04, 2015 at 10:38:08AM +0000, Chris Wilson wrote:
> Add a new API that allows the caller to skip any forced probing, which
> may require slow i2c to a remote display, and only report the currently
> active mode and encoder for a Connector. This is often the information
> of interest and is much, much faster than re-retrieving the link status
> and EDIDs, e.g. if the caller only wishes to count the number of active
> outputs.
> 
> v2: Fix error path to avoid double free after a failed GETCONNECTOR
> ioctl.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: David Herrmann <dh.herrmann@googlemail.com>

Context for David: Apparently logind sees fit to probe drm connectors,
right when X starts and not using the cached state. Which needless adds up
to a few hundred ms delay since X then blocks when it does its own cached
getconnector. I've asked Chris to extract the code from the intel ddx to
make this a bit more official and known.

> ---
>  tests/modeprint/modeprint.c | 18 ++++++++-
>  xf86drmMode.c               | 97 +++++++++++++++++++++++++++++++++++++++++++++
>  xf86drmMode.h               | 17 +++++++-
>  3 files changed, 128 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/modeprint/modeprint.c b/tests/modeprint/modeprint.c
> index 6f0d039..514d3ba 100644
> --- a/tests/modeprint/modeprint.c
> +++ b/tests/modeprint/modeprint.c
> @@ -43,6 +43,7 @@
>  
>  #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
>  
> +int current;
>  int connectors;
>  int full_props;
>  int edid;
> @@ -272,7 +273,7 @@ int printRes(int fd, drmModeResPtr res)
>  
>  	if (connectors) {
>  		for (i = 0; i < res->count_connectors; i++) {
> -			connector = drmModeGetConnector(fd, res->connectors[i]);
> +			connector = (current ? drmModeGetConnectorCurrent : drmModeGetConnector) (fd, res->connectors[i]);
>  
>  			if (!connector)
>  				printf("Could not get connector %i\n", res->connectors[i]);
> @@ -331,6 +332,7 @@ int printRes(int fd, drmModeResPtr res)
>  
>  void args(int argc, char **argv)
>  {
> +	int defaults = 1;
>  	int i;
>  
>  	fbs = 0;
> @@ -341,32 +343,41 @@ void args(int argc, char **argv)
>  	full_modes = 0;
>  	full_props = 0;
>  	connectors = 0;
> +	current = 0;
>  
>  	module_name = argv[1];
>  
>  	for (i = 2; i < argc; i++) {
>  		if (strcmp(argv[i], "-fb") == 0) {
>  			fbs = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-crtcs") == 0) {
>  			crtcs = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-cons") == 0) {
>  			connectors = 1;
>  			modes = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-modes") == 0) {
>  			connectors = 1;
>  			modes = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-full") == 0) {
>  			connectors = 1;
>  			modes = 1;
>  			full_modes = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-props") == 0) {
>  			connectors = 1;
>  			full_props = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-edids") == 0) {
>  			connectors = 1;
>  			edid = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-encoders") == 0) {
>  			encoders = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-v") == 0) {
>  			fbs = 1;
>  			edid = 1;
> @@ -376,10 +387,13 @@ void args(int argc, char **argv)
>  			full_modes = 1;
>  			full_props = 1;
>  			connectors = 1;
> +			defaults = 0;
> +		} else if (strcmp(argv[i], "-current") == 0) {
> +			current = 1;
>  		}
>  	}
>  
> -	if (argc == 2) {
> +	if (defaults) {
>  		fbs = 1;
>  		edid = 1;
>  		crtcs = 1;
> diff --git a/xf86drmMode.c b/xf86drmMode.c
> index 9ea8fe7..4433dc7 100644
> --- a/xf86drmMode.c
> +++ b/xf86drmMode.c
> @@ -572,6 +572,103 @@ err_allocs:
>  	return r;
>  }
>  
> +drmModeConnectorPtr drmModeGetConnectorCurrent(int fd, uint32_t connector_id)
> +{
> +	struct drm_mode_modeinfo mode;
> +	struct drm_mode_get_encoder enc;
> +	struct drm_mode_get_connector conn;
> +	unsigned int num_props = 0;
> +	drmModeConnectorPtr r;
> +
> +	memclear(conn);
> +	conn.connector_id = connector_id;
> +	do {
> +		if (conn.count_props) {
> +			drmFree(U642VOID(conn.props_ptr));
> +			conn.props_ptr = VOID2U64(drmMalloc(conn.count_props*sizeof(uint32_t)));
> +			if (!conn.props_ptr)
> +				goto err;
> +
> +			drmFree(U642VOID(conn.prop_values_ptr));
> +			conn.prop_values_ptr = VOID2U64(drmMalloc(conn.count_props*sizeof(uint64_t)));
> +			if (!conn.prop_values_ptr)
> +				goto err;
> +
> +			num_props = conn.count_props;
> +		}
> +
> +		conn.count_modes = 1; /* skip detect */
> +		conn.modes_ptr = (uintptr_t)&mode;
> +		conn.count_encoders = 0;
> +
> +		if (drmIoctl(fd, DRM_IOCTL_MODE_GETCONNECTOR, &conn))
> +			goto err;
> +	} while (conn.count_props != num_props);
> +
> +	/* Retrieve the current mode */
> +	memclear(mode);
> +	memclear(enc);
> +	if (conn.encoder_id) {
> +		struct drm_mode_crtc crtc;
> +
> +		enc.encoder_id = conn.encoder_id;
> +		if (drmIoctl(fd, DRM_IOCTL_MODE_GETENCODER, &enc))
> +			goto err;
> +
> +		memclear(crtc);
> +		crtc.crtc_id = enc.crtc_id;
> +		if (drmIoctl(fd, DRM_IOCTL_MODE_GETCRTC, &crtc))
> +			goto err;
> +
> +		if (crtc.mode_valid)
> +			mode = crtc.mode;
> +	}
> +
> +	if(!(r = drmMalloc(sizeof(*r))))
> +		goto err;
> +
> +	memset(r, 0, sizeof(*r));
> +	r->connector_id = conn.connector_id;
> +	r->encoder_id   = conn.encoder_id;
> +	r->connection   = conn.connection;
> +	r->mmWidth      = conn.mm_width;
> +	r->mmHeight     = conn.mm_height;
> +	/* convert subpixel from kernel to userspace */
> +	r->subpixel     = conn.subpixel + 1;
> +
> +	if (mode.clock) {
> +		r->count_modes = 1;
> +		r->modes       = drmAllocCpy(&mode, 1, sizeof(mode));
> +		if (!r->modes)
> +			goto err_copy;
> +	}
> +
> +	if (conn.encoder_id) {
> +		r->count_encoders = 1;

This only works for i915 where we only ever have 1 encoder. Other drivers
reassign encoders depending upon output type (e.g. dvi-i vs dvi-d). Imo
it'd be cleaner to do something like the below:


diff --git a/xf86drmMode.c b/xf86drmMode.c
index 9ea8fe721842..3a9bb0b6560a 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -477,7 +477,7 @@ drmModeEncoderPtr drmModeGetEncoder(int fd, uint32_t encoder_id)
  * Connector manipulation
  */
 
-drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)
+drmModeConnectorPtr __drmModeGetConnector(int fd, uint32_t connector_id, bool current)
 {
 	struct drm_mode_get_connector conn, counts;
 	drmModeConnectorPtr r = NULL;
@@ -485,6 +485,7 @@ drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)
 retry:
 	memclear(conn);
 	conn.connector_id = connector_id;
+	conn.count_modes = current;
 
 	if (drmIoctl(fd, DRM_IOCTL_MODE_GETCONNECTOR, &conn))
 		return 0;
@@ -572,6 +573,16 @@ err_allocs:
 	return r;
 }
 
+drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)
+{
+	__drmModeGetConnector(fd, connector_id, false);
+}
+
+drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)
+{
+	__drmModeGetConnector(fd, connector_id, true);
+}
+
 int drmModeAttachMode(int fd, uint32_t connector_id, drmModeModeInfoPtr mode_info)
 {
 	struct drm_mode_mode_cmd res;

Totally untested diff ;-)

Cheers, Daniel

> +		r->encoders       = drmAllocCpy(&enc, 1, sizeof(enc));
> +		if (!r->encoders)
> +			goto err_copy;
> +	}
> +
> +	r->count_props = conn.count_props;
> +	r->props       = U642VOID(conn.props_ptr);
> +	r->prop_values = U642VOID(conn.prop_values_ptr);
> +
> +	r->connector_type  = conn.connector_type;
> +	r->connector_type_id = conn.connector_type_id;
> +
> +	return r;
> +
> +err_copy:
> +	drmFree(r->modes);
> +	drmFree(r->encoders);
> +	drmFree(r);
> +err:
> +	drmFree(U642VOID(conn.props_ptr));
> +	drmFree(U642VOID(conn.prop_values_ptr));
> +	return 0;
> +}
> +
>  int drmModeAttachMode(int fd, uint32_t connector_id, drmModeModeInfoPtr mode_info)
>  {
>  	struct drm_mode_mode_cmd res;
> diff --git a/xf86drmMode.h b/xf86drmMode.h
> index 856a6bb..278da48 100644
> --- a/xf86drmMode.h
> +++ b/xf86drmMode.h
> @@ -422,10 +422,23 @@ drmModeEncoderPtr drmModeGetEncoder(int fd, uint32_t encoder_id);
>   */
>  
>  /**
> - * Retrive information about the connector connectorId.
> + * Retrieve all information about the connector connectorId. This will do a
> + * forced probe on the connector to retrieve remote information such as EDIDs
> + * from the display device.
>   */
>  extern drmModeConnectorPtr drmModeGetConnector(int fd,
> -		uint32_t connectorId);
> +					       uint32_t connectorId);
> +
> +/**
> + * Retrieve current information, i.e the currently active mode and encoder,
> + * about the connector connectorId. This will not do any probing on the
> + * connector or remote device, and only reports what is currently known.
> + * For the complete set of modes and encoders associated with the connector
> + * use drmModeGetConnector() which will do a probe to determine any display
> + * link changes first.
> + */
> +extern drmModeConnectorPtr drmModeGetConnectorCurrent(int fd,
> +						      uint32_t connector_id);
>  
>  /**
>   * Attaches the given mode to an connector.
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] mode: Retrieve only the current information for a Connector

[Daniel Stone]

Hi,

On 4 March 2015 at 11:08, Daniel Vetter <daniel@ffwll.ch> wrote:
> +drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)
> +drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)

Testing would probably reveal that you wanted to call one of these
drmModeGetConnectorCurrent. :)

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] mode: Retrieve only the current information for a Connector

[Chris Wilson]

On Wed, Mar 04, 2015 at 12:08:42PM +0100, Daniel Vetter wrote:
> On Wed, Mar 04, 2015 at 10:38:08AM +0000, Chris Wilson wrote:
> > +	if (conn.encoder_id) {
> > +		r->count_encoders = 1;
> 
> This only works for i915 where we only ever have 1 encoder. Other drivers
> reassign encoders depending upon output type (e.g. dvi-i vs dvi-d). Imo
> it'd be cleaner to do something like the below:

Not quite. This value is the currently active encoder_id for the
connector; there can only be one. As opposed to the array of associated
encoders that we normally supply on ouptut. Even i915 has multiple
encoders on connectors nowadays!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] mode: Retrieve only the current information for a Connector

[Daniel Vetter]

On Wed, Mar 4, 2015 at 12:29 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Wed, Mar 04, 2015 at 12:08:42PM +0100, Daniel Vetter wrote:
>> On Wed, Mar 04, 2015 at 10:38:08AM +0000, Chris Wilson wrote:
>> > +   if (conn.encoder_id) {
>> > +           r->count_encoders = 1;
>>
>> This only works for i915 where we only ever have 1 encoder. Other drivers
>> reassign encoders depending upon output type (e.g. dvi-i vs dvi-d). Imo
>> it'd be cleaner to do something like the below:
>
> Not quite. This value is the currently active encoder_id for the
> connector; there can only be one. As opposed to the array of associated
> encoders that we normally supply on ouptut. Even i915 has multiple
> encoders on connectors nowadays!

DRM_MAX_ENCODER in the kernel is 3. And i915 has multipled connectors
for the same encoder, but not the other way round. Having multiple
encoders is the reason for the best_encoders callback in the crtc
helpers, which we've ditched for i915 (we always pick the single
intel_connector->encoder statically assigned at load time).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] mode: Retrieve only the current information for a Connector

[Chris Wilson]

On Wed, Mar 04, 2015 at 01:31:46PM +0100, Daniel Vetter wrote:
> On Wed, Mar 4, 2015 at 12:29 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Wed, Mar 04, 2015 at 12:08:42PM +0100, Daniel Vetter wrote:
> >> On Wed, Mar 04, 2015 at 10:38:08AM +0000, Chris Wilson wrote:
> >> > +   if (conn.encoder_id) {
> >> > +           r->count_encoders = 1;
> >>
> >> This only works for i915 where we only ever have 1 encoder. Other drivers
> >> reassign encoders depending upon output type (e.g. dvi-i vs dvi-d). Imo
> >> it'd be cleaner to do something like the below:
> >
> > Not quite. This value is the currently active encoder_id for the
> > connector; there can only be one. As opposed to the array of associated
> > encoders that we normally supply on ouptut. Even i915 has multiple
> > encoders on connectors nowadays!
> 
> DRM_MAX_ENCODER in the kernel is 3. And i915 has multipled connectors
> for the same encoder, but not the other way round. Having multiple
> encoders is the reason for the best_encoders callback in the crtc
> helpers, which we've ditched for i915 (we always pick the single
> intel_connector->encoder statically assigned at load time).

We have multiple encoders on the same connector, since 3.17.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] mode: Retrieve only the current information for a Connector

[Daniel Vetter]

On Wed, Mar 4, 2015 at 1:34 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Wed, Mar 04, 2015 at 01:31:46PM +0100, Daniel Vetter wrote:
>> On Wed, Mar 4, 2015 at 12:29 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > On Wed, Mar 04, 2015 at 12:08:42PM +0100, Daniel Vetter wrote:
>> >> On Wed, Mar 04, 2015 at 10:38:08AM +0000, Chris Wilson wrote:
>> >> > +   if (conn.encoder_id) {
>> >> > +           r->count_encoders = 1;
>> >>
>> >> This only works for i915 where we only ever have 1 encoder. Other drivers
>> >> reassign encoders depending upon output type (e.g. dvi-i vs dvi-d). Imo
>> >> it'd be cleaner to do something like the below:
>> >
>> > Not quite. This value is the currently active encoder_id for the
>> > connector; there can only be one. As opposed to the array of associated
>> > encoders that we normally supply on ouptut. Even i915 has multiple
>> > encoders on connectors nowadays!
>>
>> DRM_MAX_ENCODER in the kernel is 3. And i915 has multipled connectors
>> for the same encoder, but not the other way round. Having multiple
>> encoders is the reason for the best_encoders callback in the crtc
>> helpers, which we've ditched for i915 (we always pick the single
>> intel_connector->encoder statically assigned at load time).
>
> We have multiple encoders on the same connector, since 3.17.

Ok short summary of our irc discussion: I've totally missed the fake
encoders from dp mst, i915 indeed has multiple possible encoders per
output now too.

Wrt the api discussion itself I didn't realize that Chris' code digs
out the entire current configuration (including mode and all that) and
intentionally filters out all the other information supplied by the
kernel (like the mode list or list of possible encoders). Imo that's
too high-level and artificially limits what drivers could do with
this.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3] mode: Retrieve only the current information for a Connector

[Chris Wilson]

Add a new API that allows the caller to skip any forced probing, which
may require slow i2c to a remote display, and only report the currently
active mode and encoder for a Connector. This is often the information
of interest and is much, much faster than re-retrieving the link status
and EDIDs, e.g. if the caller only wishes to count the number of active
outputs.

v2: Fix error path to avoid double free after a failed GETCONNECTOR
ioctl.

v3: Daniel strongly disapproved of my disjoint in behaviour between
GetConnector and GetConnectorCurrent, and considering how best to make a
drop in replacement for drmmode_output_init() convinced me keeping the
API as consistent as possible was the right approach.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: David Herrmann <dh.herrmann@googlemail.com>
---
 tests/modeprint/modeprint.c | 18 ++++++++++++++++--
 xf86drmMode.c               | 20 +++++++++++++++++---
 xf86drmMode.h               | 17 +++++++++++++++--
 3 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/tests/modeprint/modeprint.c b/tests/modeprint/modeprint.c
index 6f0d039..514d3ba 100644
--- a/tests/modeprint/modeprint.c
+++ b/tests/modeprint/modeprint.c
@@ -43,6 +43,7 @@
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
 
+int current;
 int connectors;
 int full_props;
 int edid;
@@ -272,7 +273,7 @@ int printRes(int fd, drmModeResPtr res)
 
 	if (connectors) {
 		for (i = 0; i < res->count_connectors; i++) {
-			connector = drmModeGetConnector(fd, res->connectors[i]);
+			connector = (current ? drmModeGetConnectorCurrent : drmModeGetConnector) (fd, res->connectors[i]);
 
 			if (!connector)
 				printf("Could not get connector %i\n", res->connectors[i]);
@@ -331,6 +332,7 @@ int printRes(int fd, drmModeResPtr res)
 
 void args(int argc, char **argv)
 {
+	int defaults = 1;
 	int i;
 
 	fbs = 0;
@@ -341,32 +343,41 @@ void args(int argc, char **argv)
 	full_modes = 0;
 	full_props = 0;
 	connectors = 0;
+	current = 0;
 
 	module_name = argv[1];
 
 	for (i = 2; i < argc; i++) {
 		if (strcmp(argv[i], "-fb") == 0) {
 			fbs = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-crtcs") == 0) {
 			crtcs = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-cons") == 0) {
 			connectors = 1;
 			modes = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-modes") == 0) {
 			connectors = 1;
 			modes = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-full") == 0) {
 			connectors = 1;
 			modes = 1;
 			full_modes = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-props") == 0) {
 			connectors = 1;
 			full_props = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-edids") == 0) {
 			connectors = 1;
 			edid = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-encoders") == 0) {
 			encoders = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-v") == 0) {
 			fbs = 1;
 			edid = 1;
@@ -376,10 +387,13 @@ void args(int argc, char **argv)
 			full_modes = 1;
 			full_props = 1;
 			connectors = 1;
+			defaults = 0;
+		} else if (strcmp(argv[i], "-current") == 0) {
+			current = 1;
 		}
 	}
 
-	if (argc == 2) {
+	if (defaults) {
 		fbs = 1;
 		edid = 1;
 		crtcs = 1;
diff --git a/xf86drmMode.c b/xf86drmMode.c
index 9ea8fe7..138f1bf 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -476,19 +476,23 @@ drmModeEncoderPtr drmModeGetEncoder(int fd, uint32_t encoder_id)
 /*
  * Connector manipulation
  */
-
-drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)
+static drmModeConnectorPtr
+_drmModeGetConnector(int fd, uint32_t connector_id, int probe)
 {
 	struct drm_mode_get_connector conn, counts;
 	drmModeConnectorPtr r = NULL;
 
-retry:
 	memclear(conn);
 	conn.connector_id = connector_id;
+	if (!probe) {
+		conn.count_modes = 1;
+		conn.modes_ptr = VOID2U64(drmMalloc(sizeof(struct drm_mode_modeinfo)));
+	}
 
 	if (drmIoctl(fd, DRM_IOCTL_MODE_GETCONNECTOR, &conn))
 		return 0;
 
+retry:
 	counts = conn;
 
 	if (conn.count_props) {
@@ -572,6 +576,16 @@ err_allocs:
 	return r;
 }
 
+drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)
+{
+	return _drmModeGetConnector(fd, connector_id, 1);
+}
+
+drmModeConnectorPtr drmModeGetConnectorCurrent(int fd, uint32_t connector_id)
+{
+	return _drmModeGetConnector(fd, connector_id, 0);
+}
+
 int drmModeAttachMode(int fd, uint32_t connector_id, drmModeModeInfoPtr mode_info)
 {
 	struct drm_mode_mode_cmd res;
diff --git a/xf86drmMode.h b/xf86drmMode.h
index 856a6bb..278da48 100644
--- a/xf86drmMode.h
+++ b/xf86drmMode.h
@@ -422,10 +422,23 @@ drmModeEncoderPtr drmModeGetEncoder(int fd, uint32_t encoder_id);
  */
 
 /**
- * Retrive information about the connector connectorId.
+ * Retrieve all information about the connector connectorId. This will do a
+ * forced probe on the connector to retrieve remote information such as EDIDs
+ * from the display device.
  */
 extern drmModeConnectorPtr drmModeGetConnector(int fd,
-		uint32_t connectorId);
+					       uint32_t connectorId);
+
+/**
+ * Retrieve current information, i.e the currently active mode and encoder,
+ * about the connector connectorId. This will not do any probing on the
+ * connector or remote device, and only reports what is currently known.
+ * For the complete set of modes and encoders associated with the connector
+ * use drmModeGetConnector() which will do a probe to determine any display
+ * link changes first.
+ */
+extern drmModeConnectorPtr drmModeGetConnectorCurrent(int fd,
+						      uint32_t connector_id);
 
 /**
  * Attaches the given mode to an connector.
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4] mode: Retrieve only the current information for a Connector

[Chris Wilson]

Add a new API that allows the caller to skip any forced probing, which
may require slow i2c to a remote display, and only report the currently
active mode and encoder for a Connector. This is often the information
of interest and is much, much faster than re-retrieving the link status
and EDIDs, e.g. if the caller only wishes to count the number of active
outputs.

v2: Fix error path to avoid double free after a failed GETCONNECTOR
ioctl.

v3: Daniel strongly disapproved of my disjoint in behaviour between
GetConnector and GetConnectorCurrent, and considering how best to make a
drop in replacement for drmmode_output_init() convinced me keeping the
API as consistent as possible was the right approach.

v4: Avoid probing on the second calls to GETCONNECTOR for unconnected
outputs.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: David Herrmann <dh.herrmann@googlemail.com>
---
 tests/modeprint/modeprint.c | 18 ++++++++++++++++--
 xf86drmMode.c               | 23 ++++++++++++++++++++---
 xf86drmMode.h               | 17 +++++++++++++++--
 3 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/tests/modeprint/modeprint.c b/tests/modeprint/modeprint.c
index 6f0d039..514d3ba 100644
--- a/tests/modeprint/modeprint.c
+++ b/tests/modeprint/modeprint.c
@@ -43,6 +43,7 @@
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
 
+int current;
 int connectors;
 int full_props;
 int edid;
@@ -272,7 +273,7 @@ int printRes(int fd, drmModeResPtr res)
 
 	if (connectors) {
 		for (i = 0; i < res->count_connectors; i++) {
-			connector = drmModeGetConnector(fd, res->connectors[i]);
+			connector = (current ? drmModeGetConnectorCurrent : drmModeGetConnector) (fd, res->connectors[i]);
 
 			if (!connector)
 				printf("Could not get connector %i\n", res->connectors[i]);
@@ -331,6 +332,7 @@ int printRes(int fd, drmModeResPtr res)
 
 void args(int argc, char **argv)
 {
+	int defaults = 1;
 	int i;
 
 	fbs = 0;
@@ -341,32 +343,41 @@ void args(int argc, char **argv)
 	full_modes = 0;
 	full_props = 0;
 	connectors = 0;
+	current = 0;
 
 	module_name = argv[1];
 
 	for (i = 2; i < argc; i++) {
 		if (strcmp(argv[i], "-fb") == 0) {
 			fbs = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-crtcs") == 0) {
 			crtcs = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-cons") == 0) {
 			connectors = 1;
 			modes = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-modes") == 0) {
 			connectors = 1;
 			modes = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-full") == 0) {
 			connectors = 1;
 			modes = 1;
 			full_modes = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-props") == 0) {
 			connectors = 1;
 			full_props = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-edids") == 0) {
 			connectors = 1;
 			edid = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-encoders") == 0) {
 			encoders = 1;
+			defaults = 0;
 		} else if (strcmp(argv[i], "-v") == 0) {
 			fbs = 1;
 			edid = 1;
@@ -376,10 +387,13 @@ void args(int argc, char **argv)
 			full_modes = 1;
 			full_props = 1;
 			connectors = 1;
+			defaults = 0;
+		} else if (strcmp(argv[i], "-current") == 0) {
+			current = 1;
 		}
 	}
 
-	if (argc == 2) {
+	if (defaults) {
 		fbs = 1;
 		edid = 1;
 		crtcs = 1;
diff --git a/xf86drmMode.c b/xf86drmMode.c
index 9ea8fe7..57d1dc9 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -476,19 +476,23 @@ drmModeEncoderPtr drmModeGetEncoder(int fd, uint32_t encoder_id)
 /*
  * Connector manipulation
  */
-
-drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)
+static drmModeConnectorPtr
+_drmModeGetConnector(int fd, uint32_t connector_id, int probe)
 {
 	struct drm_mode_get_connector conn, counts;
 	drmModeConnectorPtr r = NULL;
 
-retry:
 	memclear(conn);
 	conn.connector_id = connector_id;
+	if (!probe) {
+		conn.count_modes = 1;
+		conn.modes_ptr = VOID2U64(drmMalloc(sizeof(struct drm_mode_modeinfo)));
+	}
 
 	if (drmIoctl(fd, DRM_IOCTL_MODE_GETCONNECTOR, &conn))
 		return 0;
 
+retry:
 	counts = conn;
 
 	if (conn.count_props) {
@@ -504,6 +508,9 @@ retry:
 		conn.modes_ptr = VOID2U64(drmMalloc(conn.count_modes*sizeof(struct drm_mode_modeinfo)));
 		if (!conn.modes_ptr)
 			goto err_allocs;
+	} else {
+		conn.count_modes = 1;
+		conn.modes_ptr = VOID2U64(drmMalloc(sizeof(struct drm_mode_modeinfo)));
 	}
 
 	if (conn.count_encoders) {
@@ -572,6 +579,16 @@ err_allocs:
 	return r;
 }
 
+drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)
+{
+	return _drmModeGetConnector(fd, connector_id, 1);
+}
+
+drmModeConnectorPtr drmModeGetConnectorCurrent(int fd, uint32_t connector_id)
+{
+	return _drmModeGetConnector(fd, connector_id, 0);
+}
+
 int drmModeAttachMode(int fd, uint32_t connector_id, drmModeModeInfoPtr mode_info)
 {
 	struct drm_mode_mode_cmd res;
diff --git a/xf86drmMode.h b/xf86drmMode.h
index 856a6bb..278da48 100644
--- a/xf86drmMode.h
+++ b/xf86drmMode.h
@@ -422,10 +422,23 @@ drmModeEncoderPtr drmModeGetEncoder(int fd, uint32_t encoder_id);
  */
 
 /**
- * Retrive information about the connector connectorId.
+ * Retrieve all information about the connector connectorId. This will do a
+ * forced probe on the connector to retrieve remote information such as EDIDs
+ * from the display device.
  */
 extern drmModeConnectorPtr drmModeGetConnector(int fd,
-		uint32_t connectorId);
+					       uint32_t connectorId);
+
+/**
+ * Retrieve current information, i.e the currently active mode and encoder,
+ * about the connector connectorId. This will not do any probing on the
+ * connector or remote device, and only reports what is currently known.
+ * For the complete set of modes and encoders associated with the connector
+ * use drmModeGetConnector() which will do a probe to determine any display
+ * link changes first.
+ */
+extern drmModeConnectorPtr drmModeGetConnectorCurrent(int fd,
+						      uint32_t connector_id);
 
 /**
  * Attaches the given mode to an connector.
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4] mode: Retrieve only the current information for a Connector

[Daniel Vetter]

On Wed, Mar 04, 2015 at 01:16:20PM +0000, Chris Wilson wrote:
> Add a new API that allows the caller to skip any forced probing, which
> may require slow i2c to a remote display, and only report the currently
> active mode and encoder for a Connector. This is often the information
> of interest and is much, much faster than re-retrieving the link status
> and EDIDs, e.g. if the caller only wishes to count the number of active
> outputs.
> 
> v2: Fix error path to avoid double free after a failed GETCONNECTOR
> ioctl.
> 
> v3: Daniel strongly disapproved of my disjoint in behaviour between
> GetConnector and GetConnectorCurrent, and considering how best to make a
> drop in replacement for drmmode_output_init() convinced me keeping the
> API as consistent as possible was the right approach.
> 
> v4: Avoid probing on the second calls to GETCONNECTOR for unconnected
> outputs.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: David Herrmann <dh.herrmann@googlemail.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  tests/modeprint/modeprint.c | 18 ++++++++++++++++--
>  xf86drmMode.c               | 23 ++++++++++++++++++++---
>  xf86drmMode.h               | 17 +++++++++++++++--
>  3 files changed, 51 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/modeprint/modeprint.c b/tests/modeprint/modeprint.c
> index 6f0d039..514d3ba 100644
> --- a/tests/modeprint/modeprint.c
> +++ b/tests/modeprint/modeprint.c
> @@ -43,6 +43,7 @@
>  
>  #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
>  
> +int current;
>  int connectors;
>  int full_props;
>  int edid;
> @@ -272,7 +273,7 @@ int printRes(int fd, drmModeResPtr res)
>  
>  	if (connectors) {
>  		for (i = 0; i < res->count_connectors; i++) {
> -			connector = drmModeGetConnector(fd, res->connectors[i]);
> +			connector = (current ? drmModeGetConnectorCurrent : drmModeGetConnector) (fd, res->connectors[i]);
>  
>  			if (!connector)
>  				printf("Could not get connector %i\n", res->connectors[i]);
> @@ -331,6 +332,7 @@ int printRes(int fd, drmModeResPtr res)
>  
>  void args(int argc, char **argv)
>  {
> +	int defaults = 1;
>  	int i;
>  
>  	fbs = 0;
> @@ -341,32 +343,41 @@ void args(int argc, char **argv)
>  	full_modes = 0;
>  	full_props = 0;
>  	connectors = 0;
> +	current = 0;
>  
>  	module_name = argv[1];
>  
>  	for (i = 2; i < argc; i++) {
>  		if (strcmp(argv[i], "-fb") == 0) {
>  			fbs = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-crtcs") == 0) {
>  			crtcs = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-cons") == 0) {
>  			connectors = 1;
>  			modes = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-modes") == 0) {
>  			connectors = 1;
>  			modes = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-full") == 0) {
>  			connectors = 1;
>  			modes = 1;
>  			full_modes = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-props") == 0) {
>  			connectors = 1;
>  			full_props = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-edids") == 0) {
>  			connectors = 1;
>  			edid = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-encoders") == 0) {
>  			encoders = 1;
> +			defaults = 0;
>  		} else if (strcmp(argv[i], "-v") == 0) {
>  			fbs = 1;
>  			edid = 1;
> @@ -376,10 +387,13 @@ void args(int argc, char **argv)
>  			full_modes = 1;
>  			full_props = 1;
>  			connectors = 1;
> +			defaults = 0;
> +		} else if (strcmp(argv[i], "-current") == 0) {
> +			current = 1;
>  		}
>  	}
>  
> -	if (argc == 2) {
> +	if (defaults) {
>  		fbs = 1;
>  		edid = 1;
>  		crtcs = 1;
> diff --git a/xf86drmMode.c b/xf86drmMode.c
> index 9ea8fe7..57d1dc9 100644
> --- a/xf86drmMode.c
> +++ b/xf86drmMode.c
> @@ -476,19 +476,23 @@ drmModeEncoderPtr drmModeGetEncoder(int fd, uint32_t encoder_id)
>  /*
>   * Connector manipulation
>   */
> -
> -drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)
> +static drmModeConnectorPtr
> +_drmModeGetConnector(int fd, uint32_t connector_id, int probe)
>  {
>  	struct drm_mode_get_connector conn, counts;
>  	drmModeConnectorPtr r = NULL;
>  
> -retry:
>  	memclear(conn);
>  	conn.connector_id = connector_id;
> +	if (!probe) {
> +		conn.count_modes = 1;
> +		conn.modes_ptr = VOID2U64(drmMalloc(sizeof(struct drm_mode_modeinfo)));
> +	}
>  
>  	if (drmIoctl(fd, DRM_IOCTL_MODE_GETCONNECTOR, &conn))
>  		return 0;
>  
> +retry:
>  	counts = conn;
>  
>  	if (conn.count_props) {
> @@ -504,6 +508,9 @@ retry:
>  		conn.modes_ptr = VOID2U64(drmMalloc(conn.count_modes*sizeof(struct drm_mode_modeinfo)));
>  		if (!conn.modes_ptr)
>  			goto err_allocs;
> +	} else {
> +		conn.count_modes = 1;
> +		conn.modes_ptr = VOID2U64(drmMalloc(sizeof(struct drm_mode_modeinfo)));
>  	}
>  
>  	if (conn.count_encoders) {
> @@ -572,6 +579,16 @@ err_allocs:
>  	return r;
>  }
>  
> +drmModeConnectorPtr drmModeGetConnector(int fd, uint32_t connector_id)
> +{
> +	return _drmModeGetConnector(fd, connector_id, 1);
> +}
> +
> +drmModeConnectorPtr drmModeGetConnectorCurrent(int fd, uint32_t connector_id)
> +{
> +	return _drmModeGetConnector(fd, connector_id, 0);
> +}
> +
>  int drmModeAttachMode(int fd, uint32_t connector_id, drmModeModeInfoPtr mode_info)
>  {
>  	struct drm_mode_mode_cmd res;
> diff --git a/xf86drmMode.h b/xf86drmMode.h
> index 856a6bb..278da48 100644
> --- a/xf86drmMode.h
> +++ b/xf86drmMode.h
> @@ -422,10 +422,23 @@ drmModeEncoderPtr drmModeGetEncoder(int fd, uint32_t encoder_id);
>   */
>  
>  /**
> - * Retrive information about the connector connectorId.
> + * Retrieve all information about the connector connectorId. This will do a
> + * forced probe on the connector to retrieve remote information such as EDIDs
> + * from the display device.
>   */
>  extern drmModeConnectorPtr drmModeGetConnector(int fd,
> -		uint32_t connectorId);
> +					       uint32_t connectorId);
> +
> +/**
> + * Retrieve current information, i.e the currently active mode and encoder,
> + * about the connector connectorId. This will not do any probing on the
> + * connector or remote device, and only reports what is currently known.
> + * For the complete set of modes and encoders associated with the connector
> + * use drmModeGetConnector() which will do a probe to determine any display
> + * link changes first.
> + */
> +extern drmModeConnectorPtr drmModeGetConnectorCurrent(int fd,
> +						      uint32_t connector_id);
>  
>  /**
>   * Attaches the given mode to an connector.
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel