public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH igt 1/2] lib: Add simple sysfs accessors
@ 2016-05-26 14:14 Chris Wilson
  2016-05-26 14:14 ` [PATCH igt 2/2] lib: Override the connector status using the sysfs status attribute Chris Wilson
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Chris Wilson @ 2016-05-26 14:14 UTC (permalink / raw)
  To: intel-gfx

igt_sysfs_set() for setting an attribute via sysfs, igt_sysfs_get() for
reading.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/Makefile.sources |   2 +
 lib/igt_sysfs.c      | 145 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_sysfs.h      |  33 ++++++++++++
 3 files changed, 180 insertions(+)
 create mode 100644 lib/igt_sysfs.c
 create mode 100644 lib/igt_sysfs.h

diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 1316fd2..f50ff4d 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -16,6 +16,8 @@ libintel_tools_la_SOURCES = 	\
 	igt_gt.h		\
 	igt_stats.c		\
 	igt_stats.h		\
+	igt_sysfs.c		\
+	igt_sysfs.h		\
 	instdone.c		\
 	instdone.h		\
 	intel_batchbuffer.c	\
diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
new file mode 100644
index 0000000..4dd6089
--- /dev/null
+++ b/lib/igt_sysfs.c
@@ -0,0 +1,145 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <inttypes.h>
+#include <sys/stat.h>
+#include <sys/mount.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <limits.h>
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <i915_drm.h>
+
+#include "igt_sysfs.h"
+
+typedef struct {
+	int dir;
+} igt_sysfs_t;
+
+static int __igt_sysfs_init(void)
+{
+	char path[1024];
+	struct stat st;
+
+	for (int n = 0; n < 16; n++) {
+		int len = sprintf(path, "/sys/class/drm/card%d", n);
+		sprintf(path + len, "/error");
+		if (stat(path, &st) == 0) {
+			path[len] = '\0';
+			return open(path, O_RDONLY);
+		}
+	}
+
+	return -1;
+}
+
+static int __igt_sysfs_singleton(void)
+{
+	static int sysfs = -1;
+
+	if (sysfs == -1)
+		sysfs = __igt_sysfs_init();
+
+	return sysfs;
+}
+
+/**
+ * igt_sysfs_set:
+ * @filename: name of the sysfs node to open
+ * @value: the string to write
+ *
+ * This writes the value to the sysfs file.
+ *
+ * Returns:
+ * True on success, false on failure.
+ */
+bool igt_sysfs_set(const char *filename, const char *value)
+{
+	int sysfs = __igt_sysfs_singleton();
+	int fd, len, ret;
+
+	if (sysfs < 0)
+		return false;
+
+	fd = openat(sysfs, filename, O_WRONLY);
+	if (fd < 0)
+		return false;
+
+	len = strlen(value);
+	ret = write(fd, value, len);
+	close(fd);
+
+	return len == ret;
+}
+
+/**
+ * igt_sysfs_get:
+ * @filename: name of the sysfs node to open
+ *
+ * This reads the value from the sysfs file.
+ *
+ * Returns:
+ * A nul-terminated string, must be freed by caller after use, or NULL
+ * on failure.
+ */
+char *igt_sysfs_get(const char *filename)
+{
+	int sysfs = __igt_sysfs_singleton();
+	char *buf;
+	int len, offset, rem;
+	int ret, fd;
+
+	if (sysfs < 0)
+		return NULL;
+
+	fd = openat(sysfs, filename, O_WRONLY);
+	if (fd < 0)
+		return NULL;
+
+	offset = 0;
+	len = 64;
+	rem =  len - offset - 1;
+	buf = malloc(len);
+	if (!buf)
+		return NULL;
+
+	while ((ret = read(fd, buf + offset, rem)) == rem) {
+		char *newbuf;
+
+		len *= 2;
+		offset += ret;
+
+		newbuf = realloc(buf, len);
+		if (!newbuf)
+			break;
+
+		rem = len - offset - 1;
+	}
+
+	buf[offset] = '0';
+	return buf;
+}
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
new file mode 100644
index 0000000..48c218d
--- /dev/null
+++ b/lib/igt_sysfs.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __IGT_SYSFS_H__
+#define __IGT_SYSFS_H__
+
+#include <stdbool.h>
+
+bool igt_sysfs_set(const char *filename, const char *value);
+char *igt_sysfs_get(const char *filename);
+
+#endif /* __IGT_SYSFS_H__ */
-- 
2.8.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH igt 2/2] lib: Override the connector status using the sysfs status attribute
  2016-05-26 14:14 [PATCH igt 1/2] lib: Add simple sysfs accessors Chris Wilson
@ 2016-05-26 14:14 ` Chris Wilson
  2016-05-26 14:39   ` Ville Syrjälä
  2016-05-26 14:36 ` [PATCH igt 1/2] lib: Add simple sysfs accessors Ville Syrjälä
  2016-05-26 15:07 ` [PATCH igt v2 " Chris Wilson
  2 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2016-05-26 14:14 UTC (permalink / raw)
  To: intel-gfx

There are two paths to force enable a connector, via debugfs and via
sysfs. sysfs has the advantage of being a stable interface and of
updating the connector after application (allowing us to not force a
reprobe from userspace).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_kms.c | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 4da645a..5678248 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -48,6 +48,7 @@
 #include "igt_aux.h"
 #include "intel_chipset.h"
 #include "igt_debugfs.h"
+#include "igt_sysfs.h"
 
 /* list of connectors that need resetting on exit */
 #define MAX_CONNECTORS 32
@@ -596,9 +597,9 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
 {
 	char *path, **tmp;
 	const char *value;
-	int debugfs_fd, ret, len;
 	drmModeConnector *temp;
 	uint32_t devid;
+	int len;
 
 	devid = intel_get_drm_devid(drm_fd);
 
@@ -615,7 +616,7 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
 		value = "on";
 		break;
 	case FORCE_CONNECTOR_DIGITAL:
-		value = "digital";
+		value = "on-digital";
 		break;
 	case FORCE_CONNECTOR_OFF:
 		value = "off";
@@ -623,20 +624,14 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
 
 	default:
 	case FORCE_CONNECTOR_UNSPECIFIED:
-		value = "unspecified";
+		value = "detect";
 		break;
 	}
 
-	igt_assert_neq(asprintf(&path, "%s-%d/force", kmstest_connector_type_str(connector->connector_type), connector->connector_type_id),
+	igt_assert_neq(asprintf(&path, "%s-%d/status", kmstest_connector_type_str(connector->connector_type), connector->connector_type_id),
 		       -1);
-	debugfs_fd = igt_debugfs_open(path, O_WRONLY | O_TRUNC);
-
-	if (debugfs_fd == -1) {
+	if (!igt_sysfs_set(path, value))
 		return false;
-	}
-
-	ret = write(debugfs_fd, value, strlen(value));
-	close(debugfs_fd);
 
 	for (len = 0, tmp = forced_connectors; *tmp; tmp++) {
 		/* check the connector is not already present */
@@ -669,8 +664,7 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
 	temp = drmModeGetConnector(drm_fd, connector->connector_id);
 	drmModeFreeConnector(temp);
 
-	igt_assert(ret != -1);
-	return (ret == -1) ? false : true;
+	return true;
 }
 
 /**
@@ -2548,9 +2542,6 @@ void igt_reset_connectors(void)
 	/* reset the connectors stored in forced_connectors, avoiding any
 	 * functions that are not safe to call in signal handlers */
 
-	for (tmp = forced_connectors; *tmp; tmp++) {
-		int fd = igt_debugfs_open(*tmp, O_WRONLY | O_TRUNC);
-		igt_assert(write(fd, "unspecified", 11) == 11);
-		close(fd);
-	}
+	for (tmp = forced_connectors; *tmp; tmp++)
+		igt_sysfs_set(*tmp, "detect");
 }
-- 
2.8.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH igt 1/2] lib: Add simple sysfs accessors
  2016-05-26 14:14 [PATCH igt 1/2] lib: Add simple sysfs accessors Chris Wilson
  2016-05-26 14:14 ` [PATCH igt 2/2] lib: Override the connector status using the sysfs status attribute Chris Wilson
@ 2016-05-26 14:36 ` Ville Syrjälä
  2016-05-26 14:50   ` Chris Wilson
  2016-05-26 15:07 ` [PATCH igt v2 " Chris Wilson
  2 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2016-05-26 14:36 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Thu, May 26, 2016 at 03:14:29PM +0100, Chris Wilson wrote:
> igt_sysfs_set() for setting an attribute via sysfs, igt_sysfs_get() for
> reading.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  lib/Makefile.sources |   2 +
>  lib/igt_sysfs.c      | 145 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_sysfs.h      |  33 ++++++++++++
>  3 files changed, 180 insertions(+)
>  create mode 100644 lib/igt_sysfs.c
>  create mode 100644 lib/igt_sysfs.h
> 
> diff --git a/lib/Makefile.sources b/lib/Makefile.sources
> index 1316fd2..f50ff4d 100644
> --- a/lib/Makefile.sources
> +++ b/lib/Makefile.sources
> @@ -16,6 +16,8 @@ libintel_tools_la_SOURCES = 	\
>  	igt_gt.h		\
>  	igt_stats.c		\
>  	igt_stats.h		\
> +	igt_sysfs.c		\
> +	igt_sysfs.h		\
>  	instdone.c		\
>  	instdone.h		\
>  	intel_batchbuffer.c	\
> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> new file mode 100644
> index 0000000..4dd6089
> --- /dev/null
> +++ b/lib/igt_sysfs.c
> @@ -0,0 +1,145 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include <inttypes.h>
> +#include <sys/stat.h>
> +#include <sys/mount.h>
> +#include <errno.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <limits.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <i915_drm.h>
> +
> +#include "igt_sysfs.h"
> +
> +typedef struct {
> +	int dir;
> +} igt_sysfs_t;
> +
> +static int __igt_sysfs_init(void)
> +{
> +	char path[1024];
> +	struct stat st;


> +
> +	for (int n = 0; n < 16; n++) {
> +		int len = sprintf(path, "/sys/class/drm/card%d", n);
> +		sprintf(path + len, "/error");

That's goint to limit this to i915. I was thinking we could pass
the drm_fd here to find the correct minor for it.

Maybe drmGetDeviceNameFromFd() is the right thing for that?

> +		if (stat(path, &st) == 0) {
> +			path[len] = '\0';
> +			return open(path, O_RDONLY);
> +		}
> +	}
> +
> +	return -1;
> +}
> +
> +static int __igt_sysfs_singleton(void)
> +{
> +	static int sysfs = -1;
> +
> +	if (sysfs == -1)
> +		sysfs = __igt_sysfs_init();
> +
> +	return sysfs;
> +}
> +
> +/**
> + * igt_sysfs_set:
> + * @filename: name of the sysfs node to open
> + * @value: the string to write
> + *
> + * This writes the value to the sysfs file.
> + *
> + * Returns:
> + * True on success, false on failure.
> + */
> +bool igt_sysfs_set(const char *filename, const char *value)
> +{
> +	int sysfs = __igt_sysfs_singleton();
> +	int fd, len, ret;
> +
> +	if (sysfs < 0)
> +		return false;
> +
> +	fd = openat(sysfs, filename, O_WRONLY);
> +	if (fd < 0)
> +		return false;
> +
> +	len = strlen(value);
> +	ret = write(fd, value, len);

EINTR?

> +	close(fd);
> +
> +	return len == ret;
> +}
> +
> +/**
> + * igt_sysfs_get:
> + * @filename: name of the sysfs node to open
> + *
> + * This reads the value from the sysfs file.
> + *
> + * Returns:
> + * A nul-terminated string, must be freed by caller after use, or NULL
> + * on failure.
> + */
> +char *igt_sysfs_get(const char *filename)
> +{
> +	int sysfs = __igt_sysfs_singleton();
> +	char *buf;
> +	int len, offset, rem;
> +	int ret, fd;
> +
> +	if (sysfs < 0)
> +		return NULL;
> +
> +	fd = openat(sysfs, filename, O_WRONLY);
> +	if (fd < 0)
> +		return NULL;
> +
> +	offset = 0;
> +	len = 64;
> +	rem =  len - offset - 1;
> +	buf = malloc(len);
> +	if (!buf)
> +		return NULL;
> +
> +	while ((ret = read(fd, buf + offset, rem)) == rem) {

EINTR?

> +		char *newbuf;
> +
> +		len *= 2;
> +		offset += ret;
> +
> +		newbuf = realloc(buf, len);
> +		if (!newbuf)
> +			break;
> +
> +		rem = len - offset - 1;
> +	}
> +
> +	buf[offset] = '0';
> +	return buf;
> +}
> diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
> new file mode 100644
> index 0000000..48c218d
> --- /dev/null
> +++ b/lib/igt_sysfs.h
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef __IGT_SYSFS_H__
> +#define __IGT_SYSFS_H__
> +
> +#include <stdbool.h>
> +
> +bool igt_sysfs_set(const char *filename, const char *value);
> +char *igt_sysfs_get(const char *filename);
> +
> +#endif /* __IGT_SYSFS_H__ */
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH igt 2/2] lib: Override the connector status using the sysfs status attribute
  2016-05-26 14:14 ` [PATCH igt 2/2] lib: Override the connector status using the sysfs status attribute Chris Wilson
@ 2016-05-26 14:39   ` Ville Syrjälä
  2016-05-26 14:55     ` Chris Wilson
  0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2016-05-26 14:39 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Thu, May 26, 2016 at 03:14:30PM +0100, Chris Wilson wrote:
> There are two paths to force enable a connector, via debugfs and via
> sysfs. sysfs has the advantage of being a stable interface and of
> updating the connector after application (allowing us to not force a
> reprobe from userspace).
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/igt_kms.c | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 4da645a..5678248 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -48,6 +48,7 @@
>  #include "igt_aux.h"
>  #include "intel_chipset.h"
>  #include "igt_debugfs.h"
> +#include "igt_sysfs.h"
>  
>  /* list of connectors that need resetting on exit */
>  #define MAX_CONNECTORS 32
> @@ -596,9 +597,9 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
>  {
>  	char *path, **tmp;
>  	const char *value;
> -	int debugfs_fd, ret, len;
>  	drmModeConnector *temp;
>  	uint32_t devid;
> +	int len;
>  
>  	devid = intel_get_drm_devid(drm_fd);
>  
> @@ -615,7 +616,7 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
>  		value = "on";
>  		break;
>  	case FORCE_CONNECTOR_DIGITAL:
> -		value = "digital";
> +		value = "on-digital";
>  		break;
>  	case FORCE_CONNECTOR_OFF:
>  		value = "off";
> @@ -623,20 +624,14 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
>  
>  	default:
>  	case FORCE_CONNECTOR_UNSPECIFIED:
> -		value = "unspecified";
> +		value = "detect";
>  		break;
>  	}
>  
> -	igt_assert_neq(asprintf(&path, "%s-%d/force", kmstest_connector_type_str(connector->connector_type), connector->connector_type_id),
> +	igt_assert_neq(asprintf(&path, "%s-%d/status", kmstest_connector_type_str(connector->connector_type), connector->connector_type_id),

That'll need a cardN- prefix. Rather annoying.

>  		       -1);
> -	debugfs_fd = igt_debugfs_open(path, O_WRONLY | O_TRUNC);
> -
> -	if (debugfs_fd == -1) {
> +	if (!igt_sysfs_set(path, value))
>  		return false;
> -	}
> -
> -	ret = write(debugfs_fd, value, strlen(value));
> -	close(debugfs_fd);
>  
>  	for (len = 0, tmp = forced_connectors; *tmp; tmp++) {
>  		/* check the connector is not already present */
> @@ -669,8 +664,7 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
>  	temp = drmModeGetConnector(drm_fd, connector->connector_id);
>  	drmModeFreeConnector(temp);
>  
> -	igt_assert(ret != -1);
> -	return (ret == -1) ? false : true;
> +	return true;
>  }
>  
>  /**
> @@ -2548,9 +2542,6 @@ void igt_reset_connectors(void)
>  	/* reset the connectors stored in forced_connectors, avoiding any
>  	 * functions that are not safe to call in signal handlers */
>  
> -	for (tmp = forced_connectors; *tmp; tmp++) {
> -		int fd = igt_debugfs_open(*tmp, O_WRONLY | O_TRUNC);
> -		igt_assert(write(fd, "unspecified", 11) == 11);
> -		close(fd);
> -	}
> +	for (tmp = forced_connectors; *tmp; tmp++)
> +		igt_sysfs_set(*tmp, "detect");
>  }
> -- 
> 2.8.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH igt 1/2] lib: Add simple sysfs accessors
  2016-05-26 14:36 ` [PATCH igt 1/2] lib: Add simple sysfs accessors Ville Syrjälä
@ 2016-05-26 14:50   ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2016-05-26 14:50 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Thu, May 26, 2016 at 05:36:09PM +0300, Ville Syrjälä wrote:
> > +
> > +	for (int n = 0; n < 16; n++) {
> > +		int len = sprintf(path, "/sys/class/drm/card%d", n);
> > +		sprintf(path + len, "/error");
> 
> That's goint to limit this to i915. I was thinking we could pass
> the drm_fd here to find the correct minor for it.

Hah, I was rewriting it to avoid the i915 presumption. Using fstat and
comparing sysfs/cardN/dev is what I went with. Pretty much equivalent to
drmGetDeviceNameFromFd(), yes.

> > +bool igt_sysfs_set(const char *filename, const char *value)
> > +{
> > +	int sysfs = __igt_sysfs_singleton();
> > +	int fd, len, ret;
> > +
> > +	if (sysfs < 0)
> > +		return false;
> > +
> > +	fd = openat(sysfs, filename, O_WRONLY);
> > +	if (fd < 0)
> > +		return false;
> > +
> > +	len = strlen(value);
> > +	ret = write(fd, value, len);
> 
> EINTR?

Pesky signals.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH igt 2/2] lib: Override the connector status using the sysfs status attribute
  2016-05-26 14:39   ` Ville Syrjälä
@ 2016-05-26 14:55     ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2016-05-26 14:55 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Thu, May 26, 2016 at 05:39:32PM +0300, Ville Syrjälä wrote:
> On Thu, May 26, 2016 at 03:14:30PM +0100, Chris Wilson wrote:
> > There are two paths to force enable a connector, via debugfs and via
> > sysfs. sysfs has the advantage of being a stable interface and of
> > updating the connector after application (allowing us to not force a
> > reprobe from userspace).
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  lib/igt_kms.c | 27 +++++++++------------------
> >  1 file changed, 9 insertions(+), 18 deletions(-)
> > 
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index 4da645a..5678248 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -48,6 +48,7 @@
> >  #include "igt_aux.h"
> >  #include "intel_chipset.h"
> >  #include "igt_debugfs.h"
> > +#include "igt_sysfs.h"
> >  
> >  /* list of connectors that need resetting on exit */
> >  #define MAX_CONNECTORS 32
> > @@ -596,9 +597,9 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
> >  {
> >  	char *path, **tmp;
> >  	const char *value;
> > -	int debugfs_fd, ret, len;
> >  	drmModeConnector *temp;
> >  	uint32_t devid;
> > +	int len;
> >  
> >  	devid = intel_get_drm_devid(drm_fd);
> >  
> > @@ -615,7 +616,7 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
> >  		value = "on";
> >  		break;
> >  	case FORCE_CONNECTOR_DIGITAL:
> > -		value = "digital";
> > +		value = "on-digital";
> >  		break;
> >  	case FORCE_CONNECTOR_OFF:
> >  		value = "off";
> > @@ -623,20 +624,14 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
> >  
> >  	default:
> >  	case FORCE_CONNECTOR_UNSPECIFIED:
> > -		value = "unspecified";
> > +		value = "detect";
> >  		break;
> >  	}
> >  
> > -	igt_assert_neq(asprintf(&path, "%s-%d/force", kmstest_connector_type_str(connector->connector_type), connector->connector_type_id),
> > +	igt_assert_neq(asprintf(&path, "%s-%d/status", kmstest_connector_type_str(connector->connector_type), connector->connector_type_id),
> 
> That'll need a cardN- prefix. Rather annoying.

Oh dear. That is annoying.

Be right back.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH igt v2 1/2] lib: Add simple sysfs accessors
  2016-05-26 14:14 [PATCH igt 1/2] lib: Add simple sysfs accessors Chris Wilson
  2016-05-26 14:14 ` [PATCH igt 2/2] lib: Override the connector status using the sysfs status attribute Chris Wilson
  2016-05-26 14:36 ` [PATCH igt 1/2] lib: Add simple sysfs accessors Ville Syrjälä
@ 2016-05-26 15:07 ` Chris Wilson
  2016-05-26 15:07   ` [PATCH igt v2 2/2] lib: Override the connector status using the sysfs status attribute Chris Wilson
  2016-05-26 16:19   ` [PATCH igt v3] lib: Add simple sysfs accessors Chris Wilson
  2 siblings, 2 replies; 12+ messages in thread
From: Chris Wilson @ 2016-05-26 15:07 UTC (permalink / raw)
  To: intel-gfx

igt_sysfs_set() for setting an attribute via sysfs, igt_sysfs_get() for
reading.

v2: Lots of little bugs in igt_sysfs_get()
v3: Pass device to open, stop assuming Intel rules.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/Makefile.sources |   2 +
 lib/igt_sysfs.c      | 198 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_sysfs.h      |  34 +++++++++
 3 files changed, 234 insertions(+)
 create mode 100644 lib/igt_sysfs.c
 create mode 100644 lib/igt_sysfs.h

diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 1316fd2..f50ff4d 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -16,6 +16,8 @@ libintel_tools_la_SOURCES = 	\
 	igt_gt.h		\
 	igt_stats.c		\
 	igt_stats.h		\
+	igt_sysfs.c		\
+	igt_sysfs.h		\
 	instdone.c		\
 	instdone.h		\
 	intel_batchbuffer.c	\
diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
new file mode 100644
index 0000000..4b993dc
--- /dev/null
+++ b/lib/igt_sysfs.c
@@ -0,0 +1,198 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <inttypes.h>
+#include <sys/stat.h>
+#include <sys/mount.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <limits.h>
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <i915_drm.h>
+
+#include "igt_sysfs.h"
+
+static int readN(int fd, char *buf, int len)
+{
+	int total = 0;
+	do {
+		int ret = read(fd, buf + total, len - total);
+		if (ret < 0 && (errno == EINTR || errno == EAGAIN))
+			continue;
+
+		if (ret <= 0)
+			return total ?: ret;
+
+		total += ret;
+	} while (1);
+}
+
+static int writeN(int fd, const char *buf, int len)
+{
+	int total = 0;
+	do {
+		int ret = write(fd, buf + total, len - total);
+		if (ret < 0 && (errno == EINTR || errno == EAGAIN))
+			continue;
+
+		if (ret <= 0)
+			return total ?: ret;
+
+		total += ret;
+	} while (1);
+}
+
+/**
+ * igt_sysfs_open:
+ * @device: fd of the device (or -1 to default to Intel)
+ *
+ * This opens the sysfs directory corresponding to device for use
+ * with igt_sysfs_set() and igt_sysfs_get().
+ *
+ * Returns:
+ * The directory fd, or -1 on failure.
+ */
+int igt_sysfs_open(int fd, int *idx)
+{
+	char device[80];
+	char path[80];
+	struct stat st;
+
+	device[0] = '\0';
+	if (fd != -1) {
+		if (fstat(fd, &st) || !S_ISCHR(st.st_mode))
+			return -1;
+
+		sprintf(device, "%d:%d", major(st.st_rdev), minor(st.st_rdev));
+	}
+
+	for (int n = 0; n < 16; n++) {
+		int len = sprintf(path, "/sys/class/drm/card%d", n);
+		if (device[0]) {
+			sprintf(path + len, "/dev");
+			fd = open(path, O_RDONLY);
+			if (fd == -1)
+				continue;
+
+			len = read(fd, path, sizeof(path-1));
+			if (len < 0)
+				len = 0;
+			path[len] = '\0';
+			close(fd);
+
+			if (strcmp(path, device))
+				continue;
+		} else {
+			/* Bleh. Search for intel */
+			sprintf(path + len, "/error");
+			if (stat(path, &st))
+				continue;
+		}
+
+		path[len] = '\0';
+		if (idx)
+			*idx = n;
+		return open(path, O_RDONLY);
+	}
+
+	return -1;
+}
+
+/**
+ * igt_sysfs_set:
+ * @device: directory for the device from igt_sysfs_open()
+ * @attr: name of the sysfs node to open
+ * @value: the string to write
+ *
+ * This writes the value to the sysfs file.
+ *
+ * Returns:
+ * True on success, false on failure.
+ */
+bool igt_sysfs_set(int device, const char *attr, const char *value)
+{
+	int fd, len, ret;
+
+	fd = openat(device, attr, O_WRONLY);
+	if (fd < 0)
+		return false;
+
+	len = strlen(value);
+	ret = writeN(fd, value, len);
+	close(fd);
+
+	return len == ret;
+}
+
+/**
+ * igt_sysfs_get:
+ * @device: directory for the device from igt_sysfs_open()
+ * @attr: name of the sysfs node to open
+ *
+ * This reads the value from the sysfs file.
+ *
+ * Returns:
+ * A nul-terminated string, must be freed by caller after use, or NULL
+ * on failure.
+ */
+char *igt_sysfs_get(int device, const char *attr)
+{
+	char *buf;
+	int len, offset, rem;
+	int ret, fd;
+
+	fd = openat(device, attr, O_RDONLY);
+	if (fd < 0)
+		return NULL;
+
+	offset = 0;
+	len = 64;
+	rem =  len - offset - 1;
+	buf = malloc(len);
+	if (!buf)
+		goto out;
+
+	while ((ret = readN(fd, buf + offset, rem)) == rem) {
+		char *newbuf;
+
+		newbuf = realloc(buf, 2*len);
+		if (!newbuf)
+			break;
+
+		len *= 2;
+		offset += ret;
+		rem = len - offset - 1;
+	}
+
+	if (ret != -1)
+		offset += ret;
+	buf[offset] = '0';
+
+out:
+	close(fd);
+	return buf;
+}
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
new file mode 100644
index 0000000..5c49f64
--- /dev/null
+++ b/lib/igt_sysfs.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __IGT_SYSFS_H__
+#define __IGT_SYSFS_H__
+
+#include <stdbool.h>
+
+int igt_sysfs_open(int device, int *idx);
+bool igt_sysfs_set(int dir, const char *attr, const char *value);
+char *igt_sysfs_get(int dir, const char *attr);
+
+#endif /* __IGT_SYSFS_H__ */
-- 
2.8.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH igt v2 2/2] lib: Override the connector status using the sysfs status attribute
  2016-05-26 15:07 ` [PATCH igt v2 " Chris Wilson
@ 2016-05-26 15:07   ` Chris Wilson
  2016-05-26 16:19   ` [PATCH igt v3] lib: Add simple sysfs accessors Chris Wilson
  1 sibling, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2016-05-26 15:07 UTC (permalink / raw)
  To: intel-gfx

There are two paths to force enable a connector, via debugfs and via
sysfs. sysfs has the advantage of being a stable interface and of
updating the connector after application (allowing us to not force a
reprobe from userspace).

v2: Don't assume Intel only

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_kms.c | 45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 4da645a..b12a5b3 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -48,10 +48,12 @@
 #include "igt_aux.h"
 #include "intel_chipset.h"
 #include "igt_debugfs.h"
+#include "igt_sysfs.h"
 
 /* list of connectors that need resetting on exit */
 #define MAX_CONNECTORS 32
 static char *forced_connectors[MAX_CONNECTORS + 1];
+static int forced_connectors_device[MAX_CONNECTORS + 1];
 
 static void update_edid_csum(unsigned char *edid)
 {
@@ -596,9 +598,9 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
 {
 	char *path, **tmp;
 	const char *value;
-	int debugfs_fd, ret, len;
 	drmModeConnector *temp;
 	uint32_t devid;
+	int len, dir, idx;
 
 	devid = intel_get_drm_devid(drm_fd);
 
@@ -615,7 +617,7 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
 		value = "on";
 		break;
 	case FORCE_CONNECTOR_DIGITAL:
-		value = "digital";
+		value = "on-digital";
 		break;
 	case FORCE_CONNECTOR_OFF:
 		value = "off";
@@ -623,20 +625,26 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
 
 	default:
 	case FORCE_CONNECTOR_UNSPECIFIED:
-		value = "unspecified";
+		value = "detect";
 		break;
 	}
 
-	igt_assert_neq(asprintf(&path, "%s-%d/force", kmstest_connector_type_str(connector->connector_type), connector->connector_type_id),
-		       -1);
-	debugfs_fd = igt_debugfs_open(path, O_WRONLY | O_TRUNC);
+	dir = igt_sysfs_open(drm_fd, &idx);
+	if (dir < 0)
+		return false;
 
-	if (debugfs_fd == -1) {
+	if (asprintf(&path, "card%d-%s-%d/status",
+		     idx,
+		     kmstest_connector_type_str(connector->connector_type),
+		     connector->connector_type_id) < 0) {
+		close(dir);
 		return false;
 	}
 
-	ret = write(debugfs_fd, value, strlen(value));
-	close(debugfs_fd);
+	if (!igt_sysfs_set(drm_fd, path, value)) {
+		close(dir);
+		return false;
+	}
 
 	for (len = 0, tmp = forced_connectors; *tmp; tmp++) {
 		/* check the connector is not already present */
@@ -647,8 +655,10 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
 		len++;
 	}
 
-	if (len != -1 && len < MAX_CONNECTORS)
+	if (len != -1 && len < MAX_CONNECTORS) {
 		forced_connectors[len] = path;
+		forced_connectors_device[len] = dir;
+	}
 
 	if (len >= MAX_CONNECTORS)
 		igt_warn("Connector limit reached, %s will not be reset\n",
@@ -669,8 +679,7 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
 	temp = drmModeGetConnector(drm_fd, connector->connector_id);
 	drmModeFreeConnector(temp);
 
-	igt_assert(ret != -1);
-	return (ret == -1) ? false : true;
+	return true;
 }
 
 /**
@@ -2543,14 +2552,10 @@ void igt_enable_connectors(void)
  */
 void igt_reset_connectors(void)
 {
-	char **tmp;
-
 	/* reset the connectors stored in forced_connectors, avoiding any
 	 * functions that are not safe to call in signal handlers */
-
-	for (tmp = forced_connectors; *tmp; tmp++) {
-		int fd = igt_debugfs_open(*tmp, O_WRONLY | O_TRUNC);
-		igt_assert(write(fd, "unspecified", 11) == 11);
-		close(fd);
-	}
+	for (int i = 0; forced_connectors[i]; i++)
+		igt_sysfs_set(forced_connectors_device[i],
+			      forced_connectors[i],
+			      "detect");
 }
-- 
2.8.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH igt v3] lib: Add simple sysfs accessors
  2016-05-26 15:07 ` [PATCH igt v2 " Chris Wilson
  2016-05-26 15:07   ` [PATCH igt v2 2/2] lib: Override the connector status using the sysfs status attribute Chris Wilson
@ 2016-05-26 16:19   ` Chris Wilson
  2016-05-27 17:46     ` Ville Syrjälä
  1 sibling, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2016-05-26 16:19 UTC (permalink / raw)
  To: intel-gfx

igt_sysfs_set() for setting an attribute via sysfs, igt_sysfs_get() for
reading.

v2: Lots of little bugs in igt_sysfs_get()
v3: Pass device to open, stop assuming Intel rules.
v4: Test opening and reading!

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/Makefile.sources |   2 +
 lib/igt_sysfs.c      | 205 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_sysfs.h      |  34 +++++++++
 3 files changed, 241 insertions(+)
 create mode 100644 lib/igt_sysfs.c
 create mode 100644 lib/igt_sysfs.h

diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 1316fd2..f50ff4d 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -16,6 +16,8 @@ libintel_tools_la_SOURCES = 	\
 	igt_gt.h		\
 	igt_stats.c		\
 	igt_stats.h		\
+	igt_sysfs.c		\
+	igt_sysfs.h		\
 	instdone.c		\
 	instdone.h		\
 	intel_batchbuffer.c	\
diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
new file mode 100644
index 0000000..e9dba0a
--- /dev/null
+++ b/lib/igt_sysfs.c
@@ -0,0 +1,205 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <inttypes.h>
+#include <sys/stat.h>
+#include <sys/mount.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <limits.h>
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <i915_drm.h>
+
+#include "igt_sysfs.h"
+
+static int readN(int fd, char *buf, int len)
+{
+	int total = 0;
+	do {
+		int ret = read(fd, buf + total, len - total);
+		if (ret < 0 && (errno == EINTR || errno == EAGAIN))
+			continue;
+
+		if (ret <= 0)
+			return total ?: ret;
+
+		total += ret;
+	} while (1);
+}
+
+static int writeN(int fd, const char *buf, int len)
+{
+	int total = 0;
+	do {
+		int ret = write(fd, buf + total, len - total);
+		if (ret < 0 && (errno == EINTR || errno == EAGAIN))
+			continue;
+
+		if (ret <= 0)
+			return total ?: ret;
+
+		total += ret;
+	} while (1);
+}
+
+/**
+ * igt_sysfs_open:
+ * @device: fd of the device (or -1 to default to Intel)
+ *
+ * This opens the sysfs directory corresponding to device for use
+ * with igt_sysfs_set() and igt_sysfs_get().
+ *
+ * Returns:
+ * The directory fd, or -1 on failure.
+ */
+int igt_sysfs_open(int fd, int *idx)
+{
+	char device[80];
+	char path[80];
+	struct stat st;
+
+	device[0] = '\0';
+	if (fd != -1) {
+		if (fstat(fd, &st) || !S_ISCHR(st.st_mode))
+			return -1;
+
+		sprintf(device, "%d:%d", major(st.st_rdev), minor(st.st_rdev));
+	}
+
+	for (int n = 0; n < 16; n++) {
+		int len = sprintf(path, "/sys/class/drm/card%d", n);
+		if (device[0]) {
+			char tmp[80];
+			int ret;
+
+			sprintf(path + len, "/dev");
+			fd = open(path, O_RDONLY);
+			if (fd == -1)
+				continue;
+
+			ret = read(fd, tmp, sizeof(tmp-1));
+			if (ret < 0)
+				ret = 0;
+			tmp[ret] = '\0';
+			while (ret > 0 && tmp[ret-1] == '\n')
+				tmp[--ret] = '\0';
+			close(fd);
+
+			if (strcmp(tmp, device))
+				continue;
+		} else {
+			/* Bleh. Search for intel */
+			sprintf(path + len, "/error");
+			if (stat(path, &st))
+				continue;
+		}
+
+		path[len] = '\0';
+		if (idx)
+			*idx = n;
+		return open(path, O_RDONLY);
+	}
+
+	return -1;
+}
+
+/**
+ * igt_sysfs_set:
+ * @dir: directory for the device from igt_sysfs_open()
+ * @attr: name of the sysfs node to open
+ * @value: the string to write
+ *
+ * This writes the value to the sysfs file.
+ *
+ * Returns:
+ * True on success, false on failure.
+ */
+bool igt_sysfs_set(int dir, const char *attr, const char *value)
+{
+	int fd, len, ret;
+
+	fd = openat(dir, attr, O_WRONLY);
+	if (fd < 0)
+		return false;
+
+	len = strlen(value);
+	ret = writeN(fd, value, len);
+	close(fd);
+
+	return len == ret;
+}
+
+/**
+ * igt_sysfs_get:
+ * @dir: directory for the device from igt_sysfs_open()
+ * @attr: name of the sysfs node to open
+ *
+ * This reads the value from the sysfs file.
+ *
+ * Returns:
+ * A nul-terminated string, must be freed by caller after use, or NULL
+ * on failure.
+ */
+char *igt_sysfs_get(int dir, const char *attr)
+{
+	char *buf;
+	int len, offset, rem;
+	int ret, fd;
+
+	fd = openat(dir, attr, O_RDONLY);
+	if (fd < 0)
+		return NULL;
+
+	offset = 0;
+	len = 64;
+	rem = len - offset - 1;
+	buf = malloc(len);
+	if (!buf)
+		goto out;
+
+	while ((ret = readN(fd, buf + offset, rem)) == rem) {
+		char *newbuf;
+
+		newbuf = realloc(buf, 2*len);
+		if (!newbuf)
+			break;
+
+		len *= 2;
+		offset += ret;
+		rem = len - offset - 1;
+	}
+
+	if (ret != -1)
+		offset += ret;
+	buf[offset] = '\0';
+	while (offset > 0 && buf[offset-1] == '\n')
+		buf[--offset] = '\0';
+
+out:
+	close(fd);
+	return buf;
+}
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
new file mode 100644
index 0000000..5c49f64
--- /dev/null
+++ b/lib/igt_sysfs.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __IGT_SYSFS_H__
+#define __IGT_SYSFS_H__
+
+#include <stdbool.h>
+
+int igt_sysfs_open(int device, int *idx);
+bool igt_sysfs_set(int dir, const char *attr, const char *value);
+char *igt_sysfs_get(int dir, const char *attr);
+
+#endif /* __IGT_SYSFS_H__ */
-- 
2.8.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH igt v3] lib: Add simple sysfs accessors
  2016-05-26 16:19   ` [PATCH igt v3] lib: Add simple sysfs accessors Chris Wilson
@ 2016-05-27 17:46     ` Ville Syrjälä
  2016-05-27 18:07       ` Chris Wilson
  0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2016-05-27 17:46 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Thu, May 26, 2016 at 05:19:35PM +0100, Chris Wilson wrote:
> igt_sysfs_set() for setting an attribute via sysfs, igt_sysfs_get() for
> reading.
> 
> v2: Lots of little bugs in igt_sysfs_get()
> v3: Pass device to open, stop assuming Intel rules.
> v4: Test opening and reading!
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  lib/Makefile.sources |   2 +
>  lib/igt_sysfs.c      | 205 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_sysfs.h      |  34 +++++++++
>  3 files changed, 241 insertions(+)
>  create mode 100644 lib/igt_sysfs.c
>  create mode 100644 lib/igt_sysfs.h
> 
> diff --git a/lib/Makefile.sources b/lib/Makefile.sources
> index 1316fd2..f50ff4d 100644
> --- a/lib/Makefile.sources
> +++ b/lib/Makefile.sources
> @@ -16,6 +16,8 @@ libintel_tools_la_SOURCES = 	\
>  	igt_gt.h		\
>  	igt_stats.c		\
>  	igt_stats.h		\
> +	igt_sysfs.c		\
> +	igt_sysfs.h		\
>  	instdone.c		\
>  	instdone.h		\
>  	intel_batchbuffer.c	\
> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> new file mode 100644
> index 0000000..e9dba0a
> --- /dev/null
> +++ b/lib/igt_sysfs.c
> @@ -0,0 +1,205 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include <inttypes.h>
> +#include <sys/stat.h>
> +#include <sys/mount.h>
> +#include <errno.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <limits.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <i915_drm.h>
> +
> +#include "igt_sysfs.h"
> +
> +static int readN(int fd, char *buf, int len)
> +{
> +	int total = 0;
> +	do {
> +		int ret = read(fd, buf + total, len - total);
> +		if (ret < 0 && (errno == EINTR || errno == EAGAIN))
> +			continue;
> +
> +		if (ret <= 0)
> +			return total ?: ret;
> +
> +		total += ret;
> +	} while (1);
> +}
> +
> +static int writeN(int fd, const char *buf, int len)
> +{
> +	int total = 0;
> +	do {
> +		int ret = write(fd, buf + total, len - total);
> +		if (ret < 0 && (errno == EINTR || errno == EAGAIN))
> +			continue;
> +
> +		if (ret <= 0)
> +			return total ?: ret;
> +
> +		total += ret;
> +	} while (1);
> +}
> +
> +/**
> + * igt_sysfs_open:
> + * @device: fd of the device (or -1 to default to Intel)
> + *
> + * This opens the sysfs directory corresponding to device for use
> + * with igt_sysfs_set() and igt_sysfs_get().
> + *
> + * Returns:
> + * The directory fd, or -1 on failure.
> + */
> +int igt_sysfs_open(int fd, int *idx)
> +{
> +	char device[80];
> +	char path[80];
> +	struct stat st;
> +
> +	device[0] = '\0';
> +	if (fd != -1) {
> +		if (fstat(fd, &st) || !S_ISCHR(st.st_mode))
> +			return -1;
> +
> +		sprintf(device, "%d:%d", major(st.st_rdev), minor(st.st_rdev));
> +	}
> +
> +	for (int n = 0; n < 16; n++) {
> +		int len = sprintf(path, "/sys/class/drm/card%d", n);
> +		if (device[0]) {
> +			char tmp[80];
> +			int ret;
> +
> +			sprintf(path + len, "/dev");
> +			fd = open(path, O_RDONLY);
> +			if (fd == -1)
> +				continue;
> +
> +			ret = read(fd, tmp, sizeof(tmp-1));
> +			if (ret < 0)
> +				ret = 0;
> +			tmp[ret] = '\0';
> +			while (ret > 0 && tmp[ret-1] == '\n')
> +				tmp[--ret] = '\0';
> +			close(fd);
> +
> +			if (strcmp(tmp, device))
> +				continue;

fscanf?

> +		} else {
> +			/* Bleh. Search for intel */
> +			sprintf(path + len, "/error");
> +			if (stat(path, &st))
> +				continue;
> +		}
> +
> +		path[len] = '\0';
> +		if (idx)
> +			*idx = n;
> +		return open(path, O_RDONLY);
> +	}
> +
> +	return -1;
> +}
> +
> +/**
> + * igt_sysfs_set:
> + * @dir: directory for the device from igt_sysfs_open()
> + * @attr: name of the sysfs node to open
> + * @value: the string to write
> + *
> + * This writes the value to the sysfs file.
> + *
> + * Returns:
> + * True on success, false on failure.
> + */
> +bool igt_sysfs_set(int dir, const char *attr, const char *value)
> +{
> +	int fd, len, ret;
> +
> +	fd = openat(dir, attr, O_WRONLY);
> +	if (fd < 0)
> +		return false;
> +
> +	len = strlen(value);
> +	ret = writeN(fd, value, len);
> +	close(fd);
> +
> +	return len == ret;
> +}
> +
> +/**
> + * igt_sysfs_get:
> + * @dir: directory for the device from igt_sysfs_open()
> + * @attr: name of the sysfs node to open
> + *
> + * This reads the value from the sysfs file.
> + *
> + * Returns:
> + * A nul-terminated string, must be freed by caller after use, or NULL
> + * on failure.
> + */
> +char *igt_sysfs_get(int dir, const char *attr)
> +{
> +	char *buf;
> +	int len, offset, rem;
> +	int ret, fd;
> +
> +	fd = openat(dir, attr, O_RDONLY);
> +	if (fd < 0)
> +		return NULL;
> +
> +	offset = 0;
> +	len = 64;
> +	rem = len - offset - 1;
> +	buf = malloc(len);
> +	if (!buf)
> +		goto out;
> +
> +	while ((ret = readN(fd, buf + offset, rem)) == rem) {
> +		char *newbuf;
> +
> +		newbuf = realloc(buf, 2*len);
> +		if (!newbuf)
> +			break;
> +
> +		len *= 2;
> +		offset += ret;
> +		rem = len - offset - 1;
> +	}
> +
> +	if (ret != -1)
> +		offset += ret;
> +	buf[offset] = '\0';
> +	while (offset > 0 && buf[offset-1] == '\n')
> +		buf[--offset] = '\0';
> +
> +out:
> +	close(fd);
> +	return buf;
> +}
> diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
> new file mode 100644
> index 0000000..5c49f64
> --- /dev/null
> +++ b/lib/igt_sysfs.h
> @@ -0,0 +1,34 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef __IGT_SYSFS_H__
> +#define __IGT_SYSFS_H__
> +
> +#include <stdbool.h>
> +
> +int igt_sysfs_open(int device, int *idx);
> +bool igt_sysfs_set(int dir, const char *attr, const char *value);
> +char *igt_sysfs_get(int dir, const char *attr);
> +
> +#endif /* __IGT_SYSFS_H__ */
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH igt v3] lib: Add simple sysfs accessors
  2016-05-27 17:46     ` Ville Syrjälä
@ 2016-05-27 18:07       ` Chris Wilson
  2016-05-30 14:45         ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2016-05-27 18:07 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Fri, May 27, 2016 at 08:46:14PM +0300, Ville Syrjälä wrote:
> > +	for (int n = 0; n < 16; n++) {
> > +		int len = sprintf(path, "/sys/class/drm/card%d", n);
> > +		if (device[0]) {
> > +			char tmp[80];
> > +			int ret;
> > +
> > +			sprintf(path + len, "/dev");
> > +			fd = open(path, O_RDONLY);
> > +			if (fd == -1)
> > +				continue;
> > +
> > +			ret = read(fd, tmp, sizeof(tmp-1));
> > +			if (ret < 0)
> > +				ret = 0;
> > +			tmp[ret] = '\0';
> > +			while (ret > 0 && tmp[ret-1] == '\n')
> > +				tmp[--ret] = '\0';
> > +			close(fd);
> > +
> > +			if (strcmp(tmp, device))
> > +				continue;
> 
> fscanf?

                if (fd != -1) {
                        FILE *file;
                        int ret, maj, min;

                        sprintf(path + len, "/dev");
                        file = fopen(path, "r");
                        if (!file)
                                continue;

                        ret = fscanf(file, "%d:%d", &maj, &min);
                        fclose(file);

                        if (ret != 2 ||
                            major(st.st_rdev) != maj ||
                            minor(st.st_rdev) != min)
                                continue;

-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH igt v3] lib: Add simple sysfs accessors
  2016-05-27 18:07       ` Chris Wilson
@ 2016-05-30 14:45         ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2016-05-30 14:45 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Fri, May 27, 2016 at 07:07:27PM +0100, Chris Wilson wrote:
> On Fri, May 27, 2016 at 08:46:14PM +0300, Ville Syrjälä wrote:
> > > +	for (int n = 0; n < 16; n++) {
> > > +		int len = sprintf(path, "/sys/class/drm/card%d", n);
> > > +		if (device[0]) {
> > > +			char tmp[80];
> > > +			int ret;
> > > +
> > > +			sprintf(path + len, "/dev");
> > > +			fd = open(path, O_RDONLY);
> > > +			if (fd == -1)
> > > +				continue;
> > > +
> > > +			ret = read(fd, tmp, sizeof(tmp-1));
> > > +			if (ret < 0)
> > > +				ret = 0;
> > > +			tmp[ret] = '\0';
> > > +			while (ret > 0 && tmp[ret-1] == '\n')
> > > +				tmp[--ret] = '\0';
> > > +			close(fd);
> > > +
> > > +			if (strcmp(tmp, device))
> > > +				continue;
> > 
> > fscanf?
> 
>                 if (fd != -1) {
>                         FILE *file;
>                         int ret, maj, min;
> 
>                         sprintf(path + len, "/dev");
>                         file = fopen(path, "r");
>                         if (!file)
>                                 continue;
> 
>                         ret = fscanf(file, "%d:%d", &maj, &min);
>                         fclose(file);
> 
>                         if (ret != 2 ||
>                             major(st.st_rdev) != maj ||
>                             minor(st.st_rdev) != min)
>                                 continue;

lgtm

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-05-30 14:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-26 14:14 [PATCH igt 1/2] lib: Add simple sysfs accessors Chris Wilson
2016-05-26 14:14 ` [PATCH igt 2/2] lib: Override the connector status using the sysfs status attribute Chris Wilson
2016-05-26 14:39   ` Ville Syrjälä
2016-05-26 14:55     ` Chris Wilson
2016-05-26 14:36 ` [PATCH igt 1/2] lib: Add simple sysfs accessors Ville Syrjälä
2016-05-26 14:50   ` Chris Wilson
2016-05-26 15:07 ` [PATCH igt v2 " Chris Wilson
2016-05-26 15:07   ` [PATCH igt v2 2/2] lib: Override the connector status using the sysfs status attribute Chris Wilson
2016-05-26 16:19   ` [PATCH igt v3] lib: Add simple sysfs accessors Chris Wilson
2016-05-27 17:46     ` Ville Syrjälä
2016-05-27 18:07       ` Chris Wilson
2016-05-30 14:45         ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox