| Message ID | 20210525055614.13417-1-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 63c3c545926d7eb9af8c03930e79df8c2b46a8e9 |
| Headers | show |
| Series |
|
| Related | show |
Hi Jean-Michel, Thank you for the patch. On Tue, May 25, 2021 at 07:56:14AM +0200, jeanmichel.hautbois@ideasonboard.com wrote: > From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > The new sensor database has introduced several sensors, but ov5693 is > missing. > It is used on most MS Surface tablets, add it to the database. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/camera_sensor_properties.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > index 6ded31dc..2a6e97f7 100644 > --- a/src/libcamera/camera_sensor_properties.cpp > +++ b/src/libcamera/camera_sensor_properties.cpp > @@ -54,6 +54,9 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { "ov13858", { > .unitCellSize = { 1120, 1120 }, > } }, > + { "ov5693", { > + .unitCellSize = { 1400, 1400 }, > + } }, > }; > > const auto it = sensorProps.find(sensor);
Hi JM Thanks for the patch On 5/25/21 11:26 AM, jeanmichel.hautbois@ideasonboard.com wrote: > From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > The new sensor database has introduced several sensors, but ov5693 is > missing. > It is used on most MS Surface tablets, add it to the database. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/libcamera/camera_sensor_properties.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > index 6ded31dc..2a6e97f7 100644 > --- a/src/libcamera/camera_sensor_properties.cpp > +++ b/src/libcamera/camera_sensor_properties.cpp > @@ -54,6 +54,9 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { "ov13858", { > .unitCellSize = { 1120, 1120 }, > } }, > + { "ov5693", { > + .unitCellSize = { 1400, 1400 }, > + } }, > }; > > const auto it = sensorProps.find(sensor);
Hi Jm On Tue, May 25, 2021 at 07:56:14AM +0200, jeanmichel.hautbois@ideasonboard.com wrote: > From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > The new sensor database has introduced several sensors, but ov5693 is > missing. > It is used on most MS Surface tablets, add it to the database. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/libcamera/camera_sensor_properties.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > index 6ded31dc..2a6e97f7 100644 > --- a/src/libcamera/camera_sensor_properties.cpp > +++ b/src/libcamera/camera_sensor_properties.cpp > @@ -54,6 +54,9 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { "ov13858", { > .unitCellSize = { 1120, 1120 }, > } }, > + { "ov5693", { > + .unitCellSize = { 1400, 1400 }, Don't have a datasheet to very, so I assume that's correct! Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> thanks j > + } }, > }; > > const auto it = sensorProps.find(sensor); > -- > 2.30.2 >
On Tue, May 25, 2021 at 7:37 PM Jacopo Mondi <jacopo@jmondi.org> wrote: > > Hi Jm > > On Tue, May 25, 2021 at 07:56:14AM +0200, jeanmichel.hautbois@ideasonboard.com wrote: > > From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > > The new sensor database has introduced several sensors, but ov5693 is > > missing. > > It is used on most MS Surface tablets, add it to the database. > > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > --- > > src/libcamera/camera_sensor_properties.cpp | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > > index 6ded31dc..2a6e97f7 100644 > > --- a/src/libcamera/camera_sensor_properties.cpp > > +++ b/src/libcamera/camera_sensor_properties.cpp > > @@ -54,6 +54,9 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > > { "ov13858", { > > .unitCellSize = { 1120, 1120 }, > > } }, > > + { "ov5693", { > > + .unitCellSize = { 1400, 1400 }, > > Don't have a datasheet to very, so I assume that's correct! > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> I don't see a driver for this sensor in the kernel tree yet. I assume that this patch will wait until the driver is merged? Best regards, Tomasz
Hi Tomasz, On Thu, May 27, 2021 at 04:02:06PM +0900, Tomasz Figa wrote: > On Tue, May 25, 2021 at 7:37 PM Jacopo Mondi <jacopo@jmondi.org> wrote: > > On Tue, May 25, 2021 at 07:56:14AM +0200, jeanmichel.hautbois@ideasonboard.com wrote: > > > From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > > > > The new sensor database has introduced several sensors, but ov5693 is > > > missing. > > > It is used on most MS Surface tablets, add it to the database. > > > > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > --- > > > src/libcamera/camera_sensor_properties.cpp | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > > > index 6ded31dc..2a6e97f7 100644 > > > --- a/src/libcamera/camera_sensor_properties.cpp > > > +++ b/src/libcamera/camera_sensor_properties.cpp > > > @@ -54,6 +54,9 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > > > { "ov13858", { > > > .unitCellSize = { 1120, 1120 }, > > > } }, > > > + { "ov5693", { > > > + .unitCellSize = { 1400, 1400 }, > > > > Don't have a datasheet to very, so I assume that's correct! > > > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > I don't see a driver for this sensor in the kernel tree yet. I assume > that this patch will wait until the driver is merged? The driver has been developed by the linux-surface community (https://github.com/linux-surface) and is on its way to upstream, so I'm fine with supporting it in libcamera already. Overall, if there's no reason to distrust the upstreaming effort, I don't see a reason to wait until the patches have been merged upstream.
On Thu, May 27, 2021 at 5:23 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Tomasz, > > On Thu, May 27, 2021 at 04:02:06PM +0900, Tomasz Figa wrote: > > On Tue, May 25, 2021 at 7:37 PM Jacopo Mondi <jacopo@jmondi.org> wrote: > > > On Tue, May 25, 2021 at 07:56:14AM +0200, jeanmichel.hautbois@ideasonboard.com wrote: > > > > From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > > > > > > The new sensor database has introduced several sensors, but ov5693 is > > > > missing. > > > > It is used on most MS Surface tablets, add it to the database. > > > > > > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > > --- > > > > src/libcamera/camera_sensor_properties.cpp | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > > > > index 6ded31dc..2a6e97f7 100644 > > > > --- a/src/libcamera/camera_sensor_properties.cpp > > > > +++ b/src/libcamera/camera_sensor_properties.cpp > > > > @@ -54,6 +54,9 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > > > > { "ov13858", { > > > > .unitCellSize = { 1120, 1120 }, > > > > } }, > > > > + { "ov5693", { > > > > + .unitCellSize = { 1400, 1400 }, > > > > > > Don't have a datasheet to very, so I assume that's correct! > > > > > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > > I don't see a driver for this sensor in the kernel tree yet. I assume > > that this patch will wait until the driver is merged? > > The driver has been developed by the linux-surface community > (https://github.com/linux-surface) and is on its way to upstream, so I'm > fine with supporting it in libcamera already. Overall, if there's no > reason to distrust the upstreaming effort, I don't see a reason to wait > until the patches have been merged upstream. > There is a problem with this. We can't rely on stable UAPI guarantees until the driver lands upstream. Perhaps we should have something like staging in libcamera? > -- > Regards, > > Laurent Pinchart
Hi Tomasz, On Thu, May 27, 2021 at 05:46:11PM +0900, Tomasz Figa wrote: > On Thu, May 27, 2021 at 5:23 PM Laurent Pinchart wrote: > > On Thu, May 27, 2021 at 04:02:06PM +0900, Tomasz Figa wrote: > > > On Tue, May 25, 2021 at 7:37 PM Jacopo Mondi <jacopo@jmondi.org> wrote: > > > > On Tue, May 25, 2021 at 07:56:14AM +0200, jeanmichel.hautbois@ideasonboard.com wrote: > > > > > From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > > > > > > > > The new sensor database has introduced several sensors, but ov5693 is > > > > > missing. > > > > > It is used on most MS Surface tablets, add it to the database. > > > > > > > > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > > > --- > > > > > src/libcamera/camera_sensor_properties.cpp | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > > > > > index 6ded31dc..2a6e97f7 100644 > > > > > --- a/src/libcamera/camera_sensor_properties.cpp > > > > > +++ b/src/libcamera/camera_sensor_properties.cpp > > > > > @@ -54,6 +54,9 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > > > > > { "ov13858", { > > > > > .unitCellSize = { 1120, 1120 }, > > > > > } }, > > > > > + { "ov5693", { > > > > > + .unitCellSize = { 1400, 1400 }, > > > > > > > > Don't have a datasheet to very, so I assume that's correct! > > > > > > > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > > > > I don't see a driver for this sensor in the kernel tree yet. I assume > > > that this patch will wait until the driver is merged? > > > > The driver has been developed by the linux-surface community > > (https://github.com/linux-surface) and is on its way to upstream, so I'm > > fine with supporting it in libcamera already. Overall, if there's no > > reason to distrust the upstreaming effort, I don't see a reason to wait > > until the patches have been merged upstream. > > There is a problem with this. We can't rely on stable UAPI guarantees > until the driver lands upstream. Perhaps we should have something like > staging in libcamera? We've had ABI breakages with rkisp1 when it was getting destaged. It's not nice, but given that we haven't frozen any ABI for libcamera yet, I don't think it's a blocker at the moment. How do you handle this issue in Chrome OS ? :-)
On Thu, May 27, 2021 at 5:51 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Tomasz, > > On Thu, May 27, 2021 at 05:46:11PM +0900, Tomasz Figa wrote: > > On Thu, May 27, 2021 at 5:23 PM Laurent Pinchart wrote: > > > On Thu, May 27, 2021 at 04:02:06PM +0900, Tomasz Figa wrote: > > > > On Tue, May 25, 2021 at 7:37 PM Jacopo Mondi <jacopo@jmondi.org> wrote: > > > > > On Tue, May 25, 2021 at 07:56:14AM +0200, jeanmichel.hautbois@ideasonboard.com wrote: > > > > > > From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > > > > > > > > > > The new sensor database has introduced several sensors, but ov5693 is > > > > > > missing. > > > > > > It is used on most MS Surface tablets, add it to the database. > > > > > > > > > > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > > > > --- > > > > > > src/libcamera/camera_sensor_properties.cpp | 3 +++ > > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > > > > > > index 6ded31dc..2a6e97f7 100644 > > > > > > --- a/src/libcamera/camera_sensor_properties.cpp > > > > > > +++ b/src/libcamera/camera_sensor_properties.cpp > > > > > > @@ -54,6 +54,9 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > > > > > > { "ov13858", { > > > > > > .unitCellSize = { 1120, 1120 }, > > > > > > } }, > > > > > > + { "ov5693", { > > > > > > + .unitCellSize = { 1400, 1400 }, > > > > > > > > > > Don't have a datasheet to very, so I assume that's correct! > > > > > > > > > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > > > > > > I don't see a driver for this sensor in the kernel tree yet. I assume > > > > that this patch will wait until the driver is merged? > > > > > > The driver has been developed by the linux-surface community > > > (https://github.com/linux-surface) and is on its way to upstream, so I'm > > > fine with supporting it in libcamera already. Overall, if there's no > > > reason to distrust the upstreaming effort, I don't see a reason to wait > > > until the patches have been merged upstream. > > > > There is a problem with this. We can't rely on stable UAPI guarantees > > until the driver lands upstream. Perhaps we should have something like > > staging in libcamera? > > We've had ABI breakages with rkisp1 when it was getting destaged. It's > not nice, but given that we haven't frozen any ABI for libcamera yet, I > don't think it's a blocker at the moment. > > How do you handle this issue in Chrome OS ? :-) We carry downstream patches. :)
Hi Tomasz, On Thu, May 27, 2021 at 05:52:34PM +0900, Tomasz Figa wrote: > On Thu, May 27, 2021 at 5:51 PM Laurent Pinchart wrote: > > On Thu, May 27, 2021 at 05:46:11PM +0900, Tomasz Figa wrote: > > > On Thu, May 27, 2021 at 5:23 PM Laurent Pinchart wrote: > > > > On Thu, May 27, 2021 at 04:02:06PM +0900, Tomasz Figa wrote: > > > > > On Tue, May 25, 2021 at 7:37 PM Jacopo Mondi wrote: > > > > > > On Tue, May 25, 2021 at 07:56:14AM +0200, jeanmichel.hautbois@ideasonboard.com wrote: > > > > > > > From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > > > > > > > > > > > > The new sensor database has introduced several sensors, but ov5693 is > > > > > > > missing. > > > > > > > It is used on most MS Surface tablets, add it to the database. > > > > > > > > > > > > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > > > > > --- > > > > > > > src/libcamera/camera_sensor_properties.cpp | 3 +++ > > > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > > > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > > > > > > > index 6ded31dc..2a6e97f7 100644 > > > > > > > --- a/src/libcamera/camera_sensor_properties.cpp > > > > > > > +++ b/src/libcamera/camera_sensor_properties.cpp > > > > > > > @@ -54,6 +54,9 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > > > > > > > { "ov13858", { > > > > > > > .unitCellSize = { 1120, 1120 }, > > > > > > > } }, > > > > > > > + { "ov5693", { > > > > > > > + .unitCellSize = { 1400, 1400 }, > > > > > > > > > > > > Don't have a datasheet to very, so I assume that's correct! > > > > > > > > > > > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > > > > > > > > I don't see a driver for this sensor in the kernel tree yet. I assume > > > > > that this patch will wait until the driver is merged? > > > > > > > > The driver has been developed by the linux-surface community > > > > (https://github.com/linux-surface) and is on its way to upstream, so I'm > > > > fine with supporting it in libcamera already. Overall, if there's no > > > > reason to distrust the upstreaming effort, I don't see a reason to wait > > > > until the patches have been merged upstream. > > > > > > There is a problem with this. We can't rely on stable UAPI guarantees > > > until the driver lands upstream. Perhaps we should have something like > > > staging in libcamera? > > > > We've had ABI breakages with rkisp1 when it was getting destaged. It's > > not nice, but given that we haven't frozen any ABI for libcamera yet, I > > don't think it's a blocker at the moment. > > > > How do you handle this issue in Chrome OS ? :-) > > We carry downstream patches. :) I dread to think of the day we'll have to handle downstream libcamera versions :-)
diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp index 6ded31dc..2a6e97f7 100644 --- a/src/libcamera/camera_sensor_properties.cpp +++ b/src/libcamera/camera_sensor_properties.cpp @@ -54,6 +54,9 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { "ov13858", { .unitCellSize = { 1120, 1120 }, } }, + { "ov5693", { + .unitCellSize = { 1400, 1400 }, + } }, }; const auto it = sensorProps.find(sensor);