Skip to contentSkip to conversation

tools: Add a simple tool to read/write/decode dpcd registers

PATCH Superseded Tarun Vyas 3 participants 4 messages Sep 26, 2018 – Oct 3, 2018 Arm/Amlogic Meson SoC support

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Tarun Vyas

author
#0 lore raw

PATCH v4 1/1 tools: Add a simple tool to read/write/decode dpcd registers

This tool serves as a wrapper around the constructs provided by the drm_dpcd_aux_dev kernel module by working on the /dev/drm_dp_aux[n] devices created by the kernel module.
It supports reading and writing dpcd registers on the connected aux channels.
In the follow-up patch, support for decoding these registers will be added to facilate debugging panel related issues.

v2: (Fixes by Rodrigo but no functional changes yet):
- Indentations, Typo, Missed spaces
- Removing mentioning to decode and spec that is not implemented yet.
- Add Makefile.sources back
- Missed s/printf/igt_warn

v3:
- Addres DK's review comments from v2 above.
- Squash Rodrigo's file handling unification patch.
- Make count, offset and device id optional.

v4:
- Better error handling and refactoring.

---
 tools/Makefile.sources |   1 +
 tools/dpcd_reg.c       | 273 +++++++++++++++++++++++++++++++++++++++++++++++++
 tools/meson.build      |   1 +
 3 files changed, 275 insertions(+)
 create mode 100644 tools/dpcd_reg.c
tools/Makefile.sources +1 -0
index abd23a0f..50706f41 100644
@@ -7,6 +7,7 @@ noinst_PROGRAMS =		\
 
 tools_prog_lists =		\
 	igt_stats		\
+	dpcd_reg		\
 	intel_audio_dump	\
 	intel_reg		\
 	intel_backlight		\
tools/dpcd_reg.c new file +273 -0

273 lines not on this page · raw message

tools/meson.build +1 -0
index e4517d66..79f36aa9 100644
@@ -36,6 +36,7 @@ tools_progs = [
 	'intel_watermark',
 	'intel_gem_info',
 	'intel_gvtg_test',
+	'dpcd_reg',
 ]
 tool_deps = igt_deps
 
Signature · 6 lines
2.14.1

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev
#1 lore raw

[igt-dev] ✓ Fi.CI.BAT: success for tools: Add a simple tool to read/write/decode dpcd registers (rev4)

== Series Details ==

== Series Details ==

Series: tools: Add a simple tool to read/write/decode dpcd registers (rev4)
URL : https://patchwork.freedesktop.org/series/44736/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4874 -> IGTPW_1872 =

== Summary - SUCCESS ==

No regressions found.

External URL: https://patchwork.freedesktop.org/api/1.0/series/44736/revisions/4/mbox/

== Known issues ==

Here are the changes found in IGTPW_1872 that come from known issues:

=== IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_coherency:
      fi-gdg-551:         PASS -> DMESG-FAIL (fdo#107164)

    igt@gem_exec_suspend@basic-s4-devices:
      fi-bdw-samus:       PASS -> INCOMPLETE (fdo#107773)

    igt@kms_flip@basic-flip-vs-dpms:
      fi-hsw-4770r:       PASS -> DMESG-WARN (fdo#105602)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-cfl-8109u:       PASS -> INCOMPLETE (fdo#106070)

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     FAIL (fdo#103167) -> PASS

    igt@kms_psr@primary_page_flip:
      fi-cnl-u:           FAIL (fdo#107336) -> PASS

fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#106070 https://bugs.freedesktop.org/show_bug.cgi?id=106070
fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773

== Participating hosts (48 -> 41) ==

Additional (1): fi-skl-6700hq
Missing (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bwr-2160 fi-bsw-cyan fi-ctg-p8600 fi-skl-caroline

== Build changes ==

* IGT: IGT_4649 -> IGTPW_1872

CI_DRM_4874: 7bb42432ef1cc63b4a6a223e19455f6c4f9a1538 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_1872: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1872/
IGT_4649: 19b0c74d20d9b53d4c82be14af0909a3b6846010 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1872/issues.html
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

#2 lore raw

[igt-dev] ✓ Fi.CI.IGT: success for tools: Add a simple tool to read/write/decode dpcd registers (rev4)

In reply to Tarun Vyas
== Series Details ==

== Series Details ==

Series: tools: Add a simple tool to read/write/decode dpcd registers (rev4)
URL : https://patchwork.freedesktop.org/series/44736/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4649_full -> IGTPW_1872_full =

== Summary - SUCCESS ==

No regressions found.

External URL: https://patchwork.freedesktop.org/api/1.0/series/44736/revisions/4/mbox/

== Known issues ==

Here are the changes found in IGTPW_1872_full that come from known issues:

=== IGT changes ===

    ==== Issues hit ====

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-blt:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763, fdo#106538) +1

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-indfb-pgflip-blt:
      shard-glk:          PASS -> DMESG-FAIL (fdo#106538)

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-fullscreen:
      shard-glk:          PASS -> FAIL (fdo#103167)

    igt@kms_rotation_crc@sprite-rotation-180:
      shard-snb:          PASS -> FAIL (fdo#103925) +1

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@kms_busy@extended-pageflip-hang-newfb-render-a:
      shard-glk:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_cursor_legacy@pipe-c-forked-move:
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    igt@kms_universal_plane@universal-plane-gen9-features-pipe-a:
      shard-kbl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +14

fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

== Participating hosts (5 -> 5) ==

No changes in participating hosts

== Build changes ==

* IGT: IGT_4649 -> IGTPW_1872
* Linux: CI_DRM_4859 -> CI_DRM_4874

CI_DRM_4859: 841654aee134e50b21bf8f2aab17da8d0afd14c4 @ git://anongit.freedesktop.org/gfx-ci/linux
CI_DRM_4874: 7bb42432ef1cc63b4a6a223e19455f6c4f9a1538 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_1872: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1872/
IGT_4649: 19b0c74d20d9b53d4c82be14af0909a3b6846010 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1872/shards.html
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Reviewed-by on v4 1/1
#3 lore raw
In reply to Tarun Vyas
Quoted 115 lines · PDT Tarun Vyas

This tool serves as a wrapper around the constructs provided by the drm_dpcd_aux_dev kernel module by working on the /dev/drm_dp_aux[n] devices created by the kernel module.
It supports reading and writing dpcd registers on the connected aux channels.
In the follow-up patch, support for decoding these registers will be added to facilate debugging panel related issues.

v2: (Fixes by Rodrigo but no functional changes yet):
- Indentations, Typo, Missed spaces
- Removing mentioning to decode and spec that is not implemented yet.
- Add Makefile.sources back
- Missed s/printf/igt_warn

v3:
- Addres DK's review comments from v2 above.
- Squash Rodrigo's file handling unification patch.
- Make count, offset and device id optional.

v4:
- Better error handling and refactoring.

---
 tools/Makefile.sources |   1 +
 tools/dpcd_reg.c       | 273
+++++++++++++++++++++++++++++++++++++++++++++++++ tools/meson.build      | 
 1 +
 3 files changed, 275 insertions(+)
 create mode 100644 tools/dpcd_reg.c
tools/Makefile.sources +1 -0
index abd23a0f..50706f41 100644
@@ -7,6 +7,7 @@ noinst_PROGRAMS =		\
 
 tools_prog_lists =		\
 	igt_stats		\
+	dpcd_reg		\
 	intel_audio_dump	\
 	intel_reg		\
 	intel_backlight		\
tools/dpcd_reg.c new file +5 -0
new file mode 100644
index 00000000..8419b9e5
@@ -0,0 +1,273 @@
+/*
+ * Copyright © 2018 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.
+ *
+ * DPCD register read/write tool
+ * This tool wraps around DRM_DP_AUX_DEV module to provide DPCD register read + * and write, so CONFIG_DRM_DP_AUX_DEV needs to be set.
+ */

tools/dpcd_reg.c new file +38 -0
+
+#include <stdio.h>
+#include <errno.h>
+#include <string.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <getopt.h>
+#include <stdint.h>
+#include <unistd.h>
+#include <limits.h>
+#include <stdbool.h>
+
+#define MAX_DP_OFFSET	0xfffff
+#define DRM_AUX_MINORS	256
+
+const char aux_dev[] = "/dev/drm_dp_aux";
+
+struct dpcd_data {
+	int devid;
+	int file_op;
+	uint32_t offset;
+	enum command {
+		INVALID = -1,
+		READ = 2,
+		WRITE,
+	} cmd;
+	size_t count;
+	uint8_t val;
+};
+
+static void print_usage(void)
+{
+	printf("Usage: dpcd_reg [OPTION ...] COMMAND\n\n");
+	printf("COMMAND is one of:\n");
+	printf("  read:		Read [count] bytes dpcd reg at an offset\n");
+	printf("  write:	Write a dpcd reg at an offset\n\n");
+	printf("Options for the above COMMANDS are\n");
+	printf(" --device=DEVID		Aux device id, as listed in
Quoted 1 line

/dev/drm_dp_aux_dev[n]." + "Defaults to 0. Upper limit -

256\n");

Quoted 13 lines
tools/dpcd_reg.c new file +3 -0
+	printf(" --offset=REG_ADDR	DPCD register offset in hex. Defaults to 0x0"
+					"Upper limit - 0xfffff\n");
+	printf(" --count=BYTES		For reads, specify number of bytes to be read

from" + " the offset. Defaults to 1\n");

tools/dpcd_reg.c new file +9 -0
+	printf(" --value		For writes, specify a hex value to be written"
+					"Upper limit - 0xff\n\n");
+
+	printf(" --help: print the usage\n");
+}
+
+static inline bool strtol_err_util(char *endptr, long *val)
+{
+	return (*endptr != '\0' || *val < 0 || (*val == LONG_MAX && errno ==
Quoted 1 line

ERANGE)) \ + ? true : false;

I think writing it in this form would read better.

if (error_condition)

	return true;

return false;

Quoted 13 lines
tools/dpcd_reg.c new file +13 -0
+}
+
+static int parse_opts(struct dpcd_data *dpcd, int argc, char **argv)
+{
+	int ret, vflag = 0;
+	long temp;
+	char *endptr;
+
+	struct option longopts[] = {
+		{ "count",	required_argument,	NULL,		'c' },
+		{ "device",	required_argument,	NULL,		'd' },
+		{ "help",	no_argument,		NULL,		'h' },
+		{ "offset",	required_argument,	NULL,		'o' },

tools/dpcd_reg.c

Quoted 10 lines
tools/dpcd_reg.c new file +9 -1
+		{ "value",	required_argument,	NULL,		'v' },
+		{ 0 }
+	};
+
+	while ((ret = getopt_long(argc, argv, "-:c:d:ho:v:", longopts, NULL)) !=
-1) { +		switch (ret) {
+		case 'c':
+			temp = strtol(optarg, &endptr, 10);
+			if (strtol_err_util(endptr, &temp)) {
+				fprintf(stderr, "--count argument is invalid/negative/out-of-

range\n");

tools/dpcd_reg.c

Quoted 9 lines
tools/dpcd_reg.c new file +9 -0
+				print_usage();
+				return EXIT_FAILURE;
+			}
+			dpcd->count = temp;
+			break;
+		case 'd':
+			temp = strtol(optarg, &endptr, 10);
+			if (strtol_err_util(endptr, &temp) || temp > DRM_AUX_MINORS) {
+				fprintf(stderr, "--devid argument is invalid/negative/out-of-

range\n");

Quoted 16 lines
tools/dpcd_reg.c new file +15 -0
+				print_usage();
+				return ERANGE;
+			}
+			dpcd->devid = temp;
+			break;
+		case 'h':
+			printf("DPCD register read and write tool\n\n");
+			printf("This tool requires CONFIG_DRM_DP_AUX_CHARDEV\n"
+				"to be set in the kernel config.\n\n");
+			print_usage();
+			exit(EXIT_SUCCESS);
+		case 'o':
+			temp = strtol(optarg, &endptr, 16);
+			if (strtol_err_util(endptr, &temp) || temp > MAX_DP_OFFSET) {
+				fprintf(stderr, "--offset argument is

invalid/negative/out-of-range\n"); + print_usage();

tools/dpcd_reg.c

Quoted 9 lines
tools/dpcd_reg.c new file +9 -0
+				return ERANGE;
+			}
+			dpcd->offset = temp;
+			break;
+		case 'v':
+			vflag = 'v';
+			temp = strtol(optarg, &endptr, 16);
+			if (strtol_err_util(endptr, &temp) || temp > 0xff) {
+				fprintf(stderr, "--value argument is invalid/negative/out-of-

range\n");

Quoted 15 lines
tools/dpcd_reg.c new file +15 -0
+				print_usage();
+				return ERANGE;
+			}
+			dpcd->val = temp;
+			break;
+		/* Command parsing */
+		case 1:
+			if (strcmp(optarg, "read") == 0) {
+				temp = READ;
+			} else if (strcmp(optarg, "write") == 0) {
+				temp = WRITE;
+				dpcd->file_op = O_WRONLY;
+			} else {
+				fprintf(stderr, "Unrecognized command\n");
+				print_usage();

tools/dpcd_reg.c line 161

Quoted 10 lines
tools/dpcd_reg.c new file +10 -0
+				return EXIT_FAILURE;
+			}
+			dpcd->cmd = temp;
+			break;
+		case ':':
+			fprintf(stderr, "Option -%c requires an argument\n",
+				 optopt);
+			print_usage();
+			return EXIT_FAILURE;
+		case '?':

This is not needed.

Quoted 31 lines
tools/dpcd_reg.c new file +31 -0
+		default:
+			fprintf(stderr, "Argument of  -%c is invalid\n", optopt);
+			print_usage();
+			return EXIT_FAILURE;
+		}
+	}
+
+	if ((dpcd->count + dpcd->offset) > (MAX_DP_OFFSET + 1)) {
+		fprintf(stderr, "Out of bounds. Count + Offset <= 0x100000\n");
+		return ERANGE;
+	}
+
+	if ((dpcd->cmd == WRITE) && (vflag != 'v')) {
+		fprintf(stderr, "Write value is missing\n");
+		print_usage();
+		return EXIT_FAILURE;
+	}
+
+	return EXIT_SUCCESS;
+}
+
+static int dpcd_read(int fd, uint32_t offset, size_t count)
+{
+	int ret = EXIT_SUCCESS, pret, i;
+	uint8_t *buf = calloc(count, sizeof(uint8_t));
+
+	if (!buf) {
+		fprintf(stderr, "Can't allocate read buffer\n");
+		return ENOMEM;
+	}
+

tools/dpcd_reg.c line 202

Quoted 10 lines
tools/dpcd_reg.c new file +10 -0
+	pret = pread(fd, buf, count, offset);
+	if (pret < 0) {
+		fprintf(stderr, "Failed to read - %s\n", strerror(errno));
+		ret = errno;
+		goto out;
+	}
+
+	if (pret < count) {
+		printf("Read %u byte(s), expected %zu bytes, starting at"
+			" offset %x\n\n", pret, count, offset);

You could ignore the 80 character limit and write the whole string in one line.
And also

		ret = EXIT_FAILURE;
		print to stderr?

tools/dpcd_reg.c line 205

Quoted 3 lines
tools/dpcd_reg.c new file +3 -0
+	}
+
+	printf("0x: ");
	printf("0x%x:", offset);

With the offset added,

I'll leave it up to you to fix other nits.

Quoted 8 lines
tools/dpcd_reg.c new file +8 -0
+	for (i = 0; i < pret; i++)
+		printf(" %02x", *(buf + i));
+	printf("\n");
+
+out:
+	free(buf);
+	return ret;
+}

tools/dpcd_reg.c line 223

Quoted 10 lines
tools/dpcd_reg.c new file +10 -0
+
+static int dpcd_write(int fd, uint32_t offset, uint8_t val)
+{
+	int ret = EXIT_SUCCESS, pret;
+
+	pret = pwrite(fd, (const void *)&val, sizeof(uint8_t), offset);
+	if (pret < 0) {
+		fprintf(stderr, "Failed to write - %s\n", strerror(errno));
+		ret = errno;
+	} else if (!pret) {

I prefer comparison to 0 in this case as the value is numerical.

	ret = EXIT_FAILURE;

tools/dpcd_reg.c line 224

Quoted 1 line
tools/dpcd_reg.c new file +1 -0
+		printf("Zero bytes were written\n");

stderr here too?

Quoted 28 lines
tools/dpcd_reg.c new file +28 -0
+	}
+
+	return ret;
+}
+
+int main(int argc, char **argv)
+{
+	char dev_name[20];
+	int ret, fd;
+
+	struct dpcd_data dpcd = {
+		.devid = 0,
+		.file_op = O_RDONLY,
+		.offset = 0x0,
+		.cmd = INVALID,
+		.count = 1,
+	};
+
+	if((ret = parse_opts(&dpcd, argc, argv)) != EXIT_SUCCESS)
+		return ret;
+
+	snprintf(dev_name, strlen(aux_dev) + 4, "%s%d", aux_dev, dpcd.devid);
+
+	fd = open(dev_name, dpcd.file_op);
+	if (fd < 0) {
+		fprintf(stderr, "Failed to open %s aux device - error: %s\n", dev_name,
+			strerror(errno));
+		return errno;

tools/dpcd_reg.c line 262

Quoted 10 lines
tools/dpcd_reg.c new file +10 -0
+	}
+
+	switch (dpcd.cmd) {
+	case READ:
+		ret = dpcd_read(fd, dpcd.offset, dpcd.count);
+		break;
+	case WRITE:
+		ret = dpcd_write(fd, dpcd.offset, dpcd.val);
+		break;
+	case INVALID:

Just the default is sufficient.

Quoted 16 lines
tools/dpcd_reg.c new file +11 -0
+	default:
+		fprintf(stderr, "Please specify a command: read/write.\n");
+		print_usage();
+		ret = EXIT_FAILURE;
+		break;
+	}
+
+	close(fd);
+
+	return ret;
+}
tools/meson.build +0 -0
index e4517d66..79f36aa9 100644

tools/meson.build tools_progs = [ · line 41

Quoted 6 lines
tools/meson.build +1 -0
@@ -36,6 +36,7 @@ tools_progs = [
 	'intel_watermark',
 	'intel_gem_info',
 	'intel_gvtg_test',
+	'dpcd_reg',
 ]
 tool_deps = igt_deps

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev