Re: [RFC PATCH] media: ov5640: calculate PLL settings for modes
Hi Sam,
some comments and questions.
I still hope we find a way to merge our changes, but I need to
understand something before.
On Wed, Oct 17, 2018 at 10:31:48AM -0700, Sam Bobrowicz wrote:
> Remove the PLL settings from the register blobs and
> calculate them based on required clocks. This allows
> more mode and input clock (xclk) configurations.
>
> Also ensure that PCLK PERIOD register 0x4837 is set
> so that MIPI receivers are not broken by this patch.
>
> Last, a change to the init register blob that helps
> ensure the following DPHY spec requirement is met:
>
> MIN HS_ZERO + MIN HS_PREPARE > 145 + t_UI * 10
>
> Signed-off-by: Sam Bobrowicz
> ---
>
> This is a modified version of Maxime's patch that works on my platform.
> My platform is a dual-lane MIPI CSI2 module with xclk=24MHz connected
> to a Xilinx Zynq Ultrascale+.
>
> One issue I am currently experiencing with this patch is that some
> 15Hz framerates are not working. This seems to be due to the slower
> clocks which are generated, and may be caused by the large ADC clock
> to SCLK ratio. I will be exploring some fixes this weekend. Thoughts on
> this would be appreciated.
>
> I am submitting this so that it can be compared to Maxime's, which has
> been reported to not be functional on MIPI platforms. I do a number of
> things differently, and I hope that those which are useful will be
> integrated into his patch.
>
> I think this patch (or the modified version of Maxime's patch) should
> be tested under the following conditions:
>
> 1) MIPI mode, xclk=24MHz
> 2) MIPI mode, xclk!=24MHz
> 3) DVP mode
> 4) JPEG format
>
> I'm setup to test the first two, but don't have the hardware/software
> to test 3 and 4.
I could test with 1) and 2) with xvclk at 26MHz
>
> This patch is based on the current master of media_linux
> "media: ov5640: fix framerate update".
>
> drivers/media/i2c/ov5640.c | 332
> ++---
> 1 file changed, 281 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index eaefdb5..c076955 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -85,6 +85,7 @@
> #define OV5640_REG_POLARITY_CTRL00 0x4740
> #define OV5640_REG_MIPI_CTRL00 0x4800
> #define OV5640_REG_DEBUG_MODE0x4814
> +#define OV5640_REG_PCLK_PERIOD 0x4837
> #define OV5640_REG_ISP_FORMAT_MUX_CTRL 0x501f
> #define OV5640_REG_PRE_ISP_TEST_SET1 0x503d
> #define OV5640_REG_SDE_CTRL0 0x5580
> @@ -94,9 +95,6 @@
> #define OV5640_REG_SDE_CTRL5 0x5585
> #define OV5640_REG_AVG_READOUT 0x56a1
>
> -#define OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT 1
> -#define OV5640_SCLK_ROOT_DIVIDER_DEFAULT 2
> -
> enum ov5640_mode_id {
> OV5640_MODE_QCIF_176_144 = 0,
> OV5640_MODE_QVGA_320_240,
> @@ -171,6 +169,7 @@ struct reg_value {
> struct ov5640_mode_info {
> enum ov5640_mode_id id;
> enum ov5640_downsize_mode dn_mode;
> + bool scaler; /* Mode uses ISP scaler (reg 0x5001,BIT(5)=='1') */
> u32 hact;
> u32 htot;
> u32 vact;
> @@ -291,7 +290,7 @@ static const struct reg_value
> ov5640_init_setting_30fps_VGA[] = {
> {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
> {0x501f, 0x00, 0, 0}, {0x4713, 0x03, 0, 0}, {0x4407, 0x04, 0, 0},
> {0x440e, 0x00, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
> - {0x4837, 0x0a, 0, 0}, {0x3824, 0x02, 0, 0},
> + {0x3824, 0x02, 0, 0}, {0x482a, 0x06, 0, 0},
> {0x5000, 0xa7, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x5180, 0xff, 0, 0},
> {0x5181, 0xf2, 0, 0}, {0x5182, 0x00, 0, 0}, {0x5183, 0x14, 0, 0},
> {0x5184, 0x25, 0, 0}, {0x5185, 0x24, 0, 0}, {0x5186, 0x09, 0, 0},
> @@ -345,7 +344,7 @@ static const struct reg_value
> ov5640_init_setting_30fps_VGA[] = {
> };
>
> static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
> - {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
> + {0x3c07, 0x08, 0, 0},
> {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> {0x3814, 0x31, 0, 0},
> {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
> @@ -364,7 +363,7 @@ static const struct reg_value
> ov5640_setting_30fps_VGA_640_480[] = {
> };
>
> static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
> - {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
> + {0x3c07, 0x08, 0, 0},
> {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> {0x3814, 0x31, 0, 0},
> {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
> @@ -383,7 +382,7 @@ static const struct reg_value
> ov5640_setting_15fps_VGA_640_480[] = {
> };
>
> static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
> - {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
> + {0x3c07, 0x08, 0, 0},
> {0x3c09,
[RFC PATCH] media: ov5640: calculate PLL settings for modes
Remove the PLL settings from the register blobs and
calculate them based on required clocks. This allows
more mode and input clock (xclk) configurations.
Also ensure that PCLK PERIOD register 0x4837 is set
so that MIPI receivers are not broken by this patch.
Last, a change to the init register blob that helps
ensure the following DPHY spec requirement is met:
MIN HS_ZERO + MIN HS_PREPARE > 145 + t_UI * 10
Signed-off-by: Sam Bobrowicz
---
This is a modified version of Maxime's patch that works on my platform.
My platform is a dual-lane MIPI CSI2 module with xclk=24MHz connected
to a Xilinx Zynq Ultrascale+.
One issue I am currently experiencing with this patch is that some
15Hz framerates are not working. This seems to be due to the slower
clocks which are generated, and may be caused by the large ADC clock
to SCLK ratio. I will be exploring some fixes this weekend. Thoughts on
this would be appreciated.
I am submitting this so that it can be compared to Maxime's, which has
been reported to not be functional on MIPI platforms. I do a number of
things differently, and I hope that those which are useful will be
integrated into his patch.
I think this patch (or the modified version of Maxime's patch) should
be tested under the following conditions:
1) MIPI mode, xclk=24MHz
2) MIPI mode, xclk!=24MHz
3) DVP mode
4) JPEG format
I'm setup to test the first two, but don't have the hardware/software
to test 3 and 4.
This patch is based on the current master of media_linux
"media: ov5640: fix framerate update".
drivers/media/i2c/ov5640.c | 332 ++---
1 file changed, 281 insertions(+), 51 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index eaefdb5..c076955 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -85,6 +85,7 @@
#define OV5640_REG_POLARITY_CTRL00 0x4740
#define OV5640_REG_MIPI_CTRL00 0x4800
#define OV5640_REG_DEBUG_MODE 0x4814
+#define OV5640_REG_PCLK_PERIOD 0x4837
#define OV5640_REG_ISP_FORMAT_MUX_CTRL 0x501f
#define OV5640_REG_PRE_ISP_TEST_SET1 0x503d
#define OV5640_REG_SDE_CTRL0 0x5580
@@ -94,9 +95,6 @@
#define OV5640_REG_SDE_CTRL5 0x5585
#define OV5640_REG_AVG_READOUT 0x56a1
-#define OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT 1
-#define OV5640_SCLK_ROOT_DIVIDER_DEFAULT 2
-
enum ov5640_mode_id {
OV5640_MODE_QCIF_176_144 = 0,
OV5640_MODE_QVGA_320_240,
@@ -171,6 +169,7 @@ struct reg_value {
struct ov5640_mode_info {
enum ov5640_mode_id id;
enum ov5640_downsize_mode dn_mode;
+ bool scaler; /* Mode uses ISP scaler (reg 0x5001,BIT(5)=='1') */
u32 hact;
u32 htot;
u32 vact;
@@ -291,7 +290,7 @@ static const struct reg_value
ov5640_init_setting_30fps_VGA[] = {
{0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
{0x501f, 0x00, 0, 0}, {0x4713, 0x03, 0, 0}, {0x4407, 0x04, 0, 0},
{0x440e, 0x00, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
- {0x4837, 0x0a, 0, 0}, {0x3824, 0x02, 0, 0},
+ {0x3824, 0x02, 0, 0}, {0x482a, 0x06, 0, 0},
{0x5000, 0xa7, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x5180, 0xff, 0, 0},
{0x5181, 0xf2, 0, 0}, {0x5182, 0x00, 0, 0}, {0x5183, 0x14, 0, 0},
{0x5184, 0x25, 0, 0}, {0x5185, 0x24, 0, 0}, {0x5186, 0x09, 0, 0},
@@ -345,7 +344,7 @@ static const struct reg_value
ov5640_init_setting_30fps_VGA[] = {
};
static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
- {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+ {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -364,7 +363,7 @@ static const struct reg_value
ov5640_setting_30fps_VGA_640_480[] = {
};
static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
- {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+ {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -383,7 +382,7 @@ static const struct reg_value
ov5640_setting_15fps_VGA_640_480[] = {
};
static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
- {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+ {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -399,11 +398,10 @@ static const struct reg_value
ov5640_setting_30fps_XGA_1024_768[] = {
{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
