[libcamera-devel,04/11] Adds metadata for the ov8858, which the PinePhone Pro uses.
diff mbox series

Message ID 20221024055543.116040-5-nicholas@rothemail.net
State Superseded
Headers show
Series
  • [libcamera-devel,01/11] Fixes Bug 156, which breaks libcamera on Android < 12.
Related show

Commit Message

Nicolas Dufresne via libcamera-devel Oct. 24, 2022, 5:55 a.m. UTC 
From: Nicholas Roth <nicholas@rothemail.net>

---
 src/ipa/libipa/camera_sensor_helper.cpp    | 11 +++++++++++
 src/libcamera/camera_sensor_properties.cpp | 14 ++++++++++++++
 2 files changed, 25 insertions(+)

Comments

Laurent Pinchart Oct. 24, 2022, 1:51 p.m. UTC | #1 
Hi Nicholas,

Thank you for the patch.

On Mon, Oct 24, 2022 at 12:55:36AM -0500, Nicholas Roth via libcamera-devel wrote:
> From: Nicholas Roth <nicholas@rothemail.net>
> 
> ---
>  src/ipa/libipa/camera_sensor_helper.cpp    | 11 +++++++++++
>  src/libcamera/camera_sensor_properties.cpp | 14 ++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index 35056bec..1d9a45a7 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -476,6 +476,17 @@ public:
>  };
>  REGISTER_CAMERA_SENSOR_HELPER("ov5693", CameraSensorHelperOv5693)
>  
> +class CameraSensorHelperOv8858 : public CameraSensorHelper
> +{
> +public:
> +	CameraSensorHelperOv8858()
> +	{
> +		gainType_ = AnalogueGainLinear;
> +		gainConstants_.linear = { 1, 0, 0, 128 };

Looking at
http://www.ahdsensor.com/uploadfile/202008/55322e75316871.pdf, this
doesn't seem to match the description of the gain controls in table 5-8.
In "real gain" mode, the gain registers seem to be expressed in Q4.4
format.

> +	}
> +};
> +REGISTER_CAMERA_SENSOR_HELPER("m00_f_ov8858", CameraSensorHelperOv8858)
> +
>  class CameraSensorHelperOv8865 : public CameraSensorHelper
>  {
>  public:
> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
> index e5f27f06..d0757c15 100644
> --- a/src/libcamera/camera_sensor_properties.cpp
> +++ b/src/libcamera/camera_sensor_properties.cpp
> @@ -146,6 +146,20 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				 */
>  			},
>  		} },
> +		{ "m00_f_ov8858", {

I'm surprised by the name here. Is this from the mainline driver ?

> +			.unitCellSize = { 1200, 1200 },
> +			.testPatternModes = {
> +				{ controls::draft::TestPatternModeOff, 0 },
> +				{ controls::draft::TestPatternModeColorBars, 1 },
> +				/*
> +				 * No best corresponding test pattern for:
> +				 * 1: "Vertical Color Bar Type 1",
> +				 * 2: "Vertical Color Bar Type 2",
> +				 * 3: "Vertical Color Bar Type 3",
> +				 * 4: "Vertical Color Bar Type 4"
> +				 */
> +			},
> +		} },
>  		{ "ov8865", {
>  			.unitCellSize = { 1400, 1400 },
>  			.testPatternModes = {
Nicholas Roth Oct. 27, 2022, 4:10 a.m. UTC | #2 
> I'm surprised by the name here. Is this from the mainline driver ?
Me too. It is what libcamera reports while I'm running the latest 5.x
mainline kernel pushed to Arch/Manjaro as of 4 days ago.

> Looking at
> http://www.ahdsensor.com/uploadfile/202008/55322e75316871.pdf, this
> doesn't seem to match the description of the gain controls in table 5-8.
> In "real gain" mode, the gain registers seem to be expressed in Q4.4
> format.
I copy-pasted to get this working and forgot to come back! Fixed based on
the datasheet you linked by changing 128 to 16, which is the gain control
reported by the datasheet. See forthcoming commit and commit message for
details.

-Nicholas

On Mon, Oct 24, 2022 at 8:52 AM Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> Hi Nicholas,
>
> Thank you for the patch.
>
> On Mon, Oct 24, 2022 at 12:55:36AM -0500, Nicholas Roth via
> libcamera-devel wrote:
> > From: Nicholas Roth <nicholas@rothemail.net>
> >
> > ---
> >  src/ipa/libipa/camera_sensor_helper.cpp    | 11 +++++++++++
> >  src/libcamera/camera_sensor_properties.cpp | 14 ++++++++++++++
> >  2 files changed, 25 insertions(+)
> >
> > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp
> b/src/ipa/libipa/camera_sensor_helper.cpp
> > index 35056bec..1d9a45a7 100644
> > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > @@ -476,6 +476,17 @@ public:
> >  };
> >  REGISTER_CAMERA_SENSOR_HELPER("ov5693", CameraSensorHelperOv5693)
> >
> > +class CameraSensorHelperOv8858 : public CameraSensorHelper
> > +{
> > +public:
> > +     CameraSensorHelperOv8858()
> > +     {
> > +             gainType_ = AnalogueGainLinear;
> > +             gainConstants_.linear = { 1, 0, 0, 128 };
>
> Looking at
> http://www.ahdsensor.com/uploadfile/202008/55322e75316871.pdf, this
> doesn't seem to match the description of the gain controls in table 5-8.
> In "real gain" mode, the gain registers seem to be expressed in Q4.4
> format.
>
> > +     }
> > +};
> > +REGISTER_CAMERA_SENSOR_HELPER("m00_f_ov8858", CameraSensorHelperOv8858)
> > +
> >  class CameraSensorHelperOv8865 : public CameraSensorHelper
> >  {
> >  public:
> > diff --git a/src/libcamera/camera_sensor_properties.cpp
> b/src/libcamera/camera_sensor_properties.cpp
> > index e5f27f06..d0757c15 100644
> > --- a/src/libcamera/camera_sensor_properties.cpp
> > +++ b/src/libcamera/camera_sensor_properties.cpp
> > @@ -146,6 +146,20 @@ const CameraSensorProperties
> *CameraSensorProperties::get(const std::string &sen
> >                                */
> >                       },
> >               } },
> > +             { "m00_f_ov8858", {
>
> I'm surprised by the name here. Is this from the mainline driver ?
>
> > +                     .unitCellSize = { 1200, 1200 },
> > +                     .testPatternModes = {
> > +                             { controls::draft::TestPatternModeOff, 0 },
> > +                             {
> controls::draft::TestPatternModeColorBars, 1 },
> > +                             /*
> > +                              * No best corresponding test pattern for:
> > +                              * 1: "Vertical Color Bar Type 1",
> > +                              * 2: "Vertical Color Bar Type 2",
> > +                              * 3: "Vertical Color Bar Type 3",
> > +                              * 4: "Vertical Color Bar Type 4"
> > +                              */
> > +                     },
> > +             } },
> >               { "ov8865", {
> >                       .unitCellSize = { 1400, 1400 },
> >                       .testPatternModes = {
>
> --
> Regards,
>
> Laurent Pinchart
>

Patch
diff mbox series 

diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
index 35056bec..1d9a45a7 100644
--- a/src/ipa/libipa/camera_sensor_helper.cpp
+++ b/src/ipa/libipa/camera_sensor_helper.cpp
@@ -476,6 +476,17 @@  public:
 };
 REGISTER_CAMERA_SENSOR_HELPER("ov5693", CameraSensorHelperOv5693)
 
+class CameraSensorHelperOv8858 : public CameraSensorHelper
+{
+public:
+	CameraSensorHelperOv8858()
+	{
+		gainType_ = AnalogueGainLinear;
+		gainConstants_.linear = { 1, 0, 0, 128 };
+	}
+};
+REGISTER_CAMERA_SENSOR_HELPER("m00_f_ov8858", CameraSensorHelperOv8858)
+
 class CameraSensorHelperOv8865 : public CameraSensorHelper
 {
 public:
diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
index e5f27f06..d0757c15 100644
--- a/src/libcamera/camera_sensor_properties.cpp
+++ b/src/libcamera/camera_sensor_properties.cpp
@@ -146,6 +146,20 @@  const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
 				 */
 			},
 		} },
+		{ "m00_f_ov8858", {
+			.unitCellSize = { 1200, 1200 },
+			.testPatternModes = {
+				{ controls::draft::TestPatternModeOff, 0 },
+				{ controls::draft::TestPatternModeColorBars, 1 },
+				/*
+				 * No best corresponding test pattern for:
+				 * 1: "Vertical Color Bar Type 1",
+				 * 2: "Vertical Color Bar Type 2",
+				 * 3: "Vertical Color Bar Type 3",
+				 * 4: "Vertical Color Bar Type 4"
+				 */
+			},
+		} },
 		{ "ov8865", {
 			.unitCellSize = { 1400, 1400 },
 			.testPatternModes = {