When the test-pattern control gets set to 0 (Disabled) 0 should be written to the test-pattern register, rather then doing nothing.
Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver") Cc: stable@vger.kernel.org Signed-off-by: Hans de Goede hansg@kernel.org --- drivers/media/i2c/ov01a10.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c index 0405ec7c75fd..733e5bf0180c 100644 --- a/drivers/media/i2c/ov01a10.c +++ b/drivers/media/i2c/ov01a10.c @@ -406,10 +406,8 @@ static int ov01a10_update_digital_gain(struct ov01a10 *ov01a10, u32 d_gain)
static int ov01a10_test_pattern(struct ov01a10 *ov01a10, u32 pattern) { - if (!pattern) - return 0; - - pattern = (pattern - 1) | OV01A10_TEST_PATTERN_ENABLE; + if (pattern) + pattern = (pattern - 1) | OV01A10_TEST_PATTERN_ENABLE;
return ov01a10_write_reg(ov01a10, OV01A10_REG_TEST_PATTERN, 1, pattern); }
Hans,
Thank you for the fix. Reviewed-by: Bingbu Cao bingbu.cao@intel.com
On 10/15/25 1:40 AM, Hans de Goede wrote:
When the test-pattern control gets set to 0 (Disabled) 0 should be written to the test-pattern register, rather then doing nothing.
Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver") Cc: stable@vger.kernel.org Signed-off-by: Hans de Goede hansg@kernel.org
drivers/media/i2c/ov01a10.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c index 0405ec7c75fd..733e5bf0180c 100644 --- a/drivers/media/i2c/ov01a10.c +++ b/drivers/media/i2c/ov01a10.c @@ -406,10 +406,8 @@ static int ov01a10_update_digital_gain(struct ov01a10 *ov01a10, u32 d_gain) static int ov01a10_test_pattern(struct ov01a10 *ov01a10, u32 pattern) {
- if (!pattern)
return 0;- pattern = (pattern - 1) | OV01A10_TEST_PATTERN_ENABLE;
- if (pattern)
pattern = (pattern - 1) | OV01A10_TEST_PATTERN_ENABLE;return ov01a10_write_reg(ov01a10, OV01A10_REG_TEST_PATTERN, 1, pattern); }
Hi Hans,
Thank you for the patches!
On Tue, Oct 14, 2025 at 07:40:14PM +0200, Hans de Goede wrote:
When the test-pattern control gets set to 0 (Disabled) 0 should be written to the test-pattern register, rather then doing nothing.
A small question: Do you see any difference between test_pattern 1 and test_pattern 2 or I did not look hard enough at the screen ?
Tested-by: Mehdi Djait mehdi.djait@linux.intel.com # Dell XPS 9315 Reviewed-by: Mehdi Djait mehdi.djait@linux.intel.com
Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver") Cc: stable@vger.kernel.org Signed-off-by: Hans de Goede hansg@kernel.org
Hi Mehdi,
Thank you for all the reviews and testing.
On 28-Oct-25 1:08 PM, Mehdi Djait wrote:
Hi Hans,
Thank you for the patches!
On Tue, Oct 14, 2025 at 07:40:14PM +0200, Hans de Goede wrote:
When the test-pattern control gets set to 0 (Disabled) 0 should be written to the test-pattern register, rather then doing nothing.
A small question: Do you see any difference between test_pattern 1 and test_pattern 2 or I did not look hard enough at the screen ?
IIRC the one has the colors fading (a bit) from left to right and the other from top to bottom ?
Regards,
Hans
Hi Hans,
On Tue, Oct 28, 2025 at 03:38:28PM +0100, Hans de Goede wrote:
Hi Mehdi,
Thank you for all the reviews and testing.
On 28-Oct-25 1:08 PM, Mehdi Djait wrote:
Hi Hans,
Thank you for the patches!
On Tue, Oct 14, 2025 at 07:40:14PM +0200, Hans de Goede wrote:
When the test-pattern control gets set to 0 (Disabled) 0 should be written to the test-pattern register, rather then doing nothing.
A small question: Do you see any difference between test_pattern 1 and test_pattern 2 or I did not look hard enough at the screen ?
IIRC the one has the colors fading (a bit) from left to right and the other from top to bottom ?
I see: 1 and 2 are the same ?! 3 fading from left -> right 4 fading from top -> bottom
-- Kind Regards Mehdi Djait
Hi Medhi,
On 28-Oct-25 4:38 PM, Mehdi Djait wrote:
Hi Hans,
On Tue, Oct 28, 2025 at 03:38:28PM +0100, Hans de Goede wrote:
Hi Mehdi,
Thank you for all the reviews and testing.
On 28-Oct-25 1:08 PM, Mehdi Djait wrote:
Hi Hans,
Thank you for the patches!
On Tue, Oct 14, 2025 at 07:40:14PM +0200, Hans de Goede wrote:
When the test-pattern control gets set to 0 (Disabled) 0 should be written to the test-pattern register, rather then doing nothing.
A small question: Do you see any difference between test_pattern 1 and test_pattern 2 or I did not look hard enough at the screen ?
IIRC the one has the colors fading (a bit) from left to right and the other from top to bottom ?
I see: 1 and 2 are the same ?! 3 fading from left -> right 4 fading from top -> bottom
That might very well be correct. Unfortunately I no longer have access to the Dell XPS 13 9320 I tested this on, so I cannot confirm.
I think I should squash the following fix into this one:
diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c index 6a1ab5fa70a2..1fe643cb1e6b 100644 --- a/drivers/media/i2c/ov01a10.c +++ b/drivers/media/i2c/ov01a10.c @@ -215,9 +215,8 @@ static const struct reg_sequence ov01a1s_regs[] = { static const char * const ov01a10_test_pattern_menu[] = { "Disabled", "Color Bar", + "Left-Right Darker Color Bar", "Top-Bottom Darker Color Bar", - "Right-Left Darker Color Bar", - "Color Bar type 4", };
static const s64 link_freq_menu_items[] = { @@ -318,7 +317,7 @@ static int ov01a10_update_digital_gain(struct ov01a10 *ov01a10, u32 d_gain) static int ov01a10_test_pattern(struct ov01a10 *ov01a10, u32 pattern) { if (pattern) - pattern = (pattern - 1) | OV01A10_TEST_PATTERN_ENABLE; + pattern |= OV01A10_TEST_PATTERN_ENABLE;
return cci_write(ov01a10->regmap, OV01A10_REG_TEST_PATTERN, pattern, NULL);
This skips setting 0 as the pattern, since 0 is the same as 1, removes the weird "Color Bar type 4" from the menu and fixes the order of the 2 fading controls.
Can you give this a test ?
Regards,
Hans
Hi Hans,
On Tue, Oct 28, 2025 at 04:52:54PM +0100, Hans de Goede wrote:
Hi Medhi,
On 28-Oct-25 4:38 PM, Mehdi Djait wrote:
Hi Hans,
On Tue, Oct 28, 2025 at 03:38:28PM +0100, Hans de Goede wrote:
Hi Mehdi,
Thank you for all the reviews and testing.
On 28-Oct-25 1:08 PM, Mehdi Djait wrote:
Hi Hans,
Thank you for the patches!
On Tue, Oct 14, 2025 at 07:40:14PM +0200, Hans de Goede wrote:
When the test-pattern control gets set to 0 (Disabled) 0 should be written to the test-pattern register, rather then doing nothing.
A small question: Do you see any difference between test_pattern 1 and test_pattern 2 or I did not look hard enough at the screen ?
IIRC the one has the colors fading (a bit) from left to right and the other from top to bottom ?
I see: 1 and 2 are the same ?! 3 fading from left -> right 4 fading from top -> bottom
That might very well be correct. Unfortunately I no longer have access to the Dell XPS 13 9320 I tested this on, so I cannot confirm.
I think I should squash the following fix into this one:
diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c index 6a1ab5fa70a2..1fe643cb1e6b 100644 --- a/drivers/media/i2c/ov01a10.c +++ b/drivers/media/i2c/ov01a10.c @@ -215,9 +215,8 @@ static const struct reg_sequence ov01a1s_regs[] = { static const char * const ov01a10_test_pattern_menu[] = { "Disabled", "Color Bar",
- "Left-Right Darker Color Bar", "Top-Bottom Darker Color Bar",
This should be changed to: "Bottom-Top Darker Color Bar"
- "Right-Left Darker Color Bar",
- "Color Bar type 4",
}; static const s64 link_freq_menu_items[] = { @@ -318,7 +317,7 @@ static int ov01a10_update_digital_gain(struct ov01a10 *ov01a10, u32 d_gain) static int ov01a10_test_pattern(struct ov01a10 *ov01a10, u32 pattern) { if (pattern)
pattern = (pattern - 1) | OV01A10_TEST_PATTERN_ENABLE;
pattern |= OV01A10_TEST_PATTERN_ENABLE;return cci_write(ov01a10->regmap, OV01A10_REG_TEST_PATTERN, pattern, NULL);
This skips setting 0 as the pattern, since 0 is the same as 1, removes the weird "Color Bar type 4" from the menu and fixes the order of the 2 fading controls.
Can you give this a test ?
It works as expected. Thank you for the patch
-- Kind Regards Mehdi Djait
linux-stable-mirror@lists.linaro.org
-
Bingbu Cao -
Hans de Goede -
Mehdi Djait