Skip to contentSkip to conversation

media: Use common test pattern menu entries

PATCH RESEND Superseded Sakari Ailus 2 participants 3 messages Nov 27, 2018

Subsystems (5)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Sakari Ailus

author maintainer
#0 lore raw

PATCH RESEND 1/1 media: Use common test pattern menu entries

While the test pattern menu itself is not standardised, many devices support the same test patterns. Aligning the menu entries helps the user space to use the interface, and adding macros for the menu entry strings helps to keep them aligned.

---
Fixed Andy's email.

 drivers/media/i2c/imx258.c             | 10 +++++-----
 drivers/media/i2c/imx319.c             | 10 +++++-----
 drivers/media/i2c/imx355.c             | 10 +++++-----
 drivers/media/i2c/ov2640.c             |  4 ++--
 drivers/media/i2c/smiapp/smiapp-core.c | 10 +++++-----
 include/uapi/linux/v4l2-controls.h     |  5 +++++
 6 files changed, 27 insertions(+), 22 deletions(-)
drivers/media/i2c/imx258.c +5 -5
index f86ae18bc104..c795d4c4c0e4 100644
@@ -498,11 +498,11 @@ static const struct imx258_reg mode_1048_780_regs[] = {
 };
 
 static const char * const imx258_test_pattern_menu[] = {
-	"Disabled",
-	"Solid Colour",
-	"Eight Vertical Colour Bars",
-	"Colour Bars With Fade to Grey",
-	"Pseudorandom Sequence (PN9)",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_SOLID_COLOUR,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY,
+	V4L2_TEST_PATTERN_PN9,
 };
 
 /* Configurations for supported link frequencies */
drivers/media/i2c/imx319.c +5 -5
index 17c2e4b41221..eddaf69a67b6 100644
@@ -1647,11 +1647,11 @@ static const struct imx319_reg mode_1280x720_regs[] = {
 };
 
 static const char * const imx319_test_pattern_menu[] = {
-	"Disabled",
-	"Solid Colour",
-	"Eight Vertical Colour Bars",
-	"Colour Bars With Fade to Grey",
-	"Pseudorandom Sequence (PN9)",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_SOLID_COLOUR,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY,
+	V4L2_TEST_PATTERN_PN9,
 };
 
 /* supported link frequencies */
drivers/media/i2c/imx355.c +5 -5
index bed293b60e50..824d07156f9c 100644
@@ -875,11 +875,11 @@ static const struct imx355_reg mode_820x616_regs[] = {
 };
 
 static const char * const imx355_test_pattern_menu[] = {
-	"Disabled",
-	"Solid Colour",
-	"Eight Vertical Colour Bars",
-	"Colour Bars With Fade to Grey",
-	"Pseudorandom Sequence (PN9)",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_SOLID_COLOUR,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY,
+	V4L2_TEST_PATTERN_PN9,
 };
 
 /* supported link frequencies */
drivers/media/i2c/ov2640.c +2 -2
index 5d2d6735cc78..507ec7176a7d 100644
@@ -707,8 +707,8 @@ static int ov2640_reset(struct i2c_client *client)
 }
 
 static const char * const ov2640_test_pattern_menu[] = {
-	"Disabled",
-	"Eight Vertical Colour Bars",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
 };
 
 /*
drivers/media/i2c/smiapp/smiapp-core.c +5 -5
index 58a45c353e27..f6a92b9f178c 100644
@@ -409,11 +409,11 @@ static void smiapp_update_mbus_formats(struct smiapp_sensor *sensor)
 }
 
 static const char * const smiapp_test_patterns[] = {
-	"Disabled",
-	"Solid Colour",
-	"Eight Vertical Colour Bars",
-	"Colour Bars With Fade to Grey",
-	"Pseudorandom Sequence (PN9)",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_SOLID_COLOUR,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY,
+	V4L2_TEST_PATTERN_PN9,
 };
 
 static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl)
include/uapi/linux/v4l2-controls.h +5 -0
index 998983a6e6b7..a74ff6f1ac88 100644
@@ -1014,6 +1014,11 @@ enum v4l2_jpeg_chroma_subsampling {
 #define V4L2_CID_LINK_FREQ			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 1)
 #define V4L2_CID_PIXEL_RATE			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 2)
 #define V4L2_CID_TEST_PATTERN			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
+#define V4L2_TEST_PATTERN_DISABLED		"Disabled"
+#define V4L2_TEST_PATTERN_SOLID_COLOUR		"Solid Colour"
+#define V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS		"Eight Vertical Colour Bars"
+#define V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY "Colour Bars With Fade to Grey"
+#define V4L2_TEST_PATTERN_PN9			"Pseudorandom Sequence (PN9)"
 #define V4L2_CID_DEINTERLACING_MODE		(V4L2_CID_IMAGE_PROC_CLASS_BASE + 4)
 #define V4L2_CID_DIGITAL_GAIN			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 5)
 
Signature · 1 line
2.11.0
#1 lore raw

Hi Sakari, Bingbu,

Quoted 121 lines · Sakari Ailus

While the test pattern menu itself is not standardised, many devices support the same test patterns. Aligning the menu entries helps the user space to use the interface, and adding macros for the menu entry strings helps to keep them aligned.

---
Fixed Andy's email.

 drivers/media/i2c/imx258.c             | 10 +++++-----
 drivers/media/i2c/imx319.c             | 10 +++++-----
 drivers/media/i2c/imx355.c             | 10 +++++-----
 drivers/media/i2c/ov2640.c             |  4 ++--
 drivers/media/i2c/smiapp/smiapp-core.c | 10 +++++-----
 include/uapi/linux/v4l2-controls.h     |  5 +++++
 6 files changed, 27 insertions(+), 22 deletions(-)
drivers/media/i2c/imx258.c +5 -5
index f86ae18bc104..c795d4c4c0e4 100644
@@ -498,11 +498,11 @@ static const struct imx258_reg mode_1048_780_regs[] = {
 };
 
 static const char * const imx258_test_pattern_menu[] = {
-	"Disabled",
-	"Solid Colour",
-	"Eight Vertical Colour Bars",
-	"Colour Bars With Fade to Grey",
-	"Pseudorandom Sequence (PN9)",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_SOLID_COLOUR,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY,
+	V4L2_TEST_PATTERN_PN9,
 };
 
 /* Configurations for supported link frequencies */
drivers/media/i2c/imx319.c +5 -5
index 17c2e4b41221..eddaf69a67b6 100644
@@ -1647,11 +1647,11 @@ static const struct imx319_reg mode_1280x720_regs[] = {
 };
 
 static const char * const imx319_test_pattern_menu[] = {
-	"Disabled",
-	"Solid Colour",
-	"Eight Vertical Colour Bars",
-	"Colour Bars With Fade to Grey",
-	"Pseudorandom Sequence (PN9)",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_SOLID_COLOUR,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY,
+	V4L2_TEST_PATTERN_PN9,
 };
 
 /* supported link frequencies */
drivers/media/i2c/imx355.c +5 -5
index bed293b60e50..824d07156f9c 100644
@@ -875,11 +875,11 @@ static const struct imx355_reg mode_820x616_regs[] = {
 };
 
 static const char * const imx355_test_pattern_menu[] = {
-	"Disabled",
-	"Solid Colour",
-	"Eight Vertical Colour Bars",
-	"Colour Bars With Fade to Grey",
-	"Pseudorandom Sequence (PN9)",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_SOLID_COLOUR,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY,
+	V4L2_TEST_PATTERN_PN9,
 };
 
 /* supported link frequencies */
drivers/media/i2c/ov2640.c +2 -2
index 5d2d6735cc78..507ec7176a7d 100644
@@ -707,8 +707,8 @@ static int ov2640_reset(struct i2c_client *client)
 }
 
 static const char * const ov2640_test_pattern_menu[] = {
-	"Disabled",
-	"Eight Vertical Colour Bars",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
 };
 
 /*
drivers/media/i2c/smiapp/smiapp-core.c +5 -5
index 58a45c353e27..f6a92b9f178c 100644
@@ -409,11 +409,11 @@ static void smiapp_update_mbus_formats(struct smiapp_sensor *sensor)
 }
 
 static const char * const smiapp_test_patterns[] = {
-	"Disabled",
-	"Solid Colour",
-	"Eight Vertical Colour Bars",
-	"Colour Bars With Fade to Grey",
-	"Pseudorandom Sequence (PN9)",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_SOLID_COLOUR,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY,
+	V4L2_TEST_PATTERN_PN9,
 };
 
 static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl)
include/uapi/linux/v4l2-controls.h +0 -0
index 998983a6e6b7..a74ff6f1ac88 100644

include/uapi/linux/v4l2-controls.h enum v4l2_jpeg_chroma_subsampling { · line 1020

Quoted 7 lines
include/uapi/linux/v4l2-controls.h +4 -0
@@ -1014,6 +1014,11 @@ enum v4l2_jpeg_chroma_subsampling {
 #define V4L2_CID_LINK_FREQ			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 1)
 #define V4L2_CID_PIXEL_RATE			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 2)
 #define V4L2_CID_TEST_PATTERN			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
+#define V4L2_TEST_PATTERN_DISABLED		"Disabled"
+#define V4L2_TEST_PATTERN_SOLID_COLOUR		"Solid Colour"
+#define V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS		"Eight Vertical Colour Bars"
+#define V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY "Colour Bars With Fade to Grey"

Bingbu, can you confirm the "Colour Bars With Fade to Grey" has 8 bars? And that they are vertical?

For the rest LGTM, I'll send a patch against IMX274 to use these defines as soon as they are committed.

Signature · 1 line
Luca
author maintainer
#2 lore raw
Quoted 118 lines · Luca Ceresoli

Hi Sakari, Bingbu,

Quoted 115 lines · Sakari Ailus

While the test pattern menu itself is not standardised, many devices support the same test patterns. Aligning the menu entries helps the user space to use the interface, and adding macros for the menu entry strings helps to keep them aligned.

---
Fixed Andy's email.

 drivers/media/i2c/imx258.c             | 10 +++++-----
 drivers/media/i2c/imx319.c             | 10 +++++-----
 drivers/media/i2c/imx355.c             | 10 +++++-----
 drivers/media/i2c/ov2640.c             |  4 ++--
 drivers/media/i2c/smiapp/smiapp-core.c | 10 +++++-----
 include/uapi/linux/v4l2-controls.h     |  5 +++++
 6 files changed, 27 insertions(+), 22 deletions(-)
drivers/media/i2c/imx258.c +5 -5
index f86ae18bc104..c795d4c4c0e4 100644
@@ -498,11 +498,11 @@ static const struct imx258_reg mode_1048_780_regs[] = {
 };
 
 static const char * const imx258_test_pattern_menu[] = {
-	"Disabled",
-	"Solid Colour",
-	"Eight Vertical Colour Bars",
-	"Colour Bars With Fade to Grey",
-	"Pseudorandom Sequence (PN9)",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_SOLID_COLOUR,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY,
+	V4L2_TEST_PATTERN_PN9,
 };
 
 /* Configurations for supported link frequencies */
drivers/media/i2c/imx319.c +5 -5
index 17c2e4b41221..eddaf69a67b6 100644
@@ -1647,11 +1647,11 @@ static const struct imx319_reg mode_1280x720_regs[] = {
 };
 
 static const char * const imx319_test_pattern_menu[] = {
-	"Disabled",
-	"Solid Colour",
-	"Eight Vertical Colour Bars",
-	"Colour Bars With Fade to Grey",
-	"Pseudorandom Sequence (PN9)",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_SOLID_COLOUR,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY,
+	V4L2_TEST_PATTERN_PN9,
 };
 
 /* supported link frequencies */
drivers/media/i2c/imx355.c +5 -5
index bed293b60e50..824d07156f9c 100644
@@ -875,11 +875,11 @@ static const struct imx355_reg mode_820x616_regs[] = {
 };
 
 static const char * const imx355_test_pattern_menu[] = {
-	"Disabled",
-	"Solid Colour",
-	"Eight Vertical Colour Bars",
-	"Colour Bars With Fade to Grey",
-	"Pseudorandom Sequence (PN9)",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_SOLID_COLOUR,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY,
+	V4L2_TEST_PATTERN_PN9,
 };
 
 /* supported link frequencies */
drivers/media/i2c/ov2640.c +2 -2
index 5d2d6735cc78..507ec7176a7d 100644
@@ -707,8 +707,8 @@ static int ov2640_reset(struct i2c_client *client)
 }
 
 static const char * const ov2640_test_pattern_menu[] = {
-	"Disabled",
-	"Eight Vertical Colour Bars",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
 };
 
 /*
drivers/media/i2c/smiapp/smiapp-core.c +5 -5
index 58a45c353e27..f6a92b9f178c 100644
@@ -409,11 +409,11 @@ static void smiapp_update_mbus_formats(struct smiapp_sensor *sensor)
 }
 
 static const char * const smiapp_test_patterns[] = {
-	"Disabled",
-	"Solid Colour",
-	"Eight Vertical Colour Bars",
-	"Colour Bars With Fade to Grey",
-	"Pseudorandom Sequence (PN9)",
+	V4L2_TEST_PATTERN_DISABLED,
+	V4L2_TEST_PATTERN_SOLID_COLOUR,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS,
+	V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY,
+	V4L2_TEST_PATTERN_PN9,
 };
 
Quoted 16 lines

include/uapi/linux/v4l2-controls.h enum v4l2_jpeg_chroma_subsampling { · line 1020

Quoted 13 lines
drivers/media/i2c/smiapp/smiapp-core.c +0 -0
 static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl)
include/uapi/linux/v4l2-controls.h +4 -0
index 998983a6e6b7..a74ff6f1ac88 100644
@@ -1014,6 +1014,11 @@ enum v4l2_jpeg_chroma_subsampling {
 #define V4L2_CID_LINK_FREQ			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 1)
 #define V4L2_CID_PIXEL_RATE			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 2)
 #define V4L2_CID_TEST_PATTERN			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
+#define V4L2_TEST_PATTERN_DISABLED		"Disabled"
+#define V4L2_TEST_PATTERN_SOLID_COLOUR		"Solid Colour"
+#define V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS		"Eight Vertical Colour Bars"
+#define V4L2_TEST_PATTERN_8_VERT_COLOUR_BARS_FADE_TO_GREY "Colour Bars With Fade to Grey"

Bingbu, can you confirm the "Colour Bars With Fade to Grey" has 8 bars? And that they are vertical?

Good question. While there are eight bars in the pattern, the pattern repeats if the image width is not divisible by 8. There could also be other number of bars, so perhaps removing the number from the naming could make sense.

Quoted 3 lines

For the rest LGTM, I'll send a patch against IMX274 to use these defines as soon as they are committed.

Signature · 2 lines
Sakari Ailus
[email protected]