fpsensor: update interface

Update the FP MCU interface to include a few convenient diagnostics
functions for factory testing.

It's mostly backward compatible, but overall this interface never
shipped in anything, so not a big deal regardless.

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>

BRANCH=none
BUG=b:71986991
TEST=ectool --name=cros_fp fpinfo && ectool --name=cros_fp fpcheckpixels
CQ-DEPEND=CL:*546799

Change-Id: Ic641f891ace02d79af9339cf6cb59a2960e506a7
Reviewed-on: https://chromium-review.googlesource.com/873924
Commit-Ready: Vincent Palatin <vpalatin@chromium.org>
Tested-by: Vincent Palatin <vpalatin@chromium.org>
Reviewed-by: Shawn N <shawnn@chromium.org>

4 files changed, 244 insertions, 47 deletions

diff --git a/common/fpsensor.c b/common/fpsensor.c
index f2b7e07d00..b1016e217d 100644
--- a/common/fpsensor.c
+++ b/common/fpsensor.c
@@ -33,6 +33,11 @@ static uint8_t fp_buffer[FP_SENSOR_IMAGE_SIZE];
 #define CPRINTF(format, args...) cprintf(CC_FP, format, ## args)
 #define CPRINTS(format, args...) cprints(CC_FP, format, ## args)
 
+/* raw image offset inside the acquired frame */
+#ifndef FP_SENSOR_IMAGE_OFFSET
+#define FP_SENSOR_IMAGE_OFFSET 0
+#endif
+
 /* Events for the FPSENSOR task */
 #define TASK_EVENT_SENSOR_IRQ     TASK_EVENT_CUSTOM(1)
 #define TASK_EVENT_UPDATE_CONFIG  TASK_EVENT_CUSTOM(2)
@@ -53,6 +58,15 @@ void fps_event(enum gpio_signal signal)
 	task_set_event(TASK_ID_FPSENSOR, TASK_EVENT_SENSOR_IRQ, 0);
 }
 
+static inline int is_test_capture(uint32_t mode)
+{
+	int capture_type = FP_CAPTURE_TYPE(mode);
+
+	return (mode & FP_MODE_CAPTURE)
+		&& (capture_type == FP_CAPTURE_PATTERN0
+		    || capture_type == FP_CAPTURE_PATTERN1);
+}
+
 static void send_mkbp_event(uint32_t event)
 {
 	atomic_or(&fp_events, event);
@@ -80,9 +94,16 @@ void fp_task(void)
 
 		if (evt & TASK_EVENT_UPDATE_CONFIG) {
 			gpio_disable_interrupt(GPIO_FPS_INT);
-			if (sensor_mode & FP_MODE_ANY_DETECT_FINGER)
+			if (is_test_capture(sensor_mode)) {
+				fp_sensor_acquire_image_with_mode(fp_buffer,
+					FP_CAPTURE_TYPE(sensor_mode));
+				sensor_mode &= ~FP_MODE_CAPTURE;
+				send_mkbp_event(EC_MKBP_FP_IMAGE_READY);
+				continue;
+			} else if (sensor_mode & FP_MODE_ANY_DETECT_FINGER) {
 				/* wait for a finger on the sensor */
 				fp_sensor_configure_detect();
+			}
 			if (sensor_mode & FP_MODE_DEEPSLEEP)
 				/* Shutdown the sensor */
 				fp_sensor_low_power();
@@ -113,7 +134,9 @@ void fp_task(void)
 
 			if (st == FINGER_PRESENT &&
 			    sensor_mode & FP_MODE_CAPTURE) {
-				int res = fp_sensor_acquire_image(fp_buffer);
+				int res = fp_sensor_acquire_image_with_mode(
+						fp_buffer,
+						FP_CAPTURE_TYPE(sensor_mode));
 
 				if (!res) {
 					sensor_mode &= ~FP_MODE_CAPTURE;
@@ -220,12 +243,16 @@ static int fp_command_frame(struct host_cmd_handler_args *args)
 {
 	const struct ec_params_fp_frame *params = args->params;
 	void *out = args->response;
+	uint32_t offset = params->offset;
 
-	if (params->offset + params->size > sizeof(fp_buffer) ||
+	if (FP_CAPTURE_TYPE(sensor_mode) != FP_CAPTURE_VENDOR_FORMAT)
+		offset += FP_SENSOR_IMAGE_OFFSET;
+
+	if (offset + params->size > sizeof(fp_buffer) ||
 	    params->size > args->response_max)
 		return EC_RES_INVALID_PARAM;
 
-	memcpy(out, fp_buffer + params->offset, params->size);
+	memcpy(out, fp_buffer + offset, params->size);
 
 	args->response_size = params->size;
 	return EC_RES_SUCCESS;
@@ -235,11 +262,6 @@ DECLARE_HOST_COMMAND(EC_CMD_FP_FRAME, fp_command_frame, EC_VER_MASK(0));
 #ifdef CONFIG_CMD_FPSENSOR_DEBUG
 /* --- Debug console commands --- */
 
-/* raw image offset inside the acquired frame */
-#ifndef FP_SENSOR_IMAGE_OFFSET
-#define FP_SENSOR_IMAGE_OFFSET 0
-#endif
-
 /*
  * Send the current Fingerprint buffer to the host
  * it is formatted as an 8-bpp PGM ASCII file.
@@ -286,9 +308,19 @@ static void upload_pgm_image(uint8_t *frame)
 int command_fptest(int argc, char **argv)
 {
 	int tries = 200;
+	int capture_type = FP_CAPTURE_SIMPLE_IMAGE;
+
+	if (argc >= 2) {
+		char *e;
+
+		capture_type = strtoi(argv[1], &e, 0);
+		if (*e || capture_type < 0 || capture_type > 3)
+			return EC_ERROR_PARAM1;
+	}
 
 	ccprintf("Waiting for finger ...\n");
-	sensor_mode = FP_MODE_CAPTURE;
+	sensor_mode = FP_MODE_CAPTURE |
+		(capture_type << FP_MODE_CAPTURE_TYPE_SHIFT);
 	task_set_event(TASK_ID_FPSENSOR, TASK_EVENT_UPDATE_CONFIG, 0);
 
 	while (tries--) {
diff --git a/include/ec_commands.h b/include/ec_commands.h
index e1573e85b9..10ac439215 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -4674,22 +4674,45 @@ struct __ec_align2 ec_params_fp_sensor_config {
 #define FP_MODE_FINGER_UP     (1<<2)
 /* Capture the current finger image */
 #define FP_MODE_CAPTURE       (1<<3)
+/* Capture types defined in bits [30..28] */
+#define FP_MODE_CAPTURE_TYPE_SHIFT 28
+#define FP_MODE_CAPTURE_TYPE_MASK  0x7
+/* Full blown vendor-defined capture (produces 'frame_size' bytes) */
+#define FP_CAPTURE_VENDOR_FORMAT 0
+/* Simple raw image capture (produces width x height x bpp bits) */
+#define FP_CAPTURE_SIMPLE_IMAGE  1
+/* Self test pattern (e.g. checkerboard) */
+#define FP_CAPTURE_PATTERN0      2
+/* Self test pattern (e.g. inverted checkerboard) */
+#define FP_CAPTURE_PATTERN1      3
+/* Extracts the capture type from the sensor 'mode' word */
+#define FP_CAPTURE_TYPE(mode) (((mode) >> FP_MODE_CAPTURE_TYPE_SHIFT) \
+					& FP_MODE_CAPTURE_TYPE_MASK)
 /* special value: don't change anything just read back current mode */
 #define FP_MODE_DONT_CHANGE   (1<<31)
 
 struct __ec_align4 ec_params_fp_mode {
 	uint32_t mode; /* as defined by FP_MODE_ constants */
-	/* TBD */
 };
 
 struct __ec_align4 ec_response_fp_mode {
 	uint32_t mode; /* as defined by FP_MODE_ constants */
-	/* TBD */
 };
 
 /* Retrieve Fingerprint sensor information */
 #define EC_CMD_FP_INFO 0x0403
 
+/* Number of dead pixels detected on the last maintenance */
+#define FP_ERROR_DEAD_PIXELS(errors) ((errors) & 0x3FF)
+/* No interrupt from the sensor */
+#define FP_ERROR_NO_IRQ    (1 << 12)
+/* SPI communication error */
+#define FP_ERROR_SPI_COMM  (1 << 13)
+/* Invalid sensor Hardware ID */
+#define FP_ERROR_BAD_HWID  (1 << 14)
+/* Sensor initialization failed */
+#define FP_ERROR_INIT_FAIL (1 << 15)
+
 struct __ec_align2 ec_response_fp_info {
 	/* Sensor identification */
 	uint32_t vendor_id;
@@ -4702,9 +4725,10 @@ struct __ec_align2 ec_response_fp_info {
 	uint16_t width;
 	uint16_t height;
 	uint16_t bpp;
+	uint16_t errors; /* see FP_ERROR_ flags above */
 };
 
-/* Get the last captured finger frame: TODO: will be AES-encrypted */
+/* Get the last captured finger frame */
 #define EC_CMD_FP_FRAME 0x0404
 
 struct __ec_align4 ec_params_fp_frame {
diff --git a/include/fpsensor.h b/include/fpsensor.h
index 66b00b50d9..3a77803223 100644
--- a/include/fpsensor.h
+++ b/include/fpsensor.h
@@ -84,4 +84,13 @@ enum finger_state fp_sensor_finger_status(void);
 #define FP_SENSOR_LOW_SENSOR_COVERAGE 3
 int fp_sensor_acquire_image(uint8_t *image_data);
 
+/*
+ * Acquires a fingerprint image with specific capture mode.
+ *
+ * Same as the fp_sensor_acquire_image function above,
+ * excepted 'mode' can be set to one of the FP_CAPTURE_ constants
+ * to get a specific image type (e.g. a pattern) rather than the default one.
+ */
+int fp_sensor_acquire_image_with_mode(uint8_t *image_data, int mode);
+
 #endif /* __CROS_EC_FPSENSOR_H */
diff --git a/util/ectool.c b/util/ectool.c
index 97cfaa8262..3cb86daf22 100644
--- a/util/ectool.c
+++ b/util/ectool.c
@@ -115,6 +115,8 @@ const char help_str[] =
 	"      Reads from EC flash to a file\n"
 	"  flashwrite <offset> <infile>\n"
 	"      Writes to EC flash from a file\n"
+	"  fpcheckpixels\n"
+	"      Count the number of dead pixels on the sensor\n"
 	"  fpframe\n"
 	"      Retrieve the finger image as a PGM image\n"
 	"  fpinfo\n"
@@ -1079,16 +1081,155 @@ int cmd_rwsig_action(int argc, char *argv[])
 	return ec_command(EC_CMD_RWSIG_ACTION, 0, &req, sizeof(req), NULL, 0);
 }
 
+static void *fp_download_frame(struct ec_response_fp_info *info)
+{
+	struct ec_params_fp_frame p;
+	int rv = 0;
+	size_t stride, size;
+	void *buffer;
+	uint8_t *ptr;
+
+	rv = ec_command(EC_CMD_FP_INFO, 0, NULL, 0, info, sizeof(*info));
+	if (rv < 0)
+		return NULL;
+
+	stride = (size_t)info->width * info->bpp/8;
+	if (stride > ec_max_insize) {
+		fprintf(stderr, "Not implemented for line size %zu B "
+			"(%u pixels) > EC transfer size %d\n",
+			stride, info->width, ec_max_insize);
+		return NULL;
+	}
+	if (info->bpp != 8) {
+		fprintf(stderr, "Not implemented for BPP = %d != 8\n",
+			info->bpp);
+		return NULL;
+	}
+
+	size = stride * info->height;
+	buffer = malloc(size);
+	if (!buffer) {
+		fprintf(stderr, "Cannot allocate memory for the image\n");
+		return NULL;
+	}
+
+	ptr = buffer;
+	p.offset = 0;
+	p.size = stride;
+	while (size) {
+		rv = ec_command(EC_CMD_FP_FRAME, 0, &p, sizeof(p),
+				ptr, stride);
+		if (rv < 0) {
+			free(buffer);
+			return NULL;
+		}
+		p.offset += stride;
+		size -= stride;
+		ptr += stride;
+	}
+
+	return buffer;
+}
+
+static int fp_pattern_frame(int capt_type, const char *title, int inv)
+{
+	struct ec_response_fp_info info;
+	struct ec_params_fp_mode p;
+	struct ec_response_fp_mode r;
+	void *pattern;
+	uint8_t *ptr;
+	int rv;
+	int bad = 0;
+	int64_t cnt = 0, lo_sum = 0, hi_sum = 0;
+	int64_t lo_sum_squares = 0, hi_sum_squares = 0;
+	int x, y;
+
+	p.mode = FP_MODE_CAPTURE | (capt_type << FP_MODE_CAPTURE_TYPE_SHIFT);
+	rv = ec_command(EC_CMD_FP_MODE, 0, &p, sizeof(p), &r, sizeof(r));
+	if (rv < 0)
+		return -1;
+	/* ensure the capture has happened without using event support */
+	usleep(50000);
+	pattern = fp_download_frame(&info);
+	if (!pattern)
+		return -1;
+
+	ptr = pattern;
+	for (y = 0; y < info.height; y++)
+		for (x = 0; x < info.width; x++, ptr++) {
+			uint8_t v = *ptr;
+			int hi = !!(v & 128);
+
+			/*
+			 * Verify whether the captured image matches the expected
+			 * checkerboard pattern.
+			 */
+			if ((hi ^ inv) == ((x & 1) ^ (y & 1))) {
+				bad++;
+			} else {
+				/*
+				 * For all black pixels and all white pixels of
+				 * the checkerboard pattern, we will compute
+				 * their average and their variance in order to
+				 * have quality metrics later.
+				 * Do the sum and the sum of squares for each
+				 * category here, we will finalize the
+				 * computations outside of the loop.
+				 */
+				cnt++;
+				if (hi) {
+					hi_sum += v;
+					hi_sum_squares += v * v;
+				} else {
+					lo_sum += v;
+					lo_sum_squares += v * v;
+				}
+			}
+		}
+	printf("%s: bad %d\n", title, bad);
+	/*
+	 * For each category of pixels: black aka 'lo' and white aka 'hi',
+	 * the variance is: Avg[v^2] - Avg[v]^2
+	 * which is equivalent to  Sum(v^2) / cnt - Sum(v) * Sum(v) / cnt / cnt
+	 * where v is the pixel grayscale value.
+	 */
+	if (cnt)
+		printf("%s: distribution average %" PRId64 "/%" PRId64
+		       " variance %" PRId64 "/%" PRId64 "\n",
+		       title, lo_sum / cnt, hi_sum / cnt,
+		       (lo_sum_squares / cnt - lo_sum * lo_sum / cnt / cnt),
+		       (hi_sum_squares / cnt - hi_sum * hi_sum / cnt / cnt));
+	free(pattern);
+	return bad;
+}
+
+int cmd_fp_check_pixels(int argc, char *argv[])
+{
+	int bad0, bad1;
+
+	bad0 = fp_pattern_frame(FP_CAPTURE_PATTERN0, "Checkerboard", 0);
+	bad1 = fp_pattern_frame(FP_CAPTURE_PATTERN1, "Inv. Checkerboard", 1);
+	if (bad0 < 0 || bad1 < 0) {
+		fprintf(stderr, "Failed to acquire FP patterns\n");
+		return -1;
+	}
+	printf("Defects: dead %d (pattern0 %d pattern1 %d)\n",
+	       bad0 + bad1, bad0, bad1);
+	return 0;
+}
+
 int cmd_fp_mode(int argc, char *argv[])
 {
 	struct ec_params_fp_mode p;
 	struct ec_response_fp_mode r;
 	uint32_t mode = 0;
+	uint32_t capture_type = FP_CAPTURE_SIMPLE_IMAGE;
 	int i, rv;
 
 	if (argc == 1)
 		mode = FP_MODE_DONT_CHANGE;
 	for (i = 1; i < argc; i++) {
+		/* modes */
 		if (!strncmp(argv[i], "deepsleep", 9))
 			mode |= FP_MODE_DEEPSLEEP;
 		else if (!strncmp(argv[i], "fingerdown", 10))
@@ -1097,7 +1238,16 @@ int cmd_fp_mode(int argc, char *argv[])
 			mode |= FP_MODE_FINGER_UP;
 		else if (!strncmp(argv[i], "capture", 7))
 			mode |= FP_MODE_CAPTURE;
+		/* capture types */
+		else if (!strncmp(argv[i], "vendor", 6))
+			capture_type = FP_CAPTURE_VENDOR_FORMAT;
+		else if (!strncmp(argv[i], "pattern0", 8))
+			capture_type = FP_CAPTURE_PATTERN0;
+		else if (!strncmp(argv[i], "pattern1", 8))
+			capture_type = FP_CAPTURE_PATTERN1;
 	}
+	if (mode & FP_MODE_CAPTURE)
+		mode |= capture_type << FP_MODE_CAPTURE_TYPE_SHIFT;
 
 	p.mode = mode;
 	rv = ec_command(EC_CMD_FP_MODE, 0, &p, sizeof(p), &r, sizeof(r));
@@ -1129,6 +1279,12 @@ int cmd_fp_info(int argc, char *argv[])
 	printf("Fingerprint sensor: vendor %x product %x model %x version %x\n",
 		r.vendor_id, r.product_id, r.model_id, r.version);
 	printf("Image: size %dx%d %d bpp\n", r.width, r.height, r.bpp);
+	printf("Error flags: %s%s%s%s\nDead pixels: %u\n",
+	       r.errors & FP_ERROR_NO_IRQ ? "NO_IRQ " : "",
+	       r.errors & FP_ERROR_SPI_COMM ? "SPI_COMM " : "",
+	       r.errors & FP_ERROR_BAD_HWID ? "BAD_HWID " : "",
+	       r.errors & FP_ERROR_INIT_FAIL ? "INIT_FAIL " : "",
+	       FP_ERROR_DEAD_PIXELS(r.errors));
 
 	return 0;
 }
@@ -1136,50 +1292,25 @@ int cmd_fp_info(int argc, char *argv[])
 int cmd_fp_frame(int argc, char *argv[])
 {
 	struct ec_response_fp_info r;
-	struct ec_params_fp_frame p;
-	int rv = 0;
-	size_t stride, size;
-	uint8_t *buffer8 = ec_inbuf;
+	void *buffer = fp_download_frame(&r);
+	uint8_t *ptr = buffer;
+	int x, y;
 
-	rv = ec_command(EC_CMD_FP_INFO, 0, NULL, 0, &r, sizeof(r));
-	if (rv < 0)
-		return rv;
-
-	stride = (size_t)r.width * r.bpp/8;
-	if (stride > ec_max_insize) {
-		fprintf(stderr, "Not implemented for line size %zu B "
-			"(%u pixels) > EC transfer size %d\n",
-			stride, r.width, ec_max_insize);
-		return -1;
-	}
-	if (r.bpp != 8) {
-		fprintf(stderr, "Not implemented for BPP = %d != 8\n", r.bpp);
+	if (!buffer) {
+		fprintf(stderr, "Failed to get FP sensor frame\n");
 		return -1;
 	}
 
-	size = stride * r.height;
-
 	/* Print 8-bpp PGM ASCII header */
 	printf("P2\n%d %d\n%d\n", r.width, r.height, (1 << r.bpp) - 1);
 
-	p.offset = 0;
-	p.size = stride;
-	while (size) {
-		int x;
-
-		rv = ec_command(EC_CMD_FP_FRAME, 0, &p, sizeof(p),
-				ec_inbuf, stride);
-		if (rv < 0)
-			return rv;
-		p.offset += stride;
-		size -= stride;
-
-		for (x = 0; x < stride; x++)
-			printf("%d ", buffer8[x]);
+	for (y = 0; y < r.height; y++) {
+		for (x = 0; x < r.width; x++, ptr++)
+			printf("%d ", *ptr);
 		printf("\n");
 	}
 	printf("# END OF FILE\n");
-
+	free(buffer);
 	return 0;
 }
 
@@ -7498,6 +7629,7 @@ const struct command commands[] = {
 	{"flashspiinfo", cmd_flash_spi_info},
 	{"flashpd", cmd_flash_pd},
 	{"forcelidopen", cmd_force_lid_open},
+	{"fpcheckpixels", cmd_fp_check_pixels},
 	{"fpframe", cmd_fp_frame},
 	{"fpinfo", cmd_fp_info},
 	{"fpmode", cmd_fp_mode},