[PATCH 0/2] Fix OV02C10 camera color and stability issues

[PATCH 0/2] Fix OV02C10 camera color and stability issues

[Saikiran]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 3517 bytes --]

This series fixes critical color rendition and stability issues with the
OV02C10 camera sensor on Snapdragon X Elite platforms, specifically tested
on the Lenovo Yoga Slim 7x (x1e80100).

Relationship to other patch series:
------------------------------------
This is part of a coordinated effort to fully enable the OV02C10 camera
on Snapdragon X Elite:

1. **Device tree enablement** (by Aleksandrs Vinarskis - Unknown):
   - Fix RGB camera regulator supplies (avdd/dvdd/dovdd)
   - Enable camera privacy indicator LED
   - To be submitted to devicetree@vger.kernel.org

2. **Driver fixes for color and basic stability** (this series):
   - Fix incorrect Bayer pattern causing green tint
   - Fix race condition causing crashes on removal
   - Fix reset timing causing initialization failures

3. **Driver fixes for power cycling stability** (submitted Jan 24):
   - Fix qcom-camss pipeline lock leak
   - Add error checking in disable_streams
   - Enforce cool-down period to prevent brownout
   - Submitted separately, currently under review

All three series are complementary and address different aspects of
camera functionality. This series (2) can be reviewed independently
and benefits all OV02C10 platforms, not just x1e80100.

Issues fixed in this series:
-----------------------------
1. Incorrect Bayer pattern (SBGGR10 → SGRBG10)
   - Symptom: Severe green color tint in all captured images
   - Impact: Camera produces completely unusable images
   - Cause: Driver reports wrong pixel format to userspace
   - Testing: Compared all 4 Bayer patterns, only SGRBG10 produces
              natural colors on this hardware

2. Race condition in ov02c10_remove()
   - Symptom: Kernel oops with "Execute from non-executable memory"
   - Impact: System crashes when camera application closes or sensor removed
   - Cause: v4l2_ctrl_handler and media_entity freed before device powered off
   - Trigger: Concurrent userspace access during removal (PipeWire/WirePlumber)

3. Insufficient reset timing margins
   - Symptom: "master 1 queue 0 timeout" CCI I2C errors during initialization
   - Impact: Camera fails to initialize intermittently (10-20% failure rate)
   - Cause: Sensor microcontroller needs longer boot time than driver provides
   - Fix: Assert reset 2ms→5ms, post-reset delay 5ms→20ms

Without these fixes, even with correct power management, the camera produces
green-tinted unusable images and crashes during normal operations.

Testing:
--------
All patches tested together on Linux 6.19-rc5:
- libcamera/qcam: Color accuracy verification with test patterns
- Browser WebRTC: Vivaldi, Firefox, Brave camera permission flows
- PipeWire/WirePlumber: System integration testing
- Stress testing: 100+ camera open/close/remove cycles

Hardware tested:
- Lenovo Yoga Slim 7x (Snapdragon X Elite x1e80100)
- OmniVision OV02C10 2MP RGB camera sensor

Note on testing order:
The complete camera stack requires all patches, but these driver fixes
were validated by:
1. Applying Aleksandrs' DTS patches locally (hardware enablement)
2. Testing this series (fixes green tint and crashes)
3. Testing with brownout series (prevents power cycling issues)

Result: Fully functional camera with natural colors and stable operation.

Saikiran (2):
  media: i2c: ov02c10: Fix incorrect Bayer pattern to SGRBG10
  media: i2c: ov02c10: Fix race condition in remove and relax reset
    timings

 drivers/media/i2c/ov02c10.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

--
2.51.0

[PATCH] media: i2c: ov02c10: Fix incorrect Bayer pattern to SGRBG10

[Saikiran]

The OV02C10 driver incorrectly reports SBGGR10 Bayer pattern, but
the actual sensor hardware outputs SGRBG10 (Green-Red-Blue-Green).
This mismatch causes severe green color tint in all captured images.

The correct pattern was determined through:
1. Testing on Lenovo Yoga Slim 7x (Snapdragon X Elite)
2. Comparison of all four Bayer patterns (SBGGR10, SGBRG10, SGRBG10, SRGGB10)
3. Visual verification with libcamera/qcam showing natural colors with SGRBG10

Without this fix, the camera produces unusable images with overwhelming
green tint regardless of lighting conditions or camera application used.

Tested-on: Lenovo Yoga Slim 7x (Snapdragon X Elite)
Signed-off-by: Saikiran <bjsaikiran@gmail.com>
---
 drivers/media/i2c/ov02c10.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index f156f647081f..cf93d36032e1 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -577,7 +577,7 @@ static void ov02c10_update_pad_format(const struct ov02c10_mode *mode,
 {
 	fmt->width = mode->width;
 	fmt->height = mode->height;
-	fmt->code = MEDIA_BUS_FMT_SBGGR10_1X10;
+	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
 	fmt->field = V4L2_FIELD_NONE;
 }
 
@@ -732,7 +732,7 @@ static int ov02c10_enum_mbus_code(struct v4l2_subdev *sd,
 	if (code->index > 0)
 		return -EINVAL;
 
-	code->code = MEDIA_BUS_FMT_SBGGR10_1X10;
+	code->code = MEDIA_BUS_FMT_SGRBG10_1X10;
 
 	return 0;
 }
@@ -744,7 +744,7 @@ static int ov02c10_enum_frame_size(struct v4l2_subdev *sd,
 	if (fse->index >= ARRAY_SIZE(supported_modes))
 		return -EINVAL;
 
-	if (fse->code != MEDIA_BUS_FMT_SBGGR10_1X10)
+	if (fse->code != MEDIA_BUS_FMT_SGRBG10_1X10)
 		return -EINVAL;
 
 	fse->min_width = supported_modes[fse->index].width;
-- 
2.51.0

Re: [PATCH] media: i2c: ov02c10: Fix incorrect Bayer pattern to SGRBG10

[Hans de Goede]

Hi,

On 24-Jan-26 08:43, Saikiran wrote:
> The OV02C10 driver incorrectly reports SBGGR10 Bayer pattern, but
> the actual sensor hardware outputs SGRBG10 (Green-Red-Blue-Green).
> This mismatch causes severe green color tint in all captured images.
> 
> The correct pattern was determined through:
> 1. Testing on Lenovo Yoga Slim 7x (Snapdragon X Elite)
> 2. Comparison of all four Bayer patterns (SBGGR10, SGBRG10, SGRBG10, SRGGB10)
> 3. Visual verification with libcamera/qcam showing natural colors with SGRBG10
> 
> Without this fix, the camera produces unusable images with overwhelming
> green tint regardless of lighting conditions or camera application used.
> 
> Tested-on: Lenovo Yoga Slim 7x (Snapdragon X Elite)
> Signed-off-by: Saikiran <bjsaikiran@gmail.com>

this is already fixed, see:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/media/i2c/ov02c10.c?id=905120d7470e5ed79d59b61ef6aa13344ffca229

NACK (to avoid this regressing again)

Regards,

Hans




> ---
>  drivers/media/i2c/ov02c10.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
> index f156f647081f..cf93d36032e1 100644
> --- a/drivers/media/i2c/ov02c10.c
> +++ b/drivers/media/i2c/ov02c10.c
> @@ -577,7 +577,7 @@ static void ov02c10_update_pad_format(const struct ov02c10_mode *mode,
>  {
>  	fmt->width = mode->width;
>  	fmt->height = mode->height;
> -	fmt->code = MEDIA_BUS_FMT_SBGGR10_1X10;
> +	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
>  	fmt->field = V4L2_FIELD_NONE;
>  }
>  
> @@ -732,7 +732,7 @@ static int ov02c10_enum_mbus_code(struct v4l2_subdev *sd,
>  	if (code->index > 0)
>  		return -EINVAL;
>  
> -	code->code = MEDIA_BUS_FMT_SBGGR10_1X10;
> +	code->code = MEDIA_BUS_FMT_SGRBG10_1X10;
>  
>  	return 0;
>  }
> @@ -744,7 +744,7 @@ static int ov02c10_enum_frame_size(struct v4l2_subdev *sd,
>  	if (fse->index >= ARRAY_SIZE(supported_modes))
>  		return -EINVAL;
>  
> -	if (fse->code != MEDIA_BUS_FMT_SBGGR10_1X10)
> +	if (fse->code != MEDIA_BUS_FMT_SGRBG10_1X10)
>  		return -EINVAL;
>  
>  	fse->min_width = supported_modes[fse->index].width;

[PATCH] media: i2c: ov02c10: Fix race condition in remove and relax reset timings

[Saikiran]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 2187 bytes --]

The ov02c10_remove() function has a race condition where v4l2_ctrl_handler
and media_entity resources are freed before the device is powered off.
If userspace (e.g., PipeWire/WirePlumber) accesses the device during
removal, this causes a Use-After-Free leading to kernel oops with
"Execute from non-executable memory" errors.

Fix by reordering cleanup: disable runtime PM and power off the device
BEFORE freeing v4l2_ctrl_handler and media_entity resources.

Additionally, relax reset timings to prevent CCI I2C timeout errors.
The sensor microcontroller occasionally fails to boot within the
original timing windows, causing "master 1 queue 0 timeout" errors:
- Assert reset delay: 2ms → 5ms
- Post-reset boot delay: 5ms → 20ms

These two fixes address different but related stability issues that
manifest during camera initialization and removal.

Tested-on: Lenovo Yoga Slim 7x (Snapdragon X Elite)
Signed-off-by: Saikiran <bjsaikiran@gmail.com>
---
 drivers/media/i2c/ov02c10.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index cf93d36032e1..b86cae3d2b74 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -692,9 +692,9 @@ static int ov02c10_power_on(struct device *dev)
 
 	if (ov02c10->reset) {
 		/* Assert reset for at least 2ms on back to back off-on */
-		usleep_range(2000, 2200);
+		usleep_range(5000, 5500);
 		gpiod_set_value_cansleep(ov02c10->reset, 0);
-		usleep_range(5000, 5100);
+		usleep_range(20000, 21000);
 	}
 
 	return 0;
@@ -864,14 +864,14 @@ static void ov02c10_remove(struct i2c_client *client)
 	struct ov02c10 *ov02c10 = to_ov02c10(sd);
 
 	v4l2_async_unregister_subdev(sd);
-	v4l2_subdev_cleanup(sd);
-	media_entity_cleanup(&sd->entity);
-	v4l2_ctrl_handler_free(sd->ctrl_handler);
 	pm_runtime_disable(ov02c10->dev);
 	if (!pm_runtime_status_suspended(ov02c10->dev)) {
 		ov02c10_power_off(ov02c10->dev);
 		pm_runtime_set_suspended(ov02c10->dev);
 	}
+	v4l2_subdev_cleanup(sd);
+	media_entity_cleanup(&sd->entity);
+	v4l2_ctrl_handler_free(sd->ctrl_handler);
 }
 
 static int ov02c10_probe(struct i2c_client *client)
-- 
2.51.0

Re: [PATCH] media: i2c: ov02c10: Fix race condition in remove and relax reset timings

[Hans de Goede]

Hi,

On 24-Jan-26 08:43, Saikiran wrote:
> The ov02c10_remove() function has a race condition where v4l2_ctrl_handler
> and media_entity resources are freed before the device is powered off.
> If userspace (e.g., PipeWire/WirePlumber) accesses the device during
> removal, this causes a Use-After-Free leading to kernel oops with
> "Execute from non-executable memory" errors.
> 
> Fix by reordering cleanup: disable runtime PM and power off the device
> BEFORE freeing v4l2_ctrl_handler and media_entity resources.
> 
> Additionally, relax reset timings to prevent CCI I2C timeout errors.
> The sensor microcontroller occasionally fails to boot within the
> original timing windows, causing "master 1 queue 0 timeout" errors:
> - Assert reset delay: 2ms → 5ms
> - Post-reset boot delay: 5ms → 20ms
> 
> These two fixes address different but related stability issues that
> manifest during camera initialization and removal.
> 
> Tested-on: Lenovo Yoga Slim 7x (Snapdragon X Elite)
> Signed-off-by: Saikiran <bjsaikiran@gmail.com>

Please split this into 2 separate patches, 1 for the reset
timing change and one for moving the cleanup to after
the poweroff.

Regards,

Hans



> ---
>  drivers/media/i2c/ov02c10.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
> index cf93d36032e1..b86cae3d2b74 100644
> --- a/drivers/media/i2c/ov02c10.c
> +++ b/drivers/media/i2c/ov02c10.c
> @@ -692,9 +692,9 @@ static int ov02c10_power_on(struct device *dev)
>  
>  	if (ov02c10->reset) {
>  		/* Assert reset for at least 2ms on back to back off-on */
> -		usleep_range(2000, 2200);
> +		usleep_range(5000, 5500);
>  		gpiod_set_value_cansleep(ov02c10->reset, 0);
> -		usleep_range(5000, 5100);
> +		usleep_range(20000, 21000);
>  	}
>  
>  	return 0;
> @@ -864,14 +864,14 @@ static void ov02c10_remove(struct i2c_client *client)
>  	struct ov02c10 *ov02c10 = to_ov02c10(sd);
>  
>  	v4l2_async_unregister_subdev(sd);
> -	v4l2_subdev_cleanup(sd);
> -	media_entity_cleanup(&sd->entity);
> -	v4l2_ctrl_handler_free(sd->ctrl_handler);
>  	pm_runtime_disable(ov02c10->dev);
>  	if (!pm_runtime_status_suspended(ov02c10->dev)) {
>  		ov02c10_power_off(ov02c10->dev);
>  		pm_runtime_set_suspended(ov02c10->dev);
>  	}
> +	v4l2_subdev_cleanup(sd);
> +	media_entity_cleanup(&sd->entity);
> +	v4l2_ctrl_handler_free(sd->ctrl_handler);
>  }
>  
>  static int ov02c10_probe(struct i2c_client *client)