[igt-dev] [PATCH i-g-t 0/2] Move dpcd_(read/write) to igt_dpcd

[igt-dev] [PATCH i-g-t 0/2] Move dpcd_(read/write) to igt_dpcd

[Kunal Joshi]

tests can use dpcd_read and dpcd_write for many use cases
one such use case is reading crc present at below address

DP_TEST_CRC_R_CR	0x240
DP_TEST_CRC_G_Y		0x242
DP_TEST_CRC_B_CB	0x244


Kunal Joshi (2):
  lib/igt_dpcd: move dpcd_read and dpcd_write to lib
  lib/meson.build: Add igt_dpcd for compilation

 lib/igt_dpcd.c   | 47 +++++++++++++++++++++++++++++++++++++
 lib/igt_dpcd.h   | 19 +++++++++++++++
 lib/meson.build  |  5 ++++
 tools/dpcd_reg.c | 60 +-----------------------------------------------
 4 files changed, 72 insertions(+), 59 deletions(-)
 create mode 100644 lib/igt_dpcd.c
 create mode 100644 lib/igt_dpcd.h

-- 
2.25.1

[igt-dev] [PATCH i-g-t 1/2] lib/igt_dpcd: move dpcd_read and dpcd_write to lib

[Kunal Joshi]

moved dpcd_read and dpcd_write to lib/igt_dpcd

Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 lib/igt_dpcd.c   | 47 +++++++++++++++++++++++++++++++++++++
 lib/igt_dpcd.h   | 19 +++++++++++++++
 tools/dpcd_reg.c | 60 +-----------------------------------------------
 3 files changed, 67 insertions(+), 59 deletions(-)
 create mode 100644 lib/igt_dpcd.c
 create mode 100644 lib/igt_dpcd.h

diff --git a/lib/igt_dpcd.c b/lib/igt_dpcd.c
new file mode 100644
index 000000000..eecc13b15
--- /dev/null
+++ b/lib/igt_dpcd.c
@@ -0,0 +1,47 @@
+#include "igt_dpcd.h"
+
+int dpcd_read(int fd, uint32_t offset, size_t count)
+{
+    int ret = EXIT_SUCCESS;
+    uint8_t *buf = calloc(count, sizeof(uint8_t));
+    ssize_t bytes_read;
+
+    if (!buf) {
+        fprintf(stderr, "Can't allocate read buffer\n");
+        return ENOMEM;
+    }
+
+    bytes_read = pread(fd, buf, count, offset);
+
+    if (bytes_read < 0) {
+        fprintf(stderr, "Failed to read - %s\n", strerror(errno));
+        ret = errno;
+    } else {
+        printf("0x%04x: ", offset);
+        for (ssize_t i = 0; i < bytes_read; i++) {
+            printf(" %02x", buf[i]);
+        }
+        printf("\n");
+    }
+
+    free(buf);
+    return ret;
+}
+
+int dpcd_write(int fd, uint32_t offset, uint8_t val)
+{
+    int ret = EXIT_SUCCESS;
+    ssize_t bytes_written;
+
+    bytes_written = pwrite(fd, &val, sizeof(uint8_t), offset);
+
+    if (bytes_written < 0) {
+        fprintf(stderr, "Failed to write - %s\n", strerror(errno));
+        ret = errno;
+    } else if (bytes_written == 0) {
+        fprintf(stderr, "Zero bytes were written\n");
+        ret = EXIT_FAILURE;
+    }
+
+    return ret;
+}
diff --git a/lib/igt_dpcd.h b/lib/igt_dpcd.h
new file mode 100644
index 000000000..b88e34e01
--- /dev/null
+++ b/lib/igt_dpcd.h
@@ -0,0 +1,19 @@
+#ifndef IGT_DPCD_H
+#define IGT_DPCD_H
+
+#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>
+#include <stdint.h>
+
+int dpcd_read(int fd, uint32_t offset, size_t count);
+int dpcd_write(int fd, uint32_t offset, uint8_t val);
+
+#endif /* IGT_DPCD_H */
diff --git a/tools/dpcd_reg.c b/tools/dpcd_reg.c
index 2761168d0..b7454cca1 100644
--- a/tools/dpcd_reg.c
+++ b/tools/dpcd_reg.c
@@ -25,16 +25,7 @@
  * and write, so CONFIG_DRM_DP_AUX_DEV needs to be set.
  */
 
-#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>
+#include "igt_dpcd.h"
 
 #define MAX_DP_OFFSET	0xfffff
 #define DRM_AUX_MINORS	256
@@ -214,55 +205,6 @@ static int parse_opts(struct dpcd_data *dpcd, int argc, char **argv)
 	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;
-	}
-
-	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) {
-		fprintf(stderr,
-			"Read %u byte(s), expected %zu bytes, starting at offset %x\n\n", pret, count, offset);
-		ret = EXIT_FAILURE;
-	}
-
-	printf("0x%04x: ", offset);
-	for (i = 0; i < pret; i++)
-		printf(" %02x", *(buf + i));
-	printf("\n");
-
-out:
-	free(buf);
-	return ret;
-}
-
-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 == 0) {
-		fprintf(stderr, "Zero bytes were written\n");
-		ret = EXIT_FAILURE;
-	}
-
-	return ret;
-}
-
 static int dpcd_dump(int fd)
 {
 	size_t count;
-- 
2.25.1

[igt-dev] [PATCH i-g-t 2/2] lib/meson.build: Add igt_dpcd for compilation

[Kunal Joshi]

Added igt_dpcd in meson.build for compilation

Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 lib/meson.build | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/meson.build b/lib/meson.build
index a7bccafc3..5e62d3eda 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -24,6 +24,7 @@ lib_sources = [
 	'igt_device_scan.c',
 	'igt_drm_clients.h',
 	'igt_drm_fdinfo.c',
+	'igt_dpcd.c',
 	'igt_aux.c',
 	'igt_gt.c',
 	'igt_halffloat.c',
@@ -254,6 +255,10 @@ lib_igt_perf_build = static_library('igt_perf',
 lib_igt_perf = declare_dependency(link_with : lib_igt_perf_build,
 				  include_directories : inc)
 
+lib_igt_dpcd = static_library('igt_dpcd',
+	['igt_dpcd.c'],
+	include_directories : inc)
+
 scan_dep = [
 	glib,
 	libudev,
-- 
2.25.1

[igt-dev] [PATCH i-g-t 1/2] lib/igt_dpcd: move dpcd_read and dpcd_write to lib

[Kunal Joshi]

moved dpcd_read and dpcd_write to lib/igt_dpcd

Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_dpcd.c   | 47 +++++++++++++++++++++++++++++++++++++
 lib/igt_dpcd.h   | 19 +++++++++++++++
 tools/dpcd_reg.c | 60 +-----------------------------------------------
 3 files changed, 67 insertions(+), 59 deletions(-)
 create mode 100644 lib/igt_dpcd.c
 create mode 100644 lib/igt_dpcd.h

diff --git a/lib/igt_dpcd.c b/lib/igt_dpcd.c
new file mode 100644
index 000000000..eecc13b15
--- /dev/null
+++ b/lib/igt_dpcd.c
@@ -0,0 +1,47 @@
+#include "igt_dpcd.h"
+
+int dpcd_read(int fd, uint32_t offset, size_t count)
+{
+    int ret = EXIT_SUCCESS;
+    uint8_t *buf = calloc(count, sizeof(uint8_t));
+    ssize_t bytes_read;
+
+    if (!buf) {
+        fprintf(stderr, "Can't allocate read buffer\n");
+        return ENOMEM;
+    }
+
+    bytes_read = pread(fd, buf, count, offset);
+
+    if (bytes_read < 0) {
+        fprintf(stderr, "Failed to read - %s\n", strerror(errno));
+        ret = errno;
+    } else {
+        printf("0x%04x: ", offset);
+        for (ssize_t i = 0; i < bytes_read; i++) {
+            printf(" %02x", buf[i]);
+        }
+        printf("\n");
+    }
+
+    free(buf);
+    return ret;
+}
+
+int dpcd_write(int fd, uint32_t offset, uint8_t val)
+{
+    int ret = EXIT_SUCCESS;
+    ssize_t bytes_written;
+
+    bytes_written = pwrite(fd, &val, sizeof(uint8_t), offset);
+
+    if (bytes_written < 0) {
+        fprintf(stderr, "Failed to write - %s\n", strerror(errno));
+        ret = errno;
+    } else if (bytes_written == 0) {
+        fprintf(stderr, "Zero bytes were written\n");
+        ret = EXIT_FAILURE;
+    }
+
+    return ret;
+}
diff --git a/lib/igt_dpcd.h b/lib/igt_dpcd.h
new file mode 100644
index 000000000..b88e34e01
--- /dev/null
+++ b/lib/igt_dpcd.h
@@ -0,0 +1,19 @@
+#ifndef IGT_DPCD_H
+#define IGT_DPCD_H
+
+#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>
+#include <stdint.h>
+
+int dpcd_read(int fd, uint32_t offset, size_t count);
+int dpcd_write(int fd, uint32_t offset, uint8_t val);
+
+#endif /* IGT_DPCD_H */
diff --git a/tools/dpcd_reg.c b/tools/dpcd_reg.c
index 2761168d0..b7454cca1 100644
--- a/tools/dpcd_reg.c
+++ b/tools/dpcd_reg.c
@@ -25,16 +25,7 @@
  * and write, so CONFIG_DRM_DP_AUX_DEV needs to be set.
  */
 
-#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>
+#include "igt_dpcd.h"
 
 #define MAX_DP_OFFSET	0xfffff
 #define DRM_AUX_MINORS	256
@@ -214,55 +205,6 @@ static int parse_opts(struct dpcd_data *dpcd, int argc, char **argv)
 	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;
-	}
-
-	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) {
-		fprintf(stderr,
-			"Read %u byte(s), expected %zu bytes, starting at offset %x\n\n", pret, count, offset);
-		ret = EXIT_FAILURE;
-	}
-
-	printf("0x%04x: ", offset);
-	for (i = 0; i < pret; i++)
-		printf(" %02x", *(buf + i));
-	printf("\n");
-
-out:
-	free(buf);
-	return ret;
-}
-
-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 == 0) {
-		fprintf(stderr, "Zero bytes were written\n");
-		ret = EXIT_FAILURE;
-	}
-
-	return ret;
-}
-
 static int dpcd_dump(int fd)
 {
 	size_t count;
-- 
2.25.1

Re: [igt-dev] [PATCH i-g-t 1/2] lib/igt_dpcd: move dpcd_read and dpcd_write to lib

[Sharma, Swati2]

Hi Kunal,

Please find review comments inline.

On 26-Sep-23 1:57 PM, Kunal Joshi wrote:
> moved dpcd_read and dpcd_write to lib/igt_dpcd
> 
> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
>   lib/igt_dpcd.c   | 47 +++++++++++++++++++++++++++++++++++++
>   lib/igt_dpcd.h   | 19 +++++++++++++++
>   tools/dpcd_reg.c | 60 +-----------------------------------------------
>   3 files changed, 67 insertions(+), 59 deletions(-)
>   create mode 100644 lib/igt_dpcd.c
>   create mode 100644 lib/igt_dpcd.h
> 
> diff --git a/lib/igt_dpcd.c b/lib/igt_dpcd.c
> new file mode 100644
> index 000000000..eecc13b15
> --- /dev/null
> +++ b/lib/igt_dpcd.c
> @@ -0,0 +1,47 @@

Please add copyright.

> +#include "igt_dpcd.h"
> +
> +int dpcd_read(int fd, uint32_t offset, size_t count)

For the func() add comment defining parameters and return type.
Comment should start as
	/**
          *
	 */
Then only it will be visible here 
https://drm.pages.freedesktop.org/igt-gpu-tools/

> +{
> +    int ret = EXIT_SUCCESS;
> +    uint8_t *buf = calloc(count, sizeof(uint8_t));
> +    ssize_t bytes_read;
> +
> +    if (!buf) {
> +        fprintf(stderr, "Can't allocate read buffer\n");
> +        return ENOMEM;
> +    }
> +
> +    bytes_read = pread(fd, buf, count, offset);
> +
> +    if (bytes_read < 0) {
> +        fprintf(stderr, "Failed to read - %s\n", strerror(errno));
> +        ret = errno;
> +    } else {
> +        printf("0x%04x: ", offset);
> +        for (ssize_t i = 0; i < bytes_read; i++) {
> +            printf(" %02x", buf[i]);
> +        }
> +        printf("\n");
> +    }
> +
> +    free(buf);
> +    return ret;
> +}
> +
> +int dpcd_write(int fd, uint32_t offset, uint8_t val)
> +{
> +    int ret = EXIT_SUCCESS;
> +    ssize_t bytes_written;
> +
> +    bytes_written = pwrite(fd, &val, sizeof(uint8_t), offset);
> +
> +    if (bytes_written < 0) {
> +        fprintf(stderr, "Failed to write - %s\n", strerror(errno));
> +        ret = errno;
> +    } else if (bytes_written == 0) {
> +        fprintf(stderr, "Zero bytes were written\n");
> +        ret = EXIT_FAILURE;
> +    }
> +
> +    return ret;
> +}
> diff --git a/lib/igt_dpcd.h b/lib/igt_dpcd.h
> new file mode 100644
> index 000000000..b88e34e01
> --- /dev/null
> +++ b/lib/igt_dpcd.h
> @@ -0,0 +1,19 @@

Add copyright.

> +#ifndef IGT_DPCD_H
> +#define IGT_DPCD_H
> +
> +#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>
> +#include <stdint.h>

Sort headers in alphabetical order.

> +
> +int dpcd_read(int fd, uint32_t offset, size_t count);
> +int dpcd_write(int fd, uint32_t offset, uint8_t val);
> +
> +#endif /* IGT_DPCD_H */
> diff --git a/tools/dpcd_reg.c b/tools/dpcd_reg.c
> index 2761168d0..b7454cca1 100644
> --- a/tools/dpcd_reg.c
> +++ b/tools/dpcd_reg.c
> @@ -25,16 +25,7 @@
>    * and write, so CONFIG_DRM_DP_AUX_DEV needs to be set.
>    */
>   
> -#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>
> +#include "igt_dpcd.h"
>   
>   #define MAX_DP_OFFSET	0xfffff
>   #define DRM_AUX_MINORS	256
> @@ -214,55 +205,6 @@ static int parse_opts(struct dpcd_data *dpcd, int argc, char **argv)
>   	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;
> -	}
> -
> -	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) {
> -		fprintf(stderr,
> -			"Read %u byte(s), expected %zu bytes, starting at offset %x\n\n", pret, count, offset);
> -		ret = EXIT_FAILURE;
> -	}
> -
> -	printf("0x%04x: ", offset);
> -	for (i = 0; i < pret; i++)
> -		printf(" %02x", *(buf + i));
> -	printf("\n");
> -
> -out:
> -	free(buf);
> -	return ret;
> -}
> -
> -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 == 0) {
> -		fprintf(stderr, "Zero bytes were written\n");
> -		ret = EXIT_FAILURE;
> -	}
> -
> -	return ret;
> -}
> -
>   static int dpcd_dump(int fd)
>   {
>   	size_t count;