| Message ID | 20210621133449.80586-1-vedantparanjape160201@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Vedant, Thank you for the patch. On Mon, Jun 21, 2021 at 07:04:49PM +0530, Vedant Paranjape wrote: > Brief specifications available at > https://cdn.sparkfun.com/datasheets/Dev/RaspberryPi/ov5647_full.pdf > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > --- > src/libcamera/camera_sensor_properties.cpp | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > index f660743a..43030e8b 100644 > --- a/src/libcamera/camera_sensor_properties.cpp > +++ b/src/libcamera/camera_sensor_properties.cpp > @@ -81,6 +81,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { 1, controls::draft::TestPatternModeColorBars }, > }, > } }, > + { "ov5647", { > + .unitCellSize = { 1400, 1400 }, > + /* \todo fill test pattern modes for ov5647. */ > + .testPatternModes = {}, How about filling them ? :-) > + } }, > { "ov5693", { > .unitCellSize = { 1400, 1400 }, > .testPatternModes = {
Hi Vedant, On 21/06/2021 14:34, Vedant Paranjape wrote: > Brief specifications available at > https://cdn.sparkfun.com/datasheets/Dev/RaspberryPi/ov5647_full.pdf > I presume this comes from the features on page 5 stating """ 1.4 μm x 1.4 μm pixel with OmniBSI technology for high performance (high sensitivity, low crosstalk, low noise """ It might be nice to reference that specifically rather than the whole document, so readers can quickly and easily find the relevant source. > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > --- > src/libcamera/camera_sensor_properties.cpp | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > index f660743a..43030e8b 100644 > --- a/src/libcamera/camera_sensor_properties.cpp > +++ b/src/libcamera/camera_sensor_properties.cpp > @@ -81,6 +81,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { 1, controls::draft::TestPatternModeColorBars }, > }, > } }, > + { "ov5647", { > + .unitCellSize = { 1400, 1400 }, > + /* \todo fill test pattern modes for ov5647. */ It seems this todo is quite involved, as it will require updating the kernel driver to actually have test patterns :-S I can't see any available at: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/i2c/ov5647.c So the todo is likely sufficient, but I'd be more explicit saying there are no current supported test modes in the kernel. Other than those comments, Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + .testPatternModes = {}, > + } }, > { "ov5693", { > .unitCellSize = { 1400, 1400 }, > .testPatternModes = { >
On Mon, Jun 21, 2021 at 02:44:56PM +0100, Kieran Bingham wrote: > Hi Vedant, > > On 21/06/2021 14:34, Vedant Paranjape wrote: > > Brief specifications available at > > https://cdn.sparkfun.com/datasheets/Dev/RaspberryPi/ov5647_full.pdf > > > > I presume this comes from the features on page 5 stating > > """ > 1.4 μm x 1.4 μm pixel with OmniBSI technology for > high performance (high sensitivity, low crosstalk, low > noise > """ > > It might be nice to reference that specifically rather than the whole > document, so readers can quickly and easily find the relevant source. > > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > > --- > > src/libcamera/camera_sensor_properties.cpp | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > > index f660743a..43030e8b 100644 > > --- a/src/libcamera/camera_sensor_properties.cpp > > +++ b/src/libcamera/camera_sensor_properties.cpp > > @@ -81,6 +81,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > > { 1, controls::draft::TestPatternModeColorBars }, > > }, > > } }, > > + { "ov5647", { > > + .unitCellSize = { 1400, 1400 }, > > + /* \todo fill test pattern modes for ov5647. */ > > It seems this todo is quite involved, as it will require updating the > kernel driver to actually have test patterns :-S > > I can't see any available at: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/i2c/ov5647.c > > So the todo is likely sufficient, but I'd be more explicit saying there > are no current supported test modes in the kernel. I'd say there's no \todo needed in that case, if the driver doesn't support test patterns, the implementation here is enough. > Other than those comments, > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > + .testPatternModes = {}, > > + } }, > > { "ov5693", { > > .unitCellSize = { 1400, 1400 }, > > .testPatternModes = { > >
Hi Laurent and Kieran, Sent a v2 patch with requested changes. Regards, *Vedant Paranjape* On Mon, Jun 21, 2021 at 7:18 PM Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > On Mon, Jun 21, 2021 at 02:44:56PM +0100, Kieran Bingham wrote: > > Hi Vedant, > > > > On 21/06/2021 14:34, Vedant Paranjape wrote: > > > Brief specifications available at > > > https://cdn.sparkfun.com/datasheets/Dev/RaspberryPi/ov5647_full.pdf > > > > > > > I presume this comes from the features on page 5 stating > > > > """ > > 1.4 μm x 1.4 μm pixel with OmniBSI technology for > > high performance (high sensitivity, low crosstalk, low > > noise > > """ > > > > It might be nice to reference that specifically rather than the whole > > document, so readers can quickly and easily find the relevant source. > > > > > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > > > --- > > > src/libcamera/camera_sensor_properties.cpp | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/src/libcamera/camera_sensor_properties.cpp > b/src/libcamera/camera_sensor_properties.cpp > > > index f660743a..43030e8b 100644 > > > --- a/src/libcamera/camera_sensor_properties.cpp > > > +++ b/src/libcamera/camera_sensor_properties.cpp > > > @@ -81,6 +81,11 @@ const CameraSensorProperties > *CameraSensorProperties::get(const std::string &sen > > > { 1, > controls::draft::TestPatternModeColorBars }, > > > }, > > > } }, > > > + { "ov5647", { > > > + .unitCellSize = { 1400, 1400 }, > > > + /* \todo fill test pattern modes for ov5647. */ > > > > It seems this todo is quite involved, as it will require updating the > > kernel driver to actually have test patterns :-S > > > > I can't see any available at: > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/i2c/ov5647.c > > > > So the todo is likely sufficient, but I'd be more explicit saying there > > are no current supported test modes in the kernel. > > I'd say there's no \todo needed in that case, if the driver doesn't > support test patterns, the implementation here is enough. > > > Other than those comments, > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > + .testPatternModes = {}, > > > + } }, > > > { "ov5693", { > > > .unitCellSize = { 1400, 1400 }, > > > .testPatternModes = { > > > > > -- > Regards, > > Laurent Pinchart >
diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp index f660743a..43030e8b 100644 --- a/src/libcamera/camera_sensor_properties.cpp +++ b/src/libcamera/camera_sensor_properties.cpp @@ -81,6 +81,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { 1, controls::draft::TestPatternModeColorBars }, }, } }, + { "ov5647", { + .unitCellSize = { 1400, 1400 }, + /* \todo fill test pattern modes for ov5647. */ + .testPatternModes = {}, + } }, { "ov5693", { .unitCellSize = { 1400, 1400 }, .testPatternModes = {
Brief specifications available at https://cdn.sparkfun.com/datasheets/Dev/RaspberryPi/ov5647_full.pdf Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> --- src/libcamera/camera_sensor_properties.cpp | 5 +++++ 1 file changed, 5 insertions(+)